Skip to content

Commit 77e5684

Browse files
Revert if a call fails in an ERC2771Forwarder atomic batch (#6391)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 0bd6f02 commit 77e5684

File tree

3 files changed

+26
-0
lines changed

3 files changed

+26
-0
lines changed

.changeset/some-dolls-shine.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': minor
3+
---
4+
5+
`ERC2771Forwarder`: Revert the entire atomic batch if a call with value fails.

contracts/metatx/ERC2771Forwarder.sol

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ contract ERC2771Forwarder is EIP712, Nonces {
7575
*/
7676
event ExecutedForwardRequest(address indexed signer, uint256 nonce, bool success);
7777

78+
/**
79+
* @dev One of the calls in an atomic batch failed.
80+
*/
81+
error ERC2771ForwarderFailureInAtomicBatch();
82+
7883
/**
7984
* @dev The request `from` doesn't match with the recovered `signer`.
8085
*/
@@ -186,6 +191,8 @@ contract ERC2771Forwarder is EIP712, Nonces {
186191
// Some requests with value were invalid (possibly due to frontrunning).
187192
// To avoid leaving ETH in the contract this value is refunded.
188193
if (refundValue != 0) {
194+
if (atomic) revert ERC2771ForwarderFailureInAtomicBatch();
195+
189196
// We know refundReceiver != address(0) && requestsValue == msg.value
190197
// meaning we can ensure refundValue is not taken from the original contract's balance
191198
// and refundReceiver is a known account.

test/metatx/ERC2771Forwarder.test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,20 @@ describe('ERC2771Forwarder', function () {
227227
expect(await this.forwarder.nonces(request.from)).to.equal(request.nonce + 1n);
228228
}
229229
});
230+
231+
it('atomic batch with reverting request reverts the whole batch', async function () {
232+
// Add extra reverting request
233+
await this.forgeRequest(
234+
{ value: 10n, data: this.receiver.interface.encodeFunctionData('mockFunctionRevertsNoReason') },
235+
this.accounts[requestCount],
236+
).then(extraRequest => this.requests.push(extraRequest));
237+
// recompute total value with the extra request
238+
this.value = requestsValue(this.requests);
239+
240+
await expect(
241+
this.forwarder.executeBatch(this.requests, ethers.ZeroAddress, { value: this.value }),
242+
).to.be.revertedWithCustomError(this.forwarder, 'ERC2771ForwarderFailureInAtomicBatch');
243+
});
230244
});
231245

232246
describe('with tampered requests', function () {

0 commit comments

Comments
 (0)