From 89466edc3b63d5e574c7accdd59486f5dc0da06c Mon Sep 17 00:00:00 2001 From: Fangting Liu Date: Wed, 16 Oct 2024 14:03:45 -0700 Subject: [PATCH 1/3] chore: optimize Base module a bit --- src/modules/BaseModule.sol | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/modules/BaseModule.sol b/src/modules/BaseModule.sol index 84276b8d..6a41cc3c 100644 --- a/src/modules/BaseModule.sol +++ b/src/modules/BaseModule.sol @@ -57,25 +57,32 @@ abstract contract BaseModule is ERC165, IModule { /// @dev help method that returns extracted selector and calldata. If selector is executeUserOp, return the /// selector and calldata of the inner call. - function _getSelectorAndCalldata(bytes calldata data) internal pure returns (bytes4, bytes memory) { - if (bytes4(data[:4]) == IAccountExecute.executeUserOp.selector) { - (PackedUserOperation memory uo,) = abi.decode(data[4:], (PackedUserOperation, bytes32)); - bytes4 selector; - bytes memory callData = uo.callData; + function _getSelectorAndCalldata(bytes calldata data) + internal + pure + returns (bytes4 selector, bytes memory finalCalldata) + { + selector = bytes4(data[:4]); + finalCalldata = data[4:]; + if (selector == IAccountExecute.executeUserOp.selector) { + (PackedUserOperation memory uo,) = abi.decode(finalCalldata, (PackedUserOperation, bytes32)); + finalCalldata = uo.callData; // Bytes arr representation: [bytes32(len), bytes4(executeUserOp.selector), bytes4(actualSelector), // bytes(actualCallData)] - // 1. Copy actualSelector into a new var - // 2. Shorten bytes arry by 8 by: store length - 8 into the new pointer location - // 3. Move the callData pointer by 8 assembly { - selector := mload(add(callData, 36)) + // Copy actualSelector into a new var + selector := mload(add(finalCalldata, 36)) - let len := mload(callData) - mstore(add(callData, 8), sub(len, 8)) - callData := add(callData, 8) + let len := mload(finalCalldata) + + // Move the finalCalldata pointer by 8 + finalCalldata := add(finalCalldata, 8) + + // Shorten bytes arry by 8 by: store length - 8 into the new pointer location + mstore(finalCalldata, sub(len, 8)) } - return (selector, callData); + return (selector, finalCalldata); } - return (bytes4(data[:4]), data[4:]); + return (selector, finalCalldata); } } From 6b450620f18759d88a5fd65fc4e0b63557a1305b Mon Sep 17 00:00:00 2001 From: Fangting Liu Date: Wed, 16 Oct 2024 15:06:24 -0700 Subject: [PATCH 2/3] update review --- src/modules/BaseModule.sol | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/modules/BaseModule.sol b/src/modules/BaseModule.sol index 6a41cc3c..f11bfa66 100644 --- a/src/modules/BaseModule.sol +++ b/src/modules/BaseModule.sol @@ -57,21 +57,16 @@ abstract contract BaseModule is ERC165, IModule { /// @dev help method that returns extracted selector and calldata. If selector is executeUserOp, return the /// selector and calldata of the inner call. - function _getSelectorAndCalldata(bytes calldata data) - internal - pure - returns (bytes4 selector, bytes memory finalCalldata) - { - selector = bytes4(data[:4]); - finalCalldata = data[4:]; + function _getSelectorAndCalldata(bytes calldata data) internal pure returns (bytes4, bytes memory) { + bytes4 selector = bytes4(data[:4]); if (selector == IAccountExecute.executeUserOp.selector) { - (PackedUserOperation memory uo,) = abi.decode(finalCalldata, (PackedUserOperation, bytes32)); - finalCalldata = uo.callData; + (PackedUserOperation memory uo,) = abi.decode(data[4:], (PackedUserOperation, bytes32)); + bytes memory finalCalldata = uo.callData; // Bytes arr representation: [bytes32(len), bytes4(executeUserOp.selector), bytes4(actualSelector), // bytes(actualCallData)] - assembly { + assembly ("memory-safe") { // Copy actualSelector into a new var - selector := mload(add(finalCalldata, 36)) + selector := shl(224, mload(add(finalCalldata, 8))) let len := mload(finalCalldata) @@ -83,6 +78,6 @@ abstract contract BaseModule is ERC165, IModule { } return (selector, finalCalldata); } - return (selector, finalCalldata); + return (selector, data[4:]); } } From 09048d0728d353692d55b68b949f9a607a629674 Mon Sep 17 00:00:00 2001 From: Fangting Liu Date: Wed, 16 Oct 2024 15:10:01 -0700 Subject: [PATCH 3/3] add gas benchmark --- .../ModularAccount_Runtime_InstallSessionKey_Case1.snap | 2 +- .../ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap | 2 +- .../ModularAccount_Runtime_UseSessionKey_Case1_Token.snap | 2 +- .../ModularAccount_UserOp_InstallSessionKey_Case1.snap | 2 +- .../ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap | 2 +- .../ModularAccount_UserOp_UseSessionKey_Case1_Token.snap | 2 +- .../SemiModularAccount_Runtime_InstallSessionKey_Case1.snap | 2 +- .../SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap | 2 +- .../SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap | 2 +- .../SemiModularAccount_UserOp_InstallSessionKey_Case1.snap | 2 +- .../SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap | 2 +- .../SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.forge-snapshots/ModularAccount_Runtime_InstallSessionKey_Case1.snap b/.forge-snapshots/ModularAccount_Runtime_InstallSessionKey_Case1.snap index 6775df71..4f5d99f0 100644 --- a/.forge-snapshots/ModularAccount_Runtime_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/ModularAccount_Runtime_InstallSessionKey_Case1.snap @@ -1 +1 @@ -423918 \ No newline at end of file +423482 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap index 81715a67..80017d24 100644 --- a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -79484 \ No newline at end of file +78838 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap index ad48a5ac..37f58ff6 100644 --- a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -113042 \ No newline at end of file +112223 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap b/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap index 4e34790c..79070d24 100644 --- a/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap @@ -1 +1 @@ -532152 \ No newline at end of file +531716 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap index 96000a80..788c162e 100644 --- a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -195938 \ No newline at end of file +195559 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap index 13ac764b..ef671140 100644 --- a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -227062 \ No newline at end of file +226591 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap b/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap index 52041783..f51cd88c 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap @@ -1 +1 @@ -422598 \ No newline at end of file +422162 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap index dbea92a3..dfca98c2 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -79900 \ No newline at end of file +79254 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap index b94e4ddf..367e1c2f 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -113458 \ No newline at end of file +112639 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap b/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap index 25d7e4dd..90f24a8c 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap @@ -1 +1 @@ -529725 \ No newline at end of file +529289 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap index a58597c7..541cafae 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -196232 \ No newline at end of file +195853 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap index f57c5ed5..d15eabfa 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -227368 \ No newline at end of file +226897 \ No newline at end of file