Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

Custom extrinsic & Dynamic remote fee #158

Closed
wants to merge 16 commits into from
Closed

Conversation

yrong
Copy link

@yrong yrong commented Jul 16, 2024

Resolves: https://linear.app/snowfork/issue/SNO-1084#comment-0d5970ee

  • Add a helper pallet specific for xcm operations(transact, transfer) from Substrate to Ethereum through snowbridge.
  • Integrate the helper pallet into runtime of asset hub.
  • Remove the XcmExportFeeToSibling on BridgeHub which is unnecessary.
  • Revamp integration test to use the new extrinsic transfer_to_ethereum for transfer from AH to Ethereum.

@yrong
Copy link
Author

yrong commented Jul 17, 2024

In 0fe0484#diff-5ded4b43c234d6f2d43aff2b2b63adfde7a5ba2dd9fc591cd818335e20b6303b I tested with the new extrinsic transfer_assets_using_type_and_then and yeah the send_weth_asset_from_asset_hub_to_ethereum test case can pass, but does not work the way as we want.

The main issue is the limitation mentioned here it can't take fees separately.

Say we have 2 assets [(weth_location,1),(weth_location,1)], with the fee index as 0, the intention is to use the first asset as remote fee and send the second to beneficiary. But there is some deduplicated logic here the 2 assets will be merged into a single one (weth_location,2) and prevent us for taking fees separately.

The extrinsic transfer_assets_using_type_and_then won't help here and that's another reason why I add the custom call for that.

@vgeddes
Copy link

vgeddes commented Jul 17, 2024

I may be wrong, but my high-level understanding from reading the documentation for transfer_assets_using_type_and_then is that fees asset and the asset being transferred can be the same.

Or in other words, the fee amount can be deducted from the asset specified by the user.

In any case, in the new fee design I'm working on, users will most likely pay fees in DOT instead of WETH. The existence of a DEX on AssetHub does make fee currencies other than DOT a bit unnecessary.

@yrong
Copy link
Author

yrong commented Jul 17, 2024

fees asset and the asset being transferred can be the same.

That's not possible as demonstrated in 14894a1, for transfer_assets_using_type_and_then you can specify asset id of the fee but not the index.

users will most likely pay fees in DOT instead of WETH

Yeah, with the AssetExchanger config it's possible.

FYI there is one PR for that in paritytech#4375 I'm reviewing.

@yrong
Copy link
Author

yrong commented Jul 19, 2024

Close in favor of #160

@yrong yrong closed this Jul 19, 2024
origin: OriginFor<T>,
beneficiary: H160,
asset: Box<VersionedAsset>,
fee: Box<VersionedAsset>,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically just mirror the reserve_transfer_assets extrinsic from pallet_xcm with an explicit fee specified here.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 🚀

@yrong yrong reopened this Aug 3, 2024
@yrong yrong changed the title Dynamic fee Dynamic remote fee Aug 5, 2024
@yrong yrong changed the title Dynamic remote fee Custom extrinsic & Dynamic remote fee Aug 5, 2024
SendXcmFeeToAccount<Self::AssetTransactor, TreasuryAccount>,
),
>;
type MessageExporter = (
crate::bridge_to_westend_config::ToBridgeHubWestendHaulBlobExporter,
crate::bridge_to_bulletin_config::ToRococoBulletinHaulBlobExporter,
crate::bridge_to_ethereum_config::SnowbridgeExporter,
crate::bridge_to_ethereum_config::SnowbridgeExporterV2,
Copy link
Author

@yrong yrong Aug 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems better to migrate to the dynamic fee mode while still support the legacy mode(i.e. reserve transfer with static fee configured on chain), the SnowbridgeExporterV2 should work side by side with the previous version.

So we can run tests for:

  • v1
cargo test -p bridge-hub-rococo-integration-tests --lib tests::snowbridge::send_weth_asset_from_asset_hub_to_ethereum  -- --nocapture
  • v2
cargo test -p bridge-hub-rococo-integration-tests --lib tests::snowbridge_transfer_v2::send_weth_asset_from_asset_hub_to_ethereum  -- --nocapture

and both should work as expected.

@yrong yrong closed this Aug 22, 2024
@yrong yrong added the xcm label Aug 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants