From 2efa6ac6c489abc9e2b8c9135bea4d79c1dc1211 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 20 Dec 2024 14:23:35 +0100 Subject: [PATCH] fix sending eth to EOA --- contracts/account/AccountCore.sol | 6 +++- test/account/Account.behavior.js | 49 ++++++++++++++++++------------- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/contracts/account/AccountCore.sol b/contracts/account/AccountCore.sol index 0daad2f..8bab198 100644 --- a/contracts/account/AccountCore.sol +++ b/contracts/account/AccountCore.sol @@ -91,10 +91,14 @@ abstract contract AccountCore is AbstractSigner, EIP712, IAccount, IAccountExecu PackedUserOperation calldata userOp, bytes32 /*userOpHash*/ ) public virtual onlyEntryPointOrSelf { + // decode packed calldata address target = address(bytes20(userOp.callData[4:24])); uint256 value = uint256(bytes32(userOp.callData[24:56])); bytes calldata data = userOp.callData[56:]; - Address.functionCallWithValue(target, data, value); + + // we cannot use `Address.functionCallWithValue` here as it would revert on EOA targets + (bool success, bytes memory returndata) = target.call{value: value}(data); + Address.verifyCallResult(success, returndata); } /** diff --git a/test/account/Account.behavior.js b/test/account/Account.behavior.js index e41fe43..4a9fb47 100644 --- a/test/account/Account.behavior.js +++ b/test/account/Account.behavior.js @@ -27,11 +27,10 @@ function shouldBehaveLikeAnAccountBase() { }); it('should revert if the caller is not the canonical entrypoint', async function () { - const selector = this.mock.interface.getFunction('executeUserOp').selector; const operation = await this.mock .createUserOp({ callData: ethers.concat([ - selector, + this.mock.interface.getFunction('executeUserOp').selector, encodeExecuteUserOp(this.target, 0, this.target.interface.encodeFunctionData('mockFunctionExtra')), ]), }) @@ -48,11 +47,10 @@ function shouldBehaveLikeAnAccountBase() { }); it('should return SIG_VALIDATION_SUCCESS if the signature is valid', async function () { - const selector = this.mock.interface.getFunction('executeUserOp').selector; const operation = await this.mock .createUserOp({ callData: ethers.concat([ - selector, + this.mock.interface.getFunction('executeUserOp').selector, encodeExecuteUserOp(this.target, 0, this.target.interface.encodeFunctionData('mockFunctionExtra')), ]), }) @@ -66,10 +64,9 @@ function shouldBehaveLikeAnAccountBase() { }); it('should return SIG_VALIDATION_FAILURE if the signature is invalid', async function () { - const selector = this.mock.interface.getFunction('executeUserOp').selector; const operation = await this.mock.createUserOp({ callData: ethers.concat([ - selector, + this.mock.interface.getFunction('executeUserOp').selector, encodeExecuteUserOp(this.target, 0, this.target.interface.encodeFunctionData('mockFunctionExtra')), ]), }); @@ -84,11 +81,10 @@ function shouldBehaveLikeAnAccountBase() { }); it('should pay missing account funds for execution', async function () { - const selector = this.mock.interface.getFunction('executeUserOp').selector; const operation = await this.mock .createUserOp({ callData: ethers.concat([ - selector, + this.mock.interface.getFunction('executeUserOp').selector, encodeExecuteUserOp(this.target, 0, this.target.interface.encodeFunctionData('mockFunctionExtra')), ]), }) @@ -199,11 +195,10 @@ function shouldBehaveLikeAnAccountBaseExecutor({ deployable = true } = {}) { it('should revert if the caller is not the canonical entrypoint or the account itself', async function () { await this.mock.deploy(); - const selector = this.mock.interface.getFunction('executeUserOp').selector; const operation = await this.mock .createUserOp({ callData: ethers.concat([ - selector, + this.mock.interface.getFunction('executeUserOp').selector, encodeExecuteUserOp(this.target, 0, this.target.interface.encodeFunctionData('mockFunctionExtra')), ]), }) @@ -217,11 +212,10 @@ function shouldBehaveLikeAnAccountBaseExecutor({ deployable = true } = {}) { if (deployable) { describe('when not deployed', function () { it('should be created with handleOps and increase nonce', async function () { - const selector = this.mock.interface.getFunction('executeUserOp').selector; const operation = await this.mock .createUserOp({ callData: ethers.concat([ - selector, + this.mock.interface.getFunction('executeUserOp').selector, encodeExecuteUserOp(this.target, 17, this.target.interface.encodeFunctionData('mockFunctionExtra')), ]), }) @@ -237,11 +231,10 @@ function shouldBehaveLikeAnAccountBaseExecutor({ deployable = true } = {}) { }); it('should revert if the signature is invalid', async function () { - const selector = this.mock.interface.getFunction('executeUserOp').selector; const operation = await this.mock .createUserOp({ callData: ethers.concat([ - selector, + this.mock.interface.getFunction('executeUserOp').selector, encodeExecuteUserOp(this.target, 17, this.target.interface.encodeFunctionData('mockFunctionExtra')), ]), }) @@ -260,16 +253,11 @@ function shouldBehaveLikeAnAccountBaseExecutor({ deployable = true } = {}) { }); it('should increase nonce and call target', async function () { - const selector = this.mock.interface.getFunction('executeUserOp').selector; const operation = await this.mock .createUserOp({ callData: ethers.concat([ - selector, - encodeExecuteUserOp( - this.target.target, - 42, - this.target.interface.encodeFunctionData('mockFunctionExtra'), - ), + this.mock.interface.getFunction('executeUserOp').selector, + encodeExecuteUserOp(this.target, 42, this.target.interface.encodeFunctionData('mockFunctionExtra')), ]), }) .then(op => this.signUserOp(op)); @@ -280,6 +268,25 @@ function shouldBehaveLikeAnAccountBaseExecutor({ deployable = true } = {}) { .withArgs(this.mock, 42); expect(await this.mock.getNonce()).to.equal(1); }); + + it('should support sending eth to an EOA', async function () { + await setBalance(this.mock.address, ethers.parseEther('1')); + const value = 43374337n; + + const operation = await this.mock + .createUserOp({ + callData: ethers.concat([ + this.mock.interface.getFunction('executeUserOp').selector, + encodeExecuteUserOp(this.other, value), + ]), + }) + .then(op => this.signUserOp(op)); + + await expect(entrypoint.handleOps([operation.packed], this.beneficiary)).to.changeEtherBalance( + this.other, + value, + ); + }); }); }); }