Skip to content

Frame fixes medium low#617

Open
illuzen wants to merge 48 commits into
mainfrom
illuzen/frame-fixes-medium-low
Open

Frame fixes medium low#617
illuzen wants to merge 48 commits into
mainfrom
illuzen/frame-fixes-medium-low

Conversation

@illuzen

@illuzen illuzen commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Harden FRAME and runtime against V12 medium-severity audit findings

Summary

This PR addresses 27 Medium-severity findings from the V12 audit (frame-medium-low.md). Each item was triaged: 25 were confirmed and fixed, 2 were acknowledged and documented (no code change). Fixes are intentionally minimal — targeted at the specific failure mode, with regression tests where appropriate.

The branch stacks on earlier illuzen/frame-fixes work (zk root validation, genesis bounds, bounded migrations, token accounting, etc.). Against main this is 41 commits across 67 files (~3k insertions).

Disposition

Status Count
Valid (fixed) 25
Acknowledged (documented) 2
Invalid 0

Medium fixes

Macro, metadata, and codegen

  • Unknown module name panics (#93203) — get_call_names returns an empty slice for unknown modules instead of panicking.
  • Eager task enumeration exhausts memory (#93241) — task enumeration streams lazily instead of collecting all tasks into memory up front.
  • New-prefix migrations skip (#93290) — new-pallet storage version is initialized after migrations, not before.
  • Trailing Weight Tokens Ignored (#93294) — trailing tokens in #[pallet::weight(...)] are rejected at compile time instead of silently ignored.
  • Unmetered view-function decoding (#93295) — view-function inputs are decoded with a memory limit and a full-consumption check.
  • Ambiguous Genesis Field Retention (#93308) — genesis field retention uses serde-exact camelCase comparison.
  • Opaque Wrapper Breaks Bounds (#93357) — WrapperKeepOpaque enforces MaxEncodedLen on the wrapped type.
  • Unbounded runtime metadata decoding (#93479) — META_RESERVED is validated before decoding metadata payloads.
  • Counted N-map counter prefixes bypass collision checks (#93481) — CountedStorageNMap counter prefixes are included in the procedural duplicate-prefix check.

Storage, migrations, and counted maps

  • Corrupt Takes Persist (#93324) — undecodable explicit child-storage entries are cleared in take().
  • Overlay Work Misreported (#93325) — child::clear_storage counters account for overlay-only removals.
  • Overlapping Prefix Migration (#93334) — move_prefix rejects overlapping from/to prefixes to avoid self-reprocessing.
  • Default-Bound Append Bypass (#93339) — CountedStorageMap::try_append enforces bounds against the effective OnEmpty default, not just stored length.
  • Set Append Stores Duplicates (#93340) — BTreeSet storage appends deduplicate entries.
  • Prefix Clear Resets Counter (#93347) — partial clear_prefix subtracts only the removed entries instead of killing the global counter.
  • Prefix clear fallback fabricates progress (#93482) — resumed unhashed::clear_prefix walks from the supplied cursor and makes real progress.
  • Counted storage helpers corrupt key cardinality counters (#93483) — try_append, migrate_key/migrate_keys, and CountedStorageNMap::set reconcile counters against actual key existence.
  • Drainable counted-storage iterators desync counters (#93484) — all drain-capable counted map/N-map iterators (iter_keys, iter_prefix_values, etc.) wire through OnRemovalCounterUpdate; KeyPrefixIterator gains an OnRemoval hook.

Scheduler, preimage, and tickets

  • Preimages Persist After Failed Scheduling (#93360) — noted preimages are dropped when scheduling fails.
  • Stale Tickets Clear Locks (#93404) — zero-cost lone freeze/hold consideration tickets are rejected.

Token / fungible traits

  • Held transfers bypass accounts (#93409) — OnHold transfers enforce destination account existence.

Frame-system and block limits

  • Derived extrinsic caps exceed block capacity (#93453) — derived max_extrinsic caps subtract base_block.
  • Upgrade Weight Fails Drain (#93455) — runtime-upgrade weight is actually drained from the block budget.

Runtime / wormhole

  • Derivative accounts inflate wormhole soundness pool (#93477) — first use of an as_derivative pseudonym reveals it to the soundness counter (subtracting its balance from PotentialWormholeBalance).
  • Transfer-proof weight misses wrapped/internal dispatch (#93491) — as_derivative-wrapped transfers are statically counted; post_dispatch registers extra block weight for proof-recording work that static analysis cannot see (e.g. Multisig::execute, recover_funds).

Acknowledged (documented, not changed)

  • Callbacks are upgrade-unstable (#93370) — Callback stores (pallet_index, call_index) and resolves against the call layout at curry time. No in-repo consumer persists callbacks across upgrades; constraint documented for future oracle work.
  • Signer length underweights validation (#93448) — check_nonce / check_non_zero_sender use constant weights. Sound when AccountId has a fixed max_encoded_len() (true for Quantus); documented so future variable-length account types regenerate weights.

Earlier fixes on this branch (from illuzen/frame-fixes)

These predate the medium batch but are included when merging this branch to main:

  • Validate zk_tree_root on block import (frame-executive)
  • Cap genesis dev_accounts derivation (pallet-balances)
  • Bound RemovePallet / RemoveStorage deletion; try-runtime tolerates intentional remainders
  • Add move_prefix_bounded for staged prefix moves
  • Balanced::deposit checks issuance headroom before crediting
  • CheckNonce rejects MAX nonce instead of wrapping to zero
  • ensure_frozen / decrease_frozen idempotency for permitted over-freezes
  • DecodeAll for Callback::curry and NFT typed_attribute
  • Imbalance::ration overflow handling; SplitTwoWays compile-time validation
  • Transfer dust accounting (fungibles::transfer + do_transfer_approved)
  • dynamic_params inner-attribute precedence

Test plan

  • cargo test -p frame-support --lib (228 tests)
  • cargo test -p quantus-runtime --lib (36 tests, including wormhole extension lifecycle tests)
  • cargo check --workspace --all-targets
  • Full CI on PR
  • cargo test --release doctests (defensive trait docs fixed to no_run)

Focused regression areas

  • Counted storage: try_append, migrate_key, set, clear_prefix, iterator drains
  • unhashed::clear_prefix cursor resumption
  • Wormhole: as_derivative weight counting, post-dispatch weight reconciliation, derivative reveal
  • Migration helpers: bounded deletion, overlapping prefix rejection, partial clear counter accounting

Note

High Risk
Touches block import (ZK header field), runtime upgrades/migrations, counted storage invariants, and fungible issuance/transfer/nonce paths—errors could brick sync, skew counters, or break accounting; breadth across executive, support, balances, assets, and frame-system warrants careful CI and runtime upgrade testing.

Overview
Hardens FRAME and related pallets against a batch of V12 medium-severity audit issues: bounded decoding/enumeration, storage counter correctness, migration safety, and token/accounting edge cases, plus ZK tree root validation on block import.

Block import & headers: frame-executive re-derives and asserts zk_tree_root on import (with a regression test for forged roots). qp-header is wired in as a normal dependency.

Codegen & metadata: Unknown get_call_names modules return [] instead of panicking. Runtime/pallet task iterators are lazy. #[pallet::weight] and related attributes reject trailing tokens. New-pallet storage version is seeded after migrations, not in before_all_runtime_migrations. Genesis field matching uses serde-exact camelCase. RuntimeMetadataPrefixed checks META_RESERVED before decoding payloads. WrapperKeepOpaque enforces MaxEncodedLen on decode/construct. Counted N-map counter prefixes participate in duplicate-prefix checks. dynamic_params only injects outer args when the inner attribute has none.

Storage & migrations: Child take clears corrupt entries; clear_storage / unhashed::clear_prefix fix overlay accounting and cursor resumption. move_prefix is bounded via move_prefix_bounded and panics on overlapping prefixes. RemovePallet / RemoveStorage take a per-upgrade key Limit. Counted map/N-map: try_append respects OnEmpty defaults and key cardinality; set/migrate_*/partial clear_prefix and drain iterators keep counters in sync via KeyPrefixIterator OnRemoval hooks. BTreeSet::append deduplicates; bounded try_append uses effective absent length.

Tokens & system: OnHold transfers require an existing destination. deposit checks issuance headroom; expendable transfer credits reaped dust (assets transfer_approved uses burn_dust). Freeze/hold consideration rejects zero-footprint tickets; ensure_frozen / decrease_frozen idempotency fixes. CheckNonce rejects max nonce (no wrap). max_extrinsic validation uses derived caps. View functions decode with a 16 MiB mem limit and full input consumption. NFT/callback paths use DecodeAll where prefix decode was unsafe. MAX_DEV_ACCOUNTS caps genesis dev derivation.

Docs only (acknowledged): Callback upgrade instability and constant-weight signer extensions are documented, not behavior-changed.

Reviewed by Cursor Bugbot for commit 8fc3a01. Configure here.

illuzen and others added 30 commits July 2, 2026 14:01
…in halt

These OnRuntimeUpgrade helpers cleared an entire prefix with
clear_prefix(.., None) in a mandatory upgrade hook. For a prefix whose key
count users can inflate, the first post-upgrade block performs unbounded
deletion work, never fits the block weight limit, and re-runs every attempt,
halting the chain.

Add a `Limit: Get<u32>` parameter and clear at most `Limit` keys per upgrade
so the work is bounded. Excess keys remain (harmless orphaned storage) and are
logged; raise the limit or use a multi-block migration to remove the rest.

Co-authored-by: Cursor <cursoragent@cursor.com>
move_prefix drains and re-writes every key below a prefix in one call with no
limit, cursor, or progress feedback. Used in a mandatory runtime-upgrade hook
on a user-growable prefix, an attacker can inflate the prefix beforehand and
force the first post-upgrade block to perform unbounded reads/deletes/writes,
stalling the upgrade.

Add `move_prefix_bounded(from, to, limit, maybe_cursor) -> MovePrefixResult`
mirroring `clear_storage_prefix`'s bounded/cursored pattern, so a multi-block
migration can move keys in bounded chunks and charge weight per moved key.
`move_prefix` now delegates to it unbounded, preserving existing behaviour, and
its docs (plus `move_storage_from_pallet`) warn about the unbounded call.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
… balance check

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…gs at compile time

Co-authored-by: Cursor <cursoragent@cursor.com>
Credit the full amount removed from the source in generic transfer helpers so expendable reaping cannot desync issuance from balances. Burn reaping dust in transfer_approved so delegates cannot move more than the approved amount to their chosen destination.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ime checks

Co-authored-by: Cursor <cursoragent@cursor.com>
…nicking

Co-authored-by: Cursor <cursoragent@cursor.com>
…tors

Both the pallet-level Task::iter and the aggregated RuntimeTask::iter
collected every task into heap vectors before yielding the first item.
Task lists can be derived from attacker-growable storage, so eager
materialization allowed unbounded allocation in off-chain workers.

Co-authored-by: Cursor <cursoragent@cursor.com>
before_all_runtime_migrations pre-seeded the on-chain storage version for
any pallet with an empty prefix, running before the runtime's custom
migrations. A version-gated migration importing state into a new pallet
prefix (rename/split/import) then saw the version as already current and
was silently skipped, stranding state under the old namespace. The version
is now initialized after the pallet's own on_runtime_upgrade, and only if
the version key is still absent.

Co-authored-by: Cursor <cursoragent@cursor.com>
… them

FunctionAttr::parse consumed only the leading value of weight/call_index/
feeless_if/authorize/weight_of_authorize attributes and returned without
checking the buffers were exhausted, so malformed attributes compiled with
trailing tokens silently dropped. This could silently truncate a call's
weight metadata and underprice a dispatchable.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ion check

The shared ViewFunction::execute path used plain DecodeAll, bypassing the
mem-tracked decode derived on the generated request structs. View functions
are reachable via the RuntimeViewFunction runtime API with attacker-supplied
input, so decoding is now bounded by MAX_VIEW_FUNCTION_DECODE_MEM to prevent
decode-bomb allocation, while still requiring the whole input be consumed.

Co-authored-by: Cursor <cursoragent@cursor.com>
InitializedField::eq approximated serde's camelCase rename with an
underscore-insensitive, case-insensitive match that was not injective, so
distinct genesis fields like a_b (=> aB) and ab (=> ab) compared equal and
retain_initialized_fields could keep an unselected default field. The
comparison now uses serde's exact camelCase rule so distinct identifiers
never collide.

Co-authored-by: Cursor <cursoragent@cursor.com>
child::take conditioned key removal on a successful typed decode, so a
corrupted (undecodable) explicit entry returned None but was left in
storage. This broke the take_or*/take_or_default/take_or_else contract that
no explicit entry remains on return. take() now removes any explicit entry,
decodable or not.

Co-authored-by: Cursor <cursoragent@cursor.com>
The legacy storage_kill host call only reports backend deletions, so
overlay-resident child keys it also deletes were reported as zero unique/
loops. Callers using those counters for weight or cleanup accounting could
be told no work occurred. clear_storage now counts visible keys before and
after and folds overlay-only removals into unique/loops.

Co-authored-by: Cursor <cursoragent@cursor.com>
move_prefix only guarded the from==to case. If to_prefix nests inside
from_prefix (or vice versa), keys written during the drain fall back inside
the iterator's traversal domain, causing the move to reprocess its own
output (unbounded work) or terminate leaving values under the source prefix.
The prefixes must now be disjoint (neither a prefix of the other).

Co-authored-by: Cursor <cursoragent@cursor.com>
The bounded try_append helpers treated a missing decode_len as an empty
collection. For ValueQuery storage with a non-empty OnEmpty default, the
effective value may already be at capacity, so appends could bypass the
bound (and the default's membership assumptions). StorageTryAppend now
exposes the value length, and all try_append paths (value, map, double
map, n-map, counted map) fall back to the effective query-kind default
when no explicit entry exists.

Co-authored-by: Cursor <cursoragent@cursor.com>
StorageAppend is now a behavior trait instead of a pure marker. Vec and
Digest keep the raw sp_io append, but BTreeSet decodes the stored set,
inserts the item, and writes the canonical representation back. Raw
appends could previously store duplicate entries that a decoded set
silently deduplicates, letting repeated appends of the same element grow
state and decode cost without changing the logical set.

Co-authored-by: Cursor <cursoragent@cursor.com>
A None cursor from clear_prefix only means the selected partial prefix is
exhausted, not that the whole map is empty. Killing the counter reported a
non-empty map as count()==0, letting quota/emptiness checks be bypassed.
Subtract the removed entries instead.

Co-authored-by: Cursor <cursoragent@cursor.com>
WrapperKeepOpaque advertised a MaxEncodedLen derived from T but stored an
unbounded Vec<u8>, so attacker-controlled bytes could exceed the max_size
used for bounded-storage and PoV accounting. Decode now rejects opaque
data longer than T::max_encoded_len(), a checked try_from_encoded
constructor is added, and the unchecked from_encoded is deprecated.

Co-authored-by: Cursor <cursoragent@cursor.com>
StorePreimage::bound notes a preimage for oversized calls before the
scheduler runs its fallible checks. If resolve_time or place_task failed,
do_schedule/do_schedule_named returned an error without undoing the note,
leaving an unowned system-requested preimage as state bloat. Both now drop
the bounded call's preimage on every failure path, mirroring cancellation
cleanup.

Co-authored-by: Cursor <cursoragent@cursor.com>
Callback stores only (pallet_index, call_index) and resolves them against
the call layout present when curry runs, so persisting a caller-provided
callback across a runtime upgrade can decode into a different call. No
in-repo consumer currently persists such callbacks, so this documents the
constraint for future StatementOracle implementations rather than changing
the (unused) type's encoding.

Co-authored-by: Cursor <cursoragent@cursor.com>
LoneFreezeConsideration/LoneHoldConsideration encode no ticket state, so a
zero-footprint ticket leaves no freeze/hold marker yet its drop/burn clears
the whole reason bucket. A stale zero ticket could therefore thaw, release,
or burn a newer paid ticket's collateral. new/update now reject a zero
converted amount so every live ticket corresponds to a non-empty bucket.

Co-authored-by: Cursor <cursoragent@cursor.com>
illuzen and others added 11 commits July 2, 2026 19:26
`transfer_on_hold` documents that in `Restriction::OnHold` mode the
destination account must already exist, but the default implementation
never checked this: `can_deposit(.., Extant)` only concerns issuance
accounting, so a held balance could be credited to a non-existent
account, creating orphan hold state with no underlying account.

Both the `fungible` and `fungibles` default implementations now reject
`OnHold` transfers to non-existent destinations with
`TokenError::CannotCreate`.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ights

`check_non_zero_sender()` and `check_nonce()` return constant weights
while their implementations do work proportional to the encoded signer
length (scanning the encoded `AccountId` and encoding it into
transaction-pool dependency tags). That work is bounded by
`AccountId::max_encoded_len()`, so the constant model is only sound when
the weights are benchmarked against the runtime's actual `AccountId`
with a maximal-length signer. Document this requirement on `WeightInfo`
so runtimes with variable-length account identifiers regenerate their
weights instead of reusing values benchmarked with short signers.

Co-authored-by: Cursor <cursoragent@cursor.com>
`BlockWeightsBuilder::build` derived `max_extrinsic` from a class's
`max_total` minus the average initialization cost and the class base
extrinsic weight, but ignored `base_block`, which every block consumes
before any extrinsic runs. For classes whose `max_total` equals
`max_block` (e.g. the operational class from the default builders), a
transaction at the advertised cap passed pool validation yet could
never be included: `base_block + base_extrinsic + weight` exceeded
`max_block` and overflowed into (and past) the reserved allowance,
creating a valid-but-unincludable transaction class.

The derivation now subtracts `base_block` as well, keeping the
validation-time cap consistent with the block-building capacity model.

Co-authored-by: Cursor <cursoragent@cursor.com>
`set_code`, `set_code_without_checks` and `apply_authorized_upgrade`
tried to consume the rest of the block by returning `max_block` as
post-dispatch `actual_weight`, but `calc_actual_weight` caps the
post-dispatch weight at the static pre-dispatch weight (~0.17s of
ref-time), so the returned value was silently ignored and further
transactions could still be included after the code update in the same
block — executing old-runtime transactions on top of an accepted
upgrade and letting an attacker shape the migration starting state.

The upgrade dispatchables now register the remaining block weight
directly via `register_extra_weight_unchecked`, so the block really is
drained regardless of the static call weight.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ss counter

`as_derivative` pseudonym accounts receive tracked credits but never
sign an extrinsic themselves (their controller signs), so their nonce
stays zero forever: every credit to them was added to
`PotentialWormholeBalance` and nothing ever subtracted it. An attacker
could bounce fixed capital between derivative addresses to inflate the
wormhole soundness pool without bound, weakening the
`TotalWormholeExits <= PotentialWormholeBalance` backstop.

Mirror the multisig treatment: on the first use of a pseudonym,
`as_derivative` now reveals it via a new narrow
`qp_wormhole::AddressRevealer` hook (deducting its balance from the
pool) and records it in `KnownDerivatives`; the runtime's
`NonWormholeAccounts` then excludes known pseudonyms so later receipts
are not counted either.

Co-authored-by: Cursor <cursoragent@cursor.com>
`RuntimeMetadataPrefixed` used a derived `Decode`, so an invalid blob
was only rejectable after the entire `(u32, RuntimeMetadata)` tuple —
potentially an attacker-sized structure of nested vectors, maps and
strings — had been fully materialized. Decode now checks the reserved
'meta' magic after the first four bytes and fails fast on a mismatch.

Also document on the type that metadata decoding has no schema-level
caps beyond input-length proportionality, so callers decoding untrusted
blobs must bound the input size up front.

Co-authored-by: Cursor <cursoragent@cursor.com>
…-maps in duplicate check

Co-authored-by: Cursor <cursoragent@cursor.com>
…ead of fabricating progress

Co-authored-by: Cursor <cursoragent@cursor.com>
… key existence transitions

Co-authored-by: Cursor <cursoragent@cursor.com>
…hrough key and value iterators

Co-authored-by: Cursor <cursoragent@cursor.com>
…ncounted proof-recording work post-dispatch

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8fc3a01. Configure here.

pub fn move_prefix(from_prefix: &[u8], to_prefix: &[u8]) {
// Drive the bounded mover to completion in one shot. `u32::MAX` keys is effectively no bound,
// preserving the historical all-at-once behaviour for callers that know the prefix is small.
move_prefix_bounded(from_prefix, to_prefix, u32::MAX, None);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move_prefix stops at u32 max

Low Severity

move_prefix now delegates to move_prefix_bounded with a limit of u32::MAX. If more than that many keys share the source prefix, the helper stops after one pass and returns without error, leaving the remainder under the old prefix. The previous implementation drained the entire prefix in one call.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8fc3a01. Configure here.

@n13 n13 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — V12 medium-severity audit fixes

Went through all 67 files. Overall this is careful, well-tested work: each fix is narrowly targeted at the audited failure mode, and nearly every behavioral change ships with a regression test that would fail without the fix. Issues below, ordered by importance.

Blocking

1. CI is red — cargo fmt failures skip the entire test matrix.
The Fast Checks (Format) job fails on pallets/scheduler/src/tests.rs, pallets/wormhole/src/lib.rs, and runtime/src/transaction_extensions.rs, so Build & Test and Clippy never ran — nothing in this PR has CI test coverage yet. Needs a fmt pass and re-push.

Should fix / discuss

2. child::clear_storage now walks the whole child trie twice (frame/support/src/storage/child.rs).
The new key_count helper enumerates every key via next_key both before and after the kill to attribute overlay-only removals. A call with maybe_limit = Some(small) now does O(total keys) host calls — the same "user-growable state forces unbounded work" pattern this audit batch fixes elsewhere (e.g. RemovePallet). No in-repo callers today, so it's latent, but consider bounding the count or at least documenting the cost on the function.

3. Wormhole weight model not updated for the new KnownDerivatives read (runtime/src/transaction_extensions.rs / runtime/src/configs/mod.rs).
NonWormholeAccounts::contains now also reads Utility::KnownDerivatives (via is_derivative), so is_ambiguous_account costs one more read than the model assumes. Both per_transfer_weight() (4 reads) and the signer reveal weight (reads_writes(4, 1)) should become 5 reads, or the comments should say why the extra read is ignored.

Minor / questions

  1. KeyPrefixIterator drain does an extra get_raw per key even when OnRemoval = () (frame/support/src/storage/mod.rs). Every plain iter_keys().drain() on a non-counted map now pays a value read per key that only exists to feed the counter hook. Fine for correctness; could be specialized away if it ever matters somewhere hot.
  2. New-pallet detection changed from "no keys under the pallet prefix" to "no storage-version key" (hooks.rs). A pallet holding data that never wrote :__STORAGE_VERSION__: now gets its in-code version seeded after migrations, where before it was left untouched. I believe that's fine (arguably more correct) for this chain — just confirming it's intentional.
  3. BTreeSet append is now decode-insert-rewrite, i.e. O(n) per append instead of an O(1) raw append. Correct fix for the dedup issue; worth remembering for large sets.
  4. RemovePallet/RemoveStorage gained a mandatory Limit type param — breaking for downstream users of these helpers (none in-repo).

Checked and found sound

  • zk_tree_root re-derivation on import (both the execute_block state-root-gated path and final_checks), with the forged-root regression test.
  • Storage-version seeding order: in Executive::execute_on_runtime_upgrade, custom migrations (COnRuntimeUpgrade) run before AllPalletsWithSystem::on_runtime_upgrade, so version-gated migrations targeting a new prefix correctly observe version 0, and the pallet hook doesn't clobber a version a migration set.
  • Scheduler preimage ownership: the scheduler already drops the preimage after execution and on cancel, so dropping on failed placement (including the referenda v3-shim path, where the proposal is handed off at approval) is consistent and closes the leak without double-drops.
  • Counted map/N-map counter reconciliation (try_append, migrate_key(s), set, partial clear_prefix, all drain iterators), including the hasher-collapse cases.
  • Fungible(s) transfer crediting the actual debit: matches assets' reap semantics (decrease_balance can return amount + dust there); for pallet-balances dust still routes through handle_dust, so no double count. transfer_approved burning reap dust instead of over-crediting the delegate's destination is the right call.
  • Balanced::deposit issuance-headroom check mirrors mint_into; Exact fails cleanly, BestEffort caps at headroom, and the debt drop no longer silently saturates.
  • set_code weight drain via register_extra_weight_unchecked — no double counting with CheckWeight's refund since actual_weight is capped at the static weight.
  • unhashed::clear_prefix cursor resumption, move_prefix_bounded cursor + disjointness assert (equal prefixes still early-return before the assert), nonce-MAX rejection, max_extrinsic base_block subtraction, view-function 16 MiB decode bound, serde-exact camelCase genesis retention, and trailing-token rejection in pallet attributes.
  • The two acknowledged items (#93370 Callback upgrade-instability, #93448 constant-weight signer extensions) are reasonable as documentation-only given in-repo usage.

Verdict

Approve after fixes. The substance is sound and test coverage is strong, but this shouldn't merge until (1) fmt is fixed so the full CI matrix actually runs green, and ideally (2) the wormhole read count is bumped for the KnownDerivatives lookup. Item 2 (child::clear_storage cost) can be a follow-up since the function currently has no callers, but a doc warning now would be cheap.

illuzen and others added 7 commits July 3, 2026 14:38
The pre/post key counts used to attribute overlay-only removals walked
the entire child trie twice, so a call with a small limit still paid
O(total keys) next_key host calls — the same user-growable-state pattern
the limit exists to prevent. Cap both walks at limit + 1024; when the
cap saturates, overlay attribution degrades to zero (documented
under-reporting) while the backend count stays exact. Unbounded clears
keep exact counting since the host call is already O(total keys).

Co-authored-by: Cursor <cursoragent@cursor.com>
NonWormholeAccounts::contains now also checks Utility::KnownDerivatives
(via is_derivative), so is_ambiguous_account costs one more storage read
than the model assumed. Bump per_transfer_weight and the signer reveal
weight from 4 to 5 reads and update the read breakdowns in the comments.

Co-authored-by: Cursor <cursoragent@cursor.com>
The OnRemoval hook forced every drain to read the removed value even for
hooks that ignore it (the no-op default and the counted-map counter
update), costing a get_raw host call per key on plain iter_keys drains.
Add a NEEDS_VALUE associated const so such hooks opt out of the read.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	frame/support/src/migrations.rs
#	frame/support/src/storage/migration.rs
#	frame/support/src/traits/tokens/fungible/regular.rs
#	frame/support/src/traits/tokens/fungibles/regular.rs
#	pallets/balances/src/lib.rs
Both main and this branch added block_import_of_bad_zk_tree_root_fails,
and the merge auto-resolution kept both copies. Keep the variant that
pins the expected panic message.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants