Revamp xcm for the reserver tranfer from substrate to Ethreum & Implement a custom exporter #160

yrong · 2024-07-18T15:52:26Z

To use the low level execute call directly to replace the reserve_transfer_asset which has limitations.

More context in https://matrix.to/#/!gxqZwOyvhLstCgPJHO:matrix.parity.io/$uMy6wP7SCEpBbYAYbwqGU4Z2VjIpM6Gvv-4MxgVKJJM?via=matrix.parity.io&via=parity.io&via=matrix.org

New Fee flow

User bonds an estimated DOT fee on BridgeHub, that has previously been calculated off-chain
BridgeHub commits the message for delivery to Ethereum
Relayer sends message to our Gateway contract on Ethereum.
Relayer sends proof-of-delivery back to BridgeHub
BridgeHub gives bonded fee to relayer

https://docs.snowbridge.network/~/changes/HBJiUn5s4cV0iP3g68Sk/other/snowbridge-v2/unordered-delivery

...arachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs

vgeddes · 2024-07-19T07:34:30Z

Ron to clarify I meant BridgeHub is the source

vgeddes · 2024-07-22T20:39:06Z

...arachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs

+		let xcms = VersionedXcm::from(Xcm(vec![
+			WithdrawAsset(assets.clone().into()),
+			SetFeesMode { jit_withdraw: true },
+			InitiateReserveWithdraw {


OK, this looks good, fairly simple 👍

Can you check what we would need for Polkadot-native assets? I suspect its TransferReserveAsset with the destination being our gateway contract.

I suspect its TransferReserveAsset

Exactly.

destination being our gateway contract.

Currently the destination is always (Location::new(2, [GlobalConsensus(Ethereum { chain_id: CHAIN_ID })])

vgeddes · 2024-07-22T20:44:33Z

...arachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs

+
+		let xcms = VersionedXcm::from(Xcm(vec![
+			WithdrawAsset(assets.clone().into()),
+			SetFeesMode { jit_withdraw: true },


Why SetFeesMode instead of BuyExecution? I think the user should have an opportunity to pass the necessary fee amounts after estimating fees using the XCM dry run extrinsic.

Also they want to deprecate SetFeesMode in polkadot-fellows/xcm-format#57.

The fee asset(DOT) is already included in WithdrawAsset(assets.clone().into()), actually we transfer 2 kinds of assets to Ethereum(DOT and WETH) and use DOT to pay for the delivery cost on BH.

The SetFeesMode is taking the delivery cost on AH from user for InitiateReserveWithdraw, suggest to run the test case with

RUST_LOG=xcm=trace cargo test -p bridge-hub-rococo-integration-tests --lib tests::snowbridge::send_weth_asset_from_asset_hub_to_ethereum -- --nocapture

and check logs as follows for detail:

... 2024-07-23T02:04:58.003798Z TRACE xcm::fees: taking fee: Assets([Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(1106266603) }]) from origin_ref: Some(Location { parents: 0, interior: X1([AccountId32 { network: Some(Rococo), id: [142, 175, 4, 21, 22, 135, 115, 99, 38, 201, 254, 161, 126, 37, 252, 82, 135, 97, 54, 147, 201, 18, 144, 156, 178, 38, 170, 71, 148, 242, 106, 72] }]) }) in fees_mode: FeesMode { jit_withdraw: true } for a reason: InitiateReserveWithdraw

Can't the executor take fees from the holding register? DOT has already been transferred to the holding register after the execution of WithdrawAsset.

We can but I don't see benefit for adding an extra BuyExecution if SetFeesMode can work here.

Btw: polkadot-fellows/xcm-format#57 is closed so it won't impact and I expect more on polkadot-fellows/RFCs#105 which is another story.

yrong · 2024-07-23T02:29:32Z

Move the reward logic to #162 and make this PR more focused on constructing/handling xcm.

yrong · 2024-07-23T10:14:10Z

cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs

+			/// User fee for delivery cost on bridge hub. Leave some buffer here for avoid spamming
+			pub const DefaultBridgeHubEthereumBaseFee: Balance = 1_000_000_000;


Should at least cover the execution cost of the fee on BH(i.e. the local portion).

Ah, we should not bake this constant into AH. BH needs to estimate it on-chain, and then our off-chain UX needs to query it in order to estimate the total fees required

The XCM fee estimation APIs will include the local fee, so we should be fine in that regard.

The fee config here will also be used to BuyExecution for the xcm on BH which can't be zero.

It should at least cover the xcm execution cost(e.g. The ExportMessage instruction) but I would suggest to leave some buffer to also include the submit cost(i.e. the local portion) here to ensure our OutboundQueue not spammed.

Btw: it's very tiny here so IMO it's acceptable trade-off continue to be estimated on-chain with considerable buffer for this portion. Somehow mirrors the fee configuration on Ethreum side which we won't remove.

yrong · 2024-07-25T14:38:16Z

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs

@@ -205,7 +203,7 @@ impl xcm_executor::Config for XcmConfig {
 	type SubscriptionService = PolkadotXcm;
 	type PalletInstancesInfo = AllPalletsWithSystem;
 	type MaxAssetsIntoHolding = MaxAssetsIntoHolding;
-	type FeeManager = XcmFeeManagerFromComponentsBridgeHub<
+	type FeeManager = XcmFeeManagerFromComponents<


The custom FeeManager is not in used and removed, since AH is in the WaivedLocations no need to pay delivery cost for Ethreum transfer.

...arachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs

yrong · 2024-07-29T13:27:13Z

...arachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs

-	BridgeHubRococo::fund_accounts(vec![(assethub_sovereign.clone(), INITIAL_FUND)]);
-


Should remove this initial fund as there is no need to fund the sovereign here any more.

yrong · 2024-07-29T13:31:13Z

...arachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs

+		let free_balance_of_sovereign_on_bh_after =
+			<BridgeHubRococo as BridgeHubRococoPallet>::Balances::free_balance(assethub_sovereign);
+		assert_eq!(free_balance_of_sovereign_on_bh_after, 955613334);


Currently there is no need to fund the sovereign account on BH any more as we will use a InitiateTeleport instruction for an instant top up.

As we may notice after the test there is still some fee left unused in the sov account. So bad guys can leverage that to ignore the InitiateTeleport instruction later.

Another issue for current impl is that we have to charge twice from user for the DefaultBridgeHubEthereumBaseFee, one for the InitiateTeleport for an instant top up as mentioned before, the other for the delivery cost here sending the remote xcm of InitiateReserveWithdraw to Ethreum through BH for the Export.

Can run RUST_LOG=xcm=trace cargo test -p bridge-hub-rococo-integration-tests --lib tests::snowbridge::send_weth_asset_from_asset_hub_to_ethereum -- --nocapture for the execution path.

Addressed in #167

yrong commented Jul 18, 2024

View reviewed changes

...arachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs Outdated Show resolved Hide resolved

yrong requested a review from vgeddes July 18, 2024 16:17

yrong mentioned this pull request Jul 19, 2024

[bridges][AHs] Include ExportMessage fees in XCM message from AH to BH polkadot-fellows/runtimes#392

Open

yrong force-pushed the ron/custom-execute-transfer branch from 058a948 to 3a79d45 Compare July 19, 2024 09:28

yrong mentioned this pull request Jul 19, 2024

Custom extrinsic & Dynamic remote fee #158

Closed

yrong requested a review from alistair-singh July 19, 2024 09:42

yrong changed the title ~~Use low level execute directly~~ Use low level execute call for reserve based tranfer Jul 19, 2024

yrong changed the title ~~Use low level execute call for reserve based tranfer~~ Lock fee on BH and pay reward to relay Jul 21, 2024

yrong mentioned this pull request Jul 22, 2024

Reward on BH Snowfork/snowbridge#1261

Closed

yrong requested a review from claravanstaden July 22, 2024 00:35

vgeddes reviewed Jul 22, 2024

View reviewed changes

Focus on the xcm construction & Remove reward

d489756

yrong force-pushed the ron/custom-execute-transfer branch from 80537ed to d489756 Compare July 23, 2024 00:44

Change the value of local fee

6729e4b

yrong changed the title ~~Lock fee on BH and pay reward to relay~~ Revamp xcm for the reserver tranfer from substrate to Ethreeum direction Jul 23, 2024

yrong changed the title ~~Revamp xcm for the reserver tranfer from substrate to Ethreeum direction~~ Revamp xcm for the reserver tranfer from substrate to Ethreum Jul 23, 2024

yrong commented Jul 23, 2024

View reviewed changes

yrong commented Jul 25, 2024

View reviewed changes

Merge branch 'snowbridge' into ron/custom-execute-transfer

046c9fb

yrong marked this pull request as ready for review July 29, 2024 11:30

Improve test checking balance

e927ced

yrong commented Jul 29, 2024

View reviewed changes

...arachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs Outdated Show resolved Hide resolved

yrong commented Jul 29, 2024

View reviewed changes

yrong added 3 commits July 30, 2024 09:25

Adjust fees more accurate

93f952e

Add a custom SovereignReceiveTeleportRemoteExporter

88c31b1

Revamp the xcm

a5a384c

yrong changed the title ~~Revamp xcm for the reserver tranfer from substrate to Ethreum~~ Revamp xcm for the reserver tranfer from substrate to Ethreum & Implement a custom exporter Aug 1, 2024

yrong mentioned this pull request Aug 6, 2024

Initial POCs improving snowbridge paritytech/polkadot-sdk#5212

Closed

yrong closed this Aug 22, 2024

yrong added the xcm label Aug 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revamp xcm for the reserver tranfer from substrate to Ethreum & Implement a custom exporter #160

Revamp xcm for the reserver tranfer from substrate to Ethreum & Implement a custom exporter #160

yrong commented Jul 18, 2024 •

edited

Loading

vgeddes commented Jul 19, 2024

vgeddes Jul 22, 2024 •

edited

Loading

yrong Jul 23, 2024 •

edited

Loading

vgeddes Jul 22, 2024

yrong Jul 23, 2024 •

edited

Loading

vgeddes Jul 23, 2024 •

edited

Loading

yrong Jul 24, 2024

yrong commented Jul 23, 2024

yrong Jul 23, 2024 •

edited

Loading

vgeddes Jul 23, 2024

vgeddes Jul 23, 2024

yrong Jul 24, 2024 •

edited

Loading

yrong Jul 25, 2024

yrong Jul 29, 2024

yrong Jul 29, 2024 •

edited

Loading

yrong Jul 31, 2024

		/// User fee for delivery cost on bridge hub. Leave some buffer here for avoid spamming
		pub const DefaultBridgeHubEthereumBaseFee: Balance = 1_000_000_000;

		BridgeHubRococo::fund_accounts(vec![(assethub_sovereign.clone(), INITIAL_FUND)]);

Revamp xcm for the reserver tranfer from substrate to Ethreum & Implement a custom exporter #160

Revamp xcm for the reserver tranfer from substrate to Ethreum & Implement a custom exporter #160

Conversation

yrong commented Jul 18, 2024 • edited Loading

New Fee flow

vgeddes commented Jul 19, 2024

vgeddes Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

yrong Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

vgeddes Jul 22, 2024

Choose a reason for hiding this comment

yrong Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

vgeddes Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

yrong Jul 24, 2024

Choose a reason for hiding this comment

yrong commented Jul 23, 2024

yrong Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

vgeddes Jul 23, 2024

Choose a reason for hiding this comment

vgeddes Jul 23, 2024

Choose a reason for hiding this comment

yrong Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

yrong Jul 25, 2024

Choose a reason for hiding this comment

yrong Jul 29, 2024

Choose a reason for hiding this comment

yrong Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

yrong Jul 31, 2024

Choose a reason for hiding this comment

yrong commented Jul 18, 2024 •

edited

Loading

vgeddes Jul 22, 2024 •

edited

Loading

yrong Jul 23, 2024 •

edited

Loading

yrong Jul 23, 2024 •

edited

Loading

vgeddes Jul 23, 2024 •

edited

Loading

yrong Jul 23, 2024 •

edited

Loading

yrong Jul 24, 2024 •

edited

Loading

yrong Jul 29, 2024 •

edited

Loading