Allow ERC20 Teleport on Moonbase#3758
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a whitelist-driven ERC-20 teleport transactor and filter, on-chain whitelist lifecycle and counters, admin extrinsics for whitelist management, runtime wiring across Moonbase/Moonbeam/Moonriver, benchmark hook delegation, and extensive unit/integration tests for lifecycle and gate behavior. ChangesERC-20 Teleport Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
runtime/moonbase/tests/erc20_teleport.rs (1)
148-156: 💤 Low valueMisleading "idempotent-safe" comment.
The behavior asserted here is the opposite of idempotent: a second
add_teleportable_erc20call returnsErc20AlreadyTeleportablerather than succeeding as a no-op. Consider rewording to avoid confusion for future readers.✏️ Suggested wording
- // Root works and is idempotent-safe (duplicate add fails). + // Root works; duplicate add is rejected (not idempotent).🤖 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 `@runtime/moonbase/tests/erc20_teleport.rs` around lines 148 - 156, Update the misleading test comment: replace "Root works and is idempotent-safe (duplicate add fails)." with a clear description that the first call to Erc20XcmBridge::add_teleportable_erc20 with root_origin() succeeds and a duplicate call returns pallet_erc20_xcm_bridge::Error::<Runtime>::Erc20AlreadyTeleportable (i.e., the operation is not idempotent and duplicate adds error rather than being a no-op).pallets/erc20-xcm-bridge/src/lib.rs (2)
138-139: 💤 Low valueHardcoded extrinsic weights are not benchmarked.
Both
add_teleportable_erc20andremove_teleportable_erc20use a hand-rolledWeight::from_parts(15_000_000, 0).saturating_add(T::DbWeight::get().reads_writes(1, 1)). Since these are root-gated, the practical risk is low, but a generatedWeightInfowould keep them aligned with the runtime's benchmarking conventions and let weights drift with hardware/runtime updates. Consider adding aWeightInfoassociated type onConfigand benchmarking these two calls (matches the pattern used elsewhere in this repo for governance-only extrinsics).Also applies to: 158-159
🤖 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/erc20-xcm-bridge/src/lib.rs` around lines 138 - 139, The hardcoded weights on the extrinsics add_teleportable_erc20 and remove_teleportable_erc20 should be replaced with benchmark-generated weights: add a WeightInfo associated type to the pallet Config (e.g. type WeightInfo: WeightInfo;), implement a WeightInfo trait with methods add_teleportable_erc20() and remove_teleportable_erc20() (matching the repo's existing pattern), update the #[pallet::weight(...)] attributes to call T::WeightInfo::add_teleportable_erc20() and T::WeightInfo::remove_teleportable_erc20() respectively (keeping the T::DbWeight::get().reads_writes(...) additions if needed), and run the benchmarking harness to generate the concrete weights and wire them into the runtime.
419-458: 💤 Low valueConsider adding direct unit tests for the
TransactAssetimpl.The current
tests.rsexercisesIsTeleportableErc20and the governance extrinsics, but the actualwithdraw_asset/deposit_asset/internal_transfer_assetEVM paths onErc20TeleportTransactoraren't covered by unit tests in this pallet. Given these methods are the load-bearing piece of the teleport semantics (single-EVM-call vs two-call lock/unlock, error propagation viawith_storage_layer, rejection of non-whitelisted assets withAssetNotHandledso the legacy adapter takes over), at minimum it'd be worth a test that:
- deploys a tiny ERC-20 in the EVM mock, whitelists it, and checks
withdraw_assetends up with the balance onTeleportCheckingAccount;- verifies a non-whitelisted asset returns the fall-through error so the tuple-impl can defer to
Pallet<T>.Not strictly required to merge — Moonbase integration tests partially cover the happy path — but it would localize regressions to this pallet instead of the runtime test suite.
🤖 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/erc20-xcm-bridge/src/lib.rs` around lines 419 - 458, Add unit tests that directly exercise the TransactAsset implementation (Erc20TeleportTransactor) by: (1) deploying a minimal ERC-20 in the EVM mock, calling the whitelist logic (IsTeleportableErc20) and invoking withdraw_asset to assert the ERC-20 transfer path (Pallet::<T>::erc20_transfer) moves tokens to TeleportCheckingAccount and respects gas_limit_of_erc20_transfer; and (2) calling withdraw_asset/internal_transfer_asset with a non-whitelisted Asset to assert it returns AssetNotHandled (so the legacy adapter can fall through). Locate tests around the TransactAsset impl and use AccountIdConverter::convert_location, match_whitelisted, and frame_support::storage::with_storage_layer to reproduce the runtime behavior and error propagation. Ensure assertions cover both success (balance moved) and fall-through error cases.
🤖 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 `@pallets/erc20-xcm-bridge/src/lib.rs`:
- Around line 362-377: The doc comment for Erc20TeleportTransactor incorrectly
claims a `mint_asset(asset)` override exists; update the comment to remove or
correct that line and state that `mint_asset` is not implemented (defaults to
XcmError::Unimplemented) or that minting is handled via the
`ReceiveTeleportedAsset` flow using `can_check_in`/`check_in` + `DepositAsset` →
`deposit_asset`; reference Erc20TeleportTransactor, TransactAsset, mint_asset,
deposit_asset, ReceiveTeleportedAsset, can_check_in and check_in to clarify the
actual flow.
In `@runtime/common/src/apis.rs`:
- Around line 960-962: Add no-op impls of the XcmPalletTeleportBenchmark trait
for every runtime that can be built with the runtime-benchmarks feature so the
UFCS call in runtime/common/src/apis.rs compiles; specifically, add empty impl
blocks like impl
moonbeam_runtime_common::xcm_pallet_benchmark::XcmPalletTeleportBenchmark for
Runtime {} (and the analogous impl for Moonriver) in each runtime's xcm
configuration module (e.g., the same files that contain Xcm configuration such
as xcm_config.rs) so that the call to <Runtime as
XcmPalletTeleportBenchmark>::teleportable_asset_and_dest() resolves.
In `@runtime/moonbase/src/xcm_config.rs`:
- Around line 770-782: The current check accepts any local multilocation that
starts with the ERC-20 bridge pallet prefix; tighten AssetFeesFilter by also
verifying the interior sequence is exactly [PalletInstance(idx),
AccountKey20(..)]: when location.parents == 0 and the first interior matches
Junction::PalletInstance(idx) and
Erc20XcmBridgePalletLocation::get().first_interior() yields a
PalletInstance(prefix_idx) with idx == prefix_idx, additionally ensure the
second interior junction exists and matches Junction::AccountKey20(_) (or the
exact AccountKey20 shape used in the code) before returning true so only the
precise ERC-20 teleport asset shape is accepted.
---
Nitpick comments:
In `@pallets/erc20-xcm-bridge/src/lib.rs`:
- Around line 138-139: The hardcoded weights on the extrinsics
add_teleportable_erc20 and remove_teleportable_erc20 should be replaced with
benchmark-generated weights: add a WeightInfo associated type to the pallet
Config (e.g. type WeightInfo: WeightInfo;), implement a WeightInfo trait with
methods add_teleportable_erc20() and remove_teleportable_erc20() (matching the
repo's existing pattern), update the #[pallet::weight(...)] attributes to call
T::WeightInfo::add_teleportable_erc20() and
T::WeightInfo::remove_teleportable_erc20() respectively (keeping the
T::DbWeight::get().reads_writes(...) additions if needed), and run the
benchmarking harness to generate the concrete weights and wire them into the
runtime.
- Around line 419-458: Add unit tests that directly exercise the TransactAsset
implementation (Erc20TeleportTransactor) by: (1) deploying a minimal ERC-20 in
the EVM mock, calling the whitelist logic (IsTeleportableErc20) and invoking
withdraw_asset to assert the ERC-20 transfer path (Pallet::<T>::erc20_transfer)
moves tokens to TeleportCheckingAccount and respects
gas_limit_of_erc20_transfer; and (2) calling
withdraw_asset/internal_transfer_asset with a non-whitelisted Asset to assert it
returns AssetNotHandled (so the legacy adapter can fall through). Locate tests
around the TransactAsset impl and use AccountIdConverter::convert_location,
match_whitelisted, and frame_support::storage::with_storage_layer to reproduce
the runtime behavior and error propagation. Ensure assertions cover both success
(balance moved) and fall-through error cases.
In `@runtime/moonbase/tests/erc20_teleport.rs`:
- Around line 148-156: Update the misleading test comment: replace "Root works
and is idempotent-safe (duplicate add fails)." with a clear description that the
first call to Erc20XcmBridge::add_teleportable_erc20 with root_origin() succeeds
and a duplicate call returns
pallet_erc20_xcm_bridge::Error::<Runtime>::Erc20AlreadyTeleportable (i.e., the
operation is not idempotent and duplicate adds error rather than being a no-op).
🪄 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: bea0c577-96f7-4303-bebd-6d12ea4f064f
⛔ Files ignored due to path filters (2)
runtime/moonbase/src/weights/pallet_xcm.rsis excluded by!**/weights/**/*.rsruntime/moonbase/src/weights/xcm/mod.rsis excluded by!**/weights/**/*.rs
📒 Files selected for processing (13)
pallets/erc20-xcm-bridge/src/lib.rspallets/erc20-xcm-bridge/src/mock.rspallets/erc20-xcm-bridge/src/tests.rsruntime/common/src/apis.rsruntime/common/src/lib.rsruntime/common/src/xcm_pallet_benchmark.rsruntime/moonbase/src/lib.rsruntime/moonbase/src/xcm_config.rsruntime/moonbase/tests/erc20_teleport.rsruntime/moonbeam/src/lib.rsruntime/moonbeam/src/xcm_config.rsruntime/moonriver/src/lib.rsruntime/moonriver/src/xcm_config.rs
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
arturgontijo
changed the title
arturgontijo
added
B7-runtimenoteworthy
librelois
left a comment
librelois
left a comment
There was a problem hiding this comment.
Findings
-
High: inbound teleports can unlock whitelisted ERC-20s from any XCM origin, not just AssetHub.
runtime/moonbase/src/xcm_config.rs:285wiresIsTeleporter = IsTeleportableErc20<Runtime>, butpallets/erc20-xcm-bridge/src/lib.rs:479ignores theLocationargument and only checks that the asset contract is whitelisted. On inboundReceiveTeleportedAsset, XCM executor uses that location as the teleport origin; accepting any origin means any chain able to deliver XCM to Moonbase can present a whitelisted ERC-20 teleport and triggerdeposit_asset, which transfers real ERC-20s out ofTeleportCheckingAccountatpallets/erc20-xcm-bridge/src/lib.rs:403. The filter needs to bind the asset to the trusted counterparty, e.g. AssetHub, for inbound trust. -
Medium: outbound user teleports are not restricted to AssetHub.
runtime/moonbase/src/xcm_config.rs:347uses the sameIsTeleportableErc20<Runtime>asXcmTeleportFilter, andpallets/erc20-xcm-bridge/src/lib.rs:485ignores theLocationin(Location, Vec<Asset>). That lets whitelisted ERC-20 teleport calls pass for arbitrary destinations, despite the change request being specifically “to/from AssetHub”. At best this can lock user ERC-20s into the checking account and send messages to chains that reject or mishandle them. -
Medium: removing a whitelist entry can strand already locked funds.
The docs say removed assets “must still be claimed via the inbound teleport path” atpallets/erc20-xcm-bridge/src/lib.rs:154, butremove_teleportable_erc20deletes the only bit thatErc20TeleportTransactor::match_whitelistedrequires atpallets/erc20-xcm-bridge/src/lib.rs:381. After removal, inbound unlocks for that contract returnAssetNotHandled, so any in-flight or legitimately returning balances inTeleportCheckingAccountbecome inaccessible via the new path unless root re-adds the contract.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
runtime/moonbase/src/xcm_config.rs (1)
870-878: ⚡ Quick winReset benchmark
LockedSupplyfor deterministic setup.The benchmark helper uses a fixed contract; explicitly clearing
LockedSupplyhere prevents stale state from influencing repeated benchmark setup paths.Suggested change
let contract = H160([0x42; 20]); pallet_erc20_xcm_bridge::TeleportableErc20s::<Runtime>::insert( contract, pallet_erc20_xcm_bridge::TeleportableErc20Status::Registered, ); + pallet_erc20_xcm_bridge::LockedSupply::<Runtime>::remove(&contract);🤖 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 `@runtime/moonbase/src/xcm_config.rs` around lines 870 - 878, The benchmark setup uses a fixed `contract` and inserts into `pallet_erc20_xcm_bridge::TeleportableErc20s`, but it doesn't clear `pallet_erc20_xcm_bridge::LockedSupply`, which can leave stale state across runs; update the setup to explicitly remove or reset `pallet_erc20_xcm_bridge::LockedSupply` for that `contract` (or clear the entire map) before inserting into `TeleportableErc20s` and setting `TeleportableErc20Status::Registered`, ensuring deterministic benchmark state for the helper that uses `contract`.
🤖 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.
Nitpick comments:
In `@runtime/moonbase/src/xcm_config.rs`:
- Around line 870-878: The benchmark setup uses a fixed `contract` and inserts
into `pallet_erc20_xcm_bridge::TeleportableErc20s`, but it doesn't clear
`pallet_erc20_xcm_bridge::LockedSupply`, which can leave stale state across
runs; update the setup to explicitly remove or reset
`pallet_erc20_xcm_bridge::LockedSupply` for that `contract` (or clear the entire
map) before inserting into `TeleportableErc20s` and setting
`TeleportableErc20Status::Registered`, ensuring deterministic benchmark state
for the helper that uses `contract`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6dc5676d-5a0d-4aaf-beba-c036ba7002aa
📒 Files selected for processing (5)
pallets/erc20-xcm-bridge/src/lib.rspallets/erc20-xcm-bridge/src/mock.rspallets/erc20-xcm-bridge/src/tests.rsruntime/moonbase/src/xcm_config.rsruntime/moonbase/tests/erc20_teleport.rs
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pallets/erc20-xcm-bridge/src/tests.rs (1)
81-84: ⚡ Quick winUse the configured trusted-location constant instead of duplicating it in the helper.
This keeps tests aligned with mock runtime config and avoids drift if the configured location changes.
Suggested diff
fn trusted_loc() -> Location { - Location::new(1, [Junction::Parachain(1001)]) + crate::mock::TeleportTrustedLocation::get() }🤖 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/erc20-xcm-bridge/src/tests.rs` around lines 81 - 84, The helper function trusted_loc() duplicates the test's trusted location; update it to use the configured mock constant instead of hardcoding the Location so tests stay in sync with the runtime config—replace the body of trusted_loc() to return the mock runtime's trusted-location constant (e.g. mock::TeleportTrustedLocation::get() or the appropriate TeleportTrustedLocation constant) so the helper references the single source of truth used by the tests.runtime/moonbase/src/xcm_config.rs (1)
881-885: 💤 Low valueSilently discarding
append_withfailure could cause benchmark to produce invalid asset location.
Location::append_withcan fail if the interior is already at maximum capacity (8 junctions). WhileErc20XcmBridgePalletLocationonly has 1 junction (PalletInstance), addingAccountKey20should succeed. However, defensive handling would be cleaner.♻️ Suggested defensive handling
let mut asset_location = Erc20XcmBridgePalletLocation::get(); - let _ = asset_location.append_with(AccountKey20 { + asset_location.append_with(AccountKey20 { key: contract.0, network: None, - }); + }).expect("ERC-20 location append must succeed in benchmark");🤖 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 `@runtime/moonbase/src/xcm_config.rs` around lines 881 - 885, The code currently ignores the Result from asset_location.append_with(...) which can fail; change the call to handle errors from Erc20XcmBridgePalletLocation::get().append_with(AccountKey20 { key: contract.0, network: None }) by checking the Result returned by append_with on the variable asset_location and failing loudly (e.g., use expect with a clear message or propagate an error) instead of discarding it so a failed append doesn't produce an invalid asset location; reference the symbols asset_location, append_with, AccountKey20, contract.0 and update the call site to handle the Err case explicitly.
🤖 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.
Nitpick comments:
In `@pallets/erc20-xcm-bridge/src/tests.rs`:
- Around line 81-84: The helper function trusted_loc() duplicates the test's
trusted location; update it to use the configured mock constant instead of
hardcoding the Location so tests stay in sync with the runtime config—replace
the body of trusted_loc() to return the mock runtime's trusted-location constant
(e.g. mock::TeleportTrustedLocation::get() or the appropriate
TeleportTrustedLocation constant) so the helper references the single source of
truth used by the tests.
In `@runtime/moonbase/src/xcm_config.rs`:
- Around line 881-885: The code currently ignores the Result from
asset_location.append_with(...) which can fail; change the call to handle errors
from Erc20XcmBridgePalletLocation::get().append_with(AccountKey20 { key:
contract.0, network: None }) by checking the Result returned by append_with on
the variable asset_location and failing loudly (e.g., use expect with a clear
message or propagate an error) instead of discarding it so a failed append
doesn't produce an invalid asset location; reference the symbols asset_location,
append_with, AccountKey20, contract.0 and update the call site to handle the Err
case explicitly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c558faf9-9113-42d7-a0ce-6f38b433eff6
⛔ Files ignored due to path filters (2)
runtime/moonbase/src/weights/pallet_xcm.rsis excluded by!**/weights/**/*.rsruntime/moonbase/src/weights/xcm/mod.rsis excluded by!**/weights/**/*.rs
📒 Files selected for processing (13)
pallets/erc20-xcm-bridge/src/lib.rspallets/erc20-xcm-bridge/src/mock.rspallets/erc20-xcm-bridge/src/tests.rsruntime/common/src/apis.rsruntime/common/src/lib.rsruntime/common/src/xcm_pallet_benchmark.rsruntime/moonbase/src/lib.rsruntime/moonbase/src/xcm_config.rsruntime/moonbase/tests/erc20_teleport.rsruntime/moonbeam/src/lib.rsruntime/moonbeam/src/xcm_config.rsruntime/moonriver/src/lib.rsruntime/moonriver/src/xcm_config.rs
|
@librelois last commits addressed your points, so this PR is ready for another review. One edge case that the current counter logic (
But even in this scenarion Moonbase is able to purge the token contract storage data by calling |
librelois
left a comment
librelois
left a comment
There was a problem hiding this comment.
Medium finding: whitelisting an ERC-20 changes all XCM TransactAsset handling for that contract on Moonbase, not only the limited_teleport_assets path.
Because Erc20TeleportTransactor is placed before the legacy Erc20XcmBridge adapter, Registered / Active contracts are handled by the teleport transactor first. That is intentional-looking from the comment, but it means ops should treat the whitelist as “AH teleport-only / changed XCM semantics for this ERC-20,” not as a narrow teleport flag.
What to change:
- Add a Moonbase integration test for
whitelisted ERC-20 + limited_reserve_transfer_assets. - Assert the expected behavior explicitly, likely
Filteredwhen the destination isAssetHubLocationbecause pallet-xcm classifies it as teleportable and rejects reserve-transfer for teleport assets. - Add/adjust a short code comment near
AssetTransactorsor the whitelist extrinsic docs saying that whitelisting changes generic XCM handling for that ERC-20, so only tokens intended for the AH teleport path should be whitelisted.
3 participants
Goal of the changes
Add a per-contract ERC-20 teleport mechanism to
pallet-erc20-xcm-bridgeand turn it onfor Moonbase so that whitelisted ERC-20s can be teleported to/from Asset Hub (para
1001 on Moonbase). Native assets (DEV / GLMR / MOVR) stay non-teleportable. Moonbeam
and Moonriver get the pallet wired but keep their existing
IsTeleporter = ()/XcmTeleportFilter = Nothinggates, so the feature is dormant on production runtimesuntil a follow-up PR flips them.
This PR also closes a high-severity split-transactor drain on deregistered
whitelisted ERC-20s: a local
polkadotXcm.executeprogram could routeWithdrawAssetthrough the legacy reserve adapter (bookkeeping only) and
DepositAssetthrough theteleport transactor (real payout from
TeleportCheckingAccount).What reviewers need to know
Most of the diff lives in
pallets/erc20-xcm-bridgeplus a small number of runtime /weight tweaks needed to make the new flow actually executable. Highlights:
Pallet (
pallets/erc20-xcm-bridge/src/lib.rs)Added a root-gated whitelist
TeleportableErc20s: StorageMap<H160, TeleportableErc20Status>with a three-state lifecycle (
Registered,Active,Deregistered) and aper-contract
LockedSupply: StorageMap<H160, U256>counter that tracks theoutstanding cross-chain obligation (i.e. how much supply this runtime currently
holds locked in
TeleportCheckingAccountfor that contract).The counter is what makes the lifecycle robust:
withdraw_assetsaturating-adds,
deposit_assetsaturating-subs,internal_transfer_assetis ano-op (same-chain hop, never moves the checking account). Promotion to
Activehappens inside the same storage layer as the EVM transfer, so promotion + counter
balanceOfcall.balanceOfwould tie the lifecycle to acontract that may revert/honeypot, drift on direct (non-XCM) transfers, and add a
full EVM round-trip per
removecall. The counter is just aTwox64Concat → U256storage read.
ERC20.transfer(checking, …)donations are intentionally ignored (they have noforeign-asset twin to track), and adversarial off-pallet drains conservatively
keep the entry non-purgeable.
Three extrinsics gated by the existing
TeleportAdminOriginconfig item:add_teleportable_erc20(contract)→ admin-only.(none) → Registered(fresh add) or
Deregistered → Registered(revival of a previously removedcontract; preserves any pre-existing
LockedSupply). Errors withErc20AlreadyTeleportableonly onRegistered/Active(the duplicate cases).Emits
TeleportableErc20Addedon both fresh-add and revival.remove_teleportable_erc20(contract)→ dual-purpose, with the caller'srequired origin depending on the current
LockedSupplyand status:LockedSupply == 0→ purge the entry outright (storage map + counter).Origin is admin-only when the current status is
Registered(so a thirdparty can't snipe a freshly added entry before the operator finishes
configuring) and when the current status is
Active(so a third partycannot snipe a live operational entry the moment its counter momentarily
hits zero between in/out flows — the operator stays in control of when an
active contract leaves the whitelist). Origin is permissionless (any
signed origin or root) only when the current status is
Deregistered:admin already opted into wind-down by flipping the entry, and the public
sweep just finalizes that intent once the obligation is fully discharged.
Emits
TeleportableErc20Purged.LockedSupply > 0→ admin-only, flipRegistered/ActivetoDeregistered. Outbound is closed for that contract; inbound stays open fromTeleportTrustedLocationso users can keep teleporting their twin home anddecrementing the counter. Already-
DeregisteredreturnsErc20AlreadyRemoved.Emits
TeleportableErc20Removed.Erc20NotTeleportable.force_remove_teleportable_erc20(contract)→ admin escape hatch. Deletes theentry and its counter regardless of state and counter value, emitting
TeleportableErc20ForceRemoved { contract, status_before, locked_supply }sothe act is auditable from chain events. Use only when the operator is willing
to forfeit any outstanding obligation (e.g. compromised contract).
Events:
TeleportableErc20Added,TeleportableErc20Activated,TeleportableErc20Removed,TeleportableErc20Purged,TeleportableErc20ForceRemoved.Errors:
Erc20AlreadyTeleportable,Erc20NotTeleportable,Erc20AlreadyRemoved.Added a new
Erc20TeleportTransactor<T>TransactAssetimpl that performs real EVMtransfers against
T::TeleportCheckingAccountand maintains the counter +status promotion atomically:
withdraw_asset(outbound) →ERC20.transfer(user → checking), thenLockedSupply += amount, thenRegistered → Activeif applicable.Outbound-eligible only (
RegisteredorActive).deposit_asset(inbound) →ERC20.transfer(checking → beneficiary), thenLockedSupply -=(saturating) amount, thenRegistered → Activeif applicable.Admits any whitelisted state (
Registered,Active,Deregistered) so removedcontracts can still unwind supply home.
internal_transfer_asset→ERC20.transfer(from → to)(same-chain). PromotesRegistered → Active(it's a successful flow handled by this transactor) butdoes NOT touch
LockedSupply(no checking-account interaction). Outbound-eligibleonly.
can_check_in/can_check_outenforce both the asset state AND thecounterparty location (
T::TeleportTrustedLocation).match_outbound).XcmError::AssetNotFoundsothe
AssetTransactorstuple falls through to the legacyErc20XcmBridgeadapter (reserve-mode single-EVM-call optimisation).
Deregistered→XcmError::FailedToTransactAsset(no fallthrough). Thisblocks fresh outbound locks and prevents the legacy adapter from handling the
withdraw leg of a split program.
Fungible(0)after the whitelist match: no EVM call, no
LockedSupplywrite, noRegistered → Activepromotion, noActivatedevent. Adversaries can stilldeliver
Fungible(0)via SCALE-decoded XCM (the upstreamFungibility::from(u128)debug_assert_ne!only protects local typedconstruction), so without this gate they could spam-promote whitelisted
contracts and burn EVM gas on no-op transfers. Non-whitelisted contracts with
Fungible(0)still surfaceAssetNotFoundso the legacy reserve adapter cantake over.
Legacy adapter fence (
legacy_transactor_rejects_teleportable). EveryErc20XcmBridgeTransactAssetleg (withdraw_asset,deposit_asset,internal_transfer_asset) returnsAssetNotFoundfor any contract present inTeleportableErc20s(all lifecycle states). Together with the teleporttransactor's
FailedToTransactAssetonDeregisteredoutbound, this prevents asingle XCM execution from splitting withdraw and deposit across two adapters —
the audited drain on deregistered contracts.
Deposit-side invariant (documented in code).
deposit_assetpays fromTeleportCheckingAccountwithout re-validating origin onDeregisteredentries.That is sound only because the executor gates every holding-fill path upstream
(
WithdrawAssetwith real EVM debit + legacy fence,ReceiveTeleportedAsset/IsTeleporter+can_check_in,ReserveAssetDeposited/IsReserve). Reviewersshould treat any future loosening of those gates as a checking-account drain risk.
IsTeleportableErc20<T>exposes bothContainsPair<Asset, Location>(forxcm_executor::Config::IsTeleporter) andContains<(Location, Vec<Asset>)>(forpallet_xcm::Config::XcmTeleportFilter). NativeSelfReservecannot match the ERC-20prefix, so it is structurally excluded from teleport.
ContainsPairimpl ANDs the whitelist (any state —Registered,Active,or
Deregistered) with a fixed counterpartyT::TeleportTrustedLocation.Without that bind, any sibling parachain or the relay able to deliver an XCM
could present a whitelisted ERC-20 in
ReceiveTeleportedAsset, passIsTeleporter, and drainTeleportCheckingAccountvia the follow-upDepositAsset. Each runtime points it at itsAssetHubLocation. AdmittingDeregisteredhere is deliberate — it's what keeps the teleport-back pathalive after
remove.Contains<(Location, Vec<Asset>)>impl admits ONLY outbound-eligible states(
Registered,Active) sopallet_xcm::limited_teleport_assetscannot starta fresh outbound teleport for a deregistered contract. It stays asset-only
because
pallet_xcm::limited_teleport_assetscalls it with the local caller'sorigin (not the destination) before the executor runs. The destination is gated
by
IsTeleporter::contains(asset, dest)inpallet_xcm::teleport_assets_program,which runs before any local
WithdrawAssetinstruction is built — so anon-AH destination errors with
Filteredand the user's ERC-20s never reach theEVM checking account.
Erc20TeleportTransactor::can_check_inandcan_check_outalso verify the counterparty location matches
T::TeleportTrustedLocation; if afuture change loosens
IsTeleporter, the transactor still refuses.Pallet
Confignow requiresTeleportAdminOrigin,TeleportCheckingAccount, andTeleportTrustedLocation.Moonbase wiring (
runtime/moonbase/src/{lib.rs,xcm_config.rs})construct_runtime!upgradesErc20XcmBridgefrom{Pallet}to{Pallet, Call, Storage, Event<T>}so the new admin extrinsics and events aredispatchable.
parameter_types! { Erc20TeleportCheckingAccount: H160 = H160(*b"erc20-teleport-check"); }— a fixed 20-byte payload chosen so the address has no preimage from any standard
derivation (no parachain sovereign, no AccountId32 alias).
pallet_erc20_xcm_bridge::ConfigaddsTeleportAdminOrigin = EnsureRoot<AccountId>,TeleportCheckingAccount = Erc20TeleportCheckingAccount, andTeleportTrustedLocation = AssetHubLocation(para 1001).AssetTransactorsnow includespallet_erc20_xcm_bridge::Erc20TeleportTransactor<Runtime>before the legacy
Erc20XcmBridge. Whitelisted contracts use the lock/unlocktransactor; everything else still uses the original single-EVM-call reserve
optimisation.
XcmExecutorConfig::IsTeleporter = pallet_erc20_xcm_bridge::IsTeleportableErc20<Runtime>and
pallet_xcm::Config::XcmTeleportFilter = IsTeleportableErc20<Runtime>.XcmPalletTeleportBenchmark) inserts the fixture asRegisteredso the user-facing
XcmTeleportFilteradmits it; the benchmark exercises the fulloutbound path which would auto-promote to
Activein real flows.Plumbing fixes that make teleport actually executable on Moonbase
runtime/moonbase/src/weights/xcm/mod.rs—receive_teleported_assetflipped fromWeight::MAXto a real measurable weight (reused fromreserve_asset_deposited),otherwise inbound teleport messages get bounced as
UnsupportedbymessageQueue.runtime/moonbase/src/weights/pallet_xcm.rs—teleport_assetsno longer returnsWeight::MAXfrom aBenchmark::Overrideplaceholder. It now aliasesSelf::transfer_assets(), since the user-facinglimited_teleport_assetsends up doingcomparable work (EVM withdraw + HRMP send). Comment added explaining the rationale.
runtime/common/src/xcm_pallet_benchmark.rs(new) and the accompanyingruntime/common/src/{lib.rs,apis.rs}edits — introduces anXcmPalletTeleportBenchmarkper-runtime trait so Moonbase can override
teleportable_asset_and_destforbenchmarking without forcing Moonbeam/Moonriver to do the same. Default
Nonekeepsupstream behaviour for the dormant runtimes.
Moonbeam / Moonriver (
runtime/{moonbeam,moonriver}/src/{lib.rs,xcm_config.rs})construct_runtime!exposesCall, Storage, Event<T>for the pallet so the workspacebuilds.
Configis wired with the sameTeleportAdminOrigin/TeleportCheckingAccount/TeleportTrustedLocation.IsTeleporterandXcmTeleportFilterare intentionally left at()andNothing.The whitelist storage exists but no teleport flow is reachable on these runtimes until a
follow-up PR flips both gates and applies the same weight fixes.
Tests
pallets/erc20-xcm-bridge/src/tests.rs— 31 unit tests covering:Registered; duplicate add rejected onRegistered/Active;revival from
DeregisteredpreservesLockedSupply; admin-origin enforcement.withdraw_asset,deposit_asset, andinternal_transfer_assetall flipRegistered → Activeexactly once and emitTeleportableErc20Activatedonly on the actual transition.withdraw_assetaccumulates;deposit_assetsaturating-subs(handles pre-seeded twin supply by clamping to zero rather than aborting);
internal_transfer_assetdoes not move the counter.removematrix: rejects unknown;Registered + count == 0admin-only purge;Active + count == 0admin-only purge (live entries cannot be sniped on atransient-zero counter);
Deregistered + count == 0permissionless purge(any signed origin or root) — the only state from which this is allowed;
Active + count > 0admin-only flip toDeregistered;Deregistered + count > 0returnsErc20AlreadyRemoved.inbound drains counter to 0 → permissionless purge → re-add succeeds.
force_remove: admin-only; rejects unknown; purges any state and counter;audit event records
status_beforeandlocked_supply; post-force_removeinbound is refused (entry is gone).
is_teleportable_erc20,is_outbound_eligible_erc20),IsTeleporterlocation-bind,XcmTeleportFilteroutbound-eligible-only withempty/mixed-list defenses.
outbound_transactor_rejects_deregistered_and_unknown(Deregistered→FailedToTransactAsset, unknown →AssetNotFound);deregistered_split_transactor_withdraw_deposit_cannot_drain_checking_account;legacy_transactor_rejects_all_teleportable_erc20_states.Fungible(0)for whitelistedcontracts (counter unchanged, status stays
Registered, noActivatedevent), and non-whitelisted contracts with
Fungible(0)still surfaceAssetNotFound.runtime/moonbase/tests/erc20_teleport.rs— 11 integration tests on the actualmoonbase-runtime:passes both filter forms; untrusted locations rejected on
IsTeleporter.EnsureRoot;force_remove_teleportable_erc20escape hatch;deregistered_contract_keeps_inbound_open_and_blocks_outbound_via_runtime_filters.limited_teleport_assetsrejects non-AH destinations;limited_reserve_transfer_assetsto AH returnsFilteredfor whitelisted ERC-20;non-AH reserve transfer does not strand funds in the checking account.
deregistered_withdraw_deposit_program_cannot_drain_checking_account— fullXcmExecutor::prepare_and_executewith[WithdrawAsset, DepositAsset]on aderegistered contract (tuple-level regression for the audited drain).
test/suites/dev/moonbase/test-xcm-teleport/test-xcm-teleport-erc20.ts— Moonwallsuite
D024501(foundationMethods: dev, three cases):limited_teleport_assetsto mock AH (para 1001);asserts Alith balance ↓, checking ↑,
LockedSupply↑, statusActive.ReceiveTeleportedAsset+DepositAsset(DEV feeswithdrawn from AH sovereign; native cannot ride
ReceiveTeleportedAsset); unwindslocked supply to Charleth.
DrainCoincontract: deploy → whitelist → seed checking → setLockedSupply→deregister → Charleth
polkadotXcm.execute([WithdrawAsset, BuyExecution, DepositAsset])must fail with balances andLockedSupplyunchanged.Risks / compatibility
they cannot pass
IsTeleportableErc20. This is asserted by theteleport_filter_rejects_native_devintegration test.IsTeleporterandXcmTeleportFilterare unchanged, sopallet_xcm::limited_teleport_assetsis stillFiltered. The only observable surfacechange on production is the addition of three root-only / permissionless-purge admin
extrinsics (no-op until the gates flip).
After
remove_teleportable_erc20withLockedSupply > 0, users can still teleporthome from AH, but fresh outbound teleports and local
polkadotXcm.executewithdraw/depositprograms cannot extract checking-account supply via the legacy + teleport adapter split.
Deregistered.remove_teleportable_erc20accepts any signed origin (or root) only when
LockedSupply == 0and thestatus is
Deregistered. This is intentional: by deregistering the entry theadmin has explicitly opted into wind-down, and once the obligation is fully
discharged the public sweep just finalizes that intent — giving a deregistered
contract a natural terminus without requiring operator intervention.
Active + count == 0is admin-only, deliberately: a live operational contractroutinely transits through
count == 0between in/out flows, so admitting athird-party purge there would let anyone snipe an active entry the moment its
counter momentarily hits zero, forcing the admin to re-add.
Registered + count == 0is admin-only too so a third party can't undo a fresh add. Fordefense in depth there's also
force_remove_teleportable_erc20(admin-only)to delete any entry regardless of state.
Because
Erc20TeleportTransactordoes the lock inwithdraw_assetand the unlock indeposit_asset(no in-message tracker), an MB→non-AH reserve transfer of a whitelistedcontract uses the checking address as an intermediate hop. End state is identical to
today; gas cost is doubled for that specific path. Non-whitelisted ERC-20s still use the
legacy single-call reserve optimisation untouched.
teleport will succeed until Asset Hub registers the ERC-20 foreign-asset twin and calls
pallet_assets::set_reserves(asset, [{ reserve: (1, Parachain(<MB para>)), teleportable: true }]).The local-zombienet helper script we use for smoke testing handles AH-Westend (para
1001); production AH-Polkadot needs governance to do the same calls.
TeleportableErc20smap andLockedSupplycounter both start empty. The pallet index for
Erc20XcmBridgeis preserved on allthree runtimes (48 / 110 / 110). Adding
Call/Storage/Eventtoconstruct_runtime!widens the metadata but does not move state.Testing
cargo check -p pallet-erc20-xcm-bridge -p moonbase-runtime -p moonbeam-runtime -p moonriver-runtimeclean.
cargo test -p pallet-erc20-xcm-bridge— 31/31 pass (see breakdown above).cargo test --test erc20_teleport -p moonbase-runtime— 11/11 pass (see breakdown above).pnpm moonwall test dev_moonbase D024501— 3/3 pass (dev-node HRMP mock + EVM regressions).cargo check -p moonbase-runtime --features runtime-benchmarksclean (withWASM_BUILD_WORKSPACE_HINT=$PWD).ReadLintsclean on all touched files.Intentionally not in this PR yet: flipping
IsTeleporter/XcmTeleportFilteronMoonbeam (and Moonriver).