Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions src/account/AccountExecutor.sol
Original file line number Diff line number Diff line change
@@ -1,25 +1,12 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.25;

import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
import {IPlugin} from "../interfaces/IPlugin.sol";

abstract contract AccountExecutor {
error PluginExecutionDenied(address plugin);

/// @dev If the target is a plugin (as determined by its support for the IPlugin interface), revert.
/// This prevents the modular account from calling plugins (both installed and uninstalled) outside
/// of the normal flow (via execution functions installed on the account), which could lead to data
/// inconsistencies and unexpected behavior.
/// @param target The address of the contract to call.
/// @param value The value to send with the call.
/// @param data The call data.
/// @return result The return data of the call, or the error message from the call if call reverts.
function _exec(address target, uint256 value, bytes memory data) internal returns (bytes memory result) {
if (ERC165Checker.supportsInterface(target, type(IPlugin).interfaceId)) {
revert PluginExecutionDenied(target);
}

bool success;
(success, result) = target.call{value: value}(data);

Expand Down
28 changes: 5 additions & 23 deletions src/plugins/owner/SingleOwnerPlugin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -152,51 +152,33 @@ contract SingleOwnerPlugin is ISingleOwnerPlugin, BasePlugin {
function pluginManifest() external pure override returns (PluginManifest memory) {
PluginManifest memory manifest;

manifest.executionFunctions = new bytes4[](2);
manifest.executionFunctions[0] = this.transferOwnership.selector;
manifest.executionFunctions[1] = this.owner.selector;

ManifestFunction memory ownerValidationFunction = ManifestFunction({
functionType: ManifestAssociatedFunctionType.SELF,
functionId: uint8(FunctionId.VALIDATION_OWNER_OR_SELF),
dependencyIndex: 0 // Unused.
});
manifest.validationFunctions = new ManifestAssociatedFunction[](7);
manifest.validationFunctions = new ManifestAssociatedFunction[](5);
manifest.validationFunctions[0] = ManifestAssociatedFunction({
executionSelector: this.transferOwnership.selector,
associatedFunction: ownerValidationFunction
});
manifest.validationFunctions[1] = ManifestAssociatedFunction({
executionSelector: IStandardExecutor.execute.selector,
associatedFunction: ownerValidationFunction
});
manifest.validationFunctions[2] = ManifestAssociatedFunction({
manifest.validationFunctions[1] = ManifestAssociatedFunction({
executionSelector: IStandardExecutor.executeBatch.selector,
associatedFunction: ownerValidationFunction
});
manifest.validationFunctions[3] = ManifestAssociatedFunction({
manifest.validationFunctions[2] = ManifestAssociatedFunction({
executionSelector: IPluginManager.installPlugin.selector,
associatedFunction: ownerValidationFunction
});
manifest.validationFunctions[4] = ManifestAssociatedFunction({
manifest.validationFunctions[3] = ManifestAssociatedFunction({
executionSelector: IPluginManager.uninstallPlugin.selector,
associatedFunction: ownerValidationFunction
});
manifest.validationFunctions[5] = ManifestAssociatedFunction({
manifest.validationFunctions[4] = ManifestAssociatedFunction({
executionSelector: UUPSUpgradeable.upgradeToAndCall.selector,
associatedFunction: ownerValidationFunction
});

ManifestFunction memory alwaysAllowRuntime = ManifestFunction({
functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW,
functionId: 0, // Unused.
dependencyIndex: 0 // Unused.
});
manifest.validationFunctions[6] = ManifestAssociatedFunction({
executionSelector: this.owner.selector,
associatedFunction: alwaysAllowRuntime
});

manifest.signatureValidationFunctions = new uint8[](1);
manifest.signatureValidationFunctions[0] = uint8(FunctionId.SIG_VALIDATION);

Expand Down
10 changes: 3 additions & 7 deletions test/account/AccountLoupe.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,16 @@ contract AccountLoupeTest is AccountTestBase {
}

function test_pluginLoupe_getExecutionFunctionConfig_plugin() public {
bytes4[] memory selectorsToCheck = new bytes4[](2);
address[] memory expectedPluginAddress = new address[](2);
FunctionReference[] memory expectedValidations = new FunctionReference[](2);
bytes4[] memory selectorsToCheck = new bytes4[](1);
address[] memory expectedPluginAddress = new address[](1);
FunctionReference[] memory expectedValidations = new FunctionReference[](1);

selectorsToCheck[0] = comprehensivePlugin.foo.selector;
expectedPluginAddress[0] = address(comprehensivePlugin);
expectedValidations[0] = FunctionReferenceLib.pack(
address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.VALIDATION)
);

selectorsToCheck[1] = singleOwnerPlugin.transferOwnership.selector;
expectedPluginAddress[1] = address(singleOwnerPlugin);
expectedValidations[1] = ownerValidation;

for (uint256 i = 0; i < selectorsToCheck.length; i++) {
IAccountLoupe.ExecutionFunctionConfig memory config =
account1.getExecutionFunctionConfig(selectorsToCheck[i]);
Expand Down
13 changes: 9 additions & 4 deletions test/account/UpgradeableModularAccount.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ contract UpgradeableModularAccountTest is AccountTestBase {
sender: address(account2),
nonce: 0,
initCode: abi.encodePacked(address(factory), abi.encodeCall(factory.createAccount, (owner2, 0))),
callData: abi.encodeCall(SingleOwnerPlugin.transferOwnership, (owner2)),
callData: abi.encodeCall(
UpgradeableModularAccount.execute,
(address(singleOwnerPlugin), 0, abi.encodeCall(SingleOwnerPlugin.transferOwnership, (owner2)))
),
accountGasLimits: _encodeGas(VERIFICATION_GAS_LIMIT, CALL_GAS_LIMIT),
preVerificationGas: 0,
gasFees: _encodeGas(1, 2),
Expand Down Expand Up @@ -422,12 +425,14 @@ contract UpgradeableModularAccountTest is AccountTestBase {
}

function test_transferOwnership() public {
assertEq(SingleOwnerPlugin(address(account1)).owner(), owner1);
assertEq(singleOwnerPlugin.ownerOf(address(account1)), owner1);

vm.prank(owner1);
SingleOwnerPlugin(address(account1)).transferOwnership(owner2);
account1.execute(
address(singleOwnerPlugin), 0, abi.encodeCall(SingleOwnerPlugin.transferOwnership, (owner2))
);

assertEq(SingleOwnerPlugin(address(account1)).owner(), owner2);
assertEq(singleOwnerPlugin.ownerOf(address(account1)), owner2);
}

function test_isValidSignature() public {
Expand Down
4 changes: 3 additions & 1 deletion test/utils/AccountTestBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ abstract contract AccountTestBase is OptimizedTest {
function _transferOwnershipToTest() internal {
// Transfer ownership to test contract for easier invocation.
vm.prank(owner1);
SingleOwnerPlugin(address(account1)).transferOwnership(address(this));
account1.execute(
address(singleOwnerPlugin), 0, abi.encodeCall(SingleOwnerPlugin.transferOwnership, (address(this)))
);
}

// helper function to compress 2 gas values into a single bytes32
Expand Down