From 38cee68c7c7e480d3e32d1fc5139434ae6bf9476 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 22:37:28 +0200 Subject: [PATCH] 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(); });