Reserve transfer assets to Ethereum with weth as fee & UnpaidExecution #135
Conversation
| _ => None, | ||
| } | ||
| .ok_or(Error::InvalidAssetUnsupportedReserve)?; | ||
| remote.inner_mut().push(BurnAsset(fee_on_bridgehub.clone().into())); |
There was a problem hiding this comment.
Add BurnAsset into remote_xcm so that we can verify against the local cost on BH.
There was a problem hiding this comment.
Can you explain more what this BurnAsset is for?
the message arriving and executed on BridgeHub is:
Xcm(vec![
WithdrawAsset(Assets::from(vec![fee.clone()])),
BuyExecution { fees: fee, weight_limit: Unlimited },
ExportMessage {
network: Ethereum { chain_id: ethereum_chain_id },
destination: Here,
xcm: inner_xcm,
},
]);Where fee in BuyExecution { fees: fee, weight_limit: Unlimited } is DOT, no?
But this ^ is just for paying for ExportMessage which probably doesn't include relayer/ethereum tx cost, yes?
The inner_xcm is what pallet-xcm builds. And it uses WETH in inner BuyExecution? This WETH is practically the price for relayer/ethereum tx, no?
Why do you also want BurnAsset(fee_on_bridgehub) in the inner message? What does this fee_on_bridgehub (in DOT) represent?
There was a problem hiding this comment.
I've made XcmRouter on AH to use UnpaidRemoteExporter for bridging to Ethereum so there is no BuyExecution outside. Be more specific, the xcm arrives to BH(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 then you'll see it) is:
Xcm(vec![
UnpaidExecution,
ExportMessage {
network: Ethereum { chain_id: ethereum_chain_id },
destination: Here,
xcm: Xcm(vec![
WithdrawAsset(WETH,weth_amount),
ClearOrigin,
BuyExecution { fees: (WETH,weth_fee_amount)}, weight_limit: Unlimited },
DepositAsset { assets: Wild(AllCounted(1))} },
BurnAsset(Assets([Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(dot_fee_amount) }])),
SetTopic(),
},
]);
For the inner xcm it's not instructions to execute on BH, instead just to be verified in our outbound router and generate Ethereum commands accordingly.
So it's the right place to verify the fees passed from source chain(i.e. AH) as expected. Some todos added in https://github.com/Snowfork/polkadot-sdk/pull/135/files#diff-0bd5b75b88aee6992e3ddba0de8bd08d2ab07ecd38fc745c7d76daff56bba525 for that.
As you may notice the inner_xcm already contains a BuyExecution with the fee in WETH for the remote cost on Ethereum, we still need another instruction that contains the fee in DOT for the local cost on BH, that's why I added the BurnAsset(fee_on_bridgehub). With the two fees in place we can check them against the cost calculated in
It seems a bit tricky and I expect a more elegant way to construct the two fees here.
| let (ticket, _) = validate_send::<T::XcmRouter>(dest.clone(), remote_xcm.clone()) | ||
| .map_err(Error::<T>::from)?; | ||
| if origin != Here.into_location() { | ||
| Self::charge_fees(origin.clone(), fees).map_err(|error| { | ||
| log::error!( | ||
| target: "xcm::pallet_xcm::execute_xcm_transfer", | ||
| "Unable to charge fee with error {:?}", error | ||
| ); | ||
| Error::<T>::FeesNotMet |
There was a problem hiding this comment.
charge_fees based on the input fee params.
There was a problem hiding this comment.
this just allows ppl to put nothing here and get away without paying for fees - why not continue to use price coming from
let (ticket, price) = validate_send::<T::XcmRouter>(dest.clone(), remote_xcm.clone()).map_err(Error::<T>::from)?;There was a problem hiding this comment.
The price here does not make sense anymore and we charge the fees based on the input parameters directly.
As demonstrated above, assuming the fees are not correct, though the xcm can be sent to BH, it won't pass the check in the outbound router.
There was a problem hiding this comment.
Sorry, the price here is the sending cost on the source chain which should be charged. Just made a fix in 2a64631
| Some(( | ||
| XcmBridgeHubRouterFeeAssetId::get(), | ||
| BridgeHubEthereumBaseFee::get(), | ||
| ).into()) |
There was a problem hiding this comment.
Remove the BridgeHubEthereumBaseFee config which is variable/unstable and hard to maintain on chain.
yrong
force-pushed
the
ron/reserve_transfer_with_multiple_fees
branch
from
e2b528b to
5197b49
Compare
yrong
changed the title
There was a problem hiding this comment.
The todos to verify the two fees(i.e. fee in DOT for the cost on BH and fee in WETH for the cost on Ethereum) passed from AH as expected.
yrong
changed the title
| pub fn reserve_transfer_ethereum_assets( | ||
| origin: OriginFor<T>, | ||
| dest: Box<VersionedLocation>, | ||
| beneficiary: Box<VersionedLocation>, | ||
| assets: Box<VersionedAssets>, | ||
| fees: Box<VersionedAssets>, | ||
| ) -> DispatchResult { |
There was a problem hiding this comment.
A separate extrinsic for:
- transfer Polkadot native assets to Ethereum
- transfer Ethereum native assets back to Ethereum
yrong
changed the title
|
Closed for paritytech#3931 (comment) |
Only for demonstration for the comment paritytech#3931 (comment)