-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Keyless proxied account (a.k.a anonymous proxy v2) #11115
Conversation
#[pallet::weight(T::WeightInfo::anonymous(T::MaxProxies::get().into()))] | ||
pub fn anonymous( | ||
#[pallet::weight(T::WeightInfo::proxied_keyless(T::MaxProxies::get().into()))] | ||
pub fn proxied_keyless( |
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.
please avoid breaking changes
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.
Sure, this was more of a quick search and replace, I'll keep the previous "deprecated" method around. Any other things to consider when introducing this kind of API changes?
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 rename the functions though... Are you suggesting we should add a new function with a new name?
AFAIK this "breaking change" would only be breaking for JS integrations, but all txs and signers will be okay
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.
rename it is breaking for JS and ledger. let's avoid that unless absolutely necessary.
and new parameter is a transaction_version breaking change.
ensure!(!Proxies::<T>::contains_key(&anonymous), Error::<T>::Duplicate); | ||
let keyless = if let Some(account) = maybe_keyless { | ||
ensure!(Self::is_vain_enough(&account), Error::<T>::NotKeyless); | ||
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.
what? you allow people to specify the account? that for sure is a security hole.
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.
But is it though? I don't think it's sure, that's my main question here as this is based on an empirical assumption. There's gonna be a line to not cross when evaluating the "keylessness" of an account that might even get blurry at some point, but we can always be super conservative about it to stay on the safe side. The initial approach here is leaving part of the judgment to the implementer that should specify a safe MaxVanity
. Perhaps a crypto-math expert can jump in and say with certainty where the safe line is? is it much different from the safety of a pallet id(e.g. modlpy/trsry
)?
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.
No it is different thing. You allow people to pick any addresses that fits a requirement without many more complicated checking and that will allow reuse address attack etc. Allow anyone to pick their own address is almost always a bad idea.
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.
What would a more complicated checking look like? perhaps make the process more expensive/permissioned? an auction? should governance be like a registar that sells/leases the right to have those accounts? Not to sound stubborn but I think it's a feature too cool to not make the effort to make it work somehow. wouldn't 1AcaLaNetwork1111111111111111111111111111111L6H
be a nice account to have for a project?
Keyless proxy is still a confusing name. |
@bkchr Worth noting that it's not "keyless proxy" but a "proxied keyless account", I'm open for suggestions but hopefully this doesn't become another long conversation not leading anywhere like in the linked issue. One of the main confusions that need to be tackled and be clear in the name of the extrinsic, in the documentation and when talking about the feature is that there is nothing proxy about anonymous proxies, the main goal is not to create a proxy but a new (keyless)account that is proxied by the calling origin, I'd say the feature doesn't even belong to the proxy pallet as creating an account would probably fit better in the system pallet for example(to not couple system with proxies the account could be created and later "claimed" from the proxy pallet?). Whatever we call it I also think it it shouldn't be "too smart", this kind of abstractions tend to leak to the average user that then needs to be educated, I say it because as ambassador I've had to deal with that. My other proposal is to call it "virtual accounts", is a concept I've used to explain the feature saying that it's a way to create an account that only exists on-chain. |
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
Anonymous proxies are a powerful feature that might just need a few tweaks to improve their usability and become the go to primitive for wallets when creating secure user friendly accounts, in Virto for example with our main audience being the crypto-clueless and usability our main priority, anonymous proxies will be a great tool to have, likely the default kind of account for most users so its in our interest to make it better ;)
My main initial goal with this PR was to allow creating anonymous proxies that have a vanity addresses, however I see an opportunity here to tackle some extra issues that haven't received enough attention and I suppose "just going for it" can help bring attention to them and push ourselves to make decisions.
Topic to consider: