feat: [v0.8-develop, experimental] default validation

[adamegyed]

Motivation

erc6900/resources#37

By introducing a "default" validation function pool, we can reduce plugin installation costs, particularly for ownership plugins, by allowing execution functions to opt-in to a default validation pool.

This also addresses some incompatibilities between ownership plugins and semi-modular accounts, by removing the need for a plugin to know each native function implemented by an account.

Solution

While the motivation is clear, I think there are a lot of possible ways to address the issue. This is one example of how we can address it.

This PR does the following:

• Adds a new field to execution functions called allowDefaultValidation, indicating whether the defining plugin believes it should be accessible by any validation added to the "pool of default validation functions".
• This is set by the implementing plugin, rather than the end user, because the function implementation will probably be aware of the differences between a function that "just needs any user-controlled validation applied to it", and a function that needs a specific type of validation function. Examples include limiting social account recovery to only it's own validation method, while something like modifying a subscription payment plugin's subscriptions could be done by any "owner" validation.
  • I think this is the part of the design I have the least confidence in – there may be a reason for the end users to decide whether an execution function should be a part of the default pool or not. Of course, the entire install flow may be changed with [Improvement] User-supplied install config resources#9 so it's hard to tell how it will look now.
• The option to mark a validation function as in the pool of defaults or not, however, is not left up to the plugin - this is set by the user.
  • To accomplish this, this PR adds temporary extra plugin management functions called installValidation/uninstallValidation, that does a reduced-scope version of installPlugin for only a specific validation function. The user is able to control the domain of this validation function by specifying individual selectors, and whether or not the plugin should be in the default pool.
• This PR also adds a new factory and a test specifically for the default validation workflows.

New Todos

• Consolidate old tests & test base to use default validation. This is blocked on the install/uninstall plugin revamp

[0xrubes]

This would be very cool because I think it would enable a fairly straight forward path to compatibility with ERC7579's validator plugin (default validation acting as 7579's validator plugin)

[adamegyed]

The introduction of default validation has a lot of reasons to exist outside of compatibility - better support for semi-modular accounts and cheaper account deployments.

ERC7579-style validators don't make sense organizationally for a modular account, and even ZD Kernel is shifting away from it. I've previously outlined reasons why keeping some identifier or switching mechanism is needed here: #41 (comment)

And you can see how Kernel is ditching the 7579 validator model for lack of flexibility here, introducing a bytes32 id: https://github.com/zerodevapp/kernel/blob/d191610915395232ecebe80d67f82482edd87f4e/src/interfaces/IERC7579Modules.sol#L91. They also added an equivalent to 6900 pre-validation hooks that they call "policies".

[howydev]

nice

[howydev]

Did a first pass - overall this makes 6900 accounts cheaper to deploy while maintaining flexibility in deciding which native functions the default validation can cover, which is great. Would prefer if another reviewer took a look just in case I missed some things though

Are there differences in the validation functions used per selector v.s. a default validation function? Not a strong opinion, but IMO it's quite dangerous to allow plugins to specify being a default validator, think it should be an option in install configs instead

[adamegyed]

Are there differences in the validation functions used per selector v.s. a default validation function? Not a strong opinion, but IMO it's quite dangerous to allow plugins to specify being a default validator, think it should be an option in install configs instead

Yeah I agree that could be risky. In the current PR it doesn't allow that, it depends on the user calling installValidation to specify it as such. The case that I'm a bit less certain about is whether or not a selector should be allowed to be validated using a default validation - for now, this is set by the plugin that defines the execution plugin, similar to the isPublic flag, but I could see an argument for making this user-specified too.

[huaweigu]

This is set by the implementing plugin, rather than the end user

Just my 2️⃣ cents, I feel like the end user would probably prefer to control the validations themselves rather than relying on the plugin implementation. Ideally, the account should be protected by default validations. If a specific execution function's validation is overridden by a plugin, then that plugin's validation should take precedence. However, when it comes to features, the situation might be different. The end user would probably prefer to have account features extended by plugins rather than account itself.

[adamegyed]

Updated all naming of "shared" to "default"

[fangting-alchemy]

In the spirit of simplifying plugin dev work, allowDefaultValidation can be omitted in this case.

In reality, both of those two flags will default to false if not set. We should document this somewhere.

[fangting-alchemy]

Is this necessary? Is the goal to separate validation installation from other plugin installations?

Would it be easier to only separate default validation installation from all other plugin installations (include non default validation installations).

[adamegyed]

These are not final interfaces, they're side doors to the install path because we haven't fully implemented user-supplied install configs yet. More detail in IPluginManager: https://github.com/erc6900/reference-implementation/blob/adam/default-validation/src/interfaces/IPluginManager.sol

[fangting-alchemy]

This field being true also indicates the function is not state-change, right? Might be worth adding it to the docs.

[adamegyed]

Technically, it can still be state-changing, it's just that usually this will only be used for view functions. See this PR comment for more context: #61

[huaweigu]

Minor, during the installation, it seems like we also install the default validation

for (uint256 i = 0; i < selectors.length; ++i) {
            bytes4 selector = selectors[i];
            if (!_storage.selectorData[selector].validations.add(toSetValue(validationFunction))) {
                revert ValidationAlreadySet(selector, plugin, functionId);
            }
        }

should we check getAccountStorage().selectorData[selector].validations.contains() for both default and non-default?

[adamegyed]

That path in the installation is used if the user wishes to install a validation both as a default validation and for some specific selectors. If the user manually adds selectors that are already usable from default validation (e.g. execute), then the cost savings of default validation go away. Similarly, checking the per-selector storage for the default validation increases costs due to the storage reads without providing a new form of checking.

[huaweigu]

I see, that makes sense!

[huaweigu]

lgtm!