V12 audit round 2: assess all findings, fix the 32 confirmed - #647
Conversation
Confirmed findings 180560, 181329, 181347, 181395, 181418, 181327, 181394, 181396, 181393. ensure_alarm_at now schedules before cancelling; failed queue admission and evictions keep a timeout wake-up; submit/schedule_enactment no longer strand referenda or discard the approved call; cancel/kill free the track queue; nudges no longer orphan alarms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tally::support returns zero (not 100%) when the electorate is empty and clamps ayes to the member count, so a shrunken/emptied collective can no longer push a live referendum to 100% support. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… 181227) Terminal Unavailable/PermanentlyOverweight outcomes count as serviced work so a following task is not wrongly deleted; the lookup branch of service_task now charges the unconditional Retries read+write. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dian (181299, 181294, 181295, 181386) cancel_transfer makes the scheduler cancel best-effort so a removed task cannot freeze held funds; execute_transfer and per_transfer weights add depth-dependent Poseidon ref-time; genesis rejects a self-guardian high-security row. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A prior commit dropped ProposalData::call_weight and bumped STORAGE_VERSION to 1 without a migration, stranding v0 records. Adds a VersionedMigration that translates stored Proposals to the new layout, registers it in the runtime, and updates the README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mark registry (181308, 181339, 181268, 181332, 181263)
Grow the tech collective to the 5-member size the 60/61% curves assume and reject
undersized genesis seeds (critical quorum-collapse); re-seed the benchmark dev
collective; fund planck treasury signers for multisig operation; enforce the
high-security whitelist on Utility::dispatch_as{,_fallible}; register the six
active pallets missing from define_benchmarks! (with the utility bench feature).
Updates the tech-collective tests to the 5-member invariant.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…180135, 181313, 181437, 181438, 181258, 181374, 181403, 180119, 180118, 181245) Serialize the fork-choice decision under a shared import lock; validate genesis difficulty; fail closed on cumulative-work read errors; floor the retarget block time; use the full reorg window; error-log (not swallow) finalization failures; rebuild the mining candidate after a failed import and after major sync; read the build version before snapshotting; gate seal submission on the sync oracle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Review of PR #647 — V12 audit round 2 Things worth a comment or a decision
Minor / non-blocking |
|
Warning V12 could not find a connected workspace for this repository. Make sure the repository owner has signed in to V12 and connected their GitHub account (or that the V12 GitHub App is installed for the owning organization), then comment again. |
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # pallets/reversible-transfers/src/weights.rs # runtime/src/genesis_config_presets.rs # runtime/src/transaction_extensions.rs
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # runtime/Cargo.toml # runtime/src/genesis_config_presets.rs
V12 Audit Round 2 — assessment & fixes
Second V12 autonomous audit run (2026-08-06) produced 226 findings. Of these, 147 were triaged invalid by V12 itself (listed collapsed at the bottom). We independently re-assessed the remaining 79 against
mainat HEAD — not trusting the tool's triage — and reached our own verdict on each.Our verdicts on the 79 tool-"valid" findings
Fixed (32)
definitions.rs. (e37cd824)schedule_enactmentnow returns aResult; a referendum is committedApprovedonly when enactment scheduling succeeds, otherwise it staysOngoingwith a retry alarm so the call/origin are never discarded. (af122ee3)ready_for_decidingnow returns aNotQueuedbranch that arms the undeciding-timeout alarm instead of leaving the referendum with no wake-up. (af122ee3)Tally::supportreturnsPerbill::zero()when the electorate is empty (was 100%) and clamps ayes to the member count so a shrunken electorate can't exceed 100%. (5e74e221)ensure_alarm_atnow schedules the replacement alarm before cancelling the old one and keeps the existing alarm if scheduling fails, so a vote/service can't destroy a working alarm. (af122ee3)af122ee3)insert_sorted_by_keynow returns the evicted entry; both eviction sites clear the displaced referendum'sin_queueand re-arm its timeout alarm, with the extra read/write charged in the queued/requeued weights. (af122ee3)cancel_transfermakes the schedulercancel_namedcall best-effort (logs on failure, no longer propagates), so a already-removed task can no longer roll back the funds release and freeze held funds. (ccbbc7a8)a48bc7ff)genesis_buildnow assertsinitial_difficultyis within[min, max)(rejecting zero) using the existing bound helpers. (a48bc7ff)get_chain_workreads inimport_blocknow propagate errors (fail closed) instead of substituting a zero fork-choice baseline. (a48bc7ff)MIN_RETARGET_BLOCK_TIME_MS), chosen below the production divisor so it's a no-op for legitimate targeting and cannot stall fast blocks. (a48bc7ff)dispatch_as/dispatch_as_falliblenow enforceHighSecurity::is_call_allowedwhen the rewritten origin isSigned, matchingas_derivative; Root/None/other origins unchanged. (e37cd824)submitreturns a newAlarmSchedulingFailederror when the timeout alarm can't be scheduled, so the transactional extrinsic rolls back the reserve/counter/scheduler writes instead of creating an unserviceable referendum. (af122ee3)cancelandkillnow remove the referendum fromTrackQueuewhen it was still queued, freeing bounded governance capacity; the extra read/write is charged. (af122ee3)nudge_referendumclears the stored alarm only when it matches the current block, otherwise cancels the orphaned scheduler task instead of leaking it. (af122ee3)MarginalWeightInfo::service_tasknow composesservice_task_base, charging the unconditionalRetriesread+write that was only counted on the base path. (b23089e3)VersionedMigration(v0→v1) that translates eachProposalsrecord to the new layout (droppingcall_weight), registered it in the runtime, and updated the README. (d61aedba)execute_transfer_weightnow adds the depth-dependent Poseidon ref-time (insert_leaf_hash_ref_time_at_depth) from the live tree-depth snapshot, mirroring mining-rewards. (ccbbc7a8)per_transfer_weighttakes one tree-depth snapshot and adds the depth-dependent Poseidon ref-time alongside the DB ops so post-dispatch reconciliation charges the full unit. (ccbbc7a8)GenesisConfig::buildnow rejects a self-guardian high-security row (guardian == who), matching the signedset_high_securityinvariant. (ccbbc7a8)e37cd824)a48bc7ff)a48bc7ff)a48bc7ff)a48bc7ff)a48bc7ff)tech_collective_seed_memberskey so the benchmark dev chain actually seeds the collective. (e37cd824)one_fewer_decidinginto a helper and call it from the exhaustion fallback so a freed deciding slot promotes a queued referendum instead of only decrementing the count. (af122ee3)Unavailable/PermanentlyOverweightoutcomes now count as serviced work so a following task isn't misclassified and permanently deleted on the same cycle. (b23089e3)version()before snapshotting metadata inhandle_local_miningso a concurrent rebuild invalidates the stale post-search version check. (a48bc7ff)define_benchmarks!and added the missingpallet-utility/runtime-benchmarksfeature. (e37cd824)Assessed invalid (42)
set_recovered(:468-494) is documented as a Root grant primitive, not a revocation one — Root has no revoke path here either way and can already reach the same state via System::set_storage. Additionally, the implied fix (consuming ActiveRecoveries on claim) would orphan the rescuer's reserved RecoveryDeposit, since close_recovery is the only path that repatriates/unreserves it (:706-742, reserved at :574-576).BenchmarkConfig.internal_repeats(frame/benchmarking/src/utils.rs:210-224) is only reachable throughimpl frame_benchmarking::Benchmark<Block> for Runtime, which is gated by#[cfg(feature = "runtime-benchmarks")](runtime/src/apis.rs:257-299), and the only caller is the node's ownSubcommand::BenchmarkCLI, itself gated by the same feature (node/src/cli.rs:69-72, node/src/command.rs:499-548). There is no production/RPC attack surface: the value is supplied by the operator running their own benchmarking build, so an operator DoS'ing their own local benchmark run is not a vulnerability; a runtime-benchmarks build is never a production artifact.list.retain(/id/ id != tx_id)over a BoundedVec of at most 16 32-byte hashes (pallets/reversible-transfers/src/lib.rs:714-716) whose <=561-byte storage read/write is already charged in execute_transfer's 5r/5w base (weights.rs:63-64); scanning 16 hashes is nanosecond-to-microsecond scale and cannot meaningfully exceed the charged weight.Assessed obsolete (5)
V12-triaged-invalid findings (147) — not independently reviewed (V12's own triage reasons are in the export JSON)
Queued🤖 Generated with Claude Code