V12 ZK-E2E

[illuzen]

Security review: wormhole, settlement, and metering fixes

This PR addresses a batch of security-review findings against the V12 wormhole / ZK-tree
stack. Each item was first verified against the code, reproduced with a red test where
applicable, and fixed at the root cause; findings that turned out to be intended design or
operator-trusted boundaries were documented where the next reviewer will look instead.
Findings touching the turnstile (PotentialWormholeBalance reveal/ambiguity machinery) were
skipped here — the turnstile is removed in a separate PR.

Fixes

Proof recording completeness

• Exit credits pinned against double recording (903c82e3) — Verified the reported
  double-record (extension event scan + process_exit_bundle) is not reachable: bare
  extrinsics never run post_dispatch, and increase_balance emits no Minted event.
  Removed the dead bare_post_dispatch override that suggested otherwise and added
  regression tests pinning both facts.
• Hold-transfers recorded (0e7f4b6f) — Reversible-transfer guardian seizures and
  recoveries move value with transfer_on_hold, which emits Balances::TransferOnHold
  rather than Transfer; the recorder ignored it, so the guardian's credit got no ZK-tree
  leaf. The event matcher now records it like any transfer.
• Reserve repatriations recorded (1cbc1b4b) — Same class:
  recovery::close_recovery seizes the rescuer's deposit via repatriate_reserved
  (Balances::ReserveRepatriated), which the recorder also ignored. A sweep of all other
  balance-crediting APIs found no further gaps (referenda slashes burn to (), fee flows
  are excluded by design, TransferAndHold is unused).
• Zero-amount credits dropped (14d7fbe0) — Zero-value transfers (reachable from
  plain transfer_keep_alive(0) and scheduled transfers) created pure-state-growth leaves.
  record_transfer_proof now drops zero-amount credits at the single chokepoint, and
  pallet-reversible-transfers rejects zero-amount schedules outright.

Genesis

• Genesis proofs derived from balances (86612f34) — The wormhole pallet's
  independent endowed_addresses genesis vector could disagree with pallet_balances
  (duplicate, oversized, or missing entries), creating exit capacity for value never
  issued. The vector is gone; the pallet now derives its block-1 transfer proofs directly
  from the actual balances genesis, making mismatch unrepresentable.

Settlement admission

• Aggregator rebate address binding pinned (ca9da896) — The rebate to the
  public-batch aggregator is intended design (a portion of the volume fee), but the
  aggregator address binding is now pinned by a pre-dispatch regression test against
  redirection.
• Settlement proof bytes bounded and canonicalized (bc66b510) — proof_bytes
  reached to_vec() + plonky2 from_bytes unbounded on the fee-free unsigned path, and
  the parser silently ignores trailing bytes, giving one proof unboundedly many byte
  representations that each re-cost a full copy + parse at pool admission. Pre-validation
  now gates length at MAX_PROOF_BYTES (512 KiB vs ~151/224 KB real proofs) before any
  copy, and requires the bytes to round-trip through to_bytes() so each proof has exactly
  one accepted encoding. Release-mode recalibration confirmed the existing pre-validation
  weight constants still cover the added serialize pass.

RPC

• zk-tree proof RPC requires canonical hashes (cd045424) — resolve_proof_block
  accepted any backend-resolvable hash, so side-fork blocks (and fork blocks above best,
  where the one-sided window check saturates to zero) produced proof material that
  settlement always rejects against frame_system::BlockHash. The resolver now rejects
  heights above best and requires the requested hash to equal the canonical hash at its
  height.

Weight / metering

• ZK-tree Poseidon hashing charged in leaf-recording weights (80802efa) — Wormhole
  settlement, the proof-recorder extension, and reversible-transfer execution all charged
  the leaf insert's DB ops but not its depth-proportional Poseidon hashing. A new
  insert_leaf_hash_ref_time() helper on pallet-zk-tree is now charged on all three
  routes, tracking the live tree depth.
• High-security policy reads charged (50c5f0b1) — is_call_allowed hides an
  is_high_security storage read; as_derivative and as_recovered enforced the policy
  without charging it, and Multisig::propose paid it twice while documenting one read.
  The policy predicate is split into is_call_allowed_given so propose reuses its
  fetched classification, and both wrapper weights now charge the read.
• Post-dispatch event scan metered (c35ec302) — The recorder's event scan
  stream-decodes every event record present at scan time (Iterator::skip still decodes
  the prefix it discards), which was unmetered — batched remark_with_event traffic
  produced free decode work. The scan is now registered against the block
  (event_scan_weight: one Events read + a conservative 1µs/record ceiling) alongside
  the existing recording-shortfall mechanism.

Consensus

• RuntimeEnvironmentUpdated no longer deposited (9dffec26) — The QPoW header
  commits a fixed 110-byte digest window that the pre-runtime item and seal fill exactly,
  and import rejects anything larger. Upstream frame-system's 1-byte
  RuntimeEnvironmentUpdated deposit on set_code / set_heap_pages therefore made every
  environment-changing block unimportable network-wide — runtime upgrades could not land
  through normal block production. Nothing in the node stack consumes the item (clients
  detect upgrades from the :code state key), so the fork's deposits are removed and the
  no-runtime-digest-items invariant is documented on deposit_log and DIGEST_LOGS_SIZE.

Documented as accepted limitations (no code change needed)

• Circuit tree-depth limit (b570b391) — The on-chain tree may grow to depth 32 while
  the circuits fix MAX_DEPTH = 16. Documented as a deliberate "fix it when we get close"
  trade-off on MAX_TREE_DEPTH and in docs/zk-trie-architecture.md: timeline to
  exhaustion is years-to-centuries, LeafCount makes it observable far in advance, and the
  circuit-update path is spelled out.
• Genesis-builder trust model (60b2d4a1, 84ee98f8) — No input-size bound and
  panic-on-malformed-input in the genesis builder are not vulnerabilities: genesis
  construction is an operator-trusted, chain-setup-time boundary, and panicking is the
  intended FRAME error channel for BuildGenesisConfig::build. Documented in
  genesis_config_presets.rs.
• Proof-recorder coverage boundary (ab976ca9) — The extension's doc overclaimed that
  scheduler-dispatched transfers were covered; extensions never run for hook-context
  dispatch. The doc now states the real contract: transaction events are scanned,
  hook-context credits record explicitly (reversible-transfer execution, mining rewards),
  and governance-enacted calls via the scheduler are a known, accepted gap — only Root can
  reach that path, and Root can already forge leaves outright via set_storage.

Reviewed, no change

• Guardian existence at set_high_security — Rejected the proposed existence check: a
  guardian account may legitimately be funded after enrollment.
• Turnstile-related findings — Deferred to the separate turnstile-removal PR.

Test plan

• Red test written for every applicable fix before the change; green after.
• pallet-wormhole (75), pallet-reversible-transfers, pallet-utility (38),
  pallet-recovery (35), pallet-multisig (57), frame-system (76) suites pass.
• quantus-runtime lib (45) + integration (28) suites pass.
• quantus-node zk-tree RPC resolver tests pass (7).
• Release-mode pre-validation timing harness re-run after the proof-framing change;
  weight constants hold with large margin.

[n13]

Review — security fixes: wormhole, settlement, metering

Reviewed the full diff on the checked-out branch (illuzen/v12-zk-e2e), verified the load-bearing claims against the code, and ran the affected suites locally. Together with qp-zk-circuits#169 (reviewed there as well).

Verified independently

• QPoW digest window: block import rejects oversized digests at client/consensus/qpow/src/lib.rs:238 and :407, and nothing in the node stack consumes RuntimeEnvironmentUpdated. The frame-executive deposit_log (frame/executive/src/lib.rs:1018) re-deposits the block's own pre-runtime digest, so removing the runtime deposits is safe — and the fix removes a genuine "runtime upgrades can't land" liveness bug.
• Event semantics: TransferOnHold / ReserveRepatriated field names and credit direction match the vendored balances pallet (pallets/balances/src/lib.rs:418, :373) and the seizure (pallets/reversible-transfers/src/lib.rs:936) / close_recovery call sites. TransferAndHold is indeed never emitted outside balances' own tests.
• Zero-amount rejection covers both scheduling entry points via the shared do_schedule_transfer_inner (pallets/reversible-transfers/src/lib.rs:762).
• Policy-read weights: as_recovered (pallets/recovery/src/lib.rs:456) and as_derivative do call is_call_allowed → one classification read each, matching the added DbWeight::reads(1). Multisig::propose reuses the classification already fetched for the same account — sound.
• Event-scan metering: events_at_scan is captured before recording deposits new events, and equals the number of records the scan actually decodes (skip decodes the discarded prefix). Zero-amount credits returning false keeps weight reconciliation consistent.
• Proof framing: MAX_PROOF_BYTES (512 KiB) vs. real fixture sizes (~151/224 KB) leaves sane headroom, and the round-trip check pins exactly one accepted encoding per proof.

Test runs (this machine)

• pallet-zk-tree, pallet-reversible-transfers, pallet-utility, pallet-recovery, pallet-multisig, frame-system: 281 passed, 0 failed.
• pallet-wormhole --release --lib: 68 passed, 0 failed (3 ignored fixture-regen tests).
• quantus-runtime --release --lib: 33 passed, 0 failed.

Non-blocking observations

1. on_initialize(1) (pallets/wormhole/src/lib.rs): the declared weight counts account iteration + 2r/2w per recorded proof but not the per-leaf ZK-tree insert (DB ops + Poseidon hashing). Same under-counting shape as the old code, one-time hook at block 1, harmless in practice — noting for the next weight pass.
2. Genesis derivation changes the recorded set from "explicit endowed list" to "every account with a balance at block 1". For the shipped presets the sets are identical (the old config was fed the same balances list); any pallet account pre-funded outside balances would now gain a leaf keyed to an address nobody holds a secret for — dead weight, not a risk, since ZK-spending requires the account secret.
3. node/src/zktree_rpc.rs: error code 9006 now covers two distinct backend-failure modes (number resolution and canonical-hash resolution). Both are backend failures so it's arguably consistent, just slightly less precise for API consumers.
4. The per-block quadratic scan cost (each post_dispatch re-decodes every event record present) is now metered per transaction, so block-weight accounting is sound — worth keeping an eye on as event volumes grow.

Verdict

APPROVE — root-cause fixes, each pinned by a regression test, claims hold up under independent checking. No blocking issues.

[n13]

Ok Codex 5.6 also approved this (post merge) - just ran a check