diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index ffe71598b..1fd6e3c2d 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -619,8 +619,8 @@ contract UpgradeableModularAccount is && ( ( msg.sig != IPluginManager.installPlugin.selector - || msg.sig != UUPSUpgradeable.upgradeToAndCall.selector - ) && msg.sender != address(this) + && msg.sig != UUPSUpgradeable.upgradeToAndCall.selector + ) || msg.sender != address(this) ) ) { // Runtime calls cannot be made against functions with no diff --git a/test/account/UpgradeableModularAccountPluginManager.t.sol b/test/account/UpgradeableModularAccountPluginManager.t.sol index 70b65a9df..018743ef5 100644 --- a/test/account/UpgradeableModularAccountPluginManager.t.sol +++ b/test/account/UpgradeableModularAccountPluginManager.t.sol @@ -858,6 +858,41 @@ contract UpgradeableModularAccountPluginManagerTest is Test { IStandardExecutor(account2).executeBatch(calls); } + function test_uninstallAndInstallInBatch_failwithOtherCalls() external { + // Check that we can uninstall the `MultiOwnerPlugin`, leaving no + // validator on `installPlugin`, and then install a different plugin + // immediately after as part of the same batch execution. This is a + // special case: normally an execution function with no runtime + // validator cannot be runtime-called. + + // Here we test only the above is allowed, any other self-call is blocked + + vm.startPrank(owner2); + + ComprehensivePlugin plugin = new ComprehensivePlugin(); + bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); + + Call[] memory calls = new Call[](2); + calls[0] = Call({ + target: address(account2), + value: 0, + data: abi.encodeCall(IPluginManager.uninstallPlugin, (address(multiOwnerPlugin), "", "", new bytes[](0))) + }); + calls[1] = Call({ + target: address(account2), + value: 0, + data: abi.encodeCall(UpgradeableModularAccount.execute, (ethRecipient, 1 wei, "")) + }); + + vm.expectRevert( + abi.encodeWithSelector( + UpgradeableModularAccount.RuntimeValidationFunctionMissing.selector, + UpgradeableModularAccount.execute.selector + ) + ); + IStandardExecutor(account2).executeBatch(calls); + } + function test_noNonSelfInstallAfterUninstall() external { // A companion to the previous test, ensuring that `installPlugin` can't // be called directly (e.g. not via `execute` or `executeBatch`) if it