test: run remaining categories live and unify shielded-key fixtures#694
Open
0xisk wants to merge 10 commits into
Open
test: run remaining categories live and unify shielded-key fixtures#6940xisk wants to merge 10 commits into
0xisk wants to merge 10 commits into
Conversation
3 tasks
Contributor
WalkthroughThe PR consolidates shielded test-key fixtures, expands automatic live-category discovery, excludes witness suites from live runs, and updates token, access, and multisig tests for backend-specific execution. ChangesLive backend test expansion
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant LiveRunner
participant TokenTests
participant shieldedTestKey
participant LiveBackend
LiveRunner->>TokenTests: discover and execute live category
TokenTests->>shieldedTestKey: resolve recipient and nonce inputs
shieldedTestKey-->>TokenTests: return backend-aware key
TokenTests->>LiveBackend: mint real coin
LiveBackend-->>TokenTests: return minted coin
TokenTests->>LiveBackend: burn minted coin
LiveBackend-->>TokenTests: return burn result
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
0xisk
force-pushed
the
test/multisig-live-harness
branch
from
July 17, 2026 16:14
a45bda7 to
7f6e09e
Compare
Thread the token specs through the backend-aware harness so they run under MIDNIGHT_BACKEND=live against a real node, not only the dry simulator. Coin-flow assertions split into un-gated blocks (guards that revert before any coin reaches Zswap) and backend-aware blocks: * recipients resolve to the deployer wallet's own coin public key on live (a key the node can resolve as a mint / refund target) and to the synthetic test user on dry; * mint nonces are random per coin on live, since the local node persists nullifiers across runs and a fixed nonce replays a spent coin (Custom error 103); dry keeps the seeded nonce for reproducible assertions. Covers NativeShieldedToken, NativeShieldedTokenCore, NativeShieldedTokenFamily, and the supply property spec.
0xisk
force-pushed
the
test/live-harness-remaining-categories
branch
from
July 20, 2026 08:42
a2cc06d to
63ba3df
Compare
ZOwnablePK derives its owner id from SHA256(ownPublicKey(), nonce), so the spec needs the caller's own bare ZswapCoinPublicKey rather than an Either recipient. Add a shieldedTestSignerPK(alias) fixture helper: on live it returns the pooled wallet's own coin public key (published as MIDNIGHT_<ALIAS>_COIN_PK), so the off-chain owner commitment matches the on-chain one the node computes for that signer; on dry it is the deterministic synthetic key the dry backend resolves .as(alias) to. Retarget the ZOwnablePK owner/new-owner ids through the helper so the spec runs on both backends.
Now that access, token, and security carry backend-aware specs (and crypto / utils run their pure computation through a real deploy), drop the LIVE_READY allowlist from the live runner: every src/<category> with tests runs live, and a category that must not is an explicit opt-out (EXCLUDED_CATEGORIES, only legacy archive today). `--list` prints every live category, keeping the CI matrix in sync with one source of truth. Add a LIVE_EXCLUDE to the unit-live vitest project that also drops test/witnesses/** — those specs assert on wit_* helpers directly with a fabricated WitnessContext, so MIDNIGHT_BACKEND=live changes nothing and running them would only burn a worker slot. The dry unit project still covers them.
Replace the "multisig is the only live-ready category" note with the current behavior: every src/<category> with tests runs live, there is no separate live-ready allowlist, and a category is kept off live only via an explicit EXCLUDED_CATEGORIES opt-out.
The three backend-aware key helpers (shieldedTestRecipient,
shieldedTestParentKey, shieldedTestSigner) shared one env-lookup body
and differed only on which wallet they read and whether they returned
the Either or its bare `.left`. Collapse them into a single
`shieldedTestKey(alias = 'deployer')` returning an EncodedRecipient:
* `shieldedTestKey()` — the deployer's key (send recipient / drain
parent), the former shieldedTestRecipient / shieldedTestParentKey.
* `shieldedTestKey('SIGNER1')` — a named pooled signer, the former
shieldedTestSigner.
* callers needing a bare coin public key read `.left`, typed
EncodedCoinPublicKey from compact-runtime, not a hand-rolled
`{ bytes: Uint8Array }`.
Migrate every consumer (token, access, multisig, forwarder, treasury).
The only behavioral change is on the dry backend: recipient / parent
synthetic keys now derive from the `deployer` alias rather than a
cosmetic label. No spec depends on those exact bytes; the full dry
unit suite passes (1399 tests).
0xisk
changed the title
0xisk
force-pushed
the
test/live-harness-remaining-categories
branch
from
July 20, 2026 09:26
63ba3df to
acc6cde
Compare
0xisk
marked this pull request as ready for review
July 20, 2026 09:27
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
contracts/test-utils/fixtures/shieldedKey.ts (1)
26-27: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
coinPkEnvVaronly uppercases the'deployer'alias.For any other alias, the raw string is interpolated verbatim:
coinPkEnvVar('signer1')→MIDNIGHT_signer1_COIN_PK(won't match the harness-publishedMIDNIGHT_SIGNER1_COIN_PK), and evencoinPkEnvVar('Deployer')misses the=== 'deployer'check and producesMIDNIGHT_Deployer_COIN_PK. SinceshieldedTestKeyfalls back to the dry synthetic key whenever the env lookup misses, a caller alias with unexpected casing silently gets a dry key on the live backend instead of erroring — hard to debug.Uppercasing uniformly removes the special case and the asymmetry:
🔧 Proposed fix
-const coinPkEnvVar = (alias: string): string => - `MIDNIGHT_${alias === 'deployer' ? 'DEPLOYER' : alias}_COIN_PK`; +const coinPkEnvVar = (alias: string): string => + `MIDNIGHT_${alias.toUpperCase()}_COIN_PK`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/test-utils/fixtures/shieldedKey.ts` around lines 26 - 27, Update coinPkEnvVar to uppercase every alias uniformly when constructing the MIDNIGHT_*_COIN_PK environment-variable name, removing the special-case deployer branch. Preserve the existing prefix and suffix so aliases such as signer1 and Deployer resolve to their uppercase harness-published keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contracts/src/token/test/NativeShieldedToken.test.ts`:
- Around line 295-315: Gate the live _burnFromSelf tests as skipped, todo, or
expected-failure until they use a contract-held coin and recover its real
mt_index from ledger events. Apply this to the two tests in
contracts/src/token/test/NativeShieldedToken.test.ts lines 295-315, and the
corresponding live _burnFromSelf tests in
contracts/src/token/test/NativeShieldedTokenCore.test.ts lines 348-388 and
contracts/src/token/test/NativeShieldedTokenFamily.test.ts lines 291-331; no
other test behavior requires modification.
In `@scripts/test-live.ts`:
- Around line 334-340: Update the argument validation around liveCategories,
scoped, and categories to fail fast when the first argument looks like a
category name but is not an active live category, including excluded archive and
unknown category names. Report the invalid category before invoking expensive
setup such as compileVerified, env-up, or the harness, while preserving
filename-filter handling for valid category arguments and the existing
no-argument behavior.
---
Nitpick comments:
In `@contracts/test-utils/fixtures/shieldedKey.ts`:
- Around line 26-27: Update coinPkEnvVar to uppercase every alias uniformly when
constructing the MIDNIGHT_*_COIN_PK environment-variable name, removing the
special-case deployer branch. Preserve the existing prefix and suffix so aliases
such as signer1 and Deployer resolve to their uppercase harness-published keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5ee178b-fd8c-4cba-9e09-96b14c400325
📒 Files selected for processing (18)
CONTRIBUTING.mdcontracts/src/access/test/ZOwnablePK.test.tscontracts/src/multisig/test/Forwarder.test.tscontracts/src/multisig/test/ForwarderPrivate.test.tscontracts/src/multisig/test/ShieldedMultiSig.test.tscontracts/src/multisig/test/ShieldedMultiSigV3.test.tscontracts/src/multisig/test/ShieldedTreasury.test.tscontracts/src/multisig/test/ShieldedTreasuryStateless.test.tscontracts/src/multisig/test/presets/ForwarderPrivate.test.tscontracts/src/multisig/test/presets/ForwarderShielded.test.tscontracts/src/token/test/NativeShieldedToken.test.tscontracts/src/token/test/NativeShieldedTokenCore.test.tscontracts/src/token/test/NativeShieldedTokenFamily.test.tscontracts/src/token/test/extensions/NativeShieldedTokenSupply.property.test.tscontracts/test-utils/fixtures/shieldedKey.tscontracts/test-utils/fixtures/test/shieldedKey.test.tscontracts/vitest.config.tsscripts/test-live.ts
The live `_burnFromSelf` happy-path blocks mint to the deployer wallet (not the contract) and spend with a placeholder `mt_index: 0n`, so under `runIf(isLiveBackend())` they revert deterministically against a real node. Mark them `describe.skip` until the coin is minted to the contract and its real `mt_index` is recovered from the zswap ledger-events stream. The dry fabricated-coin coverage is unchanged.
A first positional arg that is not an active live category — an excluded one like `archive`, or a typo — silently degraded into a full-suite file filter: the run attempted every category with the bad name as a vitest filter, contrary to the documented contract (first arg names the category) and only discovered after the expensive compile / env-up / harness setup. Fail fast with the valid set before that setup.
`coinPkEnv` (harness publisher) and the inlined `coinPkEnvVar` (`shieldedKey` reader) both special-cased only `deployer`, interpolating any other-cased alias verbatim. A `signer1` (or `Deployer`) alias would publish and look up different env-var names, silently resolving a live alias to a dry synthetic key. Uppercase uniformly in both so publication and lookup always agree; keep the two mappings in lockstep.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Builds on #673 (the live-backend harness) to run the remaining contract categories on the live node, and consolidates the shared shielded-key test fixtures.
Blocked
Live coverage
token— the shielded-token specs (NativeShieldedToken,Core,Family, and the supply property spec) run backend-aware: node-resolvable recipients and per-run mint nonces on live, dry assertions preserved.access—ZOwnablePKruns live, binding its owner ids to pooled wallets so the negative "not the owner" checks stay honest on a real node.LIVE_READYallowlist: everysrc/<category>with tests runs live (opt-out viaEXCLUDED_CATEGORIES, only legacyarchivetoday), and--listreports the full set so CI stays in sync.Fixture refactor
shieldedTestRecipient,shieldedTestParentKey,shieldedTestSigner) into oneshieldedTestKey(alias = 'deployer')returning anEncodedRecipient..left, typedEncodedCoinPublicKeyfrom@midnight-ntwrk/compact-runtime(no hand-rolled{ bytes: Uint8Array }).Types of changes
Closes #668 Closes #669 Closes #670 Closes #671 Closes #672
PR Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation