Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [v0.8-develop, experimental] Remove function id #41

Closed
wants to merge 8 commits into from

Conversation

adam-alchemy
Copy link
Contributor

Motivation

Function IDs act as a replacement for function selectors by allowing an internal switching mechanism while preserving fixed names for plugin functions. However, they are unfamiliar to newer solidity developers, who may not understand how to handle function dispatching, and present an overhead for accounts to maintain in their mappings.

This is an experimental implementation to gather feedback on the proposed change.

Solution

  • Remove function ids from plugin functions, and refactor account internals to only hold address types for validation functions and hooks.
  • Remove the FunctionReference type, since interfaces are sufficient descriptors under this scheme.
  • Simplify the storage of associated post hooks down to a bool, since the addresses must be the same and there is no longer a function id to track.

Caveats

To replicate the behavior of multiple implementations of a plugin function, plugins may choose to switch based on the selector in calldata (or userOp.callData). However, this is only reliable for functions defined by the plugin, or otherwise known by the plugin at compile time. If the plugin is used as a dependency validator for another plugin, the selectors present there will not be known to the plugin, and switching will have to fall to a default case or fail. Not all validation plugins may have a semantically meaningful default case.

More testing is needed to determine whether or not this caveat presents a problem for the expected use cases of validation functions.

To-do

  • There is some additional simplification possible within the execution hook data structures given this restructuring, by setting the pre/post status as a boolean in a struct, rather than defining a second mapping.

@huaweigu
Copy link
Contributor

If the plugin is used as a dependency validator for another plugin, the selectors present there will not be known to the plugin.

Why would the validation plugin need to know the dependent plugin's functions selectors? Would that introduce cyclic dependency?
For instance, it makes sense for other dependent plugin to understand multi owner plugin's context. Do you mean multi owner plugin also need to be aware of other dependent plugin?

@adam-alchemy
Copy link
Contributor Author

Why would the validation plugin need to know the dependent plugin's functions selectors? Would that introduce cyclic dependency?

The case I'm trying to describe is if a plugin defines more than one validation function. For example, there could be a plugin with an "admin" validation and a "regular" validation. Previously, while the account used function IDs to track validation assignments, the user would be able to pick between using the admin validation or the regular validation as a dependency. So in this example, the user could be installing a subscription payments plugin, and then make the decision to either require regular auth or admin auth to modify the subscription settings.

However, with the removal of function IDs, plugins may only define one validation function. This validation function can still switch internally based on the calldata. So in the example of the multi-level validation plugin as before, the plugin could check to see if there's a sensitive method being called like transferOwnership and require admin validation there, but not require admin validation for other actions. However, in this setting, it is now impossible for the user of the account to require the subscription payments plugin to use admin validation, because the "internal switching mechanism" of the plugin can't take into account the other selectors it might be used for via depdendencies. This is the circular dependency that you described, and it is the drawback of removing function IDs.

The question now becomes, is this drawback big enough to require us to keep some form of function ID in the standard, or can we use other modular features to still get enough flexibility?

@adamegyed
Copy link
Contributor

Closing for now due to the aforementioned issues, and the indication that keeping function ids is relevant to supporting composable validation. We can always revisit this branch in the future.

@adamegyed adamegyed closed this Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants