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

Keyless proxied account (a.k.a anonymous proxy v2) #11115

Closed
wants to merge 3 commits into from

Conversation

olanod
Copy link
Contributor

@olanod olanod commented Mar 24, 2022

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:

  • Rename Anonymous Proxy to Something Else #7735 renaming anonymous proxy to proxied keyless account. It has been a big source of confusion and most people seem to agree the name should change.
  • Keyless account with vanity addresses. Based on my assumption that "very vain" vanity addresses can be guaranteed to not have a known private key I think it's viable to let the user provide their desired account and judge on-chain whether it's likely to be keyless or not. Initially I wanted to make it more automatic and provide just the desired pattern but maybe due to my lack of understanding of how base58 encoding works I couldn't create a reliable address generator but for Polkadot addresses at least it works fine and its kind of fun(demo) so it can be left to the client to find their account that can be judged based on some criteria like how much a character repeats in the encoded address or as seen in the test measure the entropy or something. Also accounts with vanity in the binary representation are easy to evaluate(e.g. make sure everything is 0s after the pattern).
  • Allow transaction payment from proxy target account polkadot-sdk#348

    Alternative way is allow account to approve another account to use it as the funding source of tx fee. This can also increase provider count for the spending account. So it will be just one more storage read to fetch funding source. If we bake this into system AccountData, there won't be any additional storage access.

  • anonymous proxy should add provider #8550
  • Proxy accounts with dynamic delay  #7894

#[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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please avoid breaking changes

Copy link
Contributor Author

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?

Copy link
Member

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

Copy link
Contributor

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
Copy link
Contributor

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.

Copy link
Contributor Author

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)?

Copy link
Contributor

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.

Copy link
Contributor Author

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?

@bkchr
Copy link
Member

bkchr commented Mar 27, 2022

Keyless proxy is still a confusing name.

@olanod
Copy link
Contributor Author

olanod commented Mar 28, 2022

@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.

@stale
Copy link

stale bot commented Apr 27, 2022

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.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Apr 27, 2022
@stale stale bot closed this May 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants