diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index 7c8bbcb9..72832bbe 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -431,7 +431,8 @@ contract UpgradeableModularAccount is abi.decode(validationComposition, (FunctionReference[])); emit GotHere(); - currentValidationData = _doChainedValidation(chainedValidations, userOp, signatureSegment.getBody(), userOpHash); + currentValidationData = + _doChainedValidation(chainedValidations, userOp, signatureSegment.getBody(), userOpHash); validationData = _coalescePreValidation(validationData, currentValidationData); } @@ -466,7 +467,6 @@ contract UpgradeableModularAccount is if (i + 1 < chainedValidations.length) { (signatureSegment, outerSignature) = outerSignature.getNextSegment(); } - } return validationData; diff --git a/src/plugins/owner/ComposableMultisigPlugin.sol b/src/plugins/owner/ComposableMultisigPlugin.sol index c477d23c..ccbe8cf5 100644 --- a/src/plugins/owner/ComposableMultisigPlugin.sol +++ b/src/plugins/owner/ComposableMultisigPlugin.sol @@ -6,33 +6,29 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import {FunctionReference} from "../../helpers/FunctionReferenceLib.sol"; import {IPlugin} from "../../interfaces/IPlugin.sol"; import {IValidation} from "../../interfaces/IValidation.sol"; -import {BasePlugin, IERC165} from "../BasePlugin.sol"; -import { - PluginManifest, - PluginMetadata -} from "../../interfaces/IPlugin.sol"; +import {BasePlugin} from "../BasePlugin.sol"; +import {PluginManifest, PluginMetadata} from "../../interfaces/IPlugin.sol"; // Non-threshold based multisig plugin - all owners must sign. // Supports up to 100 owners per id. contract ComposableMultisigPlugin is IValidation, BasePlugin { + struct OwnerInfo { + uint256 length; + FunctionReference[100] validations; + } uint256 internal constant _SIG_VALIDATION_PASSED = 0; uint256 internal constant _SIG_VALIDATION_FAILED = 1; + mapping(uint8 id => mapping(address account => OwnerInfo)) public ownerInfo; + error AlreadyInitialized(); error NotAuthorized(); error NotInitialized(); error InvalidOwners(); - struct OwnerInfo { - uint256 length; - FunctionReference[100] validations; - } - - mapping(uint8 id => mapping(address account => OwnerInfo)) public ownerInfo; - /// @inheritdoc IPlugin - function onInstall(bytes calldata data) external override { + function onInstall(bytes calldata data) external override { uint8 id = uint8(bytes1(data[:1])); if (ownerInfo[id][msg.sender].length != 0) { @@ -57,7 +53,7 @@ contract ComposableMultisigPlugin is IValidation, BasePlugin { uint8 id = uint8(bytes1(data[:1])); uint256 length = ownerInfo[id][msg.sender].length; - + if (length == 0) { revert NotInitialized(); } @@ -69,15 +65,6 @@ contract ComposableMultisigPlugin is IValidation, BasePlugin { ownerInfo[id][msg.sender].length = 0; } - /// @inheritdoc IValidation - function validateRuntime(uint8, address, uint256, bytes calldata, bytes calldata) - external - pure - override - { - revert NotImplemented(); - } - /// @inheritdoc IValidation function validateUserOp(uint8 functionId, PackedUserOperation calldata, bytes32) external @@ -100,6 +87,11 @@ contract ComposableMultisigPlugin is IValidation, BasePlugin { return (_SIG_VALIDATION_PASSED, abi.encode(validations)); } + /// @inheritdoc IValidation + function validateRuntime(uint8, address, uint256, bytes calldata, bytes calldata) external pure override { + revert NotImplemented(); + } + // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Execution view functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ @@ -111,18 +103,15 @@ contract ComposableMultisigPlugin is IValidation, BasePlugin { /// 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 validateSignature(uint8, address, bytes32, bytes calldata) - external - pure - override - returns (bytes4) - { + function validateSignature(uint8, address, bytes32, bytes calldata) external pure override returns (bytes4) { revert NotImplemented(); } /// @inheritdoc IPlugin + // solhint-disable-next-line no-empty-blocks function pluginManifest() external pure override returns (PluginManifest memory) {} /// @inheritdoc IPlugin + // solhint-disable-next-line no-empty-blocks function pluginMetadata() external pure virtual override returns (PluginMetadata memory) {} -} \ No newline at end of file +} diff --git a/src/plugins/owner/ECDSAValidationPlugin.sol b/src/plugins/owner/ECDSAValidationPlugin.sol index 64b6607e..988338c8 100644 --- a/src/plugins/owner/ECDSAValidationPlugin.sol +++ b/src/plugins/owner/ECDSAValidationPlugin.sol @@ -2,17 +2,13 @@ pragma solidity ^0.8.25; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol"; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {IPlugin} from "../../interfaces/IPlugin.sol"; import {IValidation} from "../../interfaces/IValidation.sol"; -import {BasePlugin, IERC165} from "../BasePlugin.sol"; -import { - PluginManifest, - PluginMetadata -} from "../../interfaces/IPlugin.sol"; +import {BasePlugin} from "../BasePlugin.sol"; +import {PluginManifest, PluginMetadata} from "../../interfaces/IPlugin.sol"; contract ECDSAValidationPlugin is IValidation, BasePlugin { using ECDSA for bytes32; @@ -25,14 +21,14 @@ contract ECDSAValidationPlugin is IValidation, BasePlugin { bytes4 internal constant _1271_MAGIC_VALUE = 0x1626ba7e; bytes4 internal constant _1271_INVALID = 0xffffffff; + mapping(uint8 id => mapping(address account => address)) public owners; + error AlreadyInitialized(); error NotAuthorized(); error NotInitialized(); - mapping(uint8 id => mapping(address account => address)) public owners; - /// @inheritdoc IPlugin - function onInstall(bytes calldata data) external override { + function onInstall(bytes calldata data) external override { uint8 id = uint8(bytes1(data[:1])); if (owners[id][msg.sender] != address(0)) { @@ -106,9 +102,10 @@ contract ECDSAValidationPlugin is IValidation, BasePlugin { } /// @inheritdoc IPlugin + // solhint-disable-next-line no-empty-blocks function pluginManifest() external pure override returns (PluginManifest memory) {} /// @inheritdoc IPlugin + // solhint-disable-next-line no-empty-blocks function pluginMetadata() external pure virtual override returns (PluginMetadata memory) {} - -} \ No newline at end of file +} diff --git a/test/account/ComposableValidation.t.sol b/test/account/ComposableValidation.t.sol index d49acd2d..f9f9b615 100644 --- a/test/account/ComposableValidation.t.sol +++ b/test/account/ComposableValidation.t.sol @@ -15,11 +15,10 @@ import {CustomValidationTestBase} from "../utils/CustomValidationTestBase.sol"; contract ComposableValidationTest is CustomValidationTestBase { using MessageHashUtils for bytes32; - ECDSAValidationPlugin ecdsaValidationPlugin; - ComposableMultisigPlugin composableMultisigPlugin; + ECDSAValidationPlugin public ecdsaValidationPlugin; + ComposableMultisigPlugin public composableMultisigPlugin; function setUp() public { - ecdsaValidationPlugin = new ECDSAValidationPlugin(); composableMultisigPlugin = new ComposableMultisigPlugin(); @@ -27,7 +26,6 @@ contract ComposableValidationTest is CustomValidationTestBase { } function test_basicUserOp_withECDSAValidation() public { - _customValidationSetup(); // Now that the account is set up with the ECDSAValidationPlugin, we can test the basic user op @@ -46,11 +44,7 @@ contract ComposableValidationTest is CustomValidationTestBase { bytes32 userOpHash = entryPoint.getUserOpHash(userOp); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); - userOp.signature = _encodeSignature( - _ownerValidation, - DEFAULT_VALIDATION, - abi.encodePacked(r, s, v) - ); + userOp.signature = _encodeSignature(_ownerValidation, DEFAULT_VALIDATION, abi.encodePacked(r, s, v)); PackedUserOperation[] memory userOps = new PackedUserOperation[](1); userOps[0] = userOp; @@ -65,8 +59,9 @@ contract ComposableValidationTest is CustomValidationTestBase { _customValidationSetup(); // Install the multisig plugin with signers 2 and 3 - - FunctionReference composableMultisigValidation = FunctionReferenceLib.pack(address(composableMultisigPlugin), uint8(0)); + + FunctionReference composableMultisigValidation = + FunctionReferenceLib.pack(address(composableMultisigPlugin), uint8(0)); FunctionReference owner2Validation = FunctionReferenceLib.pack(address(ecdsaValidationPlugin), uint8(2)); FunctionReference owner3Validation = FunctionReferenceLib.pack(address(ecdsaValidationPlugin), uint8(3)); @@ -76,12 +71,36 @@ contract ComposableValidationTest is CustomValidationTestBase { // Set up the ComposableMultisigPlugin Call[] memory calls = new Call[](3); - calls[0] = Call(address(ecdsaValidationPlugin), 0, abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(2), abi.encode(owner2))))); - calls[1] = Call(address(ecdsaValidationPlugin), 0, abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(3), abi.encode(owner3))))); - calls[2] = Call(address(account1), 0, abi.encodeCall(UpgradeableModularAccount.installValidation, (composableMultisigValidation, true, new bytes4[](0), abi.encodePacked(uint8(0), abi.encode(multisigSigners)), ""))); + calls[0] = Call( + address(ecdsaValidationPlugin), + 0, + abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(2), abi.encode(owner2)))) + ); + calls[1] = Call( + address(ecdsaValidationPlugin), + 0, + abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(3), abi.encode(owner3)))) + ); + calls[2] = Call( + address(account1), + 0, + abi.encodeCall( + UpgradeableModularAccount.installValidation, + ( + composableMultisigValidation, + true, + new bytes4[](0), + abi.encodePacked(uint8(0), abi.encode(multisigSigners)), + "" + ) + ) + ); vm.prank(owner1); - account1.executeWithAuthorization(abi.encodeCall(IStandardExecutor.executeBatch, (calls)), _encodeSignature(_ownerValidation, DEFAULT_VALIDATION, "")); + account1.executeWithAuthorization( + abi.encodeCall(IStandardExecutor.executeBatch, (calls)), + _encodeSignature(_ownerValidation, DEFAULT_VALIDATION, "") + ); // test the multisig validation @@ -108,7 +127,10 @@ contract ComposableValidationTest is CustomValidationTestBase { userOp.signature = _encodeSignature( composableMultisigValidation, DEFAULT_VALIDATION, - abi.encodePacked(_packValidationDataWithIndex(0, _packValidationDataWithIndex(0xff, owner2Signature)), _packValidationDataWithIndex(1, _packValidationDataWithIndex(0xff, owner3Signature))) + abi.encodePacked( + _packValidationDataWithIndex(0, _packValidationDataWithIndex(0xff, owner2Signature)), + _packValidationDataWithIndex(1, _packValidationDataWithIndex(0xff, owner3Signature)) + ) ); PackedUserOperation[] memory userOps = new PackedUserOperation[](1); @@ -135,7 +157,7 @@ contract ComposableValidationTest is CustomValidationTestBase { // 4 = owner4Validation FunctionReference[5] memory validations; - + validations[0] = FunctionReferenceLib.pack(address(composableMultisigPlugin), uint8(0)); validations[1] = FunctionReferenceLib.pack(address(ecdsaValidationPlugin), uint8(2)); validations[2] = FunctionReferenceLib.pack(address(composableMultisigPlugin), uint8(1)); @@ -152,14 +174,48 @@ contract ComposableValidationTest is CustomValidationTestBase { // Set up the ComposableMultisigPlugin Call[] memory calls = new Call[](5); - calls[0] = Call(address(ecdsaValidationPlugin), 0, abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(2), abi.encode(owner2))))); - calls[1] = Call(address(ecdsaValidationPlugin), 0, abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(3), abi.encode(owner3))))); - calls[2] = Call(address(ecdsaValidationPlugin), 0, abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(4), abi.encode(owner4))))); - calls[3] = Call(address(composableMultisigPlugin), 0, abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(1), abi.encode(innerMultisigSigners))))); - calls[4] = Call(address(account1), 0, abi.encodeCall(UpgradeableModularAccount.installValidation, (validations[0], true, new bytes4[](0), abi.encodePacked(uint8(0), abi.encode(outerMultisigSigners)), ""))); + calls[0] = Call( + address(ecdsaValidationPlugin), + 0, + abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(2), abi.encode(owner2)))) + ); + calls[1] = Call( + address(ecdsaValidationPlugin), + 0, + abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(3), abi.encode(owner3)))) + ); + calls[2] = Call( + address(ecdsaValidationPlugin), + 0, + abi.encodeCall(ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(4), abi.encode(owner4)))) + ); + calls[3] = Call( + address(composableMultisigPlugin), + 0, + abi.encodeCall( + ECDSAValidationPlugin.onInstall, (abi.encodePacked(uint8(1), abi.encode(innerMultisigSigners))) + ) + ); + calls[4] = Call( + address(account1), + 0, + abi.encodeCall( + UpgradeableModularAccount.installValidation, + ( + validations[0], + true, + new bytes4[](0), + abi.encodePacked(uint8(0), abi.encode(outerMultisigSigners)), + "" + ) + ) + ); vm.prank(owner1); - account1.executeWithAuthorization(abi.encodeCall(IStandardExecutor.executeBatch, (calls)), _encodeSignature(_ownerValidation, DEFAULT_VALIDATION, "")); + account1.executeWithAuthorization( + abi.encodeCall(IStandardExecutor.executeBatch, (calls)), + _encodeSignature(_ownerValidation, DEFAULT_VALIDATION, "") + ); // test the multisig of multisigs validation @@ -189,10 +245,19 @@ contract ComposableValidationTest is CustomValidationTestBase { userOp.signature = _encodeSignature( validations[0], DEFAULT_VALIDATION, - abi.encodePacked(_packValidationDataWithIndex(0, _packValidationDataWithIndex(0xff, owner2Signature)), _packValidationDataWithIndex(1, _packValidationDataWithIndex(0xff, abi.encodePacked( - _packValidationDataWithIndex(0, _packValidationDataWithIndex(0xff, owner3Signature)), - _packValidationDataWithIndex(1, _packValidationDataWithIndex(0xff, owner4Signature)) - )))) + abi.encodePacked( + _packValidationDataWithIndex(0, _packValidationDataWithIndex(0xff, owner2Signature)), + _packValidationDataWithIndex( + 1, + _packValidationDataWithIndex( + 0xff, + abi.encodePacked( + _packValidationDataWithIndex(0, _packValidationDataWithIndex(0xff, owner3Signature)), + _packValidationDataWithIndex(1, _packValidationDataWithIndex(0xff, owner4Signature)) + ) + ) + ) + ) ); PackedUserOperation[] memory userOps = new PackedUserOperation[](1); @@ -207,6 +272,12 @@ contract ComposableValidationTest is CustomValidationTestBase { override returns (FunctionReference, bool, bytes4[] memory, bytes memory, bytes memory) { - return (_ownerValidation, true, new bytes4[](0), abi.encodePacked(uint8(123), abi.encode(owner1)), abi.encodePacked("")); + return ( + _ownerValidation, + true, + new bytes4[](0), + abi.encodePacked(uint8(123), abi.encode(owner1)), + abi.encodePacked("") + ); } }