diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a8527e96..dfa5cf41 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,4 +1,4 @@ -name: ERC6900 RI Test CI +name: ERC-6900 RI Test CI on: [pull_request, workflow_dispatch] @@ -45,9 +45,9 @@ jobs: - name: "Lint the contracts" run: "pnpm lint" - - test: - name: Run Forge Tests + + test-optimized-test-deep: + name: Run Forge Tests (optimized-test-deep) runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -63,13 +63,13 @@ jobs: run: forge install - name: Build project - run: forge build + run: FOUNDRY_PROFILE=optimized-build forge build - name: Run tests - run: FOUNDRY_PROFILE=deep forge test -vvv + run: FOUNDRY_PROFILE=optimized-test-deep forge test -vvv - test-lite: - name: Run Forge Tests [lite build] + test-default: + name: Run Forge Tests (default) runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -85,7 +85,7 @@ jobs: run: forge install - name: Build project - run: FOUNDRY_PROFILE=lite forge build + run: forge build - name: Run tests - run: FOUNDRY_PROFILE=lite forge test -vvv + run: forge test -vvv diff --git a/.gitignore b/.gitignore index 3a109388..3189d156 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,9 @@ # Foundry build and cache directories out/ +out-optimized/ cache/ node_modules/ + +# Coverage +report/ +lcov.info diff --git a/README.md b/README.md index 70657779..fb3e9dbc 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# ERC-6900 Ref Implementation +# ERC-6900 Reference Implementation Reference implementation for [ERC-6900](https://eips.ethereum.org/EIPS/eip-6900). It is an early draft implementation. @@ -13,20 +13,18 @@ The implementation includes an upgradable modular account with two plugins (`Sin Anyone is welcome to submit feedback and/or PRs to improve code or add Plugins. -### Build +### Testing + +The default Foundry profile can be used to compile (without IR) and test the entire project. The default profile should be used when generating coverage and debugging. ```bash forge build - -# or use the lite profile to reduce compilation time -FOUNDRY_PROFILE=lite forge build +forge test -vvv ``` -### Test +Since IR compilation generates different bytecode, it's useful to test against the contracts compiled via IR. Since compiling the entire project (including the test suite) takes a long time, special profiles can be used to precompile just the source contracts, and have the tests deploy the relevant contracts using those artifacts. ```bash -forge test -vvv - -# or use the lite profile to reduce compilation time -FOUNDRY_PROFILE=lite forge test -vvv +FOUNDRY_PROFILE=optimized-build forge build +FOUNDRY_PROFILE=optimized-test forge test -vvv ``` diff --git a/foundry.toml b/foundry.toml index a5d071fe..2754a5b2 100644 --- a/foundry.toml +++ b/foundry.toml @@ -1,13 +1,16 @@ [profile.default] solc = '0.8.19' -via_ir = true +via_ir = false src = 'src' -out = 'out' test = 'test' libs = ['lib'] +out = 'out' optimizer = true optimizer_runs = 10_000 ignored_error_codes = [3628] +fs_permissions = [ + { access = "read", path = "./out-optimized" } +] [fuzz] runs = 500 @@ -17,12 +20,23 @@ runs=500 fail_on_revert = true depth = 10 -[profile.lite] -solc = '0.8.19' -via_ir = false -optimizer = true -optimizer_runs = 10_000 -ignored_error_codes = [3628] +[profile.optimized-build] +via_ir = true +test = 'src' +out = 'out-optimized' + +[profile.optimized-test] +src = 'test' + +[profile.optimized-test-deep] +src = 'test' + +[profile.optimized-test-deep.fuzz] +runs = 10000 + +[profile.optimized-test-deep.invariant] +runs = 5000 +depth = 32 [profile.deep.fuzz] runs = 10000 @@ -43,4 +57,4 @@ goerli = "${RPC_URL_GOERLI}" mainnet = { key = "${ETHERSCAN_API_KEY}" } goerli = { key = "${ETHERSCAN_API_KEY}" } -# See more config options https://github.com/foundry-rs/foundry/tree/master/config \ No newline at end of file +# See more config options https://github.com/foundry-rs/foundry/tree/master/config diff --git a/lib/forge-std b/lib/forge-std index 066ff16c..2f112697 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 066ff16c5c03e6f931cd041fd366bc4be1fae82a +Subproject commit 2f112697506eab12d433a65fdc31a639548fe365 diff --git a/src/account/AccountLoupe.sol b/src/account/AccountLoupe.sol new file mode 100644 index 00000000..5f68ea32 --- /dev/null +++ b/src/account/AccountLoupe.sol @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.19; + +import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; +import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; +import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; + +import {IAccountLoupe} from "../interfaces/IAccountLoupe.sol"; +import {IPluginManager} from "../interfaces/IPluginManager.sol"; +import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol"; +import { + AccountStorage, + getAccountStorage, + getPermittedCallKey, + HookGroup, + toFunctionReferenceArray +} from "../libraries/AccountStorage.sol"; +import {FunctionReference} from "../libraries/FunctionReferenceLib.sol"; + +abstract contract AccountLoupe is IAccountLoupe { + using EnumerableMap for EnumerableMap.Bytes32ToUintMap; + using EnumerableSet for EnumerableSet.AddressSet; + + error ManifestDiscrepancy(address plugin); + + /// @inheritdoc IAccountLoupe + function getExecutionFunctionConfig(bytes4 selector) + external + view + returns (ExecutionFunctionConfig memory config) + { + AccountStorage storage _storage = getAccountStorage(); + + if ( + selector == IStandardExecutor.execute.selector || selector == IStandardExecutor.executeBatch.selector + || selector == UUPSUpgradeable.upgradeTo.selector + || selector == UUPSUpgradeable.upgradeToAndCall.selector + || selector == IPluginManager.installPlugin.selector + || selector == IPluginManager.uninstallPlugin.selector + ) { + config.plugin = address(this); + } else { + config.plugin = _storage.selectorData[selector].plugin; + } + + config.userOpValidationFunction = _storage.selectorData[selector].userOpValidation; + + config.runtimeValidationFunction = _storage.selectorData[selector].runtimeValidation; + } + + /// @inheritdoc IAccountLoupe + function getExecutionHooks(bytes4 selector) external view returns (ExecutionHooks[] memory execHooks) { + execHooks = _getHooks(getAccountStorage().selectorData[selector].executionHooks); + } + + /// @inheritdoc IAccountLoupe + function getPermittedCallHooks(address callingPlugin, bytes4 selector) + external + view + returns (ExecutionHooks[] memory execHooks) + { + bytes24 key = getPermittedCallKey(callingPlugin, selector); + execHooks = _getHooks(getAccountStorage().permittedCalls[key].permittedCallHooks); + } + + /// @inheritdoc IAccountLoupe + function getPreValidationHooks(bytes4 selector) + external + view + returns ( + FunctionReference[] memory preUserOpValidationHooks, + FunctionReference[] memory preRuntimeValidationHooks + ) + { + preUserOpValidationHooks = + toFunctionReferenceArray(getAccountStorage().selectorData[selector].preUserOpValidationHooks); + preRuntimeValidationHooks = + toFunctionReferenceArray(getAccountStorage().selectorData[selector].preRuntimeValidationHooks); + } + + /// @inheritdoc IAccountLoupe + function getInstalledPlugins() external view returns (address[] memory pluginAddresses) { + pluginAddresses = getAccountStorage().plugins.values(); + } + + function _getHooks(HookGroup storage hooks) internal view returns (ExecutionHooks[] memory execHooks) { + uint256 preExecHooksLength = hooks.preHooks.length(); + uint256 postOnlyExecHooksLength = hooks.postOnlyHooks.length(); + uint256 maxExecHooksLength = postOnlyExecHooksLength; + + // There can only be as many associated post hooks to run as there are pre hooks. + for (uint256 i = 0; i < preExecHooksLength;) { + (, uint256 count) = hooks.preHooks.at(i); + unchecked { + maxExecHooksLength += (count + 1); + ++i; + } + } + + // Overallocate on length - not all of this may get filled up. We set the correct length later. + execHooks = new ExecutionHooks[](maxExecHooksLength); + uint256 actualExecHooksLength; + + for (uint256 i = 0; i < preExecHooksLength;) { + (bytes32 key,) = hooks.preHooks.at(i); + FunctionReference preExecHook = FunctionReference.wrap(bytes21(key)); + + uint256 associatedPostExecHooksLength = hooks.associatedPostHooks[preExecHook].length(); + if (associatedPostExecHooksLength > 0) { + for (uint256 j = 0; j < associatedPostExecHooksLength;) { + execHooks[actualExecHooksLength].preExecHook = preExecHook; + (key,) = hooks.associatedPostHooks[preExecHook].at(j); + execHooks[actualExecHooksLength].postExecHook = FunctionReference.wrap(bytes21(key)); + + unchecked { + ++actualExecHooksLength; + ++j; + } + } + } else { + execHooks[actualExecHooksLength].preExecHook = preExecHook; + + unchecked { + ++actualExecHooksLength; + } + } + + unchecked { + ++i; + } + } + + for (uint256 i = 0; i < postOnlyExecHooksLength;) { + (bytes32 key,) = hooks.postOnlyHooks.at(i); + execHooks[actualExecHooksLength].postExecHook = FunctionReference.wrap(bytes21(key)); + + unchecked { + ++actualExecHooksLength; + ++i; + } + } + + // Trim the exec hooks array to the actual length, since we may have overallocated. + assembly ("memory-safe") { + mstore(execHooks, actualExecHooksLength) + } + } +} diff --git a/src/account/AccountStorageInitializable.sol b/src/account/AccountStorageInitializable.sol index 68805ffe..49d5b54a 100644 --- a/src/account/AccountStorageInitializable.sol +++ b/src/account/AccountStorageInitializable.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.19; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; + import {AccountStorage, getAccountStorage} from "../libraries/AccountStorage.sol"; abstract contract AccountStorageInitializable { diff --git a/src/account/BaseModularAccountLoupe.sol b/src/account/BaseModularAccountLoupe.sol deleted file mode 100644 index 39ac5a77..00000000 --- a/src/account/BaseModularAccountLoupe.sol +++ /dev/null @@ -1,131 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0 -pragma solidity ^0.8.19; - -import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; -import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; - -import {IPluginLoupe} from "../interfaces/IPluginLoupe.sol"; -import {IPluginManager} from "../interfaces/IPluginManager.sol"; -import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol"; -import { - AccountStorage, - getAccountStorage, - getPermittedCallKey, - toFunctionReferenceArray -} from "../libraries/AccountStorage.sol"; -import {FunctionReference} from "../libraries/FunctionReferenceLib.sol"; - -abstract contract BaseModularAccountLoupe is IPluginLoupe { - using EnumerableSet for EnumerableSet.AddressSet; - - error ManifestDiscrepancy(address plugin); - - /// @notice Gets the validator and plugin configuration for a selector - /// @param selector The selector to get the configuration for - /// @return config The configuration for this selector - function getExecutionFunctionConfig(bytes4 selector) - external - view - returns (ExecutionFunctionConfig memory config) - { - AccountStorage storage _storage = getAccountStorage(); - - if ( - selector == IStandardExecutor.execute.selector || selector == IStandardExecutor.executeBatch.selector - || selector == UUPSUpgradeable.upgradeTo.selector - || selector == UUPSUpgradeable.upgradeToAndCall.selector - || selector == IPluginManager.installPlugin.selector - || selector == IPluginManager.uninstallPlugin.selector - ) { - config.plugin = address(this); - } else { - config.plugin = _storage.selectorData[selector].plugin; - } - - config.userOpValidationFunction = _storage.selectorData[selector].userOpValidation; - - config.runtimeValidationFunction = _storage.selectorData[selector].runtimeValidation; - } - - /// @notice Gets the pre and post execution hooks for a selector - /// @param selector The selector to get the hooks for - /// @return execHooks The pre and post execution hooks for this selector - function getExecutionHooks(bytes4 selector) external view returns (ExecutionHooks[] memory execHooks) { - AccountStorage storage _storage = getAccountStorage(); - - FunctionReference[] memory preExecHooks = - toFunctionReferenceArray(_storage.selectorData[selector].preExecHooks); - - uint256 numHooks = preExecHooks.length; - execHooks = new ExecutionHooks[](numHooks); - - for (uint256 i = 0; i < numHooks;) { - execHooks[i].preExecHook = preExecHooks[i]; - execHooks[i].postExecHook = _storage.selectorData[selector].associatedPostExecHooks[preExecHooks[i]]; - - unchecked { - ++i; - } - } - } - - /// @notice Gets the pre and post permitted call hooks applied for a plugin calling this selector - /// @param callingPlugin The plugin that is calling the selector - /// @param selector The selector the plugin is calling - /// @return execHooks The pre and post permitted call hooks for this selector - function getPermittedCallHooks(address callingPlugin, bytes4 selector) - external - view - returns (ExecutionHooks[] memory execHooks) - { - AccountStorage storage _storage = getAccountStorage(); - - bytes24 key = getPermittedCallKey(callingPlugin, selector); - - FunctionReference[] memory prePermittedCallHooks = - toFunctionReferenceArray(_storage.permittedCalls[key].prePermittedCallHooks); - - uint256 numHooks = prePermittedCallHooks.length; - execHooks = new ExecutionHooks[](numHooks); - - for (uint256 i = 0; i < numHooks;) { - execHooks[i].preExecHook = prePermittedCallHooks[i]; - execHooks[i].postExecHook = - _storage.permittedCalls[key].associatedPostPermittedCallHooks[prePermittedCallHooks[i]]; - - unchecked { - ++i; - } - } - } - - /// @notice Gets the pre user op validation hooks associated with a selector - /// @param selector The selector to get the hooks for - /// @return preValidationHooks The pre user op validation hooks for this selector - function getPreUserOpValidationHooks(bytes4 selector) - external - view - returns (FunctionReference[] memory preValidationHooks) - { - preValidationHooks = - toFunctionReferenceArray(getAccountStorage().selectorData[selector].preUserOpValidationHooks); - } - - /// @notice Gets the pre runtime validation hooks associated with a selector - /// @param selector The selector to get the hooks for - /// @return preValidationHooks The pre runtime validation hooks for this selector - function getPreRuntimeValidationHooks(bytes4 selector) - external - view - returns (FunctionReference[] memory preValidationHooks) - { - preValidationHooks = - toFunctionReferenceArray(getAccountStorage().selectorData[selector].preRuntimeValidationHooks); - } - - /// @notice Gets an array of all installed plugins - /// @return pluginAddresses The addresses of all installed plugins - function getInstalledPlugins() external view returns (address[] memory pluginAddresses) { - pluginAddresses = getAccountStorage().plugins.values(); - } -} diff --git a/src/account/BaseModularAccount.sol b/src/account/PluginManagerInternals.sol similarity index 79% rename from src/account/BaseModularAccount.sol rename to src/account/PluginManagerInternals.sol index b0b4f512..6f47c55a 100644 --- a/src/account/BaseModularAccount.sol +++ b/src/account/PluginManagerInternals.sol @@ -1,22 +1,22 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.19; -import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; -import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; +import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; -import {IPluginManager} from "../interfaces/IPluginManager.sol"; -import {AccountExecutor} from "./AccountExecutor.sol"; -import {FunctionReference, FunctionReferenceLib} from "../libraries/FunctionReferenceLib.sol"; import { AccountStorage, getAccountStorage, SelectorData, PermittedCallData, getPermittedCallKey, + HookGroup, PermittedExternalCallData, StoredInjectedHook } from "../libraries/AccountStorage.sol"; +import {FunctionReference, FunctionReferenceLib} from "../libraries/FunctionReferenceLib.sol"; +import {IPluginManager} from "../interfaces/IPluginManager.sol"; import { IPlugin, ManifestExecutionHook, @@ -27,24 +27,12 @@ import { PluginManifest } from "../interfaces/IPlugin.sol"; -abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 { - using EnumerableSet for EnumerableSet.Bytes32Set; +abstract contract PluginManagerInternals is IPluginManager { + using EnumerableMap for EnumerableMap.Bytes32ToUintMap; using EnumerableSet for EnumerableSet.AddressSet; - // As per the EIP-165 spec, no interface should ever match 0xffffffff - bytes4 internal constant _INTERFACE_ID_INVALID = 0xffffffff; - bytes4 internal constant _IERC165_INTERFACE_ID = 0x01ffc9a7; - error ArrayLengthMismatch(); - error ExecuteFromPluginAlreadySet(bytes4 selector, address plugin); - error PermittedExecutionSelectorNotInstalled(bytes4 selector, address plugin); - error ExecuteFromPluginNotSet(bytes4 selector, address plugin); error ExecutionFunctionAlreadySet(bytes4 selector); - error ExecutionFunctionNotSet(bytes4 selector); - error ExecutionHookAlreadySet(bytes4 selector, FunctionReference hook); - error ExecutionHookNotSet(bytes4 selector, FunctionReference hook); - error InvalidPostExecHook(bytes4 selector, FunctionReference hook); - error InvalidPostPermittedCallHook(bytes4 selector, FunctionReference hook); error InvalidDependenciesProvided(); error InvalidPluginManifest(); error MissingPluginDependency(address dependency); @@ -52,24 +40,16 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 error NullPlugin(); error PluginAlreadyInstalled(address plugin); error PluginDependencyViolation(address plugin); - error PermittedCallHookAlreadySet(bytes4 selector, address plugin, FunctionReference hook); - error PermittedCallHookNotSet(bytes4 selector, address plugin, FunctionReference hook); error PluginInstallCallbackFailed(address plugin, bytes revertReason); error PluginInterfaceNotSupported(address plugin); error PluginNotInstalled(address plugin); - error PreRuntimeValidationHookAlreadySet(bytes4 selector, FunctionReference hook); - error PreRuntimeValidationHookNotSet(bytes4 selector, FunctionReference hook); - error PreUserOpValidationHookAlreadySet(bytes4 selector, FunctionReference hook); - error PreUserOpValidationHookNotSet(bytes4 selector, FunctionReference hook); error RuntimeValidationFunctionAlreadySet(bytes4 selector, FunctionReference validationFunction); - error RuntimeValidationFunctionNotSet(bytes4 selector, FunctionReference validationFunction); error UserOpValidationFunctionAlreadySet(bytes4 selector, FunctionReference validationFunction); - error UserOpValidationFunctionNotSet(bytes4 selector, FunctionReference validationFunction); error PluginApplyHookCallbackFailed(address providingPlugin, bytes revertReason); error PluginUnapplyHookCallbackFailed(address providingPlugin, bytes revertReason); modifier notNullFunction(FunctionReference functionReference) { - if (functionReference == FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { + if (functionReference.isEmpty()) { revert NullFunctionReference(); } _; @@ -97,10 +77,6 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 function _removeExecutionFunction(bytes4 selector) internal { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - if (_selectorData.plugin == address(0)) { - revert ExecutionFunctionNotSet(selector); - } - _selectorData.plugin = address(0); } @@ -110,7 +86,7 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - if (_selectorData.userOpValidation != FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { + if (!_selectorData.userOpValidation.isEmpty()) { revert UserOpValidationFunctionAlreadySet(selector, validationFunction); } @@ -123,14 +99,6 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - if (_selectorData.userOpValidation != validationFunction) { - // Revert if there's a different validationFunction set than the one the manifest intendes to remove. - // This - // indicates something wrong with the manifest and should not be allowed. In these cases, the original - // manifest should be passed for uninstall. - revert UserOpValidationFunctionNotSet(selector, validationFunction); - } - _selectorData.userOpValidation = FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; } @@ -140,7 +108,7 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - if (_selectorData.runtimeValidation != FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { + if (!_selectorData.runtimeValidation.isEmpty()) { revert RuntimeValidationFunctionAlreadySet(selector, validationFunction); } @@ -153,53 +121,23 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - if (_selectorData.runtimeValidation != validationFunction) { - // Revert if there's a different validationFunction set than the one the manifest intendes to remove. - // This - // indicates something wrong with the manifest and should not be allowed. In these cases, the original - // manifest should be passed for uninstall. - revert RuntimeValidationFunctionNotSet(selector, validationFunction); - } - _selectorData.runtimeValidation = FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; } function _addExecHooks(bytes4 selector, FunctionReference preExecHook, FunctionReference postExecHook) internal - notNullFunction(preExecHook) { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - if (!_selectorData.preExecHooks.add(_toSetValue(preExecHook))) { - // Treat the pre-exec and post-exec hook as a single unit, identified by the pre-exec hook. - // If the pre-exec hook exists, revert. - revert ExecutionHookAlreadySet(selector, preExecHook); - } - - if (postExecHook != FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { - _selectorData.associatedPostExecHooks[preExecHook] = postExecHook; - } + _addHooks(_selectorData.executionHooks, preExecHook, postExecHook); } function _removeExecHooks(bytes4 selector, FunctionReference preExecHook, FunctionReference postExecHook) internal - notNullFunction(preExecHook) { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - // Removal also clears the flags. - if (!_selectorData.preExecHooks.remove(_toSetValue(preExecHook))) { - revert ExecutionHookNotSet(selector, preExecHook); - } - - // Remove the associated post-exec hook, if it is set to the expected value. - if (postExecHook != _selectorData.associatedPostExecHooks[preExecHook]) { - revert InvalidPostExecHook(selector, postExecHook); - } - // If the post exec hook is set, clear it. - if (postExecHook != FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { - _selectorData.associatedPostExecHooks[preExecHook] = FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; - } + _removeHooks(_selectorData.executionHooks, preExecHook, postExecHook); } function _enableExecFromPlugin(bytes4 selector, address plugin, AccountStorage storage accountStorage) @@ -207,13 +145,8 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 { bytes24 key = getPermittedCallKey(plugin, selector); - if (accountStorage.selectorData[selector].plugin == address(0)) { - revert PermittedExecutionSelectorNotInstalled(selector, plugin); - } - - if (accountStorage.permittedCalls[key].callPermitted) { - revert ExecuteFromPluginAlreadySet(selector, plugin); - } + // If there are duplicates, this will just enable the flag again. This is not a problem, since the boolean + // will be set to false twice during uninstall, which is fine. accountStorage.permittedCalls[key].callPermitted = true; } @@ -221,9 +154,6 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 internal { bytes24 key = getPermittedCallKey(plugin, selector); - if (!accountStorage.permittedCalls[key].callPermitted) { - revert ExecuteFromPluginNotSet(selector, plugin); - } accountStorage.permittedCalls[key].callPermitted = false; } @@ -232,19 +162,11 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 address plugin, FunctionReference preExecHook, FunctionReference postExecHook - ) internal notNullPlugin(plugin) notNullFunction(preExecHook) { + ) internal notNullPlugin(plugin) { bytes24 permittedCallKey = getPermittedCallKey(plugin, selector); PermittedCallData storage _permittedCalldata = getAccountStorage().permittedCalls[permittedCallKey]; - if (!_permittedCalldata.prePermittedCallHooks.add(_toSetValue(preExecHook))) { - // Treat the pre-exec and post-exec hook as a single unit, identified by the pre-exec hook. - // If the pre-exec hook exists, revert. - revert PermittedCallHookAlreadySet(selector, plugin, preExecHook); - } - - if (postExecHook != FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { - _permittedCalldata.associatedPostPermittedCallHooks[preExecHook] = postExecHook; - } + _addHooks(_permittedCalldata.permittedCallHooks, preExecHook, postExecHook); } function _removePermittedCallHooks( @@ -252,22 +174,46 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 address plugin, FunctionReference preExecHook, FunctionReference postExecHook - ) internal notNullPlugin(plugin) notNullFunction(preExecHook) { + ) internal notNullPlugin(plugin) { bytes24 permittedCallKey = getPermittedCallKey(plugin, selector); - PermittedCallData storage _permittedCalldata = getAccountStorage().permittedCalls[permittedCallKey]; + PermittedCallData storage _permittedCallData = getAccountStorage().permittedCalls[permittedCallKey]; - if (!_permittedCalldata.prePermittedCallHooks.remove(_toSetValue(preExecHook))) { - revert PermittedCallHookNotSet(selector, plugin, preExecHook); - } + _removeHooks(_permittedCallData.permittedCallHooks, preExecHook, postExecHook); + } - // Remove the associated post-exec hook, if it is set to the expected value. - if (postExecHook != _permittedCalldata.associatedPostPermittedCallHooks[preExecHook]) { - revert InvalidPostPermittedCallHook(selector, postExecHook); + function _addHooks(HookGroup storage hooks, FunctionReference preExecHook, FunctionReference postExecHook) + internal + { + if (!preExecHook.isEmpty()) { + _addOrIncrement(hooks.preHooks, _toSetValue(preExecHook)); + + if (!postExecHook.isEmpty()) { + _addOrIncrement(hooks.associatedPostHooks[preExecHook], _toSetValue(postExecHook)); + } + } else { + if (postExecHook.isEmpty()) { + // both pre and post hooks cannot be null + revert NullFunctionReference(); + } + + _addOrIncrement(hooks.postOnlyHooks, _toSetValue(postExecHook)); } - // If the post permitted call exec hook is set, clear it. - if (postExecHook != FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { - _permittedCalldata.associatedPostPermittedCallHooks[preExecHook] = - FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; + } + + function _removeHooks(HookGroup storage hooks, FunctionReference preExecHook, FunctionReference postExecHook) + internal + { + if (!preExecHook.isEmpty()) { + _removeOrDecrement(hooks.preHooks, _toSetValue(preExecHook)); + + if (!postExecHook.isEmpty()) { + _removeOrDecrement(hooks.associatedPostHooks[preExecHook], _toSetValue(postExecHook)); + } + } else { + // The case where both pre and post hooks are null was checked during installation. + + // May ignore return value, as the manifest hash is validated to ensure that the hook exists. + _removeOrDecrement(hooks.postOnlyHooks, _toSetValue(postExecHook)); } } @@ -275,52 +221,42 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 internal notNullFunction(preUserOpValidationHook) { - if ( - !getAccountStorage().selectorData[selector].preUserOpValidationHooks.add( - _toSetValue(preUserOpValidationHook) - ) - ) { - revert PreUserOpValidationHookAlreadySet(selector, preUserOpValidationHook); - } + _addOrIncrement( + getAccountStorage().selectorData[selector].preUserOpValidationHooks, + _toSetValue(preUserOpValidationHook) + ); } function _removePreUserOpValidationHook(bytes4 selector, FunctionReference preUserOpValidationHook) internal notNullFunction(preUserOpValidationHook) { - if ( - !getAccountStorage().selectorData[selector].preUserOpValidationHooks.remove( - _toSetValue(preUserOpValidationHook) - ) - ) { - revert PreUserOpValidationHookNotSet(selector, preUserOpValidationHook); - } + // May ignore return value, as the manifest hash is validated to ensure that the hook exists. + _removeOrDecrement( + getAccountStorage().selectorData[selector].preUserOpValidationHooks, + _toSetValue(preUserOpValidationHook) + ); } function _addPreRuntimeValidationHook(bytes4 selector, FunctionReference preRuntimeValidationHook) internal notNullFunction(preRuntimeValidationHook) { - if ( - !getAccountStorage().selectorData[selector].preRuntimeValidationHooks.add( - _toSetValue(preRuntimeValidationHook) - ) - ) { - revert PreRuntimeValidationHookAlreadySet(selector, preRuntimeValidationHook); - } + _addOrIncrement( + getAccountStorage().selectorData[selector].preRuntimeValidationHooks, + _toSetValue(preRuntimeValidationHook) + ); } function _removePreRuntimeValidationHook(bytes4 selector, FunctionReference preRuntimeValidationHook) internal notNullFunction(preRuntimeValidationHook) { - if ( - !getAccountStorage().selectorData[selector].preRuntimeValidationHooks.remove( - _toSetValue(preRuntimeValidationHook) - ) - ) { - revert PreRuntimeValidationHookNotSet(selector, preRuntimeValidationHook); - } + // May ignore return value, as the manifest hash is validated to ensure that the hook exists. + _removeOrDecrement( + getAccountStorage().selectorData[selector].preRuntimeValidationHooks, + _toSetValue(preRuntimeValidationHook) + ); } function _installPlugin( @@ -382,9 +318,15 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 // Update components according to the manifest. // All conflicts should revert. + + // Mark whether or not this plugin may spend native token amounts + if (manifest.canSpendNativeToken) { + _storage.pluginData[plugin].canSpendNativeToken = true; + } + length = manifest.executionFunctions.length; for (uint256 i = 0; i < length;) { - _setExecutionFunction(manifest.executionFunctions[i].selector, plugin); + _setExecutionFunction(manifest.executionFunctions[i], plugin); unchecked { ++i; @@ -402,7 +344,7 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 } // Add the permitted external calls to the account. - if (manifest.permitAnyExternalContract) { + if (manifest.permitAnyExternalAddress) { _storage.pluginData[plugin].anyExternalExecPermitted = true; } else { // Only store the specific permitted external calls if "permit any" flag was not set. @@ -615,7 +557,7 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 revert PluginInstallCallbackFailed(plugin, revertReason); } - emit PluginInstalled(plugin, manifestHash); + emit PluginInstalled(plugin, manifestHash, dependencies, injectedHooks); } function _uninstallPlugin( @@ -773,7 +715,7 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 // remove external call permissions - if (manifest.permitAnyExternalContract) { + if (manifest.permitAnyExternalAddress) { // Only clear if it was set during install time _storage.pluginData[plugin].anyExternalExecPermitted = false; } else { @@ -839,7 +781,7 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 length = manifest.executionFunctions.length; for (uint256 i = 0; i < length;) { - _removeExecutionFunction(manifest.executionFunctions[i].selector); + _removeExecutionFunction(manifest.executionFunctions[i]); unchecked { ++i; @@ -893,7 +835,26 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 onUninstallSuccess = false; } - emit PluginUninstalled(plugin, manifestHash, onUninstallSuccess); + emit PluginUninstalled(plugin, onUninstallSuccess); + } + + function _addOrIncrement(EnumerableMap.Bytes32ToUintMap storage map, bytes32 key) internal { + (bool success, uint256 value) = map.tryGet(key); + map.set(key, success ? value + 1 : 0); + } + + /// @return True if the key was removed or its value was decremented, false if the key was not found. + function _removeOrDecrement(EnumerableMap.Bytes32ToUintMap storage map, bytes32 key) internal returns (bool) { + (bool success, uint256 value) = map.tryGet(key); + if (!success) { + return false; + } + if (value == 0) { + map.remove(key); + } else { + map.set(key, value - 1); + } + return true; } function _toSetValue(FunctionReference functionReference) internal pure returns (bytes32) { diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index be4dc126..0caf3b13 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -1,34 +1,38 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.19; -import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; +import {BaseAccount} from "@eth-infinitism/account-abstraction/core/BaseAccount.sol"; +import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; - import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; +import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; -import {BaseAccount} from "@eth-infinitism/account-abstraction/core/BaseAccount.sol"; -import {BaseModularAccount} from "./BaseModularAccount.sol"; -import {BaseModularAccountLoupe} from "./BaseModularAccountLoupe.sol"; +import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; + +import {AccountExecutor} from "./AccountExecutor.sol"; +import {AccountLoupe} from "./AccountLoupe.sol"; +import {AccountStorage, HookGroup, getAccountStorage, getPermittedCallKey} from "../libraries/AccountStorage.sol"; +import {AccountStorageInitializable} from "./AccountStorageInitializable.sol"; +import {FunctionReference, FunctionReferenceLib} from "../libraries/FunctionReferenceLib.sol"; import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol"; -import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol"; import {IPluginExecutor} from "../interfaces/IPluginExecutor.sol"; -import {AccountStorage, getAccountStorage, getPermittedCallKey} from "../libraries/AccountStorage.sol"; -import {Execution} from "../libraries/ERC6900TypeUtils.sol"; -import {FunctionReference, FunctionReferenceLib} from "../libraries/FunctionReferenceLib.sol"; -import {AccountStorageInitializable} from "./AccountStorageInitializable.sol"; import {IPluginManager} from "../interfaces/IPluginManager.sol"; +import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol"; +import {PluginManagerInternals} from "./PluginManagerInternals.sol"; import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationDataHelpers.sol"; contract UpgradeableModularAccount is - IPluginManager, - BaseAccount, - BaseModularAccount, - BaseModularAccountLoupe, - UUPSUpgradeable, + AccountExecutor, + AccountLoupe, AccountStorageInitializable, + BaseAccount, + IERC165, + IPluginExecutor, IStandardExecutor, - IPluginExecutor + PluginManagerInternals, + UUPSUpgradeable { + using EnumerableMap for EnumerableMap.Bytes32ToUintMap; using EnumerableSet for EnumerableSet.Bytes32Set; struct PostExecToRun { @@ -38,6 +42,10 @@ contract UpgradeableModularAccount is IEntryPoint private immutable _ENTRY_POINT; + // As per the EIP-165 spec, no interface should ever match 0xffffffff + bytes4 internal constant _INTERFACE_ID_INVALID = 0xffffffff; + bytes4 internal constant _IERC165_INTERFACE_ID = 0x01ffc9a7; + event ModularAccountInitialized(IEntryPoint indexed entryPoint); error AlwaysDenyRule(); @@ -45,6 +53,7 @@ contract UpgradeableModularAccount is error ExecFromPluginNotPermitted(address plugin, bytes4 selector); error ExecFromPluginExternalNotPermitted(address plugin, address target, uint256 value, bytes data); error InvalidConfiguration(); + error NativeTokenSpendingNotPermitted(address plugin); error PostExecHookReverted(address plugin, uint8 functionId, bytes revertReason); error PreExecHookReverted(address plugin, uint8 functionId, bytes revertReason); error PreRuntimeValidationHookFailed(address plugin, uint8 functionId, bytes revertReason); @@ -59,7 +68,7 @@ contract UpgradeableModularAccount is modifier wrapNativeFunction() { _doRuntimeValidationIfNotFromEP(); - PostExecToRun[] memory postExecHooks = _doPreExecHooks(msg.sig); + PostExecToRun[] memory postExecHooks = _doPreExecHooks(msg.sig, msg.data); _; @@ -120,7 +129,7 @@ contract UpgradeableModularAccount is PostExecToRun[] memory postExecHooks; // Cache post-exec hooks in memory - postExecHooks = _doPreExecHooks(msg.sig); + postExecHooks = _doPreExecHooks(msg.sig, msg.data); // execute the function, bubbling up any reverts (bool execSuccess, bytes memory execReturnData) = execPlugin.call(msg.data); @@ -137,35 +146,30 @@ contract UpgradeableModularAccount is return execReturnData; } - /// @notice Executes a transaction from the account - /// @param execution The execution to perform - /// @return result The result of the execution - function execute(Execution calldata execution) + /// @inheritdoc IStandardExecutor + function execute(address target, uint256 value, bytes calldata data) external payable override wrapNativeFunction returns (bytes memory result) { - result = _exec(execution.target, execution.value, execution.data); + result = _exec(target, value, data); } - /// @notice Executes a batch of transactions from the account - /// @dev If any of the transactions revert, the entire batch reverts - /// @param executions The executions to perform - /// @return results The results of the executions - function executeBatch(Execution[] calldata executions) + /// @inheritdoc IStandardExecutor + function executeBatch(Call[] calldata calls) external payable override wrapNativeFunction returns (bytes[] memory results) { - uint256 executionsLength = executions.length; - results = new bytes[](executionsLength); + uint256 callsLength = calls.length; + results = new bytes[](callsLength); - for (uint256 i = 0; i < executionsLength;) { - results[i] = _exec(executions[i].target, executions[i].value, executions[i].data); + for (uint256 i = 0; i < callsLength;) { + results[i] = _exec(calls[i].target, calls[i].value, calls[i].data); unchecked { ++i; @@ -173,10 +177,7 @@ contract UpgradeableModularAccount is } } - /// @notice Executes a call from a plugin to another plugin - /// @dev Permissions must be granted to the calling plugin for the call to go through - /// @param data calldata to send to the plugin - /// @return The result of the call + /// @inheritdoc IPluginExecutor function executeFromPlugin(bytes calldata data) external payable override returns (bytes memory) { bytes4 selector = bytes4(data[:4]); address callingPlugin = msg.sender; @@ -189,7 +190,8 @@ contract UpgradeableModularAccount is revert ExecFromPluginNotPermitted(callingPlugin, selector); } - PostExecToRun[] memory postPermittedCallHooks = _doPrePermittedCallHooks(selector, callingPlugin); + PostExecToRun[] memory postPermittedCallHooks = + _doPrePermittedCallHooks(getPermittedCallKey(callingPlugin, selector), data); address execFunctionPlugin = _storage.selectorData[selector].plugin; @@ -197,7 +199,7 @@ contract UpgradeableModularAccount is revert UnrecognizedFunction(selector); } - PostExecToRun[] memory postExecHooks = _doPreExecHooks(selector); + PostExecToRun[] memory postExecHooks = _doPreExecHooks(selector, data); (bool success, bytes memory returnData) = execFunctionPlugin.call(data); @@ -213,12 +215,7 @@ contract UpgradeableModularAccount is return returnData; } - /// @notice Executes a call from a plugin to a non-plugin address - /// @dev Permissions must be granted to the calling plugin for the call to go through - /// @param target address of the target to call - /// @param value value to send with the call - /// @param data calldata to send to the target - /// @return The result of the call + /// @inheritdoc IPluginExecutor function executeFromPluginExternal(address target, uint256 value, bytes calldata data) external payable @@ -227,6 +224,11 @@ contract UpgradeableModularAccount is bytes4 selector = bytes4(data); AccountStorage storage _storage = getAccountStorage(); + // Make sure plugin is allowed to spend native token. + if (value > 0 && value > msg.value && !_storage.pluginData[msg.sender].canSpendNativeToken) { + revert NativeTokenSpendingNotPermitted(msg.sender); + } + // Check the caller plugin's permission to make this call // Check the target contract permission. @@ -251,11 +253,13 @@ contract UpgradeableModularAccount is // Run any pre plugin exec specific to this caller and the `executeFromPluginExternal` selector - PostExecToRun[] memory postPermittedCallHooks = - _doPrePermittedCallHooks(IPluginExecutor.executeFromPluginExternal.selector, msg.sender); + PostExecToRun[] memory postPermittedCallHooks = _doPrePermittedCallHooks( + getPermittedCallKey(msg.sender, IPluginExecutor.executeFromPluginExternal.selector), msg.data + ); // Run any pre exec hooks for this selector - PostExecToRun[] memory postExecHooks = _doPreExecHooks(IPluginExecutor.executeFromPluginExternal.selector); + PostExecToRun[] memory postExecHooks = + _doPreExecHooks(IPluginExecutor.executeFromPluginExternal.selector, msg.data); // Perform the external call bytes memory returnData = _exec(target, value, data); @@ -361,22 +365,23 @@ contract UpgradeableModularAccount is UserOperation calldata userOp, bytes32 userOpHash ) internal returns (uint256 validationData) { - if (userOpValidationFunction == FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { + if (userOpValidationFunction.isEmpty()) { revert UserOpValidationFunctionMissing(selector); } uint256 currentValidationData; // Do preUserOpValidation hooks - EnumerableSet.Bytes32Set storage preUserOpValidationHooks = + EnumerableMap.Bytes32ToUintMap storage preUserOpValidationHooks = getAccountStorage().selectorData[selector].preUserOpValidationHooks; uint256 preUserOpValidationHooksLength = preUserOpValidationHooks.length(); for (uint256 i = 0; i < preUserOpValidationHooksLength;) { - // FunctionReference preUserOpValidationHook = preUserOpValidationHooks[i]; + (bytes32 key,) = preUserOpValidationHooks.at(i); + FunctionReference preUserOpValidationHook = _toFunctionReference(key); - if (!_toFunctionReference(preUserOpValidationHooks.at(i)).isEmptyOrMagicValue()) { - (address plugin, uint8 functionId) = _toFunctionReference(preUserOpValidationHooks.at(i)).unpack(); + if (!preUserOpValidationHook.isEmptyOrMagicValue()) { + (address plugin, uint8 functionId) = preUserOpValidationHook.unpack(); try IPlugin(plugin).preUserOpValidationHook(functionId, userOp, userOpHash) returns ( uint256 returnData ) { @@ -430,12 +435,13 @@ contract UpgradeableModularAccount is AccountStorage storage _storage = getAccountStorage(); FunctionReference runtimeValidationFunction = _storage.selectorData[msg.sig].runtimeValidation; // run all preRuntimeValidation hooks - EnumerableSet.Bytes32Set storage preRuntimeValidationHooks = + EnumerableMap.Bytes32ToUintMap storage preRuntimeValidationHooks = getAccountStorage().selectorData[msg.sig].preRuntimeValidationHooks; uint256 preRuntimeValidationHooksLength = preRuntimeValidationHooks.length(); for (uint256 i = 0; i < preRuntimeValidationHooksLength;) { - FunctionReference preRuntimeValidationHook = _toFunctionReference(preRuntimeValidationHooks.at(i)); + (bytes32 key,) = preRuntimeValidationHooks.at(i); + FunctionReference preRuntimeValidationHook = _toFunctionReference(key); if (!preRuntimeValidationHook.isEmptyOrMagicValue()) { (address plugin, uint8 functionId) = preRuntimeValidationHook.unpack(); @@ -467,7 +473,7 @@ contract UpgradeableModularAccount is revert RuntimeValidationFunctionReverted(plugin, functionId, revertReason); } } else { - if (runtimeValidationFunction == FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { + if (runtimeValidationFunction.isEmpty()) { revert RuntimeValidationFunctionMissing(msg.sig); } else if (runtimeValidationFunction == FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY) { revert InvalidConfiguration(); @@ -477,125 +483,124 @@ contract UpgradeableModularAccount is } } - function _doPreExecHooks(bytes4 selector) internal returns (PostExecToRun[] memory postHooksToRun) { - EnumerableSet.Bytes32Set storage preExecHooks = getAccountStorage().selectorData[selector].preExecHooks; - - uint256 postExecHooksLength = 0; - uint256 preExecHooksLength = preExecHooks.length(); + function _doPreExecHooks(bytes4 selector, bytes calldata data) + internal + returns (PostExecToRun[] memory postHooksToRun) + { + HookGroup storage hooks = getAccountStorage().selectorData[selector].executionHooks; - // Over-allocate on length, but not all of this may get filled up. - postHooksToRun = new PostExecToRun[](preExecHooksLength); - for (uint256 i = 0; i < preExecHooksLength;) { - FunctionReference preExecHook = _toFunctionReference(preExecHooks.at(i)); + return _doPreHooks(hooks, data); + } - if (preExecHook.isEmptyOrMagicValue()) { - if (preExecHook == FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY) { - revert AlwaysDenyRule(); - } - // Function reference cannot be 0. If _RUNTIME_VALIDATION_ALWAYS_ALLOW, revert since it's an - // invalid configuration. - revert InvalidConfiguration(); - } + function _doPrePermittedCallHooks(bytes24 permittedCallKey, bytes calldata data) + internal + returns (PostExecToRun[] memory postHooksToRun) + { + HookGroup storage hooks = getAccountStorage().permittedCalls[permittedCallKey].permittedCallHooks; - (address plugin, uint8 functionId) = preExecHook.unpack(); - bytes memory preExecHookReturnData; - try IPlugin(plugin).preExecutionHook(functionId, msg.sender, msg.value, msg.data) returns ( - bytes memory returnData - ) { - preExecHookReturnData = returnData; - } catch (bytes memory revertReason) { - revert PreExecHookReverted(plugin, functionId, revertReason); - } + return _doPreHooks(hooks, data); + } - // Check to see if there is a postExec hook set for this preExec hook - FunctionReference postExecHook = - getAccountStorage().selectorData[selector].associatedPostExecHooks[preExecHook]; - if (postExecHook != FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { - postHooksToRun[postExecHooksLength].postExecHook = postExecHook; - postHooksToRun[postExecHooksLength].preExecHookReturnData = preExecHookReturnData; - unchecked { - ++postExecHooksLength; - } - } + function _doPreHooks(HookGroup storage hooks, bytes calldata data) + internal + returns (PostExecToRun[] memory postHooksToRun) + { + uint256 preExecHooksLength = hooks.preHooks.length(); + uint256 postOnlyHooksLength = hooks.postOnlyHooks.length(); + uint256 maxPostExecHooksLength = postOnlyHooksLength; + // There can only be as many associated post hooks to run as there are pre hooks. + for (uint256 i = 0; i < preExecHooksLength;) { + (, uint256 count) = hooks.preHooks.at(i); unchecked { + maxPostExecHooksLength += (count + 1); ++i; } } - } - function _doPrePermittedCallHooks(bytes4 executionSelector, address callerPlugin) - internal - returns (PostExecToRun[] memory postHooksToRun) - { - bytes24 permittedCallKey = getPermittedCallKey(callerPlugin, executionSelector); + // Overallocate on length - not all of this may get filled up. We set the correct length later. + postHooksToRun = new PostExecToRun[](maxPostExecHooksLength); + uint256 actualPostHooksToRunLength; - EnumerableSet.Bytes32Set storage preExecHooks = - getAccountStorage().permittedCalls[permittedCallKey].prePermittedCallHooks; + // Copy post-only hooks to the array. + for (uint256 i = 0; i < postOnlyHooksLength;) { + (bytes32 key,) = hooks.postOnlyHooks.at(i); + postHooksToRun[actualPostHooksToRunLength].postExecHook = _toFunctionReference(key); + unchecked { + ++actualPostHooksToRunLength; + ++i; + } + } - uint256 postExecHooksLength = 0; - uint256 preExecHooksLength = preExecHooks.length(); - postHooksToRun = new PostExecToRun[](preExecHooksLength); // Over-allocate on length, but not all of this - // may get filled up. + // Then run the pre hooks and copy the associated post hooks (along with their pre hook's return data) to + // the array. for (uint256 i = 0; i < preExecHooksLength;) { - FunctionReference preExecHook = _toFunctionReference(preExecHooks.at(i)); + (bytes32 key,) = hooks.preHooks.at(i); + FunctionReference preExecHook = _toFunctionReference(key); if (preExecHook.isEmptyOrMagicValue()) { - if (preExecHook == FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY) { - revert AlwaysDenyRule(); - } - // Function reference cannot be 0. If RUNTIME_VALIDATION_BYPASS, revert since it's an invalid - // configuration. - revert InvalidConfiguration(); - } - - (address plugin, uint8 functionId) = preExecHook.unpack(); - bytes memory preExecHookReturnData; - try IPlugin(plugin).preExecutionHook(functionId, msg.sender, msg.value, msg.data) returns ( - bytes memory returnData - ) { - preExecHookReturnData = returnData; - } catch (bytes memory revertReason) { - revert PreExecHookReverted(plugin, functionId, revertReason); + // The function reference must be PRE_HOOK_ALWAYS_DENY in this case, because zero and any other + // magic value is unassignable here. + revert AlwaysDenyRule(); } - // Check to see if there is a postExec hook set for this preExec hook - FunctionReference postExecHook = - getAccountStorage().permittedCalls[permittedCallKey].associatedPostPermittedCallHooks[preExecHook]; - if (FunctionReference.unwrap(postExecHook) != 0) { - postHooksToRun[postExecHooksLength].postExecHook = postExecHook; - postHooksToRun[postExecHooksLength].preExecHookReturnData = preExecHookReturnData; - unchecked { - ++postExecHooksLength; + uint256 associatedPostExecHooksLength = hooks.associatedPostHooks[preExecHook].length(); + if (associatedPostExecHooksLength > 0) { + for (uint256 j = 0; j < associatedPostExecHooksLength;) { + (key,) = hooks.associatedPostHooks[preExecHook].at(j); + postHooksToRun[actualPostHooksToRunLength].postExecHook = _toFunctionReference(key); + postHooksToRun[actualPostHooksToRunLength].preExecHookReturnData = + _runPreExecHook(preExecHook, data); + + unchecked { + ++actualPostHooksToRunLength; + ++j; + } } + } else { + _runPreExecHook(preExecHook, data); } unchecked { ++i; } } + + // Trim the post hook array to the actual length, since we may have overallocated. + assembly ("memory-safe") { + mstore(postHooksToRun, actualPostHooksToRunLength) + } + } + + function _runPreExecHook(FunctionReference preExecHook, bytes calldata data) + internal + returns (bytes memory preExecHookReturnData) + { + (address plugin, uint8 functionId) = preExecHook.unpack(); + try IPlugin(plugin).preExecutionHook(functionId, msg.sender, msg.value, data) returns ( + bytes memory returnData + ) { + preExecHookReturnData = returnData; + } catch (bytes memory revertReason) { + revert PreExecHookReverted(plugin, functionId, revertReason); + } } + /// @dev Associated post hooks are run in reverse order of their pre hooks. function _doCachedPostExecHooks(PostExecToRun[] memory postHooksToRun) internal { uint256 postHooksToRunLength = postHooksToRun.length; - for (uint256 i = 0; i < postHooksToRunLength;) { - PostExecToRun memory postHookToRun = postHooksToRun[i]; - FunctionReference postExecHook = postHookToRun.postExecHook; - if (postExecHook == FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) { - // Reached the end of runnable postExec hooks, stop. - // Array may be over-allocated. - return; + for (uint256 i = postHooksToRunLength; i > 0;) { + unchecked { + --i; } + + PostExecToRun memory postHookToRun = postHooksToRun[i]; (address plugin, uint8 functionId) = postHookToRun.postExecHook.unpack(); // solhint-disable-next-line no-empty-blocks try IPlugin(plugin).postExecutionHook(functionId, postHookToRun.preExecHookReturnData) {} catch (bytes memory revertReason) { revert PostExecHookReverted(plugin, functionId, revertReason); } - - unchecked { - ++i; - } } } diff --git a/src/interfaces/IAccountLoupe.sol b/src/interfaces/IAccountLoupe.sol new file mode 100644 index 00000000..cdf1525f --- /dev/null +++ b/src/interfaces/IAccountLoupe.sol @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.19; + +import {FunctionReference} from "../libraries/FunctionReferenceLib.sol"; + +interface IAccountLoupe { + /// @notice Config for an execution function, given a selector + struct ExecutionFunctionConfig { + address plugin; + FunctionReference userOpValidationFunction; + FunctionReference runtimeValidationFunction; + } + + /// @notice Pre and post hooks for a given selector + /// @dev It's possible for one of either `preExecHook` or `postExecHook` to be empty + struct ExecutionHooks { + FunctionReference preExecHook; + FunctionReference postExecHook; + } + + /// @notice Gets the validation functions and plugin address for a selector + /// @dev If the selector is a native function, the plugin address will be the address of the account + /// @param selector The selector to get the configuration for + /// @return The configuration for this selector + function getExecutionFunctionConfig(bytes4 selector) external view returns (ExecutionFunctionConfig memory); + + /// @notice Gets the pre and post execution hooks for a selector + /// @param selector The selector to get the hooks for + /// @return The pre and post execution hooks for this selector + function getExecutionHooks(bytes4 selector) external view returns (ExecutionHooks[] memory); + + /// @notice Gets the pre and post permitted call hooks applied for a plugin calling this selector + /// @param callingPlugin The plugin that is calling the selector + /// @param selector The selector the plugin is calling + /// @return The pre and post permitted call hooks for this selector + function getPermittedCallHooks(address callingPlugin, bytes4 selector) + external + view + returns (ExecutionHooks[] memory); + + /// @notice Gets the pre user op and runtime validation hooks associated with a selector + /// @param selector The selector to get the hooks for + /// @return preUserOpValidationHooks The pre user op validation hooks for this selector + /// @return preRuntimeValidationHooks The pre runtime validation hooks for this selector + function getPreValidationHooks(bytes4 selector) + external + view + returns ( + FunctionReference[] memory preUserOpValidationHooks, + FunctionReference[] memory preRuntimeValidationHooks + ); + + /// @notice Gets an array of all installed plugins + /// @return The addresses of all installed plugins + function getInstalledPlugins() external view returns (address[] memory); +} diff --git a/src/interfaces/IPlugin.sol b/src/interfaces/IPlugin.sol index c0fa8edc..c3e27b0a 100644 --- a/src/interfaces/IPlugin.sol +++ b/src/interfaces/IPlugin.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.19; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; + import {IPluginManager} from "./IPluginManager.sol"; // Forge formatter will displace the first comment for the enum field out of the enum itself, @@ -53,12 +54,13 @@ struct ManifestExternalCallPermission { bytes4[] selectors; } -struct ManifestExecutionFunction { - bytes4 selector; - string[] permissions; +struct SelectorPermission { + bytes4 functionSelector; + string permissionDescription; } -struct PluginManifest { +/// @dev A struct holding fields to describe the plugin in a purely view context. Intended for front end clients. +struct PluginMetadata { // A human-readable name of the plugin. string name; // The version of the plugin, following the semantic versioning scheme. @@ -66,6 +68,13 @@ struct PluginManifest { // The author field SHOULD be a username representing the identity of the user or organization // that created this plugin. string author; + // String desciptions of the relative sensitivity of specific functions. The selectors MUST be selectors for + // functions implemented by this plugin. + SelectorPermission[] permissionDescriptors; +} + +/// @dev A struct describing how the plugin should be installed on a modular account. +struct PluginManifest { // List of ERC-165 interfaceIds to add to account to support introspection checks. bytes4[] interfaceIds; // If this plugin depends on other plugins' validation functions and/or hooks, the interface IDs of @@ -73,12 +82,13 @@ struct PluginManifest { // members of `ManifestFunction` structs used in the manifest. bytes4[] dependencyInterfaceIds; // Execution functions defined in this plugin to be installed on the MSCA. - ManifestExecutionFunction[] executionFunctions; - // Native functions or execution functions already installed on the MSCA that this plugin will be - // able to call. + bytes4[] executionFunctions; + // Plugin execution functions already installed on the MSCA that this plugin will be able to call. bytes4[] permittedExecutionSelectors; - // External contract calls that this plugin will be able to make. - bool permitAnyExternalContract; + // Boolean to indicate whether the plugin can call any external contract addresses. + bool permitAnyExternalAddress; + // Boolean to indicate whether the plugin needs access to spend native tokens of the account. + bool canSpendNativeToken; ManifestExternalCallPermission[] permittedExternalCalls; ManifestAssociatedFunction[] userOpValidationFunctions; ManifestAssociatedFunction[] runtimeValidationFunctions; @@ -187,4 +197,9 @@ interface IPlugin { /// @dev This manifest MUST stay constant over time. /// @return A manifest describing the contents and intended configuration of the plugin. function pluginManifest() external pure returns (PluginManifest memory); + + /// @notice Describe the metadata of the plugin. + /// @dev This metadata MUST stay constant over time. + /// @return A metadata struct describing the plugin. + function pluginMetadata() external pure returns (PluginMetadata memory); } diff --git a/src/interfaces/IPluginExecutor.sol b/src/interfaces/IPluginExecutor.sol index c5e3b7e7..31269e00 100644 --- a/src/interfaces/IPluginExecutor.sol +++ b/src/interfaces/IPluginExecutor.sol @@ -2,16 +2,18 @@ pragma solidity ^0.8.19; interface IPluginExecutor { - /// @notice Method from calls made from plugins. - /// @param data The call data for the call. - /// @return The return data from the call. + /// @notice Executes a call from a plugin to another plugin. + /// @dev Permissions must be granted to the calling plugin for the call to go through. + /// @param data calldata to send to the plugin + /// @return The result of the call function executeFromPlugin(bytes calldata data) external payable returns (bytes memory); - /// @notice Method from calls made from plugins. - /// @dev If the target is a plugin, the call SHOULD revert. - /// @param target The target of the external contract to be called. - /// @param value The value to pass. - /// @param data The data to pass. + /// @notice Executes a call from a plugin to a non-plugin address. + /// @dev Permissions must be granted to the calling plugin for the call to go through. + /// @param target address of the target to call + /// @param value value to send with the call + /// @param data calldata to send to the target + /// @return The result of the call function executeFromPluginExternal(address target, uint256 value, bytes calldata data) external payable diff --git a/src/interfaces/IPluginLoupe.sol b/src/interfaces/IPluginLoupe.sol deleted file mode 100644 index 797c5567..00000000 --- a/src/interfaces/IPluginLoupe.sol +++ /dev/null @@ -1,33 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0 -pragma solidity ^0.8.19; - -import {FunctionReference} from "../libraries/FunctionReferenceLib.sol"; - -interface IPluginLoupe { - // Config for a Plugin Execution function - struct ExecutionFunctionConfig { - address plugin; - FunctionReference userOpValidationFunction; - FunctionReference runtimeValidationFunction; - } - - struct ExecutionHooks { - FunctionReference preExecHook; - FunctionReference postExecHook; - } - - function getExecutionFunctionConfig(bytes4 selector) external view returns (ExecutionFunctionConfig memory); - - function getExecutionHooks(bytes4 selector) external view returns (ExecutionHooks[] memory); - - function getPermittedCallHooks(address callingPlugin, bytes4 selector) - external - view - returns (ExecutionHooks[] memory); - - function getPreUserOpValidationHooks(bytes4 selector) external view returns (FunctionReference[] memory); - - function getPreRuntimeValidationHooks(bytes4 selector) external view returns (FunctionReference[] memory); - - function getInstalledPlugins() external view returns (address[] memory); -} diff --git a/src/interfaces/IPluginManager.sol b/src/interfaces/IPluginManager.sol index e646a501..7b7dd85f 100644 --- a/src/interfaces/IPluginManager.sol +++ b/src/interfaces/IPluginManager.sol @@ -3,9 +3,10 @@ pragma solidity ^0.8.19; import {FunctionReference} from "../libraries/FunctionReferenceLib.sol"; +/// @title Plugin Manager Interface interface IPluginManager { - // Pre/post exec hooks added by the user to limit the scope a plugin has - // These hooks are injected at plugin install time + /// @dev Pre/post exec hooks added by the user to limit the scope of a plugin. These hooks are injected at + /// plugin install time struct InjectedHook { // The plugin that provides the hook address providingPlugin; @@ -21,8 +22,15 @@ interface IPluginManager { uint8 postExecHookFunctionId; } - event PluginInstalled(address indexed plugin, bytes32 manifestHash); - event PluginUninstalled(address indexed plugin, bytes32 manifestHash, bool onUninstallSucceeded); + /// @dev Note that we strip hookApplyData from InjectedHooks in this event for gas savings + event PluginInstalled( + address indexed plugin, + bytes32 manifestHash, + FunctionReference[] dependencies, + InjectedHook[] injectedHooks + ); + + event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); /// @notice Install a plugin to the modular account. /// @param plugin The plugin to install. @@ -40,6 +48,7 @@ interface IPluginManager { ) external; /// @notice Uninstall a plugin from the modular account. + /// @dev Uninstalling owner plugins outside of a replace operation via executeBatch risks losing the account! /// @param plugin The plugin to uninstall. /// @param config An optional, implementation-specific field that accounts may use to ensure consistency /// guarantees. diff --git a/src/interfaces/IStandardExecutor.sol b/src/interfaces/IStandardExecutor.sol index 15f84872..5c3c52b3 100644 --- a/src/interfaces/IStandardExecutor.sol +++ b/src/interfaces/IStandardExecutor.sol @@ -1,18 +1,29 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.19; -import {Execution} from "../libraries/ERC6900TypeUtils.sol"; +struct Call { + // The target address for account to call. + address target; + // The value sent with the call. + uint256 value; + // The call data for the call. + bytes data; +} +/// @title Standard Executor Interface interface IStandardExecutor { /// @notice Standard execute method. /// @dev If the target is a plugin, the call SHOULD revert. - /// @param execution The execution information. + /// @param target The target address for account to call. + /// @param value The value sent with the call. + /// @param data The call data for the call. /// @return The return data from the call. - function execute(Execution calldata execution) external payable returns (bytes memory); + function execute(address target, uint256 value, bytes calldata data) external payable returns (bytes memory); /// @notice Standard executeBatch method. - /// @dev If the target is a plugin, the call SHOULD revert. - /// @param executions The array of executions. + /// @dev If the target is a plugin, the call SHOULD revert. If any of the transactions revert, the entire batch + /// reverts. + /// @param calls The array of calls. /// @return An array containing the return data from the calls. - function executeBatch(Execution[] calldata executions) external payable returns (bytes[] memory); + function executeBatch(Call[] calldata calls) external payable returns (bytes[] memory); } diff --git a/src/libraries/AccountStorage.sol b/src/libraries/AccountStorage.sol index 4254e2c2..36d2845d 100644 --- a/src/libraries/AccountStorage.sol +++ b/src/libraries/AccountStorage.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.19; import {IPlugin} from "../interfaces/IPlugin.sol"; +import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {FunctionReference} from "../libraries/FunctionReferenceLib.sol"; @@ -10,6 +11,9 @@ bytes32 constant _ACCOUNT_STORAGE_SLOT = 0x9f09680beaa4e5c9f38841db2460c40149916 struct PluginData { bool anyExternalExecPermitted; + // boolean to indicate if the plugin can spend native tokens, if any of the execution function can spend + // native tokens, a plugin is considered to be able to spend native tokens of the accounts + bool canSpendNativeToken; bytes32 manifestHash; FunctionReference[] dependencies; // Tracks the number of times this plugin has been used as a dependency function @@ -17,7 +21,7 @@ struct PluginData { StoredInjectedHook[] injectedHooks; } -// A version of IPluginManager. InjectedHook used to track injected hooks in storage. +// A version of IPluginManager.InjectedHook used to track injected hooks in storage. // Omits the hookApplyData field, which is not needed for storage, and flattens the struct. struct StoredInjectedHook { // The plugin that provides the hook @@ -34,9 +38,7 @@ struct StoredInjectedHook { // to interact with another plugin installed on the account. struct PermittedCallData { bool callPermitted; - EnumerableSet.Bytes32Set prePermittedCallHooks; - // bytes21 key = pre exec hook function reference - mapping(FunctionReference => FunctionReference) associatedPostPermittedCallHooks; + HookGroup permittedCallHooks; } // Represents data associated with a plugin's permission to use `executeFromPluginExternal` @@ -49,6 +51,14 @@ struct PermittedExternalCallData { mapping(bytes4 => bool) permittedSelectors; } +// Represets a set of pre- and post- hooks. Used to store both execution hooks and permitted call hooks. +struct HookGroup { + EnumerableMap.Bytes32ToUintMap preHooks; + // bytes21 key = pre hook function reference + mapping(FunctionReference => EnumerableMap.Bytes32ToUintMap) associatedPostHooks; + EnumerableMap.Bytes32ToUintMap postOnlyHooks; +} + // Represents data associated with a specifc function selector. struct SelectorData { // The plugin that implements this execution function. @@ -57,12 +67,10 @@ struct SelectorData { FunctionReference userOpValidation; FunctionReference runtimeValidation; // The pre validation hooks for this function selector. - EnumerableSet.Bytes32Set preUserOpValidationHooks; - EnumerableSet.Bytes32Set preRuntimeValidationHooks; + EnumerableMap.Bytes32ToUintMap preUserOpValidationHooks; + EnumerableMap.Bytes32ToUintMap preRuntimeValidationHooks; // The execution hooks for this function selector. - EnumerableSet.Bytes32Set preExecHooks; - // bytes21 key = pre exec hook function reference - mapping(FunctionReference => FunctionReference) associatedPostExecHooks; + HookGroup executionHooks; } struct AccountStorage { @@ -83,7 +91,7 @@ struct AccountStorage { } function getAccountStorage() pure returns (AccountStorage storage _storage) { - assembly { + assembly ("memory-safe") { _storage.slot := _ACCOUNT_STORAGE_SLOT } } @@ -93,15 +101,21 @@ function getPermittedCallKey(address addr, bytes4 selector) pure returns (bytes2 } // Helper function to get all elements of a set into memory. -using EnumerableSet for EnumerableSet.Bytes32Set; +using EnumerableMap for EnumerableMap.Bytes32ToUintMap; -function toFunctionReferenceArray(EnumerableSet.Bytes32Set storage set) +function toFunctionReferenceArray(EnumerableMap.Bytes32ToUintMap storage map) view returns (FunctionReference[] memory) { - FunctionReference[] memory result = new FunctionReference[](set.length()); - for (uint256 i = 0; i < set.length(); i++) { - result[i] = FunctionReference.wrap(bytes21(set.at(i))); + uint256 length = map.length(); + FunctionReference[] memory result = new FunctionReference[](length); + for (uint256 i = 0; i < length;) { + (bytes32 key,) = map.at(i); + result[i] = FunctionReference.wrap(bytes21(key)); + + unchecked { + ++i; + } } return result; } diff --git a/src/libraries/ERC6900TypeUtils.sol b/src/libraries/ERC6900TypeUtils.sol deleted file mode 100644 index 9e088121..00000000 --- a/src/libraries/ERC6900TypeUtils.sol +++ /dev/null @@ -1,11 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0 -pragma solidity ^0.8.19; - -struct Execution { - // The target contract for account to execute. - address target; - // The value for the execution. - uint256 value; - // The call data for the execution. - bytes data; -} diff --git a/src/libraries/FunctionReferenceLib.sol b/src/libraries/FunctionReferenceLib.sol index 0bef591c..897f5bd1 100644 --- a/src/libraries/FunctionReferenceLib.sol +++ b/src/libraries/FunctionReferenceLib.sol @@ -25,6 +25,10 @@ library FunctionReferenceLib { functionId = uint8(bytes1(underlying << 160)); } + function isEmpty(FunctionReference fr) internal pure returns (bool) { + return fr == _EMPTY_FUNCTION_REFERENCE; + } + function isEmptyOrMagicValue(FunctionReference fr) internal pure returns (bool) { return FunctionReference.unwrap(fr) <= bytes21(uint168(2)); } diff --git a/src/plugins/BasePlugin.sol b/src/plugins/BasePlugin.sol index e03ba129..a8d610c1 100644 --- a/src/plugins/BasePlugin.sol +++ b/src/plugins/BasePlugin.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.19; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; -import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol"; +import {IPlugin, PluginManifest, PluginMetadata} from "../interfaces/IPlugin.sol"; import {IPluginManager} from "../interfaces/IPluginManager.sol"; /// @title Base contract for plugins @@ -153,6 +153,11 @@ abstract contract BasePlugin is ERC165, IPlugin { revert NotImplemented(); } + /// @notice Describe the metadata of the plugin. + /// @dev This metadata MUST stay constant over time. + /// @return A metadata struct describing the plugin. + function pluginMetadata() external pure virtual returns (PluginMetadata memory); + /// @dev Returns true if this contract implements the interface defined by /// `interfaceId`. See the corresponding /// https://eips.ethereum.org/EIPS/eip-165#how-interfaces-are-identified[EIP section] diff --git a/src/plugins/TokenReceiverPlugin.sol b/src/plugins/TokenReceiverPlugin.sol index 8190a1c9..ac9335fd 100644 --- a/src/plugins/TokenReceiverPlugin.sol +++ b/src/plugins/TokenReceiverPlugin.sol @@ -10,7 +10,7 @@ import { ManifestAssociatedFunctionType, ManifestAssociatedFunction, PluginManifest, - ManifestExecutionFunction + PluginMetadata } from "../interfaces/IPlugin.sol"; import {BasePlugin} from "./BasePlugin.sol"; @@ -72,17 +72,11 @@ contract TokenReceiverPlugin is BasePlugin, IERC721Receiver, IERC777Recipient, I function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.name = NAME; - manifest.version = VERSION; - manifest.author = AUTHOR; - - manifest.executionFunctions = new ManifestExecutionFunction[](4); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.tokensReceived.selector, new string[](0)); - manifest.executionFunctions[1] = ManifestExecutionFunction(this.onERC721Received.selector, new string[](0)); - manifest.executionFunctions[2] = - ManifestExecutionFunction(this.onERC1155Received.selector, new string[](0)); - manifest.executionFunctions[3] = - ManifestExecutionFunction(this.onERC1155BatchReceived.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](4); + manifest.executionFunctions[0] = this.tokensReceived.selector; + manifest.executionFunctions[1] = this.onERC721Received.selector; + manifest.executionFunctions[2] = this.onERC1155Received.selector; + manifest.executionFunctions[3] = this.onERC1155BatchReceived.selector; // Only runtime validationFunction is needed since callbacks come from token contracts only ManifestFunction memory alwaysAllowFunction = ManifestFunction({ @@ -115,4 +109,13 @@ contract TokenReceiverPlugin is BasePlugin, IERC721Receiver, IERC777Recipient, I return manifest; } + + /// @inheritdoc BasePlugin + function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { + PluginMetadata memory metadata; + metadata.name = NAME; + metadata.version = VERSION; + metadata.author = AUTHOR; + return metadata; + } } diff --git a/src/plugins/owner/SingleOwnerPlugin.sol b/src/plugins/owner/SingleOwnerPlugin.sol index 91143eb3..fa8ed177 100644 --- a/src/plugins/owner/SingleOwnerPlugin.sol +++ b/src/plugins/owner/SingleOwnerPlugin.sol @@ -13,7 +13,8 @@ import { ManifestAssociatedFunctionType, ManifestAssociatedFunction, PluginManifest, - ManifestExecutionFunction + PluginMetadata, + SelectorPermission } from "../../interfaces/IPlugin.sol"; import {IStandardExecutor} from "../../interfaces/IStandardExecutor.sol"; import {BasePlugin} from "../BasePlugin.sol"; @@ -139,18 +140,10 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.name = NAME; - manifest.version = VERSION; - manifest.author = AUTHOR; - - string[] memory ownerPermissions = new string[](1); - ownerPermissions[0] = "Modify Ownership"; - - manifest.executionFunctions = new ManifestExecutionFunction[](3); - manifest.executionFunctions[0] = - ManifestExecutionFunction(this.transferOwnership.selector, ownerPermissions); - manifest.executionFunctions[1] = ManifestExecutionFunction(this.isValidSignature.selector, new string[](0)); - manifest.executionFunctions[2] = ManifestExecutionFunction(this.owner.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](3); + manifest.executionFunctions[0] = this.transferOwnership.selector; + manifest.executionFunctions[1] = this.isValidSignature.selector; + manifest.executionFunctions[2] = this.owner.selector; ManifestFunction memory ownerUserOpValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, @@ -238,6 +231,25 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 { return manifest; } + /// @inheritdoc BasePlugin + function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { + PluginMetadata memory metadata; + metadata.name = NAME; + metadata.version = VERSION; + metadata.author = AUTHOR; + + // Permission strings + string memory modifyOwnershipPermission = "Modify Ownership"; + + // Permission descriptions + metadata.permissionDescriptors = new SelectorPermission[](1); + metadata.permissionDescriptors[0] = SelectorPermission({ + functionSelector: this.transferOwnership.selector, + permissionDescription: modifyOwnershipPermission + }); + + return metadata; + } // ┏━━━━━━━━━━━━━━━┓ // ┃ EIP-165 ┃ // ┗━━━━━━━━━━━━━━━┛ diff --git a/test/TestUtils.sol b/test/TestUtils.sol deleted file mode 100644 index 26631b16..00000000 --- a/test/TestUtils.sol +++ /dev/null @@ -1,21 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.19; - -import {Test, console} from "forge-std/Test.sol"; - -contract TestUtils is Test { - function printStorageReadsAndWrites(address addr) internal { - (bytes32[] memory accountReads, bytes32[] memory accountWrites) = vm.accesses(addr); - for (uint256 i = 0; i < accountWrites.length; i++) { - bytes32 valWritten = vm.load(addr, accountWrites[i]); - console.log( - string.concat("write loc: ", vm.toString(accountWrites[i]), " val: ", vm.toString(valWritten)) - ); - } - - for (uint256 i = 0; i < accountReads.length; i++) { - bytes32 valRead = vm.load(addr, accountReads[i]); - console.log(string.concat("read: ", vm.toString(accountReads[i]), " val: ", vm.toString(valRead))); - } - } -} diff --git a/test/account/AccountExecHooks.t.sol b/test/account/AccountExecHooks.t.sol new file mode 100644 index 00000000..cd10df41 --- /dev/null +++ b/test/account/AccountExecHooks.t.sol @@ -0,0 +1,485 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; +import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; + +import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; +import { + IPlugin, + ManifestAssociatedFunctionType, + ManifestAssociatedFunction, + ManifestExecutionHook, + ManifestFunction, + PluginManifest +} from "../../src/interfaces/IPlugin.sol"; +import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; +import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; +import {FunctionReference, FunctionReferenceLib} from "../../src/libraries/FunctionReferenceLib.sol"; + +import {MockPlugin} from "../mocks/MockPlugin.sol"; +import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; + +contract AccountExecHooksTest is OptimizedTest { + using ECDSA for bytes32; + + EntryPoint public entryPoint; + SingleOwnerPlugin public singleOwnerPlugin; + MSCAFactoryFixture public factory; + + UpgradeableModularAccount public account; + + MockPlugin public mockPlugin1; + MockPlugin public mockPlugin2; + bytes32 public manifestHash1; + bytes32 public manifestHash2; + + bytes4 internal constant _EXEC_SELECTOR = bytes4(uint32(1)); + uint8 internal constant _PRE_HOOK_FUNCTION_ID_1 = 1; + uint8 internal constant _POST_HOOK_FUNCTION_ID_2 = 2; + uint8 internal constant _PRE_HOOK_FUNCTION_ID_3 = 3; + uint8 internal constant _POST_HOOK_FUNCTION_ID_4 = 4; + + PluginManifest public m1; + PluginManifest public m2; + + /// @dev Note that we strip hookApplyData from InjectedHooks in this event for gas savings + event PluginInstalled( + address indexed plugin, + bytes32 manifestHash, + FunctionReference[] dependencies, + IPluginManager.InjectedHook[] injectedHooks + ); + event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); + // emitted by MockPlugin + event ReceivedCall(bytes msgData, uint256 msgValue); + + function setUp() public { + entryPoint = new EntryPoint(); + singleOwnerPlugin = _deploySingleOwnerPlugin(); + factory = new MSCAFactoryFixture(entryPoint, singleOwnerPlugin); + + // Create an account with "this" as the owner, so we can execute along the runtime path with regular + // solidity semantics + account = factory.createAccount(address(this), 0); + + m1.executionFunctions.push(_EXEC_SELECTOR); + + m1.runtimeValidationFunctions.push( + ManifestAssociatedFunction({ + executionSelector: _EXEC_SELECTOR, + associatedFunction: ManifestFunction({ + functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, + functionId: 0, + dependencyIndex: 0 + }) + }) + ); + } + + function test_preExecHook_install() public { + _installPlugin1WithHooks( + _EXEC_SELECTOR, + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_1, + dependencyIndex: 0 + }), + ManifestFunction({functionType: ManifestAssociatedFunctionType.NONE, functionId: 0, dependencyIndex: 0}) + ); + } + + /// @dev Plugin 1 hook pair: [1, null] + /// Expected execution: [1, null] + function test_preExecHook_run() public { + test_preExecHook_install(); + + vm.expectEmit(true, true, true, true); + emit ReceivedCall( + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_1, + address(this), // caller + 0, // msg.value in call to account + abi.encodeWithSelector(_EXEC_SELECTOR) + ), + 0 // msg value in call to plugin + ); + + (bool success,) = address(account).call(abi.encodeWithSelector(_EXEC_SELECTOR)); + assertTrue(success); + } + + function test_preExecHook_uninstall() public { + test_preExecHook_install(); + + _uninstallPlugin(mockPlugin1); + } + + function test_execHookPair_install() public { + _installPlugin1WithHooks( + _EXEC_SELECTOR, + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_1, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _POST_HOOK_FUNCTION_ID_2, + dependencyIndex: 0 + }) + ); + } + + /// @dev Plugin 1 hook pair: [1, 2] + /// Expected execution: [1, 2] + function test_execHookPair_run() public { + test_execHookPair_install(); + + vm.expectEmit(true, true, true, true); + // pre hook call + emit ReceivedCall( + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_1, + address(this), // caller + 0, // msg.value in call to account + abi.encodeWithSelector(_EXEC_SELECTOR) + ), + 0 // msg value in call to plugin + ); + vm.expectEmit(true, true, true, true); + // exec call + emit ReceivedCall(abi.encodePacked(_EXEC_SELECTOR), 0); + vm.expectEmit(true, true, true, true); + // post hook call + emit ReceivedCall( + abi.encodeCall(IPlugin.postExecutionHook, (_POST_HOOK_FUNCTION_ID_2, "")), + 0 // msg value in call to plugin + ); + + (bool success,) = address(account).call(abi.encodeWithSelector(_EXEC_SELECTOR)); + assertTrue(success); + } + + function test_execHookPair_uninstall() public { + test_execHookPair_install(); + + _uninstallPlugin(mockPlugin1); + } + + function test_postOnlyExecHook_install() public { + _installPlugin1WithHooks( + _EXEC_SELECTOR, + ManifestFunction({functionType: ManifestAssociatedFunctionType.NONE, functionId: 0, dependencyIndex: 0}), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _POST_HOOK_FUNCTION_ID_2, + dependencyIndex: 0 + }) + ); + } + + /// @dev Plugin 1 hook pair: [null, 2] + /// Expected execution: [null, 2] + function test_postOnlyExecHook_run() public { + test_postOnlyExecHook_install(); + + vm.expectEmit(true, true, true, true); + emit ReceivedCall( + abi.encodeCall(IPlugin.postExecutionHook, (_POST_HOOK_FUNCTION_ID_2, "")), + 0 // msg value in call to plugin + ); + + (bool success,) = address(account).call(abi.encodeWithSelector(_EXEC_SELECTOR)); + assertTrue(success); + } + + function test_postOnlyExecHook_uninstall() public { + test_postOnlyExecHook_install(); + + _uninstallPlugin(mockPlugin1); + } + + function test_overlappingExecHookPairs_install() public { + // Install the first plugin. + _installPlugin1WithHooks( + _EXEC_SELECTOR, + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_1, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _POST_HOOK_FUNCTION_ID_2, + dependencyIndex: 0 + }) + ); + + // Install a second plugin that applies the first plugin's hook pair to the same selector. + FunctionReference[] memory dependencies = new FunctionReference[](2); + dependencies[0] = FunctionReferenceLib.pack(address(mockPlugin1), _PRE_HOOK_FUNCTION_ID_1); + dependencies[1] = FunctionReferenceLib.pack(address(mockPlugin1), _POST_HOOK_FUNCTION_ID_2); + _installPlugin2WithHooks( + _EXEC_SELECTOR, + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.DEPENDENCY, + functionId: 0, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.DEPENDENCY, + functionId: 0, + dependencyIndex: 1 + }), + dependencies + ); + + vm.stopPrank(); + } + + /// @dev Plugin 1 hook pair: [1, 2] + /// Plugin 2 hook pair: [1, 2] + /// Expected execution: [1, 2] + function test_overlappingExecHookPairs_run() public { + test_overlappingExecHookPairs_install(); + + // Expect the pre hook to be called just once. + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_1, + address(this), // caller + 0, // msg.value in call to account + abi.encodeWithSelector(_EXEC_SELECTOR) + ), + 1 + ); + + // Expect the post hook to be called just once, with the expected data. + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector(IPlugin.postExecutionHook.selector, _POST_HOOK_FUNCTION_ID_2, ""), + 1 + ); + + (bool success,) = address(account).call(abi.encodeWithSelector(_EXEC_SELECTOR)); + assertTrue(success); + } + + function test_overlappingExecHookPairs_uninstall() public { + test_overlappingExecHookPairs_install(); + + // Uninstall the second plugin. + _uninstallPlugin(mockPlugin2); + + // Expect the pre/post hooks to still exist after uninstalling a plugin with a duplicate hook. + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_1, + address(this), // caller + 0, // msg.value in call to account + abi.encodeWithSelector(_EXEC_SELECTOR) + ), + 1 + ); + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector(IPlugin.postExecutionHook.selector, _POST_HOOK_FUNCTION_ID_2, ""), + 1 + ); + (bool success,) = address(account).call(abi.encodeWithSelector(_EXEC_SELECTOR)); + assertTrue(success); + + // Uninstall the first plugin. + _uninstallPlugin(mockPlugin1); + + // Execution selector should no longer exist. + (success,) = address(account).call(abi.encodeWithSelector(_EXEC_SELECTOR)); + assertFalse(success); + } + + function test_overlappingExecHookPairsOnPost_install() public { + // Install the first plugin. + _installPlugin1WithHooks( + _EXEC_SELECTOR, + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_1, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _POST_HOOK_FUNCTION_ID_2, + dependencyIndex: 0 + }) + ); + + // Install the second plugin. + FunctionReference[] memory dependencies = new FunctionReference[](1); + dependencies[0] = FunctionReferenceLib.pack(address(mockPlugin1), _POST_HOOK_FUNCTION_ID_2); + _installPlugin2WithHooks( + _EXEC_SELECTOR, + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_3, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.DEPENDENCY, + functionId: 0, + dependencyIndex: 0 + }), + dependencies + ); + } + + /// @dev Plugin 1 hook pair: [1, 2] + /// Plugin 2 hook pair: [3, 2] + /// Expected execution: [1, 2], [3, 2] + function test_overlappingExecHookPairsOnPost_run() public { + test_overlappingExecHookPairsOnPost_install(); + + // Expect each pre hook to be called once. + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_1, + address(this), // caller + 0, // msg.value in call to account + abi.encodeWithSelector(_EXEC_SELECTOR) + ), + 1 + ); + vm.expectCall( + address(mockPlugin2), + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_3, + address(this), // caller + 0, // msg.value in call to account + abi.encodeWithSelector(_EXEC_SELECTOR) + ), + 1 + ); + + // Expect the post hook to be called twice, with the expected data. + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector(IPlugin.postExecutionHook.selector, _POST_HOOK_FUNCTION_ID_2, ""), + 2 + ); + + (bool success,) = address(account).call(abi.encodeWithSelector(_EXEC_SELECTOR)); + assertTrue(success); + } + + function test_overlappingExecHookPairsOnPost_uninstall() public { + test_overlappingExecHookPairsOnPost_install(); + + // Uninstall the second plugin. + _uninstallPlugin(mockPlugin2); + + // Expect the pre/post hooks to still exist after uninstalling a plugin with a duplicate hook. + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_1, + address(this), // caller + 0, // msg.value in call to account + abi.encodeWithSelector(_EXEC_SELECTOR) + ), + 1 + ); + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector(IPlugin.postExecutionHook.selector, _POST_HOOK_FUNCTION_ID_2, ""), + 1 + ); + (bool success,) = address(account).call(abi.encodeWithSelector(_EXEC_SELECTOR)); + assertTrue(success); + + // Uninstall the first plugin. + _uninstallPlugin(mockPlugin1); + + // Execution selector should no longer exist. + (success,) = address(account).call(abi.encodeWithSelector(_EXEC_SELECTOR)); + assertFalse(success); + } + + function _installPlugin1WithHooks( + bytes4 selector, + ManifestFunction memory preHook, + ManifestFunction memory postHook + ) internal { + m1.executionHooks.push(ManifestExecutionHook(selector, preHook, postHook)); + mockPlugin1 = new MockPlugin(m1); + manifestHash1 = keccak256(abi.encode(mockPlugin1.pluginManifest())); + + vm.expectEmit(true, true, true, true); + emit ReceivedCall(abi.encodeCall(IPlugin.onInstall, (bytes(""))), 0); + vm.expectEmit(true, true, true, true); + emit PluginInstalled( + address(mockPlugin1), manifestHash1, new FunctionReference[](0), new IPluginManager.InjectedHook[](0) + ); + + account.installPlugin({ + plugin: address(mockPlugin1), + manifestHash: manifestHash1, + pluginInitData: bytes(""), + dependencies: new FunctionReference[](0), + injectedHooks: new IPluginManager.InjectedHook[](0) + }); + } + + function _installPlugin2WithHooks( + bytes4 selector, + ManifestFunction memory preHook, + ManifestFunction memory postHook, + FunctionReference[] memory dependencies + ) internal { + if (preHook.functionType == ManifestAssociatedFunctionType.DEPENDENCY) { + m2.dependencyInterfaceIds.push(type(IPlugin).interfaceId); + } + if (postHook.functionType == ManifestAssociatedFunctionType.DEPENDENCY) { + m2.dependencyInterfaceIds.push(type(IPlugin).interfaceId); + } + + m2.executionHooks.push(ManifestExecutionHook(selector, preHook, postHook)); + + mockPlugin2 = new MockPlugin(m2); + manifestHash2 = keccak256(abi.encode(mockPlugin2.pluginManifest())); + + vm.expectEmit(true, true, true, true); + emit ReceivedCall(abi.encodeCall(IPlugin.onInstall, (bytes(""))), 0); + vm.expectEmit(true, true, true, true); + emit PluginInstalled( + address(mockPlugin2), manifestHash2, dependencies, new IPluginManager.InjectedHook[](0) + ); + + account.installPlugin({ + plugin: address(mockPlugin2), + manifestHash: manifestHash2, + pluginInitData: bytes(""), + dependencies: dependencies, + injectedHooks: new IPluginManager.InjectedHook[](0) + }); + } + + function _uninstallPlugin(MockPlugin plugin) internal { + vm.expectEmit(true, true, true, true); + emit ReceivedCall(abi.encodeCall(IPlugin.onUninstall, (bytes(""))), 0); + vm.expectEmit(true, true, true, true); + emit PluginUninstalled(address(plugin), true); + + account.uninstallPlugin(address(plugin), bytes(""), bytes(""), new bytes[](0)); + } +} diff --git a/test/account/ModularAccountLoupe.t.sol b/test/account/AccountLoupe.t.sol similarity index 93% rename from test/account/ModularAccountLoupe.t.sol rename to test/account/AccountLoupe.t.sol index 5757c001..0f016895 100644 --- a/test/account/ModularAccountLoupe.t.sol +++ b/test/account/AccountLoupe.t.sol @@ -1,8 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {Test} from "forge-std/Test.sol"; - import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; @@ -15,7 +13,7 @@ import { ManifestFunction, PluginManifest } from "../../src/interfaces/IPlugin.sol"; -import {IPluginLoupe} from "../../src/interfaces/IPluginLoupe.sol"; +import {IAccountLoupe} from "../../src/interfaces/IAccountLoupe.sol"; import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; import {IStandardExecutor} from "../../src/interfaces/IStandardExecutor.sol"; import {FunctionReference, FunctionReferenceLib} from "../../src/libraries/FunctionReferenceLib.sol"; @@ -23,8 +21,9 @@ import {FunctionReference, FunctionReferenceLib} from "../../src/libraries/Funct import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; import {ComprehensivePlugin} from "../mocks/plugins/ComprehensivePlugin.sol"; import {MockPlugin} from "../mocks/MockPlugin.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; -contract ModularAccountLoupeTest is Test { +contract AccountLoupeTest is OptimizedTest { EntryPoint public entryPoint; SingleOwnerPlugin public singleOwnerPlugin; MSCAFactoryFixture public factory; @@ -40,7 +39,7 @@ contract ModularAccountLoupeTest is Test { function setUp() public { entryPoint = new EntryPoint(); - singleOwnerPlugin = new SingleOwnerPlugin(); + singleOwnerPlugin = _deploySingleOwnerPlugin(); factory = new MSCAFactoryFixture(entryPoint, singleOwnerPlugin); comprehensivePlugin = new ComprehensivePlugin(); @@ -98,7 +97,7 @@ contract ModularAccountLoupeTest is Test { expectedRuntimeValidations[4] = ownerRuntimeValidation; for (uint256 i = 0; i < selectorsToCheck.length; i++) { - IPluginLoupe.ExecutionFunctionConfig memory config = + IAccountLoupe.ExecutionFunctionConfig memory config = account1.getExecutionFunctionConfig(selectorsToCheck[i]); assertEq(config.plugin, address(account1)); @@ -134,7 +133,7 @@ contract ModularAccountLoupeTest is Test { expectedRuntimeValidations[1] = ownerRuntimeValidation; for (uint256 i = 0; i < selectorsToCheck.length; i++) { - IPluginLoupe.ExecutionFunctionConfig memory config = + IAccountLoupe.ExecutionFunctionConfig memory config = account1.getExecutionFunctionConfig(selectorsToCheck[i]); assertEq(config.plugin, expectedPluginAddress[i]); @@ -150,7 +149,7 @@ contract ModularAccountLoupeTest is Test { } function test_pluginLoupe_getExecutionHooks() public { - IPluginLoupe.ExecutionHooks[] memory hooks = account1.getExecutionHooks(comprehensivePlugin.foo.selector); + IAccountLoupe.ExecutionHooks[] memory hooks = account1.getExecutionHooks(comprehensivePlugin.foo.selector); assertEq(hooks.length, 1); assertEq( @@ -172,7 +171,7 @@ contract ModularAccountLoupeTest is Test { } function test_pluginLoupe_getPermittedCallHooks() public { - IPluginLoupe.ExecutionHooks[] memory hooks = + IAccountLoupe.ExecutionHooks[] memory hooks = account1.getPermittedCallHooks(address(comprehensivePlugin), comprehensivePlugin.foo.selector); assertEq(hooks.length, 1); @@ -243,7 +242,7 @@ contract ModularAccountLoupeTest is Test { // Assert that the returned execution hooks are what is expected - IPluginLoupe.ExecutionHooks[] memory hooks = account1.getExecutionHooks(comprehensivePlugin.foo.selector); + IAccountLoupe.ExecutionHooks[] memory hooks = account1.getExecutionHooks(comprehensivePlugin.foo.selector); assertEq(hooks.length, 2); assertEq( @@ -295,7 +294,7 @@ contract ModularAccountLoupeTest is Test { } function test_pluginLoupe_getPreUserOpValidationHooks() public { - FunctionReference[] memory hooks = account1.getPreUserOpValidationHooks(comprehensivePlugin.foo.selector); + (FunctionReference[] memory hooks,) = account1.getPreValidationHooks(comprehensivePlugin.foo.selector); assertEq(hooks.length, 2); assertEq( @@ -319,7 +318,7 @@ contract ModularAccountLoupeTest is Test { } function test_pluginLoupe_getPreRuntimeValidationHooks() public { - FunctionReference[] memory hooks = account1.getPreRuntimeValidationHooks(comprehensivePlugin.foo.selector); + (, FunctionReference[] memory hooks) = account1.getPreValidationHooks(comprehensivePlugin.foo.selector); assertEq(hooks.length, 2); assertEq( diff --git a/test/account/AccountPermittedCallHooks.t.sol b/test/account/AccountPermittedCallHooks.t.sol new file mode 100644 index 00000000..a90eaae0 --- /dev/null +++ b/test/account/AccountPermittedCallHooks.t.sol @@ -0,0 +1,406 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; +import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; + +import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; +import { + IPlugin, + ManifestAssociatedFunctionType, + ManifestAssociatedFunction, + ManifestExecutionHook, + ManifestFunction, + PluginManifest +} from "../../src/interfaces/IPlugin.sol"; +import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; +import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; +import {FunctionReference} from "../../src/libraries/FunctionReferenceLib.sol"; + +import {MockPlugin} from "../mocks/MockPlugin.sol"; +import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; + +contract AccountPermittedCallHooksTest is OptimizedTest { + using ECDSA for bytes32; + + EntryPoint public entryPoint; + SingleOwnerPlugin public singleOwnerPlugin; + MSCAFactoryFixture public factory; + + UpgradeableModularAccount public account; + + MockPlugin public mockPlugin1; + bytes32 public manifestHash1; + + bytes4 internal constant _EXEC_SELECTOR = bytes4(uint32(1)); + uint8 internal constant _PRE_HOOK_FUNCTION_ID_1 = 1; + uint8 internal constant _POST_HOOK_FUNCTION_ID_2 = 2; + uint8 internal constant _PRE_HOOK_FUNCTION_ID_3 = 3; + uint8 internal constant _POST_HOOK_FUNCTION_ID_4 = 4; + + PluginManifest public m1; + + /// @dev Note that we strip hookApplyData from InjectedHooks in this event for gas savings + event PluginInstalled( + address indexed plugin, + bytes32 manifestHash, + FunctionReference[] dependencies, + IPluginManager.InjectedHook[] injectedHooks + ); + event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); + // emitted by MockPlugin + event ReceivedCall(bytes msgData, uint256 msgValue); + + function setUp() public { + entryPoint = new EntryPoint(); + singleOwnerPlugin = _deploySingleOwnerPlugin(); + factory = new MSCAFactoryFixture(entryPoint, singleOwnerPlugin); + + // Create an account with "this" as the owner, so we can execute along the runtime path with regular + // solidity semantics + account = factory.createAccount(address(this), 0); + + m1.executionFunctions.push(_EXEC_SELECTOR); + + m1.runtimeValidationFunctions.push( + ManifestAssociatedFunction({ + executionSelector: _EXEC_SELECTOR, + associatedFunction: ManifestFunction({ + functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, + functionId: 0, + dependencyIndex: 0 + }) + }) + ); + + m1.permittedExecutionSelectors.push(_EXEC_SELECTOR); + } + + function test_prePermittedCallHook_install() public { + _installPlugin1WithHooks( + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_1, + dependencyIndex: 0 + }), + ManifestFunction({functionType: ManifestAssociatedFunctionType.NONE, functionId: 0, dependencyIndex: 0}) + ); + } + + /// @dev Plugin hook pair(s): [1, null] + /// Expected execution: [1, null] + function test_prePermittedCallHook_run() public { + test_prePermittedCallHook_install(); + + vm.startPrank(address(mockPlugin1)); + + vm.expectEmit(true, true, true, true); + emit ReceivedCall( + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_1, + address(mockPlugin1), // caller + 0, // msg.value in call to account + abi.encodePacked(_EXEC_SELECTOR) + ), + 0 // msg value in call to plugin + ); + + account.executeFromPlugin(abi.encodePacked(_EXEC_SELECTOR)); + + vm.stopPrank(); + } + + function test_prePermittedCallHook_uninstall() public { + test_prePermittedCallHook_install(); + + _uninstallPlugin(mockPlugin1); + } + + function test_permittedCallHookPair_install() public { + _installPlugin1WithHooks( + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_1, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _POST_HOOK_FUNCTION_ID_2, + dependencyIndex: 0 + }) + ); + } + + /// @dev Plugin hook pair(s): [1, 2] + /// Expected execution: [1, 2] + function test_permittedCallHookPair_run() public { + test_permittedCallHookPair_install(); + + vm.startPrank(address(mockPlugin1)); + + vm.expectEmit(true, true, true, true); + // pre hook call + emit ReceivedCall( + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_1, + address(mockPlugin1), // caller + 0, // msg.value in call to account + abi.encodePacked(_EXEC_SELECTOR) + ), + 0 // msg value in call to plugin + ); + vm.expectEmit(true, true, true, true); + // exec call + emit ReceivedCall(abi.encodePacked(_EXEC_SELECTOR), 0); + vm.expectEmit(true, true, true, true); + // post hook call + emit ReceivedCall( + abi.encodeCall(IPlugin.postExecutionHook, (_POST_HOOK_FUNCTION_ID_2, "")), + 0 // msg value in call to plugin + ); + + account.executeFromPlugin(abi.encodePacked(_EXEC_SELECTOR)); + + vm.stopPrank(); + } + + function test_permittedCallHookPair_uninstall() public { + test_permittedCallHookPair_install(); + + _uninstallPlugin(mockPlugin1); + } + + function test_postOnlyPermittedCallHook_install() public { + _installPlugin1WithHooks( + ManifestFunction({functionType: ManifestAssociatedFunctionType.NONE, functionId: 0, dependencyIndex: 0}), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _POST_HOOK_FUNCTION_ID_2, + dependencyIndex: 0 + }) + ); + } + + /// @dev Plugin hook pair(s): [null, 2] + /// Expected execution: [null, 2] + function test_postOnlyPermittedCallHook_run() public { + test_postOnlyPermittedCallHook_install(); + + vm.startPrank(address(mockPlugin1)); + + vm.expectEmit(true, true, true, true); + emit ReceivedCall( + abi.encodeCall(IPlugin.postExecutionHook, (_POST_HOOK_FUNCTION_ID_2, "")), + 0 // msg value in call to plugin + ); + + account.executeFromPlugin(abi.encodePacked(_EXEC_SELECTOR)); + + vm.stopPrank(); + } + + function test_postOnlyPermittedCallHook_uninstall() public { + test_postOnlyPermittedCallHook_install(); + + _uninstallPlugin(mockPlugin1); + } + + function test_overlappingPermittedCallHookPairs_install() public { + _installPlugin1WithHooks( + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_1, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _POST_HOOK_FUNCTION_ID_2, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_1, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _POST_HOOK_FUNCTION_ID_2, + dependencyIndex: 0 + }) + ); + } + + /// @dev Plugin hook pair(s): [1, 2], [1, 2] + /// Expected execution: [1, 2] + function test_overlappingPermittedCallHookPairs_run() public { + test_overlappingPermittedCallHookPairs_install(); + + vm.startPrank(address(mockPlugin1)); + + // Expect the pre hook to be called just once. + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_1, + address(mockPlugin1), // caller + 0, // msg.value in call to account + abi.encodePacked(_EXEC_SELECTOR) + ), + 1 + ); + + // Expect the post hook to be called just once, with the expected data. + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector(IPlugin.postExecutionHook.selector, _POST_HOOK_FUNCTION_ID_2, ""), + 1 + ); + + account.executeFromPlugin(abi.encodePacked(_EXEC_SELECTOR)); + + vm.stopPrank(); + } + + function test_overlappingPermittedCallHookPairs_uninstall() public { + test_overlappingPermittedCallHookPairs_install(); + + _uninstallPlugin(mockPlugin1); + } + + function test_overlappingPermittedCallHookPairsOnPost_install() public { + _installPlugin1WithHooks( + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_1, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _POST_HOOK_FUNCTION_ID_2, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _PRE_HOOK_FUNCTION_ID_3, + dependencyIndex: 0 + }), + ManifestFunction({ + functionType: ManifestAssociatedFunctionType.SELF, + functionId: _POST_HOOK_FUNCTION_ID_2, + dependencyIndex: 0 + }) + ); + } + + /// @dev Plugin hook pair(s): [1, 2], [3, 2] + /// Expected execution: [1, 2], [3, 2] + function test_overlappingPermittedCallHookPairsOnPost_run() public { + test_overlappingPermittedCallHookPairsOnPost_install(); + + vm.startPrank(address(mockPlugin1)); + + // Expect each pre hook to be called once. + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_3, + address(mockPlugin1), // caller + 0, // msg.value in call to account + abi.encodePacked(_EXEC_SELECTOR) + ), + 1 + ); + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector( + IPlugin.preExecutionHook.selector, + _PRE_HOOK_FUNCTION_ID_1, + address(mockPlugin1), // caller + 0, // msg.value in call to account + abi.encodePacked(_EXEC_SELECTOR) + ), + 1 + ); + + // Expect the post hook to be called twice, with the expected data. + vm.expectCall( + address(mockPlugin1), + abi.encodeWithSelector(IPlugin.postExecutionHook.selector, _POST_HOOK_FUNCTION_ID_2, ""), + 2 + ); + + account.executeFromPlugin(abi.encodePacked(_EXEC_SELECTOR)); + + vm.stopPrank(); + } + + function test_overlappingPermittedCallHookPairsOnPost_uninstall() public { + test_overlappingPermittedCallHookPairsOnPost_install(); + + _uninstallPlugin(mockPlugin1); + } + + function _installPlugin1WithHooks(ManifestFunction memory preHook1, ManifestFunction memory postHook1) + internal + { + m1.permittedCallHooks.push(ManifestExecutionHook(_EXEC_SELECTOR, preHook1, postHook1)); + mockPlugin1 = new MockPlugin(m1); + manifestHash1 = keccak256(abi.encode(mockPlugin1.pluginManifest())); + + vm.expectEmit(true, true, true, true); + emit ReceivedCall(abi.encodeCall(IPlugin.onInstall, (bytes(""))), 0); + vm.expectEmit(true, true, true, true); + emit PluginInstalled( + address(mockPlugin1), manifestHash1, new FunctionReference[](0), new IPluginManager.InjectedHook[](0) + ); + + account.installPlugin({ + plugin: address(mockPlugin1), + manifestHash: manifestHash1, + pluginInitData: bytes(""), + dependencies: new FunctionReference[](0), + injectedHooks: new IPluginManager.InjectedHook[](0) + }); + } + + function _installPlugin1WithHooks( + ManifestFunction memory preHook1, + ManifestFunction memory postHook1, + ManifestFunction memory preHook2, + ManifestFunction memory postHook2 + ) internal { + m1.permittedCallHooks.push(ManifestExecutionHook(_EXEC_SELECTOR, preHook1, postHook1)); + m1.permittedCallHooks.push(ManifestExecutionHook(_EXEC_SELECTOR, preHook2, postHook2)); + mockPlugin1 = new MockPlugin(m1); + manifestHash1 = keccak256(abi.encode(mockPlugin1.pluginManifest())); + + vm.expectEmit(true, true, true, true); + emit ReceivedCall(abi.encodeCall(IPlugin.onInstall, (bytes(""))), 0); + vm.expectEmit(true, true, true, true); + emit PluginInstalled( + address(mockPlugin1), manifestHash1, new FunctionReference[](0), new IPluginManager.InjectedHook[](0) + ); + + account.installPlugin({ + plugin: address(mockPlugin1), + manifestHash: manifestHash1, + pluginInitData: bytes(""), + dependencies: new FunctionReference[](0), + injectedHooks: new IPluginManager.InjectedHook[](0) + }); + } + + function _uninstallPlugin(MockPlugin plugin) internal { + vm.expectEmit(true, true, true, true); + emit ReceivedCall(abi.encodeCall(IPlugin.onUninstall, (bytes(""))), 0); + vm.expectEmit(true, true, true, true); + emit PluginUninstalled(address(plugin), true); + + account.uninstallPlugin(address(plugin), bytes(""), bytes(""), new bytes[](0)); + } +} diff --git a/test/account/AccountReturnData.t.sol b/test/account/AccountReturnData.t.sol index d969c2c8..d2772a95 100644 --- a/test/account/AccountReturnData.t.sol +++ b/test/account/AccountReturnData.t.sol @@ -1,15 +1,13 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {Test} from "forge-std/Test.sol"; - import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {FunctionReference} from "../../src/libraries/FunctionReferenceLib.sol"; -import {Execution} from "../../src/libraries/ERC6900TypeUtils.sol"; +import {Call} from "../../src/interfaces/IStandardExecutor.sol"; import { RegularResultContract, @@ -17,9 +15,10 @@ import { ResultConsumerPlugin } from "../mocks/plugins/ReturnDataPluginMocks.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; // Tests all the different ways that return data can be read from plugins through an account -contract AccountReturnDataTest is Test { +contract AccountReturnDataTest is OptimizedTest { EntryPoint public entryPoint; // Just to be able to construct the factory SingleOwnerPlugin public singleOwnerPlugin; MSCAFactoryFixture public factory; @@ -32,7 +31,7 @@ contract AccountReturnDataTest is Test { function setUp() public { entryPoint = new EntryPoint(); - singleOwnerPlugin = new SingleOwnerPlugin(); + singleOwnerPlugin = _deploySingleOwnerPlugin(); factory = new MSCAFactoryFixture(entryPoint, singleOwnerPlugin); regularResultContract = new RegularResultContract(); @@ -72,9 +71,8 @@ contract AccountReturnDataTest is Test { // Tests the ability to read the results of contracts called via IStandardExecutor.execute function test_returnData_singular_execute() public { - bytes memory returnData = account.execute( - Execution(address(regularResultContract), 0, abi.encodeCall(RegularResultContract.foo, ())) - ); + bytes memory returnData = + account.execute(address(regularResultContract), 0, abi.encodeCall(RegularResultContract.foo, ())); bytes32 result = abi.decode(returnData, (bytes32)); @@ -83,13 +81,13 @@ contract AccountReturnDataTest is Test { // Tests the ability to read the results of multiple contract calls via IStandardExecutor.executeBatch function test_returnData_executeBatch() public { - Execution[] memory calls = new Execution[](2); - calls[0] = Execution({ + Call[] memory calls = new Call[](2); + calls[0] = Call({ target: address(regularResultContract), value: 0, data: abi.encodeCall(RegularResultContract.foo, ()) }); - calls[1] = Execution({ + calls[1] = Call({ target: address(regularResultContract), value: 0, data: abi.encodeCall(RegularResultContract.bar, ()) diff --git a/test/account/ExecuteFromPluginPermissions.t.sol b/test/account/ExecuteFromPluginPermissions.t.sol index 82e5d027..48b65d98 100644 --- a/test/account/ExecuteFromPluginPermissions.t.sol +++ b/test/account/ExecuteFromPluginPermissions.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {Test, console} from "forge-std/Test.sol"; +import {console} from "forge-std/Test.sol"; import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; @@ -11,18 +11,17 @@ import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {FunctionReference} from "../../src/libraries/FunctionReferenceLib.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; - import {Counter} from "../mocks/Counter.sol"; import {ResultCreatorPlugin} from "../mocks/plugins/ReturnDataPluginMocks.sol"; - import { EFPCallerPlugin, EFPCallerPluginAnyExternal, EFPPermittedCallHookPlugin, EFPExternalPermittedCallHookPlugin } from "../mocks/plugins/ExecFromPluginPermissionsMocks.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; -contract ExecuteFromPluginPermissionsTest is Test { +contract ExecuteFromPluginPermissionsTest is OptimizedTest { Counter public counter1; Counter public counter2; Counter public counter3; @@ -47,7 +46,7 @@ contract ExecuteFromPluginPermissionsTest is Test { // Initialize the contracts needed to use the account. entryPoint = new EntryPoint(); - singleOwnerPlugin = new SingleOwnerPlugin(); + singleOwnerPlugin = _deploySingleOwnerPlugin(); factory = new MSCAFactoryFixture(entryPoint, singleOwnerPlugin); // Initialize the EFP caller plugins, which will attempt to use the permissions system to authorize calls. diff --git a/test/account/ManifestValidity.t.sol b/test/account/ManifestValidity.t.sol index 034e1929..18972d07 100644 --- a/test/account/ManifestValidity.t.sol +++ b/test/account/ManifestValidity.t.sol @@ -1,13 +1,11 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {Test} from "forge-std/Test.sol"; - import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {BaseModularAccount} from "../../src/account/BaseModularAccount.sol"; +import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {FunctionReference} from "../../src/libraries/FunctionReferenceLib.sol"; @@ -22,8 +20,9 @@ import { BadHookMagicValue_RuntimeValidationFunction_Plugin, BadHookMagicValue_PostExecHook_Plugin } from "../mocks/plugins/ManifestValidityMocks.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; -contract ManifestValidityTest is Test { +contract ManifestValidityTest is OptimizedTest { EntryPoint public entryPoint; // Just to be able to construct the factory SingleOwnerPlugin public singleOwnerPlugin; MSCAFactoryFixture public factory; @@ -32,7 +31,7 @@ contract ManifestValidityTest is Test { function setUp() public { entryPoint = new EntryPoint(); - singleOwnerPlugin = new SingleOwnerPlugin(); + singleOwnerPlugin = _deploySingleOwnerPlugin(); factory = new MSCAFactoryFixture(entryPoint, singleOwnerPlugin); // Create an account with "this" as the owner, so we can execute along the runtime path with regular @@ -47,7 +46,7 @@ contract ManifestValidityTest is Test { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(BaseModularAccount.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -65,7 +64,7 @@ contract ManifestValidityTest is Test { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(BaseModularAccount.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -83,7 +82,7 @@ contract ManifestValidityTest is Test { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(BaseModularAccount.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -99,7 +98,7 @@ contract ManifestValidityTest is Test { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(BaseModularAccount.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -115,7 +114,7 @@ contract ManifestValidityTest is Test { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(BaseModularAccount.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -132,7 +131,7 @@ contract ManifestValidityTest is Test { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(BaseModularAccount.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -149,7 +148,7 @@ contract ManifestValidityTest is Test { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(BaseModularAccount.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -165,7 +164,7 @@ contract ManifestValidityTest is Test { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(BaseModularAccount.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, diff --git a/test/account/UpgradeableModularAccount.t.sol b/test/account/UpgradeableModularAccount.t.sol index 0da80e81..528eedd2 100644 --- a/test/account/UpgradeableModularAccount.t.sol +++ b/test/account/UpgradeableModularAccount.t.sol @@ -1,21 +1,21 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {Test, console} from "forge-std/Test.sol"; +import {console} from "forge-std/Test.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; -import {BaseModularAccount} from "../../src/account/BaseModularAccount.sol"; +import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {TokenReceiverPlugin} from "../../src/plugins/TokenReceiverPlugin.sol"; import {PluginManifest} from "../../src/interfaces/IPlugin.sol"; -import {IPluginLoupe} from "../../src/interfaces/IPluginLoupe.sol"; +import {IAccountLoupe} from "../../src/interfaces/IAccountLoupe.sol"; import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; import {IPluginExecutor} from "../../src/interfaces/IPluginExecutor.sol"; -import {Execution} from "../../src/libraries/ERC6900TypeUtils.sol"; +import {Call} from "../../src/interfaces/IStandardExecutor.sol"; import {FunctionReference} from "../../src/libraries/FunctionReferenceLib.sol"; import {IPlugin, PluginManifest} from "../../src/interfaces/IPlugin.sol"; @@ -23,8 +23,9 @@ import {Counter} from "../mocks/Counter.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; import {ComprehensivePlugin} from "../mocks/plugins/ComprehensivePlugin.sol"; import {MockPlugin} from "../mocks/MockPlugin.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; -contract UpgradeableModularAccountTest is Test { +contract UpgradeableModularAccountTest is OptimizedTest { using ECDSA for bytes32; EntryPoint public entryPoint; @@ -53,8 +54,13 @@ contract UpgradeableModularAccountTest is Test { uint256 public constant CALL_GAS_LIMIT = 50000; uint256 public constant VERIFICATION_GAS_LIMIT = 1200000; - event PluginInstalled(address indexed plugin, bytes32 manifestHash); - event PluginUninstalled(address indexed plugin, bytes32 manifestHash, bool onUninstallSucceeded); + event PluginInstalled( + address indexed plugin, + bytes32 manifestHash, + FunctionReference[] dependencies, + IPluginManager.InjectedHook[] injectedHooks + ); + event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); event ReceivedCall(bytes msgData, uint256 msgValue); function setUp() public { @@ -63,8 +69,8 @@ contract UpgradeableModularAccountTest is Test { beneficiary = payable(makeAddr("beneficiary")); vm.deal(beneficiary, 1 wei); - singleOwnerPlugin = new SingleOwnerPlugin(); - tokenReceiverPlugin = new TokenReceiverPlugin(); + singleOwnerPlugin = _deploySingleOwnerPlugin(); + tokenReceiverPlugin = _deployTokenReceiverPlugin(); factory = new MSCAFactoryFixture(entryPoint, singleOwnerPlugin); // Compute counterfactual address @@ -122,9 +128,7 @@ contract UpgradeableModularAccountTest is Test { sender: address(account1), nonce: 0, initCode: abi.encodePacked(address(factory), abi.encodeCall(factory.createAccount, (owner1, 0))), - callData: abi.encodeCall( - UpgradeableModularAccount(payable(account1)).execute, Execution(recipient, 1 wei, "") - ), + callData: abi.encodeCall(UpgradeableModularAccount.execute, (recipient, 1 wei, "")), callGasLimit: CALL_GAS_LIMIT, verificationGasLimit: VERIFICATION_GAS_LIMIT, preVerificationGas: 0, @@ -152,9 +156,7 @@ contract UpgradeableModularAccountTest is Test { sender: address(account2), nonce: 0, initCode: "", - callData: abi.encodeCall( - UpgradeableModularAccount(payable(account2)).execute, Execution(ethRecipient, 1 wei, "") - ), + callData: abi.encodeCall(UpgradeableModularAccount.execute, (ethRecipient, 1 wei, "")), callGasLimit: CALL_GAS_LIMIT, verificationGasLimit: VERIFICATION_GAS_LIMIT, preVerificationGas: 0, @@ -182,9 +184,7 @@ contract UpgradeableModularAccountTest is Test { sender: address(account2), nonce: 0, initCode: "", - callData: abi.encodeCall( - UpgradeableModularAccount(payable(account2)).execute, Execution(ethRecipient, 1 wei, "") - ), + callData: abi.encodeCall(UpgradeableModularAccount.execute, (ethRecipient, 1 wei, "")), callGasLimit: CALL_GAS_LIMIT, verificationGasLimit: VERIFICATION_GAS_LIMIT, preVerificationGas: 0, @@ -213,8 +213,7 @@ contract UpgradeableModularAccountTest is Test { nonce: 0, initCode: "", callData: abi.encodeCall( - UpgradeableModularAccount(payable(account2)).execute, - Execution(address(counter), 0, abi.encodeCall(counter.increment, ())) + UpgradeableModularAccount.execute, (address(counter), 0, abi.encodeCall(counter.increment, ())) ), callGasLimit: CALL_GAS_LIMIT, verificationGasLimit: VERIFICATION_GAS_LIMIT, @@ -240,16 +239,15 @@ contract UpgradeableModularAccountTest is Test { function test_batchExecute() public { // Performs both an eth send and a contract interaction with counter - Execution[] memory executions = new Execution[](2); - executions[0] = Execution({target: ethRecipient, value: 1 wei, data: ""}); - executions[1] = - Execution({target: address(counter), value: 0, data: abi.encodeCall(counter.increment, ())}); + Call[] memory calls = new Call[](2); + calls[0] = Call({target: ethRecipient, value: 1 wei, data: ""}); + calls[1] = Call({target: address(counter), value: 0, data: abi.encodeCall(counter.increment, ())}); UserOperation memory userOp = UserOperation({ sender: address(account2), nonce: 0, initCode: "", - callData: abi.encodeCall(UpgradeableModularAccount(payable(account2)).executeBatch, (executions)), + callData: abi.encodeCall(UpgradeableModularAccount.executeBatch, (calls)), callGasLimit: CALL_GAS_LIMIT, verificationGasLimit: VERIFICATION_GAS_LIMIT, preVerificationGas: 0, @@ -279,7 +277,12 @@ contract UpgradeableModularAccountTest is Test { bytes32 manifestHash = keccak256(abi.encode(tokenReceiverPlugin.pluginManifest())); vm.expectEmit(true, true, true, true); - emit PluginInstalled(address(tokenReceiverPlugin), manifestHash); + emit PluginInstalled( + address(tokenReceiverPlugin), + manifestHash, + new FunctionReference[](0), + new IPluginManager.InjectedHook[](0) + ); IPluginManager(account2).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: manifestHash, @@ -288,13 +291,13 @@ contract UpgradeableModularAccountTest is Test { injectedHooks: new IPluginManager.InjectedHook[](0) }); - address[] memory plugins = IPluginLoupe(account2).getInstalledPlugins(); + address[] memory plugins = IAccountLoupe(account2).getInstalledPlugins(); assertEq(plugins.length, 2); assertEq(plugins[0], address(singleOwnerPlugin)); assertEq(plugins[1], address(tokenReceiverPlugin)); } - function test_installPlugin_ExecuteFromPlugin_BadPermittedExecSelector() public { + function test_installPlugin_ExecuteFromPlugin_PermittedExecSelectorNotInstalled() public { vm.startPrank(owner2); PluginManifest memory m; @@ -304,13 +307,6 @@ contract UpgradeableModularAccountTest is Test { MockPlugin mockPluginWithBadPermittedExec = new MockPlugin(m); bytes32 manifestHash = keccak256(abi.encode(mockPluginWithBadPermittedExec.pluginManifest())); - vm.expectRevert( - abi.encodeWithSelector( - BaseModularAccount.PermittedExecutionSelectorNotInstalled.selector, - IPlugin.onInstall.selector, - address(mockPluginWithBadPermittedExec) - ) - ); IPluginManager(account2).installPlugin({ plugin: address(mockPluginWithBadPermittedExec), manifestHash: manifestHash, @@ -323,7 +319,7 @@ contract UpgradeableModularAccountTest is Test { function test_installPlugin_invalidManifest() public { vm.startPrank(owner2); - vm.expectRevert(abi.encodeWithSelector(BaseModularAccount.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); IPluginManager(account2).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: bytes32(0), @@ -338,7 +334,7 @@ contract UpgradeableModularAccountTest is Test { address badPlugin = address(1); vm.expectRevert( - abi.encodeWithSelector(BaseModularAccount.PluginInterfaceNotSupported.selector, address(badPlugin)) + abi.encodeWithSelector(PluginManagerInternals.PluginInterfaceNotSupported.selector, address(badPlugin)) ); IPluginManager(account2).installPlugin({ plugin: address(badPlugin), @@ -363,7 +359,7 @@ contract UpgradeableModularAccountTest is Test { vm.expectRevert( abi.encodeWithSelector( - BaseModularAccount.PluginAlreadyInstalled.selector, address(tokenReceiverPlugin) + PluginManagerInternals.PluginAlreadyInstalled.selector, address(tokenReceiverPlugin) ) ); IPluginManager(account2).installPlugin({ @@ -389,14 +385,14 @@ contract UpgradeableModularAccountTest is Test { }); vm.expectEmit(true, true, true, true); - emit PluginUninstalled(address(plugin), manifestHash, true); + emit PluginUninstalled(address(plugin), true); IPluginManager(account2).uninstallPlugin({ plugin: address(plugin), config: "", pluginUninstallData: "", hookUnapplyData: new bytes[](0) }); - address[] memory plugins = IPluginLoupe(account2).getInstalledPlugins(); + address[] memory plugins = IAccountLoupe(account2).getInstalledPlugins(); assertEq(plugins.length, 1); assertEq(plugins[0], address(singleOwnerPlugin)); } @@ -416,14 +412,14 @@ contract UpgradeableModularAccountTest is Test { }); vm.expectEmit(true, true, true, true); - emit PluginUninstalled(address(plugin), manifestHash, true); + emit PluginUninstalled(address(plugin), true); IPluginManager(account2).uninstallPlugin({ plugin: address(plugin), config: serializedManifest, pluginUninstallData: "", hookUnapplyData: new bytes[](0) }); - address[] memory plugins = IPluginLoupe(account2).getInstalledPlugins(); + address[] memory plugins = IAccountLoupe(account2).getInstalledPlugins(); assertEq(plugins.length, 1); assertEq(plugins[0], address(singleOwnerPlugin)); } @@ -445,14 +441,14 @@ contract UpgradeableModularAccountTest is Test { // Attempt to uninstall with a blank manifest PluginManifest memory blankManifest; - vm.expectRevert(abi.encodeWithSelector(BaseModularAccount.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); IPluginManager(account2).uninstallPlugin({ plugin: address(plugin), config: abi.encode(blankManifest), pluginUninstallData: "", hookUnapplyData: new bytes[](0) }); - address[] memory plugins = IPluginLoupe(account2).getInstalledPlugins(); + address[] memory plugins = IAccountLoupe(account2).getInstalledPlugins(); assertEq(plugins.length, 2); assertEq(plugins[0], address(singleOwnerPlugin)); assertEq(plugins[1], address(plugin)); @@ -481,7 +477,7 @@ contract UpgradeableModularAccountTest is Test { { hooksPlugin = _installPluginWithExecHooks(); - manifest.permitAnyExternalContract = true; + manifest.permitAnyExternalAddress = true; newPlugin = new MockPlugin(manifest); manifestHash = keccak256(abi.encode(newPlugin.pluginManifest())); @@ -493,7 +489,7 @@ contract UpgradeableModularAccountTest is Test { vm.prank(owner2); vm.expectEmit(true, true, true, true); - emit PluginInstalled(address(newPlugin), manifestHash); + emit PluginInstalled(address(newPlugin), manifestHash, new FunctionReference[](0), hooks); emit ReceivedCall(abi.encodeCall(IPlugin.onHookApply, (address(newPlugin), injectedHooksInfo, "")), 0); IPluginManager(account2).installPlugin({ plugin: address(newPlugin), @@ -547,7 +543,7 @@ contract UpgradeableModularAccountTest is Test { ); vm.expectEmit(true, true, true, true); - emit PluginInstalled(address(newPlugin), manifestHash); + emit PluginInstalled(address(newPlugin), manifestHash, new FunctionReference[](0), hooks); emit ReceivedCall( abi.encodeCall(IPlugin.onHookApply, (address(newPlugin), injectedHooksInfo, onApplyData)), 0 ); @@ -575,7 +571,7 @@ contract UpgradeableModularAccountTest is Test { ); vm.expectRevert( - abi.encodeWithSelector(BaseModularAccount.MissingPluginDependency.selector, address(hooksPlugin)) + abi.encodeWithSelector(PluginManagerInternals.MissingPluginDependency.selector, address(hooksPlugin)) ); vm.prank(owner2); IPluginManager(account2).installPlugin({ @@ -588,10 +584,10 @@ contract UpgradeableModularAccountTest is Test { } function test_injectHooksUninstall() external { - (, MockPlugin newPlugin, bytes32 manifestHash) = _installWithInjectHooks(); + (, MockPlugin newPlugin,) = _installWithInjectHooks(); vm.expectEmit(true, true, true, true); - emit PluginUninstalled(address(newPlugin), manifestHash, true); + emit PluginUninstalled(address(newPlugin), true); vm.prank(owner2); IPluginManager(account2).uninstallPlugin({ plugin: address(newPlugin), @@ -606,7 +602,7 @@ contract UpgradeableModularAccountTest is Test { vm.prank(owner2); vm.expectRevert( - abi.encodeWithSelector(BaseModularAccount.PluginDependencyViolation.selector, address(hooksPlugin)) + abi.encodeWithSelector(PluginManagerInternals.PluginDependencyViolation.selector, address(hooksPlugin)) ); IPluginManager(account2).uninstallPlugin({ plugin: address(hooksPlugin), @@ -642,7 +638,7 @@ contract UpgradeableModularAccountTest is Test { // length != installed hooks length bytes[] memory injectedHooksDatas = new bytes[](2); - vm.expectRevert(BaseModularAccount.ArrayLengthMismatch.selector); + vm.expectRevert(PluginManagerInternals.ArrayLengthMismatch.selector); vm.prank(owner2); IPluginManager(account2).uninstallPlugin({ plugin: address(newPlugin), diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index fa2a8390..b33f5187 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -1,8 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {Test} from "forge-std/Test.sol"; - import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; @@ -18,8 +16,9 @@ import { MockUserOpValidation2HookPlugin, MockUserOpValidationPlugin } from "../mocks/plugins/ValidationPluginMocks.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; -contract ValidationIntersectionTest is Test { +contract ValidationIntersectionTest is OptimizedTest { uint256 internal constant _SIG_VALIDATION_FAILED = 1; EntryPoint public entryPoint; @@ -35,7 +34,7 @@ contract ValidationIntersectionTest is Test { entryPoint = new EntryPoint(); owner1 = makeAddr("owner1"); - SingleOwnerPlugin singleOwnerPlugin = new SingleOwnerPlugin(); + SingleOwnerPlugin singleOwnerPlugin = _deploySingleOwnerPlugin(); MSCAFactoryFixture factory = new MSCAFactoryFixture(entryPoint, singleOwnerPlugin); account1 = factory.createAccount(owner1, 0); diff --git a/test/mocks/MSCAFactoryFixture.sol b/test/mocks/MSCAFactoryFixture.sol index f920eaea..03b90736 100644 --- a/test/mocks/MSCAFactoryFixture.sol +++ b/test/mocks/MSCAFactoryFixture.sol @@ -8,12 +8,14 @@ import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntry import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; + /** * @title MSCAFactoryFixture * @dev a factory that initializes UpgradeableModularAccounts with a single plugin, SingleOwnerPlugin * intended for unit tests and local development, not for production. */ -contract MSCAFactoryFixture { +contract MSCAFactoryFixture is OptimizedTest { UpgradeableModularAccount public accountImplementation; SingleOwnerPlugin public singleOwnerPlugin; bytes32 private immutable _PROXY_BYTECODE_HASH; @@ -28,7 +30,7 @@ contract MSCAFactoryFixture { constructor(IEntryPoint _entryPoint, SingleOwnerPlugin _singleOwnerPlugin) { entryPoint = _entryPoint; - accountImplementation = new UpgradeableModularAccount(_entryPoint); + accountImplementation = _deployUpgradeableModularAccount(_entryPoint); _PROXY_BYTECODE_HASH = keccak256( abi.encodePacked(type(ERC1967Proxy).creationCode, abi.encode(address(accountImplementation), "")) ); diff --git a/test/mocks/MockPlugin.sol b/test/mocks/MockPlugin.sol index 7daa8e5a..1a71e999 100644 --- a/test/mocks/MockPlugin.sol +++ b/test/mocks/MockPlugin.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.19; import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; -import {PluginManifest, IPlugin} from "../../src/interfaces/IPlugin.sol"; +import {PluginManifest, IPlugin, PluginMetadata} from "../../src/interfaces/IPlugin.sol"; contract MockPlugin is ERC165 { // It's super inefficient to hold the entire abi-encoded manifest in storage, but this is fine since it's @@ -26,10 +26,6 @@ contract MockPlugin is ERC165 { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ constructor(PluginManifest memory _pluginManifest) { - _pluginManifest.name = NAME; - _pluginManifest.version = VERSION; - _pluginManifest.author = AUTHOR; - _manifest = abi.encode(_pluginManifest); } @@ -43,7 +39,7 @@ contract MockPlugin is ERC165 { pure returns (function() internal pure returns (PluginManifest memory) fnOut) { - assembly { + assembly ("memory-safe") { fnOut := fnIn } } @@ -52,6 +48,14 @@ contract MockPlugin is ERC165 { return _castToPure(_getManifest)(); } + function pluginMetadata() external pure returns (PluginMetadata memory) { + PluginMetadata memory metadata; + metadata.name = NAME; + metadata.version = VERSION; + metadata.author = AUTHOR; + return metadata; + } + /// @dev Returns true if this contract implements the interface defined by /// `interfaceId`. See the corresponding /// https://eips.ethereum.org/EIPS/eip-165#how-interfaces-are-identified[EIP section] @@ -64,6 +68,7 @@ contract MockPlugin is ERC165 { /// making calls to plugins. /// @param interfaceId The interface ID to check for support. /// @return True if the contract supports `interfaceId`. + function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) { return interfaceId == type(IPlugin).interfaceId || super.supportsInterface(interfaceId); } @@ -77,7 +82,8 @@ contract MockPlugin is ERC165 { || msg.sig == IPlugin.preExecutionHook.selector ) { // return 0 for userOp/runtimeVal case, return bytes("") for preExecutionHook case - assembly { + assembly ("memory-safe") { + mstore(0, 0) return(0x00, 0x20) } } diff --git a/test/mocks/plugins/BadTransferOwnershipPlugin.sol b/test/mocks/plugins/BadTransferOwnershipPlugin.sol index f7e6a744..0b291c03 100644 --- a/test/mocks/plugins/BadTransferOwnershipPlugin.sol +++ b/test/mocks/plugins/BadTransferOwnershipPlugin.sol @@ -7,7 +7,7 @@ import { ManifestAssociatedFunctionType, ManifestAssociatedFunction, PluginManifest, - ManifestExecutionFunction + PluginMetadata } from "../../../src/interfaces/IPlugin.sol"; import {IPluginManager} from "../../../src/interfaces/IPluginManager.sol"; import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; @@ -41,13 +41,8 @@ contract BadTransferOwnershipPlugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.name = NAME; - manifest.version = VERSION; - manifest.author = AUTHOR; - - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = - ManifestExecutionFunction(this.evilTransferOwnership.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.evilTransferOwnership.selector; manifest.permittedExecutionSelectors = new bytes4[](1); manifest.permittedExecutionSelectors[0] = ISingleOwnerPlugin.transferOwnership.selector; @@ -64,4 +59,12 @@ contract BadTransferOwnershipPlugin is BasePlugin { return manifest; } + + function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { + PluginMetadata memory metadata; + metadata.name = NAME; + metadata.version = VERSION; + metadata.author = AUTHOR; + return metadata; + } } diff --git a/test/mocks/plugins/BaseTestPlugin.sol b/test/mocks/plugins/BaseTestPlugin.sol new file mode 100644 index 00000000..508845c1 --- /dev/null +++ b/test/mocks/plugins/BaseTestPlugin.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; +import {PluginMetadata} from "../../../src/interfaces/IPlugin.sol"; + +contract BaseTestPlugin is BasePlugin { + // Don't need to implement this in each context + function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { + revert NotImplemented(); + } +} diff --git a/test/mocks/plugins/ComprehensivePlugin.sol b/test/mocks/plugins/ComprehensivePlugin.sol index 391b1cee..4be37ff8 100644 --- a/test/mocks/plugins/ComprehensivePlugin.sol +++ b/test/mocks/plugins/ComprehensivePlugin.sol @@ -9,7 +9,7 @@ import { ManifestAssociatedFunctionType, ManifestAssociatedFunction, PluginManifest, - ManifestExecutionFunction + PluginMetadata } from "../../../src/interfaces/IPlugin.sol"; import {IStandardExecutor} from "../../../src/interfaces/IStandardExecutor.sol"; import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; @@ -118,12 +118,8 @@ contract ComprehensivePlugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.name = NAME; - manifest.version = VERSION; - manifest.author = AUTHOR; - - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.foo.selector; ManifestFunction memory fooUserOpValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, @@ -247,4 +243,12 @@ contract ComprehensivePlugin is BasePlugin { return manifest; } + + function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { + PluginMetadata memory metadata; + metadata.name = NAME; + metadata.version = VERSION; + metadata.author = AUTHOR; + return metadata; + } } diff --git a/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol b/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol index d6ed7663..8861012f 100644 --- a/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol +++ b/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol @@ -7,13 +7,12 @@ import { ManifestAssociatedFunction, ManifestExternalCallPermission, ManifestExecutionHook, - PluginManifest, - ManifestExecutionFunction + PluginManifest } from "../../../src/interfaces/IPlugin.sol"; import {IStandardExecutor} from "../../../src/interfaces/IStandardExecutor.sol"; import {IPluginExecutor} from "../../../src/interfaces/IPluginExecutor.sol"; import {IPlugin} from "../../../src/interfaces/IPlugin.sol"; -import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; +import {BaseTestPlugin} from "./BaseTestPlugin.sol"; import {FunctionReference} from "../../../src/libraries/FunctionReferenceLib.sol"; import {ResultCreatorPlugin} from "./ReturnDataPluginMocks.sol"; @@ -25,7 +24,7 @@ address constant counter1 = 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f; address constant counter2 = 0x2e234DAe75C793f67A35089C9d99245E1C58470b; address constant counter3 = 0xF62849F9A0B5Bf2913b396098F7c7019b51A820a; -contract EFPCallerPlugin is BasePlugin { +contract EFPCallerPlugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -33,29 +32,18 @@ contract EFPCallerPlugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](11); - manifest.executionFunctions[0] = - ManifestExecutionFunction(this.useEFPPermissionAllowed.selector, new string[](0)); - manifest.executionFunctions[1] = - ManifestExecutionFunction(this.useEFPPermissionNotAllowed.selector, new string[](0)); - manifest.executionFunctions[2] = - ManifestExecutionFunction(this.setNumberCounter1.selector, new string[](0)); - manifest.executionFunctions[3] = - ManifestExecutionFunction(this.getNumberCounter1.selector, new string[](0)); - manifest.executionFunctions[4] = - ManifestExecutionFunction(this.incrementCounter1.selector, new string[](0)); - manifest.executionFunctions[5] = - ManifestExecutionFunction(this.setNumberCounter2.selector, new string[](0)); - manifest.executionFunctions[6] = - ManifestExecutionFunction(this.getNumberCounter2.selector, new string[](0)); - manifest.executionFunctions[7] = - ManifestExecutionFunction(this.incrementCounter2.selector, new string[](0)); - manifest.executionFunctions[8] = - ManifestExecutionFunction(this.setNumberCounter3.selector, new string[](0)); - manifest.executionFunctions[9] = - ManifestExecutionFunction(this.getNumberCounter3.selector, new string[](0)); - manifest.executionFunctions[10] = - ManifestExecutionFunction(this.incrementCounter3.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](11); + manifest.executionFunctions[0] = this.useEFPPermissionAllowed.selector; + manifest.executionFunctions[1] = this.useEFPPermissionNotAllowed.selector; + manifest.executionFunctions[2] = this.setNumberCounter1.selector; + manifest.executionFunctions[3] = this.getNumberCounter1.selector; + manifest.executionFunctions[4] = this.incrementCounter1.selector; + manifest.executionFunctions[5] = this.setNumberCounter2.selector; + manifest.executionFunctions[6] = this.getNumberCounter2.selector; + manifest.executionFunctions[7] = this.incrementCounter2.selector; + manifest.executionFunctions[8] = this.setNumberCounter3.selector; + manifest.executionFunctions[9] = this.getNumberCounter3.selector; + manifest.executionFunctions[10] = this.incrementCounter3.selector; manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](11); @@ -67,7 +55,7 @@ contract EFPCallerPlugin is BasePlugin { for (uint256 i = 0; i < manifest.executionFunctions.length; i++) { manifest.runtimeValidationFunctions[i] = ManifestAssociatedFunction({ - executionSelector: manifest.executionFunctions[i].selector, + executionSelector: manifest.executionFunctions[i], associatedFunction: alwaysAllowValidationFunction }); } @@ -182,7 +170,7 @@ contract EFPCallerPlugin is BasePlugin { } } -contract EFPCallerPluginAnyExternal is BasePlugin { +contract EFPCallerPluginAnyExternal is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -190,9 +178,8 @@ contract EFPCallerPluginAnyExternal is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = - ManifestExecutionFunction(this.passthroughExecute.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.passthroughExecute.selector; manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ @@ -204,7 +191,7 @@ contract EFPCallerPluginAnyExternal is BasePlugin { }) }); - manifest.permitAnyExternalContract = true; + manifest.permitAnyExternalAddress = true; return manifest; } @@ -219,7 +206,7 @@ contract EFPCallerPluginAnyExternal is BasePlugin { } // Create pre and post permitted call hooks for calling ResultCreatorPlugin.foo via `executeFromPlugin` -contract EFPPermittedCallHookPlugin is BasePlugin { +contract EFPPermittedCallHookPlugin is BaseTestPlugin { bool public preExecHookCalled; bool public postExecHookCalled; @@ -242,8 +229,8 @@ contract EFPPermittedCallHookPlugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.performEFPCall.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.performEFPCall.selector; manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ @@ -282,7 +269,7 @@ contract EFPPermittedCallHookPlugin is BasePlugin { } // Creates pre and post permitted call hooks for `executeFromPluginExternal` -contract EFPExternalPermittedCallHookPlugin is BasePlugin { +contract EFPExternalPermittedCallHookPlugin is BaseTestPlugin { bool public preExecHookCalled; bool public postExecHookCalled; @@ -305,8 +292,8 @@ contract EFPExternalPermittedCallHookPlugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.performIncrement.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.performIncrement.selector; manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ @@ -333,7 +320,7 @@ contract EFPExternalPermittedCallHookPlugin is BasePlugin { }) }); - manifest.permitAnyExternalContract = true; + manifest.permitAnyExternalAddress = true; return manifest; } diff --git a/test/mocks/plugins/ManifestValidityMocks.sol b/test/mocks/plugins/ManifestValidityMocks.sol index 4241a4d2..6ec719f4 100644 --- a/test/mocks/plugins/ManifestValidityMocks.sol +++ b/test/mocks/plugins/ManifestValidityMocks.sol @@ -7,16 +7,15 @@ import { ManifestAssociatedFunction, ManifestExecutionHook, ManifestExternalCallPermission, - PluginManifest, - ManifestExecutionFunction + PluginManifest } from "../../../src/interfaces/IPlugin.sol"; import {IStandardExecutor} from "../../../src/interfaces/IStandardExecutor.sol"; import {IPluginExecutor} from "../../../src/interfaces/IPluginExecutor.sol"; import {IPlugin} from "../../../src/interfaces/IPlugin.sol"; -import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; +import {BaseTestPlugin} from "./BaseTestPlugin.sol"; import {FunctionReference} from "../../../src/libraries/FunctionReferenceLib.sol"; -contract BadValidationMagicValue_UserOp_Plugin is BasePlugin { +contract BadValidationMagicValue_UserOp_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -28,8 +27,8 @@ contract BadValidationMagicValue_UserOp_Plugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.foo.selector; manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ @@ -46,7 +45,7 @@ contract BadValidationMagicValue_UserOp_Plugin is BasePlugin { } } -contract BadValidationMagicValue_PreRuntimeValidationHook_Plugin is BasePlugin { +contract BadValidationMagicValue_PreRuntimeValidationHook_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -58,8 +57,8 @@ contract BadValidationMagicValue_PreRuntimeValidationHook_Plugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.foo.selector; manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ @@ -72,7 +71,6 @@ contract BadValidationMagicValue_PreRuntimeValidationHook_Plugin is BasePlugin { }); manifest.preRuntimeValidationHooks = new ManifestAssociatedFunction[](1); - // Illegal assignment: validation always allow only usable on runtime validation functions manifest.preRuntimeValidationHooks[0] = ManifestAssociatedFunction({ executionSelector: this.foo.selector, @@ -87,7 +85,7 @@ contract BadValidationMagicValue_PreRuntimeValidationHook_Plugin is BasePlugin { } } -contract BadValidationMagicValue_PreUserOpValidationHook_Plugin is BasePlugin { +contract BadValidationMagicValue_PreUserOpValidationHook_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -99,8 +97,8 @@ contract BadValidationMagicValue_PreUserOpValidationHook_Plugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.foo.selector; manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ @@ -113,7 +111,6 @@ contract BadValidationMagicValue_PreUserOpValidationHook_Plugin is BasePlugin { }); manifest.preUserOpValidationHooks = new ManifestAssociatedFunction[](1); - // Illegal assignment: validation always allow only usable on runtime validation functions manifest.preUserOpValidationHooks[0] = ManifestAssociatedFunction({ executionSelector: this.foo.selector, @@ -128,7 +125,7 @@ contract BadValidationMagicValue_PreUserOpValidationHook_Plugin is BasePlugin { } } -contract BadValidationMagicValue_PreExecHook_Plugin is BasePlugin { +contract BadValidationMagicValue_PreExecHook_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -140,8 +137,8 @@ contract BadValidationMagicValue_PreExecHook_Plugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.foo.selector; manifest.executionHooks = new ManifestExecutionHook[](1); @@ -164,7 +161,7 @@ contract BadValidationMagicValue_PreExecHook_Plugin is BasePlugin { } } -contract BadValidationMagicValue_PostExecHook_Plugin is BasePlugin { +contract BadValidationMagicValue_PostExecHook_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -176,11 +173,10 @@ contract BadValidationMagicValue_PostExecHook_Plugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.foo.selector; manifest.executionHooks = new ManifestExecutionHook[](1); - // Illegal assignment: validation always allow only usable on runtime validation functions manifest.executionHooks[0] = ManifestExecutionHook({ executionSelector: this.foo.selector, @@ -200,7 +196,7 @@ contract BadValidationMagicValue_PostExecHook_Plugin is BasePlugin { } } -contract BadHookMagicValue_UserOpValidationFunction_Plugin is BasePlugin { +contract BadHookMagicValue_UserOpValidationFunction_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -212,8 +208,8 @@ contract BadHookMagicValue_UserOpValidationFunction_Plugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.foo.selector; manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ @@ -229,7 +225,7 @@ contract BadHookMagicValue_UserOpValidationFunction_Plugin is BasePlugin { } } -contract BadHookMagicValue_RuntimeValidationFunction_Plugin is BasePlugin { +contract BadHookMagicValue_RuntimeValidationFunction_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -241,8 +237,8 @@ contract BadHookMagicValue_RuntimeValidationFunction_Plugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.foo.selector; manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ @@ -258,7 +254,7 @@ contract BadHookMagicValue_RuntimeValidationFunction_Plugin is BasePlugin { } } -contract BadHookMagicValue_PostExecHook_Plugin is BasePlugin { +contract BadHookMagicValue_PostExecHook_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -270,11 +266,10 @@ contract BadHookMagicValue_PostExecHook_Plugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.foo.selector; manifest.executionHooks = new ManifestExecutionHook[](1); - // Illegal assignment: hook always deny only usable on runtime validation functions manifest.executionHooks[0] = ManifestExecutionHook({ executionSelector: this.foo.selector, diff --git a/test/mocks/plugins/ReturnDataPluginMocks.sol b/test/mocks/plugins/ReturnDataPluginMocks.sol index c0dd4d85..e226a329 100644 --- a/test/mocks/plugins/ReturnDataPluginMocks.sol +++ b/test/mocks/plugins/ReturnDataPluginMocks.sol @@ -6,13 +6,12 @@ import { ManifestAssociatedFunctionType, ManifestAssociatedFunction, ManifestExternalCallPermission, - PluginManifest, - ManifestExecutionFunction + PluginManifest } from "../../../src/interfaces/IPlugin.sol"; import {IStandardExecutor} from "../../../src/interfaces/IStandardExecutor.sol"; import {IPluginExecutor} from "../../../src/interfaces/IPluginExecutor.sol"; import {IPlugin} from "../../../src/interfaces/IPlugin.sol"; -import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; +import {BaseTestPlugin} from "./BaseTestPlugin.sol"; import {FunctionReference} from "../../../src/libraries/FunctionReferenceLib.sol"; contract RegularResultContract { @@ -25,7 +24,7 @@ contract RegularResultContract { } } -contract ResultCreatorPlugin is BasePlugin { +contract ResultCreatorPlugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -41,9 +40,9 @@ contract ResultCreatorPlugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](2); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); - manifest.executionFunctions[1] = ManifestExecutionFunction(this.bar.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](2); + manifest.executionFunctions[0] = this.foo.selector; + manifest.executionFunctions[1] = this.bar.selector; manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ @@ -59,7 +58,7 @@ contract ResultCreatorPlugin is BasePlugin { } } -contract ResultConsumerPlugin is BasePlugin { +contract ResultConsumerPlugin is BaseTestPlugin { ResultCreatorPlugin public immutable resultCreator; RegularResultContract public immutable regularResultContract; @@ -117,11 +116,9 @@ contract ResultConsumerPlugin is BasePlugin { function _innerPluginManifest() internal view returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](2); - manifest.executionFunctions[0] = - ManifestExecutionFunction(this.checkResultEFPFallback.selector, new string[](0)); - manifest.executionFunctions[1] = - ManifestExecutionFunction(this.checkResultEFPExternal.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](2); + manifest.executionFunctions[0] = this.checkResultEFPFallback.selector; + manifest.executionFunctions[1] = this.checkResultEFPExternal.selector; manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](2); manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ diff --git a/test/mocks/plugins/ValidationPluginMocks.sol b/test/mocks/plugins/ValidationPluginMocks.sol index adcc7052..1e6cc941 100644 --- a/test/mocks/plugins/ValidationPluginMocks.sol +++ b/test/mocks/plugins/ValidationPluginMocks.sol @@ -7,12 +7,11 @@ import { ManifestFunction, ManifestAssociatedFunctionType, ManifestAssociatedFunction, - PluginManifest, - ManifestExecutionFunction + PluginManifest } from "../../../src/interfaces/IPlugin.sol"; -import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; +import {BaseTestPlugin} from "./BaseTestPlugin.sol"; -abstract contract MockBaseUserOpValidationPlugin is BasePlugin { +abstract contract MockBaseUserOpValidationPlugin is BaseTestPlugin { enum FunctionId { USER_OP_VALIDATION, PRE_USER_OP_VALIDATION_HOOK_1, @@ -76,8 +75,8 @@ contract MockUserOpValidationPlugin is MockBaseUserOpValidationPlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.foo.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.foo.selector; manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ @@ -114,8 +113,8 @@ contract MockUserOpValidation1HookPlugin is MockBaseUserOpValidationPlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.bar.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.bar.selector; ManifestFunction memory userOpValidationFunctionRef = ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, @@ -166,8 +165,8 @@ contract MockUserOpValidation2HookPlugin is MockBaseUserOpValidationPlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.baz.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.baz.selector; ManifestFunction memory userOpValidationFunctionRef = ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, diff --git a/test/plugin/SingleOwnerPlugin.t.sol b/test/plugin/SingleOwnerPlugin.t.sol index d64d4a1d..090e5be1 100644 --- a/test/plugin/SingleOwnerPlugin.t.sol +++ b/test/plugin/SingleOwnerPlugin.t.sol @@ -1,8 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {Test} from "forge-std/Test.sol"; - import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; @@ -10,8 +8,9 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.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"; -contract SingleOwnerPluginTest is Test { +contract SingleOwnerPluginTest is OptimizedTest { using ECDSA for bytes32; SingleOwnerPlugin public plugin; @@ -29,7 +28,7 @@ contract SingleOwnerPluginTest is Test { event OwnershipTransferred(address indexed account, address indexed previousOwner, address indexed newOwner); function setUp() public { - plugin = new SingleOwnerPlugin(); + plugin = _deploySingleOwnerPlugin(); entryPoint = new EntryPoint(); a = makeAddr("a"); diff --git a/test/plugin/TokenReceiverPlugin.t.sol b/test/plugin/TokenReceiverPlugin.t.sol index e664e68b..009b42e9 100644 --- a/test/plugin/TokenReceiverPlugin.t.sol +++ b/test/plugin/TokenReceiverPlugin.t.sol @@ -1,8 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {Test} from "forge-std/Test.sol"; - import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; import {ERC721PresetMinterPauserAutoId} from @@ -10,7 +8,6 @@ import {ERC721PresetMinterPauserAutoId} from import {IERC777Recipient} from "@openzeppelin/contracts/token/ERC777/IERC777Recipient.sol"; import {IERC1155Receiver} from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol"; -import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {TokenReceiverPlugin} from "../../src/plugins/TokenReceiverPlugin.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {FunctionReference} from "../../src/libraries/FunctionReferenceLib.sol"; @@ -19,8 +16,9 @@ import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; import {MockERC777} from "../mocks/MockERC777.sol"; import {MockERC1155} from "../mocks/MockERC1155.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; -contract TokenReceiverPluginTest is Test, IERC1155Receiver { +contract TokenReceiverPluginTest is OptimizedTest, IERC1155Receiver { UpgradeableModularAccount public acct; TokenReceiverPlugin public plugin; @@ -39,10 +37,10 @@ contract TokenReceiverPluginTest is Test, IERC1155Receiver { uint256 internal constant _BATCH_TOKEN_IDS = 5; function setUp() public { - MSCAFactoryFixture factory = new MSCAFactoryFixture(IEntryPoint(address(0)), new SingleOwnerPlugin()); + MSCAFactoryFixture factory = new MSCAFactoryFixture(IEntryPoint(address(0)), _deploySingleOwnerPlugin()); acct = factory.createAccount(address(this), 0); - plugin = new TokenReceiverPlugin(); + plugin = _deployTokenReceiverPlugin(); t0 = new ERC721PresetMinterPauserAutoId("t0", "t0", ""); t0.mint(address(this)); diff --git a/test/utils/OptimizedTest.sol b/test/utils/OptimizedTest.sol new file mode 100644 index 00000000..f9431acc --- /dev/null +++ b/test/utils/OptimizedTest.sol @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {Test} from "forge-std/Test.sol"; + +import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; + +import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; +import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; +import {TokenReceiverPlugin} from "../../src/plugins/TokenReceiverPlugin.sol"; + +/// @dev This contract provides functions to deploy optimized (via IR) precompiled contracts. By compiling just +/// the source contracts (excluding the test suite) via IR, and using the resulting bytecode within the tests +/// (built without IR), we can avoid the significant overhead of compiling the entire test suite via IR. +/// +/// To use the optimized precompiled contracts, the project must first be built with the "optimized-build" profile +/// to populate the artifacts in the `out-optimized` directory. Then use the "optimized-test" or +/// "optimized-test-deep" profile to run the tests. +/// +/// To bypass this behavior for coverage or debugging, use the "default" profile. +abstract contract OptimizedTest is Test { + function _isOptimizedTest() internal returns (bool) { + string memory profile = vm.envOr("FOUNDRY_PROFILE", string("default")); + return _isStringEq(profile, "optimized-test-deep") || _isStringEq(profile, "optimized-test"); + } + + function _isStringEq(string memory a, string memory b) internal pure returns (bool) { + return keccak256(abi.encodePacked(a)) == keccak256(abi.encodePacked(b)); + } + + function _deployUpgradeableModularAccount(IEntryPoint entryPoint) + internal + returns (UpgradeableModularAccount) + { + return _isOptimizedTest() + ? UpgradeableModularAccount( + payable( + deployCode( + "out-optimized/UpgradeableModularAccount.sol/UpgradeableModularAccount.json", + abi.encode(entryPoint) + ) + ) + ) + : new UpgradeableModularAccount(entryPoint); + } + + function _deploySingleOwnerPlugin() internal returns (SingleOwnerPlugin) { + return _isOptimizedTest() + ? SingleOwnerPlugin(deployCode("out-optimized/SingleOwnerPlugin.sol/SingleOwnerPlugin.json")) + : new SingleOwnerPlugin(); + } + + function _deployTokenReceiverPlugin() internal returns (TokenReceiverPlugin) { + return _isOptimizedTest() + ? TokenReceiverPlugin(deployCode("out-optimized/TokenReceiverPlugin.sol/TokenReceiverPlugin.json")) + : new TokenReceiverPlugin(); + } +}