Inbound queue v2 #3

claravanstaden · 2024-11-05T11:46:44Z

…ue-v2

claravanstaden · 2024-11-07T09:19:19Z

@yrong would appreciate an early review here. 🙏🏻

yrong · 2024-11-08T10:02:16Z

bridges/snowbridge/primitives/router/src/inbound/v2.rs

+		let fee_value = 1_000_000_000u128; // TODO needs to be dry-run to get the fee but also
+									 // need to add a fee here for the dry run... Chicken/egg problem?
+		let fee: Asset = (fee_asset, fee_value).into();


IIUC the dry_run on BH is to convert the InboundMessage to a XCM. Then UI can dry run that XCM on AH for the cost.

So IMO InboundMessage should contain the fee like V1 before, it can be zero for estimation, after that filled in with the result for calling the Gateway contract from UX.

@yrong do you think the AH execution fee (which can be 0 for estimation) should be added as a field in MessageV2?

@vgeddes or how did you imagine the AH execution fee be specified? I see there is a fee added for register token (which is the foreign asset creation deposit) but no other place in the contracts to specify the AH execution fee. Iiuc this will be the flow:

Inbound queue runtime API to convert command to XCM

UX takes XCM and dry-runs the XCM on AH to get fee

UX takes fee number and adds it to the command, emits OutboundEvent

I don't see such a fee parameter in the contract code, so just wanna get your thoughts on this.

Another option as mentioned in #3 (comment)

bridges/snowbridge/primitives/router/src/inbound/dry_run.rs

bridges/snowbridge/pallets/inbound-queue-v2/src/lib.rs

vgeddes · 2024-11-20T10:58:05Z

bridges/snowbridge/primitives/router/src/inbound/v2.rs


-const MINIMUM_DEPOSIT: u128 = 1;
+const LOG_TARGET: &str = "snowbridge-router-primitives";

 /// Messages from Ethereum are versioned. This is because in future,
 /// we may want to evolve the protocol so that the ethereum side sends XCM messages directly.
 /// Instead having BridgeHub transcode the messages into XCM.
 #[derive(Clone, Encode, Decode, RuntimeDebug)]
 pub enum VersionedMessage {


This versioned type is unused in V2, in the sense that the ethereum contracts do not send it.

@vgeddes I wonder why we don't version the message this time around, I made a comment about it here too: yrong#4 (comment)

bridges/snowbridge/primitives/router/src/inbound/v2.rs

vgeddes · 2024-11-20T11:10:29Z

bridges/snowbridge/primitives/router/src/inbound/v2.rs


+		let fee_asset = Location::new(1, Here);
+		let fee_value = 1_000_000_000u128; // TODO get from command


As I noted in a previous comment, this fee should also be burnt from the relayers account on BH.

Some other notes:

Should make the fee configurable via runtime config, ie add T::XcmPrologueFee: Balance pallet config

Add to your TODOs to implement an XCM simulation test that calculates an appropriate minimum fee, assuming the worst case where a maximum of eight assets are supplied. Then we add 2x buffer on top of that minimum.

Should make the fee configurable via runtime config, ie add T::XcmPrologueFee: Balance pallet config

Can we make this fee configurable? Iirc we cannot, because it could include arbitrary instructions such as Transact. I explained the flow as I see it should work over here: #3 (comment)

Can we make this fee configurable? Iirc we cannot, because it could include arbitrary instructions such as Transact. I explained the flow as I see it should work over here: #3 (comment)

Yeah, I still think InboundMessage should contain the fee like V1 before with AssetId including.

But there is also another option to make it configurable, A Nmap storage item on BH something like:

Destination Operation FeeId FeeAmount

AH TransferENA WETH 0.0001

AH TransferENA DOT 0.01

AH TransferPNA DOT 0.01

Moonbeam TransferENA GLMR 0.1

Moonbeam Call_Contract_Method_1 GLMR 0.5

Moonbeam Call_Contract_Method_2 GLMR 10

Destination chain can config multiple assets as fee. e.g. for TransferENA to AH both WETH and DOT can be configured as fee.

For multiple hop operations the fee is total of the execution cost on AH and on destination chain. e.g. for TransferENA to Moonbeam the fee is 0.0001 WETH + 0.1 GLMR

Operations with Destination to AH should be configured with governance call from Polkadot

Operations with Destination to Moonbeam should be configured with governance call from Moonbeam

IMO this is doer-able because

fee on Substrate is very limited and invariable, comparing with the fee on Ethereum side, so dry_run every time may not be necessary

we can add SetAppendix(DepositToBeneficiary) and return the extra fee to end user, so set a high fee safe enough is not a big deal

That's an interesting idea @yrong. It could work. I think the main downside to this (from my perspective) is that many governance calls will be a pain to do. It will require more invention from our team's side, more coordination with other teams.

What is your concern with the dry-run approach? Isn't runtime API calls free so there's no cost to doing the estimation?

I still think InboundMessage should contain the fee like V1 before but with AssetId including.

Yeah, that's why I still prefer this.

@vgeddes let me know what you think too.

I think there's a misunderstanding here how fees are supposed to be paid on AH. Please read again the API docs for IGateway.sendMessage:

// The `xcm` should contain the necessary instructions to: // 1. Pay XCM execution fees for `xcm`, either from assets in holding, // or from the sovereign account of `msg.sender`. // 2. Handle the assets in holding, either depositing them into // some account, or forwarding them to another destination.

Or in other words, its the users responsibility to provide assets and xcm instructions to cover fees. I.e the user-provided xcm can contain PayFee instructions.

Please let me know if there is anything that I should clarify about this.

Its only the fixed xcm prologue that requires a small fixed fee to be teleported by the relayer account on BH. To cover the first few instructions which are prepending to the users xcm.

Our governance messages for registering ENA tokens will use the same API.

I see, thanks for clearing it up @vgeddes. I will add it.

Co-authored-by: Vincent Geddes <117534+vgeddes@users.noreply.github.com>

…ueue-v2

claravanstaden · 2024-11-21T08:15:13Z

Closing in favour of #4. The diff in this PR included changes already made in another PR, so it was hard to see the true diff.

yrong and others added 9 commits October 25, 2024 00:12

Rebase inbound queue

0e9cca0

Merge branch 'outbound-queue-v2' into inbound-queue-v2

8f59cc4

Merge branch 'outbound-queue-v2' into inbound-queue-v2

bd02251

Merge branch 'outbound-queue-v2' into inbound-queue-v2

9e39dca

inbound v2

cf8f536

send token unit test

3e9b03d

xcm decoding

5b92bd0

adds more tests

41170b3

Merge remote-tracking branch 'ron/outbound-queue-v2' into inbound-que…

2250d9b

…ue-v2

claravanstaden mentioned this pull request Nov 5, 2024

Inbound queue v2 yrong/polkadot-sdk#6

Closed

claravanstaden added 4 commits November 5, 2024 14:46

converter

d81affc

inbound progress

1638660

tests

716404b

tests

89a157d

claravanstaden added 3 commits November 7, 2024 11:25

fix alias origin

e814b29

cleanup runtime interface

697b51c

dry run xcm

b20b728

yrong reviewed Nov 8, 2024

View reviewed changes

bridges/snowbridge/primitives/router/src/inbound/dry_run.rs Outdated Show resolved Hide resolved

yrong mentioned this pull request Nov 17, 2024

Inbound queue v2 yrong/polkadot-sdk#5

Closed

cleanup

a0fe8de

claravanstaden changed the base branch from outbound-queue-v2 to snowbridge-v2 November 18, 2024 07:39

claravanstaden changed the base branch from snowbridge-v2 to outbound-queue-v2 November 18, 2024 07:40

claravanstaden added 2 commits November 18, 2024 10:24

remove unnecessary dry-run logic

490d091

cleanup

bf500d2

claravanstaden commented Nov 18, 2024

View reviewed changes

bridges/snowbridge/pallets/inbound-queue-v2/src/lib.rs Show resolved Hide resolved

add foreign erc20 token id lookup

cb9111f

vgeddes reviewed Nov 20, 2024

View reviewed changes

claravanstaden marked this pull request as ready for review November 20, 2024 11:14

claravanstaden and others added 3 commits November 20, 2024 13:23

Update bridges/snowbridge/primitives/router/src/inbound/v2.rs

8372cae

Co-authored-by: Vincent Geddes <117534+vgeddes@users.noreply.github.com>

Update bridges/snowbridge/pallets/inbound-queue-v2/src/lib.rs

767b0ef

Co-authored-by: Vincent Geddes <117534+vgeddes@users.noreply.github.com>

Update bridges/snowbridge/pallets/inbound-queue-v2/src/lib.rs

324f0c0

Co-authored-by: Vincent Geddes <117534+vgeddes@users.noreply.github.com>

claravanstaden changed the base branch from outbound-queue-v2 to snowbridge-v2 November 20, 2024 13:05

claravanstaden added 2 commits November 20, 2024 15:07

rename InboundAsset to Asset

b9a4e17

Merge remote-tracking branch 'origin/inbound-queue-v2' into inbound-q…

47fd268

…ueue-v2

claravanstaden changed the base branch from snowbridge-v2 to outbound-queue-v2 November 21, 2024 07:32

claravanstaden closed this Nov 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inbound queue v2 #3

Inbound queue v2 #3

claravanstaden commented Nov 5, 2024

claravanstaden commented Nov 7, 2024

yrong Nov 8, 2024 •

edited

Loading

claravanstaden Nov 18, 2024

claravanstaden Nov 18, 2024

yrong Nov 21, 2024

vgeddes Nov 20, 2024

claravanstaden Nov 20, 2024

vgeddes Nov 20, 2024

claravanstaden Nov 20, 2024

yrong Nov 21, 2024 •

edited

Loading

claravanstaden Nov 21, 2024

yrong Nov 21, 2024

claravanstaden Nov 21, 2024

vgeddes Nov 21, 2024 •

edited

Loading

claravanstaden Nov 22, 2024

claravanstaden commented Nov 21, 2024


		let fee_asset = Location::new(1, Here);
		let fee_value = 1_000_000_000u128; // TODO get from command

Destination	Operation	FeeId	FeeAmount
AH	TransferENA	WETH	0.0001
AH	TransferENA	DOT	0.01
AH	TransferPNA	DOT	0.01
Moonbeam	TransferENA	GLMR	0.1
Moonbeam	Call_Contract_Method_1	GLMR	0.5
Moonbeam	Call_Contract_Method_2	GLMR	10

Inbound queue v2 #3

Inbound queue v2 #3

Conversation

claravanstaden commented Nov 5, 2024

claravanstaden commented Nov 7, 2024

yrong Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yrong Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vgeddes Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

claravanstaden commented Nov 21, 2024

yrong Nov 8, 2024 •

edited

Loading

yrong Nov 21, 2024 •

edited

Loading

vgeddes Nov 21, 2024 •

edited

Loading