Repository navigation
Hedge every ML-DSA signature and add --keep-alive to send and multisig transfer - #181
Conversation
…g transfer Deterministic ML-DSA plus a nonce reset by reaping re-signs a byte-identical extrinsic: same hash, and the first one becomes replayable once the account is re-funded. Every signature the CLI makes on the host (extrinsics and airdrop claims) now mixes in 32 bytes of OS randomness; an RNG failure is a hard error. `send` and `multisig propose transfer` keep transfer_allow_death as their default and gain an opt-in --keep-alive that proposes transfer_keep_alive.
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT-6.1 Sol
Verdict (advisory): Approve
Reviewed head fc9842f582dde08de87947bf52d1f49b75d07a4b against base 413a6e23f7fc9ebbe8145d9cd887cd52a70cc933. No blocking findings. Both ML-DSA schemes retain their runtime-specific context and verification behavior; host signing paths, including the claim signer, receive fresh OS randomness. The shared transfer helper preserves the default call and selects transfer_keep_alive when requested. High-security multisig proposals reject the flag before submission.
Nonblocking hardening note (P3): At src/chain/signing.rs:39, returning the named SensitiveBytes32 local by move can leave a copy in the helper's vacated stack slot. A standalone release-mode probe with a forced helper call boundary found one surviving 32-byte copy in all 10 runs after wiping the returned wrapper. Returning core::mem::replace(&mut hedge, SensitiveBytes32::zeroed()) cleared that copy in all 10 control runs; filling a caller-owned wrapper would also avoid this move. An additional optimized ML-DSA-87 signing-path probe cleared the marker bytes on this host, so this is optional cleanup hardening. Consider adding the same optimized stack-residue regression used by the crypto dependency.
Replay scope: Hedging gives freshly signed extrinsics different hashes. An already signed extrinsic can still be replayed after a nonce reset while its mortal era remains valid; hedging does not invalidate that signature. The optional keep-alive call prevents transfer-induced reaping. Please keep this distinction clear when describing the fix.
Validation passed:
git diff --check,taplo format --check --config taplo.toml, andcargo +nightly-2026-08-31 fmt --all -- --check.SKIP_CIRCUIT_BUILD=1 cargo clippy --all-targets --locked -- -D warnings.- 85 focused library tests across signing, send, cold signing, multisig, runtime, airdrop, and NEAR signing/cold-response handling, using
SKIP_CIRCUIT_BUILD=1 cargo test --locked --lib <module>.
GitHub's Ubuntu build/tests, analysis/docs, formatting, security audit, and dependency checks have passed. The macOS build/tests and examples jobs remain in progress. Local validation skipped circuit generation and did not submit live transactions.
#181 added --keep-alive to send for transfer_keep_alive; one flag now serves both a fixed amount and --all, where it is transfer_all's keep_alive.
…lity at the head ExecutionMode::install, a process-wide flag only main set, becomes ExecutionMode::scope: a task-local that execute_command enters, so a library call to the dispatcher reads at the block its mode names and concurrent callers never overwrite each other's choice. capture_tx_context anchors the 256-block mortal era at the head even under --finalized, like the nonce: anchored ~100 blocks back the cold signer would have ~156 blocks to answer, none once finality lags by 256. handle_send_command keeps its fixed-amount signature; send --all goes through the new handle_send_all_command. Both take the one --keep-alive flag from #181, which for a sweep is transfer_all's keep_alive.
…--all (#169) * fix: read at the head by default, add a global --finalized, add send --all Sending 1.99 QTC from an account holding exactly 2 QTC was refused right after mainnet enacted spec 153: Insufficient balance for send. Have: 2 QTC, Need: 2.000167025 QTC (estimated fee: 0.010167025 QTC) subxt's partial_fee_estimate calls TransactionPaymentApi_query_info at latest_finalized_block_ref. QPoW finality trails the head by ~100 blocks, so the estimate was computed by spec 152 -- the runtime the upgrade had just replaced -- and 153 cut FEE_SCALE tenfold. The real fee was ~0.001 QTC. The transfer was affordable; only the quote was stale. Estimate against the same block as every other read, via QuantusClient::partial_fee. Unify the finality choice. ExecutionMode::finalized already decided how long to wait for a transaction; it now also decides which block reads are taken at. Reads happen in ~80 places that have no reason to carry an ExecutionMode, so main publishes the flag once with ExecutionMode::install and QuantusClient::get_latest_block consults it -- one switch for waits and reads. wormhole_tip_block's own head/finalized branch and at_finalized_block go away, since get_latest_block already answers that question. --finalized-tx becomes --finalized (kept as an alias) because it no longer only governs transactions. Add send --all, which submits Balances::transfer_all and lets the chain deduct the exact fee. Sweeping an account by subtracting an estimate cannot be done reliably -- too low strands dust, too high is refused -- and --keep-alive chooses between reaping the account and leaving the existential deposit. * fix: address review on the global --finalized switch - Wormhole proof tips follow the ExecutionMode the flow is handed (QuantusClient::head_or_finalized). Library callers never install the switch, and the proof block must match the wait. - system --runtime reads the runtime version and header at one selected block, so --finalized moves both. - send reports the fee from the included transaction's MiningRewards::FeesCollected. A balance delta cannot separate it from what --all moved, and TransactionFeePaid omits the refund a reaped account forfeits. - The signing nonce is always read at the head. Under --finalized the finalized nonce made the extrinsic outdated whenever the account had anything unfinalized. * fix: keep the SDK block helpers and the manual-nonce submit signature - Restore at_finalized_block and its lib.rs re-export, and pin at_best_block to the head. Both fetch the block they name via head_or_finalized, whatever --finalized installed. - submit_transaction_with_nonce returns the hash again. send uses the new submit_transaction_with_nonce_and_inclusion_block, mirroring submit_transaction and submit_transaction_with_inclusion_block. * fix: scope --finalized per task, keep the send API, anchor cold mortality at the head ExecutionMode::install, a process-wide flag only main set, becomes ExecutionMode::scope: a task-local that execute_command enters, so a library call to the dispatcher reads at the block its mode names and concurrent callers never overwrite each other's choice. capture_tx_context anchors the 256-block mortal era at the head even under --finalized, like the nonce: anchored ~100 blocks back the cold signer would have ~156 blocks to answer, none once finality lags by 256. handle_send_command keeps its fixed-amount signature; send --all goes through the new handle_send_all_command. Both take the one --keep-alive flag from #181, which for a sweep is transfer_all's keep_alive. * fix: align exercise waits with its reads, keep --finalized-tx after events ExerciseCtx::wait_mode hardcoded best-block inclusion while its balance checks follow the scoped read block. Under --finalized, setup returned from the inclusion wait, read the old finalized balances and aborted on a zero funding delta. Waits now go to the finality reads are taken at, via ExecutionMode::wait_at_read_finality. events has its own --finalized, which shadows the global flag and its finalized-tx alias. The local flag carries the alias too, so `events --finalized-tx` parses again. events also read the header for the reported block number at the best block while the hash followed the scoped mode. Finalized and latest now share one branch that reads the header at the chosen hash. * fix(exercise): refuse --finalized instead of following it Following the finalized block broke exercise's short-lived proposals. Multisig and treasury proposals expire 3 to 100 blocks past the block read, and each step waits on the last, so finality's ~100-block lag put those deadlines in the past after setup had already moved funds. Exercise goes back to head reads and inclusion waits, and handle_exercise_command refuses a finalized scope up front rather than ignore the flag.
Why
An integrator hit duplicate extrinsic hashes on Quantus: an account sent its whole balance, was reaped (nonce reset to 0), was re-funded, and sent the same amount again. With deterministic ML-DSA signing the second extrinsic was byte-identical to the first, so it had the same hash, and the first one became replayable after the re-fund. This is the CLI half of the fix; the JS package half is Quantus-Network/quantus-wasm#11.
What changes
chain::signing::hedge()draws 32 bytes fromrand::rngs::OsRng(already a dependency) and every ML-DSA signature the CLI makes on the host passes it as the FIPS 204 hedge: the extrinsic signer (sign_ml_dsa_87/sign_ml_dsa_65, whichQuantusSigner::signand the cold-sign simulator go through) and the airdrop claim signature. Two signings of one payload now differ in the signature bytes only, so no two signed extrinsics share a hash. An RNG failure is a hard error (panic with the OS error), never a fallback to deterministic. Hedging also closes the known-mask fault attacks on deterministic ML-DSA.--keep-aliveonsendandmultisig propose transfer. The default is unchanged (transfer_allow_death, call index 0);--keep-alivebuildstransfer_keep_alive(call index 3), so the transfer fails rather than reaping the sender. Both commands pick the call through one helper (send::build_transfer_call_for_account_id); the multisig proposal reads the pallet/call name from that static payload instead of a string literal.--keep-aliveon a high-security multisig is an error, since HS transfers go throughReversibleTransfers::schedule_transfer.chain::test_metadata().Signatures made by external cold devices are only verified by the CLI and are untouched.
Tests
two_signings_of_the_same_payload_differ_and_both_verify(ML-DSA-87 and ML-DSA-65).pre_148_runtimes_verify_the_contextless_signaturereplaces the byte-equality check againstsp_core::Pair::sign, which hedging makes impossible; it now verifies throughPair::verify, which is what pre-148 runtimes do.send_transfers_allow_death_unless_keep_alive: encodes both calls against the checked-in metadata and asserts call index 0 by default, 3 with--keep-alive, and that every other byte is identical.Run locally (all green):
Not in this PR
.mortal(256).transfer,transfer_with_nonce,batch_transferandpropose_transferkeep their signatures and theirtransfer_allow_deathbehaviour; only the two CLI commands gained the flag.propose_transfer(library only,#[allow(dead_code)]) still hand-encodes the call with a hardcoded Balances pallet index of 5, while the generated bindings and checked-in metadata put Balances at index 2. Worth a separate look.