diff --git a/bridges/snowbridge/pallets/inbound-queue/src/test.rs b/bridges/snowbridge/pallets/inbound-queue/src/test.rs index bd993c968df7..41c38460aabf 100644 --- a/bridges/snowbridge/pallets/inbound-queue/src/test.rs +++ b/bridges/snowbridge/pallets/inbound-queue/src/test.rs @@ -40,8 +40,8 @@ fn test_submit_happy_path() { .into(), nonce: 1, message_id: [ - 57, 61, 232, 3, 66, 61, 25, 190, 234, 188, 193, 174, 13, 186, 1, 64, 237, 94, 73, - 83, 14, 18, 209, 213, 78, 121, 43, 108, 251, 245, 107, 67, + 255, 125, 48, 71, 174, 185, 100, 26, 159, 43, 108, 6, 116, 218, 55, 155, 223, 143, + 141, 22, 124, 110, 241, 18, 122, 217, 130, 29, 139, 76, 97, 201, ], fee_burned: 110000000000, } diff --git a/bridges/snowbridge/primitives/router/src/inbound/mod.rs b/bridges/snowbridge/primitives/router/src/inbound/mod.rs index 5cff8413af66..a10884b45531 100644 --- a/bridges/snowbridge/primitives/router/src/inbound/mod.rs +++ b/bridges/snowbridge/primitives/router/src/inbound/mod.rs @@ -249,7 +249,7 @@ impl< let total_amount = fee + CreateAssetDeposit::get(); let total: Asset = (Location::parent(), total_amount).into(); - let bridge_location: Location = (Parent, Parent, GlobalConsensus(network)).into(); + let bridge_location = Location::new(2, GlobalConsensus(network)); let owner = GlobalConsensusEthereumConvertsFor::<[u8; 32]>::from_chain_id(&chain_id); let asset_id = Self::convert_token_address(network, token); @@ -262,8 +262,15 @@ impl< // Pay for execution. BuyExecution { fees: xcm_fee, weight_limit: Unlimited }, // Fund the snowbridge sovereign with the required deposit for creation. - DepositAsset { assets: Definite(deposit.into()), beneficiary: bridge_location }, - // Only our inbound-queue pallet is allowed to invoke `UniversalOrigin` + DepositAsset { assets: Definite(deposit.into()), beneficiary: bridge_location.clone() }, + // This `SetAppendix` ensures that `xcm_fee` not spent by `Transact` will be + // deposited to snowbridge sovereign, instead of being trapped, regardless of + // `Transact` success or not. + SetAppendix(Xcm(vec![ + RefundSurplus, + DepositAsset { assets: AllCounted(1).into(), beneficiary: bridge_location }, + ])), + // Only our inbound-queue pallet is allowed to invoke `UniversalOrigin`. DescendOrigin(PalletInstance(inbound_queue_pallet_index).into()), // Change origin to the bridge. UniversalOrigin(GlobalConsensus(network)), @@ -280,12 +287,10 @@ impl< .encode() .into(), }, - RefundSurplus, - // Clear the origin so that remaining assets in holding - // are claimable by the physical origin (BridgeHub) - ClearOrigin, // Forward message id to Asset Hub SetTopic(message_id.into()), + // Once the program ends here, appendix program will run, which will deposit any + // leftover fee to snowbridge sovereign. ] .into(); @@ -340,17 +345,24 @@ impl< match dest_para_id { Some(dest_para_id) => { let dest_para_fee_asset: Asset = (Location::parent(), dest_para_fee).into(); + let bridge_location = Location::new(2, GlobalConsensus(network)); instructions.extend(vec![ + // After program finishes deposit any leftover assets to the snowbridge + // sovereign. + SetAppendix(Xcm(vec![DepositAsset { + assets: Wild(AllCounted(2)), + beneficiary: bridge_location, + }])), // Perform a deposit reserve to send to destination chain. DepositReserveAsset { - assets: Definite(vec![dest_para_fee_asset.clone(), asset.clone()].into()), + assets: Definite(vec![dest_para_fee_asset.clone(), asset].into()), dest: Location::new(1, [Parachain(dest_para_id)]), xcm: vec![ // Buy execution on target. BuyExecution { fees: dest_para_fee_asset, weight_limit: Unlimited }, - // Deposit asset to beneficiary. - DepositAsset { assets: Definite(asset.into()), beneficiary }, + // Deposit assets to beneficiary. + DepositAsset { assets: Wild(AllCounted(2)), beneficiary }, // Forward message id to destination parachain. SetTopic(message_id.into()), ] @@ -371,6 +383,8 @@ impl< // Forward message id to Asset Hub. instructions.push(SetTopic(message_id.into())); + // The `instructions` to forward to AssetHub, and the `total_fees` to locally burn (since + // they are teleported within `instructions`). (instructions.into(), total_fees.into()) } diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/lib.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/lib.rs index ac08e48ded68..9d54d431e839 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/lib.rs @@ -55,9 +55,12 @@ mod imports { BridgeHubRococoXcmConfig, EthereumBeaconClient, EthereumInboundQueue, }, penpal_emulated_chain::{ - penpal_runtime::xcm_config::{ - CustomizableAssetFromSystemAssetHub as PenpalCustomizableAssetFromSystemAssetHub, - UniversalLocation as PenpalUniversalLocation, + penpal_runtime::{ + self, + xcm_config::{ + CustomizableAssetFromSystemAssetHub as PenpalCustomizableAssetFromSystemAssetHub, + UniversalLocation as PenpalUniversalLocation, + }, }, PenpalAParaPallet as PenpalAPallet, PenpalAssetOwner, }, @@ -72,9 +75,8 @@ mod imports { BridgeHubRococoParaReceiver as BridgeHubRococoReceiver, BridgeHubRococoParaSender as BridgeHubRococoSender, BridgeHubWestendPara as BridgeHubWestend, PenpalAPara as PenpalA, - PenpalAParaReceiver as PenpalAReceiver, PenpalAParaSender as PenpalASender, - RococoRelay as Rococo, RococoRelayReceiver as RococoReceiver, - RococoRelaySender as RococoSender, + PenpalAParaSender as PenpalASender, RococoRelay as Rococo, + RococoRelayReceiver as RococoReceiver, RococoRelaySender as RococoSender, }; pub const ASSET_MIN_BALANCE: u128 = 1000; diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs index 84328fb7c6d2..d91a0c6895f9 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs @@ -286,11 +286,19 @@ fn send_token_from_ethereum_to_penpal() { // Fund AssetHub sovereign account so it can pay execution fees for the asset transfer BridgeHubRococo::fund_accounts(vec![(asset_hub_sovereign.clone(), INITIAL_FUND)]); - // Fund PenPal sender and receiver - PenpalA::fund_accounts(vec![ - (PenpalAReceiver::get(), INITIAL_FUND), - (PenpalASender::get(), INITIAL_FUND), - ]); + // Fund PenPal receiver (covering ED) + let native_id: Location = Parent.into(); + let receiver: AccountId = [ + 28, 189, 45, 67, 83, 10, 68, 112, 90, 208, 136, 175, 49, 62, 24, 248, 11, 83, 239, 22, 179, + 97, 119, 205, 75, 119, 184, 70, 242, 165, 240, 124, + ] + .into(); + PenpalA::mint_foreign_asset( + ::RuntimeOrigin::signed(PenpalAssetOwner::get()), + native_id, + receiver, + penpal_runtime::EXISTENTIAL_DEPOSIT, + ); PenpalA::execute_with(|| { assert_ok!(::System::set_storage( diff --git a/prdoc/pr_5563.prdoc b/prdoc/pr_5563.prdoc new file mode 100644 index 000000000000..cbf436125bb5 --- /dev/null +++ b/prdoc/pr_5563.prdoc @@ -0,0 +1,14 @@ +title: "snowbridge: improve destination fee handling to avoid trapping fees dust" + +doc: + - audience: Runtime User + description: | + On Ethereum -> Polkadot Asset Hub messages, whether they are a token transfer + or a `Transact` for registering a new token, any unspent fees are deposited to + Snowbridge's sovereign account on Asset Hub, rather than trapped in AH's asset trap. + +crates: + - name: snowbridge-router-primitives + bump: patch + - name: snowbridge-pallet-inbound-queue + bump: patch