Use pallet instance in inbound queue origin #1033
Conversation
| @@ -8,6 +8,8 @@ | |||
| use frame_support::parameter_types; | |||
| use xcm::opaque::lts::NetworkId; | |||
|
|
|||
| pub const INBOUND_QUEUE_MESSAGES_PALLET_INDEX: u8 = 80; | |||
There was a problem hiding this comment.
Looks like there is an API for retrieving a pallet's index: https://substrate.stackexchange.com/a/794
There was a problem hiding this comment.
This works nicely in BridgeHub, but I need the EthereumInboundQueue pallet index on BridgeHub in the AssetHub runtime too: https://github.com/Snowfork/polkadot-sdk/pull/51/files#diff-a86375df98e04ca3cce1ea35c40257a222e2d5087f5f528ff33307678b78dc2dR863
I don't think it would be correct to import bridge hub as a dependency in the AssetHub crate, and I don't know how else to share the pallet index.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1033 +/- ##
==========================================
+ Coverage 81.50% 81.53% +0.03%
==========================================
Files 54 54
Lines 2233 2237 +4
Branches 70 70
==========================================
+ Hits 1820 1824 +4
Misses 398 398
Partials 15 15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -165,6 +185,7 @@ where | |||
| BuyExecution { fees: xcm_fee, weight_limit: Unlimited }, | |||
| // Fund the snowbridge sovereign with the required deposit for creation. | |||
| DepositAsset { assets: Definite(deposit.into()), beneficiary: bridge_location }, | |||
| DescendOrigin(X1(PalletInstance(inbound_queue_pallet_index))), | |||
There was a problem hiding this comment.
Shouldn't this DescendOrigin be placed at the start of message, before BuyExecution?
As otherwise, we have three origins in this message, which makes it more complicated.
There was a problem hiding this comment.
This doesn't work, putting the DescendOrigin before BuyExecution then results in a barrier error. The way I understand it, the DescendOrigin shouldn't be before BuyExecution because it's not really related to the BuyExecution. BuyExecution is to pay for fees in DOT, and then the DescendOrigin is to further specialize the origin to be BridgeHub/Pallet(EthereumInboundQueue). @alistair-singh correct me if I am wrong.
There was a problem hiding this comment.
@claravanstaden I agree with @vgeddes here, IMO it's better to move DescendOrigin to first, add another PR with some revamp accordingly in
https://github.com/Snowfork/snowbridge/pull/1034/files#diff-9218e396410f01117c1c305b9cf006f6498bb0b3c0aedb8952a8450b4b1c5715 for us to review.
There was a problem hiding this comment.
Awesome! Thanks Ron, taking a look...
There was a problem hiding this comment.
Thanks @yrong, I merged it.
Change to accomodate this requested change: paritytech/polkadot-sdk#2522 (comment) The EthereumInboundQueue pallet ID should form part of the origin to AssetHub.
Companion PR: Snowfork/polkadot-sdk#51