From 1ebe0c2852c208e7c353ef4ab91d0d2bb55b17a1 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 16 Sep 2024 13:46:37 +0200 Subject: [PATCH] AA-446: Separate 'nonce' into two 256-bit values 'nonceKey' and 'nonceSequence' --- contracts/predeploys/NonceManager.sol | 23 ++++++------- contracts/test/NonceManager.t.sol | 18 +++++----- yarn.lock | 49 +++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 21 deletions(-) diff --git a/contracts/predeploys/NonceManager.sol b/contracts/predeploys/NonceManager.sol index 00bc22a..eed1d40 100644 --- a/contracts/predeploys/NonceManager.sol +++ b/contracts/predeploys/NonceManager.sol @@ -27,21 +27,20 @@ contract NonceManager { function _get(bytes calldata /* data */) internal view returns (uint256 nonce) { assembly { // Check if calldata is 44 bytes long - if iszero(eq(calldatasize(), 44)) { + 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))) + nonce := sload(keccak256(0x00, 0x34)) } } @@ -51,16 +50,16 @@ contract NonceManager { 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 nonceSlot := keccak256(0x00, 0x34) let currentNonce := sload(nonceSlot) // Revert if nonce mismatch - if iszero(eq(shr(192, mload(add(ptr, 44))), currentNonce)) { + if iszero(eq(mload(add(ptr, 52)), currentNonce)) { mstore(0, 0) revert(0, 0) // Revert if nonce mismatch } 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"