diff --git a/src/account/PluginManager2.sol b/src/account/PluginManager2.sol index 2c9c4d6a..28eb0ecf 100644 --- a/src/account/PluginManager2.sol +++ b/src/account/PluginManager2.sol @@ -4,19 +4,20 @@ pragma solidity ^0.8.25; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {IPlugin} from "../interfaces/IPlugin.sol"; -import {FunctionReference} from "../interfaces/IPluginManager.sol"; +import {FunctionReference, ValidationConfig} from "../interfaces/IPluginManager.sol"; import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol"; -import {AccountStorage, getAccountStorage, toSetValue, toFunctionReference} from "./AccountStorage.sol"; +import {ValidationConfigLib} from "../helpers/ValidationConfigLib.sol"; +import {ValidationData, getAccountStorage, toSetValue, toFunctionReference} from "./AccountStorage.sol"; import {ExecutionHook} from "../interfaces/IAccountLoupe.sol"; // Temporary additional functions for a user-controlled install flow for validation functions. abstract contract PluginManager2 { using EnumerableSet for EnumerableSet.Bytes32Set; + using ValidationConfigLib for ValidationConfig; // Index marking the start of the data for the validation function. uint8 internal constant _RESERVED_VALIDATION_DATA_INDEX = 255; - error GlobalValidationAlreadySet(FunctionReference validationFunction); error PreValidationAlreadySet(FunctionReference validationFunction, FunctionReference preValidationFunction); error ValidationAlreadySet(bytes4 selector, FunctionReference validationFunction); error ValidationNotSet(bytes4 selector, FunctionReference validationFunction); @@ -24,17 +25,14 @@ abstract contract PluginManager2 { error PreValidationHookLimitExceeded(); function _installValidation( - FunctionReference validationFunction, - bool isGlobal, + ValidationConfig validationConfig, bytes4[] memory selectors, bytes calldata installData, bytes memory preValidationHooks, bytes memory permissionHooks - ) - // TODO: flag for signature validation - internal - { - AccountStorage storage _storage = getAccountStorage(); + ) internal { + ValidationData storage _validationData = + getAccountStorage().validationData[validationConfig.functionReference()]; if (preValidationHooks.length > 0) { (FunctionReference[] memory preValidationFunctions, bytes[] memory initDatas) = @@ -43,7 +41,7 @@ abstract contract PluginManager2 { for (uint256 i = 0; i < preValidationFunctions.length; ++i) { FunctionReference preValidationFunction = preValidationFunctions[i]; - _storage.validationData[validationFunction].preValidationHooks.push(preValidationFunction); + _validationData.preValidationHooks.push(preValidationFunction); if (initDatas[i].length > 0) { (address preValidationPlugin,) = FunctionReferenceLib.unpack(preValidationFunction); @@ -52,10 +50,7 @@ abstract contract PluginManager2 { } // Avoid collision between reserved index and actual indices - if ( - _storage.validationData[validationFunction].preValidationHooks.length - > _RESERVED_VALIDATION_DATA_INDEX - ) { + if (_validationData.preValidationHooks.length > _RESERVED_VALIDATION_DATA_INDEX) { revert PreValidationHookLimitExceeded(); } } @@ -67,10 +62,8 @@ abstract contract PluginManager2 { for (uint256 i = 0; i < permissionFunctions.length; ++i) { ExecutionHook memory permissionFunction = permissionFunctions[i]; - if ( - !_storage.validationData[validationFunction].permissionHooks.add(toSetValue(permissionFunction)) - ) { - revert PermissionAlreadySet(validationFunction, permissionFunction); + if (!_validationData.permissionHooks.add(toSetValue(permissionFunction))) { + revert PermissionAlreadySet(validationConfig.functionReference(), permissionFunction); } if (initDatas[i].length > 0) { @@ -80,22 +73,18 @@ abstract contract PluginManager2 { } } - if (isGlobal) { - if (_storage.validationData[validationFunction].isGlobal) { - revert GlobalValidationAlreadySet(validationFunction); - } - _storage.validationData[validationFunction].isGlobal = true; - } + _validationData.isGlobal = validationConfig.isGlobal(); + _validationData.isSignatureValidation = validationConfig.isSignatureValidation(); for (uint256 i = 0; i < selectors.length; ++i) { bytes4 selector = selectors[i]; - if (!_storage.validationData[validationFunction].selectors.add(toSetValue(selector))) { - revert ValidationAlreadySet(selector, validationFunction); + if (!_validationData.selectors.add(toSetValue(selector))) { + revert ValidationAlreadySet(selector, validationConfig.functionReference()); } } if (installData.length > 0) { - (address plugin,) = FunctionReferenceLib.unpack(validationFunction); + address plugin = validationConfig.plugin(); IPlugin(plugin).onInstall(installData); } } @@ -106,17 +95,16 @@ abstract contract PluginManager2 { bytes calldata preValidationHookUninstallData, bytes calldata permissionHookUninstallData ) internal { - AccountStorage storage _storage = getAccountStorage(); + ValidationData storage _validationData = getAccountStorage().validationData[validationFunction]; - _storage.validationData[validationFunction].isGlobal = false; - _storage.validationData[validationFunction].isSignatureValidation = false; + _validationData.isGlobal = false; + _validationData.isSignatureValidation = false; { bytes[] memory preValidationHookUninstallDatas = abi.decode(preValidationHookUninstallData, (bytes[])); // Clear pre validation hooks - FunctionReference[] storage preValidationHooks = - _storage.validationData[validationFunction].preValidationHooks; + FunctionReference[] storage preValidationHooks = _validationData.preValidationHooks; for (uint256 i = 0; i < preValidationHooks.length; ++i) { FunctionReference preValidationFunction = preValidationHooks[i]; if (preValidationHookUninstallDatas[0].length > 0) { @@ -124,15 +112,14 @@ abstract contract PluginManager2 { IPlugin(preValidationPlugin).onUninstall(preValidationHookUninstallDatas[0]); } } - delete _storage.validationData[validationFunction].preValidationHooks; + delete _validationData.preValidationHooks; } { bytes[] memory permissionHookUninstallDatas = abi.decode(permissionHookUninstallData, (bytes[])); // Clear permission hooks - EnumerableSet.Bytes32Set storage permissionHooks = - _storage.validationData[validationFunction].permissionHooks; + EnumerableSet.Bytes32Set storage permissionHooks = _validationData.permissionHooks; uint256 i = 0; while (permissionHooks.length() > 0) { FunctionReference permissionHook = toFunctionReference(permissionHooks.at(0)); @@ -141,12 +128,12 @@ abstract contract PluginManager2 { IPlugin(permissionHookPlugin).onUninstall(permissionHookUninstallDatas[i++]); } } - delete _storage.validationData[validationFunction].preValidationHooks; + delete _validationData.preValidationHooks; // Clear selectors - while (_storage.validationData[validationFunction].selectors.length() > 0) { - bytes32 selector = _storage.validationData[validationFunction].selectors.at(0); - _storage.validationData[validationFunction].selectors.remove(selector); + while (_validationData.selectors.length() > 0) { + bytes32 selector = _validationData.selectors.at(0); + _validationData.selectors.remove(selector); } if (uninstallData.length > 0) { diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index 42db6967..d2d1b2cc 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -11,13 +11,14 @@ import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol"; +import {ValidationConfigLib} from "../helpers/ValidationConfigLib.sol"; import {SparseCalldataSegmentLib} from "../helpers/SparseCalldataSegmentLib.sol"; import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationDataHelpers.sol"; import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol"; import {IValidation} from "../interfaces/IValidation.sol"; import {IValidationHook} from "../interfaces/IValidationHook.sol"; import {IExecutionHook} from "../interfaces/IExecutionHook.sol"; -import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol"; +import {FunctionReference, IPluginManager, ValidationConfig} from "../interfaces/IPluginManager.sol"; import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol"; import {AccountExecutor} from "./AccountExecutor.sol"; import {AccountLoupe} from "./AccountLoupe.sol"; @@ -41,6 +42,7 @@ contract UpgradeableModularAccount is { using EnumerableSet for EnumerableSet.Bytes32Set; using FunctionReferenceLib for FunctionReference; + using ValidationConfigLib for ValidationConfig; using SparseCalldataSegmentLib for bytes; struct PostExecToRun { @@ -274,32 +276,26 @@ contract UpgradeableModularAccount is /// with user install configs. /// @dev This function is only callable once, and only by the EntryPoint. function initializeWithValidation( - FunctionReference validationFunction, - bool isGlobal, + ValidationConfig validationConfig, bytes4[] memory selectors, bytes calldata installData, bytes calldata preValidationHooks, bytes calldata permissionHooks ) external initializer { - _installValidation( - validationFunction, isGlobal, selectors, installData, preValidationHooks, permissionHooks - ); + _installValidation(validationConfig, selectors, installData, preValidationHooks, permissionHooks); emit ModularAccountInitialized(_ENTRY_POINT); } /// @inheritdoc IPluginManager /// @notice May be validated by a global validation. function installValidation( - FunctionReference validationFunction, - bool isGlobal, + ValidationConfig validationConfig, bytes4[] memory selectors, bytes calldata installData, bytes calldata preValidationHooks, bytes calldata permissionHooks ) external wrapNativeFunction { - _installValidation( - validationFunction, isGlobal, selectors, installData, preValidationHooks, permissionHooks - ); + _installValidation(validationConfig, selectors, installData, preValidationHooks, permissionHooks); } /// @inheritdoc IPluginManager diff --git a/src/helpers/ValidationConfigLib.sol b/src/helpers/ValidationConfigLib.sol new file mode 100644 index 00000000..71639f80 --- /dev/null +++ b/src/helpers/ValidationConfigLib.sol @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.25; + +import {FunctionReference, ValidationConfig} from "../interfaces/IPluginManager.sol"; + +// Validation config is a packed representation of a validation function and flags for its configuration. +// Layout: +// 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address +// 0x________________________________________BB______________________ // Function ID +// 0x__________________________________________CC____________________ // isGlobal +// 0x____________________________________________DD__________________ // isSignatureValidation +// 0x______________________________________________000000000000000000 // unused + +library ValidationConfigLib { + function pack(FunctionReference _validationFunction, bool _isGlobal, bool _isSignatureValidation) + internal + pure + returns (ValidationConfig) + { + return ValidationConfig.wrap( + bytes23( + bytes23(FunctionReference.unwrap(_validationFunction)) + // isGlobal flag stored in the 22nd byte + | bytes23(bytes32(_isGlobal ? uint256(1) << 80 : 0)) + // isSignatureValidation flag stored in the 23rd byte + | bytes23(bytes32(_isSignatureValidation ? uint256(1) << 72 : 0)) + ) + ); + } + + function pack(address _plugin, uint8 _functionId, bool _isGlobal, bool _isSignatureValidation) + internal + pure + returns (ValidationConfig) + { + return ValidationConfig.wrap( + bytes23( + // plugin address stored in the first 20 bytes + bytes23(bytes20(_plugin)) + // functionId stored in the 21st byte + | bytes23(bytes32(uint256(_functionId) << 168)) + // isGlobal flag stored in the 22nd byte + | bytes23(bytes32(_isGlobal ? uint256(1) << 80 : 0)) + // isSignatureValidation flag stored in the 23rd byte + | bytes23(bytes32(_isSignatureValidation ? uint256(1) << 72 : 0)) + ) + ); + } + + function unpackUnderlying(ValidationConfig config) + internal + pure + returns (address _plugin, uint8 _functionId, bool _isGlobal, bool _isSignatureValidation) + { + bytes23 configBytes = ValidationConfig.unwrap(config); + _plugin = address(bytes20(configBytes)); + _functionId = uint8(configBytes[20]); + _isGlobal = uint8(configBytes[21]) == 1; + _isSignatureValidation = uint8(configBytes[22]) == 1; + } + + function unpack(ValidationConfig config) + internal + pure + returns (FunctionReference _validationFunction, bool _isGlobal, bool _isSignatureValidation) + { + bytes23 configBytes = ValidationConfig.unwrap(config); + _validationFunction = FunctionReference.wrap(bytes21(configBytes)); + _isGlobal = uint8(configBytes[21]) == 1; + _isSignatureValidation = uint8(configBytes[22]) == 1; + } + + function plugin(ValidationConfig config) internal pure returns (address) { + return address(bytes20(ValidationConfig.unwrap(config))); + } + + function functionId(ValidationConfig config) internal pure returns (uint8) { + return uint8(ValidationConfig.unwrap(config)[20]); + } + + function functionReference(ValidationConfig config) internal pure returns (FunctionReference) { + return FunctionReference.wrap(bytes21(ValidationConfig.unwrap(config))); + } + + function isGlobal(ValidationConfig config) internal pure returns (bool) { + return uint8(ValidationConfig.unwrap(config)[21]) == 1; + } + + function isSignatureValidation(ValidationConfig config) internal pure returns (bool) { + return uint8(ValidationConfig.unwrap(config)[22]) == 1; + } +} diff --git a/src/interfaces/IPluginManager.sol b/src/interfaces/IPluginManager.sol index d73da718..bf1296e1 100644 --- a/src/interfaces/IPluginManager.sol +++ b/src/interfaces/IPluginManager.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.25; type FunctionReference is bytes21; +type ValidationConfig is bytes23; + interface IPluginManager { event PluginInstalled(address indexed plugin, bytes32 manifestHash); @@ -21,15 +23,13 @@ interface IPluginManager { /// validation. /// TODO: remove or update. /// @dev This does not validate anything against the manifest - the caller must ensure validity. - /// @param validationFunction The validation function to install. - /// @param isGlobal Whether the validation function applies for all selectors in the global pool. + /// @param validationConfig The validation function to install, along with configuration flags. /// @param selectors The selectors to install the validation function for. /// @param installData Optional data to be decoded and used by the plugin to setup initial plugin state. /// @param preValidationHooks Optional pre-validation hooks to install for the validation function. /// @param permissionHooks Optional permission hooks to install for the validation function. function installValidation( - FunctionReference validationFunction, - bool isGlobal, + ValidationConfig validationConfig, bytes4[] memory selectors, bytes calldata installData, bytes calldata preValidationHooks, diff --git a/src/plugins/owner/ISingleOwnerPlugin.sol b/src/plugins/owner/ISingleOwnerPlugin.sol deleted file mode 100644 index c8f0df43..00000000 --- a/src/plugins/owner/ISingleOwnerPlugin.sol +++ /dev/null @@ -1,36 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0 -pragma solidity ^0.8.25; - -import {IValidation} from "../../interfaces/IValidation.sol"; - -interface ISingleOwnerPlugin is IValidation { - enum FunctionId { - VALIDATION_OWNER - } - - /// @notice This event is emitted when ownership of the account changes. - /// @param account The account whose ownership changed. - /// @param previousOwner The address of the previous owner. - /// @param newOwner The address of the new owner. - event OwnershipTransferred(address indexed account, address indexed previousOwner, address indexed newOwner); - - error NotAuthorized(); - - /// @notice Transfer ownership of the account to `newOwner`. - /// @dev This function is installed on the account as part of plugin installation, and should - /// only be called from an account. - /// @param newOwner The address of the new owner. - function transferOwnership(address newOwner) external; - - /// @notice Get the owner of the account. - /// @dev This function is installed on the account as part of plugin installation, and should - /// only be called from an account. - /// @return The address of the owner. - function owner() external view returns (address); - - /// @notice Get the owner of `account`. - /// @dev This function is not installed on the account, and can be called by anyone. - /// @param account The account to get the owner of. - /// @return The address of the owner. - function ownerOf(address account) external view returns (address); -} diff --git a/src/plugins/owner/SingleOwnerPlugin.sol b/src/plugins/owner/SingleOwnerPlugin.sol index 779d3839..dbb5d56a 100644 --- a/src/plugins/owner/SingleOwnerPlugin.sol +++ b/src/plugins/owner/SingleOwnerPlugin.sol @@ -6,16 +6,10 @@ import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/Signa import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; -import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; -import {IPluginManager} from "../../interfaces/IPluginManager.sol"; -import { - PluginManifest, ManifestValidation, PluginMetadata, SelectorPermission -} from "../../interfaces/IPlugin.sol"; -import {IStandardExecutor} from "../../interfaces/IStandardExecutor.sol"; +import {PluginManifest, PluginMetadata, SelectorPermission} from "../../interfaces/IPlugin.sol"; import {IPlugin} from "../../interfaces/IPlugin.sol"; import {IValidation} from "../../interfaces/IValidation.sol"; import {BasePlugin, IERC165} from "../BasePlugin.sol"; -import {ISingleOwnerPlugin} from "./ISingleOwnerPlugin.sol"; /// @title Single Owner Plugin /// @author ERC-6900 Authors @@ -33,13 +27,13 @@ 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 ISingleOwnerPlugin, BasePlugin { +contract SingleOwnerPlugin is IValidation, BasePlugin { using ECDSA for bytes32; using MessageHashUtils for bytes32; - string public constant NAME = "Single Owner Plugin"; - string public constant VERSION = "1.0.0"; - string public constant AUTHOR = "ERC-6900 Authors"; + string internal constant _NAME = "Single Owner Plugin"; + string internal constant _VERSION = "1.0.0"; + string internal constant _AUTHOR = "ERC-6900 Authors"; uint256 internal constant _SIG_VALIDATION_PASSED = 0; uint256 internal constant _SIG_VALIDATION_FAILED = 1; @@ -48,15 +42,27 @@ contract SingleOwnerPlugin is ISingleOwnerPlugin, BasePlugin { bytes4 internal constant _1271_MAGIC_VALUE = 0x1626ba7e; bytes4 internal constant _1271_INVALID = 0xffffffff; - mapping(address => address) internal _owners; + mapping(uint8 id => mapping(address account => address)) public owners; + + /// @notice This event is emitted when ownership of the account changes. + /// @param account The account whose ownership changed. + /// @param previousOwner The address of the previous owner. + /// @param newOwner The address of the new owner. + event OwnershipTransferred(address indexed account, address indexed previousOwner, address indexed newOwner); + + error AlreadyInitialized(); + error NotAuthorized(); + error NotInitialized(); // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Execution functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - /// @inheritdoc ISingleOwnerPlugin - function transferOwnership(address newOwner) external { - _transferOwnership(newOwner); + /// @notice Transfer ownership of an account and ID to `newOwner`. The caller address (`msg.sender`) is user to + /// identify the account. + /// @param newOwner The address of the new owner. + function transferOwnership(uint8 id, address newOwner) external { + _transferOwnership(id, newOwner); } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -65,12 +71,14 @@ contract SingleOwnerPlugin is ISingleOwnerPlugin, BasePlugin { /// @inheritdoc IPlugin function onInstall(bytes calldata data) external override { - _transferOwnership(abi.decode(data, (address))); + (uint8 id, address owner) = abi.decode(data, (uint8, address)); + _transferOwnership(id, owner); } /// @inheritdoc IPlugin - function onUninstall(bytes calldata) external override { - _transferOwnership(address(0)); + function onUninstall(bytes calldata data) external override { + uint8 id = abi.decode(data, (uint8)); + _transferOwnership(id, address(0)); } /// @inheritdoc IValidation @@ -79,14 +87,11 @@ contract SingleOwnerPlugin is ISingleOwnerPlugin, BasePlugin { view override { - if (functionId == uint8(FunctionId.VALIDATION_OWNER)) { - // Validate that the sender is the owner of the account or self. - if (sender != _owners[msg.sender] && sender != msg.sender) { - revert NotAuthorized(); - } - return; + // Validate that the sender is the owner of the account or self. + if (sender != owners[functionId][msg.sender]) { + revert NotAuthorized(); } - revert NotImplemented(); + return; } /// @inheritdoc IValidation @@ -96,15 +101,15 @@ contract SingleOwnerPlugin is ISingleOwnerPlugin, BasePlugin { override returns (uint256) { - if (functionId == uint8(FunctionId.VALIDATION_OWNER)) { - // Validate the user op signature against the owner. - (address signer,,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); - if (signer == address(0) || signer != _owners[msg.sender]) { - return _SIG_VALIDATION_FAILED; - } + // Validate the user op signature against the owner. + if ( + SignatureChecker.isValidSignatureNow( + owners[functionId][msg.sender], userOpHash.toEthSignedMessageHash(), userOp.signature + ) + ) { return _SIG_VALIDATION_PASSED; } - revert NotImplemented(); + return _SIG_VALIDATION_FAILED; } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -124,60 +129,28 @@ contract SingleOwnerPlugin is ISingleOwnerPlugin, BasePlugin { override returns (bytes4) { - if (functionId == uint8(FunctionId.VALIDATION_OWNER)) { - if (SignatureChecker.isValidSignatureNow(_owners[msg.sender], digest, signature)) { - return _1271_MAGIC_VALUE; - } - return _1271_INVALID; + if (SignatureChecker.isValidSignatureNow(owners[functionId][msg.sender], digest, signature)) { + return _1271_MAGIC_VALUE; } - revert NotImplemented(); - } - - /// @inheritdoc ISingleOwnerPlugin - function owner() external view returns (address) { - return _owners[msg.sender]; + return _1271_INVALID; } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Plugin view functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - /// @inheritdoc ISingleOwnerPlugin - function ownerOf(address account) external view returns (address) { - return _owners[account]; - } - /// @inheritdoc IPlugin function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - - // TODO: use global validation instead - bytes4[] memory accountSelectors = new bytes4[](5); - accountSelectors[0] = IStandardExecutor.execute.selector; - accountSelectors[1] = IStandardExecutor.executeBatch.selector; - accountSelectors[2] = IPluginManager.installPlugin.selector; - accountSelectors[3] = IPluginManager.uninstallPlugin.selector; - accountSelectors[4] = UUPSUpgradeable.upgradeToAndCall.selector; - - ManifestValidation memory ownerValidationFunction = ManifestValidation({ - functionId: uint8(FunctionId.VALIDATION_OWNER), - isDefault: false, - isSignatureValidation: true, - selectors: accountSelectors - }); - - manifest.validationFunctions = new ManifestValidation[](1); - manifest.validationFunctions[0] = ownerValidationFunction; - return manifest; } /// @inheritdoc IPlugin function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { PluginMetadata memory metadata; - metadata.name = NAME; - metadata.version = VERSION; - metadata.author = AUTHOR; + metadata.name = _NAME; + metadata.version = _VERSION; + metadata.author = _AUTHOR; // Permission strings string memory modifyOwnershipPermission = "Modify Ownership"; @@ -198,16 +171,18 @@ contract SingleOwnerPlugin is ISingleOwnerPlugin, BasePlugin { /// @inheritdoc BasePlugin function supportsInterface(bytes4 interfaceId) public view override(BasePlugin, IERC165) returns (bool) { - return interfaceId == type(ISingleOwnerPlugin).interfaceId || super.supportsInterface(interfaceId); + return interfaceId == type(IValidation).interfaceId || super.supportsInterface(interfaceId); } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Internal / Private functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - function _transferOwnership(address newOwner) internal { - address previousOwner = _owners[msg.sender]; - _owners[msg.sender] = newOwner; + // Transfers ownership and emits and event. + function _transferOwnership(uint8 id, address newOwner) internal { + address previousOwner = owners[id][msg.sender]; + owners[id][msg.sender] = newOwner; + // Todo: include id in event emit OwnershipTransferred(msg.sender, previousOwner, newOwner); } } diff --git a/test/account/AccountLoupe.t.sol b/test/account/AccountLoupe.t.sol index 15803fa8..69563b51 100644 --- a/test/account/AccountLoupe.t.sol +++ b/test/account/AccountLoupe.t.sol @@ -9,49 +9,29 @@ import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; import {IStandardExecutor} from "../../src/interfaces/IStandardExecutor.sol"; import {ComprehensivePlugin} from "../mocks/plugins/ComprehensivePlugin.sol"; -import {AccountTestBase} from "../utils/AccountTestBase.sol"; +import {CustomValidationTestBase} from "../utils/CustomValidationTestBase.sol"; -contract AccountLoupeTest is AccountTestBase { +contract AccountLoupeTest is CustomValidationTestBase { ComprehensivePlugin public comprehensivePlugin; event ReceivedCall(bytes msgData, uint256 msgValue); function setUp() public { - _transferOwnershipToTest(); - comprehensivePlugin = new ComprehensivePlugin(); + _customValidationSetup(); + bytes32 manifestHash = keccak256(abi.encode(comprehensivePlugin.pluginManifest())); vm.prank(address(entryPoint)); account1.installPlugin(address(comprehensivePlugin), manifestHash, ""); - - FunctionReference[] memory preValidationHooks = new FunctionReference[](2); - preValidationHooks[0] = FunctionReferenceLib.pack( - address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.PRE_VALIDATION_HOOK_1) - ); - preValidationHooks[1] = FunctionReferenceLib.pack( - address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.PRE_VALIDATION_HOOK_2) - ); - - bytes[] memory installDatas = new bytes[](2); - vm.prank(address(entryPoint)); - account1.installValidation( - _ownerValidation, - true, - new bytes4[](0), - bytes(""), - abi.encode(preValidationHooks, installDatas), - bytes("") - ); } function test_pluginLoupe_getInstalledPlugins_initial() public { address[] memory plugins = account1.getInstalledPlugins(); - assertEq(plugins.length, 2); + assertEq(plugins.length, 1); - assertEq(plugins[0], address(singleOwnerPlugin)); - assertEq(plugins[1], address(comprehensivePlugin)); + assertEq(plugins[0], address(comprehensivePlugin)); } function test_pluginLoupe_getExecutionFunctionHandler_native() public { @@ -157,4 +137,32 @@ contract AccountLoupeTest is AccountTestBase { ) ); } + + // Test config + + function _initialValidationConfig() + internal + virtual + override + returns (FunctionReference, bool, bool, bytes4[] memory, bytes memory, bytes memory, bytes memory) + { + FunctionReference[] memory preValidationHooks = new FunctionReference[](2); + preValidationHooks[0] = FunctionReferenceLib.pack( + address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.PRE_VALIDATION_HOOK_1) + ); + preValidationHooks[1] = FunctionReferenceLib.pack( + address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.PRE_VALIDATION_HOOK_2) + ); + + bytes[] memory installDatas = new bytes[](2); + return ( + _ownerValidation, + true, + true, + new bytes4[](0), + bytes(""), + abi.encode(preValidationHooks, installDatas), + "" + ); + } } diff --git a/test/account/AccountReturnData.t.sol b/test/account/AccountReturnData.t.sol index 99a8b516..1738c722 100644 --- a/test/account/AccountReturnData.t.sol +++ b/test/account/AccountReturnData.t.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.19; import {FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; import {Call} from "../../src/interfaces/IStandardExecutor.sol"; -import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; import { RegularResultContract, @@ -11,6 +10,7 @@ import { ResultConsumerPlugin } from "../mocks/plugins/ReturnDataPluginMocks.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; +import {TEST_DEFAULT_OWNER_FUNCTION_ID} from "../utils/TestConstants.sol"; // Tests all the different ways that return data can be read from plugins through an account contract AccountReturnDataTest is AccountTestBase { @@ -58,10 +58,8 @@ contract AccountReturnDataTest is AccountTestBase { (address(regularResultContract), 0, abi.encodeCall(RegularResultContract.foo, ())) ), _encodeSignature( - FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ), - SELECTOR_ASSOCIATED_VALIDATION, + FunctionReferenceLib.pack(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID), + GLOBAL_VALIDATION, "" ) ); @@ -88,10 +86,8 @@ contract AccountReturnDataTest is AccountTestBase { bytes memory retData = account1.executeWithAuthorization( abi.encodeCall(account1.executeBatch, (calls)), _encodeSignature( - FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ), - SELECTOR_ASSOCIATED_VALIDATION, + FunctionReferenceLib.pack(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID), + GLOBAL_VALIDATION, "" ) ); diff --git a/test/account/MultiValidation.t.sol b/test/account/MultiValidation.t.sol index e0e69a37..5254c50d 100644 --- a/test/account/MultiValidation.t.sol +++ b/test/account/MultiValidation.t.sol @@ -11,10 +11,11 @@ import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAcc import {FunctionReference} from "../../src/interfaces/IPluginManager.sol"; import {IStandardExecutor} from "../../src/interfaces/IStandardExecutor.sol"; import {FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; +import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; -import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; +import {TEST_DEFAULT_OWNER_FUNCTION_ID} from "../utils/TestConstants.sol"; contract MultiValidationTest is AccountTestBase { using ECDSA for bytes32; @@ -32,16 +33,18 @@ contract MultiValidationTest is AccountTestBase { } function test_overlappingValidationInstall() public { - bytes32 manifestHash = keccak256(abi.encode(validator2.pluginManifest())); vm.prank(address(entryPoint)); - account1.installPlugin(address(validator2), manifestHash, abi.encode(owner2)); + account1.installValidation( + ValidationConfigLib.pack(address(validator2), TEST_DEFAULT_OWNER_FUNCTION_ID, true, true), + new bytes4[](0), + abi.encode(TEST_DEFAULT_OWNER_FUNCTION_ID, owner2), + "", + "" + ); FunctionReference[] memory validations = new FunctionReference[](2); - validations[0] = FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ); - validations[1] = - FunctionReferenceLib.pack(address(validator2), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER)); + validations[0] = FunctionReferenceLib.pack(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID); + validations[1] = FunctionReferenceLib.pack(address(validator2), TEST_DEFAULT_OWNER_FUNCTION_ID); bytes4[] memory selectors0 = account1.getSelectors(validations[0]); bytes4[] memory selectors1 = account1.getSelectors(validations[1]); @@ -68,10 +71,8 @@ contract MultiValidationTest is AccountTestBase { account1.executeWithAuthorization( abi.encodeCall(IStandardExecutor.execute, (address(0), 0, "")), _encodeSignature( - FunctionReferenceLib.pack( - address(validator2), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ), - SELECTOR_ASSOCIATED_VALIDATION, + FunctionReferenceLib.pack(address(validator2), TEST_DEFAULT_OWNER_FUNCTION_ID), + GLOBAL_VALIDATION, "" ) ); @@ -80,10 +81,8 @@ contract MultiValidationTest is AccountTestBase { account1.executeWithAuthorization( abi.encodeCall(IStandardExecutor.execute, (address(0), 0, "")), _encodeSignature( - FunctionReferenceLib.pack( - address(validator2), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ), - SELECTOR_ASSOCIATED_VALIDATION, + FunctionReferenceLib.pack(address(validator2), TEST_DEFAULT_OWNER_FUNCTION_ID), + GLOBAL_VALIDATION, "" ) ); @@ -110,8 +109,8 @@ contract MultiValidationTest is AccountTestBase { bytes32 userOpHash = entryPoint.getUserOpHash(userOp); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner2Key, userOpHash.toEthSignedMessageHash()); userOp.signature = _encodeSignature( - FunctionReferenceLib.pack(address(validator2), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER)), - SELECTOR_ASSOCIATED_VALIDATION, + FunctionReferenceLib.pack(address(validator2), TEST_DEFAULT_OWNER_FUNCTION_ID), + GLOBAL_VALIDATION, abi.encodePacked(r, s, v) ); @@ -125,8 +124,8 @@ contract MultiValidationTest is AccountTestBase { userOp.nonce = 1; (v, r, s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); userOp.signature = _encodeSignature( - FunctionReferenceLib.pack(address(validator2), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER)), - SELECTOR_ASSOCIATED_VALIDATION, + FunctionReferenceLib.pack(address(validator2), TEST_DEFAULT_OWNER_FUNCTION_ID), + GLOBAL_VALIDATION, abi.encodePacked(r, s, v) ); diff --git a/test/account/PerHookData.t.sol b/test/account/PerHookData.t.sol index 44641238..9c90cf5c 100644 --- a/test/account/PerHookData.t.sol +++ b/test/account/PerHookData.t.sol @@ -325,7 +325,7 @@ contract PerHookDataTest is CustomValidationTestBase { internal virtual override - returns (FunctionReference, bool, bytes4[] memory, bytes memory, bytes memory, bytes memory) + returns (FunctionReference, bool, bool, bytes4[] memory, bytes memory, bytes memory, bytes memory) { FunctionReference accessControlHook = FunctionReferenceLib.pack( address(_accessControlHookPlugin), uint8(MockAccessControlHookPlugin.FunctionId.PRE_VALIDATION_HOOK) @@ -340,6 +340,14 @@ contract PerHookDataTest is CustomValidationTestBase { bytes memory packedPreValidationHooks = abi.encode(preValidationHooks, preValidationHookData); - return (_ownerValidation, true, new bytes4[](0), abi.encode(owner1), packedPreValidationHooks, ""); + return ( + _ownerValidation, + true, + true, + new bytes4[](0), + abi.encode(TEST_DEFAULT_OWNER_FUNCTION_ID, owner1), + packedPreValidationHooks, + "" + ); } } diff --git a/test/account/SelfCallAuthorization.t.sol b/test/account/SelfCallAuthorization.t.sol index a2cc7838..c97194fd 100644 --- a/test/account/SelfCallAuthorization.t.sol +++ b/test/account/SelfCallAuthorization.t.sol @@ -8,6 +8,7 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {IStandardExecutor, Call} from "../../src/interfaces/IStandardExecutor.sol"; import {FunctionReference, FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; +import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; import {GlobalValidationFactoryFixture} from "../mocks/GlobalValidationFactoryFixture.sol"; @@ -310,7 +311,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { account1.executeWithAuthorization( abi.encodeCall( UpgradeableModularAccount.installValidation, - (comprehensivePluginValidation, false, selectors, "", "", "") + (ValidationConfigLib.pack(comprehensivePluginValidation, false, false), selectors, "", "", "") ), _encodeSignature(_ownerValidation, GLOBAL_VALIDATION, "") ); diff --git a/test/account/UpgradeableModularAccount.t.sol b/test/account/UpgradeableModularAccount.t.sol index de3fda30..dda78c66 100644 --- a/test/account/UpgradeableModularAccount.t.sol +++ b/test/account/UpgradeableModularAccount.t.sol @@ -16,12 +16,12 @@ import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; import {Call} from "../../src/interfaces/IStandardExecutor.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {TokenReceiverPlugin} from "../../src/plugins/TokenReceiverPlugin.sol"; -import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; import {Counter} from "../mocks/Counter.sol"; import {ComprehensivePlugin} from "../mocks/plugins/ComprehensivePlugin.sol"; import {MockPlugin} from "../mocks/MockPlugin.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; +import {TEST_DEFAULT_OWNER_FUNCTION_ID} from "../utils/TestConstants.sol"; contract UpgradeableModularAccountTest is AccountTestBase { using ECDSA for bytes32; @@ -77,8 +77,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { // Generate signature bytes32 userOpHash = entryPoint.getUserOpHash(userOp); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); - userOp.signature = - _encodeSignature(_ownerValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(r, s, v)); + userOp.signature = _encodeSignature(_ownerValidation, GLOBAL_VALIDATION, abi.encodePacked(r, s, v)); PackedUserOperation[] memory userOps = new PackedUserOperation[](1); userOps[0] = userOp; @@ -95,7 +94,11 @@ contract UpgradeableModularAccountTest is AccountTestBase { initCode: abi.encodePacked(address(factory), abi.encodeCall(factory.createAccount, (owner2, 0))), callData: abi.encodeCall( UpgradeableModularAccount.execute, - (address(singleOwnerPlugin), 0, abi.encodeCall(SingleOwnerPlugin.transferOwnership, (owner2))) + ( + address(singleOwnerPlugin), + 0, + abi.encodeCall(SingleOwnerPlugin.transferOwnership, (TEST_DEFAULT_OWNER_FUNCTION_ID, owner2)) + ) ), accountGasLimits: _encodeGas(VERIFICATION_GAS_LIMIT, CALL_GAS_LIMIT), preVerificationGas: 0, @@ -107,8 +110,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { // Generate signature bytes32 userOpHash = entryPoint.getUserOpHash(userOp); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner2Key, userOpHash.toEthSignedMessageHash()); - userOp.signature = - _encodeSignature(_ownerValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(r, s, v)); + userOp.signature = _encodeSignature(_ownerValidation, GLOBAL_VALIDATION, abi.encodePacked(r, s, v)); PackedUserOperation[] memory userOps = new PackedUserOperation[](1); userOps[0] = userOp; @@ -134,8 +136,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { // Generate signature bytes32 userOpHash = entryPoint.getUserOpHash(userOp); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner2Key, userOpHash.toEthSignedMessageHash()); - userOp.signature = - _encodeSignature(_ownerValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(r, s, v)); + userOp.signature = _encodeSignature(_ownerValidation, GLOBAL_VALIDATION, abi.encodePacked(r, s, v)); PackedUserOperation[] memory userOps = new PackedUserOperation[](1); userOps[0] = userOp; @@ -161,8 +162,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { // Generate signature bytes32 userOpHash = entryPoint.getUserOpHash(userOp); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); - userOp.signature = - _encodeSignature(_ownerValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(r, s, v)); + userOp.signature = _encodeSignature(_ownerValidation, GLOBAL_VALIDATION, abi.encodePacked(r, s, v)); PackedUserOperation[] memory userOps = new PackedUserOperation[](1); userOps[0] = userOp; @@ -190,8 +190,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { // Generate signature bytes32 userOpHash = entryPoint.getUserOpHash(userOp); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); - userOp.signature = - _encodeSignature(_ownerValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(r, s, v)); + userOp.signature = _encodeSignature(_ownerValidation, GLOBAL_VALIDATION, abi.encodePacked(r, s, v)); PackedUserOperation[] memory userOps = new PackedUserOperation[](1); userOps[0] = userOp; @@ -222,8 +221,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { // Generate signature bytes32 userOpHash = entryPoint.getUserOpHash(userOp); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); - userOp.signature = - _encodeSignature(_ownerValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(r, s, v)); + userOp.signature = _encodeSignature(_ownerValidation, GLOBAL_VALIDATION, abi.encodePacked(r, s, v)); PackedUserOperation[] memory userOps = new PackedUserOperation[](1); userOps[0] = userOp; @@ -248,9 +246,8 @@ contract UpgradeableModularAccountTest is AccountTestBase { }); address[] memory plugins = IAccountLoupe(account1).getInstalledPlugins(); - assertEq(plugins.length, 2); - assertEq(plugins[0], address(singleOwnerPlugin)); - assertEq(plugins[1], address(tokenReceiverPlugin)); + assertEq(plugins.length, 1); + assertEq(plugins[0], address(tokenReceiverPlugin)); } function test_installPlugin_PermittedCallSelectorNotInstalled() public { @@ -330,8 +327,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { emit PluginUninstalled(address(plugin), true); IPluginManager(account1).uninstallPlugin({plugin: address(plugin), config: "", pluginUninstallData: ""}); address[] memory plugins = IAccountLoupe(account1).getInstalledPlugins(); - assertEq(plugins.length, 1); - assertEq(plugins[0], address(singleOwnerPlugin)); + assertEq(plugins.length, 0); } function test_uninstallPlugin_manifestParameter() public { @@ -354,8 +350,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { pluginUninstallData: "" }); address[] memory plugins = IAccountLoupe(account1).getInstalledPlugins(); - assertEq(plugins.length, 1); - assertEq(plugins[0], address(singleOwnerPlugin)); + assertEq(plugins.length, 0); } function test_uninstallPlugin_invalidManifestFails() public { @@ -380,9 +375,8 @@ contract UpgradeableModularAccountTest is AccountTestBase { pluginUninstallData: "" }); address[] memory plugins = IAccountLoupe(account1).getInstalledPlugins(); - assertEq(plugins.length, 2); - assertEq(plugins[0], address(singleOwnerPlugin)); - assertEq(plugins[1], address(plugin)); + assertEq(plugins.length, 1); + assertEq(plugins[0], address(plugin)); } function _installPluginWithExecHooks() internal returns (MockPlugin plugin) { @@ -415,14 +409,16 @@ contract UpgradeableModularAccountTest is AccountTestBase { } function test_transferOwnership() public { - assertEq(singleOwnerPlugin.ownerOf(address(account1)), owner1); + assertEq(singleOwnerPlugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, address(account1)), owner1); vm.prank(address(entryPoint)); account1.execute( - address(singleOwnerPlugin), 0, abi.encodeCall(SingleOwnerPlugin.transferOwnership, (owner2)) + address(singleOwnerPlugin), + 0, + abi.encodeCall(SingleOwnerPlugin.transferOwnership, (TEST_DEFAULT_OWNER_FUNCTION_ID, owner2)) ); - assertEq(singleOwnerPlugin.ownerOf(address(account1)), owner2); + assertEq(singleOwnerPlugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, address(account1)), owner2); } function test_isValidSignature() public { @@ -432,9 +428,8 @@ contract UpgradeableModularAccountTest is AccountTestBase { // singleOwnerPlugin.ownerOf(address(account1)); - bytes memory signature = abi.encodePacked( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), r, s, v - ); + bytes memory signature = + abi.encodePacked(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID, r, s, v); bytes4 validationResult = IERC1271(address(account1)).isValidSignature(message, signature); diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index d3fce6d8..7f245031 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -5,6 +5,7 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {FunctionReference, FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; +import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; import { MockBaseUserOpValidationPlugin, @@ -65,8 +66,7 @@ contract ValidationIntersectionTest is AccountTestBase { }); bytes[] memory installDatas = new bytes[](1); account1.installValidation( - oneHookValidation, - true, + ValidationConfigLib.pack(oneHookValidation, true, true), new bytes4[](0), bytes(""), abi.encode(preValidationHooks, installDatas), @@ -89,8 +89,7 @@ contract ValidationIntersectionTest is AccountTestBase { }); installDatas = new bytes[](2); account1.installValidation( - twoHookValidation, - true, + ValidationConfigLib.pack(twoHookValidation, true, true), new bytes4[](0), bytes(""), abi.encode(preValidationHooks, installDatas), diff --git a/test/mocks/GlobalValidationFactoryFixture.sol b/test/mocks/GlobalValidationFactoryFixture.sol index bc61fb21..abe8c1c9 100644 --- a/test/mocks/GlobalValidationFactoryFixture.sol +++ b/test/mocks/GlobalValidationFactoryFixture.sol @@ -6,11 +6,11 @@ import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.s import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; -import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; +import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {OptimizedTest} from "../utils/OptimizedTest.sol"; +import {TEST_DEFAULT_OWNER_FUNCTION_ID} from "../utils/TestConstants.sol"; contract GlobalValidationFactoryFixture is OptimizedTest { UpgradeableModularAccount public accountImplementation; @@ -50,16 +50,13 @@ contract GlobalValidationFactoryFixture is OptimizedTest { // short circuit if exists if (addr.code.length == 0) { - bytes memory pluginInstallData = abi.encode(owner); + bytes memory pluginInstallData = abi.encode(TEST_DEFAULT_OWNER_FUNCTION_ID, owner); // not necessary to check return addr since next call will fail if so new ERC1967Proxy{salt: getSalt(owner, salt)}(address(accountImplementation), ""); // point proxy to actual implementation and init plugins UpgradeableModularAccount(payable(addr)).initializeWithValidation( - FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ), - true, + ValidationConfigLib.pack(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID, true, true), new bytes4[](0), pluginInstallData, "", diff --git a/test/mocks/MSCAFactoryFixture.sol b/test/mocks/MSCAFactoryFixture.sol index cc8ae60a..8ca3a51f 100644 --- a/test/mocks/MSCAFactoryFixture.sol +++ b/test/mocks/MSCAFactoryFixture.sol @@ -6,9 +6,11 @@ import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.s import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; +import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {OptimizedTest} from "../utils/OptimizedTest.sol"; +import {TEST_DEFAULT_OWNER_FUNCTION_ID} from "../utils/TestConstants.sol"; /** * @title MSCAFactoryFixture @@ -24,10 +26,6 @@ contract MSCAFactoryFixture is OptimizedTest { IEntryPoint public entryPoint; - address public self; - - bytes32 public singleOwnerPluginManifestHash; - constructor(IEntryPoint _entryPoint, SingleOwnerPlugin _singleOwnerPlugin) { entryPoint = _entryPoint; accountImplementation = _deployUpgradeableModularAccount(_entryPoint); @@ -35,10 +33,6 @@ contract MSCAFactoryFixture is OptimizedTest { abi.encodePacked(type(ERC1967Proxy).creationCode, abi.encode(address(accountImplementation), "")) ); singleOwnerPlugin = _singleOwnerPlugin; - self = address(this); - // The manifest hash is set this way in this factory just for testing purposes. - // For production factories the manifest hashes should be passed as a constructor argument. - singleOwnerPluginManifestHash = keccak256(abi.encode(singleOwnerPlugin.pluginManifest())); } /** @@ -53,17 +47,18 @@ contract MSCAFactoryFixture is OptimizedTest { // short circuit if exists if (addr.code.length == 0) { - address[] memory plugins = new address[](1); - plugins[0] = address(singleOwnerPlugin); - bytes32[] memory pluginManifestHashes = new bytes32[](1); - pluginManifestHashes[0] = keccak256(abi.encode(singleOwnerPlugin.pluginManifest())); - bytes[] memory pluginInstallData = new bytes[](1); - pluginInstallData[0] = abi.encode(owner); + bytes memory pluginInstallData = abi.encode(TEST_DEFAULT_OWNER_FUNCTION_ID, owner); // not necessary to check return addr since next call will fail if so new ERC1967Proxy{salt: getSalt(owner, salt)}(address(accountImplementation), ""); // point proxy to actual implementation and init plugins - UpgradeableModularAccount(payable(addr)).initialize(plugins, pluginManifestHashes, pluginInstallData); + UpgradeableModularAccount(payable(addr)).initializeWithValidation( + ValidationConfigLib.pack(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID, true, true), + new bytes4[](0), + pluginInstallData, + "", + "" + ); } return UpgradeableModularAccount(payable(addr)); diff --git a/test/plugin/SingleOwnerPlugin.t.sol b/test/plugin/SingleOwnerPlugin.t.sol index f86a29f4..ddfe4d41 100644 --- a/test/plugin/SingleOwnerPlugin.t.sol +++ b/test/plugin/SingleOwnerPlugin.t.sol @@ -7,9 +7,10 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; -import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; + import {ContractOwner} from "../mocks/ContractOwner.sol"; import {OptimizedTest} from "../utils/OptimizedTest.sol"; +import {TEST_DEFAULT_OWNER_FUNCTION_ID} from "../utils/TestConstants.sol"; contract SingleOwnerPluginTest is OptimizedTest { using ECDSA for bytes32; @@ -51,74 +52,74 @@ contract SingleOwnerPluginTest is OptimizedTest { function test_uninitializedOwner() public { vm.startPrank(a); - assertEq(address(0), plugin.owner()); + assertEq(address(0), plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); } function test_ownerInitialization() public { vm.startPrank(a); - assertEq(address(0), plugin.owner()); - plugin.transferOwnership(owner1); - assertEq(owner1, plugin.owner()); + assertEq(address(0), plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, owner1); + assertEq(owner1, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); } function test_ownerInitializationEvent() public { vm.startPrank(a); - assertEq(address(0), plugin.owner()); + assertEq(address(0), plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); vm.expectEmit(true, true, true, true); emit OwnershipTransferred(a, address(0), owner1); - plugin.transferOwnership(owner1); - assertEq(owner1, plugin.owner()); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, owner1); + assertEq(owner1, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); } function test_ownerMigration() public { vm.startPrank(a); - assertEq(address(0), plugin.owner()); - plugin.transferOwnership(owner1); - assertEq(owner1, plugin.owner()); - plugin.transferOwnership(owner2); - assertEq(owner2, plugin.owner()); + assertEq(address(0), plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, owner1); + assertEq(owner1, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, owner2); + assertEq(owner2, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); } function test_ownerMigrationEvents() public { vm.startPrank(a); - assertEq(address(0), plugin.owner()); + assertEq(address(0), plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); vm.expectEmit(true, true, true, true); emit OwnershipTransferred(a, address(0), owner1); - plugin.transferOwnership(owner1); - assertEq(owner1, plugin.owner()); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, owner1); + assertEq(owner1, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); vm.expectEmit(true, true, true, true); emit OwnershipTransferred(a, owner1, owner2); - plugin.transferOwnership(owner2); - assertEq(owner2, plugin.owner()); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, owner2); + assertEq(owner2, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); } function test_ownerForSender() public { vm.startPrank(a); - assertEq(address(0), plugin.owner()); - plugin.transferOwnership(owner1); - assertEq(owner1, plugin.owner()); + assertEq(address(0), plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, owner1); + assertEq(owner1, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); vm.startPrank(b); - assertEq(address(0), plugin.owner()); - plugin.transferOwnership(owner2); - assertEq(owner2, plugin.owner()); + assertEq(address(0), plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, b)); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, owner2); + assertEq(owner2, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, b)); } function test_requireOwner() public { vm.startPrank(a); - assertEq(address(0), plugin.owner()); - plugin.transferOwnership(owner1); - assertEq(owner1, plugin.owner()); - plugin.validateRuntime(uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), owner1, 0, "", ""); + assertEq(address(0), plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, owner1); + assertEq(owner1, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); + plugin.validateRuntime(TEST_DEFAULT_OWNER_FUNCTION_ID, owner1, 0, "", ""); vm.startPrank(b); - vm.expectRevert(ISingleOwnerPlugin.NotAuthorized.selector); - plugin.validateRuntime(uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), owner1, 0, "", ""); + vm.expectRevert(SingleOwnerPlugin.NotAuthorized.selector); + plugin.validateRuntime(TEST_DEFAULT_OWNER_FUNCTION_ID, owner1, 0, "", ""); } function testFuzz_validateUserOpSig(string memory salt, PackedUserOperation memory userOp) public { @@ -133,16 +134,15 @@ contract SingleOwnerPluginTest is OptimizedTest { userOp.signature = abi.encodePacked(r, s, v); // sig check should fail - uint256 success = - plugin.validateUserOp(uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), userOp, userOpHash); + uint256 success = plugin.validateUserOp(TEST_DEFAULT_OWNER_FUNCTION_ID, userOp, userOpHash); assertEq(success, 1); // transfer ownership to signer - plugin.transferOwnership(signer); - assertEq(signer, plugin.owner()); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, signer); + assertEq(signer, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); // sig check should pass - success = plugin.validateUserOp(uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), userOp, userOpHash); + success = plugin.validateUserOp(TEST_DEFAULT_OWNER_FUNCTION_ID, userOp, userOpHash); assertEq(success, 0); } @@ -156,25 +156,19 @@ contract SingleOwnerPluginTest is OptimizedTest { // sig check should fail assertEq( plugin.validateSignature( - uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), - address(this), - digest, - abi.encodePacked(r, s, v) + TEST_DEFAULT_OWNER_FUNCTION_ID, address(this), digest, abi.encodePacked(r, s, v) ), bytes4(0xFFFFFFFF) ); // transfer ownership to signer - plugin.transferOwnership(signer); - assertEq(signer, plugin.owner()); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, signer); + assertEq(signer, plugin.owners(TEST_DEFAULT_OWNER_FUNCTION_ID, a)); // sig check should pass assertEq( plugin.validateSignature( - uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), - address(this), - digest, - abi.encodePacked(r, s, v) + TEST_DEFAULT_OWNER_FUNCTION_ID, address(this), digest, abi.encodePacked(r, s, v) ), _1271_MAGIC_VALUE ); @@ -182,12 +176,10 @@ contract SingleOwnerPluginTest is OptimizedTest { function testFuzz_isValidSignatureForContractOwner(bytes32 digest) public { vm.startPrank(a); - plugin.transferOwnership(address(contractOwner)); + plugin.transferOwnership(TEST_DEFAULT_OWNER_FUNCTION_ID, address(contractOwner)); bytes memory signature = contractOwner.sign(digest); assertEq( - plugin.validateSignature( - uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), address(this), digest, signature - ), + plugin.validateSignature(TEST_DEFAULT_OWNER_FUNCTION_ID, address(this), digest, signature), _1271_MAGIC_VALUE ); } diff --git a/test/samples/AllowlistPlugin.t.sol b/test/samples/AllowlistPlugin.t.sol index da70c8c3..d81d5f79 100644 --- a/test/samples/AllowlistPlugin.t.sol +++ b/test/samples/AllowlistPlugin.t.sol @@ -290,7 +290,7 @@ contract AllowlistPluginTest is CustomValidationTestBase { internal virtual override - returns (FunctionReference, bool, bytes4[] memory, bytes memory, bytes memory, bytes memory) + returns (FunctionReference, bool, bool, bytes4[] memory, bytes memory, bytes memory, bytes memory) { FunctionReference accessControlHook = FunctionReferenceLib.pack( address(allowlistPlugin), uint8(AllowlistPlugin.FunctionId.PRE_VALIDATION_HOOK) @@ -305,7 +305,15 @@ contract AllowlistPluginTest is CustomValidationTestBase { bytes memory packedPreValidationHooks = abi.encode(preValidationHooks, preValidationHookData); - return (_ownerValidation, true, new bytes4[](0), abi.encode(owner1), packedPreValidationHooks, ""); + return ( + _ownerValidation, + true, + true, + new bytes4[](0), + abi.encode(TEST_DEFAULT_OWNER_FUNCTION_ID, owner1), + packedPreValidationHooks, + "" + ); } // Unfortunately, this is a feature that solidity has only implemented in via-ir, so we need to do it manually diff --git a/test/utils/AccountTestBase.sol b/test/utils/AccountTestBase.sol index 4383f393..a312fbdf 100644 --- a/test/utils/AccountTestBase.sol +++ b/test/utils/AccountTestBase.sol @@ -8,10 +8,10 @@ import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/Messa import {FunctionReference, FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; import {IStandardExecutor, Call} from "../../src/interfaces/IStandardExecutor.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {OptimizedTest} from "./OptimizedTest.sol"; +import {TEST_DEFAULT_OWNER_FUNCTION_ID as EXT_CONST_TEST_DEFAULT_OWNER_FUNCTION_ID} from "./TestConstants.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; @@ -35,6 +35,9 @@ abstract contract AccountTestBase is OptimizedTest { uint8 public constant SELECTOR_ASSOCIATED_VALIDATION = 0; uint8 public constant GLOBAL_VALIDATION = 1; + // Re-declare the constant to prevent derived test contracts from having to import it + uint8 public constant TEST_DEFAULT_OWNER_FUNCTION_ID = EXT_CONST_TEST_DEFAULT_OWNER_FUNCTION_ID; + uint256 public constant CALL_GAS_LIMIT = 100000; uint256 public constant VERIFICATION_GAS_LIMIT = 1200000; @@ -54,9 +57,7 @@ abstract contract AccountTestBase is OptimizedTest { account1 = factory.createAccount(owner1, 0); vm.deal(address(account1), 100 ether); - _ownerValidation = FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ); + _ownerValidation = FunctionReferenceLib.pack(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID); } function _runExecUserOp(address target, bytes memory callData) internal { @@ -99,9 +100,7 @@ abstract contract AccountTestBase is OptimizedTest { (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); userOp.signature = _encodeSignature( - FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ), + FunctionReferenceLib.pack(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID), GLOBAL_VALIDATION, abi.encodePacked(r, s, v) ); @@ -154,9 +153,7 @@ abstract contract AccountTestBase is OptimizedTest { account1.executeWithAuthorization( callData, _encodeSignature( - FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ), + FunctionReferenceLib.pack(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID), GLOBAL_VALIDATION, "" ) @@ -171,9 +168,7 @@ abstract contract AccountTestBase is OptimizedTest { account1.executeWithAuthorization( callData, _encodeSignature( - FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ), + FunctionReferenceLib.pack(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID), GLOBAL_VALIDATION, "" ) @@ -189,14 +184,14 @@ abstract contract AccountTestBase is OptimizedTest { ( address(singleOwnerPlugin), 0, - abi.encodeCall(SingleOwnerPlugin.transferOwnership, (address(this))) + abi.encodeCall( + SingleOwnerPlugin.transferOwnership, (TEST_DEFAULT_OWNER_FUNCTION_ID, address(this)) + ) ) ), _encodeSignature( - FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER) - ), - SELECTOR_ASSOCIATED_VALIDATION, + FunctionReferenceLib.pack(address(singleOwnerPlugin), TEST_DEFAULT_OWNER_FUNCTION_ID), + GLOBAL_VALIDATION, "" ) ); diff --git a/test/utils/CustomValidationTestBase.sol b/test/utils/CustomValidationTestBase.sol index 1bf76c3a..2244c865 100644 --- a/test/utils/CustomValidationTestBase.sol +++ b/test/utils/CustomValidationTestBase.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.25; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; +import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {AccountTestBase} from "./AccountTestBase.sol"; @@ -16,7 +17,8 @@ abstract contract CustomValidationTestBase is AccountTestBase { function _customValidationSetup() internal { ( FunctionReference validationFunction, - bool shared, + bool isGlobal, + bool isSignatureValidation, bytes4[] memory selectors, bytes memory installData, bytes memory preValidationHooks, @@ -28,7 +30,11 @@ abstract contract CustomValidationTestBase is AccountTestBase { account1 = UpgradeableModularAccount(payable(new ERC1967Proxy{salt: 0}(accountImplementation, ""))); account1.initializeWithValidation( - validationFunction, shared, selectors, installData, preValidationHooks, permissionHooks + ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation), + selectors, + installData, + preValidationHooks, + permissionHooks ); vm.deal(address(account1), 100 ether); @@ -40,6 +46,7 @@ abstract contract CustomValidationTestBase is AccountTestBase { returns ( FunctionReference validationFunction, bool shared, + bool isSignatureValidation, bytes4[] memory selectors, bytes memory installData, bytes memory preValidationHooks, diff --git a/test/utils/TestConstants.sol b/test/utils/TestConstants.sol new file mode 100644 index 00000000..923692a7 --- /dev/null +++ b/test/utils/TestConstants.sol @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.25; + +uint8 constant TEST_DEFAULT_OWNER_FUNCTION_ID = 0;