Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/some-dolls-shine.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`ERC2771Forwarder`: Revert the entire atomic batch if a call with value fails.
7 changes: 7 additions & 0 deletions contracts/metatx/ERC2771Forwarder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ contract ERC2771Forwarder is EIP712, Nonces {
*/
event ExecutedForwardRequest(address indexed signer, uint256 nonce, bool success);

/**
* @dev One of the calls in an atomic batch failed.
*/
error ERC2771ForwarderFailureInAtomicBatch();

/**
* @dev The request `from` doesn't match with the recovered `signer`.
*/
Expand Down Expand Up @@ -186,6 +191,8 @@ contract ERC2771Forwarder is EIP712, Nonces {
// Some requests with value were invalid (possibly due to frontrunning).
// To avoid leaving ETH in the contract this value is refunded.
if (refundValue != 0) {
if (atomic) revert ERC2771ForwarderFailureInAtomicBatch();

// We know refundReceiver != address(0) && requestsValue == msg.value
// meaning we can ensure refundValue is not taken from the original contract's balance
// and refundReceiver is a known account.
Expand Down
14 changes: 14 additions & 0 deletions test/metatx/ERC2771Forwarder.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,20 @@ describe('ERC2771Forwarder', function () {
expect(await this.forwarder.nonces(request.from)).to.equal(request.nonce + 1n);
}
});

it('atomic batch with reverting request reverts the whole batch', async function () {
// Add extra reverting request
await this.forgeRequest(
{ value: 10n, data: this.receiver.interface.encodeFunctionData('mockFunctionRevertsNoReason') },
this.accounts[requestCount],
).then(extraRequest => this.requests.push(extraRequest));
// recompute total value with the extra request
this.value = requestsValue(this.requests);

await expect(
this.forwarder.executeBatch(this.requests, ethers.ZeroAddress, { value: this.value }),
).to.be.revertedWithCustomError(this.forwarder, 'ERC2771ForwarderFailureInAtomicBatch');
});
});

describe('with tampered requests', function () {
Expand Down