From bebc77d1030c0b1fe2468d8a5c0727a6d9880087 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 21 Oct 2024 16:13:35 +0200 Subject: [PATCH 1/9] Add semi-abstracted nonce variant --- .changeset/lovely-dodos-lay.md | 5 + contracts/utils/NoncesSemiAbstracted.sol | 69 ++++++++++ contracts/utils/README.adoc | 1 + test/utils/Nonces.behavior.js | 158 +++++++++++++++++++++++ test/utils/Nonces.test.js | 65 +--------- test/utils/NoncesSemiAbstracted.test.js | 17 +++ 6 files changed, 253 insertions(+), 62 deletions(-) create mode 100644 .changeset/lovely-dodos-lay.md create mode 100644 contracts/utils/NoncesSemiAbstracted.sol create mode 100644 test/utils/Nonces.behavior.js create mode 100644 test/utils/NoncesSemiAbstracted.test.js diff --git a/.changeset/lovely-dodos-lay.md b/.changeset/lovely-dodos-lay.md new file mode 100644 index 00000000000..4701219c3a9 --- /dev/null +++ b/.changeset/lovely-dodos-lay.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`NoncesSemiAbstracted`: Add a variant of `Nonces` that implements ERC-4337 semi-abstracted nonce system. diff --git a/contracts/utils/NoncesSemiAbstracted.sol b/contracts/utils/NoncesSemiAbstracted.sol new file mode 100644 index 00000000000..9f9910bb925 --- /dev/null +++ b/contracts/utils/NoncesSemiAbstracted.sol @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {Nonces} from "./Nonces.sol"; + +/** + * @dev Alternative to {Nonces}, that support key-ed nonces. + * + * Follows the https://eips.ethereum.org/EIPS/eip-4337#semi-abstracted-nonce-support[ERC-4337's semi-abstracted nonce system]. + */ +abstract contract NoncesSemiAbstracted is Nonces { + mapping(address owner => mapping(uint192 key => uint64)) private _nonce; + + /** + * @dev Returns the next unused nonce for an address. + */ + function nonces(address owner) public view virtual override returns (uint256) { + return nonces(owner, 0); + } + + /** + * @dev Returns the next unused nonce for an address and key. Result contains the key prefix. + */ + function nonces(address owner, uint192 key) public view virtual returns (uint256) { + return (uint256(key) << 64) | _nonce[owner][key]; + } + + /** + * @dev Consumes a nonce from the default key. + * + * Returns the current value and increments nonce. + */ + function _useNonce(address owner) internal virtual override returns (uint256) { + return _useNonce(owner, 0); + } + + /** + * @dev Consumes a nonce from the given key. + * + * Returns the current value and increments nonce. + */ + function _useNonce(address owner, uint192 key) internal virtual returns (uint256) { + // TODO: use unchecked here? Do we expect 2**64 nonce ever be used for a single owner? + return _nonce[owner][key]++; + } + + /** + * @dev Same as {_useNonce} but checking that `nonce` is the next valid for `owner`. + * + * This version takes a the key and the nonce in a single uint256 parameter: + * - use the first 8 bytes for the key + * - use the last 24 bytes for the nonce + */ + function _useCheckedNonce(address owner, uint256 keyNonce) internal virtual override { + _useCheckedNonce(owner, uint192(keyNonce >> 64), uint64(keyNonce)); + } + + /** + * @dev Same as {_useNonce} but checking that `nonce` is the next valid for `owner`. + * + * This version takes a the key and the nonce as two different parameters. + */ + function _useCheckedNonce(address owner, uint192 key, uint64 nonce) internal virtual { + uint256 current = _useNonce(owner, key); + if (nonce != current) { + revert InvalidAccountNonce(owner, current); + } + } +} diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index eeef84aae7c..ccbdebb3a28 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -18,6 +18,7 @@ Miscellaneous contracts and libraries containing utility functions you can use t * {ReentrancyGuardTransient}: Variant of {ReentrancyGuard} that uses transient storage (https://eips.ethereum.org/EIPS/eip-1153[EIP-1153]). * {Pausable}: A common emergency response mechanism that can pause functionality while a remediation is pending. * {Nonces}: Utility for tracking and verifying address nonces that only increment. + * {NoncesSemiAbstracted}: Alternative to {Nonces}, that support key-ed nonces following https://eips.ethereum.org/EIPS/eip-4337#semi-abstracted-nonce-support[ERC-4337 speciciations]. * {ERC165}, {ERC165Checker}: Utilities for inspecting interfaces supported by contracts. * {BitMaps}: A simple library to manage boolean value mapped to a numerical index in an efficient way. * {EnumerableMap}: A type like Solidity's https://solidity.readthedocs.io/en/latest/types.html#mapping-types[`mapping`], but with key-value _enumeration_: this will let you know how many entries a mapping has, and iterate over them (which is not possible with `mapping`). diff --git a/test/utils/Nonces.behavior.js b/test/utils/Nonces.behavior.js new file mode 100644 index 00000000000..38f1bf42963 --- /dev/null +++ b/test/utils/Nonces.behavior.js @@ -0,0 +1,158 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); + +function shouldBehaveLikeNonces() { + describe('should behave like Nonces', function () { + const sender = ethers.Wallet.createRandom(); + const other = ethers.Wallet.createRandom(); + + it('gets a nonce', async function () { + expect(await this.mock.nonces(sender)).to.equal(0n); + }); + + describe('_useNonce', function () { + it('increments a nonce', async function () { + expect(await this.mock.nonces(sender)).to.equal(0n); + + const eventName = ['return$_useNonce', 'return$_useNonce_address'].find(name => + this.mock.interface.getEvent(name), + ); + + await expect(await this.mock.$_useNonce(sender)) + .to.emit(this.mock, eventName) + .withArgs(0n); + + expect(await this.mock.nonces(sender)).to.equal(1n); + }); + + it("increments only sender's nonce", async function () { + expect(await this.mock.nonces(sender)).to.equal(0n); + expect(await this.mock.nonces(other)).to.equal(0n); + + await this.mock.$_useNonce(sender); + + expect(await this.mock.nonces(sender)).to.equal(1n); + expect(await this.mock.nonces(other)).to.equal(0n); + }); + }); + + describe('_useCheckedNonce', function () { + it('increments a nonce', async function () { + const currentNonce = await this.mock.nonces(sender); + + expect(currentNonce).to.equal(0n); + + await this.mock.$_useCheckedNonce(sender, currentNonce); + + expect(await this.mock.nonces(sender)).to.equal(1n); + }); + + it("increments only sender's nonce", async function () { + const currentNonce = await this.mock.nonces(sender); + + expect(currentNonce).to.equal(0n); + expect(await this.mock.nonces(other)).to.equal(0n); + + await this.mock.$_useCheckedNonce(sender, currentNonce); + + expect(await this.mock.nonces(sender)).to.equal(1n); + expect(await this.mock.nonces(other)).to.equal(0n); + }); + + it('reverts when nonce is not the expected', async function () { + const currentNonce = await this.mock.nonces(sender); + + await expect(this.mock.$_useCheckedNonce(sender, currentNonce + 1n)) + .to.be.revertedWithCustomError(this.mock, 'InvalidAccountNonce') + .withArgs(sender, currentNonce); + }); + }); + }); +} + +function shouldBehaveLikeNoncesSemiAbstracted() { + describe("should implement ERC-4337's semi-abstracted nonces", function () { + const sender = ethers.Wallet.createRandom(); + + const keyOffset = key => key << 64n; + + it('gets a nonce', async function () { + expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(keyOffset(0n) + 0n); + expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(keyOffset(17n) + 0n); + }); + + describe('_useNonce', function () { + it('default variant uses key 0', async function () { + expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(keyOffset(0n) + 0n); + expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(keyOffset(17n) + 0n); + + await expect(await this.mock.$_useNonce(sender)) + .to.emit(this.mock, 'return$_useNonce_address') + .withArgs(0n); + + await expect(await this.mock.$_useNonce(sender, ethers.Typed.uint192(0n))) + .to.emit(this.mock, 'return$_useNonce_address_uint192') + .withArgs(1n); + + expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(keyOffset(0n) + 2n); + expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(keyOffset(17n) + 0n); + }); + + it('use nonce at another key', async function () { + expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(keyOffset(0n) + 0n); + expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(keyOffset(17n) + 0n); + + await expect(await this.mock.$_useNonce(sender, ethers.Typed.uint192(17n))) + .to.emit(this.mock, 'return$_useNonce_address_uint192') + .withArgs(0n); + + await expect(await this.mock.$_useNonce(sender, ethers.Typed.uint192(17n))) + .to.emit(this.mock, 'return$_useNonce_address_uint192') + .withArgs(1n); + + expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(keyOffset(0n) + 0n); + expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(keyOffset(17n) + 2n); + }); + }); + + describe('_useCheckedNonce', function () { + it('default variant uses key 0', async function () { + const currentNonce = await this.mock.nonces(sender, ethers.Typed.uint192(0n)); + + await this.mock.$_useCheckedNonce(sender, currentNonce); + + expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(currentNonce + 1n); + }); + + it('use nonce at another key', async function () { + const currentNonce = await this.mock.nonces(sender, ethers.Typed.uint192(17n)); + + await this.mock.$_useCheckedNonce(sender, currentNonce); + + expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(currentNonce + 1n); + }); + + it('reverts when nonce is not the expected', async function () { + const currentNonce = await this.mock.nonces(sender, ethers.Typed.uint192(42n)); + + // use and increment + await this.mock.$_useCheckedNonce(sender, currentNonce); + + // reuse same nonce + await expect(this.mock.$_useCheckedNonce(sender, currentNonce)) + .to.be.revertedWithCustomError(this.mock, 'InvalidAccountNonce') + .withArgs(sender, 1); + + // use "future" nonce too early + await expect(this.mock.$_useCheckedNonce(sender, currentNonce + 10n)) + .to.be.revertedWithCustomError(this.mock, 'InvalidAccountNonce') + .withArgs(sender, 1); + }); + }); + }); +} + +module.exports = { + shouldBehaveLikeNonces, + shouldBehaveLikeNoncesSemiAbstracted, +}; diff --git a/test/utils/Nonces.test.js b/test/utils/Nonces.test.js index 2cb4798dea6..85aa7358a00 100644 --- a/test/utils/Nonces.test.js +++ b/test/utils/Nonces.test.js @@ -1,13 +1,10 @@ const { ethers } = require('hardhat'); -const { expect } = require('chai'); const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { shouldBehaveLikeNonces } = require('./Nonces.behavior'); async function fixture() { - const [sender, other] = await ethers.getSigners(); - const mock = await ethers.deployContract('$Nonces'); - - return { sender, other, mock }; + return { mock }; } describe('Nonces', function () { @@ -15,61 +12,5 @@ describe('Nonces', function () { Object.assign(this, await loadFixture(fixture)); }); - it('gets a nonce', async function () { - expect(await this.mock.nonces(this.sender)).to.equal(0n); - }); - - describe('_useNonce', function () { - it('increments a nonce', async function () { - expect(await this.mock.nonces(this.sender)).to.equal(0n); - - await expect(await this.mock.$_useNonce(this.sender)) - .to.emit(this.mock, 'return$_useNonce') - .withArgs(0n); - - expect(await this.mock.nonces(this.sender)).to.equal(1n); - }); - - it("increments only sender's nonce", async function () { - expect(await this.mock.nonces(this.sender)).to.equal(0n); - expect(await this.mock.nonces(this.other)).to.equal(0n); - - await this.mock.$_useNonce(this.sender); - - expect(await this.mock.nonces(this.sender)).to.equal(1n); - expect(await this.mock.nonces(this.other)).to.equal(0n); - }); - }); - - describe('_useCheckedNonce', function () { - it('increments a nonce', async function () { - const currentNonce = await this.mock.nonces(this.sender); - - expect(currentNonce).to.equal(0n); - - await this.mock.$_useCheckedNonce(this.sender, currentNonce); - - expect(await this.mock.nonces(this.sender)).to.equal(1n); - }); - - it("increments only sender's nonce", async function () { - const currentNonce = await this.mock.nonces(this.sender); - - expect(currentNonce).to.equal(0n); - expect(await this.mock.nonces(this.other)).to.equal(0n); - - await this.mock.$_useCheckedNonce(this.sender, currentNonce); - - expect(await this.mock.nonces(this.sender)).to.equal(1n); - expect(await this.mock.nonces(this.other)).to.equal(0n); - }); - - it('reverts when nonce is not the expected', async function () { - const currentNonce = await this.mock.nonces(this.sender); - - await expect(this.mock.$_useCheckedNonce(this.sender, currentNonce + 1n)) - .to.be.revertedWithCustomError(this.mock, 'InvalidAccountNonce') - .withArgs(this.sender, currentNonce); - }); - }); + shouldBehaveLikeNonces(); }); diff --git a/test/utils/NoncesSemiAbstracted.test.js b/test/utils/NoncesSemiAbstracted.test.js new file mode 100644 index 00000000000..fb1d468f8b6 --- /dev/null +++ b/test/utils/NoncesSemiAbstracted.test.js @@ -0,0 +1,17 @@ +const { ethers } = require('hardhat'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { shouldBehaveLikeNonces, shouldBehaveLikeNoncesSemiAbstracted } = require('./Nonces.behavior'); + +async function fixture() { + const mock = await ethers.deployContract('$NoncesSemiAbstracted'); + return { mock }; +} + +describe('NoncesSemiAbstracted', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + shouldBehaveLikeNonces(); + shouldBehaveLikeNoncesSemiAbstracted(); +}); From 5d92abceb0a3ef930c94b2d486366645f23f30de Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 21 Oct 2024 16:20:52 +0200 Subject: [PATCH 2/9] Update README.adoc --- contracts/utils/README.adoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index ccbdebb3a28..60b86ab6394 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -86,6 +86,8 @@ Because Solidity does not support generic types, {EnumerableMap} and {Enumerable {{Nonces}} +{{NoncesSemiAbstracted}} + == Introspection This set of interfaces and contracts deal with https://en.wikipedia.org/wiki/Type_introspection[type introspection] of contracts, that is, examining which functions can be called on them. This is usually referred to as a contract's _interface_. From dd82bbdd7e0c3f9c45f417e4906fe140a093f646 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 10:44:03 +0200 Subject: [PATCH 3/9] update --- contracts/utils/NoncesSemiAbstracted.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/utils/NoncesSemiAbstracted.sol b/contracts/utils/NoncesSemiAbstracted.sol index 9f9910bb925..b4f5941fb51 100644 --- a/contracts/utils/NoncesSemiAbstracted.sol +++ b/contracts/utils/NoncesSemiAbstracted.sol @@ -9,7 +9,7 @@ import {Nonces} from "./Nonces.sol"; * Follows the https://eips.ethereum.org/EIPS/eip-4337#semi-abstracted-nonce-support[ERC-4337's semi-abstracted nonce system]. */ abstract contract NoncesSemiAbstracted is Nonces { - mapping(address owner => mapping(uint192 key => uint64)) private _nonce; + mapping(address owner => mapping(uint192 key => uint64)) private _nounces; /** * @dev Returns the next unused nonce for an address. @@ -22,7 +22,7 @@ abstract contract NoncesSemiAbstracted is Nonces { * @dev Returns the next unused nonce for an address and key. Result contains the key prefix. */ function nonces(address owner, uint192 key) public view virtual returns (uint256) { - return (uint256(key) << 64) | _nonce[owner][key]; + return (uint256(key) << 64) | _nounces[owner][key]; } /** @@ -41,13 +41,13 @@ abstract contract NoncesSemiAbstracted is Nonces { */ function _useNonce(address owner, uint192 key) internal virtual returns (uint256) { // TODO: use unchecked here? Do we expect 2**64 nonce ever be used for a single owner? - return _nonce[owner][key]++; + return _nounces[owner][key]++; } /** * @dev Same as {_useNonce} but checking that `nonce` is the next valid for `owner`. * - * This version takes a the key and the nonce in a single uint256 parameter: + * This version takes the key and the nonce in a single uint256 parameter: * - use the first 8 bytes for the key * - use the last 24 bytes for the nonce */ @@ -58,7 +58,7 @@ abstract contract NoncesSemiAbstracted is Nonces { /** * @dev Same as {_useNonce} but checking that `nonce` is the next valid for `owner`. * - * This version takes a the key and the nonce as two different parameters. + * This version takes the key and the nonce as two different parameters. */ function _useCheckedNonce(address owner, uint192 key, uint64 nonce) internal virtual { uint256 current = _useNonce(owner, key); From 0cd63aa61f3a7ab7483b904d81a9d195a2c42213 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 14:29:18 +0200 Subject: [PATCH 4/9] cleanup tests --- test/utils/Nonces.behavior.js | 74 ++++++++++++++++------------------- 1 file changed, 34 insertions(+), 40 deletions(-) diff --git a/test/utils/Nonces.behavior.js b/test/utils/Nonces.behavior.js index 38f1bf42963..bb2b5c4131a 100644 --- a/test/utils/Nonces.behavior.js +++ b/test/utils/Nonces.behavior.js @@ -7,56 +7,52 @@ function shouldBehaveLikeNonces() { const other = ethers.Wallet.createRandom(); it('gets a nonce', async function () { - expect(await this.mock.nonces(sender)).to.equal(0n); + expect(this.mock.nonces(sender)).to.eventually.equal(0n); }); describe('_useNonce', function () { it('increments a nonce', async function () { - expect(await this.mock.nonces(sender)).to.equal(0n); + expect(this.mock.nonces(sender)).to.eventually.equal(0n); const eventName = ['return$_useNonce', 'return$_useNonce_address'].find(name => this.mock.interface.getEvent(name), ); - await expect(await this.mock.$_useNonce(sender)) - .to.emit(this.mock, eventName) - .withArgs(0n); + await expect(this.mock.$_useNonce(sender)).to.emit(this.mock, eventName).withArgs(0n); - expect(await this.mock.nonces(sender)).to.equal(1n); + expect(this.mock.nonces(sender)).to.eventually.equal(1n); }); it("increments only sender's nonce", async function () { - expect(await this.mock.nonces(sender)).to.equal(0n); - expect(await this.mock.nonces(other)).to.equal(0n); + expect(this.mock.nonces(sender)).to.eventually.equal(0n); + expect(this.mock.nonces(other)).to.eventually.equal(0n); await this.mock.$_useNonce(sender); - expect(await this.mock.nonces(sender)).to.equal(1n); - expect(await this.mock.nonces(other)).to.equal(0n); + expect(this.mock.nonces(sender)).to.eventually.equal(1n); + expect(this.mock.nonces(other)).to.eventually.equal(0n); }); }); describe('_useCheckedNonce', function () { it('increments a nonce', async function () { - const currentNonce = await this.mock.nonces(sender); + // current nonce is 0n + expect(this.mock.nonces(sender)).to.eventually.equal(0n); - expect(currentNonce).to.equal(0n); - - await this.mock.$_useCheckedNonce(sender, currentNonce); + await this.mock.$_useCheckedNonce(sender, 0n); - expect(await this.mock.nonces(sender)).to.equal(1n); + expect(this.mock.nonces(sender)).to.eventually.equal(1n); }); it("increments only sender's nonce", async function () { - const currentNonce = await this.mock.nonces(sender); - - expect(currentNonce).to.equal(0n); - expect(await this.mock.nonces(other)).to.equal(0n); + // current nonce is 0n + expect(this.mock.nonces(sender)).to.eventually.equal(0n); + expect(this.mock.nonces(other)).to.eventually.equal(0n); - await this.mock.$_useCheckedNonce(sender, currentNonce); + await this.mock.$_useCheckedNonce(sender, 0n); - expect(await this.mock.nonces(sender)).to.equal(1n); - expect(await this.mock.nonces(other)).to.equal(0n); + expect(this.mock.nonces(sender)).to.eventually.equal(1n); + expect(this.mock.nonces(other)).to.eventually.equal(0n); }); it('reverts when nonce is not the expected', async function () { @@ -77,41 +73,39 @@ function shouldBehaveLikeNoncesSemiAbstracted() { const keyOffset = key => key << 64n; it('gets a nonce', async function () { - expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(keyOffset(0n) + 0n); - expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(keyOffset(17n) + 0n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.eventually.equal(keyOffset(0n) + 0n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.eventually.equal(keyOffset(17n) + 0n); }); describe('_useNonce', function () { it('default variant uses key 0', async function () { - expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(keyOffset(0n) + 0n); - expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(keyOffset(17n) + 0n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.eventually.equal(keyOffset(0n) + 0n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.eventually.equal(keyOffset(17n) + 0n); - await expect(await this.mock.$_useNonce(sender)) - .to.emit(this.mock, 'return$_useNonce_address') - .withArgs(0n); + await expect(this.mock.$_useNonce(sender)).to.emit(this.mock, 'return$_useNonce_address').withArgs(0n); - await expect(await this.mock.$_useNonce(sender, ethers.Typed.uint192(0n))) + await expect(this.mock.$_useNonce(sender, ethers.Typed.uint192(0n))) .to.emit(this.mock, 'return$_useNonce_address_uint192') .withArgs(1n); - expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(keyOffset(0n) + 2n); - expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(keyOffset(17n) + 0n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.eventually.equal(keyOffset(0n) + 2n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.eventually.equal(keyOffset(17n) + 0n); }); it('use nonce at another key', async function () { - expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(keyOffset(0n) + 0n); - expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(keyOffset(17n) + 0n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.eventually.equal(keyOffset(0n) + 0n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.eventually.equal(keyOffset(17n) + 0n); - await expect(await this.mock.$_useNonce(sender, ethers.Typed.uint192(17n))) + await expect(this.mock.$_useNonce(sender, ethers.Typed.uint192(17n))) .to.emit(this.mock, 'return$_useNonce_address_uint192') .withArgs(0n); - await expect(await this.mock.$_useNonce(sender, ethers.Typed.uint192(17n))) + await expect(this.mock.$_useNonce(sender, ethers.Typed.uint192(17n))) .to.emit(this.mock, 'return$_useNonce_address_uint192') .withArgs(1n); - expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(keyOffset(0n) + 0n); - expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(keyOffset(17n) + 2n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.eventually.equal(keyOffset(0n) + 0n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.eventually.equal(keyOffset(17n) + 2n); }); }); @@ -121,7 +115,7 @@ function shouldBehaveLikeNoncesSemiAbstracted() { await this.mock.$_useCheckedNonce(sender, currentNonce); - expect(await this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.equal(currentNonce + 1n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(0n))).to.eventually.equal(currentNonce + 1n); }); it('use nonce at another key', async function () { @@ -129,7 +123,7 @@ function shouldBehaveLikeNoncesSemiAbstracted() { await this.mock.$_useCheckedNonce(sender, currentNonce); - expect(await this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.equal(currentNonce + 1n); + expect(this.mock.nonces(sender, ethers.Typed.uint192(17n))).to.eventually.equal(currentNonce + 1n); }); it('reverts when nonce is not the expected', async function () { From d9ea82e6e9f068bf6123d96fcac5c0491c974b87 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 18:32:45 +0200 Subject: [PATCH 5/9] rename --- contracts/utils/NoncesSemiAbstracted.sol | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/contracts/utils/NoncesSemiAbstracted.sol b/contracts/utils/NoncesSemiAbstracted.sol index b4f5941fb51..1f5ab08d949 100644 --- a/contracts/utils/NoncesSemiAbstracted.sol +++ b/contracts/utils/NoncesSemiAbstracted.sol @@ -9,7 +9,7 @@ import {Nonces} from "./Nonces.sol"; * Follows the https://eips.ethereum.org/EIPS/eip-4337#semi-abstracted-nonce-support[ERC-4337's semi-abstracted nonce system]. */ abstract contract NoncesSemiAbstracted is Nonces { - mapping(address owner => mapping(uint192 key => uint64)) private _nounces; + mapping(address owner => mapping(uint192 key => uint64)) private _nonces; /** * @dev Returns the next unused nonce for an address. @@ -22,7 +22,7 @@ abstract contract NoncesSemiAbstracted is Nonces { * @dev Returns the next unused nonce for an address and key. Result contains the key prefix. */ function nonces(address owner, uint192 key) public view virtual returns (uint256) { - return (uint256(key) << 64) | _nounces[owner][key]; + return (uint256(key) << 64) | _nonces[owner][key]; } /** @@ -40,8 +40,12 @@ abstract contract NoncesSemiAbstracted is Nonces { * Returns the current value and increments nonce. */ function _useNonce(address owner, uint192 key) internal virtual returns (uint256) { - // TODO: use unchecked here? Do we expect 2**64 nonce ever be used for a single owner? - return _nounces[owner][key]++; + // For each account, the nonce has an initial value of 0, can only be incremented by one, and cannot be + // decremented or reset. This guarantees that the nonce never overflows. + unchecked { + // It is important to do x++ and not ++x here. + return _nonces[owner][key]++; + } } /** From 38cee68c7c7e480d3e32d1fc5139434ae6bf9476 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 22:37:28 +0200 Subject: [PATCH 6/9] rewrite as a proper override to Nonces (that calls super) --- contracts/mocks/Stateless.sol | 2 + ...ncesSemiAbstracted.sol => NoncesKeyed.sol} | 41 +++++++------------ contracts/utils/README.adoc | 4 +- test/utils/Nonces.behavior.js | 6 +-- ...Abstracted.test.js => NoncesKeyed.test.js} | 8 ++-- 5 files changed, 25 insertions(+), 36 deletions(-) rename contracts/utils/{NoncesSemiAbstracted.sol => NoncesKeyed.sol} (62%) rename test/utils/{NoncesSemiAbstracted.test.js => NoncesKeyed.test.js} (52%) diff --git a/contracts/mocks/Stateless.sol b/contracts/mocks/Stateless.sol index 98e7eaf7443..5af11310bef 100644 --- a/contracts/mocks/Stateless.sol +++ b/contracts/mocks/Stateless.sol @@ -29,6 +29,8 @@ import {Heap} from "../utils/structs/Heap.sol"; import {Math} from "../utils/math/Math.sol"; import {MerkleProof} from "../utils/cryptography/MerkleProof.sol"; import {MessageHashUtils} from "../utils/cryptography/MessageHashUtils.sol"; +import {Nonces} from "../utils/Nonces.sol"; +import {NoncesKeyed} from "../utils/NoncesKeyed.sol"; import {P256} from "../utils/cryptography/P256.sol"; import {Panic} from "../utils/Panic.sol"; import {Packing} from "../utils/Packing.sol"; diff --git a/contracts/utils/NoncesSemiAbstracted.sol b/contracts/utils/NoncesKeyed.sol similarity index 62% rename from contracts/utils/NoncesSemiAbstracted.sol rename to contracts/utils/NoncesKeyed.sol index 1f5ab08d949..f8d936e7070 100644 --- a/contracts/utils/NoncesSemiAbstracted.sol +++ b/contracts/utils/NoncesKeyed.sol @@ -8,43 +8,26 @@ import {Nonces} from "./Nonces.sol"; * * Follows the https://eips.ethereum.org/EIPS/eip-4337#semi-abstracted-nonce-support[ERC-4337's semi-abstracted nonce system]. */ -abstract contract NoncesSemiAbstracted is Nonces { +abstract contract NoncesKeyed is Nonces { mapping(address owner => mapping(uint192 key => uint64)) private _nonces; - /** - * @dev Returns the next unused nonce for an address. - */ - function nonces(address owner) public view virtual override returns (uint256) { - return nonces(owner, 0); - } - - /** - * @dev Returns the next unused nonce for an address and key. Result contains the key prefix. - */ + /// @dev Returns the next unused nonce for an address and key. Result contains the key prefix. function nonces(address owner, uint192 key) public view virtual returns (uint256) { - return (uint256(key) << 64) | _nonces[owner][key]; - } - - /** - * @dev Consumes a nonce from the default key. - * - * Returns the current value and increments nonce. - */ - function _useNonce(address owner) internal virtual override returns (uint256) { - return _useNonce(owner, 0); + return key == 0 ? nonces(owner) : (uint256(key) << 64) | _nonces[owner][key]; } /** - * @dev Consumes a nonce from the given key. + * @dev Consumes the next unsused nonce for an address and key. * - * Returns the current value and increments nonce. + * Returns the current value without the key prefix. Consumed nonce is increased, so calling this functions twice + * with the same arguments will return different (sequential) results. */ function _useNonce(address owner, uint192 key) internal virtual returns (uint256) { // For each account, the nonce has an initial value of 0, can only be incremented by one, and cannot be // decremented or reset. This guarantees that the nonce never overflows. unchecked { // It is important to do x++ and not ++x here. - return _nonces[owner][key]++; + return key == 0 ? _useNonce(owner) : _nonces[owner][key]++; } } @@ -65,9 +48,13 @@ abstract contract NoncesSemiAbstracted is Nonces { * This version takes the key and the nonce as two different parameters. */ function _useCheckedNonce(address owner, uint192 key, uint64 nonce) internal virtual { - uint256 current = _useNonce(owner, key); - if (nonce != current) { - revert InvalidAccountNonce(owner, current); + if (key == 0) { + super._useCheckedNonce(owner, nonce); + } else { + uint256 current = _useNonce(owner, key); + if (nonce != current) { + revert InvalidAccountNonce(owner, current); + } } } } diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index 60b86ab6394..432b806e3c4 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -18,7 +18,7 @@ Miscellaneous contracts and libraries containing utility functions you can use t * {ReentrancyGuardTransient}: Variant of {ReentrancyGuard} that uses transient storage (https://eips.ethereum.org/EIPS/eip-1153[EIP-1153]). * {Pausable}: A common emergency response mechanism that can pause functionality while a remediation is pending. * {Nonces}: Utility for tracking and verifying address nonces that only increment. - * {NoncesSemiAbstracted}: Alternative to {Nonces}, that support key-ed nonces following https://eips.ethereum.org/EIPS/eip-4337#semi-abstracted-nonce-support[ERC-4337 speciciations]. + * {NoncesKeyed}: Alternative to {Nonces}, that support key-ed nonces following https://eips.ethereum.org/EIPS/eip-4337#semi-abstracted-nonce-support[ERC-4337 speciciations]. * {ERC165}, {ERC165Checker}: Utilities for inspecting interfaces supported by contracts. * {BitMaps}: A simple library to manage boolean value mapped to a numerical index in an efficient way. * {EnumerableMap}: A type like Solidity's https://solidity.readthedocs.io/en/latest/types.html#mapping-types[`mapping`], but with key-value _enumeration_: this will let you know how many entries a mapping has, and iterate over them (which is not possible with `mapping`). @@ -86,7 +86,7 @@ Because Solidity does not support generic types, {EnumerableMap} and {Enumerable {{Nonces}} -{{NoncesSemiAbstracted}} +{{NoncesKeyed}} == Introspection diff --git a/test/utils/Nonces.behavior.js b/test/utils/Nonces.behavior.js index bb2b5c4131a..17073966427 100644 --- a/test/utils/Nonces.behavior.js +++ b/test/utils/Nonces.behavior.js @@ -66,8 +66,8 @@ function shouldBehaveLikeNonces() { }); } -function shouldBehaveLikeNoncesSemiAbstracted() { - describe("should implement ERC-4337's semi-abstracted nonces", function () { +function shouldBehaveLikeNoncesKeyed() { + describe('should support nonces with keys', function () { const sender = ethers.Wallet.createRandom(); const keyOffset = key => key << 64n; @@ -148,5 +148,5 @@ function shouldBehaveLikeNoncesSemiAbstracted() { module.exports = { shouldBehaveLikeNonces, - shouldBehaveLikeNoncesSemiAbstracted, + shouldBehaveLikeNoncesKeyed, }; diff --git a/test/utils/NoncesSemiAbstracted.test.js b/test/utils/NoncesKeyed.test.js similarity index 52% rename from test/utils/NoncesSemiAbstracted.test.js rename to test/utils/NoncesKeyed.test.js index fb1d468f8b6..c46948ee402 100644 --- a/test/utils/NoncesSemiAbstracted.test.js +++ b/test/utils/NoncesKeyed.test.js @@ -1,17 +1,17 @@ const { ethers } = require('hardhat'); const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); -const { shouldBehaveLikeNonces, shouldBehaveLikeNoncesSemiAbstracted } = require('./Nonces.behavior'); +const { shouldBehaveLikeNonces, shouldBehaveLikeNoncesKeyed } = require('./Nonces.behavior'); async function fixture() { - const mock = await ethers.deployContract('$NoncesSemiAbstracted'); + const mock = await ethers.deployContract('$NoncesKeyed'); return { mock }; } -describe('NoncesSemiAbstracted', function () { +describe('NoncesKeyed', function () { beforeEach(async function () { Object.assign(this, await loadFixture(fixture)); }); shouldBehaveLikeNonces(); - shouldBehaveLikeNoncesSemiAbstracted(); + shouldBehaveLikeNoncesKeyed(); }); From bbaf337c0c883814ecfe5d215fe8ff084951893d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 22:42:44 +0200 Subject: [PATCH 7/9] Update lovely-dodos-lay.md --- .changeset/lovely-dodos-lay.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/lovely-dodos-lay.md b/.changeset/lovely-dodos-lay.md index 4701219c3a9..da225132630 100644 --- a/.changeset/lovely-dodos-lay.md +++ b/.changeset/lovely-dodos-lay.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -`NoncesSemiAbstracted`: Add a variant of `Nonces` that implements ERC-4337 semi-abstracted nonce system. +`NoncesKeyed`: Add a variant of `Nonces` that implements the ERC-4337 entrypoint nonce system. From c4a95c56ef95f82a97370a7c286d02ad88e2b0d1 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 22:43:39 +0200 Subject: [PATCH 8/9] Update NoncesKeyed.sol --- contracts/utils/NoncesKeyed.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/utils/NoncesKeyed.sol b/contracts/utils/NoncesKeyed.sol index f8d936e7070..80ac86129e7 100644 --- a/contracts/utils/NoncesKeyed.sol +++ b/contracts/utils/NoncesKeyed.sol @@ -17,7 +17,7 @@ abstract contract NoncesKeyed is Nonces { } /** - * @dev Consumes the next unsused nonce for an address and key. + * @dev Consumes the next unused nonce for an address and key. * * Returns the current value without the key prefix. Consumed nonce is increased, so calling this functions twice * with the same arguments will return different (sequential) results. From 86718f6a65e0e7a24cc51795f1904136b78e012b Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 22:44:50 +0200 Subject: [PATCH 9/9] add () for clarity --- contracts/utils/NoncesKeyed.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/utils/NoncesKeyed.sol b/contracts/utils/NoncesKeyed.sol index f8d936e7070..08a4f5300be 100644 --- a/contracts/utils/NoncesKeyed.sol +++ b/contracts/utils/NoncesKeyed.sol @@ -13,7 +13,7 @@ abstract contract NoncesKeyed is Nonces { /// @dev Returns the next unused nonce for an address and key. Result contains the key prefix. function nonces(address owner, uint192 key) public view virtual returns (uint256) { - return key == 0 ? nonces(owner) : (uint256(key) << 64) | _nonces[owner][key]; + return key == 0 ? nonces(owner) : ((uint256(key) << 64) | _nonces[owner][key]); } /**