Skip to content

Chore update polkadot-sdk to stable2603#3746

Open
manuelmauro wants to merge 39 commits into
masterfrom
chore-update-polkadot-sdk-stable2603
Open

Chore update polkadot-sdk to stable2603#3746
manuelmauro wants to merge 39 commits into
masterfrom
chore-update-polkadot-sdk-stable2603

Conversation

@manuelmauro

@manuelmauro manuelmauro commented May 8, 2026

Copy link
Copy Markdown
Contributor

What does it do?

Upgrades polkadot-sdk — and the frontier / evm / ethereum / moonkit forks — from stable2512 to stable2603 (upstream tag polkadot-stable2603-1, released 2026-05-01).

Concretely:

  • Dependency pins + lockfile. Cargo.toml swaps moonbeam-polkadot-stable2512moonbeam-polkadot-stable2603 across all five forks (~180 occurrences); Cargo.lock re-resolved.
  • XCM credit-based holding migration — the substantive piece. stable2603 reworked xcm_executor::AssetsInHolding to carry real fungible::Credit imbalances instead of plain Asset descriptors. The TransactAsset, WeightTrader, and FeeManager::handle_fee impls were migrated accordingly:
    • erc20-xcm-bridge and moonbeam-foreign-assets gain a notional.rs notional-credit type (erc20 is an EVM-side balance, not a Substrate fungible, so it has no real Credit; the token movement still happens via EVM calls in withdraw_asset/deposit_asset, and the notional credit only threads the asset through holding).
    • xcm-weight-trader reworks buy_weight / refund_weight / Drop for the credit model; mint_asset is now implemented in moonbeam-foreign-assets.
  • API drift fixes to compile green across the realistic feature matrix (default / runtime-benchmarks / try-runtime): RuntimeDebugDebug (51 sites / 14 files, upstream #10582); proof-recording refactor and removal of cumulus-client-consensus-proposer in the lazy-loading manual-seal (upstream #9947); node/service client-side drift; new cumulus_pallet_parachain_system::WeightInfo methods added to weight files; runtime-API and XCM-config signature changes (see Breaking Changes).
  • Regenerated TypeScript API types and updated the dev / XCM TS test fixtures for the new relay and credit-model XCM events.
  • Docs: adds the cherry-pick tracker polkadot-sdk-stable2603.md and the working upgrade plan UPGRADE-stable2603.md.

What important points should reviewers know?

  • Fork branches. Each fork has a fresh moonbeam-polkadot-stable2603 branch with the moonbeam-only cherry-picks re-applied. Bases: polkadot-sdk from the polkadot-stable2603-1 tag (rebased onto upstream/stable2603 head to absorb 4 backports); frontier from the official upstream/stable2603 (cut after our DIY base-bump landed upstream as polkadot-evm/frontier#1892); evm stays on the 0.43.x line; ethereum off upstream/master.
  • moonkit is on an interim branch. moonbeam-polkadot-stable2603 is cut off the head of the still-open base-bump PR Moonsong-Labs/moonkit#95 (main + base-bump, linear). It must be reconciled against the merge/squash commit once that PR lands in main — see follow-ups.
  • No [patch.crates-io] — the fork branches resolve directly from the moonbeam-foundation (and Moonsong-Labs for moonkit) remotes.
  • --all-features is NOT a valid check for moonbeam. It forces the mutually-exclusive disable-genesis-builder feature on, which strips genesis_config_preset (needed by the node's chain_spec). Verification uses the realistic feature matrix (default / runtime-benchmarks / try-runtime).
  • Scope of this PR. It gets the workspace compiling green and the dev/integration fixtures updated. No spec_version bump, no StorageVersion migrations, and no weight regeneration are included here — those are deliberately deferred (see follow-ups). Reviewers should treat this as the dependency/API-drift layer, not a runtime-behavior change.

Is there something left for follow-up PRs?

Yes — the post-compile phases of the upgrade plan:

  • Runtime / weights / version (Phase 4): re-run benchmarks for every pallet whose weight signature changed, run weight-regression analysis over the diff, bump spec_version in each runtime, author migrations for any pallet whose StorageVersion advanced upstream, and run try-runtime against production state for moonbase / moonriver / moonbeam.
  • Bridge maintenance (Phase 5): rebuild substrate-relay against stable2603, regenerate zombienet relay chain specs, and run the bridge integration tests.
  • moonkit reconciliation: once moonkit#95 merges to main, reset/rebase the moonbeam-polkadot-stable2603 release branch onto the merge commit and confirm content parity.
  • Tracker finalization (Phase 2): run merge-base verification per fork for the rows still marked Verify/TBD in polkadot-sdk-stable2603.md.
  • Full verification sweep (Phase 6): workspace tests, smoke tests, and XCM cross-chain scenarios.

What alternative implementations were considered?

  • frontier base — wait vs. DIY. polkadot-evm/frontier had not cut stable2603 when this started. Rather than block, we did the base-bump ourselves; it was then opened upstream (#1892) and squash-merged, after which upstream cut stable2603. We rebased onto the official branch and dropped the DIY one.
  • moonkit base — wait vs. author + interim branch. No upstream base-bump existed, so we authored #95 and cut an interim release branch off its head to unblock moonbeam Phase 3, rather than waiting for the PR to merge first.
  • evm — stay on 0.43.x vs. move to v1.0. rust-ethereum/evm upstream has moved to v1.0; we stay on the 0.43.x fork (the stable2603 branch is effectively a rename of stable2512). Moving to v1.0 was rejected as out of scope — any new upstream EIP/gas work must be backported onto 0.43.x instead.
  • erc20 in the credit model — real Credit vs. notional credit. erc20 tokens are EVM-side balances with no Substrate fungible::Credit to hold. Mirroring the executor's own MockCredit, a notional amount-only credit threads the asset through holding while the real token movement stays in the EVM withdraw_asset/deposit_asset calls.

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

Upstream changes that drove this work:

  • polkadot-sdk: #9947 (proof-recording refactor; removed cumulus-client-consensus-proposer), #10582 (RuntimeDebug removal). Backports absorbed via rebase: #11964, #11856, #11987, #12017.
  • frontier: #1892 (the base-bump we authored, merged upstream → stable2603 cut). Numerous prior moonbeam cherry-picks are now inherited from upstream/stable2603 (EIP-7939/7883/7823/7825, several eth_* RPC correctness fixes, etc.) — full reconciliation in the tracker.
  • moonkit: #95 (base-bump, open), with #92 and #94 already inherited from main.
  • evm: rust-ethereum/evm#400 (EIP-7939 CLZ, inherited from the 0.43.x base), plus #405 (stale delegation.rs test fix, upstreamed).
  • ethereum: rust-ethereum/ethereum#77 (encoded-length methods, still carried as a cherry-pick).

Per-fork branch + cherry-pick detail lives in docs/cherry-picks/polkadot-sdk-stable2603.md.

What value does it bring to the blockchain users?

Keeping Moonbeam current with the latest polkadot-sdk stable line is what lets users benefit from upstream security fixes, performance improvements, and new protocol features, and keeps the maintenance gap to upstream small. The frontier upgrade also folds in a batch of EVM-RPC correctness fixes (block-resolution, receipt-race, and eth_* consistency fixes now inherited from upstream) and additional Osaka-era EIP support. The user-facing runtime impact is realized once the follow-up Phase 4 work (version bump + weights + migrations) ships; this PR is the dependency/API-drift foundation that work builds on.

⚠️ Breaking Changes ⚠️

This upgrades polkadot-sdk (and the frontier/evm/ethereum/moonkit forks) from stable2512 to stable2603. Besides the internal API churn, the following are externally-visible breaking changes:

  • SessionKeys runtime API → V2. generate_session_keys now takes an owner argument and returns OpaqueGeneratedSessionKeys instead of Vec<u8>. Tooling that calls the runtime API directly to rotate/generate session keys must be updated.
  • EthereumRuntimeRPCApi::call signature change. It gained a Geth-style state_override parameter (API version bump). Code calling the Ethereum runtime API directly (custom eth_call/tracing tooling) must pass the new argument.
  • EVM pallet_evm::Runner::call gained a state_override argument (frontier). Any integration building EVM calls against the Runner trait must update.
  • XCM AssetsInHolding redesign. The XCM executor now tracks real fungible::Credit imbalances instead of Asset descriptors. The TransactAsset (deposit_asset/withdraw_asset/internal_transfer_asset), WeightTrader (buy_weight/refund_weight), and FeeManager::handle_fee signatures changed; custom XCM asset transactors / weight traders must migrate.
  • RuntimeDebug removed from sp_core/sp_runtime/frame_support (use Debug). Downstream crates depending on moonbeam/runtime types must adjust.
  • Proof-recording refactor. sp_consensus::Proposer lost its Proof associated type and cumulus-client-consensus-proposer was removed; manual-seal / dev-service tooling relying on these must update.

No runtime storage migrations are introduced in this PR; StorageVersion bumps and weight regeneration are tracked as follow-up (Phase 4) work.

Draft tracker carries forward Included rows from stable2512 with TBD
commit hashes; rows whose upstream PR is expected to be in stable2603
are pre-marked Dropped pending merge-base verification. UPGRADE doc is
a temporary checklist covering forks, moonbeam Cargo.toml swap,
benchmarks, bridge regen, migrations, and verification.
Resolved upstream bases for polkadot-sdk, evm, ethereum. Two new
blockers: polkadot-evm/frontier has not branched stable2603 yet, and
Moonsong-Labs/moonkit has no stable2603 base-bump PR. Also noted
frontier #1856 is now in upstream/master (tracker said "PR not
merged"); needs correction during verification.
We will author the upstream base-bump PRs ourselves rather than wait
on polkadot-evm/frontier and Moonsong-Labs/moonkit. Adds Phase 1.4a
(moonkit) and Phase 1.5a (frontier) covering the upstream branches we
own, and notes the rebase plan once polkadot-evm cuts an official
frontier stable2603.
Branch moonbeam-polkadot-stable2603 pushed with 4 cherry-picks on top
of polkadot-stable2603-1. Three previously-Included rows were verified
to be in stable2603 upstream and are now Dropped in the tracker:
charge_transaction_payment benchmark fix (#10444), storage benchmark
--keys-limit, and pallet-revive removal from pallet-xcm.

PrecompileWasmCmd needed an adaptation for stable2603's
BackendRuntimeCode::new(state, TryPendingCode) signature change.
@manuelmauro manuelmauro self-assigned this May 8, 2026
@manuelmauro manuelmauro added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited breaking Needs to be mentioned in breaking changes XCM Run option xcm-emulator tests labels May 8, 2026
@coderabbitai

coderabbitai Bot commented May 8, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Bumps workspace git pins to moonbeam-polkadot-stable2603; migrates XCM TransactAsset APIs to AssetsInHolding with NotionalImbalance; updates runtime/EVM call signatures, node lazy-loading/manual-seal wiring, XCM executor configs, mocks, derives, and tests.

Changes

Stable2603 Upgrade & XCM Asset Holding Migration

Layer / File(s) Summary
Dependency upgrade and upgrade docs
Cargo.toml, docs/cherry-picks/*
Workspace git pins moved from moonbeam-polkadot-stable2512moonbeam-polkadot-stable2603; upgrade planning and cherry-pick tracker docs added/updated.
Runtime API & EVM call signature updates
runtime/common/src/apis.rs, runtime/common/src/impl_xcm_evm_runner.rs, pallets/moonbeam-foreign-assets/src/evm.rs, runtime API tests
Session key generation now takes an owner and returns opaque session keys; Ethereum RPC call and EVM runner Runner::call add a state_override parameter; call sites and tests updated; benchmark wiring adjusted.
Notional imbalance and TransactAsset migration
pallets/erc20-xcm-bridge/*, pallets/moonbeam-foreign-assets/*, pallets/*/notional.rs, tests
Introduces NotionalImbalance; TransactAsset impls now accept AssetsInHolding and return Result<(), (AssetsInHolding, XcmError)>; withdraw/deposit/mint internals updated to use notional credits and storage transactions; ERC20 bridge trap/claim added.
XCM weight trader and mocks
pallets/xcm-weight-trader/*, pallets/xcm-transactor/src/mock.rs, precompiles/*/src/mock.rs, precompiles/xcm-utils/src/mock.rs, precompiles/xtokens/src/mock.rs
Trader stores withheld fees as AssetsInHolding; buy_weight/refund_weight/Drop updated; many mock TransactAsset/WeightTrader signatures and test helpers adjusted to new AssetsInHolding and error-tuple shapes.
Node service lazy-loading & manual sealing
node/service/src/lazy_loading/*, node/service/src/lib.rs, node/service/Cargo.toml, node/service/src/lazy_loading/substrate_backend.rs
CallExecutor::runtime_version gains CallContext; manual sealer removes explicit proof generic and uses ProposeArgs with ProofRecorder; build_network calls pass spawn_essential_handle; set_block_data signature extended.
XCM executor config rewiring (runtimes & mocks)
runtime/*/src/xcm_config.rs, runtime/*/tests/xcm_mock/*
Removed AssetClaims associated type and re-wired Trader, ResponseHandler, and AssetTrap across moonbase/moonbeam/moonriver and test mocks.
Debug derive consolidation & runtime limits
various pallets/*, primitives/*, runtime/*
Replaced RuntimeDebug with standard Debug derives across many public types/enums; BlockLength config updated to builder API in runtimes.
Integration & unit test adjustments
test/suites/dev/*, pallets/*/tests.rs, pallets/xcm-weight-trader/src/tests.rs
Tests updated to reflect new XCM holdings/return types, EVM state_override argument, mocked relay expectations, and XCM fee extraction from balances.Deposit events.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

dependencies

Suggested reviewers

  • librelois
  • arturgontijo
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore-update-polkadot-sdk-stable2603

The tracker `polkadot-sdk-stable2603.md` was assembled from the prior
cycle's tracker plus known deltas — it is not guaranteed to capture
every cherry-pick that has actually landed on each fork's
`moonbeam-polkadot-stable2512` branch.

Insert a pre-Phase-1 audit that enumerates every commit unique to
`origin/moonbeam-polkadot-stable2512` for each fork (polkadot-sdk,
frontier, evm, ethereum, moonkit) and reconciles it against the
tracker, so undocumented cherry-picks surface and get rows added
before re-application starts. Parallelizable via one sub-agent per
fork.
Run the Phase 0.5 audit against `origin/moonbeam-polkadot-stable2512`
on each fork using PR-number set-difference — raw SHA-diff is
misleading because both we and upstream apply the same backport PRs
under different SHAs.

Findings per fork:

* polkadot-sdk — 106 of 108 PR refs on our fork are also on
  upstream/stable2512 (they will inherit on rebase). Three new rows
  for the genuinely moonbeam-only commits: weight reclaim log
  improvements, an xcm-emulator BlockProducer trait override for
  Nimbus, and a bridges GRANDPA-justification experiment plus its
  revert.

* frontier — upstream/stable2512 has been frozen since 2026-01-13,
  so all 35 fork-only commits are moonbeam-authored. Six new rows
  (#1881 logs journal memory bound, #247 CI triggers, canonical
  hash mapping repair, Saturate U256, configurable tx gas-limit cap,
  MBF ethereum fork pin) and three tracker corrections: row #1820
  flipped from `Applied: No` to `Yes` (commit IS on the branch);
  rows previously listing PRs #1794 and #1787 corrected to #1824
  and #1862 (the PR numbers were typos for what is actually on the
  branch).

* evm — one moonbeam-authored commit (MBF ethereum fork dependency
  pin) added as a row.

* moonkit — one row for upstream PR #94 (relay offset dynamic);
  revisit during Phase 1.4 once the moonkit base-bump lands.

UPGRADE-stable2603.md gets the methodology note explaining why we
use PR-number set-difference, a per-fork results table, and a Phase
2 follow-up list for rows still TBD on Included vs Dropped.
Sixteen rows in the stable2603 and stable2512 trackers carried no
commit SHA and described upstream PRs that landed on our fork via
merging upstream/stable2512, not via moonbeam cherry-picks. The
Phase 0.5 PR-number set-difference confirmed every one of them is
also on upstream/stable2512 — they do not belong in a cherry-pick
tracker.

stable2506 keeps its rows for the same PR numbers because there
every row carries a real moonbeam-foundation commit SHA and
`Cherry pick: Included` — those were genuine cherry-picks of that
release cycle, not upstream content carried over.
Operational changes on the polkadot-sdk fork branch:

1. Rebase `moonbeam-polkadot-stable2603` from the
   `polkadot-stable2603-1` tag onto `upstream/stable2603` head
   (`afb51b7a8c6`), absorbing four upstream backports past the tag
   (#11964, #11856, #11987, #12017).

2. Cherry-pick the two Phase 0.5 finds that were missing from the
   branch:
   - `improve weight reclaim logs (call metadata, warn level)` →
     `161cd252773`.
   - `xcm-emulator: make slot/digest producer overridable for
     non-Aura parachains` → `beaf6b6c50a`. Trivial additive conflict
     with stable2603's `native_total_supply_tracker` macro arm,
     resolved by keeping both arms.

Doc updates:

- Refresh the six polkadot-sdk row commit hashes in
  `polkadot-sdk-stable2603.md` to match the post-rebase SHAs.
- Extend the Phase 1.1 checklist in `UPGRADE-stable2603.md` with the
  two new cherry-picks plus an explicit `Drop` for the bridges
  GRANDPA retry experiment that was reverted on stable2512.
@manuelmauro manuelmauro force-pushed the chore-update-polkadot-sdk-stable2603 branch from 4e7949e to 87c1d45 Compare May 8, 2026 10:49
…write

Rewrote the commit messages of all six cherry-picks on
`moonbeam-foundation/polkadot-sdk:moonbeam-polkadot-stable2603` to
explain *why* each cherry-pick exists, not just *what* it does. In
particular, the auto-generated "Merge pull request #8" subject was
replaced with a real description of the pallet-parameters benchmark
fix. Each rewritten message now mirrors the context that lived only
in the moonbeam tracker, so the polkadot-sdk fork is self-explanatory
to anyone reading `git log` without the tracker open.

The rewrite changed all six SHAs. Update the tracker's commit links
and the Phase 1.1 checklist in `UPGRADE-stable2603.md` to point at
the new SHAs.
@manuelmauro manuelmauro force-pushed the chore-update-polkadot-sdk-stable2603 branch from 5353713 to 27487be Compare May 8, 2026 10:58
…2603

The row in `polkadot-sdk-stable2603.md` claimed `Applied: Yes` and
`Dropped but needs refactoring` against `paritytech/polkadot-sdk#9214`,
but that PR was never cherry-picked — the stable2506 tracker noted
"Not found on the branch — may not have been carried over". The
relevant code (`ParachainTracingExecuteBlock`) reaches stable2603 via
upstream's own backport of `#9871`, which superseded `#9214` (the
prdoc shipping at `prdoc/stable2509-2/pr_9871.prdoc` is upstream's
own metadata). It is pure upstream content, not a moonbeam
cherry-pick, so it does not belong in the active-cycle cherry-pick
tracker.

The moonbeam-side follow-up — wiring
`Some(Arc::new(ParachainTracingExecuteBlock::new(...)))` into the
parachain service — is already captured in the Phase 3 checklist of
UPGRADE-stable2603.md, so no information is lost.

The matching row in `polkadot-sdk-stable2512.md` is preserved for
historical reasons: it documents `moonbeam-foundation/polkadot-sdk#20`,
the attempted cherry-pick of `#9214` that was eventually superseded
by upstream's `#9871`. Same policy as the named cherry-pick rows in
`polkadot-sdk-stable2506.md`.
@manuelmauro manuelmauro force-pushed the chore-update-polkadot-sdk-stable2603 branch from 8a0042f to 402ca09 Compare May 8, 2026 11:16
manuelmauro added 11 commits May 8, 2026 14:21
Created `moonbeam-polkadot-stable2603` on
`moonbeam-foundation/ethereum`. Since `rust-ethereum/ethereum`'s
master has not advanced past the merge-base with our
`moonbeam-polkadot-stable2512` branch (`d7bdf2888253a30f160d434688e378636e253870`,
which is also master's head), the new branch shares the same tip SHA
as stable2512 (`58a5a8a`) — the only commit ahead of upstream is the
unmerged rust-ethereum/ethereum#77 cherry-pick, which is preserved
as-is.

Tick Phase 1.3 boxes in UPGRADE-stable2603.md and replace the `TBD`
placeholder in the ethereum row of `polkadot-sdk-stable2603.md` with
the actual commit link.
Created `moonbeam-polkadot-stable2603` on `moonbeam-foundation/evm`.
Upstream `rust-ethereum/evm` has moved to v1.0; the moonbeam fork
stays on the 0.43.x line and there is no new upstream commit to
pull. The new branch shares the same tip SHA as stable2512
(`bb9cdde4`) so both moonbeam-only commits — `a656db90` (the
EIP-7939 CLZ-opcode cherry-pick of rust-ethereum/evm#400, which was
merged upstream only on v1.0) and `bb9cdde4` (the MBF ethereum fork
dep pin discovered in Phase 0.5) — are inherited as-is.

Tick Phase 1.2 boxes in UPGRADE-stable2603.md and replace the `TBD`
placeholder in the EIP-7939 row of `polkadot-sdk-stable2603.md` with
the actual commit link.
…3 and stable2512

The "Refactor transaction signature validation" row referenced
rust-ethereum/ethereum#75, which is merged into upstream master at
`d7bdf28` — exactly the merge-base for our `moonbeam-polkadot-stable2603`
and `moonbeam-polkadot-stable2512` branches. The change is therefore
on our branches via upstream, not via cherry-pick.

The row carried no commit SHA and no moonbeam-side PR link, and the
`Applied` field disagreed across cycles (`No` in stable2603, `Yes` in
stable2512). It was not documenting a moonbeam cherry-pick action, so
it does not belong in either active or recent cherry-pick tracker —
same policy as the bulk of the upstream-only Backport PR rows we
removed earlier.

stable2506 keeps its version of the row because there it carries a
real `[moonbeam-foundation/ethereum@933ccae]` commit reference: that
documents the actual moonbeam-side work — a fork commit was
prepared, never applied (Applied: No, Cherry pick: Included), and
the PR was eventually merged upstream, making the cherry-pick moot.
That is the historically meaningful record; the 2512/2603 copies had
lost the SHA and only kept a now-redundant pointer to the upstream
PR.
Created `moonbeam-foundation/frontier:moonbeam-polkadot-stable2603`
on top of `upstream/stable2603` (`baf505d8f`) with 12 cherry-picks
and one manual `Cargo.toml` dep-redirect commit:

- Cherry-picks (#247 CI triggers, #1546 withdraw-ability, #1547
  ethereum execution info, #203 dispatch-error decoding,
  #1564+#224 squashed tx-size, #244 POV underestimations,
  #1568 lru_cache, #252 parity-db migration, frame-metadata,
  #254 validate tx size, Saturate U256, canonical hash mapping
  repair).
- One conflict on POV Underestimations (resolved by keeping
  stable2603's `match` block while adding `mut`); one trivial
  conflict on parity-db migration (kept upstream's deref form).
- Manual dep-redirect commit replaces the prior cycle's two
  cherry-picks (CI branch ref + MBF ethereum fork pin), pointing
  polkadot-sdk, ethereum, and evm at moonbeam-foundation forks on
  the `moonbeam-polkadot-stable2603` branch.

Phase 0.5 follow-ups resolved:
- 17 frontier PRs were inherited from `upstream/stable2603` (it
  was cut from the same master commit that absorbed our
  Phase 1.5a base-bump as PR#1892); their tracker rows are now
  confirmed `Dropped, PR Upstream Merged`.
- Three Phase 0.5–added rows flipped from TBD/TBD to Dropped:
  #1881 logs journal memory, #1856 latest-on-pruned, and the
  no-PR "Make tx gas limit cap configurable" (upstreamed as
  `b2088f29b`).

Side effect: `moonbeam-foundation/evm:moonbeam-polkadot-stable2603`
bumped to `7dd6ecc6` so its `ethereum` dep points at the stable2603
branch (was stable2512). Without this, cargo pulls in two distinct
versions of the `ethereum` crate when frontier consumes both.

Phase 1.5a is also marked complete because upstream merged our DIY
base-bump as polkadot-evm/frontier#1892 and cut `stable2603` from
the resulting master commit; our local `mb/polkadot-sdk-stable2603`
branch was redundant and has been dropped.

`cargo check --workspace` on the new frontier branch is clean (one
harmless unused-const warning from the dispatch-error cherry-pick).
- Add an evm row for the `delegation.rs` test-module fix
  (moonbeam-foundation/evm@a122857), upstreamed as rust-ethereum/evm#405.
- Correct the EIP-7939 row: PR #400 merged into `rust-ethereum/evm:v0.x`,
  so it is inherited from the upstream base, not a moonbeam cherry-pick
  (`Included` -> `Dropped`). Fix the matching "v1.0 only" claim in the
  Phase 1.2 plan.
Create moonbeam-polkadot-stable2603 off the open base-bump PR head
(mb/polkadot-sdk-stable2603, PR #95) to unblock moonbeam Phase 3 while
the upstream review is pending. PR #95 is linearly main + base-bump, so
the release branch already equals what it would be cut from the merge
commit; reconcile once #95 lands in main.

Verified the moonkit cherry-pick table: #92 (using_fake_author) and #94
(make relay offset dynamic) are both inherited from main, so no extra
cherry-picks are needed. Flip the #94 TBD row to Dropped/PR Merged and
add the #95 link to the base-bump row.
…al drift

Phase 3 of the polkadot-sdk stable2512 -> stable2603 upgrade.

- Cargo.toml: repoint all 180 fork deps to the moonbeam-polkadot-stable2603
  branches (polkadot-sdk, frontier, evm, ethereum, moonkit).
- Cargo.lock: re-resolved to stable2603. moonkit bumped to ba06fb0 (which
  redirects its polkadot-sdk/frontier deps to the moonbeam-foundation forks),
  unifying the tree on a single SDK source and eliminating a duplicate
  polkadot-sdk (canonical paritytech alongside the fork) that caused E0221/
  E0308 ambiguous-associated-type errors.
- Drop removed crate cumulus-client-consensus-proposer (upstream PR #9947 folded
  it into sp-consensus/sc-basic-authorship); it was an unused dependency.
- Bump num_enum to 0.7.6 to satisfy frontier fp-evm's new ^0.7.6 requirement.
- RuntimeDebug -> Debug across 51 sites / 14 files (upstream PR #10582 removed
  RuntimeDebug from sp_core/sp_runtime/frame_support; it is now plain Debug).
- pallet_evm Runner::call: add the new state_override argument (8 call sites).

Known remaining: erc20-xcm-bridge and moonbeam-foreign-assets need migrating to
the credit-based AssetsInHolding model (stable2603 XCM redesign); tracked
separately.
Scope-mapping checkpoint. stable2603 reworked AssetsInHolding to hold
fungible::Credit imbalances instead of Asset descriptors; the WeightTrader,
TransactAsset and FeeManager trait surfaces moved with it.

Mechanical fixes applied:
- TransactAsset::internal_transfer_asset now returns Result<Asset> (was
  Result<AssetsInHolding>); drop the .into().
- FeeManager::handle_fee now takes AssetsInHolding (was Assets).

todo!()-stubbed pending the real credit-based migration (tracked):
- erc20-xcm-bridge / moonbeam-foreign-assets: deposit_asset, withdraw_asset.
- xcm-weight-trader: buy_weight, refund_weight, and the Drop fee re-deposit.

These stubs let the compile-fix loop proceed past the XCM pallets to reveal
the runtime-layer drift. DO NOT SHIP without completing the migration.
Runtime-integration fixes that get all three runtimes compiling.

B — signature drift in the shared runtime macros (runtime/common):
- pallet_evm Runner::call gained a state_override (Geth-style) argument; thread
  it through apis.rs (API call + tracing call) and impl_xcm_evm_runner.rs.
- fp_rpc EthereumRuntimeRPCApi::call gained a state_override parameter.
- SessionKeys::generate_session_keys now takes an owner and returns
  OpaqueGeneratedSessionKeys (V2).

C — cumulus_pallet_parachain_system::WeightInfo gained three methods
  (block_weight_tx_extension_{max_weight,stays_fraction_of_core,full_core});
  add them (Weight::zero, matching upstream) to all three runtime weight files.

XCM Config — stable2603 merged asset claiming into the trap config: AssetClaims
  was removed from xcm_executor::Config and AssetTrap must now also implement
  ClaimAssets. Implement ClaimAssets for AssetTrapWrapper (delegates to the inner
  claimer; erc20 assets are filtered on drop so plain delegation is correct) and
  drop the now-invalid type AssetClaims from each runtime's xcm_config.
Gets moonbeam-service compiling; with this, cargo check --workspace passes
on default features.

- sc_service/cumulus BuildNetworkParams gained spawn_essential_handle.
- new_full_parts_record_import gained a pruning_filters argument.
- sc_client_api::CallExecutor::runtime_version gained a call_context param;
  Backend::set_block_data gained a register_as_leaf param (lazy-loading impls).
- moonkit: NimbusManualSealConsensusDataProvider dropped its _phantom field;
  MockValidationDataInherentDataProvider gained relay_parent_offset.
- Proof-recording refactor (upstream #9947): sp_consensus::Proposer lost its
  Proof associated type and Proposal lost its proof field. Rewrite the
  lazy-loading manual-seal (run_manual_seal/seal_block) to mirror upstream:
  drop the P generic, record the storage proof via a ProofRecorder wired into
  the proposal extensions, and drain it after proposing.
Replaces the scope-mapping stubs with real implementations of the stable2603
credit-based holding model (AssetsInHolding now carries fungible::Credit
imbalances rather than Asset descriptors).

erc20 tokens are EVM-side balances, not a Substrate fungible, so they have no
real Credit. Introduce a notional (amount-only) ImbalanceAccounting credit
(mirroring the executor's MockCredit) to represent erc20 amounts in holding;
the real token movement still happens via EVM calls.

- erc20-xcm-bridge & moonbeam-foreign-assets: real deposit_asset/withdraw_asset.
  withdraw returns AssetsInHolding::new_from_fungible_credit(id, NotionalImbalance);
  deposit iterates the holding's fungible assets (collected first so the holding
  can be returned unspent on error) and performs the EVM transfer/mint, or
  records a pending deposit when frozen.
- xcm-weight-trader: real buy_weight/refund_weight. The Trader now holds the
  withheld fee as an AssetsInHolding (was Option<Asset>): buy_weight try_take()s
  the fee from payment and subsumes it; refund_weight splits the refund back out;
  Drop deposits the remainder to the fees account via the new deposit_asset.

cargo check --workspace is green on default features.
- Cargo.lock: bump moonbeam-foundation/polkadot-sdk to ddba2453, which adds the
  missing GetCallMetadata bound to the cumulus-pallet-weight-reclaim benchmark
  (the weight-reclaim logging cherry-pick changed the TransactionExtension impl
  bound but not the benchmark where-clause, breaking runtime-benchmarks builds).
- apis.rs: pallet_transaction_payment::benchmarking is now private; reference the
  benchmark via pallet_transaction_payment::Pallet and the re-exported
  BenchmarkConfig.
- apis.rs: XcmBenchmarks worst_case_holding now returns AssetsInHolding. Build it
  with the canonical pallet_xcm_benchmarks::generate_holding_assets helper and
  register the held assets for the weight trader as before.
…ord Phase 3

- moonbeam-foreign-assets tests: deposit_asset now takes an owned AssetsInHolding
  and returns Result<(), (AssetsInHolding, XcmError)>. Add a holding() helper that
  wraps a single asset in a notional-credit holding, and map the error tuple back
  to XcmError in the error-path assertions. 17/17 tests pass.
- UPGRADE-stable2603.md: mark Phase 3 done across the realistic feature matrix
  (default / runtime-benchmarks / try-runtime), document the drift handled, the
  XCM credit-holding migration, the fork-side fixes (moonkit ba06fb0, sdk
  ddba2453), and why blanket --all-features is invalid (disable-genesis-builder).
Brings the stable2603 upgrade branch up to date with master (spec version 4500,
dependabot dep bumps, Rust/TS bindings). Conflicts resolved:

- parachain-staking/types.rs (Bond, DelegatorStatus): combine both sides —
  keep the stable2603 RuntimeDebug->Debug change and master's added
  MaxEncodedLen derive / codec(mel_bound).
- node/service lib.rs + lazy_loading/mod.rs: stable2603's
  MockValidationDataInherentDataProvider has a dedicated relay_parent_offset
  field (relay-parent descendants), so map master's runtime-API
  relay_parent_offset variable to that field and keep relay_offset as just the
  additional offset (master had folded them together for lack of the field).
- Cargo.lock: re-resolved from the merged Cargo.toml on the stable2603 base.

cargo check --workspace is green.
Production fix:
- moonbeam-foreign-assets: implement the new TransactAsset::mint_asset. stable2603
  added mint_asset, which the XCM executor calls for ReserveAssetDeposited and
  ReceiveTeleportedAsset to put incoming assets into holding (the default is
  Unimplemented). Without it, every reserve transfer into the chain failed
  (assets never entered holding), which broke ~all XCM cross-chain tests. erc20
  has no real fungible::Credit, so mint_asset returns a notional-credit holding
  (mirroring withdraw_asset without the burn); the actual erc20 mint stays in
  deposit_asset. erc20-xcm-bridge keeps the Unimplemented default (local erc20s
  use the trace/deposit path, and the tuple transactor falls through on it).

Test fixes (TransactAsset/WeightTrader signature changes + AssetsInHolding):
- precompile/pallet mocks (xcm-utils, xcm-transactor, gmp, relay-encoder,
  xtokens, weight-trader): update deposit_asset/withdraw_asset/buy_weight to the
  new signatures; AssetsInHolding::default() -> ::new(); migrate the real
  checked_sub trader to try_take.
- drop the removed type AssetClaims from all xcm_mock/test runtime configs.
- xcm-weight-trader tests: add a notional test credit + holding() builders, map
  the (AssetsInHolding, XcmError) error tuple for assertions.
- runtime_apis tests (x3): pass the new state_override arg to EthereumRPCApi::call.

cargo test --workspace --no-fail-fast is green (previously 81 XCM tests failed).
Regenerate the polkadot.js typegen bindings (api-augment) for all three runtimes
from the stable2603 runtime metadata.

- New cumulus types: CumulusPalletParachainSystemBlockWeightBlockWeightMode,
  CumulusPalletParachainSystemPoVMessages, CumulusPalletXcmpQueueOutboundChannelFlags.
- New parachain-system storage (blockWeightMode, poVMessagesTracker, etc.).
- Drop the SessionKeys runtime-API V1 binding (the API moved to V2 with the
  owner/OpaqueGeneratedSessionKeys change; polkadot.js typegen only ships the V1
  definition).
- Various upstream doc-comment updates.

tsc typechecks clean for moonbase/moonbeam/moonriver.
…ents

dev_moonbase/moonbeam/moonriver TypeScript suites now pass (previously 4 unique
failing files).

- test-block-mocked-relay / test-precompile-relay-verifier: stable2603 routes the
  relay-parent offset through the cumulus mock's dedicated relay_parent_offset
  (async-backing descendants) instead of folding it into relay_offset, so the dev
  relayParentNumber is the un-shifted parent (0, 1) and latestRelayBlockNumber is 1.
  This matches the relay offset not being applied to relay_offset (reverted in
  production). Note: relay_parent_offset must stay = the runtime value in the dev
  service — zeroing it makes async-backing panic ("Relay slot to exist").

- test-transactional-outcomes: stable2603's credit-based asset model moves native
  value via Withdraw/Deposit events rather than minting (no balances.Minted), so the
  execution fee is now derived from the treasury Deposit. Behaviour is unchanged:
  the sovereign is debited the full DEPOSIT and Baltathar receives DEPOSIT minus the
  fee even though the erc20 leg of the multi-asset DepositAsset fails.

test-delegate-with-auto-compound7 passes unchanged once the runtime wasm is present;
its earlier ENOENT was a local debug-build artifact (compact.compressed.wasm is only
emitted by release builds), not a code/fixture regression.
…atio

CI builds with -D warnings, which promotes the stable2603 deprecation of
frame_system::limits::BlockLength::max_with_normal_ratio to a hard error.
The recommended BlockLength::builder().normal_ratio(..) helper is not yet
present in the pinned polkadot-sdk fork, so reproduce the same limits with
the builder + modify_max_length_for_class (Normal = ratio * max, other
classes = max). Behavior is unchanged.
@manuelmauro manuelmauro marked this pull request as ready for review June 9, 2026 09:51
…recording

stable2603 removed the proof-recording type marker from ProposerFactory;
with_proof_recording is now a deprecated shim that forwards to new(), so the
two are behaviorally identical (proof recording is handled at the propose
layer). CI builds with -D warnings, which turned the deprecation into a hard
error in moonbeam-service. Swap the 3 collator call sites to new().

@coderabbitai coderabbitai 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.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
pallets/xcm-weight-trader/src/mock.rs (1)

137-163: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reject multi-asset holdings explicitly in this mock transactor.

deposit_asset processes only the first fungible asset from what and returns success, which can silently drop additional assets and hide failures in tests. Please enforce “exactly one fungible asset” (or iterate and handle all) before depositing.

Suggested patch
-		// Extract the (single) fungible asset from the holding.
-		let asset = what.fungible_assets_iter().next();
-		let asset = match asset {
+		// This mock supports exactly one fungible asset; reject bundles.
+		if what.fungible_assets_iter().count() != 1 {
+			return Err((what, XcmError::AssetNotFound));
+		}
+		let asset = match what.fungible_assets_iter().next() {
 			Some(asset) => asset,
 			None => return Err((what, XcmError::AssetNotFound)),
 		};
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pallets/xcm-weight-trader/src/mock.rs` around lines 137 - 163, The mock
transactor currently takes only the first fungible asset from `what` and can
silently drop others; in the `deposit_asset` implementation check that
`what.fungible_assets_iter()` contains exactly one fungible asset before
proceeding (e.g., pull the first item, then assert the iterator has no second
item), and if there are zero or more-than-one fungible assets return an explicit
Err((what, XcmError::...)) (choose an appropriate XCM error variant or add a new
one) instead of proceeding; adjust the logic around `XcmAssetId(location)` /
`Fungibility::Fungible(amount)` to run only after this exact-one check so extra
assets are rejected rather than ignored.
node/service/src/lazy_loading/substrate_backend.rs (1)

605-613: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Propagate register_as_leaf instead of discarding it.

The new parameter is ignored, but leaf tracking is applied unconditionally later. If callers pass false, this backend will still register leaves and diverge from import semantics.

Suggested fix
 struct PendingBlock<B: BlockT> {
 	block: StoredBlock<B>,
 	state: NewBlockState,
+	register_as_leaf: bool,
 }

 // ...
 fn set_block_data(
 	&mut self,
 	header: <Block as BlockT>::Header,
 	body: Option<Vec<<Block as BlockT>::Extrinsic>>,
 	_indexed_body: Option<Vec<Vec<u8>>>,
 	justifications: Option<Justifications>,
 	state: NewBlockState,
-	_register_as_leaf: bool,
+	register_as_leaf: bool,
 ) -> sp_blockchain::Result<()> {
 	// ...
 	self.pending_block = Some(PendingBlock {
 		block: StoredBlock::new(header, body, justifications),
 		state,
+		register_as_leaf,
 	});
 	Ok(())
 }
- pub fn insert(..., new_state: NewBlockState) -> sp_blockchain::Result<()> {
+ pub fn insert(..., new_state: NewBlockState, register_as_leaf: bool) -> sp_blockchain::Result<()> {
   // ...
-  storage.leaves.import(hash, number, *header.parent_hash());
+  if register_as_leaf {
+      storage.leaves.import(hash, number, *header.parent_hash());
+  }
 }
 self.blockchain.insert(
   hash,
   header,
   justification,
   body,
-  pending_block.state
+  pending_block.state,
+  pending_block.register_as_leaf
 )?;

Run this to verify current behavior and impact scope:

#!/bin/bash
set -euo pipefail

echo "1) Updated trait method signature and ignored parameter:"
rg -n "fn set_block_data\\(|register_as_leaf|_register_as_leaf" node/service/src/lazy_loading/substrate_backend.rs -C 3

echo
echo "2) Leaf registration path (check if conditional or unconditional):"
rg -n "leaves\\.import\\(" node/service/src/lazy_loading/substrate_backend.rs -C 3

echo
echo "3) Local call sites passing set_block_data args in lazy-loading code:"
rg -n "\\.set_block_data\\(" node/service/src/lazy_loading -C 4
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@node/service/src/lazy_loading/substrate_backend.rs` around lines 605 - 613,
The new boolean parameter _register_as_leaf is ignored in set_block_data causing
unconditional leaf registration; update set_block_data (and any local call
sites) to propagate the caller's register_as_leaf value so the leaves.import
call is executed only when register_as_leaf is true: locate the set_block_data
function in substrate_backend.rs, replace the unconditional leaves.import(...)
invocation with a conditional guarded by the register_as_leaf parameter (or
rename _register_as_leaf to register_as_leaf and use it), and update any callers
of set_block_data to pass the intended boolean so import semantics match the
caller's intent.
🧹 Nitpick comments (1)
pallets/moonbeam-foreign-assets/src/notional.rs (1)

31-60: 💤 Low value

Consider extracting NotionalImbalance to a shared crate.

This type is identical to the one in pallets/erc20-xcm-bridge/src/notional.rs. Extracting it to a shared primitives crate (e.g., primitives/xcm-notional or similar) would avoid the duplication and ensure both pallets stay in sync.

This is a low-priority suggestion since the current approach keeps each pallet self-contained.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pallets/moonbeam-foreign-assets/src/notional.rs` around lines 31 - 60, The
NotionalImbalance type and its trait impls (NotionalImbalance,
UnsafeConstructorDestructor::unsafe_clone/forget_imbalance,
UnsafeManualAccounting::saturating_subsume, and
ImbalanceAccounting::amount/saturating_take) are duplicated in another pallet;
extract this struct and its impls into a shared primitives crate (e.g.,
primitives/xcm-notional) and replace the local definition with a re-export or
use of that shared type in pallets/moonbeam-foreign-assets and
pallets/erc20-xcm-bridge so both pallets import the single canonical
NotionalImbalance implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/cherry-picks/polkadot-sdk-stable2603.md`:
- Line 35: The table row shows contradictory tracker fields: "Applied = No"
while "Cherry pick = Included" with a concrete commit; update the row so the
state is consistent (e.g., set "Applied" to "Yes" to match "Cherry pick =
Included" and the listed commit), and ensure any adjacent status columns or
notes reflect the same resolution (remove or update the "Upstream PR not merged"
remark if it conflicts); verify the commit link
([moonbeam-foundation/evm@a122857]) and PR reference ([rust-ethereum/evm#405])
remain intact.

In `@docs/cherry-picks/UPGRADE-stable2603.md`:
- Line 210: Update the Phase 6 checklist to remove or replace the invalid "cargo
check --workspace --all-features" verification step: locate the Phase 6
checklist entry referencing "cargo check --workspace --all-features" and change
it to use the realistic feature matrix (e.g., default / runtime-benchmarks /
try-runtime) or a workspace-level check that does not pass --all-features, and
add a short note referencing the earlier guidance that --all-features forces
disable-genesis-builder and breaks Moonbeam's chain_spec and pallet standalone
tests.

In `@node/service/src/lazy_loading/manual_sealing.rs`:
- Line 18: Replace the brittle private import
frame_benchmarking::__private::codec::Encode with the public
parity_scale_codec::Encode in this module: update the use line to import
parity_scale_codec::Encode so calls like proof.encoded_size() still compile;
keep the existing logic around ProofRecorder::<B>::default(),
ProofSizeExt::new(storage_proof_recorder.clone()), passing
storage_proof_recorder: Some(storage_proof_recorder.clone()) into ProposeArgs,
and the storage_proof_recorder.drain_storage_proof() flow unchanged.

In `@pallets/erc20-xcm-bridge/src/lib.rs`:
- Around line 172-176: The current deposit_asset implementation collects assets
from what and performs per-asset executor drains and EVM transfers inside the
loop, but on a later failure returns Err((what, err)) with the original
AssetsInHolding, making already-processed assets appear unspent; fix by making
the whole batch atomic or by returning only the actual remaining/unspent
holding: either wrap the entire per-asset loop (including the drain and
transfers) inside a single frame_support::storage::with_storage_layer call so
any transfer/drain side-effects roll back on failure, or alternatively
update/remove processed assets from what (or build a new remaining holding) as
each asset succeeds and return that remaining AssetsInHolding on error; adjust
deposit_asset, matches_fungibles handling and any usage of drain/executor
context and outcome to follow the new atomic or incremental semantics.

In `@pallets/moonbeam-foreign-assets/src/lib.rs`:
- Around line 830-872: deposit_asset currently mutates state per-asset and can
return early leaving partial writes; wrap the entire per-asset processing loop
in a single atomic storage transaction or do a pure validation pass first.
Specifically, move the frame_support::storage::with_storage_layer(...) to
encompass the whole loop in deposit_asset (so all PendingDeposits::<T>::insert
and EvmCaller::<T>::erc20_mint_into calls happen inside one closure) or
alternatively first iterate assets calling
ForeignAssetsMatcher::<T>::match_asset, check for
AssetStatus::FrozenXcmDepositForbidden, compute and overflow-check new totals
for PendingDeposits::<T> without writing, and only after all validations succeed
perform the writes/erc20_mint_into; ensure no early returns outside the
storage::with_storage_layer and keep references to AssetStatus,
PendingDeposits::<T>, EvmCaller::<T>::erc20_mint_into, and
ForeignAssetsMatcher::<T>::match_asset to locate the changes.

---

Outside diff comments:
In `@node/service/src/lazy_loading/substrate_backend.rs`:
- Around line 605-613: The new boolean parameter _register_as_leaf is ignored in
set_block_data causing unconditional leaf registration; update set_block_data
(and any local call sites) to propagate the caller's register_as_leaf value so
the leaves.import call is executed only when register_as_leaf is true: locate
the set_block_data function in substrate_backend.rs, replace the unconditional
leaves.import(...) invocation with a conditional guarded by the register_as_leaf
parameter (or rename _register_as_leaf to register_as_leaf and use it), and
update any callers of set_block_data to pass the intended boolean so import
semantics match the caller's intent.

In `@pallets/xcm-weight-trader/src/mock.rs`:
- Around line 137-163: The mock transactor currently takes only the first
fungible asset from `what` and can silently drop others; in the `deposit_asset`
implementation check that `what.fungible_assets_iter()` contains exactly one
fungible asset before proceeding (e.g., pull the first item, then assert the
iterator has no second item), and if there are zero or more-than-one fungible
assets return an explicit Err((what, XcmError::...)) (choose an appropriate XCM
error variant or add a new one) instead of proceeding; adjust the logic around
`XcmAssetId(location)` / `Fungibility::Fungible(amount)` to run only after this
exact-one check so extra assets are rejected rather than ignored.

---

Nitpick comments:
In `@pallets/moonbeam-foreign-assets/src/notional.rs`:
- Around line 31-60: The NotionalImbalance type and its trait impls
(NotionalImbalance, UnsafeConstructorDestructor::unsafe_clone/forget_imbalance,
UnsafeManualAccounting::saturating_subsume, and
ImbalanceAccounting::amount/saturating_take) are duplicated in another pallet;
extract this struct and its impls into a shared primitives crate (e.g.,
primitives/xcm-notional) and replace the local definition with a re-export or
use of that shared type in pallets/moonbeam-foreign-assets and
pallets/erc20-xcm-bridge so both pallets import the single canonical
NotionalImbalance implementation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 77543f61-3c39-4aa5-8321-fc1a016d16ff

📥 Commits

Reviewing files that changed from the base of the PR and between 19a706e and 1f68990.

⛔ Files ignored due to path filters (22)
  • Cargo.lock is excluded by !**/*.lock
  • runtime/moonbase/src/weights/cumulus_pallet_parachain_system.rs is excluded by !**/weights/**/*.rs
  • runtime/moonbeam/src/weights/cumulus_pallet_parachain_system.rs is excluded by !**/weights/**/*.rs
  • runtime/moonriver/src/weights/cumulus_pallet_parachain_system.rs is excluded by !**/weights/**/*.rs
  • typescript-api/src/moonbase/interfaces/augment-api-query.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbase/interfaces/augment-api-runtime.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbase/interfaces/augment-api-tx.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbase/interfaces/lookup.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbase/interfaces/registry.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbase/interfaces/types-lookup.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/augment-api-query.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/augment-api-runtime.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/augment-api-tx.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/lookup.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/registry.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/types-lookup.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/augment-api-query.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/augment-api-runtime.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/augment-api-tx.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/lookup.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/registry.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/types-lookup.ts is excluded by !typescript-api/**
📒 Files selected for processing (65)
  • Cargo.toml
  • docs/cherry-picks/UPGRADE-stable2603.md
  • docs/cherry-picks/polkadot-sdk-stable2512.md
  • docs/cherry-picks/polkadot-sdk-stable2603.md
  • node/service/Cargo.toml
  • node/service/src/lazy_loading/call_executor.rs
  • node/service/src/lazy_loading/helpers.rs
  • node/service/src/lazy_loading/manual_sealing.rs
  • node/service/src/lazy_loading/mod.rs
  • node/service/src/lazy_loading/substrate_backend.rs
  • node/service/src/lib.rs
  • pallets/crowdloan-rewards/src/lib.rs
  • pallets/erc20-xcm-bridge/src/erc20_trap.rs
  • pallets/erc20-xcm-bridge/src/lib.rs
  • pallets/erc20-xcm-bridge/src/notional.rs
  • pallets/erc20-xcm-bridge/src/xcm_holding_ext.rs
  • pallets/ethereum-xcm/src/lib.rs
  • pallets/moonbeam-foreign-assets/src/evm.rs
  • pallets/moonbeam-foreign-assets/src/lib.rs
  • pallets/moonbeam-foreign-assets/src/notional.rs
  • pallets/moonbeam-foreign-assets/src/tests.rs
  • pallets/moonbeam-orbiters/src/types.rs
  • pallets/parachain-staking/src/auto_compound.rs
  • pallets/parachain-staking/src/delegation_requests.rs
  • pallets/parachain-staking/src/inflation.rs
  • pallets/parachain-staking/src/set.rs
  • pallets/parachain-staking/src/types.rs
  • pallets/xcm-transactor/src/lib.rs
  • pallets/xcm-transactor/src/mock.rs
  • pallets/xcm-weight-trader/src/lib.rs
  • pallets/xcm-weight-trader/src/mock.rs
  • pallets/xcm-weight-trader/src/tests.rs
  • precompiles/gmp/src/mock.rs
  • precompiles/relay-encoder/src/mock.rs
  • precompiles/xcm-transactor/src/mock.rs
  • precompiles/xcm-utils/src/mock.rs
  • precompiles/xtokens/src/mock.rs
  • primitives/account/src/lib.rs
  • primitives/rpc/txpool/src/lib.rs
  • runtime/common/src/apis.rs
  • runtime/common/src/impl_xcm_evm_runner.rs
  • runtime/moonbase/src/governance/origins.rs
  • runtime/moonbase/src/lib.rs
  • runtime/moonbase/src/xcm_config.rs
  • runtime/moonbase/tests/runtime_apis.rs
  • runtime/moonbase/tests/xcm_mock/parachain.rs
  • runtime/moonbase/tests/xcm_mock/relay_chain.rs
  • runtime/moonbase/tests/xcm_mock/statemint_like.rs
  • runtime/moonbeam/src/governance/origins.rs
  • runtime/moonbeam/src/lib.rs
  • runtime/moonbeam/src/xcm_config.rs
  • runtime/moonbeam/tests/runtime_apis.rs
  • runtime/moonbeam/tests/xcm_mock/parachain.rs
  • runtime/moonbeam/tests/xcm_mock/relay_chain.rs
  • runtime/moonbeam/tests/xcm_mock/statemint_like.rs
  • runtime/moonriver/src/governance/origins.rs
  • runtime/moonriver/src/lib.rs
  • runtime/moonriver/src/xcm_config.rs
  • runtime/moonriver/tests/runtime_apis.rs
  • runtime/moonriver/tests/xcm_mock/parachain.rs
  • runtime/moonriver/tests/xcm_mock/relay_chain.rs
  • runtime/moonriver/tests/xcm_mock/statemine_like.rs
  • test/suites/dev/common/test-block/test-block-mocked-relay.ts
  • test/suites/dev/common/test-xcm/test-transactional-outcomes.ts
  • test/suites/dev/moonbase/test-precompile/test-precompile-relay-verifier.ts
💤 Files with no reviewable changes (14)
  • docs/cherry-picks/polkadot-sdk-stable2512.md
  • runtime/moonbase/tests/xcm_mock/parachain.rs
  • runtime/moonbase/tests/xcm_mock/relay_chain.rs
  • node/service/Cargo.toml
  • runtime/moonriver/tests/xcm_mock/statemine_like.rs
  • runtime/moonbeam/src/xcm_config.rs
  • runtime/moonriver/src/xcm_config.rs
  • runtime/moonbeam/tests/xcm_mock/parachain.rs
  • runtime/moonbeam/tests/xcm_mock/statemint_like.rs
  • runtime/moonbeam/tests/xcm_mock/relay_chain.rs
  • runtime/moonriver/tests/xcm_mock/relay_chain.rs
  • runtime/moonriver/tests/xcm_mock/parachain.rs
  • runtime/moonbase/tests/xcm_mock/statemint_like.rs
  • runtime/moonbase/src/xcm_config.rs

Comment thread docs/cherry-picks/polkadot-sdk-stable2603.md Outdated
Comment thread docs/cherry-picks/UPGRADE-stable2603.md
Comment thread node/service/src/lazy_loading/manual_sealing.rs Outdated
Comment thread pallets/erc20-xcm-bridge/src/lib.rs Outdated
Comment thread pallets/moonbeam-foreign-assets/src/lib.rs Outdated
- evm 'Fix stale evm-core delegation.rs test module': set Applied=Yes to match
  Cherry pick=Included and the listed commit a122857 (the fix is in the fork).
- Phase 6 checklist: replace 'cargo check --workspace --all-features' (which the
  Phase 3 note flags as invalid for moonbeam) with the realistic feature matrix
  (default / runtime-benchmarks / try-runtime).
…_sealing

Avoid the brittle frame_benchmarking::__private::codec re-export; parity-scale-codec
is already a direct dependency of node/service.
The stable2603 TransactAsset signature change widened deposit_asset's input
from a single `&Asset` to a multi-asset `AssetsInHolding` and added the
`(unspent_assets, error)` contract. The rewritten loops mutated state per asset
but returned the full original `what` on a later-asset failure, so an
already-credited asset could be reported as unspent. The executor only ever
calls deposit_asset per single asset (deposit_assets_with_retry +
transactional_process), so this isn't reachable today, but the loops accept a
batch and were only correct for one.

Wrap each per-asset loop in a single with_storage_layer so any failure rolls
back the mints/pending-writes/transfers already applied, making the unspent
contract honest for any batch size. XcmError isn't From<DispatchError>, so the
real error is carried out via `captured` and a dummy DispatchError triggers the
rollback. Success and single-asset paths are unchanged (pallet tests pass).
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 9, 2026
Replace the self-contained per-asset storage-layer loop with the pattern the
SDK's own TransactAsset adapters (e.g. fungibles_adapter) use: assert the
single-asset invariant with defensive_assert!(what.len() == 1) and process that
one asset, relying on the runtime's TransactionalProcessor for cross-asset
rollback. All three runtimes configure FrameTransactionalProcessor, which wraps
the whole DepositAsset instruction in a frame transactional layer, and the
executor only ever deposits one fungible per call (deposit_assets_with_retry),
so the previous custom with_storage_layer was redundant and non-idiomatic.

Net change vs master is now just the forced signature update (&Asset ->
AssetsInHolding, (unspent, error) return) plus the single-asset extraction and
defensive assert. Pallet tests pass (erc20 9/9, foreign-assets 17/17).
@manuelmauro

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
test/suites/dev/common/test-block/test-block-mocked-relay.ts (1)

18-36: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Recompute the setValidationData index for the second block.

index is resolved against blockData and then reused against blockData2. If the extrinsic ordering changes between blocks, Line 33 will read the wrong extrinsic or undefined, so the second assertion stops validating the second block's relay parent.

Proposed fix
         const index = blockData.block.extrinsics.findIndex(
           (e) => e.method.method === "setValidationData"
         );
+        expect(index).toBeGreaterThan(-1);
         expect(
           (
             blockData.block.extrinsics[index].method
               .args[0] as CumulusPrimitivesParachainInherentParachainInherentData
           ).validationData.relayParentNumber.toString()
         ).to.eq("0");
         const blockResult2 = await context.createBlock();
         const blockData2 = await context.polkadotJs().rpc.chain.getBlock(blockResult2.block.hash);
+        const index2 = blockData2.block.extrinsics.findIndex(
+          (e) => e.method.method === "setValidationData"
+        );
+        expect(index2).toBeGreaterThan(-1);
         expect(
           (
-            blockData2.block.extrinsics[index].method
+            blockData2.block.extrinsics[index2].method
               .args[0] as CumulusPrimitivesParachainInherentParachainInherentData
           ).validationData.relayParentNumber.toString()
         ).to.eq("1");
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/suites/dev/common/test-block/test-block-mocked-relay.ts` around lines 18
- 36, The test captures an `index` by searching
`blockData.block.extrinsics.findIndex(...)` for the `setValidationData`
extrinsic and then reuses that same `index` for `blockData2`; recompute the
index against `blockData2.block.extrinsics` after fetching `blockData2` (i.e.,
run the same findIndex call again before accessing
`blockData2.block.extrinsics[index]`) so the second assertion reads the correct
extrinsic for the second block when using `context.createBlock()` and
`context.polkadotJs().rpc.chain.getBlock`.
🧹 Nitpick comments (1)
pallets/moonbeam-foreign-assets/src/tests.rs (1)

28-31: ⚡ Quick win

Fail fast for non-fungible assets in the holding test helper.

Silently converting non-fungible assets to 0 can hide broken test setup and produce false positives. Make this helper explicit by panicking/asserting on non-fungible input.

Suggested change
 fn holding(asset: &Asset) -> xcm_executor::AssetsInHolding {
 	let amount = match asset.fun {
 		Fungibility::Fungible(amount) => amount,
-		_ => 0,
+		_ => panic!("holding() expects a fungible asset in these tests"),
 	};
 	xcm_executor::AssetsInHolding::new_from_fungible_credit(
 		asset.id.clone(),
 		Box::new(crate::notional::NotionalImbalance(amount)),
 	)
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pallets/moonbeam-foreign-assets/src/tests.rs` around lines 28 - 31, The match
in the holding test helper silently maps non-fungible assets to 0 (matching on
asset.fun and extracting Fungibility::Fungible), which can hide test setup
errors; change this to fail-fast by replacing the fallback arm with an explicit
panic or assert (e.g., assert!(matches!(asset.fun, Fungibility::Fungible(_))) or
panic! with a clear message) so that any non-fungible input to the holding
helper immediately surfaces as an error; update the match in the holding helper
that binds amount from Fungibility::Fungible to remove the `_ => 0` branch and
use the assertion/panic instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/cherry-picks/UPGRADE-stable2603.md`:
- Line 208: Update the checklist item "- [ ] Update `tests/` TS fixtures
referencing changed types/RPCs." in UPGRADE-stable2603.md to reflect that TS
test/fixture updates are included in this PR: either change the box to checked
("- [x] ...") or revise the description to narrow the remaining follow-up work
(e.g., specify which fixtures still require a built node binary) so the
checklist accurately matches what was shipped; locate the exact line containing
that checklist text and modify it accordingly.
- Line 11: The document contains two inconsistent dates for the same release
identifier `polkadot-stable2603-1`; update the two occurrences of the release
date (the `Target upstream:` line and the later mention on Line 37) so they
match—pick the correct date (either 2026-05-01 or 2026-05-04) and replace the
other occurrence so both references to `polkadot-stable2603-1` use the same
release date.

In `@pallets/xcm-weight-trader/src/mock.rs`:
- Around line 137-143: The deposit_asset implementation currently only reads the
first fungible from what.fungible_assets_iter().next() and silently ignores
additional assets in the AssetsInHolding; update
MockAssetTransactor::deposit_asset to explicitly reject or handle multi-asset
holdings by checking for extra fungible assets (e.g., call
fungible_assets_iter() and ensure only one item or test
iterator.next().is_some() after the first) and return a clear error (add or
reuse an XcmError variant such as MultiAssetNotSupported/TooManyAssets) instead
of proceeding, and adjust tests that use holding_vec(...) to expect the new
explicit rejection when multiple fungibles are present.

---

Outside diff comments:
In `@test/suites/dev/common/test-block/test-block-mocked-relay.ts`:
- Around line 18-36: The test captures an `index` by searching
`blockData.block.extrinsics.findIndex(...)` for the `setValidationData`
extrinsic and then reuses that same `index` for `blockData2`; recompute the
index against `blockData2.block.extrinsics` after fetching `blockData2` (i.e.,
run the same findIndex call again before accessing
`blockData2.block.extrinsics[index]`) so the second assertion reads the correct
extrinsic for the second block when using `context.createBlock()` and
`context.polkadotJs().rpc.chain.getBlock`.

---

Nitpick comments:
In `@pallets/moonbeam-foreign-assets/src/tests.rs`:
- Around line 28-31: The match in the holding test helper silently maps
non-fungible assets to 0 (matching on asset.fun and extracting
Fungibility::Fungible), which can hide test setup errors; change this to
fail-fast by replacing the fallback arm with an explicit panic or assert (e.g.,
assert!(matches!(asset.fun, Fungibility::Fungible(_))) or panic! with a clear
message) so that any non-fungible input to the holding helper immediately
surfaces as an error; update the match in the holding helper that binds amount
from Fungibility::Fungible to remove the `_ => 0` branch and use the
assertion/panic instead.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8b1a4c90-404b-4b4d-9bef-e947327f9032

📥 Commits

Reviewing files that changed from the base of the PR and between 19a706e and bf0c0d1.

⛔ Files ignored due to path filters (22)
  • Cargo.lock is excluded by !**/*.lock
  • runtime/moonbase/src/weights/cumulus_pallet_parachain_system.rs is excluded by !**/weights/**/*.rs
  • runtime/moonbeam/src/weights/cumulus_pallet_parachain_system.rs is excluded by !**/weights/**/*.rs
  • runtime/moonriver/src/weights/cumulus_pallet_parachain_system.rs is excluded by !**/weights/**/*.rs
  • typescript-api/src/moonbase/interfaces/augment-api-query.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbase/interfaces/augment-api-runtime.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbase/interfaces/augment-api-tx.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbase/interfaces/lookup.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbase/interfaces/registry.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbase/interfaces/types-lookup.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/augment-api-query.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/augment-api-runtime.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/augment-api-tx.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/lookup.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/registry.ts is excluded by !typescript-api/**
  • typescript-api/src/moonbeam/interfaces/types-lookup.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/augment-api-query.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/augment-api-runtime.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/augment-api-tx.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/lookup.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/registry.ts is excluded by !typescript-api/**
  • typescript-api/src/moonriver/interfaces/types-lookup.ts is excluded by !typescript-api/**
📒 Files selected for processing (65)
  • Cargo.toml
  • docs/cherry-picks/UPGRADE-stable2603.md
  • docs/cherry-picks/polkadot-sdk-stable2512.md
  • docs/cherry-picks/polkadot-sdk-stable2603.md
  • node/service/Cargo.toml
  • node/service/src/lazy_loading/call_executor.rs
  • node/service/src/lazy_loading/helpers.rs
  • node/service/src/lazy_loading/manual_sealing.rs
  • node/service/src/lazy_loading/mod.rs
  • node/service/src/lazy_loading/substrate_backend.rs
  • node/service/src/lib.rs
  • pallets/crowdloan-rewards/src/lib.rs
  • pallets/erc20-xcm-bridge/src/erc20_trap.rs
  • pallets/erc20-xcm-bridge/src/lib.rs
  • pallets/erc20-xcm-bridge/src/notional.rs
  • pallets/erc20-xcm-bridge/src/xcm_holding_ext.rs
  • pallets/ethereum-xcm/src/lib.rs
  • pallets/moonbeam-foreign-assets/src/evm.rs
  • pallets/moonbeam-foreign-assets/src/lib.rs
  • pallets/moonbeam-foreign-assets/src/notional.rs
  • pallets/moonbeam-foreign-assets/src/tests.rs
  • pallets/moonbeam-orbiters/src/types.rs
  • pallets/parachain-staking/src/auto_compound.rs
  • pallets/parachain-staking/src/delegation_requests.rs
  • pallets/parachain-staking/src/inflation.rs
  • pallets/parachain-staking/src/set.rs
  • pallets/parachain-staking/src/types.rs
  • pallets/xcm-transactor/src/lib.rs
  • pallets/xcm-transactor/src/mock.rs
  • pallets/xcm-weight-trader/src/lib.rs
  • pallets/xcm-weight-trader/src/mock.rs
  • pallets/xcm-weight-trader/src/tests.rs
  • precompiles/gmp/src/mock.rs
  • precompiles/relay-encoder/src/mock.rs
  • precompiles/xcm-transactor/src/mock.rs
  • precompiles/xcm-utils/src/mock.rs
  • precompiles/xtokens/src/mock.rs
  • primitives/account/src/lib.rs
  • primitives/rpc/txpool/src/lib.rs
  • runtime/common/src/apis.rs
  • runtime/common/src/impl_xcm_evm_runner.rs
  • runtime/moonbase/src/governance/origins.rs
  • runtime/moonbase/src/lib.rs
  • runtime/moonbase/src/xcm_config.rs
  • runtime/moonbase/tests/runtime_apis.rs
  • runtime/moonbase/tests/xcm_mock/parachain.rs
  • runtime/moonbase/tests/xcm_mock/relay_chain.rs
  • runtime/moonbase/tests/xcm_mock/statemint_like.rs
  • runtime/moonbeam/src/governance/origins.rs
  • runtime/moonbeam/src/lib.rs
  • runtime/moonbeam/src/xcm_config.rs
  • runtime/moonbeam/tests/runtime_apis.rs
  • runtime/moonbeam/tests/xcm_mock/parachain.rs
  • runtime/moonbeam/tests/xcm_mock/relay_chain.rs
  • runtime/moonbeam/tests/xcm_mock/statemint_like.rs
  • runtime/moonriver/src/governance/origins.rs
  • runtime/moonriver/src/lib.rs
  • runtime/moonriver/src/xcm_config.rs
  • runtime/moonriver/tests/runtime_apis.rs
  • runtime/moonriver/tests/xcm_mock/parachain.rs
  • runtime/moonriver/tests/xcm_mock/relay_chain.rs
  • runtime/moonriver/tests/xcm_mock/statemine_like.rs
  • test/suites/dev/common/test-block/test-block-mocked-relay.ts
  • test/suites/dev/common/test-xcm/test-transactional-outcomes.ts
  • test/suites/dev/moonbase/test-precompile/test-precompile-relay-verifier.ts
💤 Files with no reviewable changes (14)
  • runtime/moonriver/tests/xcm_mock/statemine_like.rs
  • runtime/moonriver/tests/xcm_mock/relay_chain.rs
  • runtime/moonbase/src/xcm_config.rs
  • runtime/moonbeam/tests/xcm_mock/relay_chain.rs
  • runtime/moonbeam/tests/xcm_mock/statemint_like.rs
  • runtime/moonbase/tests/xcm_mock/relay_chain.rs
  • node/service/Cargo.toml
  • runtime/moonriver/src/xcm_config.rs
  • docs/cherry-picks/polkadot-sdk-stable2512.md
  • runtime/moonriver/tests/xcm_mock/parachain.rs
  • runtime/moonbase/tests/xcm_mock/parachain.rs
  • runtime/moonbase/tests/xcm_mock/statemint_like.rs
  • runtime/moonbeam/src/xcm_config.rs
  • runtime/moonbeam/tests/xcm_mock/parachain.rs

## Context

- **Current pin (Cargo.toml):** `moonbeam-polkadot-stable2512` for polkadot-sdk, frontier, evm, ethereum, moonkit.
- **Target upstream:** `polkadot-stable2603-1` (paritytech/polkadot-sdk, released 2026-05-04).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use one release date for polkadot-stable2603-1.

Line 11 says 2026-05-04, but Line 37 says 2026-05-01. Please align these so the tracker stays internally consistent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/cherry-picks/UPGRADE-stable2603.md` at line 11, The document contains
two inconsistent dates for the same release identifier `polkadot-stable2603-1`;
update the two occurrences of the release date (the `Target upstream:` line and
the later mention on Line 37) so they match—pick the correct date (either
2026-05-01 or 2026-05-04) and replace the other occurrence so both references to
`polkadot-stable2603-1` use the same release date.

- **runtime-benchmarks**: weight-reclaim benchmark needed the `GetCallMetadata` bound the logging cherry-pick added (fixed in the fork at `ddba2453`); `pallet_transaction_payment::benchmarking` is now private (use `Pallet` + `BenchmarkConfig`); `worst_case_holding` returns `AssetsInHolding` (built via `generate_holding_assets`).
- [x] **XCM credit-based holding migration** (the substantive piece): stable2603 reworked `AssetsInHolding` to carry `fungible::Credit` imbalances. Real impls for `erc20-xcm-bridge` / `moonbeam-foreign-assets` (`deposit_asset`/`withdraw_asset` via a notional credit, since erc20 isn't a Substrate `fungible`) and `xcm-weight-trader` (`buy_weight`/`refund_weight`/`Drop`).
- [x] No `[patch.crates-io]` needed — the fork branches resolve directly from the moonbeam-foundation remotes.
- [ ] Update `tests/` TS fixtures referencing changed types/RPCs. — *pending; requires a built node binary.*

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Phase 3's TS-fixture status is stale.

This line still marks the TypeScript fixture work as pending, but this PR already includes TS test/fixture updates. Please mark it done or narrow the remaining follow-up so the checklist matches the shipped work.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/cherry-picks/UPGRADE-stable2603.md` at line 208, Update the checklist
item "- [ ] Update `tests/` TS fixtures referencing changed types/RPCs." in
UPGRADE-stable2603.md to reflect that TS test/fixture updates are included in
this PR: either change the box to checked ("- [x] ...") or revise the
description to narrow the remaining follow-up work (e.g., specify which fixtures
still require a built node binary) so the checklist accurately matches what was
shipped; locate the exact line containing that checklist text and modify it
accordingly.

Comment on lines +137 to 143
// Extract the (single) fungible asset from the holding.
let asset = what.fungible_assets_iter().next();
let asset = match asset {
Some(asset) => asset,
None => return Err((what, XcmError::AssetNotFound)),
};
match (asset.id.clone(), asset.fun.clone()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether tests currently exercise multi-fungible holdings for this mock.

rg -n -C3 'deposit_asset\(|fungible_assets_iter\(\)\.next\(\)|AssetsInHolding' pallets/xcm-weight-trader/src/mock.rs pallets/xcm-weight-trader/src/tests.rs
rg -n -C3 'MultiAssets|Fungibility::Fungible|AssetId|AssetNotFound' pallets/xcm-weight-trader/src/tests.rs

Repository: moonbeam-foundation/moonbeam

Length of output: 10343


🏁 Script executed:

#!/bin/bash
# Inspect where multi-asset holdings are used and what the tests assert.
set -euo pipefail

rg -n "holding_vec\(" pallets/xcm-weight-trader/src/tests.rs

# Show the surrounding test blocks for multi-asset usage.
# (Pick the likely region around the first match.)
awk 'NR>=540 && NR<=700 {print NR ":" $0}' pallets/xcm-weight-trader/src/tests.rs

# Also inspect the mock implementation fully (for any other relevant logic).
awk 'NR>=110 && NR<=200 {print NR ":" $0}' pallets/xcm-weight-trader/src/mock.rs

Repository: moonbeam-foundation/moonbeam

Length of output: 8463


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "holding_vec\(" pallets/xcm-weight-trader/src/tests.rs

# Print small context around every holding_vec usage
rg -n -C5 "holding_vec\(" pallets/xcm-weight-trader/src/tests.rs

# Also search for any direct multi-asset construction without holding_vec
rg -n "subsume_assets\(|new_from_fungible_credit\(" pallets/xcm-weight-trader/src/tests.rs

Repository: moonbeam-foundation/moonbeam

Length of output: 1008


Reject or handle multi-asset holdings explicitly in MockAssetTransactor::deposit_asset

deposit_asset pulls only what.fungible_assets_iter().next() and matches that single fungible asset, ignoring any additional fungibles in the AssetsInHolding. The mock’s tests don’t cover the success path with a multi-fungible holding—holding_vec(...) is only used to assert TooExpensive when the supported asset isn’t first—so this mock fidelity gap can hide issues in multi-asset credit/deposit flows.

Suggested guard to keep mock behavior explicit
 	) -> Result<(), (xcm_executor::AssetsInHolding, XcmError)> {
-		// Extract the (single) fungible asset from the holding.
+		// Mock supports exactly one fungible asset; reject multi-asset holdings explicitly.
+		if what.fungible_assets_iter().nth(1).is_some() {
+			return Err((what, XcmError::AssetNotFound));
+		}
+
+		// Extract the (single) fungible asset from the holding.
 		let asset = what.fungible_assets_iter().next();
 		let asset = match asset {
 			Some(asset) => asset,
 			None => return Err((what, XcmError::AssetNotFound)),
 		};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Extract the (single) fungible asset from the holding.
let asset = what.fungible_assets_iter().next();
let asset = match asset {
Some(asset) => asset,
None => return Err((what, XcmError::AssetNotFound)),
};
match (asset.id.clone(), asset.fun.clone()) {
// Mock supports exactly one fungible asset; reject multi-asset holdings explicitly.
if what.fungible_assets_iter().nth(1).is_some() {
return Err((what, XcmError::AssetNotFound));
}
// Extract the (single) fungible asset from the holding.
let asset = what.fungible_assets_iter().next();
let asset = match asset {
Some(asset) => asset,
None => return Err((what, XcmError::AssetNotFound)),
};
match (asset.id.clone(), asset.fun.clone()) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pallets/xcm-weight-trader/src/mock.rs` around lines 137 - 143, The
deposit_asset implementation currently only reads the first fungible from
what.fungible_assets_iter().next() and silently ignores additional assets in the
AssetsInHolding; update MockAssetTransactor::deposit_asset to explicitly reject
or handle multi-asset holdings by checking for extra fungible assets (e.g., call
fungible_assets_iter() and ensure only one item or test
iterator.next().is_some() after the first) and return a clear error (add or
reuse an XcmError variant such as MultiAssetNotSupported/TooManyAssets) instead
of proceeding, and adjust tests that use holding_vec(...) to expect the new
explicit rejection when multiple fungibles are present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited XCM Run option xcm-emulator tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant