[MOON-863] add proxy precompile

[nbaztec]

What does it do?

Adds proxy precompile with the following functions:

• addProxy(address delegate, uint8 proxyType, uint32 delay)
• removeProxy(address delegate, uint8 proxyType, uint32 delay)
• removeProxies()
• isProxy(address delegate, uint8 proxyType)

TODO:

• [x] add smart contract usage example/test

What important points reviewers should know?

:warning: With this PR, a smart contract is now able to proxy a real account and effectively make substrate calls on its behalf, something that wasn't possible before.

:warning: The precompile disallows executing add_proxy if a proxy (irrespective of the type) already exists. This is to prevent a privilege escalation attack. Currently this attack is not possible since EVM requires the presence of Origin::Root. As an additional precautionary step Call::EVM is now also filtered out.

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

[crystalin]

Should we provide a function to list the proxies for a given address, like substrate does ?

[nbaztec]

Should we provide a function to list the proxies for a given address, like substrate does ?

I tried to achieve that functionality via providing is_proxy to avoid having to return an unbounded vector. Should we still add the functionality for the list of proxies?

[crystalin]

It can be bounded to the maximum number of proxy one can have. I believe it is better to provide the same functions as substrate if possible. It might be good to ask the rest of the team

[librelois]

Should we provide a function to list the proxies for a given address, like substrate does ?

I tried to achieve that functionality via providing is_proxy to avoid having to return an unbounded vector. Should we still add the > functionality for the list of proxies?

I think there is a misunderstanding here. Sustrate lists proxies by "real account", not by "proxy account.". So it can't be provided by is_proxy since it is the reverse mapping.

My suggestion: keep is_proxy as is, and add another function get_proxies.

[nbaztec]

I think there is a misunderstanding here. Sustrate lists proxies by "real account", not by "proxy account.". So it can't be provided by is_proxy since it is the reverse mapping.

My suggestion: keep is_proxy as is, and add another function get_proxies.

Yeah so if a subcontract calls the precompile to add other accounts as a proxy, the SC is the real account. In this scenario both is_proxy(delegate, type) and get_proxies() have same mapping direction, because they are index by the real account and return a list of delegated accounts and the proxy types. is_proxy merely checks if an entry in that list exists or not with the given parameters.

[crystalin]

How about using the same name as the substrate extrinsics ?

[nbaztec]

How about using the same name as the substrate extrinsics ?

You mean instead of is_proxy? Substrate has no such extrinsic and only the storage item proxies that returns all proxies per account

[crystalin]

No I mean instead of get_proxies (like @librelois suggested) to use proxies. I'm not sure if the is_proxy is to keep or not. On one side, it is allowed to check proxy with less gas_execution, but on the other side, the proxies shouldn't get retrieved by the smart-contract on-chain but off-chain to really optimize it

[librelois]

SGTM

[nbaztec]

@librelois I had to make isProxy a view function so I could no longer rely on the origin - could you take a look if that's fine as well please?

[nbaztec]

@nanocryk think I saw a comment from you about adding ForbidRecursion to the precompile but I can no longer find it. Should I still add it?

[librelois]

@librelois I had to make isProxy a view function so I could no longer rely on the origin - could you take a look if that's fine as well please?

Good catch, I just reread your commit and everything looks good to me

[librelois]

@nanocryk think I saw a comment from you about adding ForbidRecursion to the precompile but I can no longer find it. Should I still add it?

ForbidRecursion is already the default behavior.

[jboetticher]

a smart contract is now able to proxy a real account and effectively make substrate calls on its behalf

Just checking: does the precompile allow smart contracts to call as a proxy? The interface makes it look like the proxy precompile only allows the addition/removal/checking of proxy accounts.

[crystalin]

No, the proxy precompiles is not allowed to be called by Smart Contracts.