Remove assets pallet

[illuzen]

Summary

• Remove unused pallet-assets / pallet-assets-holder from the runtime. Indices 17 and 18 stay vacant so Multisig / Wormhole / ZkTree remain 19 / 20 / 21.
• Delete the inlined crates from the workspace (pallets/assets, pallets/assets-holder) and drop their path deps from the root Cargo.toml / lockfile.
• Decouple reversible-transfers from assets: remove schedule_asset_transfer and schedule_asset_transfer_with_delay, vacate call indices 5 and 6 so recover_funds stays at 7. Keep PendingTransfer.asset_id: Option<_> for storage/event compatibility; new schedules are native-only (None).
• Clean remaining wiring: wormhole unused Assets associated type, genesis assets config, tx-extension asset event matching, node txwatch asset parsing, mocks/deps, and dead asset unit tests.

Test plan

• cargo check -p quantus-runtime
• cargo check -p quantus-node
• cargo test -p pallet-reversible-transfers
• cargo test -p pallet-wormhole
• cargo test -p pallet-multisig
• cargo test -p quantus-runtime --lib transaction_extensions
• Confirm metadata/call encoding: Multisig still index 19, Wormhole 20, ZkTree 21; recover_funds still call index 7
• Smoke: native schedule_transfer / cancel / execute still works; no Assets pallet (or crates) in runtime metadata / workspace

---

Note

High Risk
This is a breaking runtime surface change (removed pallets and extrinsics) that touches wormhole proof recording, genesis, and high-security call whitelisting—areas that affect funds and transaction validity.

Overview
Removes the assets stack from the chain by dropping pallet-assets and pallet-assets-holder from the workspace, runtime, node, and dependent pallets. Runtime pallet indices 17 and 18 stay vacant so Multisig (19), Wormhole (20), and ZkTree (21) indices are unchanged.

Reversible transfers are native-only. schedule_asset_transfer and schedule_asset_transfer_with_delay are removed (call indices 5 and 6 left vacant so recover_funds remains at 7). New schedules always use native balance holds; PendingTransfer.asset_id stays in storage/events for compatibility, but Some(_) paths return AssetsNotSupported. Config now takes a generic AssetId type instead of coupling to pallet_assets.

Supporting cleanup: wormhole loses the Assets config type and asset mint/transfer proof paths; genesis no longer creates asset id 0; high-security whitelist and tx-extension proof counting no longer match Assets calls/events; node txwatch only parses balance transfers; mocks, benchmarks, weights, and asset-specific tests are removed or disabled.

^{Reviewed by Cursor Bugbot for commit 77098e8. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

^{❌ 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 77098e8. Configure here.}

[cursor]

Missing runtime spec_version bump

High Severity

This PR removes two runtime pallets, deletes reversible-transfer calls, and adds AssetsNotSupported, all of which change metadata. spec_version stays at 137, so clients caching metadata by version will keep stale call/event/error indexes after upgrade.

 

^{Triggered by learned rule: Bump spec_version when adding/renaming calls, events, or errors in runtime}

^{Reviewed by Cursor Bugbot for commit 77098e8. Configure here.}

[cursor]

Stale runtime surface documentation

Medium Severity

RUNTIME_SURFACE.md still lists Assets / AssetsHolder at indices 17/18, documents removed reversible asset calls, and claims wormhole Assets = Assets. Genesis preset docs still describe treasury ownership of asset id 0 after that genesis config was removed.

 

^{Triggered by learned rule: Pallet dependencies must not hardcode features = ["std"] — use no-std + feature gate}

^{Reviewed by Cursor Bugbot for commit 77098e8. Configure here.}

[n13]

Verdict: Request changes

The runtime/pallet changes themselves are careful and correct — index preservation (pallet 17/18, call 5/6, recover_funds at 7) is handled properly, Error::AssetsNotSupported is appended so existing error indices stay stable, Config::AssetId = u32 in the runtime keeps the SCALE encoding of PendingTransfer storage/events unchanged, and the tx-extension weight matcher and event recorder were updated in lockstep. CI is green including both full test matrices. Two things need to be resolved before merge, though.

1. PR description doesn't match the branch (blocking)

The summary says the inlined crates are deleted from the workspace and their path deps dropped from the root Cargo.toml / lockfile. On illuzen/remove-assets, pallets/assets/ and pallets/assets-holder/ still exist, and the root Cargo.toml still lists them as workspace members (lines 33–34) with path deps (lines 258–259). The Cargo.lock diff only removes them from consumers' dependency lists. Either push the actual crate deletion or fix the description — as-is, the workspace keeps compiling two dead crates (and their CI cost) while the PR claims otherwise.

2. Pre-existing pending asset transfers become permanently stuck (needs verification or a migration)

After this upgrade, any PendingTransfers entry with asset_id: Some(_) already on chain is unrecoverable through every path:

• do_execute_transfer errors with AssetsNotSupported before releasing anything — the scheduled task fires once, fails, and the entry stays.
• cancel goes through release_held_funds_with_fee, which now errors on Some(_) first, so the whole cancel reverts.
• recover_funds skips it (emitting TransferRecoveryFailed every call) and deliberately leaves the metadata.

The asset balances themselves being unreachable is inherent to removing the pallet, but the stuck metadata has a concrete side effect: those entries permanently occupy PendingTransfersBySender slots (MaxPendingPerAccount = 16), so an affected high-security account can be permanently blocked from scheduling new transfers. There's no on_runtime_upgrade migration in the PR.

Ask: either confirm via try-runtime (or a storage scan of Heisenberg/Planck) that no Some(asset_id) pending transfers exist at upgrade time, or add a small migration that drains those entries (remove from PendingTransfers + PendingTransfersBySender, cancel the scheduler task).

Related, optional: Assets / AssetsHolder storage prefixes (including the genesis asset 0) stay orphaned on chain forever. A frame_support::migrations::RemovePallet pair in the Migrations tuple would clean that up cheaply.

3. spec_version not bumped

runtime/src/lib.rs still has spec_version: 137. This is a metadata-breaking change and ships via governance per docs/RUNTIME_UPDATE.md, so it needs a bump — fine if that happens in a separate release PR, but worth stating in the test plan.

Test coverage

Several tests are disabled with #[cfg(any())] rather than deleted. Two of them — recover_funds_is_atomic_when_release_fails and recover_funds_weight_accounts_for_failed_releases — cover properties that still exist for native transfers (recovery atomicity, weight accounting for failed releases); they only used assets as a convenient way to force a release failure. Disabling them silently drops coverage of live behavior. Suggest porting them to force the failure natively, and deleting the genuinely asset-only ones outright — #[cfg(any())] blocks referencing removed APIs will just bit-rot.

Nit

do_schedule_transfer_inner still takes asset_id: Option<AssetIdOf<T>>, but both remaining callers pass None, making the new ensure! unreachable. Dropping the parameter and hardcoding asset_id: None into the stored struct would be simpler; keeping the guards in do_execute_transfer / release_held_funds_with_fee (which read stored data) is right.

[n13]

Point 1 and 3 are valid

[n13]

Verdict: Approve

Re-review after the latest push (8fd25edf delete crates, 59ec749b docs sync, 5621a925 spec bump). Both blocking points from the previous review are resolved, and the core runtime changes are unchanged since they were verified.

Previous review points

1. Crates not actually deleted — fixed. pallets/assets/ and pallets/assets-holder/ are gone from the branch, removed from workspace members and the [workspace.dependencies] path deps, and Cargo.lock no longer contains either crate. A full grep of the branch finds no remaining non-comment references to pallet_assets — the surviving mentions are the intentional documentation in the wormhole asset-0 defense path and generic upstream doc comments.

2. Stuck Some(asset_id) pending transfers — accepted as not applicable per discussion. No migration was added, which is fine given no asset-based reversible transfers exist on chain. For the record, the safety condition is: PendingTransfers must contain no Some(asset_id) entries at upgrade time, since all three exit paths (execute, cancel, recover_funds) reject them and each stuck entry permanently occupies one of the sender's 16 PendingTransfersBySender slots. A quick storage scan of Heisenberg/Planck before the upgrade ships would close this conclusively. The orphaned Assets/AssetsHolder storage prefixes can be cleaned up later with a RemovePallet pair if desired — not blocking.

3. spec_version — fixed. Bumped to 138. transaction_version stays at 3, which is correct: no existing call or pallet index is renumbered (17/18 and call indices 5/6 are left vacant), so previously-valid transaction encodings are unaffected.

New since last review

• docs/RUNTIME_SURFACE.md sync is accurate against the actual changes: vacant indices 17/18 documented, reversible-transfers call table shows 5/6 vacant with recover_funds at 7, AssetId = u32 rationale noted, tx-extension #11 description narrowed to native Balances events, and the genesis asset-0 note removed.
• The #[cfg(any())]-disabled asset tests in reversible-transfers are now fully deleted rather than left as dead code, and the stale asset-0 genesis comment is gone.

Carried-over verification (unchanged files, previously confirmed)

Pallet indices 17/18 and call indices 5/6 vacated with comments; AssetsNotSupported appended as the last Error variant so existing error indices are stable; runtime Config::AssetId = u32 preserves the SCALE encoding of PendingTransfer storage and events; tx-extension weight matcher and proof recorder updated in lockstep; wormhole keeps the asset-0 Some(0) drop guard.

CI

Format and clippy are green on the final commit; the two Build & Test matrix jobs were still running at review time. Both matrices passed on the previously reviewed commit, and the delta since is crate deletion, docs, and the version bump, so risk is low — but merge should wait for them to finish.