-
Notifications
You must be signed in to change notification settings - Fork 47
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
feat: better XCM fee management #528
Conversation
@@ -74,7 +75,7 @@ impl xcm_executor::Config for XcmConfig { | |||
type IsTeleporter = (); | |||
type MaxAssetsIntoHolding = ConstU32<64>; | |||
type MessageExporter = (); | |||
type OriginConverter = SiblingParachainAsNative<cumulus_pallet_xcm::Origin, RuntimeOrigin>; | |||
type OriginConverter = AccountIdJunctionAsParachain<ConstU32<2_000>, cumulus_pallet_xcm::Origin, RuntimeOrigin>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type OriginConverter = SovereignSignedViaLocation<(ForeignChainAliasAccount), RuntimeOrigin>;
What are the cons of using this type of configuration above?
Since DescentOrigin is modifying and extending the interior with the account. Can ForeignChainAliasAccount be used to convert the origin to a signed origin?
The consumer can filter out any calls and only allow the call function to be executed via XCM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pallet expects a parachain origin, hence using SovereignSignedViaLocation
won't work, unfrotunately.
pub UnitWeightCost: Weight = Weight::from_ref_time(1_000); | ||
pub UniversalLocation: InteriorMultiLocation = Parachain(ParachainInfo::parachain_id().into()).into(); | ||
} | ||
|
||
pub type Barrier = AllowTopLevelPaidExecutionFrom<Everything>; | ||
pub type AssetTransactorLocationConverter = SiblingParachainConvertsVia<Sibling, AccountId>; | ||
pub type Barrier = OkOrElseCheckForParachainProvider<AllowTopLevelPaidExecutionFrom<Nothing>, ConstU32<2_000>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be accomplished with this?
type Barrier = WithComputedOrigin<(AllowTopLevelPaidExecutionFrom<DerivativeCustomerLocations>);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed, the design at the time of this comment was flawed. I have now added a ExpectOrigin
instruction early on (d683a3f). For now, this is a sufficiently good approach, which means that the WithComputedOrigin
barrier would not let the call through, which is not what we want. Also the AllowTopLevelPaidExecutionFrom
would fail. There is no way around using the OkOrElseCheckForParachainProvider
barrier provided here. If some barrier must explicitly fail, the ErrOrElseCheckForParachainProvider
should be used.
Feature branch for everything DIP. It will collect other PRs until we are happy with the features, and will add the DIP to some of our runtimes and merge this into `develop`. ## WIP Checklist for the open tasks for v1 - [x] Basic structure -> #489 - [x] Merkleization of DID Documents -> #492 - [x] `RuntimeCall` verification logic -> #502 - [x] DID signature verification -> #516 - [x] Add support for linked accounts and web3name -> #525 - [x] Configurable origin for `commit_identity` -> #526 - [x] Proper fee management -> #528 - [x] Update to Polkadot 0.9.43 -> c18a6ce - [x] Replace XCM with state proofs -> #543 - [x] Add support for relaychain consumer -> #553 (part of #543) - [x] Proper error handling -> #572 - [x] Add support for versioning -> #573 - [x] Take deposits for identity commitments -> #574 - [x] Expose common definitions usable by consumers -> #577 - [x] Change ensure_signed! to configurable origin also for the `dispatch_as` function -> #577 - [x] Proper benchmarking and weights -> #585 - [x] Comments and docs -> #584 - [x] Revert Dockerfile changes in #587 - [x] [OPTIONAL] Add support for Zombienet -> #587 - [x] [OPTIONAL] Add chain spec loading from file for template runtimes -> #587 - [x] Big, final review -> #494 (review) - [x] Improvements n.1 PR -> #591 - [x] Improvements n.2 PR -> #592 - [x] Add to Peregrine runtime -> #594 - [ ] Deploy on Peregrine - [ ] Unit tests - [ ] Add to Spiritnet runtime - [ ] Deploy on Spiritnet - [ ] [OPTIONAL] Move DIP-related stuff into its own repo --------- Co-authored-by: Adel Golghalyani <48685760+Ad96el@users.noreply.github.com> Co-authored-by: Chris Chinchilla <chris@chrischinchilla.com> Co-authored-by: Albrecht <albrecht@kilt.io>
Feature branch for everything DIP. It will collect other PRs until we are happy with the features, and will add the DIP to some of our runtimes and merge this into `develop`. ## WIP Checklist for the open tasks for v1 - [x] Basic structure -> KILTprotocol/kilt-node#489 - [x] Merkleization of DID Documents -> KILTprotocol/kilt-node#492 - [x] `RuntimeCall` verification logic -> KILTprotocol/kilt-node#502 - [x] DID signature verification -> KILTprotocol/kilt-node#516 - [x] Add support for linked accounts and web3name -> KILTprotocol/kilt-node#525 - [x] Configurable origin for `commit_identity` -> KILTprotocol/kilt-node#526 - [x] Proper fee management -> KILTprotocol/kilt-node#528 - [x] Update to Polkadot 0.9.43 -> KILTprotocol/kilt-node@c18a6ce - [x] Replace XCM with state proofs -> KILTprotocol/kilt-node#543 - [x] Add support for relaychain consumer -> KILTprotocol/kilt-node#553 (part of KILTprotocol/kilt-node#543) - [x] Proper error handling -> KILTprotocol/kilt-node#572 - [x] Add support for versioning -> KILTprotocol/kilt-node#573 - [x] Take deposits for identity commitments -> KILTprotocol/kilt-node#574 - [x] Expose common definitions usable by consumers -> KILTprotocol/kilt-node#577 - [x] Change ensure_signed! to configurable origin also for the `dispatch_as` function -> KILTprotocol/kilt-node#577 - [x] Proper benchmarking and weights -> KILTprotocol/kilt-node#585 - [x] Comments and docs -> KILTprotocol/kilt-node#584 - [x] Revert Dockerfile changes in KILTprotocol/kilt-node#587 - [x] [OPTIONAL] Add support for Zombienet -> KILTprotocol/kilt-node#587 - [x] [OPTIONAL] Add chain spec loading from file for template runtimes -> KILTprotocol/kilt-node#587 - [x] Big, final review -> KILTprotocol/kilt-node#494 (review) - [x] Improvements n.1 PR -> KILTprotocol/kilt-node#591 - [x] Improvements n.2 PR -> KILTprotocol/kilt-node#592 - [x] Add to Peregrine runtime -> KILTprotocol/kilt-node#594 - [ ] Deploy on Peregrine - [ ] Unit tests - [ ] Add to Spiritnet runtime - [ ] Deploy on Spiritnet - [ ] [OPTIONAL] Move DIP-related stuff into its own repo --------- Co-authored-by: Adel Golghalyani <48685760+Ad96el@users.noreply.github.com> Co-authored-by: Chris Chinchilla <chris@chrischinchilla.com> Co-authored-by: Albrecht <albrecht@kilt.io>
Feature branch for everything DIP. It will collect other PRs until we are happy with the features, and will add the DIP to some of our runtimes and merge this into `develop`. ## WIP Checklist for the open tasks for v1 - [x] Basic structure -> #489 - [x] Merkleization of DID Documents -> #492 - [x] `RuntimeCall` verification logic -> #502 - [x] DID signature verification -> #516 - [x] Add support for linked accounts and web3name -> #525 - [x] Configurable origin for `commit_identity` -> #526 - [x] Proper fee management -> #528 - [x] Update to Polkadot 0.9.43 -> c18a6ce - [x] Replace XCM with state proofs -> #543 - [x] Add support for relaychain consumer -> #553 (part of #543) - [x] Proper error handling -> #572 - [x] Add support for versioning -> #573 - [x] Take deposits for identity commitments -> #574 - [x] Expose common definitions usable by consumers -> #577 - [x] Change ensure_signed! to configurable origin also for the `dispatch_as` function -> #577 - [x] Proper benchmarking and weights -> #585 - [x] Comments and docs -> #584 - [x] Revert Dockerfile changes in #587 - [x] [OPTIONAL] Add support for Zombienet -> #587 - [x] [OPTIONAL] Add chain spec loading from file for template runtimes -> #587 - [x] Big, final review -> #494 (review) - [x] Improvements n.1 PR -> #591 - [x] Improvements n.2 PR -> #592 - [x] Add to Peregrine runtime -> #594 - [ ] Deploy on Peregrine - [ ] Unit tests - [ ] Add to Spiritnet runtime - [ ] Deploy on Spiritnet - [ ] [OPTIONAL] Move DIP-related stuff into its own repo --------- Co-authored-by: Adel Golghalyani <48685760+Ad96el@users.noreply.github.com> Co-authored-by: Chris Chinchilla <chris@chrischinchilla.com> Co-authored-by: Albrecht <albrecht@kilt.io>
Feature branch for everything DIP. It will collect other PRs until we are happy with the features, and will add the DIP to some of our runtimes and merge this into `develop`. ## WIP Checklist for the open tasks for v1 - [x] Basic structure -> #489 - [x] Merkleization of DID Documents -> #492 - [x] `RuntimeCall` verification logic -> #502 - [x] DID signature verification -> #516 - [x] Add support for linked accounts and web3name -> #525 - [x] Configurable origin for `commit_identity` -> #526 - [x] Proper fee management -> #528 - [x] Update to Polkadot 0.9.43 -> c18a6ce - [x] Replace XCM with state proofs -> #543 - [x] Add support for relaychain consumer -> #553 (part of #543) - [x] Proper error handling -> #572 - [x] Add support for versioning -> #573 - [x] Take deposits for identity commitments -> #574 - [x] Expose common definitions usable by consumers -> #577 - [x] Change ensure_signed! to configurable origin also for the `dispatch_as` function -> #577 - [x] Proper benchmarking and weights -> #585 - [x] Comments and docs -> #584 - [x] Revert Dockerfile changes in #587 - [x] [OPTIONAL] Add support for Zombienet -> #587 - [x] [OPTIONAL] Add chain spec loading from file for template runtimes -> #587 - [x] Big, final review -> #494 (review) - [x] Improvements n.1 PR -> #591 - [x] Improvements n.2 PR -> #592 - [x] Add to Peregrine runtime -> #594 - [ ] Deploy on Peregrine - [ ] Unit tests - [ ] Add to Spiritnet runtime - [ ] Deploy on Spiritnet - [ ] [OPTIONAL] Move DIP-related stuff into its own repo --------- Co-authored-by: Adel Golghalyani <48685760+Ad96el@users.noreply.github.com> Co-authored-by: Chris Chinchilla <chris@chrischinchilla.com> Co-authored-by: Albrecht <albrecht@kilt.io>
Fixes https://github.com/KILTprotocol/ticket/issues/2566.
This PR changes the way XCM fees are handled for DIP. Before, the provider parachain sovereign account on the consumer chain was paying for the XCM fees, while now it is the sovereign account of the user that triggered the operation on the provider chain, to also pay for the fees on the receiver chain in whatever asset specified in the extrinsic. With XCM v3, it's not possible for a parachain to charge an interior location (e.g., with a
DescendOrigin
instruction), hence a new barrier and a new origin converter have been introduced, which are supposed to be used together to implement the feeing model DIP has envisioned.Relevant changes
Reworked
XcmRouterIdentityDispatcher
The
XcmRouterIdentityDispatcher
type used to be part of thepallet-dip-provider
pallet. Because it is really specific to KILT, it has been moved into thekilt-dip-support
crate.Additionally, the router now packages the DIP XCM program with the following instructions:
ExpectOrigin
with the provider parachain origin, making sure the same XCM message cannot be sent by regular users via the XCM pallet.DescendOrigin
with a newAccountId32
representing the dispatcher account ID on the provider chain.WithdrawAsset
with theMultiAsset
specified in thecommit_identity
extrinsic in thepallet-dip-provider
palletBuyExecution
with the maxWeight
specified in thecommit_identity
extrinsic in thepallet-dip-provider
palletTransact
withOriginKind::Native
and the same weight limit as aboveRefundSurplus
DepositAsset
with any remaining assets in the holding registry back to the dispatcher's account on the consumer chainIn the demo runtimes and tests, the
Account32Hash
location converter has been used to convert the account ID information coming from theX2
junction containing the sibling provider parachain and the disptacher account.Configurable origin for the
pallet_dip_provider::commit_identity
extrinsicThe
CommitOrigin
origin was already configurable, but now itsSuccess
result is required to implement theSubmitterInfo
trait, which returns theAccountId
of the tx submitter, so that the information can be used to compose the XCM message that will be sent to destination (specifically it is included in theDescendOrigin
andDepositAsset
instructions).The trait has been implemented for the
AccountId32
type, which is returned by theEnsureSigned
origin check (which could be used for tests), and for theDidRawOrigin
type, which is returned by theEnsureDidOrigin
origin check (which has been used in the demo runtimes).The
AllowParachainProviderAsSubaccount
barrierFor DIP to implement the feeing model devised here, where the user pays for the fees on both chain, a new barrier + origin converter must be integrated into the XCM configuration.
The
AllowParachainProviderAsSubaccount
barrier verifies that 1. the origin of the XCM is a parachain with an ID that is returned by theProviderParaId
generic type, and 2. the order of the instructions in the XCM program matches exactly what is expected in the DIP flow. If either the origin or the set of instructions is different, the barrier will return an error.To extend existing barrier, there are two additional types:
OkOrElseCheckForParachainProvider
which tries to match the incoming XCM message against the newAllowParachainProviderAsSubaccount
barrier if none of the previous barriers have succeeded (explicit approval), andErrOrElseCheckForParachainProvider
which tries to match the XCM message against the new barrier only if none of the previous barriers have failed (explicit rejection).The
AccountIdJunctionAsParachain
origin converterSince there is no way to convert an
X2
junction of parachain + sub-account, theAccountIdJunctionAsParachain
does just that. It takes theX2
junction of a sibling parachain + sub-account, if the parachain matches the ID given by theProviderParaId
type, and converts it into aParachainOrigin
.The origin converter and the barrier are meant to be used together, otherwise configuring only one or the other opens the way to security issues on the consumer chain!