feat: add events to AllowlistModule

src/modules/permissionhooks/AllowlistModule.sol

@@ -27,6 +27,13 @@ contract AllowlistModule is IValidationHook, BaseModule {
         uint32 entityId => mapping(address target => mapping(bytes4 selector => mapping(address account => bool)))
     ) public selectorAllowlist;
 
+    event AllowlistTargetUpdated(
+        uint32 indexed entityId, address indexed account, address indexed target, AllowlistEntry entry
+    );
+    event AllowlistSelectorUpdated(
+        uint32 indexed entityId, address indexed account, bytes24 indexed targetAndSelector, bool allowed
+    );
+
     error TargetNotAllowed();
     error SelectorNotAllowed();
     error NoSelectorSpecified();
@@ -35,12 +42,11 @@ contract AllowlistModule is IValidationHook, BaseModule {
         (uint32 entityId, AllowlistInit[] memory init) = abi.decode(data, (uint32, AllowlistInit[]));
 
         for (uint256 i = 0; i < init.length; i++) {
-            targetAllowlist[entityId][init[i].target][msg.sender] =
-                AllowlistEntry(true, init[i].hasSelectorAllowlist);
+            setAllowlistTarget(entityId, init[i].target, true, init[i].hasSelectorAllowlist);
 
             if (init[i].hasSelectorAllowlist) {
                 for (uint256 j = 0; j < init[i].selectors.length; j++) {
-                    selectorAllowlist[entityId][init[i].target][init[i].selectors[j]][msg.sender] = true;
+                    setAllowlistSelector(entityId, init[i].target, init[i].selectors[j], true);
                 }
             }
         }
@@ -50,26 +56,16 @@ contract AllowlistModule is IValidationHook, BaseModule {
         (uint32 entityId, AllowlistInit[] memory init) = abi.decode(data, (uint32, AllowlistInit[]));
 
         for (uint256 i = 0; i < init.length; i++) {
-            delete targetAllowlist[entityId][init[i].target][msg.sender];
+            setAllowlistTarget(entityId, init[i].target, false, false);
 
             if (init[i].hasSelectorAllowlist) {
                 for (uint256 j = 0; j < init[i].selectors.length; j++) {
-                    delete selectorAllowlist[entityId][init[i].target][init[i].selectors[j]][msg.sender];
+                    setAllowlistSelector(entityId, init[i].target, init[i].selectors[j], false);
                 }
             }
         }
     }
 
-    function setAllowlistTarget(uint32 entityId, address target, bool allowed, bool hasSelectorAllowlist)
-        external
-    {
-        targetAllowlist[entityId][target][msg.sender] = AllowlistEntry(allowed, hasSelectorAllowlist);
-    }
-
-    function setAllowlistSelector(uint32 entityId, address target, bytes4 selector, bool allowed) external {
-        selectorAllowlist[entityId][target][selector][msg.sender] = allowed;
-    }
-
     function preUserOpValidationHook(uint32 entityId, PackedUserOperation calldata userOp, bytes32)
         external
         view
@@ -98,6 +94,18 @@ contract AllowlistModule is IValidationHook, BaseModule {
         return metadata;
     }
 
+    function setAllowlistTarget(uint32 entityId, address target, bool allowed, bool hasSelectorAllowlist) public {
+        AllowlistEntry memory entry = AllowlistEntry(allowed, hasSelectorAllowlist);
+        targetAllowlist[entityId][target][msg.sender] = entry;
+        emit AllowlistTargetUpdated(entityId, msg.sender, target, entry);
+    }
+
+    function setAllowlistSelector(uint32 entityId, address target, bytes4 selector, bool allowed) public {
+        selectorAllowlist[entityId][target][selector][msg.sender] = allowed;
+        bytes24 targetAndSelector = bytes24(bytes24(bytes20(target)) | (bytes24(selector) >> 160));
+        emit AllowlistSelectorUpdated(entityId, msg.sender, targetAndSelector, allowed);
+    }
+
     function checkAllowlistCalldata(uint32 entityId, bytes calldata callData) public view {
         if (bytes4(callData[:4]) == IStandardExecutor.execute.selector) {
             (address target,, bytes memory data) = abi.decode(callData[4:], (address, uint256, bytes));

test/module/AllowlistModule.t.sol

@@ -23,6 +23,16 @@ contract AllowlistModuleTest is CustomValidationTestBase {
 
     uint32 public constant HOOK_ENTITY_ID = 0;
 
+    event AllowlistTargetUpdated(
+        uint32 indexed entityId,
+        address indexed account,
+        address indexed target,
+        AllowlistModule.AllowlistEntry entry
+    );
+    event AllowlistSelectorUpdated(
+        uint32 indexed entityId, address indexed account, bytes24 indexed targetAndSelector, bool allowed
+    );
+
     function setUp() public {
         allowlistModule = new AllowlistModule();
 
@@ -99,6 +109,34 @@ contract AllowlistModuleTest is CustomValidationTestBase {
         }
     }
 
+    function _beforeInstallStep(address accountImpl) internal override {
+        // Expect events to be emitted from onInstall
+        for (uint256 i = 0; i < allowlistInit.length; i++) {
+            vm.expectEmit(address(allowlistModule));
+            emit AllowlistTargetUpdated(
+                HOOK_ENTITY_ID,
+                accountImpl,
+                allowlistInit[i].target,
+                AllowlistModule.AllowlistEntry({
+                    allowed: true,
+                    hasSelectorAllowlist: allowlistInit[i].hasSelectorAllowlist
+                })
+            );
+
+            if (!allowlistInit[i].hasSelectorAllowlist) {
+                continue;
+            }
+
+            for (uint256 j = 0; j < allowlistInit[i].selectors.length; j++) {
+                bytes24 targetAndSelector = bytes24(
+                    bytes24(bytes20(allowlistInit[i].target)) | (bytes24(allowlistInit[i].selectors[j]) >> 160)
+                );
+                vm.expectEmit(address(allowlistModule));
+                emit AllowlistSelectorUpdated(HOOK_ENTITY_ID, accountImpl, targetAndSelector, true);
+            }
+        }
+    }
+
     function _generateRandomCalls(uint256 seed) internal view returns (Call[] memory, uint256) {
         uint256 length = seed % 10;
         seed = _next(seed);
@@ -285,12 +323,6 @@ contract AllowlistModuleTest is CustomValidationTestBase {
         return (init, seed);
     }
 
-    // todo: runtime paths
-
-    // fuzz targets, fuzz target selectors.
-
-    // Maybe pull out the helper function for running user ops and possibly expect a failure?
-
     function _next(uint256 seed) internal pure returns (uint256) {
         return uint256(keccak256(abi.encodePacked(seed)));
     }

test/utils/CustomValidationTestBase.sol

@@ -28,6 +28,8 @@ abstract contract CustomValidationTestBase is AccountTestBase {
 
         account1 = UpgradeableModularAccount(payable(new ERC1967Proxy{salt: 0}(accountImplementation, "")));
 
+        _beforeInstallStep(address(account1));
+
         account1.initializeWithValidation(
             ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation),
             selectors,
@@ -49,4 +51,12 @@ abstract contract CustomValidationTestBase is AccountTestBase {
             bytes memory installData,
             bytes[] memory hooks
         );
+
+    // If the test needs to perform any setup or checks after the account is created, but before the call to
+    // `initializeWithValidation`,
+    // it should override this function.
+    function _beforeInstallStep(address accountImpl) internal virtual {
+        // Does nothing by default
+        (accountImpl);
+    }
 }