feat: [v0.8-develop] remove execute from plugin

[adamegyed]

Motivation

The function executeFromPlugin and executeFromPluginExternal, along with the manifest fields associated with these functions, were created before we had multiple validation function support & better management of pre-validation hooks, as a way to allow cross-plugin and plugin-to-external-contract interactions.

With multi-validation, it is no longer necessary to have these two parallel paths for performing calls from plugins. The "permitted call" authorization path still exists via the fallback and native functions, and any function wishing to invoke external contracts can simply ask for permission to validate execute/executeBatch, and validate itself.

Additionally, executeFromPluginExternal required accounts to have some opinionated access control functions built-in as part of the standard. This type of access control management is best left to actual plugin implementations, and with the possibility to install pre-validation hooks at the same time as validations via #64, the same type of controls can be placed on any plugin requesting access to execute / executeBatch at install time.

Solution

• Remove executeFromPlugin and executeFromPluginExternal from the account, and delete IPluginExecutor.
• Move executeWithAuthorization (part of multi-validation) out to IStandardExecutor.
• Update previous tests of executeFromPlugin + executeFromPluginExternal to use the permitted call flow and/or executeWithAuthorization, to show the functional equivalence.
• Remove fields in the plugin manifest related to executeFromPluginExternal.
  • Note that this made it so that there are no fixed-size fields in the plugin manifest. In tests where we declare a public state variable for a plugin manifest, this results in the error The struct has all its members omitted, therefore the getter cannot return any values.. To address this, the fields are made internal, and the linter is updated to not require renaming all instances of these.

Future work

• It may be worth merging account interfaces to simplify in the future, with a required interface, and possibly optional extension for loupe functions.
• Since these permission flows mostly depend on runtime validation, and the difference between user op validation and runtime validation is the ability to spend account gas, it may be worthwhile to add a representation to the manifest that would allow and account to restrict a validation function to "only runtime". This would allow users inspecting a plugin before installation to be sure that its permissions only encompass runtime auth, and not the ability to spend gas.

[huaweigu]

lgtm, just one question!

[huaweigu]

I'm curious to hear your thoughts on why you prefer the new function to be part of the standard executor rather than an extension executor :)

[adamegyed]

To be honest I don't have a strict reasoning - I think we could split it out, or we could try to bring together all of the required account functions into one interface for simplicity. I think I'm leaning more towards the latter to make it easier to read the spec, but open to discussing.

[fangting-alchemy]

I am more lean towards to split it out.
StandardExecutor indicates the functions here are following a certain standard (ERC-4337).

[adamegyed]

Technically the functions in IStandardExecutor are only standardized in ERC-6900, but 6900 also requires the plugin installation functions, which are in a separate interface IPluginManager.

As for executeWithAuthorization, it's something that all 6900 compliant accounts will need to implement after this spec change. Do y'all have any suggestions for what to name an interface that only holds this one function? Or how to rename IStandardExecutor or other existing account interfaces to hold it?

[fangting-alchemy]

Actually to rethink on this, especially since executeWithAuthorization is something that all 6900 compliant accounts will need to implement. We can just keep it in there.

[howydev]

hmm why do we need to change this?

[howydev]

oh tests, yeah those have been annoying

[jaypaik]

Can we fix the tests instead?

[adamegyed]

Rolled back the styling change, and fixed the tests

[fangting-alchemy]

LGTM with a question.