Add quantus exercise chain smoke/fuzz suite#115
Conversation
Adds a scenario-based exercise command that drives every call path the CLI wraps against a live node, for pre-runtime-upgrade regression testing and CI: - Phases: reads, balances, reversible, multisig, recovery, preimage, governance, negative, seeded fuzz; opt-in wormhole and a governance-driven runtime-upgrade phase (--upgrade-wasm, requires a fast-governance node) with automatic post-upgrade re-run. - Negative scenarios pin expected rejections (incl. scheduler-disabled and removed-pallet canaries) so upgrades that loosen validation fail. - CI-friendly: --json report, nonzero exit on failure, --seed for reproducible fuzzing, --fail-fast; nightly exercise.yml workflow that builds a fast-governance node and runs the suite. - Regenerate subxt metadata for spec 134 and drop the removed Referenda/ConvictionVoting modules and disabled Scheduler calls. - Add quiet logging mode so expected-failure transaction chatter does not drown the per-step report output. Co-authored-by: Cursor <cursoragent@cursor.com>
Proof generation is fast enough on release builds; use --skip wormhole on debug builds. The post-upgrade re-run now also covers wormhole. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Remove the truncated secret prefix from check-nullifiers output and the full secret / generated mnemonic from verbose multiround logs, so no secret material reaches logs (flagged by CodeQL cleartext-logging). Co-authored-by: Cursor <cursoragent@cursor.com>
n13
left a comment
There was a problem hiding this comment.
Review — GLM 5.2
Reviewed PR #115 (feat: add quantus exercise chain smoke/fuzz suite) by checking out the branch locally, reading every changed source file, and running the toolchain.
Verification performed
cargo check --release --locked→ clean compile (48s, exit 0; only a pre-existingtrie-dbfuture-incompat note from a transitive dep)cargo clippy --release --locked→ no lints (only build-script info from ZK circuit generation)- No dangling references to the deleted
referenda/referenda_decodemodules or the removedSchedulerCommands::ScheduleRemark/submit_scheduler_*stubs - Cross-checked every reused helper signature against its definition (
transfer_with_nonce,submit_transaction_with_nonce,batch_transfer,submit_preimage,predict_multisig_address,get_multisig_info,list_proposals,vote_on_referendum,is_member, …) — all match - Confirmed
rand = "0.9"sorandom()/random_range()usage is correct - Confirmed dev wallets are encrypted with
""→ matches the wormhole phase'sSome(String::new()) - Confirmed
.github/actions/ubuntucomposite action exists;configcompat table bumped 131 → 134 matching the regenerated metadata
Strengths
- Negative scenarios pin specific expected errors (
FundsUnavailable,BadOrigin,DelayTooShort,AccountNotHighSecurity,GuardianCannotBeSelf,NotRecoverable) — exactly the right design for a pre-upgrade regression gate: an upgrade that loosens validation fails the suite instead of silently passing. - Metadata canaries (
scheduler_calls_disabled,removed_pallets_absent) assert Scheduler stays dispatch-disabled andReferenda/ConvictionVoting/Sudostay absent. Clever, low-cost regression detection. - Fuzz invariant is well-scoped: clean success or clean rejection both pass; only hangs / undecodable failures fail. Boundary amounts (0, 1, ED±1,
u128::MAX) hit the interesting edges. QuietGuardRAII restores logging on every exit path;--verbosecorrectly overrides quiet.- Security: commit
4ef3f49removed the mnemonic / secret / secret-prefix logging that CodeQL flagged. - Good reuse of existing CLI handlers (
send::transfer,multisig::*,tech_collective::*, wormhole multiround) — no parallel implementations.build_submit_callis shared between governance and upgrade scenarios (DRY). ephemeral_accounts.max(4)guaranteeseph[0..3]indexing can't panic;transfer_below_edguardsED <= 1;stale_nonceguardscurrent_nonce == 0.- Upgrade phase reconnects the client to pick up new metadata and includes the referendum state in the failure message for debugging.
- CI workflow uses
if: always()for artifact upload and node cleanup, has an RPC warmup loop, and a generous 180-min timeout.
Non-blocking notes / nits
reads.rs:19naming mismatch — thescheduler_agendastep actually callsget_last_processed_timestamp, not the agenda. Cosmetic, but misleading in the JSON report.- DRY: referendum submission is duplicated between
governance.rs::referendum_flowandupgrade.rs::governance_set_code(note preimage → submit → decision deposit → 3× vote). A sharedsubmit_referendum_with_votes(ctx, encoded, hash, len) -> Result<u32>helper would let upgrade just add polling + refund on top. Aligns with the repo's DRY rule. upgrade.rsrefund timing — the deposit refunds are submitted viactx.clientbefore the caller reconnects, i.e. with the pre-upgrade metadata. If a future upgrade changestech_referendacall indices, the refund would fail to decode. It's best-effort (doesn't fail the step), so non-blocking, but worth a one-line comment or moving the refund after reconnect.fuzz.rsclassifyheuristic is broad — any error containing"Transaction … failed"/"invalid"/"error"/"dropped"/"Failed to submit"passes. Intentional for the stated invariant (catch hangs / streams ending) and partly documented, but it means a novel undecodable error that happens to contain"error"would still pass. Worth a sentence in the doc comment calling out that trade-off.governance.rs::referendum_flowassumes bob + charlie are tech-collective members (votes with all three, asserts ≥3 ayes) butmembership_readsonly asserts alice is a member. If dev genesis ever ships only alice, the bob/charlie votes would fail at dispatch with a less obvious error. Asserting all three memberships inmembership_readswould make the dependency explicit and fail fast.- CI:
actions/checkoutofQuantus-Network/chain— the workflow'sGITHUB_TOKENis scoped to the cli repo (contents: read). Ifchainis private, that checkout will fail to authenticate. If it's public, no issue — worth confirming. - CodeQL alerts on this PR are non-issues: the "hard-coded cryptographic value" on nonce
0instale_nonceis an intentional stale nonce, not a secret; the "cleartext logging" hits are public on-chain data (addresses, nonces, balances). The one real secret-logging path was fixed in4ef3f49.
Verdict
Approve. The change compiles cleanly, passes clippy, removes dead code (referenda modules, scheduler dispatch stubs) without leaving dangling references, and the one real security finding was addressed. The design is strong — especially the pinned-error negative scenarios and the metadata canaries, which are exactly what you want around a runtime upgrade. The remaining items are nits / non-blocking notes. The only residual risk is the upgrade phase being marked not-yet-exercised end-to-end, which is clearly documented in the PR body and test plan.
n13
left a comment
There was a problem hiding this comment.
Reviewed the branch locally at 4ef3f49: cargo build --locked, cargo clippy --all-targets --locked, and cargo test --locked (210 passed) are all clean, and all 9 CI checks are green. The overall design is strong — scenarios reuse the real CLI handlers so the suite tests the actual code paths, negative scenarios pin specific expected errors, and the scheduler-disabled / removed-pallet canaries are exactly the kind of thing that catches an upgrade silently loosening validation. The referenda/scheduler removal left no dangling references, and the wormhole secret-logging cleanup addresses the CodeQL findings.
Should fix
1. The upgrade phase cannot fully pass with a single-entry compatibility table.
reads::runtime_version fails unless is_runtime_compatible(spec, tx), and COMPATIBLE_RUNTIMES pins exactly {spec: 134, tx: 2}. The upgrade phase requires the WASM's spec_version to be higher than the current one (it polls for the bump), and the post-upgrade re-run starts with post-upgrade:reads / runtime_version against that new spec — which by construction is not in the table. So a successful upgrade still records a failure and the suite exits nonzero. (Flipping the table to the new spec instead just moves the failure to the pre-upgrade pass.) The table already supports multiple entries, so the fix is either documenting "add the candidate spec to COMPATIBLE_RUNTIMES before an --upgrade-wasm run" or special-casing the check in the post-upgrade pass. As written, the first real upgrade-gate run will report red regardless of outcome — consistent with the PR's own "not yet exercised end-to-end" caveat, but worth fixing before relying on it.
2. --seed reproduction fails setup on a non-fresh chain.
Ephemeral keypairs derive deterministically from the seed, and fund_ephemeral_accounts asserts the post-funding balance is exactly 1000 tokens. Re-running with the same seed against the same chain — the exact "reproduce this fuzz failure" workflow the seed exists for — hits leftover balances from the previous run and dies in setup. Checking the balance delta instead would fix it.
Nits
args.ephemeral_accounts.max(4)silently overrides the flag. The minimum is real (scenarios indexeph[0..=3]), so either document it in the help text or reject values below 4.stale_noncepasses on any error containing"Invalid", which is broad enough to mask an unrelated rejection.- Fuzz recipients are fresh throwaway accounts 1/3 of the time, so funds steadily leak out of the ephemeral set; with a large
--fuzz-iterationsthe post-upgradebalancespass could start failing onFundsUnavailable. Fine at the default 25. exercise.ymlbuilds the CLI withSKIP_CIRCUIT_BUILD=1, so wormhole circuit generation happens at runtime inside the suite (viaensure_bins_dir). It works, but it silently inflates the wormhole phase timing on every cold CI run.- With
--json, the report is printed after the human-readable status lines and CItees both into one artifact, so anything consuming the JSON has to strip the preamble first. - The wormhole phase was promoted into the default set (at 5×5) after the stated "full suite green" run, so the first nightly is effectively its validation run.
One thing that looked like a bug but isn't: the post-upgrade governance re-run re-notes a deterministic preimage (same treasury portion → same hash), but submit_preimage treats AlreadyNoted as success, so the rerun is safe. Likewise the QuietGuard drop impl correctly restores logging on every exit path.
Verdict
Approve with minor fixes. The default nightly path (all phases except upgrade) is sound and CI-ready as-is. #1 should be addressed — in code or docs — before the upgrade phase is used as the real pre-upgrade gate, and #2 is worth fixing so --seed delivers the reproducibility it advertises. Neither blocks merging the suite itself.
There was a problem hiding this comment.
These should be fixed
-
The upgrade phase cannot fully pass with a single-entry compatibility table.
reads::runtime_version fails unless is_runtime_compatible(spec, tx), and COMPATIBLE_RUNTIMES pins exactly {spec: 134, tx: 2}. The upgrade phase requires the WASM's spec_version to be higher than the current one (it polls for the bump), and the post-upgrade re-run starts with post-upgrade:reads / runtime_version against that new spec — which by construction is not in the table. So a successful upgrade still records a failure and the suite exits nonzero. (Flipping the table to the new spec instead just moves the failure to the pre-upgrade pass.) The table already supports multiple entries, so the fix is either documenting "add the candidate spec to COMPATIBLE_RUNTIMES before an --upgrade-wasm run" or special-casing the check in the post-upgrade pass. As written, the first real upgrade-gate run will report red regardless of outcome — consistent with the PR's own "not yet exercised end-to-end" caveat, but worth fixing before relying on it. -
--seed reproduction fails setup on a non-fresh chain.
Ephemeral keypairs derive deterministically from the seed, and fund_ephemeral_accounts asserts the post-funding balance is exactly 1000 tokens. Re-running with the same seed against the same chain — the exact "reproduce this fuzz failure" workflow the seed exists for — hits leftover balances from the previous run and dies in setup. Checking the balance delta instead would fix it.
3 - How is upgrade supposed to work if the time is set to 24h voting?
Fix 2: seeded funding now asserts on the delta
@ -0,0 +1,53 @@
feat: add
quantus exercisechain smoke/fuzz suiteSummary
Adds a single command —
quantus exercise— that runs a scenario-based smoke and fuzz suite against a live node, covering every call path the CLI wraps. It is designed as a pre-runtime-upgrade regression gate: run it before and after an upgrade and any extrinsic that stops working (or any validation that silently loosens) fails the suite with a nonzero exit code, making it CI-ready.What it covers
Utility::batch, transfer with tip, manual nonce,remark_with_eventset_high_security+ HS default-delay transfer + guardian cancelinitiate_recoverypinned as cleanNotRecoverable(CLI lackscreate_recovery— documented limitation)note_preimage+ status verify;request_preimagepinned asBadOriginadd_memberpinned asBadOrigin--skip wormholeon debug buildsSystem::set_codevia TechReferenda on a fast-governance node, poll for spec bump, then automatically re-run all phases against the upgraded runtimeNegative scenarios assert the specific expected error, so a runtime upgrade that accidentally loosens validation is caught, not just crashes.
Flags
--phases/--skip,--fuzz-iterations N(default 25),--seed <u64>(reproducible fuzzing; seed is always printed),--upgrade-wasm <path>,--upgrade-timeout-secs,--ephemeral-accounts N,--fail-fast,--json.Also in this PR
Referenda/ConvictionVotingpallets removed,Schedulerdispatchables disabled. The stalereferendaCLI modules and scheduler dispatch code are removed accordingly; compatibility table updated.src/log.rs): the exercise suite suppresses per-transaction chatter (spinners, expected-failure error banners) so the per-step report is the single source of truth.--verboserestores full output. Normal CLI commands are unaffected..github/workflows/exercise.yml): buildsquantus-nodefrom the chain repo withfast-governance, starts a dev node, runs the suite with--json, uploads report + node log as artifacts. Nightly +workflow_dispatch(with chain ref and fuzz-iteration inputs) rather than per-PR, since node build + QPoW block times dominate.Test plan
cargo +nightly fmt --check,clippy --all-targets(no warnings),cargo test --locked(210 passed)--upgrade-wasm) before relying on it for the real upgradeexercise.ymlrun on CI to validate timingKnown limitations
create_recoverywrapper).