From 024511cae970e8350614e0781332cb1b03fe8fb2 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Tue, 24 Sep 2024 14:14:43 +0300 Subject: [PATCH] BACKPORT-CONFLICT --- .../pallets/inbound-queue/src/test.rs | 4 +- .../primitives/router/src/inbound/mod.rs | 44 +++++++++++++++++-- .../bridges/bridge-hub-rococo/src/lib.rs | 14 ++++-- .../bridge-hub-rococo/src/tests/snowbridge.rs | 18 +++++--- prdoc/pr_5563.prdoc | 14 ++++++ 5 files changed, 80 insertions(+), 14 deletions(-) create mode 100644 prdoc/pr_5563.prdoc 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 54e47a7a8b6a..151c3978b4bf 100644 --- a/bridges/snowbridge/primitives/router/src/inbound/mod.rs +++ b/bridges/snowbridge/primitives/router/src/inbound/mod.rs @@ -167,7 +167,7 @@ where 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); @@ -180,8 +180,15 @@ where // 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)), @@ -198,10 +205,17 @@ where .encode() .into(), }, +<<<<<<< HEAD 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. +>>>>>>> 62534e5 (snowbridge: improve destination fee handling to avoid trapping fees dust (#5563)) ] .into(); @@ -255,17 +269,31 @@ where 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 }, +<<<<<<< HEAD // 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()), +>>>>>>> 62534e5 (snowbridge: improve destination fee handling to avoid trapping fees dust (#5563)) ] .into(), }, @@ -281,6 +309,14 @@ where }, } +<<<<<<< HEAD +======= + // 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`). +>>>>>>> 62534e5 (snowbridge: improve destination fee handling to avoid trapping fees dust (#5563)) (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 04466a611c71..cf4a3c3df379 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 @@ -53,9 +53,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, }, @@ -66,8 +69,13 @@ mod imports { AssetHubWestendParaSender as AssetHubWestendSender, BridgeHubRococoPara as BridgeHubRococo, BridgeHubRococoParaSender as BridgeHubRococoSender, BridgeHubWestendPara as BridgeHubWestend, PenpalAPara as PenpalA, +<<<<<<< HEAD PenpalAParaReceiver as PenpalAReceiver, PenpalAParaSender as PenpalASender, RococoRelay as Rococo, +======= + PenpalAParaSender as PenpalASender, RococoRelay as Rococo, + RococoRelayReceiver as RococoReceiver, RococoRelaySender as RococoSender, +>>>>>>> 62534e5 (snowbridge: improve destination fee handling to avoid trapping fees dust (#5563)) }; 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 40a1968ec557..b8617413211d 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