Skip to content

Commit

Permalink
fix: remove PluginIgnoredUninstallCallbackFailure event
Browse files Browse the repository at this point in the history
  • Loading branch information
jaypaik committed Jan 23, 2024
1 parent f7fb5ff commit f0c2b06
Show file tree
Hide file tree
Showing 7 changed files with 11 additions and 16 deletions.
11 changes: 5 additions & 6 deletions src/account/PluginManagerInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
emit PluginInstalled(plugin, manifestHash, dependencies);
}

function _uninstallPlugin(UninstallPluginArgs memory args, bytes calldata uninstallData) internal {
function _uninstallPlugin(UninstallPluginArgs memory args, bytes calldata pluginUninstallData) internal {
AccountStorage storage storage_ = _getAccountStorage();

// Check if the plugin exists.
Expand Down Expand Up @@ -628,18 +628,17 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
}

// Clear the plugin storage for the account.
bool callbacksSucceeded = true;
bool onUninstallSucceeded = true;
// solhint-disable-next-line no-empty-blocks
try IPlugin(args.plugin).onUninstall{gas: args.callbackGasLimit}(uninstallData) {}
try IPlugin(args.plugin).onUninstall{gas: args.callbackGasLimit}(pluginUninstallData) {}
catch (bytes memory revertReason) {
if (!args.forceUninstall) {
revert PluginUninstallCallbackFailed(args.plugin, revertReason);
}
callbacksSucceeded = false;
emit PluginIgnoredUninstallCallbackFailure(args.plugin);
onUninstallSucceeded = false;
}

emit PluginUninstalled(args.plugin, callbacksSucceeded);
emit PluginUninstalled(args.plugin, onUninstallSucceeded);
}

function _isValidPluginManifest(PluginManifest memory manifest, bytes32 manifestHash)
Expand Down
2 changes: 1 addition & 1 deletion src/account/UpgradeableModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ contract UpgradeableModularAccount is
// plugin manifest has changed. If empty, uses the default behavior of
// calling the plugin to get its current manifest.
bytes serializedManifest;
// If true, will complete the uninstall even if `onUninstall` callbacks revert. Available as an escape
// If true, will complete the uninstall even if the `onUninstall` callback reverts. Available as an escape
// hatch if a plugin is blocking uninstall.
bool forceUninstall;
// Maximum amount of gas allowed for each uninstall callback function
Expand Down
3 changes: 1 addition & 2 deletions src/interfaces/IPluginManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ type FunctionReference is bytes21;
/// @title Plugin Manager Interface
interface IPluginManager {
event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies);
event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded);
event PluginIgnoredUninstallCallbackFailure(address indexed plugin);
event PluginUninstalled(address indexed plugin, bool indexed onUninstallSucceeded);

/// @notice Install a plugin to the modular account.
/// @param plugin The plugin to install.
Expand Down
2 changes: 1 addition & 1 deletion test/account/AccountExecHooks.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ contract UpgradeableModularAccountExecHooksTest is Test {
PluginManifest public m2;

event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies);
event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded);
event PluginUninstalled(address indexed plugin, bool indexed onUninstallSucceeded);

function setUp() public {
entryPoint = IEntryPoint(address(new EntryPoint()));
Expand Down
2 changes: 1 addition & 1 deletion test/account/AccountPreValidationHooks.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ contract UpgradeableModularAccountPreValidationHooksTest is Test {
uint256 public constant VERIFICATION_GAS_LIMIT = 1000000;

event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies);
event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded);
event PluginUninstalled(address indexed plugin, bool indexed onUninstallSucceeded);

function setUp() public {
entryPoint = IEntryPoint(address(new EntryPoint()));
Expand Down
5 changes: 1 addition & 4 deletions test/account/UpgradeableModularAccountPluginManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ contract UpgradeableModularAccountPluginManagerTest is Test {
uint256 public constant VERIFICATION_GAS_LIMIT = 2000000;

event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies);
event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded);
event PluginIgnoredUninstallCallbackFailure(address indexed plugin);
event PluginUninstalled(address indexed plugin, bool indexed onUninstallSucceeded);
event ReceivedCall(bytes msgData, uint256 msgValue);

function setUp() public {
Expand Down Expand Up @@ -554,8 +553,6 @@ contract UpgradeableModularAccountPluginManagerTest is Test {
})
);
vm.expectEmit(true, true, true, true);
emit PluginIgnoredUninstallCallbackFailure(plugin);
vm.expectEmit(true, true, true, true);
emit PluginUninstalled(plugin, false);
IPluginManager(account2).uninstallPlugin{gas: 100_000}({
plugin: plugin,
Expand Down
2 changes: 1 addition & 1 deletion test/account/phases/AccountStatePhases.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ contract AccountStatePhasesTest is Test {

// Event re-declarations for vm.expectEmit
event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies);
event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded);
event PluginUninstalled(address indexed plugin, bool indexed onUninstallSucceeded);
event ReceivedCall(bytes msgData, uint256 msgValue);

// Empty arrays for convenience
Expand Down

0 comments on commit f0c2b06

Please sign in to comment.