[draft] feat: [v0.8-develop] composable validation example

.solhint-test.json

@@ -5,6 +5,7 @@
       "immutable-vars-naming": ["error"],
       "no-unused-import": ["error"],
       "compiler-version": ["error", ">=0.8.19"],
+      "custom-errors": "off",
       "func-visibility": ["error", { "ignoreConstructors": true }],
       "max-line-length": ["error", 120],
       "max-states-count": ["warn", 30],

src/account/AccountLoupe.sol

@@ -7,7 +7,7 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet
 import {IAccountLoupe, ExecutionHook} from "../interfaces/IAccountLoupe.sol";
 import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol";
 import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol";
-import {getAccountStorage, SelectorData, toFunctionReferenceArray, toExecutionHook} from "./AccountStorage.sol";
+import {getAccountStorage, SelectorData, toExecutionHook, toSelector} from "./AccountStorage.sol";
 
 abstract contract AccountLoupe is IAccountLoupe {
     using EnumerableSet for EnumerableSet.Bytes32Set;
@@ -28,8 +28,16 @@ abstract contract AccountLoupe is IAccountLoupe {
     }
 
     /// @inheritdoc IAccountLoupe
-    function getValidations(bytes4 selector) external view override returns (FunctionReference[] memory) {
-        return toFunctionReferenceArray(getAccountStorage().selectorData[selector].validations);
+    function getSelectors(FunctionReference validationFunction) external view returns (bytes4[] memory) {
+        uint256 length = getAccountStorage().validationData[validationFunction].selectors.length();
+
+        bytes4[] memory selectors = new bytes4[](length);
+
+        for (uint256 i = 0; i < length; ++i) {
+            selectors[i] = toSelector(getAccountStorage().validationData[validationFunction].selectors.at(i));
+        }
+
+        return selectors;
     }
 
     /// @inheritdoc IAccountLoupe
@@ -58,8 +66,7 @@ abstract contract AccountLoupe is IAccountLoupe {
         override
         returns (FunctionReference[] memory preValidationHooks)
     {
-        preValidationHooks =
-            toFunctionReferenceArray(getAccountStorage().validationData[validationFunction].preValidationHooks);
+        preValidationHooks = getAccountStorage().validationData[validationFunction].preValidationHooks;
     }
 
     /// @inheritdoc IAccountLoupe

src/account/AccountStorage.sol

@@ -29,8 +29,6 @@ struct SelectorData {
     bool allowDefaultValidation;
     // The execution hooks for this function selector.
     EnumerableSet.Bytes32Set executionHooks;
-    // Which validation functions are associated with this function selector.
-    EnumerableSet.Bytes32Set validations;
 }
 
 struct ValidationData {
@@ -39,7 +37,9 @@ struct ValidationData {
     // Whether or not this validation is a signature validator.
     bool isSignatureValidation;
     // The pre validation hooks for this function selector.
-    EnumerableSet.Bytes32Set preValidationHooks;
+    FunctionReference[] preValidationHooks;
+    // The set of selectors that may be validated by this validation function.
+    EnumerableSet.Bytes32Set selectors;
 }
 
 struct AccountStorage {
@@ -93,6 +93,14 @@ function toExecutionHook(bytes32 setValue)
     isPostHook = (uint256(setValue) >> 72) & 0xFF == 1;
 }
 
+function toSetValue(bytes4 selector) pure returns (bytes32) {
+    return bytes32(selector);
+}
+
+function toSelector(bytes32 setValue) pure returns (bytes4) {
+    return bytes4(setValue);
+}
+
 /// @dev Helper function to get all elements of a set into memory.
 function toFunctionReferenceArray(EnumerableSet.Bytes32Set storage set)
     view

src/account/PluginManager2.sol

@@ -6,16 +6,20 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet
 import {IPlugin} from "../interfaces/IPlugin.sol";
 import {FunctionReference} from "../interfaces/IPluginManager.sol";
 import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol";
-import {AccountStorage, getAccountStorage, toSetValue, toFunctionReference} from "./AccountStorage.sol";
+import {AccountStorage, getAccountStorage, toSetValue} from "./AccountStorage.sol";
 
 // Temporary additional functions for a user-controlled install flow for validation functions.
 abstract contract PluginManager2 {
     using EnumerableSet for EnumerableSet.Bytes32Set;
 
+    // Index marking the start of the data for the validation function.
+    uint8 internal constant _RESERVED_VALIDATION_DATA_INDEX = 255;
+
     error DefaultValidationAlreadySet(FunctionReference validationFunction);
     error PreValidationAlreadySet(FunctionReference validationFunction, FunctionReference preValidationFunction);
     error ValidationAlreadySet(bytes4 selector, FunctionReference validationFunction);
     error ValidationNotSet(bytes4 selector, FunctionReference validationFunction);
+    error PreValidationHookLimitExceeded();
 
     function _installValidation(
         FunctionReference validationFunction,
@@ -36,19 +40,21 @@ abstract contract PluginManager2 {
             for (uint256 i = 0; i < preValidationFunctions.length; ++i) {
                 FunctionReference preValidationFunction = preValidationFunctions[i];
 
-                if (
-                    !_storage.validationData[validationFunction].preValidationHooks.add(
-                        toSetValue(preValidationFunction)
-                    )
-                ) {
-                    revert PreValidationAlreadySet(validationFunction, preValidationFunction);
-                }
+                _storage.validationData[validationFunction].preValidationHooks.push(preValidationFunction);
 
                 if (initDatas[i].length > 0) {
                     (address preValidationPlugin,) = FunctionReferenceLib.unpack(preValidationFunction);
                     IPlugin(preValidationPlugin).onInstall(initDatas[i]);
                 }
             }
+
+            // Avoid collision between reserved index and actual indices
+            if (
+                _storage.validationData[validationFunction].preValidationHooks.length
+                    > _RESERVED_VALIDATION_DATA_INDEX
+            ) {
+                revert PreValidationHookLimitExceeded();
+            }
         }
 
         if (isDefault) {
@@ -60,7 +66,7 @@ abstract contract PluginManager2 {
 
         for (uint256 i = 0; i < selectors.length; ++i) {
             bytes4 selector = selectors[i];
-            if (!_storage.selectorData[selector].validations.add(toSetValue(validationFunction))) {
+            if (!_storage.validationData[validationFunction].selectors.add(toSetValue(selector))) {
                 revert ValidationAlreadySet(selector, validationFunction);
             }
         }
@@ -73,7 +79,6 @@ abstract contract PluginManager2 {
 
     function _uninstallValidation(
         FunctionReference validationFunction,
-        bytes4[] calldata selectors,
         bytes calldata uninstallData,
         bytes calldata preValidationHookUninstallData
     ) internal {
@@ -85,25 +90,21 @@ abstract contract PluginManager2 {
         bytes[] memory preValidationHookUninstallDatas = abi.decode(preValidationHookUninstallData, (bytes[]));
 
         // Clear pre validation hooks
-        EnumerableSet.Bytes32Set storage preValidationHooks =
+        FunctionReference[] storage preValidationHooks =
             _storage.validationData[validationFunction].preValidationHooks;
-        while (preValidationHooks.length() > 0) {
-            FunctionReference preValidationFunction = toFunctionReference(preValidationHooks.at(0));
-            preValidationHooks.remove(toSetValue(preValidationFunction));
-            (address preValidationPlugin,) = FunctionReferenceLib.unpack(preValidationFunction);
+        for (uint256 i = 0; i < preValidationHooks.length; ++i) {
+            FunctionReference preValidationFunction = preValidationHooks[i];
             if (preValidationHookUninstallDatas[0].length > 0) {
+                (address preValidationPlugin,) = FunctionReferenceLib.unpack(preValidationFunction);
                 IPlugin(preValidationPlugin).onUninstall(preValidationHookUninstallDatas[0]);
             }
         }
+        delete _storage.validationData[validationFunction].preValidationHooks;
 
-        // Because this function also calls `onUninstall`, and removes the default flag from validation, we must
-        // assume these selectors passed in to be exhaustive.
-        // TODO: consider enforcing this from user-supplied install config.
-        for (uint256 i = 0; i < selectors.length; ++i) {
-            bytes4 selector = selectors[i];
-            if (!_storage.selectorData[selector].validations.remove(toSetValue(validationFunction))) {
-                revert ValidationNotSet(selector, validationFunction);
-            }
+        // Clear selectors
+        while (_storage.validationData[validationFunction].selectors.length() > 0) {
+            bytes32 selector = _storage.validationData[validationFunction].selectors.at(0);
+            _storage.validationData[validationFunction].selectors.remove(selector);
         }
 
         if (uninstallData.length > 0) {

src/account/PluginManagerInternals.sol

@@ -103,12 +103,10 @@ abstract contract PluginManagerInternals is IPluginManager {
         internal
         notNullFunction(validationFunction)
     {
-        SelectorData storage _selectorData = getAccountStorage().selectorData[selector];
-
         // Fail on duplicate validation functions. Otherwise, dependency validation functions could shadow
         // non-depdency validation functions. Then, if a either plugin is uninstalled, it would cause a partial
         // uninstall of the other.
-        if (!_selectorData.validations.add(toSetValue(validationFunction))) {
+        if (!getAccountStorage().validationData[validationFunction].selectors.add(toSetValue(selector))) {
             revert ValidationFunctionAlreadySet(selector, validationFunction);
         }
     }
@@ -117,11 +115,9 @@ abstract contract PluginManagerInternals is IPluginManager {
         internal
         notNullFunction(validationFunction)
     {
-        SelectorData storage _selectorData = getAccountStorage().selectorData[selector];
-
         // May ignore return value, as the manifest hash is validated to ensure that the validation function
         // exists.
-        _selectorData.validations.remove(toSetValue(validationFunction));
+        getAccountStorage().validationData[validationFunction].selectors.remove(toSetValue(selector));
     }
 
     function _addExecHooks(