From b38a294a7c35007610d0ce5498f053d2ee2f58e3 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 21 Oct 2024 14:47:26 +0200 Subject: [PATCH] AA-446: Separate 'nonce' into two 256-bit values 'nonceKey' and 'nonceSequence' (#2) * AA-446: Separate 'nonce' into two 256-bit values 'nonceKey' and 'nonceSequence' * Fix comment * Add comment describing the data format * Rename variables --- contracts/predeploys/NonceManager.sol | 51 +++++++++++++++------------ contracts/test/NonceManager.t.sol | 18 +++++----- yarn.lock | 49 +++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 31 deletions(-) diff --git a/contracts/predeploys/NonceManager.sol b/contracts/predeploys/NonceManager.sol index 00bc22a..74e36d2 100644 --- a/contracts/predeploys/NonceManager.sol +++ b/contracts/predeploys/NonceManager.sol @@ -11,6 +11,13 @@ contract NonceManager { /// @notice The EntryPoint address defined at RIP-7560. address internal constant AA_ENTRY_POINT = 0x0000000000000000000000000000000000007560; + /// @notice There are no public functions in the NonceManager and the action is determined by the caller address. + /// + /// In order to query the current 'nonceSequence' of an address for a given 'nonceKey' make a view call with data: + /// sender{20 bytes} nonceKey{32 bytes} + /// + /// In order to validate and increment the current 'nonceSequence' of an address AA_ENTRY_POINT calls it with data: + /// sender{20 bytes} nonceKey{32 bytes} nonceSequence{32 bytes} fallback(bytes calldata data) external returns (bytes memory) { if (msg.sender == AA_ENTRY_POINT) { _validateIncrement(data); @@ -20,53 +27,53 @@ contract NonceManager { } } - /// @notice Return the next nonce for this sender. Within a given key, the nonce values are sequenced + /// @notice Return the next nonce sequence for this sender. + /// @notice Within a given key, the nonce values are sequenced. /// (starting with zero, and incremented by one on each transaction). /// But transactions with different keys can come with arbitrary order. - /// @return nonce a full nonce to pass for next transaction with given sender and key. - function _get(bytes calldata /* data */) internal view returns (uint256 nonce) { + /// @return nonceSeq a nonce sequence value to pass for next transaction with given sender and key. + function _get(bytes calldata /* data */) internal view returns (uint256 nonceSeq) { assembly { - // Check if calldata is 44 bytes long - if iszero(eq(calldatasize(), 44)) { + // Check if calldata is 20+32 bytes long + if iszero(eq(calldatasize(), 52)) { mstore(0x00, 0x947d5a84) // 'InvalidLength()' revert(0x1c, 0x04) } let ptr := mload(0x40) - calldatacopy(ptr, 0, 44) + calldatacopy(ptr, 0, 52) - // Extract key and sender from calldata - let key := shr(64, mload(add(ptr, 20))) - mstore(0x00, key) - mstore(0x14, shr(96, mload(ptr))) + // Extract sender and key from calldata + mstore(0x00, shr(96, mload(ptr))) + mstore(0x14, mload(add(ptr, 20))) - // Load nonce from storage - nonce := or(shl(64, key), sload(keccak256(0x04, 0x30))) + // Load nonce sequence from storage + nonceSeq := sload(keccak256(0x00, 0x34)) } } - /// @notice validate nonce uniqueness for this account. Called by AA_ENTRY_POINT. + /// @notice validate nonce key and sequence uniqueness for this account. Called by AA_ENTRY_POINT. function _validateIncrement(bytes calldata /* data */) internal { assembly { let ptr := mload(0x40) calldatacopy(ptr, 0, calldatasize()) - // Store key and sender in memory - mstore(0x00, shr(64, mload(add(ptr, 20)))) - mstore(0x14, shr(96, mload(ptr))) + // Store sender and key in memory + mstore(0x00, shr(96, mload(ptr))) + mstore(0x14, mload(add(ptr, 20))) - // Calculate storage slot and load current nonce - let nonceSlot := keccak256(0x04, 0x30) - let currentNonce := sload(nonceSlot) + // Calculate storage slot and load current nonce sequence + let nonceSeqSlot := keccak256(0x00, 0x34) + let currentNonceSeq := sload(nonceSeqSlot) // Revert if nonce mismatch - if iszero(eq(shr(192, mload(add(ptr, 44))), currentNonce)) { + if iszero(eq(mload(add(ptr, 52)), currentNonceSeq)) { mstore(0, 0) revert(0, 0) // Revert if nonce mismatch } - // Increment nonce - sstore(nonceSlot, add(currentNonce, 1)) + // Increment nonce sequence + sstore(nonceSeqSlot, add(currentNonceSeq, 1)) } } } diff --git a/contracts/test/NonceManager.t.sol b/contracts/test/NonceManager.t.sol index f617032..7139ec8 100644 --- a/contracts/test/NonceManager.t.sol +++ b/contracts/test/NonceManager.t.sol @@ -8,21 +8,21 @@ import "../predeploys/NonceManager.sol"; contract NonceManagerTest is Test { address private constant aaEntryPoint = 0x0000000000000000000000000000000000007560; address private constant alice = 0x0000000000000000000000000000000000007777; - uint192 public aliceKey; + uint256 public aliceKey; error InvalidLength(); NonceManager public nonceManager; /// @dev Sets up the test suite. function setUp() public virtual { nonceManager = new NonceManager(); - aliceKey = uint192(3); + aliceKey = uint256(3); } /// @dev Tests that increasing nonce is done properly. function test_validateIncrement_succeeds() public { vm.prank(aaEntryPoint); (bool success, bytes memory returnData) = - address(nonceManager).call(abi.encodePacked(alice, aliceKey, uint64(0))); + address(nonceManager).call(abi.encodePacked(alice, aliceKey, uint256(0))); assertTrue(success); assertEq(returnData, new bytes(0)); } @@ -30,7 +30,7 @@ contract NonceManagerTest is Test { /// @dev Tests that increasing nonce with invalid nonce fails. function test_validateIncrement_invalidNonce_fails() external { vm.prank(aaEntryPoint); - (bool success,) = address(nonceManager).call(abi.encodePacked(alice, aliceKey, uint64(1))); + (bool success,) = address(nonceManager).call(abi.encodePacked(alice, aliceKey, uint256(1))); assertFalse(success); } @@ -38,19 +38,19 @@ contract NonceManagerTest is Test { function test_fallback_get_succeeds() external { (bool success, bytes memory returnData) = address(nonceManager).call(abi.encodePacked(alice, aliceKey)); assertTrue(success); - assertEq(returnData, abi.encodePacked(uint192(aliceKey), uint64(0))); + assertEq(returnData, abi.encodePacked(uint256(0))); test_validateIncrement_succeeds(); (success, returnData) = address(nonceManager).call(abi.encodePacked(alice, aliceKey)); assertTrue(success); - assertEq(returnData, abi.encodePacked(uint192(aliceKey), uint64(1))); + assertEq(returnData, abi.encodePacked(uint256(1))); } /// @dev Tests that getting nonce with invalid length fails. function test_fallback_get_invalidLength_fails() external { vm.expectRevert(InvalidLength.selector); - (bool success,) = address(nonceManager).call(abi.encodePacked(alice, aliceKey, uint64(1))); + (bool success,) = address(nonceManager).call(abi.encodePacked(alice, aliceKey, uint256(1))); (success); // silence unused variable warning } @@ -62,12 +62,12 @@ contract NonceManagerTest is Test { n = n % 1000; // limit the number of iterations to prevent OOG for (uint256 i = 0; i < n; i++) { vm.prank(aaEntryPoint); - (success,) = address(nonceManager).call(abi.encodePacked(alice, aliceKey, uint64(i))); + (success,) = address(nonceManager).call(abi.encodePacked(alice, aliceKey, uint256(i))); assertTrue(success); } (success, returnData) = address(nonceManager).call(abi.encodePacked(alice, aliceKey)); assertTrue(success); - assertEq(returnData, abi.encodePacked(uint192(aliceKey), uint64(n))); + assertEq(returnData, abi.encodePacked(uint256(n))); } } diff --git a/yarn.lock b/yarn.lock index 3b3116f..98b291d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,6 +2,14 @@ # yarn lockfile v1 +"@account-abstraction/contracts@^0.7.0": + version "0.7.0" + resolved "https://registry.yarnpkg.com/@account-abstraction/contracts/-/contracts-0.7.0.tgz#f0f60d73e8d6ef57b794b07f8ab7b4779a3814c6" + integrity sha512-Bt/66ilu3u8I9+vFZ9fTd+cWs55fdb9J5YKfrhsrFafH1drkzwuCSL/xEot1GGyXXNJLQuXbMRztQPyelNbY1A== + dependencies: + "@openzeppelin/contracts" "^5.0.0" + "@uniswap/v3-periphery" "^1.4.3" + "@adraffy/ens-normalize@1.10.1": version "1.10.1" resolved "https://registry.yarnpkg.com/@adraffy/ens-normalize/-/ens-normalize-1.10.1.tgz#63430d04bd8c5e74f8d7d049338f1cd9d4f02069" @@ -685,6 +693,16 @@ "@nomicfoundation/solidity-analyzer-linux-x64-musl" "0.1.2" "@nomicfoundation/solidity-analyzer-win32-x64-msvc" "0.1.2" +"@openzeppelin/contracts@3.4.2-solc-0.7": + version "3.4.2-solc-0.7" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-3.4.2-solc-0.7.tgz#38f4dbab672631034076ccdf2f3201fab1726635" + integrity sha512-W6QmqgkADuFcTLzHL8vVoNBtkwjvQRpYIAom7KiUNoLKghyx3FgH0GBjt8NRvigV1ZmMOBllvE1By1C+bi8WpA== + +"@openzeppelin/contracts@^5.0.0": + version "5.0.2" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-5.0.2.tgz#b1d03075e49290d06570b2fd42154d76c2a5d210" + integrity sha512-ytPc6eLGcHHnapAZ9S+5qsdomhjo6QBHTDRRBFfTxXIpsicMhVPouPgmUPebZZZGX7vt9USA+Z+0M0dSVtSUEA== + "@scure/base@~1.1.0", "@scure/base@~1.1.6": version "1.1.7" resolved "https://registry.yarnpkg.com/@scure/base/-/base-1.1.7.tgz#fe973311a5c6267846aa131bc72e96c5d40d2b30" @@ -948,6 +966,32 @@ dependencies: "@types/node" "*" +"@uniswap/lib@^4.0.1-alpha": + version "4.0.1-alpha" + resolved "https://registry.yarnpkg.com/@uniswap/lib/-/lib-4.0.1-alpha.tgz#2881008e55f075344675b3bca93f020b028fbd02" + integrity sha512-f6UIliwBbRsgVLxIaBANF6w09tYqc6Y/qXdsrbEmXHyFA7ILiKrIwRFXe1yOg8M3cksgVsO9N7yuL2DdCGQKBA== + +"@uniswap/v2-core@^1.0.1": + version "1.0.1" + resolved "https://registry.yarnpkg.com/@uniswap/v2-core/-/v2-core-1.0.1.tgz#af8f508bf183204779938969e2e54043e147d425" + integrity sha512-MtybtkUPSyysqLY2U210NBDeCHX+ltHt3oADGdjqoThZaFRDKwM6k1Nb3F0A3hk5hwuQvytFWhrWHOEq6nVJ8Q== + +"@uniswap/v3-core@^1.0.0": + version "1.0.1" + resolved "https://registry.yarnpkg.com/@uniswap/v3-core/-/v3-core-1.0.1.tgz#b6d2bdc6ba3c3fbd610bdc502395d86cd35264a0" + integrity sha512-7pVk4hEm00j9tc71Y9+ssYpO6ytkeI0y7WE9P6UcmNzhxPePwyAxImuhVsTqWK9YFvzgtvzJHi64pBl4jUzKMQ== + +"@uniswap/v3-periphery@^1.4.3": + version "1.4.4" + resolved "https://registry.yarnpkg.com/@uniswap/v3-periphery/-/v3-periphery-1.4.4.tgz#d2756c23b69718173c5874f37fd4ad57d2f021b7" + integrity sha512-S4+m+wh8HbWSO3DKk4LwUCPZJTpCugIsHrWR86m/OrUyvSqGDTXKFfc2sMuGXCZrD1ZqO3rhQsKgdWg3Hbb2Kw== + dependencies: + "@openzeppelin/contracts" "3.4.2-solc-0.7" + "@uniswap/lib" "^4.0.1-alpha" + "@uniswap/v2-core" "^1.0.1" + "@uniswap/v3-core" "^1.0.0" + base64-sol "1.0.1" + abbrev@1: version "1.1.1" resolved "https://registry.yarnpkg.com/abbrev/-/abbrev-1.1.1.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8" @@ -1159,6 +1203,11 @@ base-x@^3.0.2: dependencies: safe-buffer "^5.0.1" +base64-sol@1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/base64-sol/-/base64-sol-1.0.1.tgz#91317aa341f0bc763811783c5729f1c2574600f6" + integrity sha512-ld3cCNMeXt4uJXmLZBHFGMvVpK9KsLVEhPpFRXnvSVAqABKbuNZg/+dsq3NuM+wxFLb/UrVkz7m1ciWmkMfTbg== + bech32@1.1.4: version "1.1.4" resolved "https://registry.yarnpkg.com/bech32/-/bech32-1.1.4.tgz#e38c9f37bf179b8eb16ae3a772b40c356d4832e9"