Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Implications of Using AccountId::default() as parent AccountId for Location-Conversion #4687

Closed
mustermeiszer opened this issue Jan 10, 2022 · 2 comments · Fixed by #4712
Closed

Comments

@mustermeiszer
Copy link
Contributor

Given a note on the default behaviour of AccountId32

Quote:

We recently discovered that anyone can sign transactions in Substrate with a zero account (AccountId::default()). The reason behind this is that cryptographically a private key of zero makes to an actual complete curve in sr25519 and ed255519. This is coming directly from schnorrkel.

Resolved for substrate via paritytech/substrate#10403

Aren't the following lines a bit dangerous, given that many parachains will most likely plug in the normal AccountId32 type there?

pub struct ParentIsDefault<AccountId>(PhantomData<AccountId>);
impl<AccountId: Default + Eq + Clone> Convert<MultiLocation, AccountId>
for ParentIsDefault<AccountId>
{
fn convert_ref(location: impl Borrow<MultiLocation>) -> Result<AccountId, ()> {
if location.borrow().contains_parents_only(1) {
Ok(AccountId::default())
} else {
Err(())
}
}
fn reverse_ref(who: impl Borrow<AccountId>) -> Result<MultiLocation, ()> {
if who.borrow() == &AccountId::default() {
Ok(Parent.into())
} else {
Err(())
}
}
}

@bkchr
Copy link
Member

bkchr commented Jan 11, 2022

CC @KiChjang

@KiChjang
Copy link
Contributor

Yeah, we should remove that, I guess this just wasn't done in the polkadot repo yet.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants