Skip to content
Open
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
26 changes: 13 additions & 13 deletions snapshots/BenchmarkTest.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
"testERC20Transfer_ERC4337MinimalAccount": "171906",
"testERC20Transfer_ERC4337MinimalAccount_AppSponsor": "168893",
"testERC20Transfer_ERC4337MinimalAccount_ERC20SelfPay": "217488",
"testERC20Transfer_IthacaAccount": "129892",
"testERC20Transfer_IthacaAccountWithSpendLimits": "190799",
"testERC20Transfer_IthacaAccount_AppSponsor": "141312",
"testERC20Transfer_IthacaAccount_ERC20SelfPay": "147545",
"testERC20Transfer_IthacaAccount": "129938",
"testERC20Transfer_IthacaAccountWithSpendLimits": "190845",
"testERC20Transfer_IthacaAccount_AppSponsor": "141358",
"testERC20Transfer_IthacaAccount_ERC20SelfPay": "147591",
"testERC20Transfer_Safe4337": "197515",
"testERC20Transfer_Safe4337_AppSponsor": "191679",
"testERC20Transfer_Safe4337_ERC20SelfPay": "238658",
Expand All @@ -28,24 +28,24 @@
"testERC20Transfer_ZerodevKernel_ERC20SelfPay": "252683",
"testERC20Transfer_batch100_AlchemyModularAccount": "10104466",
"testERC20Transfer_batch100_AlchemyModularAccount_ERC20SelfPay": "11635798",
"testERC20Transfer_batch100_IthacaAccount": "7693196",
"testERC20Transfer_batch100_IthacaAccount_AppSponsor": "8392216",
"testERC20Transfer_batch100_IthacaAccount_ERC20SelfPay": "7537848",
"testERC20Transfer_batch100_IthacaAccount": "7697796",
"testERC20Transfer_batch100_IthacaAccount_AppSponsor": "8396816",
"testERC20Transfer_batch100_IthacaAccount_ERC20SelfPay": "7542448",
"testERC20Transfer_batch100_ZerodevKernel": "12626718",
"testERC20Transfer_batch100_ZerodevKernel_ERC20SelfPay": "14176437",
"testNativeTransfer_AlchemyModularAccount": "180829",
"testNativeTransfer_CoinbaseSmartWallet": "178916",
"testNativeTransfer_IthacaAccount": "131294",
"testNativeTransfer_IthacaAccount_AppSponsor": "142757",
"testNativeTransfer_IthacaAccount_ERC20SelfPay": "156247",
"testNativeTransfer_IthacaAccount": "131340",
"testNativeTransfer_IthacaAccount_AppSponsor": "142803",
"testNativeTransfer_IthacaAccount_ERC20SelfPay": "156293",
"testNativeTransfer_Safe4337": "198595",
"testNativeTransfer_ZerodevKernel": "208635",
"testUniswapV2Swap_AlchemyModularAccount": "238767",
"testUniswapV2Swap_CoinbaseSmartWallet": "237571",
"testUniswapV2Swap_ERC4337MinimalAccount": "231254",
"testUniswapV2Swap_IthacaAccount": "189178",
"testUniswapV2Swap_IthacaAccount_AppSponsor": "200586",
"testUniswapV2Swap_IthacaAccount_ERC20SelfPay": "211643",
"testUniswapV2Swap_IthacaAccount": "189224",
"testUniswapV2Swap_IthacaAccount_AppSponsor": "200632",
"testUniswapV2Swap_IthacaAccount_ERC20SelfPay": "211689",
"testUniswapV2Swap_Safe4337": "257453",
"testUniswapV2Swap_ZerodevKernel": "266487"
}
2 changes: 1 addition & 1 deletion src/GuardedExecutor.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.23;

import {ERC7821} from "solady/accounts/ERC7821.sol";
import {LibSort} from "solady/utils/LibSort.sol";
import {LibBytes} from "solady/utils/LibBytes.sol";
import {LibZip} from "solady/utils/LibZip.sol";
Expand All @@ -13,6 +12,7 @@ import {SafeTransferLib} from "solady/utils/SafeTransferLib.sol";
import {FixedPointMathLib as Math} from "solady/utils/FixedPointMathLib.sol";
import {DateTimeLib} from "solady/utils/DateTimeLib.sol";
import {ICallChecker} from "./interfaces/ICallChecker.sol";
import {ERC7821Ithaca as ERC7821} from "./libraries/ERC7821Ithaca.sol";

/// @title GuardedExecutor
/// @notice Mixin for spend limits and calldata execution guards.
Expand Down
77 changes: 75 additions & 2 deletions src/IthacaAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,38 @@ contract IthacaAccount is IIthacaAccount, EIP712, GuardedExecutor {
return isMultichain ? _hashTypedDataSansChainId(structHash) : _hashTypedData(structHash);
}

function computeDigest(CallSansTo[] calldata calls, address to, uint256 nonce)
public
view
virtual
returns (bytes32 result)
{
// If to is 0, it will be replaced with address(this)
assembly ("memory-safe") {
let t := shr(96, shl(96, to))
to := or(mul(address(), iszero(t)), t)
}

bytes32[] memory a = EfficientHashLib.malloc(calls.length);
for (uint256 i; i < calls.length; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that someone can take a valid CallSansTo execution bundle and submit with the Call mode

Would prefer if we hash this a little differently to prevent that, its good design to only allow 1 possible way things can be executed with the same sig

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good point, but does it matter if they are used interchangeably?
the nonce already does replay protection.

But yes maybe we should hash it separately, just to prevent footguns in the future

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing obvious, but yep best to not have hanging things like this, there are attacks that could happen when you combine a bunch of hanging things

(uint256 value, bytes calldata data) = _get(calls, i);
a.set(
i,
EfficientHashLib.hash(
CALL_TYPEHASH,
bytes32(uint256(uint160(to))),
bytes32(value),
EfficientHashLib.hashCalldata(data)
)
);
}
bool isMultichain = nonce >> 240 == MULTICHAIN_NONCE_PREFIX;
bytes32 structHash = EfficientHashLib.hash(
uint256(EXECUTE_TYPEHASH), LibBit.toUint(isMultichain), uint256(a.hash()), nonce
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isMultichain is redundant since we already get it from domain

);
return isMultichain ? _hashTypedDataSansChainId(structHash) : _hashTypedData(structHash);
}

/// @dev Returns if the signature is valid, along with its `keyHash`.
/// The `signature` is a wrapped signature, given by
/// `abi.encodePacked(bytes(innerSignature), bytes32(keyHash), bool(prehash))`.
Expand Down Expand Up @@ -675,6 +707,49 @@ contract IthacaAccount is IIthacaAccount, EIP712, GuardedExecutor {
// ERC7821
////////////////////////////////////////////////////////////////////////

function _execute(
bytes32,
bytes calldata,
address to,
CallSansTo[] calldata calls,
bytes calldata opData
) internal virtual override {
// Orchestrator workflow.
if (msg.sender == ORCHESTRATOR) {
// opdata
// 0x00: keyHash
if (opData.length != 0x20) revert OpDataError();
bytes32 _keyHash = LibBytes.loadCalldata(opData, 0x00);

LibTStack.TStack(_KEYHASH_STACK_TRANSIENT_SLOT).push(_keyHash);
_execute(calls, to, _keyHash);
LibTStack.TStack(_KEYHASH_STACK_TRANSIENT_SLOT).pop();

return;
}

// Simple workflow without `opData`.
if (opData.length == uint256(0)) {
if (msg.sender != address(this)) revert Unauthorized();
return _execute(calls, to, bytes32(0));
}

// Simple workflow with `opData`.
if (opData.length < 0x20) revert OpDataError();
uint256 nonce = uint256(LibBytes.loadCalldata(opData, 0x00));
LibNonce.checkAndIncrement(_getAccountStorage().nonceSeqs, nonce);
emit NonceInvalidated(nonce);

(bool isValid, bytes32 keyHash) = unwrapAndValidateSignature(
computeDigest(calls, to, nonce), LibBytes.sliceCalldata(opData, 0x20)
);

if (!isValid) revert Unauthorized();
LibTStack.TStack(_KEYHASH_STACK_TRANSIENT_SLOT).push(keyHash);
_execute(calls, to, keyHash);
LibTStack.TStack(_KEYHASH_STACK_TRANSIENT_SLOT).pop();
}

/// @dev For ERC7821.
function _execute(bytes32, bytes calldata, Call[] calldata calls, bytes calldata opData)
internal
Expand Down Expand Up @@ -711,8 +786,6 @@ contract IthacaAccount is IIthacaAccount, EIP712, GuardedExecutor {
computeDigest(calls, nonce), LibBytes.sliceCalldata(opData, 0x20)
);
if (!isValid) revert Unauthorized();

// TODO: Figure out where else to add these operations, after removing delegate call.
LibTStack.TStack(_KEYHASH_STACK_TRANSIENT_SLOT).push(keyHash);
_execute(calls, keyHash);
LibTStack.TStack(_KEYHASH_STACK_TRANSIENT_SLOT).pop();
Expand Down
Loading
Loading