update BaseModularAccount to PluginManagerInternals and clean it up

erc6900 · Nov 28, 2023 · 67b7583 · 67b7583
1 parent 2693cc8
commit 67b7583
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 35 deletions.
diff --git 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/BaseModularAccount.sol → src/account/PluginManagerInternals.sol b/src/account/BaseModularAccount.sol → src/account/PluginManagerInternals.sol
@@ -1,13 +1,11 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity ^0.8.19;
 
+import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
 import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
-import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 
-import {IPluginManager} from "../interfaces/IPluginManager.sol";
 import {AccountExecutor} from "./AccountExecutor.sol";
-import {FunctionReference, FunctionReferenceLib} from "../libraries/FunctionReferenceLib.sol";
 import {
     AccountStorage,
     getAccountStorage,
@@ -17,6 +15,8 @@ import {
     PermittedExternalCallData,
     StoredInjectedHook
 } from "../libraries/AccountStorage.sol";
+import {FunctionReference, FunctionReferenceLib} from "../libraries/FunctionReferenceLib.sol";
+import {IPluginManager} from "../interfaces/IPluginManager.sol";
 import {
     IPlugin,
     ManifestExecutionHook,
@@ -27,14 +27,10 @@ import {
     PluginManifest
 } from "../interfaces/IPlugin.sol";
 
-abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 {
+abstract contract PluginManagerInternals is IPluginManager {
     using EnumerableSet for EnumerableSet.Bytes32Set;
     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);

diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol
@@ -1,31 +1,35 @@
 // 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 {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 {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
+
+import {AccountExecutor} from "./AccountExecutor.sol";
 import {AccountLoupe} from "./AccountLoupe.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 {AccountStorageInitializable} from "./AccountStorageInitializable.sol";
 import {Execution} from "../libraries/ERC6900TypeUtils.sol";
 import {FunctionReference, FunctionReferenceLib} from "../libraries/FunctionReferenceLib.sol";
-import {AccountStorageInitializable} from "./AccountStorageInitializable.sol";
+import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol";
+import {IPluginExecutor} from "../interfaces/IPluginExecutor.sol";
 import {IPluginManager} from "../interfaces/IPluginManager.sol";
+import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol";
+import {PluginManagerInternals} from "./PluginManagerInternals.sol";
 import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationDataHelpers.sol";
 
 contract UpgradeableModularAccount is
+    AccountExecutor,
     IPluginManager,
     BaseAccount,
-    BaseModularAccount,
+    PluginManagerInternals,
     AccountLoupe,
     UUPSUpgradeable,
     AccountStorageInitializable,
+    IERC165,
     IStandardExecutor,
     IPluginExecutor
 {
@@ -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();

diff --git 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,

diff --git a/test/account/ManifestValidity.t.sol b/test/account/ManifestValidity.t.sol
@@ -7,7 +7,7 @@ import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.so
 
 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";
 
@@ -47,7 +47,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 +65,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 +83,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 +99,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 +115,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 +132,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 +149,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 +165,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
@@ -7,7 +7,7 @@ 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";
@@ -316,7 +316,7 @@ contract UpgradeableModularAccountTest is Test {
 
         vm.expectRevert(
             abi.encodeWithSelector(
-                BaseModularAccount.PermittedExecutionSelectorNotInstalled.selector,
+                PluginManagerInternals.PermittedExecutionSelectorNotInstalled.selector,
                 IPlugin.onInstall.selector,
                 address(mockPluginWithBadPermittedExec)
             )
@@ -333,7 +333,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),
@@ -348,7 +348,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),
@@ -373,7 +373,7 @@ contract UpgradeableModularAccountTest is Test {
 
         vm.expectRevert(
             abi.encodeWithSelector(
-                BaseModularAccount.PluginAlreadyInstalled.selector, address(tokenReceiverPlugin)
+                PluginManagerInternals.PluginAlreadyInstalled.selector, address(tokenReceiverPlugin)
             )
         );
         IPluginManager(account2).installPlugin({
@@ -455,7 +455,7 @@ 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),
@@ -585,7 +585,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({
@@ -616,7 +616,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),
@@ -652,7 +652,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),