diff --git a/src/account/BaseModularAccount.sol b/src/account/BaseModularAccount.sol index b0b4f512..9f1bfb5c 100644 --- a/src/account/BaseModularAccount.sol +++ b/src/account/BaseModularAccount.sol @@ -615,7 +615,7 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 revert PluginInstallCallbackFailed(plugin, revertReason); } - emit PluginInstalled(plugin, manifestHash); + emit PluginInstalled(plugin, manifestHash, dependencies, injectedHooks); } function _uninstallPlugin( @@ -893,7 +893,7 @@ abstract contract BaseModularAccount is IPluginManager, AccountExecutor, IERC165 onUninstallSuccess = false; } - emit PluginUninstalled(plugin, manifestHash, onUninstallSuccess); + emit PluginUninstalled(plugin, onUninstallSuccess); } function _toSetValue(FunctionReference functionReference) internal pure returns (bytes32) { 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/test/account/UpgradeableModularAccount.t.sol b/test/account/UpgradeableModularAccount.t.sol index 0da80e81..77cdfa44 100644 --- a/test/account/UpgradeableModularAccount.t.sol +++ b/test/account/UpgradeableModularAccount.t.sol @@ -53,8 +53,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 { @@ -279,7 +284,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, @@ -389,7 +399,7 @@ 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: "", @@ -416,7 +426,7 @@ 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, @@ -493,7 +503,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 +557,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 ); @@ -591,7 +601,7 @@ contract UpgradeableModularAccountTest is Test { (, MockPlugin newPlugin, bytes32 manifestHash) = _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),