refactor: move FunctionReference to IPluginManager

src/account/AccountLoupe.sol

@@ -5,11 +5,10 @@ import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeab
 import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol";
 import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 
-import {AccountStorage, getAccountStorage, HookGroup, toFunctionReferenceArray} from "./AccountStorage.sol";
-import {FunctionReference} from "../helpers/FunctionReferenceLib.sol";
 import {IAccountLoupe} from "../interfaces/IAccountLoupe.sol";
-import {IPluginManager} from "../interfaces/IPluginManager.sol";
+import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol";
 import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol";
+import {AccountStorage, getAccountStorage, HookGroup, toFunctionReferenceArray} from "./AccountStorage.sol";
 
 abstract contract AccountLoupe is IAccountLoupe {
     using EnumerableMap for EnumerableMap.Bytes32ToUintMap;

src/account/AccountStorage.sol

@@ -4,8 +4,8 @@ pragma solidity ^0.8.19;
 import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol";
 import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 
-import {FunctionReference} from "../helpers/FunctionReferenceLib.sol";
 import {IPlugin} from "../interfaces/IPlugin.sol";
+import {FunctionReference} from "../interfaces/IPluginManager.sol";
 
 // bytes = keccak256("ERC6900.UpgradeableModularAccount.Storage")
 bytes32 constant _ACCOUNT_STORAGE_SLOT = 0x9f09680beaa4e5c9f38841db2460c401499164f368baef687948c315d9073e40;

src/account/PluginManagerInternals.sol

@@ -1,20 +1,11 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity ^0.8.19;
 
+import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.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 {
-    AccountStorage,
-    getAccountStorage,
-    SelectorData,
-    getPermittedCallKey,
-    HookGroup,
-    PermittedExternalCallData
-} from "./AccountStorage.sol";
-import {FunctionReference, FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol";
-import {IPluginManager} from "../interfaces/IPluginManager.sol";
+import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol";
 import {
     IPlugin,
     ManifestExecutionHook,
@@ -24,10 +15,20 @@ import {
     ManifestExternalCallPermission,
     PluginManifest
 } from "../interfaces/IPlugin.sol";
+import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol";
+import {
+    AccountStorage,
+    getAccountStorage,
+    SelectorData,
+    getPermittedCallKey,
+    HookGroup,
+    PermittedExternalCallData
+} from "./AccountStorage.sol";
 
 abstract contract PluginManagerInternals is IPluginManager {
     using EnumerableMap for EnumerableMap.Bytes32ToUintMap;
     using EnumerableSet for EnumerableSet.AddressSet;
+    using FunctionReferenceLib for FunctionReference;
 
     error ArrayLengthMismatch();
     error ExecutionFunctionAlreadySet(bytes4 selector);

src/account/UpgradeableModularAccount.sol

@@ -2,24 +2,24 @@
 pragma solidity ^0.8.19;
 
 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 {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.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 {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol";
+import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationDataHelpers.sol";
+import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol";
+import {IPluginExecutor} from "../interfaces/IPluginExecutor.sol";
+import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol";
+import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol";
 import {AccountExecutor} from "./AccountExecutor.sol";
 import {AccountLoupe} from "./AccountLoupe.sol";
 import {AccountStorage, HookGroup, getAccountStorage, getPermittedCallKey} from "./AccountStorage.sol";
 import {AccountStorageInitializable} from "./AccountStorageInitializable.sol";
-import {FunctionReference, FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol";
-import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol";
-import {IPluginExecutor} from "../interfaces/IPluginExecutor.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
     AccountExecutor,
@@ -34,6 +34,7 @@ contract UpgradeableModularAccount is
 {
     using EnumerableMap for EnumerableMap.Bytes32ToUintMap;
     using EnumerableSet for EnumerableSet.Bytes32Set;
+    using FunctionReferenceLib for FunctionReference;
 
     struct PostExecToRun {
         bytes preExecHookReturnData;
@@ -437,7 +438,7 @@ contract UpgradeableModularAccount is
                     ++i;
                 }
             } else {
-                if (preRuntimeValidationHook == FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY) {
+                if (preRuntimeValidationHook.eq(FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY)) {
                     revert AlwaysDenyRule();
                 }
                 // Function reference cannot be 0 or _RUNTIME_VALIDATION_ALWAYS_ALLOW.
@@ -457,7 +458,7 @@ contract UpgradeableModularAccount is
             } else {
                 if (runtimeValidationFunction.isEmpty()) {
                     revert RuntimeValidationFunctionMissing(msg.sig);
-                } else if (runtimeValidationFunction == FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY) {
+                } else if (runtimeValidationFunction.eq(FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY)) {
                     revert InvalidConfiguration();
                 }
                 // If _RUNTIME_VALIDATION_ALWAYS_ALLOW, just let the function finish.

src/helpers/FunctionReferenceLib.sol

@@ -1,10 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity ^0.8.19;
 
-type FunctionReference is bytes21;
-
-using {eq as ==, notEq as !=} for FunctionReference global;
-using FunctionReferenceLib for FunctionReference global;
+import {FunctionReference} from "../interfaces/IPluginManager.sol";
 
 library FunctionReferenceLib {
     // Empty or unset function reference.
@@ -26,18 +23,18 @@ library FunctionReferenceLib {
     }
 
     function isEmpty(FunctionReference fr) internal pure returns (bool) {
-        return fr == _EMPTY_FUNCTION_REFERENCE;
+        return FunctionReference.unwrap(fr) == bytes21(0);
     }
 
     function isEmptyOrMagicValue(FunctionReference fr) internal pure returns (bool) {
         return FunctionReference.unwrap(fr) <= bytes21(uint168(2));
     }
-}
 
-function eq(FunctionReference a, FunctionReference b) pure returns (bool) {
-    return FunctionReference.unwrap(a) == FunctionReference.unwrap(b);
-}
+    function eq(FunctionReference a, FunctionReference b) internal pure returns (bool) {
+        return FunctionReference.unwrap(a) == FunctionReference.unwrap(b);
+    }
 
-function notEq(FunctionReference a, FunctionReference b) pure returns (bool) {
-    return FunctionReference.unwrap(a) != FunctionReference.unwrap(b);
+    function notEq(FunctionReference a, FunctionReference b) internal pure returns (bool) {
+        return FunctionReference.unwrap(a) != FunctionReference.unwrap(b);
+    }
 }

src/interfaces/IAccountLoupe.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: CC0-1.0
 pragma solidity ^0.8.19;
 
-import {FunctionReference} from "../helpers/FunctionReferenceLib.sol";
+import {FunctionReference} from "../interfaces/IPluginManager.sol";
 
 interface IAccountLoupe {
     /// @notice Config for an execution function, given a selector.

src/interfaces/IPluginManager.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: CC0-1.0
 pragma solidity ^0.8.19;
 
-import {FunctionReference} from "../helpers/FunctionReferenceLib.sol";
+type FunctionReference is bytes21;
 
 interface IPluginManager {
     event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies);

test/libraries/FunctionReferenceLib.t.sol

@@ -3,9 +3,12 @@ pragma solidity ^0.8.19;
 
 import {Test} from "forge-std/Test.sol";
 
-import {FunctionReference, FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol";
+import {FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol";
+import {FunctionReference} from "../../src/interfaces/IPluginManager.sol";
 
 contract FunctionReferenceLibTest is Test {
+    using FunctionReferenceLib for FunctionReference;
+
     function testFuzz_functionReference_packing(address addr, uint8 functionId) public {
         // console.log("addr: ", addr);
         // console.log("functionId: ", vm.toString(functionId));
@@ -19,19 +22,19 @@ contract FunctionReferenceLibTest is Test {
     }
 
     function testFuzz_functionReference_operators(FunctionReference a, FunctionReference b) public {
-        assertTrue(a == a);
-        assertTrue(b == b);
+        assertTrue(a.eq(a));
+        assertTrue(b.eq(b));
 
         if (FunctionReference.unwrap(a) == FunctionReference.unwrap(b)) {
-            assertTrue(a == b);
-            assertTrue(b == a);
-            assertFalse(a != b);
-            assertFalse(b != a);
+            assertTrue(a.eq(b));
+            assertTrue(b.eq(a));
+            assertFalse(a.notEq(b));
+            assertFalse(b.notEq(a));
         } else {
-            assertTrue(a != b);
-            assertTrue(b != a);
-            assertFalse(a == b);
-            assertFalse(b == a);
+            assertTrue(a.notEq(b));
+            assertTrue(b.notEq(a));
+            assertFalse(a.eq(b));
+            assertFalse(b.eq(a));
         }
     }
 }