Fee for Gateway.sendToken should cover all costs

[vgeddes]

For token transfers from Ethereum, if the final destination of the transfer is not AssetHub, we need to charge double the sendToken fee to cover cost of the extra XCM message to the final destination.

I've also improved the API that allows users to query fees for registering and sending tokens. The APIs give more detailed fee info, and in the case of SendToken, accounts for special case above.

requires: Snowfork/polkadot-sdk#49

Fixes: SNO-761

[codecov]

Codecov Report

Attention: 33 lines in your changes are missing coverage. Please review.

Comparison is base (0120c60) 80.97% compared to head (770c965) 81.50%.
Report is 2 commits behind head on main.

Files	Patch %	Lines
parachain/primitives/router/src/inbound/mod.rs	25.00%	18 Missing ⚠️
parachain/pallets/control/src/weights.rs	0.00%	4 Missing ⚠️
parachain/pallets/inbound-queue/src/lib.rs	72.72%	3 Missing ⚠️
parachain/primitives/core/src/pricing.rs	80.00%	3 Missing ⚠️
contracts/src/Gateway.sol	96.07%	2 Missing ⚠️
parachain/primitives/core/src/outbound.rs	88.88%	2 Missing ⚠️
parachain/pallets/control/src/lib.rs	95.83%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1015      +/-   ##
==========================================
+ Coverage   80.97%   81.50%   +0.52%     
==========================================
  Files          52       54       +2     
  Lines        2150     2233      +83     
  Branches       67       70       +3     
==========================================
+ Hits         1741     1820      +79     
- Misses        394      398       +4     
  Partials       15       15              

Flag	Coverage Δ
rust	81.45% <75.39%> (+0.40%)	⬆️
solidity	81.72% <97.10%> (+1.15%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yrong]

For token transfers from Ethereum, if the final destination of the transfer is not AssetHub, we need to charge double the sendToken fee to cover cost of the extra XCM message to the final destination.

The estimation to double the cost may not be accurate, the xcm fee totally depends on respective xcm config/impl in a specific parachain and may not be same as(or even differs a lot from) assetHub.

E.g. Like for acala the trader they use to calculate the fee are totally different from trader in assetHub and they don't even use the asset pallet for MultiCurrency support.

[alistair-singh]

Minor suggestion. But looks good!

[alistair-singh]

Suggestion: Why not call sendTokenFee(ParaID assetHubParaID, ParaID destinationChain) here? Is it because we are worried about gas costs because its external? I think we should have an internal implementation and use it for both here and in sendTokenFee(ParaID assetHubParaID, ParaID destinationChain) so that the fee can be tuned in one place.

[vgeddes]

yeah mostly worried about gas costs.

Gonna have to redo this code though, see my latest comment further below.

[vgeddes]

That's a good point Ron. And we will have the same issue for Xcm::Transact. I understand now that we need to implement something like this:

1. The sendToken function takes an destinationChainFee parameter, which is the amount of DOT it will cost to execute the XCM.
2. When called, the destinationChainFee is converted to ETH using a DOT/ETH exchange rate
3. This converted amount is transferred from the user to the agent contract for AssetHub
4. We need to include destinationChainFee amount in the message sent to BridgeHub, so that the XCM messages can have the correct BuyExecution instructions.

I think we can reasonably expect that parachains should accept DOT as the fee currency. This simplifies things. No need for custom fee tokens.

There are other changes to make if we go with this approach. For example, Channel.outboundFee should be in DOT, not ETH. Then we convert to ETH using the exchange rate when sendToken or registerToken is called.

Potential issues: The currency conversion with a lagging & mostly static exchange rate would lead to either of the following scenarios:

Scenario 1:

1. Suppose actual XR is 1ETH/100DOT, however Gateway is configured with an XR of 1ETH/50DOT.
2. Then user calls sendToken, paying a fee of 1 ETH for a XCM fee of 50 DOT.
3. User ends up overpaying ETH to AssetHub's agent, as DOT is worth less in reality.
4. Bridge security & operations are maintained, however users are potentially unhappy.

Scenario 2:

1. Suppose actual XR is 1ETH/100DOT, however Gateway is configured with an XR of 1ETH/200DOT.
2. Then user calls sendToken, paying a fee of 1 ETH for a XCM fee of 200 DOT.
3. User ends up underpaying ETH to AssetHub's agent, as DOT is worth more in reality.
4. Bridge security & operations are maintained. however, in the longer run, AssetHub may end up short of funds to pay for rewards on BridgeHub.

[yrong]

The sendToken function takes a destinationChainFee parameter, which is the amount of DOT it will cost to execute the XCM.

Or could be loaded from some map storage something like
(Multilocation(Acala)->0.01DOT),(MultiLocation(Moonbeam)->0.02DOT)
), That storage can be configured from the control pallet.

When called, the destinationChainFee is converted to ETH using a DOT/ETH exchange rate

Since we've already introduced the exchange rate configurable in

snowbridge/parachain/pallets/outbound-queue/src/lib.rs

Line 242 in eb13b90

exchange_rate: FixedU128::saturating_from_rational(1, 400),

, so maybe we can reuse it on Ethereum side with another control command set bothboth at the same time.

As you mentioned the main issue is the outdated price. So we may need to introduce some price oracle like Chainlink Data Feeds or remember as @musnit mentioned before with an on-chain swap.

[vgeddes]

The sendToken function takes a destinationChainFee parameter, which is the amount of DOT it will cost to execute the XCM.

Or could be loaded from some map storage something like (Multilocation(Acala)->0.01DOT),(MultiLocation(Moonbeam)->0.02DOT), That storage can be configured from the control pallet.

Well the idea is that indeed there need to be some mapping of parachain to fees. But its much simpler for clients to maintain that mapping offchain, and pass the required destinationChainFee to our API.

When called, the destinationChainFee is converted to ETH using a DOT/ETH exchange rate

Since we've already introduced the exchange rate configurable in

snowbridge/parachain/pallets/outbound-queue/src/lib.rs

Line 242 in eb13b90

exchange_rate: FixedU128::saturating_from_rational(1, 400),

, so maybe we can reuse it on Ethereum side with another control command set both at the same time.

Agree 👍

I'm implementing that in this PR.

As you mentioned the main issue is the outdated price. So we may need to introduce some price oracle like Chainlink Data Feeds or remember as @musnit mentioned before with an on-chain swap.

I think a outdated exchange rate is OK for our bridge, for various reasons:

1. The exchange rate is only used to calculate fees, which are generally much less than the actual value of the tokens being transferred.
2. At least for the past few months the ETH/DOT rate has been quite stable around 1/400.
3. We can always upgrade the bridge to implement a better mechanism (such as on-chain swaps)

The issue with all Chainlink services is that they are all centralized. They are not actually decentralised and trustless, since they are governed and maintained by a single company.

[yrong]

snowbridge/parachain/pallets/control/src/lib.rs

Line 339 in 2fcc415

outbound_fee: u128,

Actually this param can be removed now, should be exactly the same as(cover) the cost in

snowbridge/parachain/pallets/inbound-queue/src/lib.rs

Line 256 in 2fcc415

refund.saturating_add(T::Reward::get()),

[alistair-singh]

Scenario 1:

1. Suppose actual XR is 1ETH/100DOT, however Gateway is configured with an XR of 1ETH/50DOT.
2. Then user calls sendToken, paying a fee of 1 ETH for a XCM fee of 50 DOT.
3. User ends up overpaying ETH to AssetHub's agent, as DOT is worth less in reality.
4. Bridge security & operations are maintained, however users are potentially unhappy.

Scenario 2:

1. Suppose actual XR is 1ETH/100DOT, however Gateway is configured with an XR of 1ETH/200DOT.
2. Then user calls sendToken, paying a fee of 1 ETH for a XCM fee of 200 DOT.
3. User ends up underpaying ETH to AssetHub's agent, as DOT is worth more in reality.
4. Bridge security & operations are maintained. however, in the longer run, AssetHub may end up short of funds to pay for rewards on BridgeHub.

Both these scenarios apply to our conversion rate on the substrate side as well. So we may want one extrinsic so that governance can upgrade the conversion rates on both sides of the bridge, maybe from the control pallet?