feat: [v0.8-develop, experimental] signature validator interface

src/account/AccountStorage.sol

@@ -61,6 +61,8 @@ struct AccountStorage {
     mapping(IPlugin => mapping(address => PermittedExternalCallData)) permittedExternalCalls;
     // For ERC165 introspection
     mapping(bytes4 => uint256) supportedIfaces;
+    // Holds all added signature validators.
+    EnumerableSet.AddressSet signatureValidators;
 }
 
 function getAccountStorage() pure returns (AccountStorage storage _storage) {

src/account/PluginManagerInternals.sol

@@ -55,19 +55,28 @@ abstract contract PluginManagerInternals is IPluginManager {
     // Storage update operations
 
     function _setExecutionFunction(bytes4 selector, address plugin) internal notNullPlugin(IPlugin(plugin)) {
-        SelectorData storage _selectorData = getAccountStorage().selectorData[selector];
+        // Temporary specification mechanism for signature validators, prior to multi-validation support.
+        if (selector == IPlugin.isValidSignatureWithSender.selector) {
+            getAccountStorage().signatureValidators.add(plugin);
+        } else {
+            SelectorData storage _selectorData = getAccountStorage().selectorData[selector];
 
-        if (_selectorData.plugin != address(0)) {
-            revert ExecutionFunctionAlreadySet(selector);
-        }
+            if (_selectorData.plugin != address(0)) {
+                revert ExecutionFunctionAlreadySet(selector);
+            }
 
-        _selectorData.plugin = plugin;
+            _selectorData.plugin = plugin;
+        }
     }
 
     function _removeExecutionFunction(bytes4 selector) internal {
-        SelectorData storage _selectorData = getAccountStorage().selectorData[selector];
+        if (selector == IPlugin.isValidSignatureWithSender.selector) {
+            getAccountStorage().signatureValidators.remove(msg.sender);
+        } else {
+            SelectorData storage _selectorData = getAccountStorage().selectorData[selector];
 
-        _selectorData.plugin = address(0);
+            _selectorData.plugin = address(0);
+        }
     }
 
     function _addValidationFunction(bytes4 selector, IPlugin validation) internal notNullPlugin(validation) {

src/account/UpgradeableModularAccount.sol

@@ -6,6 +6,7 @@ import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntry
 import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol";
 import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
 import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
+import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol";
 import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol";
 import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 
@@ -28,13 +29,15 @@ contract UpgradeableModularAccount is
     AccountStorageInitializable,
     BaseAccount,
     IERC165,
+    IERC1271,
     IPluginExecutor,
     IStandardExecutor,
     PluginManagerInternals,
     UUPSUpgradeable
 {
     using EnumerableMap for EnumerableMap.Bytes32ToUintMap;
     using EnumerableSet for EnumerableSet.Bytes32Set;
+    using EnumerableSet for EnumerableSet.AddressSet;
 
     struct PostExecToRun {
         bytes preExecHookReturnData;
@@ -300,6 +303,14 @@ contract UpgradeableModularAccount is
         return getAccountStorage().supportedIfaces[interfaceId] > 0;
     }
 
+    function isValidSignature(bytes32 hash, bytes calldata signature) external view override returns (bytes4) {
+        // Uses the first signature validator that is added.
+        // Multiple validation support will be implemented in a separate PR.
+        return IPlugin(getAccountStorage().signatureValidators.at(0)).isValidSignatureWithSender(
+            msg.sender, hash, signature
+        );
+    }
+
     /// @inheritdoc UUPSUpgradeable
     function upgradeTo(address newImplementation) public override onlyProxy wrapNativeFunction {
         _upgradeToAndCallUUPS(newImplementation, new bytes(0), false);

src/interfaces/IPlugin.sol

@@ -152,6 +152,17 @@ interface IPlugin {
     /// @param preExecHookData The context returned by its associated pre execution hook.
     function postExecutionHook(bytes calldata preExecHookData) external;
 
+    /// @notice Validates a signature using ERC-1271.
+    /// @param sender the address that sent the ERC-1271 request to the smart account
+    /// @param hash the hash of the ERC-1271 request
+    /// @param signature the signature of the ERC-1271 request
+    ///
+    /// @return the ERC-1271 `MAGIC_VALUE` if the signature is valid, or 0xFFFFFFFF if invalid.
+    function isValidSignatureWithSender(address sender, bytes32 hash, bytes calldata signature)
+        external
+        view
+        returns (bytes4);
+
     /// @notice Describe the contents and intended configuration of the plugin.
     /// @dev This manifest MUST stay constant over time.
     /// @return A manifest describing the contents and intended configuration of the plugin.

src/plugins/BasePlugin.sol

@@ -73,6 +73,17 @@ abstract contract BasePlugin is ERC165, IPlugin {
         revert NotImplemented();
     }
 
+    /// @inheritdoc IPlugin
+    function isValidSignatureWithSender(address sender, bytes32 hash, bytes calldata signature)
+        external
+        view
+        virtual
+        returns (bytes4)
+    {
+        (sender, hash, signature);
+        revert NotImplemented();
+    }
+
     /// @inheritdoc IPlugin
     function pluginManifest() external pure virtual returns (PluginManifest memory) {
         revert NotImplemented();

src/plugins/owner/SingleOwnerPlugin.sol

@@ -2,7 +2,6 @@
 pragma solidity ^0.8.19;
 
 import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
-import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol";
 import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
 import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol";
 
@@ -36,7 +35,7 @@ import {ISingleOwnerPlugin} from "./ISingleOwnerPlugin.sol";
 /// the account, violating storage access rules. This also means that the
 /// owner of a modular account may not be another modular account if you want to
 /// send user operations through a bundler.
-contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 {
+contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin {
     using ECDSA for bytes32;
 
     string public constant NAME = "Single Owner Plugin";
@@ -60,14 +59,19 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 {
         _transferOwnership(newOwner);
     }
 
-    /// @inheritdoc IERC1271
     /// @dev The signature is valid if it is signed by the owner's private key
     /// (if the owner is an EOA) or if it is a valid ERC-1271 signature from the
     /// owner (if the owner is a contract). Note that unlike the signature
     /// validation used in `validateUserOp`, this does///*not** wrap the digest in
     /// an "Ethereum Signed Message" envelope before checking the signature in
     /// the EOA-owner case.
-    function isValidSignature(bytes32 digest, bytes memory signature) public view override returns (bytes4) {
+    function isValidSignatureWithSender(address sender, bytes32 digest, bytes memory signature)
+        public
+        view
+        override
+        returns (bytes4)
+    {
+        (sender);
         if (SignatureChecker.isValidSignatureNow(_owners[msg.sender], digest, signature)) {
             return _1271_MAGIC_VALUE;
         }
@@ -132,14 +136,14 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 {
 
         manifest.executionFunctions = new bytes4[](3);
         manifest.executionFunctions[0] = this.transferOwnership.selector;
-        manifest.executionFunctions[1] = this.isValidSignature.selector;
+        manifest.executionFunctions[1] = this.isValidSignatureWithSender.selector;
         manifest.executionFunctions[2] = this.owner.selector;
 
         ManifestFunction memory ownerValidationFunction = ManifestFunction({
             functionType: ManifestAssociatedFunctionType.SELF,
             dependencyIndex: 0 // Unused.
         });
-        manifest.validationFunctions = new ManifestAssociatedFunction[](8);
+        manifest.validationFunctions = new ManifestAssociatedFunction[](7);
         manifest.validationFunctions[0] = ManifestAssociatedFunction({
             executionSelector: this.transferOwnership.selector,
             associatedFunction: ownerValidationFunction
@@ -169,15 +173,6 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 {
             associatedFunction: ownerValidationFunction
         });
 
-        ManifestFunction memory alwaysAllowFunction = ManifestFunction({
-            functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW,
-            dependencyIndex: 0 // Unused.
-        });
-        manifest.validationFunctions[7] = ManifestAssociatedFunction({
-            executionSelector: this.isValidSignature.selector,
-            associatedFunction: alwaysAllowFunction
-        });
-
         return manifest;
     }
 

test/account/UpgradeableModularAccount.t.sol

@@ -420,6 +420,14 @@ contract UpgradeableModularAccountTest is OptimizedTest {
         assertEq(plugins[1], address(plugin));
     }
 
+    function test_isValidSignature_EOA() public {
+        bytes32 digest = keccak256("test");
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner2Key, digest);
+
+        // sig check should pass using 1271 magic value
+        assertEq(bytes4(0x1626ba7e), account2.isValidSignature(digest, abi.encodePacked(r, s, v)));
+    }
+
     function _installPluginWithExecHooks() internal returns (MockPlugin plugin) {
         vm.startPrank(owner2);
 

test/plugin/SingleOwnerPlugin.t.sol

@@ -151,20 +151,24 @@ contract SingleOwnerPluginTest is OptimizedTest {
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(privateKey, digest);
 
         // sig check should fail
-        assertEq(plugin.isValidSignature(digest, abi.encodePacked(r, s, v)), bytes4(0xFFFFFFFF));
+        assertEq(
+            plugin.isValidSignatureWithSender(address(0), digest, abi.encodePacked(r, s, v)), bytes4(0xFFFFFFFF)
+        );
 
         // transfer ownership to signer
         plugin.transferOwnership(signer);
         assertEq(signer, plugin.owner());
 
         // sig check should pass
-        assertEq(plugin.isValidSignature(digest, abi.encodePacked(r, s, v)), _1271_MAGIC_VALUE);
+        assertEq(
+            plugin.isValidSignatureWithSender(address(0), digest, abi.encodePacked(r, s, v)), _1271_MAGIC_VALUE
+        );
     }
 
     function testFuzz_isValidSignatureForContractOwner(bytes32 digest) public {
         vm.startPrank(a);
         plugin.transferOwnership(address(contractOwner));
         bytes memory signature = contractOwner.sign(digest);
-        assertEq(plugin.isValidSignature(digest, signature), _1271_MAGIC_VALUE);
+        assertEq(plugin.isValidSignatureWithSender(address(0), digest, signature), _1271_MAGIC_VALUE);
     }
 }