Skip to content

Commit

Permalink
chore: add events emision for validation install and uninstall
Browse files Browse the repository at this point in the history
  • Loading branch information
fangting-alchemy committed Jul 31, 2024
1 parent 2fadf6c commit e1a0783
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/account/AccountLoupe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {HookConfigLib} from "../helpers/HookConfigLib.sol";
import {ExecutionDataView, IAccountLoupe, ValidationDataView} from "../interfaces/IAccountLoupe.sol";
import {HookConfig, IModuleManager, ModuleEntity} from "../interfaces/IModuleManager.sol";
import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol";
import {ExecutionData, ValidationData, getAccountStorage, toHookConfig, toSelector} from "./AccountStorage.sol";
import {ExecutionData, ValidationData, getAccountStorage} from "./AccountStorage.sol";

abstract contract AccountLoupe is IAccountLoupe {
using EnumerableSet for EnumerableSet.Bytes32Set;
Expand Down
25 changes: 18 additions & 7 deletions src/account/ModuleManagerInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,15 @@ abstract contract ModuleManagerInternals is IModuleManager {
}
}

function _onUninstall(address module, bytes calldata data) internal {
function _onUninstall(address module, bytes calldata data) internal returns (bool onUninstallSuccess) {
onUninstallSuccess = true;
if (data.length > 0) {
IModule(module).onUninstall(data);
// Clear the module storage for the account.
// solhint-disable-next-line no-empty-blocks
try IModule(module).onUninstall(data) {}
catch {
onUninstallSuccess = false;
}
}
}

Expand All @@ -241,6 +247,7 @@ abstract contract ModuleManagerInternals is IModuleManager {
) internal {
ValidationData storage _validationData =
getAccountStorage().validationData[validationConfig.moduleEntity()];
ModuleEntity moduleEntity = validationConfig.moduleEntity();

for (uint256 i = 0; i < hooks.length; ++i) {
HookConfig hookConfig = HookConfig.wrap(bytes26(hooks[i][:26]));
Expand All @@ -255,7 +262,7 @@ abstract contract ModuleManagerInternals is IModuleManager {
}
} // Hook is an execution hook
else if (!_validationData.permissionHooks.add(toSetValue(hookConfig))) {
revert PermissionAlreadySet(validationConfig.moduleEntity(), hookConfig);
revert PermissionAlreadySet(moduleEntity, hookConfig);
}

_onInstall(hookConfig.module(), hookData);
Expand All @@ -264,14 +271,15 @@ abstract contract ModuleManagerInternals is IModuleManager {
for (uint256 i = 0; i < selectors.length; ++i) {
bytes4 selector = selectors[i];
if (!_validationData.selectors.add(toSetValue(selector))) {
revert ValidationAlreadySet(selector, validationConfig.moduleEntity());
revert ValidationAlreadySet(selector, moduleEntity);
}
}

_validationData.isGlobal = validationConfig.isGlobal();
_validationData.isSignatureValidation = validationConfig.isSignatureValidation();

_onInstall(validationConfig.module(), installData);
emit ValidationInstalled(moduleEntity);
}

function _uninstallValidation(
Expand All @@ -280,6 +288,7 @@ abstract contract ModuleManagerInternals is IModuleManager {
bytes[] calldata hookUninstallDatas
) internal {
ValidationData storage _validationData = getAccountStorage().validationData[validationFunction];
bool onUninstallSuccess = true;

_removeValidationFunction(validationFunction);

Expand All @@ -298,15 +307,15 @@ abstract contract ModuleManagerInternals is IModuleManager {
for (uint256 i = 0; i < _validationData.preValidationHooks.length; ++i) {
bytes calldata hookData = hookUninstallDatas[hookIndex];
(address hookModule,) = ModuleEntityLib.unpack(_validationData.preValidationHooks[i]);
_onUninstall(hookModule, hookData);
onUninstallSuccess = _onUninstall(hookModule, hookData);
hookIndex++;
}

for (uint256 i = 0; i < _validationData.permissionHooks.length(); ++i) {
bytes calldata hookData = hookUninstallDatas[hookIndex];
(address hookModule,) =
ModuleEntityLib.unpack(toModuleEntity(_validationData.permissionHooks.at(i)));
_onUninstall(hookModule, hookData);
onUninstallSuccess = _onUninstall(hookModule, hookData);
hookIndex++;
}
}
Expand All @@ -329,6 +338,8 @@ abstract contract ModuleManagerInternals is IModuleManager {
}

(address module,) = ModuleEntityLib.unpack(validationFunction);
_onUninstall(module, uninstallData);
onUninstallSuccess = _onUninstall(module, uninstallData);

emit ValidationUninstalled(validationFunction, onUninstallSuccess);
}
}
2 changes: 1 addition & 1 deletion src/interfaces/IAccountLoupe.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: CC0-1.0
pragma solidity ^0.8.25;

import {HookConfig, ModuleEntity} from "../interfaces/IModuleManager.sol";
import {ModuleEntity} from "../interfaces/IModuleManager.sol";

// Represents data associated with a specifc function selector.
struct ExecutionDataView {
Expand Down
3 changes: 2 additions & 1 deletion src/interfaces/IModuleManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ type HookConfig is bytes26;

interface IModuleManager {
event ModuleInstalled(address indexed module);

event ModuleUninstalled(address indexed module, bool indexed onUninstallSucceeded);
event ValidationInstalled(ModuleEntity indexed moduleEntity);
event ValidationUninstalled(ModuleEntity indexed moduleEntity, bool indexed onUninstallSucceeded);

/// @notice Install a module to the modular account.
/// @param module The module to install.
Expand Down
4 changes: 4 additions & 0 deletions test/account/DirectCallsFromModule.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ contract DirectCallsFromModuleTest is AccountTestBase {
DirectCallModule internal _module;
ModuleEntity internal _moduleEntity;

event ValidationUninstalled(ModuleEntity indexed moduleEntity, bool indexed onUninstallSucceeded);

function setUp() public {
_module = new DirectCallModule();
assertFalse(_module.preHookRan());
Expand Down Expand Up @@ -124,6 +126,8 @@ contract DirectCallsFromModuleTest is AccountTestBase {

function _uninstallExecution() internal {
vm.prank(address(entryPoint));
vm.expectEmit(true, true, true, true);
emit ValidationUninstalled(_moduleEntity, true);
account1.uninstallValidation(_moduleEntity, "", new bytes[](1));
}

Expand Down
6 changes: 6 additions & 0 deletions test/module/SingleSignerValidation.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/Messa
import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol";
import {ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol";
import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol";
import {ModuleEntity} from "../../src/interfaces/IModuleManager.sol";

import {ContractOwner} from "../mocks/ContractOwner.sol";
import {AccountTestBase} from "../utils/AccountTestBase.sol";
Expand All @@ -24,6 +25,8 @@ contract SingleSignerValidationTest is AccountTestBase {

ContractOwner public contractOwner;

event ValidationInstalled(ModuleEntity indexed moduleEntity);

function setUp() public {
ethRecipient = makeAddr("ethRecipient");
(owner2, owner2Key) = makeAddrAndKey("owner2");
Expand Down Expand Up @@ -79,6 +82,9 @@ contract SingleSignerValidationTest is AccountTestBase {
function test_runtime_with2SameValidationInstalled() public {
uint32 newEntityId = TEST_DEFAULT_VALIDATION_ENTITY_ID + 1;
vm.prank(address(entryPoint));

vm.expectEmit(true, true, true, true);
emit ValidationInstalled(ModuleEntityLib.pack(address(singleSignerValidation), newEntityId));
account.installValidation(
ValidationConfigLib.pack(address(singleSignerValidation), newEntityId, true, false),
new bytes4[](0),
Expand Down

0 comments on commit e1a0783

Please sign in to comment.