Acala icon indicating copy to clipboard operation
Acala copied to clipboard

refactor evm origin with DispatchContext

Open xlc opened this issue 2 years ago • 13 comments

new feature from https://github.com/paritytech/substrate/pull/13468

we can use this to implement evm origin

xlc avatar Feb 27 '23 20:02 xlc

the way the DispatchContext API is structured may not make it possible for evm origin use case

xlc avatar Feb 27 '23 20:02 xlc

the way the DispatchContext API is structured may not make it possible for evm origin use case

What is the problem? What do you need?

bkchr avatar Feb 27 '23 22:02 bkchr

I think we need to implement it to know if more changes are required. Here are two use cases we have currently:

Set origin for tx in pre_dispatch for signed tx https://github.com/AcalaNetwork/Acala/blob/1cf443e85bbad490e5b0a3d4fe266aae027acb99/modules/evm/src/lib.rs#L2021-L2041

Set origin for xcm call https://github.com/AcalaNetwork/Acala/blob/1cf443e85bbad490e5b0a3d4fe266aae027acb99/runtime/common/src/xcm_impl.rs#L250-L269

xlc avatar Feb 28 '23 01:02 xlc

Yeah those would not work, as they are still outside the dispatch context! Not sure if you could set store these origins in some call.

bkchr avatar Feb 28 '23 20:02 bkchr

We cannot set origin in some call. Because for example a swap can trigger ERC20 transfer. This means we need to set it on every call.

xlc avatar Feb 28 '23 20:02 xlc

If wanted, we could also let the initial dispatch context being created in frame-executive. This should then help for your use case. I also thought about having some kind of support to know the outer origin/account id while implementing this feature.

bkchr avatar Feb 28 '23 20:02 bkchr

Yeah that will be lot more useful. It will be great if you can have it cover initialize/finalize hook and xcm execution as well so we don't need to do special handling in our runtime.

xlc avatar Feb 28 '23 21:02 xlc

initialize/finalize hook

That goes a little bit against the idea of the "dispatch context". It should be in the context of calling dispatch and then should also be deleted before doing the next dispatch. What kind of information are you storing in initialize/finalize hooks that require special handling?

bkchr avatar Mar 01 '23 12:03 bkchr

The idea is there is always a context for any code execution.

In our use case, evm execution always requires an origin and we can’t rule out the possibility that someone may want to propose a governance call that executes evm call. In that case it will try to access the dispatch context. It could return None and we do special handling knowing it means the call must be dispatched by a hook. Or there could e a dispatch context API allow me define a default context. Then we could create a context with some special evm address as the origin.

xlc avatar Mar 01 '23 18:03 xlc

In our use case, evm execution always requires an origin and we can’t rule out the possibility that someone may want to propose a governance call that executes evm call.

But that means that this will be some kind of EvmPallet::execute_evm { evm_call } call? And you execute this call with dispatch? So, there will be a context. You could also start the context before calling dispatch and set the origin.

bkchr avatar Mar 01 '23 19:03 bkchr

In the example of governance, the call is created by democracy or referendum pallet, executed by scheduler pallet. There is no place for me to inject context. Unless we create special variant of call for the purpose of governance which I think will work but make generic governance UI harder to implement.

xlc avatar Mar 01 '23 20:03 xlc

Democracy and Referenda are using dispatch to dispatch calls, which results in a context being created. Every dispatch call in the runtime will create/reuse this context.

bkchr avatar Mar 06 '23 10:03 bkchr

I see. I guess it could work. We just need to give it a try and see.

xlc avatar Mar 06 '23 20:03 xlc