From 64402315603d9470b39744375aaac714c0ee1a55 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Wed, 9 Nov 2022 14:03:14 -0500 Subject: [PATCH 1/3] bitwise optimizations --- ...permitBatchTransferFromMultipleTokens.snap | 2 +- .../permitBatchTransferFromSingleToken.snap | 2 +- .../permitTransferFromBatchTypedWitness.snap | 2 +- .../permitTransferFromCompactSig.snap | 2 +- .../permitTransferFromSingleToken.snap | 2 +- .../permitTransferFromTypedWitness.snap | 2 +- .../single recipient 2 tokens.snap | 2 +- .../single recipient many tokens.snap | 2 +- .gas-snapshot | 62 +++++++++---------- src/SignatureTransfer.sol | 13 ++-- test/NonceBitmap.t.sol | 6 ++ 11 files changed, 53 insertions(+), 44 deletions(-) diff --git a/.forge-snapshots/permitBatchTransferFromMultipleTokens.snap b/.forge-snapshots/permitBatchTransferFromMultipleTokens.snap index 27ca5b34..6c70c779 100644 --- a/.forge-snapshots/permitBatchTransferFromMultipleTokens.snap +++ b/.forge-snapshots/permitBatchTransferFromMultipleTokens.snap @@ -1 +1 @@ -143500 \ No newline at end of file +143347 \ No newline at end of file diff --git a/.forge-snapshots/permitBatchTransferFromSingleToken.snap b/.forge-snapshots/permitBatchTransferFromSingleToken.snap index 6977b61a..d2f1fc26 100644 --- a/.forge-snapshots/permitBatchTransferFromSingleToken.snap +++ b/.forge-snapshots/permitBatchTransferFromSingleToken.snap @@ -1 +1 @@ -88950 \ No newline at end of file +88797 \ No newline at end of file diff --git a/.forge-snapshots/permitTransferFromBatchTypedWitness.snap b/.forge-snapshots/permitTransferFromBatchTypedWitness.snap index 33581732..d1d7ff6c 100644 --- a/.forge-snapshots/permitTransferFromBatchTypedWitness.snap +++ b/.forge-snapshots/permitTransferFromBatchTypedWitness.snap @@ -1 +1 @@ -121317 \ No newline at end of file +121164 \ No newline at end of file diff --git a/.forge-snapshots/permitTransferFromCompactSig.snap b/.forge-snapshots/permitTransferFromCompactSig.snap index 122e4017..adf64bd3 100644 --- a/.forge-snapshots/permitTransferFromCompactSig.snap +++ b/.forge-snapshots/permitTransferFromCompactSig.snap @@ -1 +1 @@ -88685 \ No newline at end of file +88532 \ No newline at end of file diff --git a/.forge-snapshots/permitTransferFromSingleToken.snap b/.forge-snapshots/permitTransferFromSingleToken.snap index 6c3f5018..d54c0afa 100644 --- a/.forge-snapshots/permitTransferFromSingleToken.snap +++ b/.forge-snapshots/permitTransferFromSingleToken.snap @@ -1 +1 @@ -88737 \ No newline at end of file +88584 \ No newline at end of file diff --git a/.forge-snapshots/permitTransferFromTypedWitness.snap b/.forge-snapshots/permitTransferFromTypedWitness.snap index 93cd094f..9e71284b 100644 --- a/.forge-snapshots/permitTransferFromTypedWitness.snap +++ b/.forge-snapshots/permitTransferFromTypedWitness.snap @@ -1 +1 @@ -91157 \ No newline at end of file +91004 \ No newline at end of file diff --git a/.forge-snapshots/single recipient 2 tokens.snap b/.forge-snapshots/single recipient 2 tokens.snap index effa30f9..991dab52 100644 --- a/.forge-snapshots/single recipient 2 tokens.snap +++ b/.forge-snapshots/single recipient 2 tokens.snap @@ -1 +1 @@ -118623 \ No newline at end of file +118470 \ No newline at end of file diff --git a/.forge-snapshots/single recipient many tokens.snap b/.forge-snapshots/single recipient many tokens.snap index 9493e70d..06e1e14e 100644 --- a/.forge-snapshots/single recipient many tokens.snap +++ b/.forge-snapshots/single recipient many tokens.snap @@ -1 +1 @@ -133765 \ No newline at end of file +133612 \ No newline at end of file diff --git a/.gas-snapshot b/.gas-snapshot index 02a741cf..a61ae5e1 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -28,13 +28,13 @@ CompactSignature:testCompactSignature27() (gas: 253) CompactSignature:testCompactSignature28() (gas: 141) EIP712Test:testDomainSeparator() (gas: 5759) EIP712Test:testDomainSeparatorAfterFork() (gas: 10652) -NonceBitmapTest:testHighNonces() (gas: 36088) -NonceBitmapTest:testInvalidateFullWord() (gas: 62606) -NonceBitmapTest:testInvalidateNonzeroWord() (gas: 85721) -NonceBitmapTest:testLowNonces() (gas: 41019) -NonceBitmapTest:testNonceWordBoundary() (gas: 57948) -NonceBitmapTest:testUseTwoRandomNonces(uint256,uint256) (runs: 256, μ: 49304, ~: 51829) -NonceBitmapTest:testUsingNonceTwiceFails(uint256) (runs: 256, μ: 32616, ~: 32616) +NonceBitmapTest:testHighNonces() (gas: 35958) +NonceBitmapTest:testInvalidateFullWord() (gas: 62805) +NonceBitmapTest:testInvalidateNonzeroWord() (gas: 85285) +NonceBitmapTest:testLowNonces() (gas: 40671) +NonceBitmapTest:testNonceWordBoundary() (gas: 42042) +NonceBitmapTest:testUseTwoRandomNonces(uint256,uint256) (runs: 256, μ: 48832, ~: 51523) +NonceBitmapTest:testUsingNonceTwiceFails(uint256) (runs: 256, μ: 21775, ~: 21798) Permit2LibTest:testOZSafePermit() (gas: 24311) Permit2LibTest:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129066) Permit2LibTest:testOZSafeTransferFrom() (gas: 38787) @@ -50,28 +50,28 @@ Permit2LibTest:testTransferFrom2() (gas: 38404) Permit2LibTest:testTransferFrom2Full() (gas: 53437) Permit2LibTest:testTransferFrom2InvalidAmount() (gas: 12519) Permit2LibTest:testTransferFrom2NonPermitToken() (gas: 53305) -SignatureTransferTest:testGasMultiplePermitBatchTransferFrom() (gas: 270959) -SignatureTransferTest:testGasSinglePermitBatchTransferFrom() (gas: 186309) -SignatureTransferTest:testGasSinglePermitTransferFrom() (gas: 124095) -SignatureTransferTest:testInvalidateUnorderedNonces() (gas: 56540) -SignatureTransferTest:testPermitBatchTransferFrom() (gas: 162076) -SignatureTransferTest:testPermitBatchTransferFromSingleRecipient() (gas: 190355) -SignatureTransferTest:testPermitBatchTransferFromTypedWitness() (gas: 242493) -SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidType() (gas: 87340) -SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidTypeHash() (gas: 86803) -SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidTypeName() (gas: 88276) -SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidWitness() (gas: 88321) +SignatureTransferTest:testGasMultiplePermitBatchTransferFrom() (gas: 270806) +SignatureTransferTest:testGasSinglePermitBatchTransferFrom() (gas: 186156) +SignatureTransferTest:testGasSinglePermitTransferFrom() (gas: 123942) +SignatureTransferTest:testInvalidateUnorderedNonces() (gas: 41090) +SignatureTransferTest:testPermitBatchTransferFrom() (gas: 161923) +SignatureTransferTest:testPermitBatchTransferFromSingleRecipient() (gas: 190202) +SignatureTransferTest:testPermitBatchTransferFromTypedWitness() (gas: 242340) +SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidType() (gas: 87187) +SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidTypeHash() (gas: 86650) +SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidTypeName() (gas: 88123) +SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidWitness() (gas: 88168) SignatureTransferTest:testPermitBatchTransferInvalidAmountsLengthMismatch() (gas: 43738) -SignatureTransferTest:testPermitBatchTransferMultiAddr() (gas: 160450) -SignatureTransferTest:testPermitBatchTransferSingleRecipientManyTokens() (gas: 211974) -SignatureTransferTest:testPermitTransferFrom() (gas: 93208) -SignatureTransferTest:testPermitTransferFromCompactSig() (gas: 124159) -SignatureTransferTest:testPermitTransferFromIncorrectSigLength() (gas: 51430) -SignatureTransferTest:testPermitTransferFromInvalidNonce() (gas: 93215) -SignatureTransferTest:testPermitTransferFromRandomNonceAndAmount(uint256,uint128) (runs: 256, μ: 95842, ~: 96818) -SignatureTransferTest:testPermitTransferFromToSpender() (gas: 93494) -SignatureTransferTest:testPermitTransferFromTypedWitness() (gas: 127964) -SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidType() (gas: 58668) -SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidTypeName() (gas: 59722) -SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidTypehash() (gas: 57658) -SignatureTransferTest:testPermitTransferSpendLessThanFull(uint256,uint128) (runs: 256, μ: 98177, ~: 99895) +SignatureTransferTest:testPermitBatchTransferMultiAddr() (gas: 160297) +SignatureTransferTest:testPermitBatchTransferSingleRecipientManyTokens() (gas: 211821) +SignatureTransferTest:testPermitTransferFrom() (gas: 93055) +SignatureTransferTest:testPermitTransferFromCompactSig() (gas: 124006) +SignatureTransferTest:testPermitTransferFromIncorrectSigLength() (gas: 51277) +SignatureTransferTest:testPermitTransferFromInvalidNonce() (gas: 73250) +SignatureTransferTest:testPermitTransferFromRandomNonceAndAmount(uint256,uint128) (runs: 256, μ: 95689, ~: 96665) +SignatureTransferTest:testPermitTransferFromToSpender() (gas: 93341) +SignatureTransferTest:testPermitTransferFromTypedWitness() (gas: 127811) +SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidType() (gas: 58515) +SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidTypeName() (gas: 59569) +SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidTypehash() (gas: 57505) +SignatureTransferTest:testPermitTransferSpendLessThanFull(uint256,uint128) (runs: 256, μ: 98024, ~: 99742) diff --git a/src/SignatureTransfer.sol b/src/SignatureTransfer.sol index c58529e6..9ffa1fda 100644 --- a/src/SignatureTransfer.sol +++ b/src/SignatureTransfer.sol @@ -9,6 +9,8 @@ import {SignatureVerification} from "./libraries/SignatureVerification.sol"; import {PermitHash} from "./libraries/PermitHash.sol"; import {EIP712} from "./EIP712.sol"; +import {console2} from "forge-std/console2.sol"; + contract SignatureTransfer is ISignatureTransfer, EIP712 { using SignatureVerification for bytes; using SafeTransferLib for ERC20; @@ -143,7 +145,7 @@ contract SignatureTransfer is ISignatureTransfer, EIP712 { /// @dev The last 8 bits of the nonce value is the position of the bit in the bitmap function bitmapPositions(uint256 nonce) private pure returns (uint256 wordPos, uint256 bitPos) { wordPos = uint248(nonce >> 8); - bitPos = uint8(nonce & 255); + bitPos = uint8(nonce); } /// @notice Checks whether a nonce is taken and sets the bit at the bit position in the bitmap at the word position @@ -151,10 +153,11 @@ contract SignatureTransfer is ISignatureTransfer, EIP712 { /// @param nonce The nonce to spend function _useUnorderedNonce(address from, uint256 nonce) internal { (uint256 wordPos, uint256 bitPos) = bitmapPositions(nonce); - uint256 bitmap = nonceBitmap[from][wordPos]; - - if ((bitmap >> bitPos) & 1 == 1) revert InvalidNonce(); + uint256 bit = 1 << bitPos; + uint256 flipped = nonceBitmap[from][wordPos] ^= bit; - nonceBitmap[from][wordPos] = bitmap | (1 << bitPos); + console2.log(bit); + console2.log(flipped); + if (flipped & bit == 0) revert InvalidNonce(); } } diff --git a/test/NonceBitmap.t.sol b/test/NonceBitmap.t.sol index 55713750..891cd63a 100644 --- a/test/NonceBitmap.t.sol +++ b/test/NonceBitmap.t.sol @@ -13,6 +13,12 @@ contract NonceBitmapTest is Test { permit2 = new MockPermit2(); } + function testNonceLLL() public { + permit2.useUnorderedNonce(address(this), 5); + vm.expectRevert(InvalidNonce.selector); + permit2.useUnorderedNonce(address(this), 5); + } + function testLowNonces() public { permit2.useUnorderedNonce(address(this), 5); permit2.useUnorderedNonce(address(this), 0); From d865412f1e5341fd3a2ffb5179f6f3374b818848 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Wed, 9 Nov 2022 14:08:44 -0500 Subject: [PATCH 2/3] gas --- .gas-snapshot | 11 ++++++----- src/SignatureTransfer.sol | 4 ---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index a61ae5e1..8bbb30df 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -28,13 +28,14 @@ CompactSignature:testCompactSignature27() (gas: 253) CompactSignature:testCompactSignature28() (gas: 141) EIP712Test:testDomainSeparator() (gas: 5759) EIP712Test:testDomainSeparatorAfterFork() (gas: 10652) -NonceBitmapTest:testHighNonces() (gas: 35958) +NonceBitmapTest:testHighNonces() (gas: 35980) NonceBitmapTest:testInvalidateFullWord() (gas: 62805) NonceBitmapTest:testInvalidateNonzeroWord() (gas: 85285) -NonceBitmapTest:testLowNonces() (gas: 40671) -NonceBitmapTest:testNonceWordBoundary() (gas: 42042) -NonceBitmapTest:testUseTwoRandomNonces(uint256,uint256) (runs: 256, μ: 48832, ~: 51523) -NonceBitmapTest:testUsingNonceTwiceFails(uint256) (runs: 256, μ: 21775, ~: 21798) +NonceBitmapTest:testLowNonces() (gas: 40693) +NonceBitmapTest:testNonceLLL() (gas: 21789) +NonceBitmapTest:testNonceWordBoundary() (gas: 42060) +NonceBitmapTest:testUseTwoRandomNonces(uint256,uint256) (runs: 256, μ: 48918, ~: 51523) +NonceBitmapTest:testUsingNonceTwiceFails(uint256) (runs: 256, μ: 21793, ~: 21816) Permit2LibTest:testOZSafePermit() (gas: 24311) Permit2LibTest:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129066) Permit2LibTest:testOZSafeTransferFrom() (gas: 38787) diff --git a/src/SignatureTransfer.sol b/src/SignatureTransfer.sol index 9ffa1fda..35d1b220 100644 --- a/src/SignatureTransfer.sol +++ b/src/SignatureTransfer.sol @@ -9,8 +9,6 @@ import {SignatureVerification} from "./libraries/SignatureVerification.sol"; import {PermitHash} from "./libraries/PermitHash.sol"; import {EIP712} from "./EIP712.sol"; -import {console2} from "forge-std/console2.sol"; - contract SignatureTransfer is ISignatureTransfer, EIP712 { using SignatureVerification for bytes; using SafeTransferLib for ERC20; @@ -156,8 +154,6 @@ contract SignatureTransfer is ISignatureTransfer, EIP712 { uint256 bit = 1 << bitPos; uint256 flipped = nonceBitmap[from][wordPos] ^= bit; - console2.log(bit); - console2.log(flipped); if (flipped & bit == 0) revert InvalidNonce(); } } From d63e7f4cc41b1c955d8f02aaec9f27a1d315f0e1 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Thu, 10 Nov 2022 10:53:34 -0500 Subject: [PATCH 3/3] remove repeat nonce test --- .gas-snapshot | 11 +++++------ test/NonceBitmap.t.sol | 6 ------ 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 8bbb30df..a61ae5e1 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -28,14 +28,13 @@ CompactSignature:testCompactSignature27() (gas: 253) CompactSignature:testCompactSignature28() (gas: 141) EIP712Test:testDomainSeparator() (gas: 5759) EIP712Test:testDomainSeparatorAfterFork() (gas: 10652) -NonceBitmapTest:testHighNonces() (gas: 35980) +NonceBitmapTest:testHighNonces() (gas: 35958) NonceBitmapTest:testInvalidateFullWord() (gas: 62805) NonceBitmapTest:testInvalidateNonzeroWord() (gas: 85285) -NonceBitmapTest:testLowNonces() (gas: 40693) -NonceBitmapTest:testNonceLLL() (gas: 21789) -NonceBitmapTest:testNonceWordBoundary() (gas: 42060) -NonceBitmapTest:testUseTwoRandomNonces(uint256,uint256) (runs: 256, μ: 48918, ~: 51523) -NonceBitmapTest:testUsingNonceTwiceFails(uint256) (runs: 256, μ: 21793, ~: 21816) +NonceBitmapTest:testLowNonces() (gas: 40671) +NonceBitmapTest:testNonceWordBoundary() (gas: 42042) +NonceBitmapTest:testUseTwoRandomNonces(uint256,uint256) (runs: 256, μ: 48832, ~: 51523) +NonceBitmapTest:testUsingNonceTwiceFails(uint256) (runs: 256, μ: 21775, ~: 21798) Permit2LibTest:testOZSafePermit() (gas: 24311) Permit2LibTest:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129066) Permit2LibTest:testOZSafeTransferFrom() (gas: 38787) diff --git a/test/NonceBitmap.t.sol b/test/NonceBitmap.t.sol index 891cd63a..55713750 100644 --- a/test/NonceBitmap.t.sol +++ b/test/NonceBitmap.t.sol @@ -13,12 +13,6 @@ contract NonceBitmapTest is Test { permit2 = new MockPermit2(); } - function testNonceLLL() public { - permit2.useUnorderedNonce(address(this), 5); - vm.expectRevert(InvalidNonce.selector); - permit2.useUnorderedNonce(address(this), 5); - } - function testLowNonces() public { permit2.useUnorderedNonce(address(this), 5); permit2.useUnorderedNonce(address(this), 0);