Skip to content

Commit

Permalink
BACKPORT-CONFLICT
Browse files Browse the repository at this point in the history
  • Loading branch information
acatangiu authored and github-actions[bot] committed Sep 24, 2024
1 parent 51a1705 commit 024511c
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 14 deletions.
4 changes: 2 additions & 2 deletions bridges/snowbridge/pallets/inbound-queue/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
44 changes: 40 additions & 4 deletions bridges/snowbridge/primitives/router/src/inbound/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)),
Expand All @@ -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();

Expand Down Expand Up @@ -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(),
},
Expand All @@ -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())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<PenpalA as Chain>::RuntimeOrigin::signed(PenpalAssetOwner::get()),
native_id,
receiver,
penpal_runtime::EXISTENTIAL_DEPOSIT,
);

PenpalA::execute_with(|| {
assert_ok!(<PenpalA as Chain>::System::set_storage(
Expand Down
14 changes: 14 additions & 0 deletions prdoc/pr_5563.prdoc
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 024511c

Please sign in to comment.