-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Remove Default
bound on AccountId
types under the xcm directory
#4712
Remove Default
bound on AccountId
types under the xcm directory
#4712
Conversation
Ok(AccountId::decode(&mut TrailingZeroInput::zeroes()) | ||
.expect("infinite length input; no invalid inputs for type; qed")) |
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.
For what is this account being used?
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.
It looks like it is only used in testing, in cases where the test wants to convert a message originating from a parent location to just the zero account, i.e. a "don't care but signed" account.
As such, there's not really a huge issue leaving it as ParentIsDefault
, other than to keep in line with the Default
bound removal on AccountId
.
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.
I think we should remove this trait from the public api and make it test only.
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.
Given we want to avoid the default / zero accounts, should we make it ParentIs<AccountId, A: Get<AccountId>>
and use something else for parachain runtime configuration.
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.
Honestly I don't even know why the default was being used here. I had assumed that default account == zero account, but it doesn't seem to be the case for me anymore via reading the comments. Is there somehow a default non-zero account associated with AccountId
s in statemint/e?
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.
I think default account is always zero account.
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.
Yeah Default == Zero
.
I'm also for removing this or doing it like @xlc proposed.
@KiChjang what is the status on this? I need this for my Substrate pr that removes the |
@bkchr I'll need to figure out what the parent accounts are for Statemint/e, currently in the test networks I still have them set to the zero account, and it is definitely not going to be the zero account on production networks. |
@KiChjang you could use something like |
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
bot merge |
* Rename ParentIsDefault to ParentIsAllZeroes * Fixes * Create ParentAccounts for respective networks * Fixes * Use b"Parent" as the basis for generating parent AccountId * Fixes * Use preset parent account ID * update lockfile for {"polkadot"} Co-authored-by: parity-processbot <>
This reverts commit 9c977d6.
This reverts commit 9c977d6.
This reverts commit 9c977d6.
* Revert "Companion for substrate#10632 (#895)" This reverts commit fd14576. * Revert "Companion for paritytech/polkadot#4712 (#901)" This reverts commit 9c977d6. * Prepare branch * Make sure to use `state_version = 0` for now * Fix lock file * Minimising changes to cargo lock * updating to include weights update. Co-authored-by: Bastian Köcher <info@kchr.de>
…aritytech#4712) * Refactor ParentIsDefault to ParentIsAllZeroes * Remove Default bound on all AccountId types under the xcm directory * Change to ParentIs<A: Get<AccountId>, AccountId> * Provide a better account for ParentIs * Fixes * Fixes * Fixes * Fixes * Update xcm/xcm-builder/src/currency_adapter.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Use preset account ID value for parent MultiLocations Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
* Rename ParentIsDefault to ParentIsAllZeroes * Fixes * Create ParentAccounts for respective networks * Fixes * Use b"Parent" as the basis for generating parent AccountId * Fixes * Use preset parent account ID * update lockfile for {"polkadot"} Co-authored-by: parity-processbot <>
* Rename ParentIsDefault to ParentIsAllZeroes * Fixes * Create ParentAccounts for respective networks * Fixes * Use b"Parent" as the basis for generating parent AccountId * Fixes * Use preset parent account ID * update lockfile for {"polkadot"} Co-authored-by: parity-processbot <>
* Rename ParentIsDefault to ParentIsAllZeroes * Fixes * Create ParentAccounts for respective networks * Fixes * Use b"Parent" as the basis for generating parent AccountId * Fixes * Use preset parent account ID * update lockfile for {"polkadot"} Co-authored-by: parity-processbot <>
Fixes #4687.
cumulus companion: paritytech/cumulus#901