update IPluginManager

erc6900 · Nov 28, 2023 · bc2ce9d · bc2ce9d
1 parent 450cd64
commit bc2ce9d
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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),