Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make bridge fees as params of reserve_transfer_assets #3931

Closed
2 tasks done
yrong opened this issue Apr 2, 2024 · 7 comments
Closed
2 tasks done

Make bridge fees as params of reserve_transfer_assets #3931

yrong opened this issue Apr 2, 2024 · 7 comments
Labels
I5-enhancement An additional feature request. T6-XCM This PR/Issue is related to XCM.

Comments

@yrong
Copy link
Contributor

yrong commented Apr 2, 2024

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Motivation

Context

Currently, the bridge/export fee is configured in BridgeTable as

Some((
XcmBridgeHubRouterFeeAssetId::get(),
BridgeHubEthereumBaseFee::get(),
).into())
which will cover all the costs for the export fee in Relay token.

As Snowbridge is delivering more bridge features(e.g. transfer native token like DOT to Ethereum, transfer WETH back to Ethereum, Call Arbitrary Transact on Ethereum, etc...) each has a different cost, it does not make sense to be a runtime configuration anymore. Instead, we'd like the fees to be params of the extrinsic reserve_transfer_assets.

Another issue is that it's the total cost including the cost of the two hops execution on BridgeHub and on Ethereum. We'd like to separate them as two different fees. For the first hop fee in Relay token and the second hop fee in WETH.

We can provide runtime APIs to calculate fees for each of the hops and it's the prerequisite to remove the exchange-rate on BH which is another runtime configuration difficult to maintain on chain.

Request

  • Make bridge fees as params of reserve_transfer_assets
  • Separate the fee as two hops, for the second hop charge fee in WETH.

Solution

There is an attempt for the enhancement in Snowfork#132 which could be problematic. Maybe it's better to add another extrinsic into pallet-xcm specific for this case as the fee modal here is somehow different from the current implementation.

Meanwhile, notice Parity team is also trying to improve the xcm fee, potential relevant issues/pull requests including:

#3847

#690

#3434

#3872

We'd suggest taking the use case here into consideration as well.

No response

Are you willing to help with this request?

Yes!

@yrong yrong added the I5-enhancement An additional feature request. label Apr 2, 2024
@acatangiu
Copy link
Contributor

IIUC you would like to add extra fee parameters to pallet-xcm extrinsics.
Are there parameters generic or are they bridge-specific?

For better understanding of the request, can you provide an example end-to-end use case and explain the missing pieces/current limitations?

Ideally please provide a proposal for this new extrinsic function signature, what are these new fees params, who calculates them to pass into pallet-xcm, and how is pallet-xcm supposed to use them to fix/support the described e2e use-case.


From the high-level understanding I have now, I believe we will not need new fee params once we have #690 and #3434. This way you can charge fees along the way from this new fee_register that can hold multiple assets.

@acatangiu acatangiu added the T6-XCM This PR/Issue is related to XCM. label Apr 2, 2024
@acatangiu acatangiu moved this from Draft to Backlog in Parity Roadmap Apr 2, 2024
@yrong
Copy link
Contributor Author

yrong commented Apr 2, 2024

Thanks for the information above and good to know that.

I'm not keen on adding a bridge-specific extrinsic, actually in this PR I attempted to just reuse the current reserve_transfer_assets with fee in WETH.

The main concern is to not force the user to have WETH in advance and able to pay all fees in DOT, potentially a swap should baked-in on top of the current one.

@acatangiu
Copy link
Contributor

I still think you should clearly explain here your e2e use-case and limitations encountered. You've referenced some PRs where you tried some solutions, but expecting others to understand the problem by auditing Snowbridge code will yield very little success :)

@yrong
Copy link
Contributor Author

yrong commented Apr 2, 2024

Sure, let me try to be more specific.

Just as #3847 (comment) describes the ideal extrinsic signature is:

pub fn transfer_assets(
    origin: OriginFor<T>,
    dest: Box<VersionedLocation>,
    beneficiary: Box<VersionedLocation>,
    assets: Box<VersionedAssets>,
    fees: Box<VersionedAssets>,
    weight_limit: WeightLimit,
) -> DispatchResult;

In my case param dest as Ethereum, assets as [WETH], fees as [DOT, WETH] with the first DOT to pay for the cost on BridgeHub and the second WETH to pay for the cost on Ethereum.

I realize my previous PR is not ideal or even correct, it will cause distraction so please just ignore it. Meanwhile, I added a new PR here Snowfork#135 which follows the pattern above.

This PR is only for demonstration and internal discussion. I don't want to pollute existing codes so add a separate extrinsic only for the Ethereum bridge, there are very few snowbridge specific codes involved so hope it's easy to understand now. The e2e test is:

cargo test -p bridge-hub-rococo-integration-tests --lib tests::snowbridge::send_weth_asset_from_asset_hub_to_ethereum  -- --nocapture

@acatangiu Let me know if anything more I can provide.

@acatangiu
Copy link
Contributor

Meanwhile, I added a new PR here Snowfork#135 which follows the pattern above.

Thank you! This clears it up significantly 👍

So what we really need is a way to be able to use different assets as fees for different hops along the way.

I am seeing similar issue when thinking about more complex transfers over the P<>K bridge. The problem is we can't (or shouldn't) add dedicated extrinsics to pallet-xcm for custom usecases.. E.g. the ethereum_transfer_extrinsic you suggested only works if called on AH, but we'd be adding it to all parachains using pallet-xcm. Same with any custom P<>K bridge scenario.

I will think about a more generic solution to both our problems.

@yrong
Copy link
Contributor Author

yrong commented Jul 8, 2024

@acatangiu Hey Adrian, any update here?

Seems the runtime api query_delivery_fees is already merged, which can be leverged to estimate the delivery cost for the export xcm to remote network like Ethereum.

So based on that is the next step to remove the remote portion from the runtime config BridgeHubEthereumBaseFee and make it an user input for the reserve_transfer_assets?

Please let me know WDYT and anything I can help.

@acatangiu
Copy link
Contributor

acatangiu commented Jul 9, 2024

Hey Ron, sorry for the delay in answering here.

The best way to achieve this and many other scenarios using a scalable solution is to use pallet_xcm::execute() directly providing your custom/complex asset transfer program(s).

I've also opened #4736 to create a tool for easier building of these complex/custom asset transfer scenarios.

In the meantime, once runtimes https://github.com/polkadot-fellows/runtimes/releases/tag/v1.2.8 are live, they will include:

Polkadot chains: allow arbitrary XCM execution (polkadot-fellows/runtimes#345)

So you can have the Snowbridge UI build custom XCM programs paying fees in various assets at each hop and directly execute it using pallet_xcm::execute.


Or you can use pallet_xcm::transfer_assets_using_type_and_then() (see emulated tests for examples) to initiate transfer to the first hop, while specifying using XCM what happens further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I5-enhancement An additional feature request. T6-XCM This PR/Issue is related to XCM.
Projects
Status: Done
Development

No branches or pull requests

2 participants