From 364ce1e016edff8d9083f43781a7dce539743b80 Mon Sep 17 00:00:00 2001 From: zer0dot Date: Wed, 22 May 2024 00:19:09 +0800 Subject: [PATCH 01/13] feat: initial implementation of an experimental contract creation flow --- src/common/BaseLightAccount.sol | 16 ++++++++++++++++ test/LightAccount.t.sol | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index 473f8e5..b333b9f 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -74,6 +74,22 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg _call(dest[i], value[i], func[i]); } } + /// @notice Creates a contract, this can only be called by this account. + /// @param initCode The initCode to deploy. NOTE: This could be replaced with transient storage in the near future, + /// depending on gas savings, if any. + function create(bytes calldata initCode) external payable virtual { + assembly ("memory-safe") { + // Check that the caller is this account, compiles to the same as inverting the condition. + if iszero(eq(caller(), address())) { + mstore(0, 0x913e98f1) + revert(28, 4) + } + let len := initCode.length + calldatacopy(0, initCode.offset, len) + let succ := create(callvalue(), 0, len) + return(0, 0) + } + } /// @notice Deposit more funds for this account in the entry point. function addDeposit() external payable { diff --git a/test/LightAccount.t.sol b/test/LightAccount.t.sol index 96e24fa..fdd8806 100644 --- a/test/LightAccount.t.sol +++ b/test/LightAccount.t.sol @@ -477,6 +477,24 @@ contract LightAccountTest is Test { assertEq(initialized, 1); } + function testRevertCreateContract_IncorrectCaller() public { + vm.expectRevert(BaseLightAccount.OnlyCallableBySelf.selector); + account.create(hex"1234"); + } + + function testCreateContract() public { + vm.prank(eoaAddress); + address expected = vm.computeCreateAddress(address(account), vm.getNonce(address(account))); + account.execute( + address(account), + 0, + abi.encodeCall( + account.create, (abi.encodePacked(type(LightAccount).creationCode, abi.encode(address(0x4546b)))) + ) + ); + assertEq(address(LightAccount(payable(expected)).entryPoint()), address(0x4546b)); + } + function _useContractOwner() internal { vm.prank(eoaAddress); account.transferOwnership(address(contractOwner)); From cc5388a2c55636fac5b58a60bce5dcf0d8f01cca Mon Sep 17 00:00:00 2001 From: zer0dot Date: Wed, 22 May 2024 00:19:50 +0800 Subject: [PATCH 02/13] misc: add error definition --- src/common/BaseLightAccount.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index b333b9f..f935245 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -24,6 +24,7 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg error InvalidSignatureType(); error NotAuthorized(address caller); error ZeroAddressNotAllowed(); + error OnlyCallableBySelf(); modifier onlyAuthorized() { _onlyAuthorized(); From ff938ce1af9d80ad0991799a2794eb0f5989e0db Mon Sep 17 00:00:00 2001 From: zer0dot Date: Wed, 22 May 2024 00:21:02 +0800 Subject: [PATCH 03/13] misc: formatting --- src/common/BaseLightAccount.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index f935245..3f60455 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -75,6 +75,7 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg _call(dest[i], value[i], func[i]); } } + /// @notice Creates a contract, this can only be called by this account. /// @param initCode The initCode to deploy. NOTE: This could be replaced with transient storage in the near future, /// depending on gas savings, if any. From 920aad0fa52889feba397a3d2c80c77a500572ae Mon Sep 17 00:00:00 2001 From: zer0dot Date: Wed, 22 May 2024 00:36:32 +0800 Subject: [PATCH 04/13] feat: handle create errors --- src/common/BaseLightAccount.sol | 14 +++++++++++--- test/LightAccount.t.sol | 13 +++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index 3f60455..7b0c7ca 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -25,6 +25,7 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg error NotAuthorized(address caller); error ZeroAddressNotAllowed(); error OnlyCallableBySelf(); + error CreateFailed(); modifier onlyAuthorized() { _onlyAuthorized(); @@ -81,15 +82,22 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg /// depending on gas savings, if any. function create(bytes calldata initCode) external payable virtual { assembly ("memory-safe") { - // Check that the caller is this account, compiles to the same as inverting the condition. + // Check that the caller is this account, this compiles to the same as inverting the condition if iszero(eq(caller(), address())) { - mstore(0, 0x913e98f1) + mstore(0, 0x913e98f1) // OnlyCallableBySelf() revert(28, 4) } + + // Copy the initCode to memory, then deploy the contract let len := initCode.length calldatacopy(0, initCode.offset, len) let succ := create(callvalue(), 0, len) - return(0, 0) + + // If the creation fails, revert + if iszero(succ) { + mstore(0, 0x7e16b8cd) // CreateFailed() + revert(28, 4) + } } } diff --git a/test/LightAccount.t.sol b/test/LightAccount.t.sol index fdd8806..5468183 100644 --- a/test/LightAccount.t.sol +++ b/test/LightAccount.t.sol @@ -482,6 +482,19 @@ contract LightAccountTest is Test { account.create(hex"1234"); } + function testRevertCreateContract_CreateFailed() public { + vm.prank(eoaAddress); + vm.expectRevert(BaseLightAccount.CreateFailed.selector); + account.execute( + address(account), + 0, + abi.encodeCall( + account.create, + (hex"01") // Attempt to deploy a contract with a single "ADD" opcode as the whole initcode, which will revert. + ) + ); + } + function testCreateContract() public { vm.prank(eoaAddress); address expected = vm.computeCreateAddress(address(account), vm.getNonce(address(account))); From ba9f228aa0643b8c8adfb921f408ae0be9c67ccb Mon Sep 17 00:00:00 2001 From: zer0dot Date: Fri, 24 May 2024 00:41:07 +0800 Subject: [PATCH 05/13] refactor: make create and create2 standard execution functions --- src/common/BaseLightAccount.sol | 19 ++++++++++---- test/LightAccount.t.sol | 44 ++++++++++++++++++++++----------- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index 7b0c7ca..4db6e4d 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -80,18 +80,27 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg /// @notice Creates a contract, this can only be called by this account. /// @param initCode The initCode to deploy. NOTE: This could be replaced with transient storage in the near future, /// depending on gas savings, if any. - function create(bytes calldata initCode) external payable virtual { + function create(bytes calldata initCode, uint256 value) external payable virtual onlyAuthorized { assembly ("memory-safe") { - // Check that the caller is this account, this compiles to the same as inverting the condition - if iszero(eq(caller(), address())) { - mstore(0, 0x913e98f1) // OnlyCallableBySelf() + // Copy the initCode to memory, then deploy the contract + let len := initCode.length + calldatacopy(0, initCode.offset, len) + let succ := create(value, 0, len) + + // If the creation fails, revert + if iszero(succ) { + mstore(0, 0x7e16b8cd) // CreateFailed() revert(28, 4) } + } + } + function create2(bytes calldata initCode, bytes32 salt, uint256 value) external payable virtual onlyAuthorized { + assembly ("memory-safe") { // Copy the initCode to memory, then deploy the contract let len := initCode.length calldatacopy(0, initCode.offset, len) - let succ := create(callvalue(), 0, len) + let succ := create2(value, 0, len, salt) // If the creation fails, revert if iszero(succ) { diff --git a/test/LightAccount.t.sol b/test/LightAccount.t.sol index 5468183..008e268 100644 --- a/test/LightAccount.t.sol +++ b/test/LightAccount.t.sol @@ -477,12 +477,12 @@ contract LightAccountTest is Test { assertEq(initialized, 1); } - function testRevertCreateContract_IncorrectCaller() public { - vm.expectRevert(BaseLightAccount.OnlyCallableBySelf.selector); - account.create(hex"1234"); + function testRevertCreate_IncorrectCaller() public { + vm.expectRevert(abi.encodeWithSelector(BaseLightAccount.NotAuthorized.selector, address(this))); + account.create(hex"1234", 0); } - function testRevertCreateContract_CreateFailed() public { + function testRevertCreate_CreateFailed() public { vm.prank(eoaAddress); vm.expectRevert(BaseLightAccount.CreateFailed.selector); account.execute( @@ -490,22 +490,38 @@ contract LightAccountTest is Test { 0, abi.encodeCall( account.create, - (hex"01") // Attempt to deploy a contract with a single "ADD" opcode as the whole initcode, which will revert. + (hex"01", 0) // Attempt to deploy a contract with a single "ADD" opcode as the whole initcode, which will revert. ) ); } - function testCreateContract() public { + function testRevertCreate2_IncorrectCaller() public { + vm.expectRevert(abi.encodeWithSelector(BaseLightAccount.NotAuthorized.selector, address(this))); + account.create2(hex"1234", bytes32(0), 0); + } + + function testRevertCreate2_CreateFailed() public { + vm.prank(eoaAddress); + vm.expectRevert(BaseLightAccount.CreateFailed.selector); + account.create2(hex"01", bytes32(0), 0); + } + + function testCreate() public { vm.prank(eoaAddress); address expected = vm.computeCreateAddress(address(account), vm.getNonce(address(account))); - account.execute( - address(account), - 0, - abi.encodeCall( - account.create, (abi.encodePacked(type(LightAccount).creationCode, abi.encode(address(0x4546b)))) - ) - ); - assertEq(address(LightAccount(payable(expected)).entryPoint()), address(0x4546b)); + account.create(abi.encodePacked(type(LightAccount).creationCode, abi.encode(address(entryPoint))), 0); + assertEq(address(LightAccount(payable(expected)).entryPoint()), address(entryPoint)); + } + + function testCreate2() public { + vm.prank(eoaAddress); + bytes memory initCode = abi.encodePacked(type(LightAccount).creationCode, abi.encode(address(entryPoint))); + bytes32 initCodeHash = keccak256(initCode); + bytes32 salt = bytes32(hex"04546b"); + address expected = vm.computeCreate2Address(salt, initCodeHash, address(account)); + + account.create2(abi.encodePacked(type(LightAccount).creationCode, abi.encode(address(entryPoint))), salt, 0); + assertEq(address(LightAccount(payable(expected)).entryPoint()), address(entryPoint)); } function _useContractOwner() internal { From 46e7b5324f10ff565c1710e0bf9d8fd3b5b1d690 Mon Sep 17 00:00:00 2001 From: zer0dot Date: Fri, 24 May 2024 20:44:05 +0800 Subject: [PATCH 06/13] chore: remove unused error --- src/common/BaseLightAccount.sol | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index 4db6e4d..d921566 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -24,7 +24,6 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg error InvalidSignatureType(); error NotAuthorized(address caller); error ZeroAddressNotAllowed(); - error OnlyCallableBySelf(); error CreateFailed(); modifier onlyAuthorized() { @@ -158,10 +157,12 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg } function _call(address target, uint256 value, bytes memory data) internal { - (bool success, bytes memory result) = target.call{value: value}(data); - if (!success) { - assembly ("memory-safe") { - revert(add(result, 32), mload(result)) + assembly ("memory-safe") { + let succ := call(gas(), target, value, add(data, 32), mload(data), 0, 0) + if iszero(succ) { + // We can overwrite memory since we're going to revert out of this call frame anyway + returndatacopy(0, 0, returndatasize()) + revert(0, returndatasize()) } } } From cd5609f546fd931fc061709e4bae12d52427df46 Mon Sep 17 00:00:00 2001 From: zer0dot Date: Fri, 24 May 2024 22:22:11 +0800 Subject: [PATCH 07/13] fix: replace a panicking with a reverting mock contract to not mess with gas reports too much --- test/LightAccount.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/LightAccount.t.sol b/test/LightAccount.t.sol index 008e268..1d1ab00 100644 --- a/test/LightAccount.t.sol +++ b/test/LightAccount.t.sol @@ -490,7 +490,7 @@ contract LightAccountTest is Test { 0, abi.encodeCall( account.create, - (hex"01", 0) // Attempt to deploy a contract with a single "ADD" opcode as the whole initcode, which will revert. + (hex"3d3dfd", 0) // Attempt to deploy a contract with creation code that reverts. ) ); } @@ -503,7 +503,7 @@ contract LightAccountTest is Test { function testRevertCreate2_CreateFailed() public { vm.prank(eoaAddress); vm.expectRevert(BaseLightAccount.CreateFailed.selector); - account.create2(hex"01", bytes32(0), 0); + account.create2(hex"3d3dfd", bytes32(0), 0); } function testCreate() public { From 3191aa01986dc3bc864f97731d4b1478db8cb7ae Mon Sep 17 00:00:00 2001 From: zer0dot Date: Mon, 27 May 2024 20:36:29 +0800 Subject: [PATCH 08/13] fix: memory-safety for IR optimizer --- src/common/BaseLightAccount.sol | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index d921566..d4b25d0 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -82,9 +82,10 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg function create(bytes calldata initCode, uint256 value) external payable virtual onlyAuthorized { assembly ("memory-safe") { // Copy the initCode to memory, then deploy the contract + let ptr := mload(64) let len := initCode.length - calldatacopy(0, initCode.offset, len) - let succ := create(value, 0, len) + calldatacopy(ptr, initCode.offset, len) + let succ := create(value, ptr, len) // If the creation fails, revert if iszero(succ) { @@ -97,9 +98,10 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg function create2(bytes calldata initCode, bytes32 salt, uint256 value) external payable virtual onlyAuthorized { assembly ("memory-safe") { // Copy the initCode to memory, then deploy the contract + let ptr := mload(64) let len := initCode.length - calldatacopy(0, initCode.offset, len) - let succ := create2(value, 0, len, salt) + calldatacopy(ptr, initCode.offset, len) + let succ := create2(value, ptr, len, salt) // If the creation fails, revert if iszero(succ) { @@ -161,8 +163,9 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg let succ := call(gas(), target, value, add(data, 32), mload(data), 0, 0) if iszero(succ) { // We can overwrite memory since we're going to revert out of this call frame anyway - returndatacopy(0, 0, returndatasize()) - revert(0, returndatasize()) + let ptr := mload(64) + returndatacopy(ptr, 0, returndatasize()) + revert(ptr, returndatasize()) } } } From bbd76db03f32574d95e75660b045d932b21cb6d0 Mon Sep 17 00:00:00 2001 From: zer0dot Date: Tue, 28 May 2024 17:31:50 +0800 Subject: [PATCH 09/13] refactor: use hex for data locations --- src/common/BaseLightAccount.sol | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index d4b25d0..24be259 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -82,15 +82,15 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg function create(bytes calldata initCode, uint256 value) external payable virtual onlyAuthorized { assembly ("memory-safe") { // Copy the initCode to memory, then deploy the contract - let ptr := mload(64) + let ptr := mload(0x40) let len := initCode.length calldatacopy(ptr, initCode.offset, len) let succ := create(value, ptr, len) // If the creation fails, revert if iszero(succ) { - mstore(0, 0x7e16b8cd) // CreateFailed() - revert(28, 4) + mstore(0x00, 0x7e16b8cd) // CreateFailed() + revert(0x1c, 0x04) } } } @@ -98,15 +98,15 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg function create2(bytes calldata initCode, bytes32 salt, uint256 value) external payable virtual onlyAuthorized { assembly ("memory-safe") { // Copy the initCode to memory, then deploy the contract - let ptr := mload(64) + let ptr := mload(0x40) let len := initCode.length calldatacopy(ptr, initCode.offset, len) let succ := create2(value, ptr, len, salt) // If the creation fails, revert if iszero(succ) { - mstore(0, 0x7e16b8cd) // CreateFailed() - revert(28, 4) + mstore(0x00, 0x7e16b8cd) // CreateFailed() + revert(0x1c, 0x04) } } } @@ -160,11 +160,11 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg function _call(address target, uint256 value, bytes memory data) internal { assembly ("memory-safe") { - let succ := call(gas(), target, value, add(data, 32), mload(data), 0, 0) + let succ := call(gas(), target, value, add(data, 0x20), mload(data), 0x00, 0) if iszero(succ) { // We can overwrite memory since we're going to revert out of this call frame anyway - let ptr := mload(64) - returndatacopy(ptr, 0, returndatasize()) + let ptr := mload(0x40) + returndatacopy(ptr, 0x00, returndatasize()) revert(ptr, returndatasize()) } } From 85bb117eace5b5b4c1a9be794d661db452060514 Mon Sep 17 00:00:00 2001 From: zer0dot Date: Wed, 29 May 2024 18:53:12 +0800 Subject: [PATCH 10/13] feat: return created addr, adopt jtriley style guide for yul --- src/common/BaseLightAccount.sol | 78 +++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 23 deletions(-) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index 24be259..be87955 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -76,36 +76,56 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg } } - /// @notice Creates a contract, this can only be called by this account. - /// @param initCode The initCode to deploy. NOTE: This could be replaced with transient storage in the near future, - /// depending on gas savings, if any. - function create(bytes calldata initCode, uint256 value) external payable virtual onlyAuthorized { + /// @notice Creates a contract. + /// @param initCode The initCode to deploy. + /// @param value The value to send to the new contract constructor. + /// @return createdAddr The created contract address. + /// + /// @dev Assembly procedure: + /// 1. Load the free memory pointer. + /// 2. Get the initCode length. + /// 3. Copy the initCode from callata to memory at the free memory pointer. + /// 4. Create the contract. + /// 5. If creation failed (the address returned is zero), revert with CreateFailed(). + function create(bytes calldata initCode, uint256 value) external payable virtual onlyAuthorized returns (address createdAddr) { assembly ("memory-safe") { - // Copy the initCode to memory, then deploy the contract - let ptr := mload(0x40) + + let fmp := mload(0x40) let len := initCode.length - calldatacopy(ptr, initCode.offset, len) - let succ := create(value, ptr, len) + calldatacopy(fmp, initCode.offset, len) - // If the creation fails, revert - if iszero(succ) { - mstore(0x00, 0x7e16b8cd) // CreateFailed() + createdAddr := create(value, fmp, len) + + if iszero(createdAddr) { + mstore(0x00, 0x7e16b8cd) revert(0x1c, 0x04) } } } - function create2(bytes calldata initCode, bytes32 salt, uint256 value) external payable virtual onlyAuthorized { + /// @notice Creates a contract using create2 deterministic deployment. + /// @param initCode The initCode to deploy. + /// @param salt The salt to use for the create2 operation. + /// @param value The value to send to the new contract constructor. + /// @return createdAddr The created contract address. + /// + /// @dev Assembly procedure: + /// 1. Load the free memory pointer. + /// 2. Get the initCode length. + /// 3. Copy the initCode from callata to memory at the free memory pointer. + /// 4. Create the contract using Create2 with the passed salt parameter. + /// 5. If creation failed (the address returned is zero), revert with CreateFailed(). + function create2(bytes calldata initCode, bytes32 salt, uint256 value) external payable virtual onlyAuthorized returns (address createdAddr) { assembly ("memory-safe") { - // Copy the initCode to memory, then deploy the contract - let ptr := mload(0x40) + + let fmp := mload(0x40) let len := initCode.length - calldatacopy(ptr, initCode.offset, len) - let succ := create2(value, ptr, len, salt) + calldatacopy(fmp, initCode.offset, len) + + createdAddr := create2(value, fmp, len, salt) - // If the creation fails, revert - if iszero(succ) { - mstore(0x00, 0x7e16b8cd) // CreateFailed() + if iszero(createdAddr) { + mstore(0x00, 0x7e16b8cd) revert(0x1c, 0x04) } } @@ -158,14 +178,26 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg return success ? SIG_VALIDATION_SUCCESS : SIG_VALIDATION_FAILED; } + /// @dev Assembly procedure: + /// 1. Execute the call, passing: + /// 1. The gas + /// 2. The target address + /// 3. The call value + /// 4. The pointer to the start location of the callData in memory + /// 5. The length of the calldata + /// 2. If the call failed, bubble up the revert reason by doing the following: + /// 1. Load the free memory pointer + /// 2. Copy the return data (which is the revert reason) to memory at the free memory pointer + /// 3. Revert with the copied return data function _call(address target, uint256 value, bytes memory data) internal { assembly ("memory-safe") { + let succ := call(gas(), target, value, add(data, 0x20), mload(data), 0x00, 0) + if iszero(succ) { - // We can overwrite memory since we're going to revert out of this call frame anyway - let ptr := mload(0x40) - returndatacopy(ptr, 0x00, returndatasize()) - revert(ptr, returndatasize()) + let fmp := mload(0x40) + returndatacopy(fmp, 0x00, returndatasize()) + revert(fmp, returndatasize()) } } } From f73ef69c5eabb8035837419d78a7896c43946a3e Mon Sep 17 00:00:00 2001 From: zer0dot Date: Wed, 29 May 2024 18:54:01 +0800 Subject: [PATCH 11/13] chore: sort errors --- src/common/BaseLightAccount.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index be87955..863058b 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -21,10 +21,10 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg } error ArrayLengthMismatch(); + error CreateFailed(); error InvalidSignatureType(); error NotAuthorized(address caller); error ZeroAddressNotAllowed(); - error CreateFailed(); modifier onlyAuthorized() { _onlyAuthorized(); @@ -89,7 +89,7 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg /// 5. If creation failed (the address returned is zero), revert with CreateFailed(). function create(bytes calldata initCode, uint256 value) external payable virtual onlyAuthorized returns (address createdAddr) { assembly ("memory-safe") { - + let fmp := mload(0x40) let len := initCode.length calldatacopy(fmp, initCode.offset, len) From cbdd9d6b25e23ab562d411e485ce67a872aa486d Mon Sep 17 00:00:00 2001 From: zer0dot Date: Wed, 29 May 2024 19:28:13 +0800 Subject: [PATCH 12/13] feat: update signatures and tests to match --- src/common/BaseLightAccount.sol | 8 +++--- test/LightAccount.t.sol | 49 ++++++++++++++++++++++++--------- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index 863058b..5a20794 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -77,8 +77,8 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg } /// @notice Creates a contract. - /// @param initCode The initCode to deploy. /// @param value The value to send to the new contract constructor. + /// @param initCode The initCode to deploy. /// @return createdAddr The created contract address. /// /// @dev Assembly procedure: @@ -87,7 +87,7 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg /// 3. Copy the initCode from callata to memory at the free memory pointer. /// 4. Create the contract. /// 5. If creation failed (the address returned is zero), revert with CreateFailed(). - function create(bytes calldata initCode, uint256 value) external payable virtual onlyAuthorized returns (address createdAddr) { + function performCreate(uint256 value, bytes calldata initCode) external payable virtual onlyAuthorized returns (address createdAddr) { assembly ("memory-safe") { let fmp := mload(0x40) @@ -104,9 +104,9 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg } /// @notice Creates a contract using create2 deterministic deployment. + /// @param value The value to send to the new contract constructor. /// @param initCode The initCode to deploy. /// @param salt The salt to use for the create2 operation. - /// @param value The value to send to the new contract constructor. /// @return createdAddr The created contract address. /// /// @dev Assembly procedure: @@ -115,7 +115,7 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg /// 3. Copy the initCode from callata to memory at the free memory pointer. /// 4. Create the contract using Create2 with the passed salt parameter. /// 5. If creation failed (the address returned is zero), revert with CreateFailed(). - function create2(bytes calldata initCode, bytes32 salt, uint256 value) external payable virtual onlyAuthorized returns (address createdAddr) { + function performCreate2(uint256 value, bytes calldata initCode, bytes32 salt) external payable virtual onlyAuthorized returns (address createdAddr) { assembly ("memory-safe") { let fmp := mload(0x40) diff --git a/test/LightAccount.t.sol b/test/LightAccount.t.sol index 1d1ab00..6657bb0 100644 --- a/test/LightAccount.t.sol +++ b/test/LightAccount.t.sol @@ -479,38 +479,45 @@ contract LightAccountTest is Test { function testRevertCreate_IncorrectCaller() public { vm.expectRevert(abi.encodeWithSelector(BaseLightAccount.NotAuthorized.selector, address(this))); - account.create(hex"1234", 0); + account.performCreate(0, hex"1234"); } function testRevertCreate_CreateFailed() public { vm.prank(eoaAddress); vm.expectRevert(BaseLightAccount.CreateFailed.selector); - account.execute( - address(account), - 0, - abi.encodeCall( - account.create, - (hex"3d3dfd", 0) // Attempt to deploy a contract with creation code that reverts. - ) - ); + account.performCreate(0, hex"3d3dfd"); } function testRevertCreate2_IncorrectCaller() public { vm.expectRevert(abi.encodeWithSelector(BaseLightAccount.NotAuthorized.selector, address(this))); - account.create2(hex"1234", bytes32(0), 0); + account.performCreate2(0, hex"1234", bytes32(0)); } function testRevertCreate2_CreateFailed() public { vm.prank(eoaAddress); vm.expectRevert(BaseLightAccount.CreateFailed.selector); - account.create2(hex"3d3dfd", bytes32(0), 0); + account.performCreate2(0, hex"3d3dfd", bytes32(0)); } function testCreate() public { vm.prank(eoaAddress); address expected = vm.computeCreateAddress(address(account), vm.getNonce(address(account))); - account.create(abi.encodePacked(type(LightAccount).creationCode, abi.encode(address(entryPoint))), 0); + + address returnedAddress = account.performCreate(0, abi.encodePacked(type(LightAccount).creationCode, abi.encode(address(entryPoint)))); assertEq(address(LightAccount(payable(expected)).entryPoint()), address(entryPoint)); + assertEq(returnedAddress, expected); + } + + function testCreateValue() public { + vm.prank(eoaAddress); + address expected = vm.computeCreateAddress(address(account), vm.getNonce(address(account))); + + uint256 value = 1 ether; + deal(address(account), value); + + address returnedAddress = account.performCreate(value, ""); + assertEq(returnedAddress, expected); + assertEq(returnedAddress.balance, value); } function testCreate2() public { @@ -520,8 +527,24 @@ contract LightAccountTest is Test { bytes32 salt = bytes32(hex"04546b"); address expected = vm.computeCreate2Address(salt, initCodeHash, address(account)); - account.create2(abi.encodePacked(type(LightAccount).creationCode, abi.encode(address(entryPoint))), salt, 0); + address returnedAddress = account.performCreate2(0, initCode, salt); assertEq(address(LightAccount(payable(expected)).entryPoint()), address(entryPoint)); + assertEq(returnedAddress, expected); + } + + function testCreate2Value() public { + vm.prank(eoaAddress); + bytes memory initCode = ""; + bytes32 initCodeHash = keccak256(initCode); + bytes32 salt = bytes32(hex"04546b"); + address expected = vm.computeCreate2Address(salt, initCodeHash, address(account)); + + uint256 value = 1 ether; + deal(address(account), value); + + address returnedAddress = account.performCreate2(value, initCode, salt); + assertEq(returnedAddress, expected); + assertEq(returnedAddress.balance, value); } function _useContractOwner() internal { From e2335c26870fcf8f4213f8885db6dd1eb984c615 Mon Sep 17 00:00:00 2001 From: zer0dot Date: Wed, 29 May 2024 19:57:03 +0800 Subject: [PATCH 13/13] chore: formatting --- src/common/BaseLightAccount.sol | 27 ++++++++++++++++++--------- test/LightAccount.t.sol | 9 +++++---- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index 5a20794..bcf5e48 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -80,16 +80,21 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg /// @param value The value to send to the new contract constructor. /// @param initCode The initCode to deploy. /// @return createdAddr The created contract address. - /// + /// /// @dev Assembly procedure: /// 1. Load the free memory pointer. /// 2. Get the initCode length. /// 3. Copy the initCode from callata to memory at the free memory pointer. /// 4. Create the contract. /// 5. If creation failed (the address returned is zero), revert with CreateFailed(). - function performCreate(uint256 value, bytes calldata initCode) external payable virtual onlyAuthorized returns (address createdAddr) { + function performCreate(uint256 value, bytes calldata initCode) + external + payable + virtual + onlyAuthorized + returns (address createdAddr) + { assembly ("memory-safe") { - let fmp := mload(0x40) let len := initCode.length calldatacopy(fmp, initCode.offset, len) @@ -108,20 +113,25 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg /// @param initCode The initCode to deploy. /// @param salt The salt to use for the create2 operation. /// @return createdAddr The created contract address. - /// + /// /// @dev Assembly procedure: /// 1. Load the free memory pointer. /// 2. Get the initCode length. /// 3. Copy the initCode from callata to memory at the free memory pointer. /// 4. Create the contract using Create2 with the passed salt parameter. /// 5. If creation failed (the address returned is zero), revert with CreateFailed(). - function performCreate2(uint256 value, bytes calldata initCode, bytes32 salt) external payable virtual onlyAuthorized returns (address createdAddr) { + function performCreate2(uint256 value, bytes calldata initCode, bytes32 salt) + external + payable + virtual + onlyAuthorized + returns (address createdAddr) + { assembly ("memory-safe") { - let fmp := mload(0x40) let len := initCode.length calldatacopy(fmp, initCode.offset, len) - + createdAddr := create2(value, fmp, len, salt) if iszero(createdAddr) { @@ -191,9 +201,8 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg /// 3. Revert with the copied return data function _call(address target, uint256 value, bytes memory data) internal { assembly ("memory-safe") { - let succ := call(gas(), target, value, add(data, 0x20), mload(data), 0x00, 0) - + if iszero(succ) { let fmp := mload(0x40) returndatacopy(fmp, 0x00, returndatasize()) diff --git a/test/LightAccount.t.sol b/test/LightAccount.t.sol index 6657bb0..cbf9385 100644 --- a/test/LightAccount.t.sol +++ b/test/LightAccount.t.sol @@ -502,8 +502,9 @@ contract LightAccountTest is Test { function testCreate() public { vm.prank(eoaAddress); address expected = vm.computeCreateAddress(address(account), vm.getNonce(address(account))); - - address returnedAddress = account.performCreate(0, abi.encodePacked(type(LightAccount).creationCode, abi.encode(address(entryPoint)))); + + address returnedAddress = + account.performCreate(0, abi.encodePacked(type(LightAccount).creationCode, abi.encode(address(entryPoint)))); assertEq(address(LightAccount(payable(expected)).entryPoint()), address(entryPoint)); assertEq(returnedAddress, expected); } @@ -511,7 +512,7 @@ contract LightAccountTest is Test { function testCreateValue() public { vm.prank(eoaAddress); address expected = vm.computeCreateAddress(address(account), vm.getNonce(address(account))); - + uint256 value = 1 ether; deal(address(account), value); @@ -538,7 +539,7 @@ contract LightAccountTest is Test { bytes32 initCodeHash = keccak256(initCode); bytes32 salt = bytes32(hex"04546b"); address expected = vm.computeCreate2Address(salt, initCodeHash, address(account)); - + uint256 value = 1 ether; deal(address(account), value);