Owner-level ERC721 Permit (#153)

* checkpointing

* move decrease and collect to transient storage

* remove returns since they are now saved to transient storage

* draft: delta closing

* wip

* Sra/edits (#137)

* consolidate using owner, update burn

* fix: accrue deltas to caller in increase

* Rip Out Vanilla (#138)

* rip out vanilla and benchmark

* fix gas benchmark

* check posm is the locker before allowing access to external functions

* restore execute tests

* posm takes as 6909; remove legacy deadcode

* restore tests

* move helpers to the same file

* move operator to NFTposm; move nonce to ERC721Permit; owner-level nonce

* tests for operator/permit

* snapshots

* gas benchmarks for permit

* test fixes

* unordered nonces

* fix tests / cheatcode usage

* fix tests

---------

Co-authored-by: Sara Reynolds <[email protected]>

.forge-snapshots/autocompound_exactUnclaimedFees.snap

@@ -1 +1 @@
-177823
+179921

.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap

@@ -1 +1 @@
-192626
+194724

.forge-snapshots/autocompound_excessFeesCredit.snap

@@ -1 +1 @@
-192327
+194425

.forge-snapshots/collect_erc20.snap

@@ -1 +1 @@
-170049
+172148

.forge-snapshots/decreaseLiquidity_erc20.snap

@@ -1 +1 @@
-135099
+137197

.forge-snapshots/decreaseLiquidity_erc6909.snap

@@ -1 +1 @@
-135099
+137197

.forge-snapshots/increaseLiquidity_erc20.snap

@@ -1 +1 @@
-165128
+167226

.forge-snapshots/increaseLiquidity_erc6909.snap

@@ -1 +1 @@
-165128
+167226

.forge-snapshots/mint.snap

@@ -1 +1 @@
-444628
+441402

.forge-snapshots/mintWithLiquidity.snap

@@ -0,0 +1 @@
+490196

.forge-snapshots/mint_differentRanges.snap

@@ -1 +1 @@
-410428
+407202

.forge-snapshots/mint_same_tickLower.snap

@@ -1 +1 @@
-404432
+401206

.forge-snapshots/mint_same_tickUpper.snap

@@ -1 +1 @@
-405074
+401848

.forge-snapshots/permit.snap

@@ -0,0 +1 @@
+75049

.forge-snapshots/permit_secondPosition.snap

@@ -0,0 +1 @@
+57937

.forge-snapshots/permit_twice.snap

@@ -0,0 +1 @@
+40849

.forge-snapshots/sameRange_collect.snap

@@ -1 +1 @@
-170049
+172148

.forge-snapshots/sameRange_decreaseAllLiquidity.snap

@@ -1 +1 @@
-148151
+150250

.forge-snapshots/sameRange_mint.snap

@@ -1 +1 @@
-347778
+344552

contracts/NonfungiblePositionManager.sol

@@ -105,7 +105,7 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
 
         (delta,) = _modifyLiquidity(range, liquidity.toInt256(), bytes32(tokenId), hookData);
 
-        tokenPositions[tokenId] = TokenPosition({owner: owner, range: range});
+        tokenPositions[tokenId] = TokenPosition({owner: owner, range: range, operator: address(0x0)});
     }
 
     // Note: Calling increase with 0 will accrue any underlying fees.
@@ -159,19 +159,24 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
         TokenPosition storage tokenPosition = tokenPositions[tokenId];
         LiquidityRangeId rangeId = tokenPosition.range.toId();
         Position storage position = positions[from][rangeId];
-        position.operator = address(0x0);
 
         // transfer position data to destination
         positions[to][rangeId] = position;
         delete positions[from][rangeId];
 
         // update token position
-        tokenPositions[tokenId] = TokenPosition({owner: to, range: tokenPosition.range});
+        tokenPositions[tokenId] = TokenPosition({owner: to, range: tokenPosition.range, operator: address(0x0)});
     }
 
-    function _getAndIncrementNonce(uint256 tokenId) internal override returns (uint256) {
-        TokenPosition memory tokenPosition = tokenPositions[tokenId];
-        return uint256(positions[tokenPosition.owner][tokenPosition.range.toId()].nonce++);
+    // override ERC721 approval by setting operator
+    function _approve(address spender, uint256 tokenId) internal override {
+        tokenPositions[tokenId].operator = spender;
+    }
+
+    function getApproved(uint256 tokenId) public view override returns (address) {
+        require(_exists(tokenId), "ERC721: approved query for nonexistent token");
+
+        return tokenPositions[tokenId].operator;
     }
 
     modifier isAuthorizedForToken(uint256 tokenId, address sender) {

contracts/base/ERC721Permit.sol

@@ -11,8 +11,7 @@ import {IERC1271} from "../interfaces/external/IERC1271.sol";
 /// @title ERC721 with permit
 /// @notice Nonfungible tokens that support an approve via signature, i.e. permit
 abstract contract ERC721Permit is ERC721, IERC721Permit {
-    /// @dev Gets the current nonce for a token ID and then increments it, returning the original value
-    function _getAndIncrementNonce(uint256 tokenId) internal virtual returns (uint256);
+    mapping(address owner => mapping(uint256 word => uint256 bitmap)) public nonces;
 
     /// @dev The hash of the name used in the permit signature verification
     bytes32 private immutable nameHash;
@@ -46,23 +45,18 @@ abstract contract ERC721Permit is ERC721, IERC721Permit {
         0x49ecf333e5b8c95c40fdafc95c1ad136e8914a8fb55e9dc8bb01eaa83a2df9ad;
 
     /// @inheritdoc IERC721Permit
-    function permit(address spender, uint256 tokenId, uint256 deadline, uint8 v, bytes32 r, bytes32 s)
+    function permit(address spender, uint256 tokenId, uint256 deadline, uint256 nonce, uint8 v, bytes32 r, bytes32 s)
         external
         payable
         override
     {
         require(block.timestamp <= deadline, "Permit expired");
 
-        bytes32 digest = keccak256(
-            abi.encodePacked(
-                "\x19\x01",
-                DOMAIN_SEPARATOR(),
-                keccak256(abi.encode(PERMIT_TYPEHASH, spender, tokenId, _getAndIncrementNonce(tokenId), deadline))
-            )
-        );
         address owner = ownerOf(tokenId);
         require(spender != owner, "ERC721Permit: approval to current owner");
 
+        bytes32 digest = getDigest(spender, tokenId, nonce, deadline);
+
         if (Address.isContract(owner)) {
             require(IERC1271(owner).isValidSignature(digest, abi.encodePacked(r, s, v)) == 0x1626ba7e, "Unauthorized");
         } else {
@@ -71,6 +65,40 @@ abstract contract ERC721Permit is ERC721, IERC721Permit {
             require(recoveredAddress == owner, "Unauthorized");
         }
 
+        _useUnorderedNonce(owner, nonce);
         approve(spender, tokenId);
     }
+
+    function getDigest(address spender, uint256 tokenId, uint256 _nonce, uint256 deadline)
+        public
+        view
+        returns (bytes32 digest)
+    {
+        digest = keccak256(
+            abi.encodePacked(
+                "\x19\x01",
+                DOMAIN_SEPARATOR(),
+                keccak256(abi.encode(PERMIT_TYPEHASH, spender, tokenId, _nonce, deadline))
+            )
+        );
+    }
+
+    /// @notice Returns the index of the bitmap and the bit position within the bitmap. Used for unordered nonces
+    /// @param nonce The nonce to get the associated word and bit positions
+    /// @return wordPos The word position or index into the nonceBitmap
+    /// @return bitPos The bit position
+    /// @dev The first 248 bits of the nonce value is the index of the desired bitmap
+    /// @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);
+    }
+
+    function _useUnorderedNonce(address from, uint256 nonce) internal {
+        (uint256 wordPos, uint256 bitPos) = bitmapPositions(nonce);
+        uint256 bit = 1 << bitPos;
+        uint256 flipped = nonces[from][wordPos] ^= bit;
+
+        if (flipped & bit == 0) revert NonceAlreadyUsed();
+    }
 }

contracts/interfaces/IBaseLiquidityManagement.sol

@@ -11,10 +11,6 @@ interface IBaseLiquidityManagement {
 
     // details about the liquidity position
     struct Position {
-        // the nonce for permits
-        uint96 nonce;
-        // the address that is approved for spending this token
-        address operator;
         uint256 liquidity;
         // the fee growth of the aggregate position as of the last action on the individual position
         uint256 feeGrowthInside0LastX128;

contracts/interfaces/IERC721Permit.sol

@@ -4,6 +4,8 @@ pragma solidity >=0.7.5;
 /// @title ERC721 with permit
 /// @notice Extension to ERC721 that includes a permit function for signature based approvals
 interface IERC721Permit {
+    error NonceAlreadyUsed();
+
     /// @notice The permit typehash used in the permit signature
     /// @return The typehash for the permit
     function PERMIT_TYPEHASH() external pure returns (bytes32);
@@ -19,7 +21,7 @@ interface IERC721Permit {
     /// @param v Must produce valid secp256k1 signature from the holder along with `r` and `s`
     /// @param r Must produce valid secp256k1 signature from the holder along with `v` and `s`
     /// @param s Must produce valid secp256k1 signature from the holder along with `r` and `v`
-    function permit(address spender, uint256 tokenId, uint256 deadline, uint8 v, bytes32 r, bytes32 s)
+    function permit(address spender, uint256 tokenId, uint256 deadline, uint256 nonce, uint8 v, bytes32 r, bytes32 s)
         external
         payable;
 }

contracts/interfaces/INonfungiblePositionManager.sol

@@ -20,13 +20,14 @@ interface INonfungiblePositionManager {
     struct TokenPosition {
         address owner;
         LiquidityRange range;
+        address operator;
     }
 
     error MustBeUnlockedByThisContract();
     error DeadlinePassed();
     error UnsupportedAction();
 
-    function tokenPositions(uint256 tokenId) external view returns (address, LiquidityRange memory);
+    function tokenPositions(uint256 tokenId) external view returns (address, LiquidityRange memory, address);
 
     /// @notice Batches many liquidity modification calls to pool manager
     /// @param payload is an encoding of actions, params, and currencies

test/position-managers/FeeCollection.t.sol

@@ -174,8 +174,8 @@ contract FeeCollectionTest is Test, Deployers, GasSnapshot, LiquidityFuzzers, Li
         uint256 tokenIdBob = lpm.nextTokenId() - 1;
 
         // confirm the positions are same range
-        (, LiquidityRange memory rangeAlice) = lpm.tokenPositions(tokenIdAlice);
-        (, LiquidityRange memory rangeBob) = lpm.tokenPositions(tokenIdBob);
+        (, LiquidityRange memory rangeAlice,) = lpm.tokenPositions(tokenIdAlice);
+        (, LiquidityRange memory rangeBob,) = lpm.tokenPositions(tokenIdBob);
         assertEq(rangeAlice.tickLower, rangeBob.tickLower);
         assertEq(rangeAlice.tickUpper, rangeBob.tickUpper);
 

test/position-managers/Gas.t.sol

@@ -37,8 +37,10 @@ contract GasTest is Test, Deployers, GasSnapshot, LiquidityOperations {
     using FeeMath for INonfungiblePositionManager;
 
     PoolId poolId;
-    address alice = makeAddr("ALICE");
-    address bob = makeAddr("BOB");
+    address alice;
+    uint256 alicePK;
+    address bob;
+    uint256 bobPK;
 
     uint256 constant STARTING_USER_BALANCE = 10_000_000 ether;
 
@@ -48,6 +50,9 @@ contract GasTest is Test, Deployers, GasSnapshot, LiquidityOperations {
     LiquidityRange range;
 
     function setUp() public {
+        (alice, alicePK) = makeAddrAndKey("ALICE");
+        (bob, bobPK) = makeAddrAndKey("BOB");
+
         Deployers.deployFreshManagerAndRouters();
         Deployers.deployMintAndApprove2Currencies();
 
@@ -332,6 +337,82 @@ contract GasTest is Test, Deployers, GasSnapshot, LiquidityOperations {
 
     function test_gas_burn() public {}
     function test_gas_burnEmpty() public {}
+    function test_gas_collect() public {}
+
+    function test_gas_permit() public {
+        // alice permits for the first time
+        uint256 liquidityAlice = 1e18;
+        vm.prank(alice);
+        _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        uint256 tokenIdAlice = lpm.nextTokenId() - 1;
+
+        // alice gives operator permission to bob
+        uint256 nonce = 1;
+        bytes32 digest = lpm.getDigest(bob, tokenIdAlice, nonce, block.timestamp + 1);
+
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(alicePK, digest);
+
+        vm.prank(alice);
+        lpm.permit(bob, tokenIdAlice, block.timestamp + 1, nonce, v, r, s);
+        snapLastCall("permit");
+    }
+
+    function test_gas_permit_secondPosition() public {
+        // alice permits for her two tokens, benchmark the 2nd permit
+        uint256 liquidityAlice = 1e18;
+        vm.prank(alice);
+        _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        uint256 tokenIdAlice = lpm.nextTokenId() - 1;
+
+        // alice gives operator permission to bob
+        uint256 nonce = 1;
+        bytes32 digest = lpm.getDigest(bob, tokenIdAlice, nonce, block.timestamp + 1);
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(alicePK, digest);
+
+        vm.prank(alice);
+        lpm.permit(bob, tokenIdAlice, block.timestamp + 1, nonce, v, r, s);
+
+        // alice creates another position
+        vm.prank(alice);
+        _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        tokenIdAlice = lpm.nextTokenId() - 1;
+
+        // alice gives operator permission to bob
+        nonce = 2;
+        digest = lpm.getDigest(bob, tokenIdAlice, nonce, block.timestamp + 1);
+        (v, r, s) = vm.sign(alicePK, digest);
+
+        vm.prank(alice);
+        lpm.permit(bob, tokenIdAlice, block.timestamp + 1, nonce, v, r, s);
+        snapLastCall("permit_secondPosition");
+    }
+
+    function test_gas_permit_twice() public {
+        // alice permits the same token, twice
+        address charlie = makeAddr("CHARLIE");
+
+        uint256 liquidityAlice = 1e18;
+        vm.prank(alice);
+        _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        uint256 tokenIdAlice = lpm.nextTokenId() - 1;
+
+        // alice gives operator permission to bob
+        uint256 nonce = 1;
+        bytes32 digest = lpm.getDigest(bob, tokenIdAlice, nonce, block.timestamp + 1);
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(alicePK, digest);
+
+        vm.prank(alice);
+        lpm.permit(bob, tokenIdAlice, block.timestamp + 1, nonce, v, r, s);
+
+        // alice gives operator permission to charlie
+        nonce = 2;
+        digest = lpm.getDigest(charlie, tokenIdAlice, nonce, block.timestamp + 1);
+        (v, r, s) = vm.sign(alicePK, digest);
+
+        vm.prank(alice);
+        lpm.permit(charlie, tokenIdAlice, block.timestamp + 1, nonce, v, r, s);
+        snapLastCall("permit_twice");
+    }
 
     function test_gas_collect_erc20() public {
         _mint(range, 10_000 ether, block.timestamp + 1, address(this), ZERO_BYTES);

test/position-managers/NonfungiblePositionManager.t.sol

@@ -121,6 +121,7 @@ contract NonfungiblePositionManagerTest is Test, Deployers, GasSnapshot, Liquidi
 
         assertEq(tokenId, 1);
         assertEq(lpm.ownerOf(1), address(this));
+
         assertEq(uint256(int256(-delta.amount0())), amount0Desired);
         assertEq(uint256(int256(-delta.amount1())), amount1Desired);
         assertEq(balance0Before - balance0After, uint256(int256(-delta.amount0())));
@@ -209,7 +210,7 @@ contract NonfungiblePositionManagerTest is Test, Deployers, GasSnapshot, Liquidi
         // burn liquidity
         uint256 balance0BeforeBurn = currency0.balanceOfSelf();
         uint256 balance1BeforeBurn = currency1.balanceOfSelf();
-        // TODO, encode this under one call
+
         BalanceDelta deltaDecrease = _decreaseLiquidity(tokenId, liquidity, ZERO_BYTES);
         _burn(tokenId);
 
@@ -275,7 +276,7 @@ contract NonfungiblePositionManagerTest is Test, Deployers, GasSnapshot, Liquidi
     //     uint256 balance0Before = currency0.balanceOfSelf();
     //     uint256 balance1Before = currency1.balanceOfSelf();
     //             BalanceDelta delta = lpm.decreaseLiquidity(tokenId, decreaseLiquidityDelta, ZERO_BYTES, false);
-    //     (,, uint256 liquidity,,,,) = lpm.positions(address(this), range.toId());
+    //     (uint256 liquidity,,,,) = lpm.positions(address(this), range.toId());
     //     assertEq(liquidity, uint256(params.liquidityDelta) - decreaseLiquidityDelta);
 
     //     // express key.fee as wad (i.e. 3000 = 0.003e18)