Skip to content

Commit abeec1a

Browse files
AmxxAmxx
authored
Fix executeUserOp with eoa (#50)
1 parent e453685 commit abeec1a

File tree

7 files changed

+122
-151
lines changed

7 files changed

+122
-151
lines changed

‎contracts/account/AccountCore.sol

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,14 @@ abstract contract AccountCore is AbstractSigner, EIP712, IAccount, IAccountExecu
9191
PackedUserOperation calldata userOp,
9292
bytes32 /*userOpHash*/
9393
) public virtual onlyEntryPointOrSelf {
94-
(address target, uint256 value, bytes memory data) = abi.decode(userOp.callData[4:], (address, uint256, bytes));
95-
Address.functionCallWithValue(target, data, value);
94+
// decode packed calldata
95+
address target = address(bytes20(userOp.callData[4:24]));
96+
uint256 value = uint256(bytes32(userOp.callData[24:56]));
97+
bytes calldata data = userOp.callData[56:];
98+
99+
// we cannot use `Address.functionCallWithValue` here as it would revert on EOA targets
100+
(bool success, bytes memory returndata) = target.call{value: value}(data);
101+
Address.verifyCallResult(success, returndata);
96102
}
97103

98104
/**

‎test/account/Account.behavior.js

Lines changed: 95 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ const {
88
shouldSupportInterfaces,
99
} = require('@openzeppelin/contracts/test/utils/introspection/SupportsInterface.behavior');
1010

11-
function shouldBehaveLikeAnAccountBase() {
11+
const value = ethers.parseEther('0.1');
12+
13+
function shouldBehaveLikeAccountCore() {
1214
describe('entryPoint', function () {
1315
it('should return the canonical entrypoint', async function () {
1416
await this.mock.deploy();
15-
expect(await this.mock.entryPoint()).to.equal(entrypoint);
17+
expect(this.mock.entryPoint()).to.eventually.equal(entrypoint);
1618
});
1719
});
1820

@@ -23,18 +25,8 @@ function shouldBehaveLikeAnAccountBase() {
2325
});
2426

2527
it('should revert if the caller is not the canonical entrypoint', async function () {
26-
const selector = this.mock.interface.getFunction('executeUserOp').selector;
27-
const operation = await this.mock
28-
.createUserOp({
29-
callData: ethers.concat([
30-
selector,
31-
ethers.AbiCoder.defaultAbiCoder().encode(
32-
['address', 'uint256', 'bytes'],
33-
[this.target.target, 0, this.target.interface.encodeFunctionData('mockFunctionExtra')],
34-
),
35-
]),
36-
})
37-
.then(op => this.signUserOp(op));
28+
// empty operation (does nothing)
29+
const operation = await this.mock.createUserOp({}).then(op => this.signUserOp(op));
3830

3931
await expect(this.mock.connect(this.other).validateUserOp(operation.packed, operation.hash(), 0))
4032
.to.be.revertedWithCustomError(this.mock, 'AccountUnauthorized')
@@ -47,18 +39,8 @@ function shouldBehaveLikeAnAccountBase() {
4739
});
4840

4941
it('should return SIG_VALIDATION_SUCCESS if the signature is valid', async function () {
50-
const selector = this.mock.interface.getFunction('executeUserOp').selector;
51-
const operation = await this.mock
52-
.createUserOp({
53-
callData: ethers.concat([
54-
selector,
55-
ethers.AbiCoder.defaultAbiCoder().encode(
56-
['address', 'uint256', 'bytes'],
57-
[this.target.target, 0, this.target.interface.encodeFunctionData('mockFunctionExtra')],
58-
),
59-
]),
60-
})
61-
.then(op => this.signUserOp(op));
42+
// empty operation (does nothing)
43+
const operation = await this.mock.createUserOp({}).then(op => this.signUserOp(op));
6244

6345
expect(
6446
await this.mock
@@ -68,17 +50,8 @@ function shouldBehaveLikeAnAccountBase() {
6850
});
6951

7052
it('should return SIG_VALIDATION_FAILURE if the signature is invalid', async function () {
71-
const selector = this.mock.interface.getFunction('executeUserOp').selector;
72-
const operation = await this.mock.createUserOp({
73-
callData: ethers.concat([
74-
selector,
75-
ethers.AbiCoder.defaultAbiCoder().encode(
76-
['address', 'uint256', 'bytes'],
77-
[this.target.target, 0, this.target.interface.encodeFunctionData('mockFunctionExtra')],
78-
),
79-
]),
80-
});
81-
53+
// empty operation (does nothing)
54+
const operation = await this.mock.createUserOp({});
8255
operation.signature = '0x00';
8356

8457
expect(
@@ -89,46 +62,24 @@ function shouldBehaveLikeAnAccountBase() {
8962
});
9063

9164
it('should pay missing account funds for execution', async function () {
92-
const selector = this.mock.interface.getFunction('executeUserOp').selector;
93-
const operation = await this.mock
94-
.createUserOp({
95-
callData: ethers.concat([
96-
selector,
97-
ethers.AbiCoder.defaultAbiCoder().encode(
98-
['address', 'uint256', 'bytes'],
99-
[this.target.target, 0, this.target.interface.encodeFunctionData('mockFunctionExtra')],
100-
),
101-
]),
102-
})
103-
.then(op => this.signUserOp(op));
104-
105-
const prevAccountBalance = await ethers.provider.getBalance(this.mock);
106-
const prevEntrypointBalance = await ethers.provider.getBalance(entrypoint);
107-
const amount = ethers.parseEther('0.1');
108-
109-
const tx = await this.mock
110-
.connect(this.entrypointAsSigner)
111-
.validateUserOp(operation.packed, operation.hash(), amount);
65+
// empty operation (does nothing)
66+
const operation = await this.mock.createUserOp({}).then(op => this.signUserOp(op));
11267

113-
const receipt = await tx.wait();
114-
const callerFees = receipt.gasUsed * tx.gasPrice;
115-
116-
expect(await ethers.provider.getBalance(this.mock)).to.equal(prevAccountBalance - amount);
117-
expect(await ethers.provider.getBalance(entrypoint)).to.equal(prevEntrypointBalance + amount - callerFees);
68+
await expect(
69+
this.mock.connect(this.entrypointAsSigner).validateUserOp(operation.packed, operation.hash(), value),
70+
).to.changeEtherBalances([this.mock, entrypoint], [-value, value]);
11871
});
11972
});
12073
});
12174

12275
describe('fallback', function () {
12376
it('should receive ether', async function () {
12477
await this.mock.deploy();
125-
await setBalance(this.other.address, ethers.parseEther('1'));
126-
127-
const prevBalance = await ethers.provider.getBalance(this.mock);
128-
const amount = ethers.parseEther('0.1');
129-
await this.other.sendTransaction({ to: this.mock, value: amount });
13078

131-
expect(await ethers.provider.getBalance(this.mock)).to.equal(prevBalance + amount);
79+
await expect(this.other.sendTransaction({ to: this.mock, value })).to.changeEtherBalances(
80+
[this.other, this.mock],
81+
[-value, value],
82+
);
13283
});
13384
});
13485
}
@@ -147,76 +98,85 @@ function shouldBehaveLikeAccountHolder() {
14798
const data = '0x12345678';
14899

149100
beforeEach(async function () {
150-
[this.owner] = await ethers.getSigners();
151101
this.token = await ethers.deployContract('$ERC1155Mock', ['https://somedomain.com/{id}.json']);
152-
await this.token.$_mintBatch(this.owner, ids, values, '0x');
102+
await this.token.$_mintBatch(this.other, ids, values, '0x');
153103
});
154104

155105
it('receives ERC1155 tokens from a single ID', async function () {
156-
await this.token.connect(this.owner).safeTransferFrom(this.owner, this.mock, ids[0], values[0], data);
157-
expect(await this.token.balanceOf(this.mock, ids[0])).to.equal(values[0]);
158-
for (let i = 1; i < ids.length; i++) {
159-
expect(await this.token.balanceOf(this.mock, ids[i])).to.equal(0n);
160-
}
106+
await this.token.connect(this.other).safeTransferFrom(this.other, this.mock, ids[0], values[0], data);
107+
108+
expect(
109+
this.token.balanceOfBatch(
110+
ids.map(() => this.mock),
111+
ids,
112+
),
113+
).to.eventually.deep.equal(values.map((v, i) => (i == 0 ? v : 0n)));
161114
});
162115

163116
it('receives ERC1155 tokens from a multiple IDs', async function () {
164117
expect(
165-
await this.token.balanceOfBatch(
118+
this.token.balanceOfBatch(
166119
ids.map(() => this.mock),
167120
ids,
168121
),
169-
).to.deep.equal(ids.map(() => 0n));
170-
await this.token.connect(this.owner).safeBatchTransferFrom(this.owner, this.mock, ids, values, data);
122+
).to.eventually.deep.equal(ids.map(() => 0n));
123+
124+
await this.token.connect(this.other).safeBatchTransferFrom(this.other, this.mock, ids, values, data);
171125
expect(
172-
await this.token.balanceOfBatch(
126+
this.token.balanceOfBatch(
173127
ids.map(() => this.mock),
174128
ids,
175129
),
176-
).to.deep.equal(values);
130+
).to.eventually.deep.equal(values);
177131
});
178132
});
179133

180134
describe('onERC721Received', function () {
181-
it('receives an ERC721 token', async function () {
182-
const name = 'Some NFT';
183-
const symbol = 'SNFT';
184-
const tokenId = 1n;
135+
const tokenId = 1n;
185136

186-
const [owner] = await ethers.getSigners();
187-
188-
const token = await ethers.deployContract('$ERC721Mock', [name, symbol]);
189-
await token.$_mint(owner, tokenId);
137+
beforeEach(async function () {
138+
this.token = await ethers.deployContract('$ERC721Mock', ['Some NFT', 'SNFT']);
139+
await this.token.$_mint(this.other, tokenId);
140+
});
190141

191-
await token.connect(owner).safeTransferFrom(owner, this.mock, tokenId);
142+
it('receives an ERC721 token', async function () {
143+
await this.token.connect(this.other).safeTransferFrom(this.other, this.mock, tokenId);
192144

193-
expect(await token.ownerOf(tokenId)).to.equal(this.mock);
145+
expect(this.token.ownerOf(tokenId)).to.eventually.equal(this.mock);
194146
});
195147
});
196148
});
197149
}
198150

199-
function shouldBehaveLikeAnAccountBaseExecutor({ deployable = true } = {}) {
151+
function shouldBehaveLikeAccountExecutor({ deployable = true } = {}) {
200152
describe('executeUserOp', function () {
201153
beforeEach(async function () {
154+
// give eth to the account (before deployment)
202155
await setBalance(this.mock.target, ethers.parseEther('1'));
203-
expect(await ethers.provider.getCode(this.mock)).to.equal('0x');
204-
this.entrypointAsSigner = await impersonate(entrypoint.target);
156+
157+
// account is not initially deployed
158+
expect(ethers.provider.getCode(this.mock)).to.eventually.equal('0x');
159+
160+
this.encodeUserOpCalldata = (to, value, calldata) =>
161+
ethers.concat([
162+
this.mock.interface.getFunction('executeUserOp').selector,
163+
ethers.solidityPacked(
164+
['address', 'uint256', 'bytes'],
165+
[to.target ?? to.address ?? to, value ?? 0, calldata ?? '0x'],
166+
),
167+
]);
205168
});
206169

207170
it('should revert if the caller is not the canonical entrypoint or the account itself', async function () {
208171
await this.mock.deploy();
209172

210-
const selector = this.mock.interface.getFunction('executeUserOp').selector;
211173
const operation = await this.mock
212174
.createUserOp({
213-
callData: ethers.concat([
214-
selector,
215-
ethers.AbiCoder.defaultAbiCoder().encode(
216-
['address', 'uint256', 'bytes'],
217-
[this.target.target, 0, this.target.interface.encodeFunctionData('mockFunctionExtra')],
218-
),
219-
]),
175+
callData: this.encodeUserOpCalldata(
176+
this.target,
177+
0,
178+
this.target.interface.encodeFunctionData('mockFunctionExtra'),
179+
),
220180
})
221181
.then(op => this.signUserOp(op));
222182

@@ -228,39 +188,34 @@ function shouldBehaveLikeAnAccountBaseExecutor({ deployable = true } = {}) {
228188
if (deployable) {
229189
describe('when not deployed', function () {
230190
it('should be created with handleOps and increase nonce', async function () {
231-
const selector = this.mock.interface.getFunction('executeUserOp').selector;
232191
const operation = await this.mock
233192
.createUserOp({
234-
callData: ethers.concat([
235-
selector,
236-
ethers.AbiCoder.defaultAbiCoder().encode(
237-
['address', 'uint256', 'bytes'],
238-
[this.target.target, 17, this.target.interface.encodeFunctionData('mockFunctionExtra')],
239-
),
240-
]),
193+
callData: this.encodeUserOpCalldata(
194+
this.target,
195+
17,
196+
this.target.interface.encodeFunctionData('mockFunctionExtra'),
197+
),
241198
})
242199
.then(op => op.addInitCode())
243200
.then(op => this.signUserOp(op));
244201

202+
expect(this.mock.getNonce()).to.eventually.equal(0);
245203
await expect(entrypoint.handleOps([operation.packed], this.beneficiary))
246204
.to.emit(entrypoint, 'AccountDeployed')
247205
.withArgs(operation.hash(), this.mock, this.factory, ethers.ZeroAddress)
248206
.to.emit(this.target, 'MockFunctionCalledExtra')
249207
.withArgs(this.mock, 17);
250-
expect(await this.mock.getNonce()).to.equal(1);
208+
expect(this.mock.getNonce()).to.eventually.equal(1);
251209
});
252210

253211
it('should revert if the signature is invalid', async function () {
254-
const selector = this.mock.interface.getFunction('executeUserOp').selector;
255212
const operation = await this.mock
256213
.createUserOp({
257-
callData: ethers.concat([
258-
selector,
259-
ethers.AbiCoder.defaultAbiCoder().encode(
260-
['address', 'uint256', 'bytes'],
261-
[this.target.target, 17, this.target.interface.encodeFunctionData('mockFunctionExtra')],
262-
),
263-
]),
214+
callData: this.encodeUserOpCalldata(
215+
this.target,
216+
17,
217+
this.target.interface.encodeFunctionData('mockFunctionExtra'),
218+
),
264219
})
265220
.then(op => op.addInitCode());
266221

@@ -277,31 +232,41 @@ function shouldBehaveLikeAnAccountBaseExecutor({ deployable = true } = {}) {
277232
});
278233

279234
it('should increase nonce and call target', async function () {
280-
const selector = this.mock.interface.getFunction('executeUserOp').selector;
281235
const operation = await this.mock
282236
.createUserOp({
283-
callData: ethers.concat([
284-
selector,
285-
ethers.AbiCoder.defaultAbiCoder().encode(
286-
['address', 'uint256', 'bytes'],
287-
[this.target.target, 42, this.target.interface.encodeFunctionData('mockFunctionExtra')],
288-
),
289-
]),
237+
callData: this.encodeUserOpCalldata(
238+
this.target,
239+
42,
240+
this.target.interface.encodeFunctionData('mockFunctionExtra'),
241+
),
290242
})
291243
.then(op => this.signUserOp(op));
292244

293-
expect(await this.mock.getNonce()).to.equal(0);
245+
expect(this.mock.getNonce()).to.eventually.equal(0);
294246
await expect(entrypoint.handleOps([operation.packed], this.beneficiary))
295247
.to.emit(this.target, 'MockFunctionCalledExtra')
296248
.withArgs(this.mock, 42);
297-
expect(await this.mock.getNonce()).to.equal(1);
249+
expect(this.mock.getNonce()).to.eventually.equal(1);
250+
});
251+
252+
it('should support sending eth to an EOA', async function () {
253+
const operation = await this.mock
254+
.createUserOp({ callData: this.encodeUserOpCalldata(this.other, value) })
255+
.then(op => this.signUserOp(op));
256+
257+
expect(this.mock.getNonce()).to.eventually.equal(0);
258+
await expect(entrypoint.handleOps([operation.packed], this.beneficiary)).to.changeEtherBalance(
259+
this.other,
260+
value,
261+
);
262+
expect(this.mock.getNonce()).to.eventually.equal(1);
298263
});
299264
});
300265
});
301266
}
302267

303268
module.exports = {
304-
shouldBehaveLikeAnAccountBase,
269+
shouldBehaveLikeAccountCore,
305270
shouldBehaveLikeAccountHolder,
306-
shouldBehaveLikeAnAccountBaseExecutor,
271+
shouldBehaveLikeAccountExecutor,
307272
};

‎test/account/AccountBase.test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
33
const { ERC4337Helper } = require('../helpers/erc4337');
44
const { NonNativeSigner } = require('../helpers/signers');
55

6-
const { shouldBehaveLikeAnAccountBase, shouldBehaveLikeAnAccountBaseExecutor } = require('./Account.behavior');
6+
const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountExecutor } = require('./Account.behavior');
77

88
async function fixture() {
99
// EOAs and environment
@@ -31,6 +31,6 @@ describe('AccountBase', function () {
3131
Object.assign(this, await loadFixture(fixture));
3232
});
3333

34-
shouldBehaveLikeAnAccountBase();
35-
shouldBehaveLikeAnAccountBaseExecutor();
34+
shouldBehaveLikeAccountCore();
35+
shouldBehaveLikeAccountExecutor();
3636
});

0 commit comments

Comments
 (0)