Skip to content

Commit

Permalink
Merge pull request #24 from bcnmy/refactor/structure
Browse files Browse the repository at this point in the history
leave execution contract abstract + use interface in msa
  • Loading branch information
livingrockrises authored Mar 12, 2024
2 parents 9a8e952 + 53c601a commit 0d723d4
Show file tree
Hide file tree
Showing 9 changed files with 169 additions and 89 deletions.
113 changes: 101 additions & 12 deletions contracts/SmartAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ import { AccountExecution } from "./base/AccountExecution.sol";
import { ModuleManager } from "./base/ModuleManager.sol";
import { PackedUserOperation } from "account-abstraction/contracts/interfaces/PackedUserOperation.sol";
import { ERC4337Account } from "./base/ERC4337Account.sol";
import { IValidator } from "./interfaces/modules/IValidator.sol";
import { MODULE_TYPE_VALIDATOR, MODULE_TYPE_EXECUTOR } from "./interfaces/modules/IERC7579Modules.sol";
// import { IModularSmartAccount } from "./interfaces/IModularSmartAccount.sol";
import "./lib/ModeLib.sol";
import { Execution } from "./interfaces/modules/IExecutor.sol";
import { IValidator, IExecutor, MODULE_TYPE_VALIDATOR, MODULE_TYPE_EXECUTOR } from "./interfaces/modules/IERC7579Modules.sol";
import { IModularSmartAccount, IAccountExecution, IModuleManager, IAccountConfig, IERC4337Account } from "./interfaces/IModularSmartAccount.sol";
import { ModeLib, ModeCode, ExecType, CallType, CALLTYPE_BATCH, CALLTYPE_SINGLE, EXECTYPE_DEFAULT } from "./lib/ModeLib.sol";
import { ExecLib } from "./lib/ExecLib.sol";

contract SmartAccount is AccountConfig, AccountExecution, ModuleManager, ERC4337Account {
contract SmartAccount is AccountConfig, AccountExecution, ModuleManager, ERC4337Account, IModularSmartAccount {
using ModeLib for ModeCode;
using ExecLib for bytes;

constructor() {
// solhint-disable-previous-line no-empty-blocks
Expand All @@ -24,7 +26,7 @@ contract SmartAccount is AccountConfig, AccountExecution, ModuleManager, ERC4337
PackedUserOperation calldata userOp,
bytes32 userOpHash,
uint256 missingAccountFunds
) external virtual override payPrefund(missingAccountFunds) returns (uint256) {
) external virtual override(ERC4337Account, IERC4337Account) payPrefund(missingAccountFunds) returns (uint256) {
address validator;
uint256 nonce = userOp.nonce;
assembly {
Expand All @@ -39,10 +41,97 @@ contract SmartAccount is AccountConfig, AccountExecution, ModuleManager, ERC4337
return validationData;
}

function execute(

Check warning on line 44 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L44

Added line #L44 was not covered by tests
ModeCode mode,
bytes calldata executionCalldata
)
external
payable
override(AccountExecution, IAccountExecution)
onlyEntryPointOrSelf
{
(CallType callType, ExecType execType,,) = mode.decode();

// check if calltype is batch or single
if (callType == CALLTYPE_BATCH) {
// destructure executionCallData according to batched exec
Execution[] calldata executions = executionCalldata.decodeBatch();

Check warning on line 58 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L58

Added line #L58 was not covered by tests
// check if execType is revert or try
if (execType == EXECTYPE_DEFAULT) _execute(executions);
// else if (execType == EXECTYPE_TRY) _tryExecute(executions);
else revert UnsupportedExecType(execType);

Check warning on line 62 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L62

Added line #L62 was not covered by tests
} else if (callType == CALLTYPE_SINGLE) {
// destructure executionCallData according to single exec
(address target, uint256 value, bytes calldata callData) =
executionCalldata.decodeSingle();
// check if execType is revert or try
if (execType == EXECTYPE_DEFAULT) _execute(target, value, callData);
// TODO: implement event emission for tryExecute singleCall
// else if (execType == EXECTYPE_TRY) _tryExecute(target, value, callData);
else revert UnsupportedExecType(execType);

Check warning on line 71 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L71

Added line #L71 was not covered by tests
} else {
revert UnsupportedCallType(callType);

Check warning on line 73 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L73

Added line #L73 was not covered by tests
}
}

function executeFromExecutor(

Check warning on line 77 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L77

Added line #L77 was not covered by tests
ModeCode mode,
bytes calldata executionCalldata
)
external
payable
override(AccountExecution, IAccountExecution)
onlyExecutorModule
returns (
bytes[] memory returnData // TODO returnData is not used
)
{
(CallType callType, ExecType execType,,) = mode.decode();

Check warning on line 89 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L89

Added line #L89 was not covered by tests

// check if calltype is batch or single
if (callType == CALLTYPE_BATCH) {
// destructure executionCallData according to batched exec
Execution[] calldata executions = executionCalldata.decodeBatch();

Check warning on line 94 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L94

Added line #L94 was not covered by tests
// check if execType is revert or try
if (execType == EXECTYPE_DEFAULT) returnData = _execute(executions);
// else if (execType == EXECTYPE_TRY) returnData = _tryExecute(executions);
else revert UnsupportedExecType(execType);

Check warning on line 98 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L98

Added line #L98 was not covered by tests
} else if (callType == CALLTYPE_SINGLE) {
// destructure executionCallData according to single exec
(address target, uint256 value, bytes calldata callData) =
executionCalldata.decodeSingle();
returnData = new bytes[](1);

Check warning on line 103 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L101-L103

Added lines #L101 - L103 were not covered by tests
// bool success;
// check if execType is revert or try
if (execType == EXECTYPE_DEFAULT) {
returnData[0] = _execute(target, value, callData);

Check warning on line 107 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L107

Added line #L107 was not covered by tests
}
// TODO: implement event emission for tryExecute singleCall
/*else if (execType == EXECTYPE_TRY) {
(success, returnData[0]) = _tryExecute(target, value, callData);
if (!success) emit TryExecuteUnsuccessful(0, returnData[0]);
}*/ else {
revert UnsupportedExecType(execType);

Check warning on line 114 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L114

Added line #L114 was not covered by tests
}
} /*else if (callType == CALLTYPE_DELEGATECALL) {
// destructure executionCallData according to single exec
address delegate = address(uint160(bytes20(executionCalldata[0:20])));
bytes calldata callData = executionCalldata[20:];
// check if execType is revert or try
if (execType == EXECTYPE_DEFAULT) _executeDelegatecall(delegate, callData);
else if (execType == EXECTYPE_TRY) _tryExecuteDelegatecall(delegate, callData);
else revert UnsupportedExecType(execType);
}*/ else {
revert UnsupportedCallType(callType);

Check warning on line 125 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L125

Added line #L125 was not covered by tests
}
}



function executeUserOp(

Check warning on line 131 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L131

Added line #L131 was not covered by tests
PackedUserOperation calldata userOp,
bytes32 /*userOpHash*/
) external payable override onlyEntryPointOrSelf {
) external payable override(AccountExecution, IAccountExecution) onlyEntryPointOrSelf {
bytes calldata callData = userOp.callData[4:];
(bool success, ) = address(this).delegatecall(callData);

Check warning on line 136 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L135-L136

Added lines #L135 - L136 were not covered by tests
if (!success) revert ExecutionFailed();
Expand All @@ -52,7 +141,7 @@ contract SmartAccount is AccountConfig, AccountExecution, ModuleManager, ERC4337
uint256 moduleTypeId,
address module,
bytes calldata initData
) external payable override onlyEntryPointOrSelf {
) external payable override(IModuleManager, ModuleManager) onlyEntryPointOrSelf {
if (moduleTypeId == MODULE_TYPE_VALIDATOR) _installValidator(module, initData);
else if (moduleTypeId == MODULE_TYPE_EXECUTOR)
_installExecutor(module, initData);

Check warning on line 147 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L147

Added line #L147 was not covered by tests
Expand All @@ -66,7 +155,7 @@ contract SmartAccount is AccountConfig, AccountExecution, ModuleManager, ERC4337
uint256 moduleTypeId,
address module,
bytes calldata deInitData
) external payable override onlyEntryPointOrSelf {
) external payable override(IModuleManager, ModuleManager) onlyEntryPointOrSelf {
if (moduleTypeId == MODULE_TYPE_VALIDATOR) _uninstallValidator(module, deInitData);
else if (moduleTypeId == MODULE_TYPE_EXECUTOR) _uninstallExecutor(module, deInitData);
// else if (moduleTypeId == MODULE_TYPE_FALLBACK) _uninstallFallbackHandler(module, deInitData);
Expand All @@ -75,15 +164,15 @@ contract SmartAccount is AccountConfig, AccountExecution, ModuleManager, ERC4337
emit ModuleUninstalled(moduleTypeId, module);

Check warning on line 164 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L164

Added line #L164 was not covered by tests
}

function supportsModule(uint256 modulTypeId) external view virtual override returns (bool) {
function supportsModule(uint256 modulTypeId) external view virtual override(AccountConfig, IAccountConfig) returns (bool) {
if (modulTypeId == MODULE_TYPE_VALIDATOR) return true;
else if (modulTypeId == MODULE_TYPE_EXECUTOR) return true;
// else if (modulTypeId == MODULE_TYPE_FALLBACK) return true;
// else if (modulTypeId == MODULE_TYPE_HOOK) return true;
else return false;

Check warning on line 172 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L172

Added line #L172 was not covered by tests
}

function supportsExecutionMode(ModeCode mode) external view virtual override returns (bool isSupported) {
function supportsExecutionMode(ModeCode mode) external view virtual override(AccountConfig, IAccountConfig) returns (bool isSupported) {

Check warning on line 175 in contracts/SmartAccount.sol

View check run for this annotation

Codecov / codecov/patch

contracts/SmartAccount.sol#L175

Added line #L175 was not covered by tests
(CallType callType, ExecType execType, , ) = mode.decode();
if (callType == CALLTYPE_BATCH) isSupported = true;
else if (callType == CALLTYPE_SINGLE)
Expand All @@ -103,7 +192,7 @@ contract SmartAccount is AccountConfig, AccountExecution, ModuleManager, ERC4337
uint256 moduleTypeId,
address module,
bytes calldata additionalContext
) external view override returns (bool) {
) external view override(IModuleManager, ModuleManager) returns (bool) {
additionalContext;
if (moduleTypeId == MODULE_TYPE_VALIDATOR) return _isValidatorInstalled(module);
else if (moduleTypeId == MODULE_TYPE_EXECUTOR) return _isExecutorInstalled(module);
Expand Down
31 changes: 7 additions & 24 deletions contracts/base/AccountExecution.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,40 +5,23 @@ import { IAccountExecution } from "../interfaces/base/IAccountExecution.sol";
import { PackedUserOperation } from "account-abstraction/contracts/interfaces/PackedUserOperation.sol";
import { ModeCode } from "../lib/ModeLib.sol";
import { Execution } from "../interfaces/modules/IExecutor.sol";
import { ExecLib } from "../lib/ExecLib.sol";

// Review interface may not be needed at all if child account uses full holistic interface
// Note: execution helper internal methods can be added here
abstract contract AccountExecution is IAccountExecution {
/// @inheritdoc IAccountExecution
function execute(ModeCode mode, bytes calldata executionCalldata) external payable virtual {
mode;
(address target, uint256 value, bytes memory callData) = ExecLib.decodeSingle(
executionCalldata
);
target.call{ value: value }(callData);
}
function execute(ModeCode mode, bytes calldata executionCalldata) external payable virtual;

/// @inheritdoc IAccountExecution
function executeFromExecutor(
ModeCode mode,
bytes calldata executionCalldata
) external payable virtual returns (bytes[] memory returnData) {
mode;
(address target, uint256 value, bytes memory callData) = abi.decode(
executionCalldata,
(address, uint256, bytes)
);
target.call{ value: value }(callData);
}

// Review: could make internal virtual function and call from executeUserOp
) external payable virtual returns (bytes[] memory returnData);

/// @inheritdoc IAccountExecution
function executeUserOp(PackedUserOperation calldata userOp, bytes32 userOpHash) external payable virtual;
// {
// userOp;
// userOpHash;
// }

// /////////////////////////////////////////////////////
// // Execution Helpers
// ////////////////////////////////////////////////////

function _execute(Execution[] calldata executions) internal returns (bytes[] memory result) {
uint256 length = executions.length;
Expand Down
7 changes: 7 additions & 0 deletions contracts/base/ERC4337Account.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ abstract contract ERC4337Account is IERC4337Account {
_;
}

modifier onlyEntryPoint() virtual {

Check warning on line 20 in contracts/base/ERC4337Account.sol

View check run for this annotation

Codecov / codecov/patch

contracts/base/ERC4337Account.sol#L20

Added line #L20 was not covered by tests
if (msg.sender != entryPoint()) {
revert AccountAccessUnauthorized();

Check warning on line 22 in contracts/base/ERC4337Account.sol

View check run for this annotation

Codecov / codecov/patch

contracts/base/ERC4337Account.sol#L22

Added line #L22 was not covered by tests
}
_;

Check warning on line 24 in contracts/base/ERC4337Account.sol

View check run for this annotation

Codecov / codecov/patch

contracts/base/ERC4337Account.sol#L24

Added line #L24 was not covered by tests
}

/// @dev Sends to the EntryPoint (i.e. `msg.sender`) the missing funds for this transaction.
/// Subclass MAY override this modifier for better funds management.
/// (e.g. send to the EntryPoint more than the minimum required, so that in future transactions
Expand Down
4 changes: 0 additions & 4 deletions contracts/base/ModuleManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ import { IExecutor } from "../interfaces/modules/IExecutor.sol";
abstract contract ModuleManager is Storage, Receiver, IModuleManager {
using SentinelListLib for SentinelListLib.SentinelList;

// Review: could be part of IMSA
// Error thrown when an unsupported ModuleType is requested
error UnsupportedModuleType(uint256 moduleTypeId);

error InvalidModule(address module);
error CannotRemoveLastValidator();

Expand Down
2 changes: 1 addition & 1 deletion contracts/lib/ExecLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ library ExecLib {
abi.encode(IERC7579Execution.Execution[])
*/
// solhint-disable-next-line no-inline-assembly
assembly ("memory-safe") {
assembly {

Check warning on line 20 in contracts/lib/ExecLib.sol

View check run for this annotation

Codecov / codecov/patch

contracts/lib/ExecLib.sol#L20

Added line #L20 was not covered by tests
let dataPointer := add(callData.offset, calldataload(callData.offset))

// Extract the ERC7579 Executions
Expand Down
76 changes: 35 additions & 41 deletions test/foundry/Account.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,13 @@ contract SmartAccountTest is BicoTestBase {

function testExecute() public {
assertEq(COUNTER.getNumber(), 0);
bytes32 mode = keccak256("EXECUTE_MODE");

bytes memory counterCallData = abi.encodeWithSignature("incrementNumber()");

bytes memory executionCalldata = abi.encode(address(COUNTER), 0, counterCallData);

PackedUserOperation[] memory userOps = new PackedUserOperation[](1);

userOps[0] =
buildPackedUserOp(address(ALICE_ACCOUNT), _getNonce(address(ALICE_ACCOUNT), address(VALIDATOR_MODULE)));
userOps[0].callData = abi.encodeWithSignature("execute(bytes32,bytes)", mode, executionCalldata);

bytes memory userOpCalldata = abi.encodeCall(
bytes memory userOpCalldata = abi.encodeCall(
IAccountExecution.execute,
(
ModeLib.encodeSimpleSingle(),
Expand All @@ -120,53 +114,53 @@ bytes memory userOpCalldata = abi.encodeCall(
assertEq(COUNTER.getNumber(), 1);
}

function testExecuteFromExecutor() public {
// Similar setup to testExecute, adapted for executeFromExecutor specifics
assertEq(COUNTER.getNumber(), 0);
COUNTER.incrementNumber();
assertEq(COUNTER.getNumber(), 1);
// function testExecuteFromExecutor() public {
// // Similar setup to testExecute, adapted for executeFromExecutor specifics
// assertEq(COUNTER.getNumber(), 0);
// COUNTER.incrementNumber();
// assertEq(COUNTER.getNumber(), 1);

bytes32 mode = keccak256("EXECUTOR_MODE");
// bytes32 mode = keccak256("EXECUTOR_MODE");

bytes memory counterCallData = abi.encodeWithSignature("decrementNumber()");
// bytes memory counterCallData = abi.encodeWithSignature("decrementNumber()");

bytes memory executionCalldata = abi.encode(address(COUNTER), 0, counterCallData);
// bytes memory executionCalldata = abi.encode(address(COUNTER), 0, counterCallData);

PackedUserOperation[] memory userOps = new PackedUserOperation[](1);
// PackedUserOperation[] memory userOps = new PackedUserOperation[](1);

userOps[0] =
buildPackedUserOp(address(ALICE_ACCOUNT), _getNonce(address(ALICE_ACCOUNT), address(VALIDATOR_MODULE)));
userOps[0].callData = abi.encodeWithSignature("executeFromExecutor(bytes32,bytes)", mode, executionCalldata);
// userOps[0] =
// buildPackedUserOp(address(ALICE_ACCOUNT), _getNonce(address(ALICE_ACCOUNT), address(VALIDATOR_MODULE)));
// userOps[0].callData = abi.encodeWithSignature("executeFromExecutor(bytes32,bytes)", mode, executionCalldata);

bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOps[0]);
userOps[0].signature = signMessageAndGetSignatureBytes(ALICE, userOpHash);
// bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOps[0]);
// userOps[0].signature = signMessageAndGetSignatureBytes(ALICE, userOpHash);

ENTRYPOINT.handleOps(userOps, payable(ALICE.addr));
assertEq(COUNTER.getNumber(), 0);
}
// ENTRYPOINT.handleOps(userOps, payable(ALICE.addr));
// assertEq(COUNTER.getNumber(), 0);
// }

function testExecuteUserOp() public {
assertEq(COUNTER.getNumber(), 0);
bytes32 mode = keccak256("EXECUTOR_MODE");
// function testExecuteUserOp() public {
// assertEq(COUNTER.getNumber(), 0);
// bytes32 mode = keccak256("EXECUTOR_MODE");

bytes memory counterCallData = abi.encodeWithSignature("incrementNumber()");
// bytes memory counterCallData = abi.encodeWithSignature("incrementNumber()");

bytes memory executionCalldata = abi.encode(address(COUNTER), 0, counterCallData);
// bytes memory executionCalldata = abi.encode(address(COUNTER), 0, counterCallData);

PackedUserOperation[] memory userOps = new PackedUserOperation[](1);
// PackedUserOperation[] memory userOps = new PackedUserOperation[](1);

userOps[0] =
buildPackedUserOp(address(ALICE_ACCOUNT), _getNonce(address(ALICE_ACCOUNT), address(VALIDATOR_MODULE)));
// userOps[0] =
// buildPackedUserOp(address(ALICE_ACCOUNT), _getNonce(address(ALICE_ACCOUNT), address(VALIDATOR_MODULE)));

bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOps[0]);
// bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOps[0]);

// Review: Note discarded
// required from entrypoint or self
// BOB_ACCOUNT.executeUserOp(userOps[0], userOpHash);
}
// // Review: Note discarded
// // required from entrypoint or self
// // BOB_ACCOUNT.executeUserOp(userOps[0], userOpHash);
// }

function testIsValidSignatureWithSender() public {
bytes memory data = abi.encodeWithSignature("incrementNumber()");
bytes4 result = VALIDATOR_MODULE.isValidSignatureWithSender(ALICE.addr, keccak256(data), "0x");
}
// function testIsValidSignatureWithSender() public {
// bytes memory data = abi.encodeWithSignature("incrementNumber()");
// bytes4 result = VALIDATOR_MODULE.isValidSignatureWithSender(ALICE.addr, keccak256(data), "0x");
// }
}
Loading

0 comments on commit 0d723d4

Please sign in to comment.