From af1f50a9bf273f318226fd51b8e7714dd443249e Mon Sep 17 00:00:00 2001 From: saucepoint <98790946+saucepoint@users.noreply.github.com> Date: Wed, 17 Jul 2024 15:34:20 -0400 Subject: [PATCH] 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 <30504811+snreynolds@users.noreply.github.com> --- .../autocompound_exactUnclaimedFees.snap | 2 +- ...exactUnclaimedFees_exactCustodiedFees.snap | 2 +- .../autocompound_excessFeesCredit.snap | 2 +- .forge-snapshots/collect_erc20.snap | 2 +- .forge-snapshots/decreaseLiquidity_erc20.snap | 2 +- .../decreaseLiquidity_erc6909.snap | 2 +- .forge-snapshots/increaseLiquidity_erc20.snap | 2 +- .../increaseLiquidity_erc6909.snap | 2 +- .forge-snapshots/mint.snap | 2 +- .forge-snapshots/mintWithLiquidity.snap | 1 + .forge-snapshots/mint_differentRanges.snap | 2 +- .forge-snapshots/mint_same_tickLower.snap | 2 +- .forge-snapshots/mint_same_tickUpper.snap | 2 +- .forge-snapshots/permit.snap | 1 + .forge-snapshots/permit_secondPosition.snap | 1 + .forge-snapshots/permit_twice.snap | 1 + .forge-snapshots/sameRange_collect.snap | 2 +- .../sameRange_decreaseAllLiquidity.snap | 2 +- .forge-snapshots/sameRange_mint.snap | 2 +- contracts/NonfungiblePositionManager.sol | 17 +- contracts/base/ERC721Permit.sol | 48 ++- .../interfaces/IBaseLiquidityManagement.sol | 4 - contracts/interfaces/IERC721Permit.sol | 4 +- .../INonfungiblePositionManager.sol | 3 +- test/position-managers/FeeCollection.t.sol | 4 +- test/position-managers/Gas.t.sol | 85 +++++- .../NonfungiblePositionManager.t.sol | 5 +- test/position-managers/Permit.t.sol | 282 ++++++++++++++++++ test/shared/FeeMath.sol | 2 +- test/shared/LiquidityOperations.sol | 101 ++++++- 30 files changed, 536 insertions(+), 53 deletions(-) create mode 100644 .forge-snapshots/mintWithLiquidity.snap create mode 100644 .forge-snapshots/permit.snap create mode 100644 .forge-snapshots/permit_secondPosition.snap create mode 100644 .forge-snapshots/permit_twice.snap create mode 100644 test/position-managers/Permit.t.sol diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees.snap index ccc61b70..ff43b36e 100644 --- a/.forge-snapshots/autocompound_exactUnclaimedFees.snap +++ b/.forge-snapshots/autocompound_exactUnclaimedFees.snap @@ -1 +1 @@ -177823 \ No newline at end of file +179921 \ No newline at end of file diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap index 349667e7..31456f87 100644 --- a/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap +++ b/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap @@ -1 +1 @@ -192626 \ No newline at end of file +194724 \ No newline at end of file diff --git a/.forge-snapshots/autocompound_excessFeesCredit.snap b/.forge-snapshots/autocompound_excessFeesCredit.snap index 551e2c97..f7b70cac 100644 --- a/.forge-snapshots/autocompound_excessFeesCredit.snap +++ b/.forge-snapshots/autocompound_excessFeesCredit.snap @@ -1 +1 @@ -192327 \ No newline at end of file +194425 \ No newline at end of file diff --git a/.forge-snapshots/collect_erc20.snap b/.forge-snapshots/collect_erc20.snap index 99346ec9..bdbaf724 100644 --- a/.forge-snapshots/collect_erc20.snap +++ b/.forge-snapshots/collect_erc20.snap @@ -1 +1 @@ -170049 \ No newline at end of file +172148 \ No newline at end of file diff --git a/.forge-snapshots/decreaseLiquidity_erc20.snap b/.forge-snapshots/decreaseLiquidity_erc20.snap index 9bfafe5d..f499ab6a 100644 --- a/.forge-snapshots/decreaseLiquidity_erc20.snap +++ b/.forge-snapshots/decreaseLiquidity_erc20.snap @@ -1 +1 @@ -135099 \ No newline at end of file +137197 \ No newline at end of file diff --git a/.forge-snapshots/decreaseLiquidity_erc6909.snap b/.forge-snapshots/decreaseLiquidity_erc6909.snap index 9bfafe5d..f499ab6a 100644 --- a/.forge-snapshots/decreaseLiquidity_erc6909.snap +++ b/.forge-snapshots/decreaseLiquidity_erc6909.snap @@ -1 +1 @@ -135099 \ No newline at end of file +137197 \ No newline at end of file diff --git a/.forge-snapshots/increaseLiquidity_erc20.snap b/.forge-snapshots/increaseLiquidity_erc20.snap index dd91c8f5..f7e2fbd0 100644 --- a/.forge-snapshots/increaseLiquidity_erc20.snap +++ b/.forge-snapshots/increaseLiquidity_erc20.snap @@ -1 +1 @@ -165128 \ No newline at end of file +167226 \ No newline at end of file diff --git a/.forge-snapshots/increaseLiquidity_erc6909.snap b/.forge-snapshots/increaseLiquidity_erc6909.snap index dd91c8f5..f7e2fbd0 100644 --- a/.forge-snapshots/increaseLiquidity_erc6909.snap +++ b/.forge-snapshots/increaseLiquidity_erc6909.snap @@ -1 +1 @@ -165128 \ No newline at end of file +167226 \ No newline at end of file diff --git a/.forge-snapshots/mint.snap b/.forge-snapshots/mint.snap index 7f0aa9ea..334c9f54 100644 --- a/.forge-snapshots/mint.snap +++ b/.forge-snapshots/mint.snap @@ -1 +1 @@ -444628 \ No newline at end of file +441402 \ No newline at end of file diff --git a/.forge-snapshots/mintWithLiquidity.snap b/.forge-snapshots/mintWithLiquidity.snap new file mode 100644 index 00000000..ebad633c --- /dev/null +++ b/.forge-snapshots/mintWithLiquidity.snap @@ -0,0 +1 @@ +490196 \ No newline at end of file diff --git a/.forge-snapshots/mint_differentRanges.snap b/.forge-snapshots/mint_differentRanges.snap index 2238c582..f07f5fd3 100644 --- a/.forge-snapshots/mint_differentRanges.snap +++ b/.forge-snapshots/mint_differentRanges.snap @@ -1 +1 @@ -410428 \ No newline at end of file +407202 \ No newline at end of file diff --git a/.forge-snapshots/mint_same_tickLower.snap b/.forge-snapshots/mint_same_tickLower.snap index efe9556c..5bb35423 100644 --- a/.forge-snapshots/mint_same_tickLower.snap +++ b/.forge-snapshots/mint_same_tickLower.snap @@ -1 +1 @@ -404432 \ No newline at end of file +401206 \ No newline at end of file diff --git a/.forge-snapshots/mint_same_tickUpper.snap b/.forge-snapshots/mint_same_tickUpper.snap index 526f7432..7317abb2 100644 --- a/.forge-snapshots/mint_same_tickUpper.snap +++ b/.forge-snapshots/mint_same_tickUpper.snap @@ -1 +1 @@ -405074 \ No newline at end of file +401848 \ No newline at end of file diff --git a/.forge-snapshots/permit.snap b/.forge-snapshots/permit.snap new file mode 100644 index 00000000..f51e74ce --- /dev/null +++ b/.forge-snapshots/permit.snap @@ -0,0 +1 @@ +75049 \ No newline at end of file diff --git a/.forge-snapshots/permit_secondPosition.snap b/.forge-snapshots/permit_secondPosition.snap new file mode 100644 index 00000000..8977b204 --- /dev/null +++ b/.forge-snapshots/permit_secondPosition.snap @@ -0,0 +1 @@ +57937 \ No newline at end of file diff --git a/.forge-snapshots/permit_twice.snap b/.forge-snapshots/permit_twice.snap new file mode 100644 index 00000000..f1519e1c --- /dev/null +++ b/.forge-snapshots/permit_twice.snap @@ -0,0 +1 @@ +40849 \ No newline at end of file diff --git a/.forge-snapshots/sameRange_collect.snap b/.forge-snapshots/sameRange_collect.snap index 99346ec9..bdbaf724 100644 --- a/.forge-snapshots/sameRange_collect.snap +++ b/.forge-snapshots/sameRange_collect.snap @@ -1 +1 @@ -170049 \ No newline at end of file +172148 \ No newline at end of file diff --git a/.forge-snapshots/sameRange_decreaseAllLiquidity.snap b/.forge-snapshots/sameRange_decreaseAllLiquidity.snap index 4f7651cb..5f87c079 100644 --- a/.forge-snapshots/sameRange_decreaseAllLiquidity.snap +++ b/.forge-snapshots/sameRange_decreaseAllLiquidity.snap @@ -1 +1 @@ -148151 \ No newline at end of file +150250 \ No newline at end of file diff --git a/.forge-snapshots/sameRange_mint.snap b/.forge-snapshots/sameRange_mint.snap index b4d1f854..37cd9a21 100644 --- a/.forge-snapshots/sameRange_mint.snap +++ b/.forge-snapshots/sameRange_mint.snap @@ -1 +1 @@ -347778 \ No newline at end of file +344552 \ No newline at end of file diff --git a/contracts/NonfungiblePositionManager.sol b/contracts/NonfungiblePositionManager.sol index 5efbb88b..b73e6178 100644 --- a/contracts/NonfungiblePositionManager.sol +++ b/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) { diff --git a/contracts/base/ERC721Permit.sol b/contracts/base/ERC721Permit.sol index 8eb86521..4668f2c5 100644 --- a/contracts/base/ERC721Permit.sol +++ b/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(); + } } diff --git a/contracts/interfaces/IBaseLiquidityManagement.sol b/contracts/interfaces/IBaseLiquidityManagement.sol index 636480ea..05424934 100644 --- a/contracts/interfaces/IBaseLiquidityManagement.sol +++ b/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; diff --git a/contracts/interfaces/IERC721Permit.sol b/contracts/interfaces/IERC721Permit.sol index daa27030..213bca2a 100644 --- a/contracts/interfaces/IERC721Permit.sol +++ b/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; } diff --git a/contracts/interfaces/INonfungiblePositionManager.sol b/contracts/interfaces/INonfungiblePositionManager.sol index 5125fb96..a13380f2 100644 --- a/contracts/interfaces/INonfungiblePositionManager.sol +++ b/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 diff --git a/test/position-managers/FeeCollection.t.sol b/test/position-managers/FeeCollection.t.sol index 798e36db..9e608ef0 100644 --- a/test/position-managers/FeeCollection.t.sol +++ b/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); diff --git a/test/position-managers/Gas.t.sol b/test/position-managers/Gas.t.sol index 6d8a8a6d..af45f538 100644 --- a/test/position-managers/Gas.t.sol +++ b/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); diff --git a/test/position-managers/NonfungiblePositionManager.t.sol b/test/position-managers/NonfungiblePositionManager.t.sol index 29a83f4d..23452fa2 100644 --- a/test/position-managers/NonfungiblePositionManager.t.sol +++ b/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) diff --git a/test/position-managers/Permit.t.sol b/test/position-managers/Permit.t.sol new file mode 100644 index 00000000..d51717d9 --- /dev/null +++ b/test/position-managers/Permit.t.sol @@ -0,0 +1,282 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; +import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; +import {PoolManager} from "@uniswap/v4-core/src/PoolManager.sol"; +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol"; +import {Deployers} from "@uniswap/v4-core/test/utils/Deployers.sol"; +import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol"; +import {PoolId, PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol"; +import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; +import {BalanceDelta, toBalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; +import {PoolSwapTest} from "@uniswap/v4-core/src/test/PoolSwapTest.sol"; +import {LiquidityAmounts} from "../../contracts/libraries/LiquidityAmounts.sol"; +import {TickMath} from "@uniswap/v4-core/src/libraries/TickMath.sol"; +import {FixedPointMathLib} from "solmate/utils/FixedPointMathLib.sol"; +import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; + +import {IERC20} from "forge-std/interfaces/IERC20.sol"; +import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol"; +import {IERC721Permit} from "../../contracts/interfaces/IERC721Permit.sol"; + +import {NonfungiblePositionManager} from "../../contracts/NonfungiblePositionManager.sol"; +import {LiquidityRange, LiquidityRangeId, LiquidityRangeIdLibrary} from "../../contracts/types/LiquidityRange.sol"; + +import {Fuzzers} from "@uniswap/v4-core/src/test/Fuzzers.sol"; + +import {LiquidityOperations} from "../shared/LiquidityOperations.sol"; + +contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperations { + using FixedPointMathLib for uint256; + using CurrencyLibrary for Currency; + using LiquidityRangeIdLibrary for LiquidityRange; + using PoolIdLibrary for PoolKey; + using StateLibrary for IPoolManager; + + PoolId poolId; + address alice; + uint256 alicePK; + address bob; + uint256 bobPK; + + uint256 constant STARTING_USER_BALANCE = 10_000_000 ether; + + // expresses the fee as a wad (i.e. 3000 = 0.003e18 = 0.30%) + uint256 FEE_WAD; + + LiquidityRange range; + + function setUp() public { + (alice, alicePK) = makeAddrAndKey("ALICE"); + (bob, bobPK) = makeAddrAndKey("BOB"); + + Deployers.deployFreshManagerAndRouters(); + Deployers.deployMintAndApprove2Currencies(); + + (key, poolId) = initPool(currency0, currency1, IHooks(address(0)), 3000, SQRT_PRICE_1_1, ZERO_BYTES); + FEE_WAD = uint256(key.fee).mulDivDown(FixedPointMathLib.WAD, 1_000_000); + + lpm = new NonfungiblePositionManager(manager); + IERC20(Currency.unwrap(currency0)).approve(address(lpm), type(uint256).max); + IERC20(Currency.unwrap(currency1)).approve(address(lpm), type(uint256).max); + + // Give tokens to Alice and Bob, with approvals + IERC20(Currency.unwrap(currency0)).transfer(alice, STARTING_USER_BALANCE); + IERC20(Currency.unwrap(currency1)).transfer(alice, STARTING_USER_BALANCE); + IERC20(Currency.unwrap(currency0)).transfer(bob, STARTING_USER_BALANCE); + IERC20(Currency.unwrap(currency1)).transfer(bob, STARTING_USER_BALANCE); + vm.startPrank(alice); + IERC20(Currency.unwrap(currency0)).approve(address(lpm), type(uint256).max); + IERC20(Currency.unwrap(currency1)).approve(address(lpm), type(uint256).max); + vm.stopPrank(); + vm.startPrank(bob); + IERC20(Currency.unwrap(currency0)).approve(address(lpm), type(uint256).max); + IERC20(Currency.unwrap(currency1)).approve(address(lpm), type(uint256).max); + vm.stopPrank(); + + // define a reusable range + range = LiquidityRange({poolKey: key, tickLower: -300, tickUpper: 300}); + } + + function test_permit_increaseLiquidity() public { + uint256 liquidityAlice = 1e18; + vm.prank(alice); + _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + // alice gives bob operator permissions + _permit(alice, alicePK, tokenIdAlice, bob, 1); + + // bob can increase liquidity on alice's token + uint256 newLiquidity = 2e18; + uint256 balance0BobBefore = currency0.balanceOf(bob); + uint256 balance1BobBefore = currency1.balanceOf(bob); + vm.startPrank(bob); + _increaseLiquidity(tokenIdAlice, newLiquidity, ZERO_BYTES); + vm.stopPrank(); + + // alice's position has new liquidity + bytes32 positionId = + keccak256(abi.encodePacked(address(lpm), range.tickLower, range.tickUpper, bytes32(tokenIdAlice))); + (uint256 liquidity,,) = manager.getPositionInfo(range.poolKey.toId(), positionId); + assertEq(liquidity, liquidityAlice + newLiquidity); + + // bob used his tokens to increase liquidity + assertGt(balance0BobBefore, currency0.balanceOf(bob)); + assertGt(balance1BobBefore, currency1.balanceOf(bob)); + } + + function test_permit_decreaseLiquidity() public { + uint256 liquidityAlice = 1e18; + vm.prank(alice); + _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + // alice gives bob operator permissions + _permit(alice, alicePK, tokenIdAlice, bob, 1); + + // bob can decrease liquidity on alice's token + uint256 liquidityToRemove = 0.4444e18; + vm.startPrank(bob); + _decreaseLiquidity(tokenIdAlice, liquidityToRemove, ZERO_BYTES); + vm.stopPrank(); + + // alice's position decreased liquidity + bytes32 positionId = + keccak256(abi.encodePacked(address(lpm), range.tickLower, range.tickUpper, bytes32(tokenIdAlice))); + (uint256 liquidity,,) = manager.getPositionInfo(range.poolKey.toId(), positionId); + + assertEq(liquidity, liquidityAlice - liquidityToRemove); + } + + function test_permit_collect() public { + uint256 liquidityAlice = 1e18; + vm.prank(alice); + _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + // donate to create fee revenue + uint256 currency0Revenue = 0.4444e18; + uint256 currency1Revenue = 0.2222e18; + donateRouter.donate(key, currency0Revenue, currency1Revenue, ZERO_BYTES); + + // alice gives bob operator permissions + _permit(alice, alicePK, tokenIdAlice, bob, 1); + + // TODO: test collection to recipient with a permissioned operator + + // bob collects fees to himself + address recipient = bob; + uint256 balance0BobBefore = currency0.balanceOf(bob); + uint256 balance1BobBefore = currency1.balanceOf(bob); + vm.startPrank(bob); + _collect(tokenIdAlice, recipient, ZERO_BYTES); + vm.stopPrank(); + + assertApproxEqAbs(currency0.balanceOf(recipient), balance0BobBefore + currency0Revenue, 1 wei); + assertApproxEqAbs(currency1.balanceOf(recipient), balance1BobBefore + currency1Revenue, 1 wei); + } + + // --- Fail Scenarios --- // + function test_permit_notOwnerRevert() public { + // calling permit on a token that is not owned will fail + + uint256 liquidityAlice = 1e18; + vm.prank(alice); + _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + // bob cannot permit himself on alice's token + bytes32 digest = lpm.getDigest(bob, tokenIdAlice, 0, block.timestamp + 1); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(bobPK, digest); + + vm.startPrank(bob); + vm.expectRevert("Unauthorized"); + lpm.permit(bob, tokenIdAlice, block.timestamp + 1, 0, v, r, s); + vm.stopPrank(); + } + + function test_noPermit_increaseLiquidityRevert() public { + // increaseLiquidity fails if the owner did not permit + uint256 liquidityAlice = 1e18; + vm.prank(alice); + _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + // bob cannot increase liquidity on alice's token + uint256 newLiquidity = 2e18; + bytes memory increase = LiquidityOperations.getIncreaseEncoded(tokenIdAlice, newLiquidity, ZERO_BYTES); + vm.startPrank(bob); + vm.expectRevert("Not approved"); + lpm.modifyLiquidities(increase); + vm.stopPrank(); + } + + function test_noPermit_decreaseLiquidityRevert() public { + // decreaseLiquidity fails if the owner did not permit + uint256 liquidityAlice = 1e18; + vm.prank(alice); + _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + // bob cannot decrease liquidity on alice's token + uint256 liquidityToRemove = 0.4444e18; + bytes memory decrease = LiquidityOperations.getDecreaseEncoded(tokenIdAlice, 0.4444e18, ZERO_BYTES); + vm.startPrank(bob); + vm.expectRevert("Not approved"); + lpm.modifyLiquidities(decrease); + vm.stopPrank(); + } + + function test_noPermit_collectRevert() public { + // collect fails if the owner did not permit + uint256 liquidityAlice = 1e18; + vm.prank(alice); + _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + // donate to create fee revenue + uint256 currency0Revenue = 0.4444e18; + uint256 currency1Revenue = 0.2222e18; + donateRouter.donate(key, currency0Revenue, currency1Revenue, ZERO_BYTES); + + // bob cannot collect fees to a recipient + address recipient = address(0x00444400); + bytes memory collect = LiquidityOperations.getCollectEncoded(tokenIdAlice, recipient, ZERO_BYTES); + vm.startPrank(bob); + vm.expectRevert("Not approved"); + lpm.modifyLiquidities(collect); + vm.stopPrank(); + } + + function test_permit_nonceAlreadyUsed() public { + uint256 liquidityAlice = 1e18; + vm.prank(alice); + _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + // alice gives bob operator permissions + uint256 nonce = 1; + _permit(alice, alicePK, tokenIdAlice, bob, nonce); + + // alice cannot reuse the nonce + bytes32 digest = lpm.getDigest(bob, tokenIdAlice, nonce, block.timestamp + 1); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(alicePK, digest); + + vm.startPrank(alice); + vm.expectRevert(IERC721Permit.NonceAlreadyUsed.selector); + lpm.permit(bob, tokenIdAlice, block.timestamp + 1, nonce, v, r, s); + vm.stopPrank(); + } + + function test_permit_nonceAlreadyUsed_twoPositions() public { + uint256 liquidityAlice = 1e18; + vm.prank(alice); + _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + vm.prank(alice); + range.tickLower = -600; + range.tickUpper = 600; + _mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES); + uint256 tokenIdAlice2 = lpm.nextTokenId() - 1; + + // alice gives bob operator permissions for first token + uint256 nonce = 1; + _permit(alice, alicePK, tokenIdAlice, bob, nonce); + + // alice cannot reuse the nonce for the second token + bytes32 digest = lpm.getDigest(bob, tokenIdAlice2, nonce, block.timestamp + 1); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(alicePK, digest); + + vm.startPrank(alice); + vm.expectRevert(IERC721Permit.NonceAlreadyUsed.selector); + lpm.permit(bob, tokenIdAlice2, block.timestamp + 1, nonce, v, r, s); + vm.stopPrank(); + } +} diff --git a/test/shared/FeeMath.sol b/test/shared/FeeMath.sol index 2c88ebce..ad8746c0 100644 --- a/test/shared/FeeMath.sol +++ b/test/shared/FeeMath.sol @@ -27,7 +27,7 @@ library FeeMath { view returns (BalanceDelta feesOwed) { - (, LiquidityRange memory range) = posm.tokenPositions(tokenId); + (, LiquidityRange memory range,) = posm.tokenPositions(tokenId); // getPosition(poolId, owner, tL, tU, salt) // owner is the position manager diff --git a/test/shared/LiquidityOperations.sol b/test/shared/LiquidityOperations.sol index 8271b077..5e74db07 100644 --- a/test/shared/LiquidityOperations.sol +++ b/test/shared/LiquidityOperations.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.24; +import {Vm} from "forge-std/Vm.sol"; import {Currency} from "@uniswap/v4-core/src/types/Currency.sol"; import {BalanceDelta, toBalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; @@ -9,6 +10,7 @@ import {LiquidityRange} from "../../contracts/types/LiquidityRange.sol"; import {Planner} from "../utils/Planner.sol"; contract LiquidityOperations { + Vm internal constant _vm1 = Vm(address(uint160(uint256(keccak256("hevm cheat code"))))); NonfungiblePositionManager lpm; using Planner for Planner.Plan; @@ -28,36 +30,67 @@ contract LiquidityOperations { return abi.decode(result[0], (BalanceDelta)); } - function _increaseLiquidity(uint256 tokenId, uint256 liquidityToAdd, bytes memory hookData) internal { + // we overloaded this function because vm.prank was hitting .tokenPositions() + // TODO: now that vm.prank is hitting Planner, we can probably consolidate to a single function + function _increaseLiquidity(uint256 tokenId, uint256 liquidityToAdd, bytes memory hookData) + internal + returns (BalanceDelta) + { + (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); + return _increaseLiquidity(_range, tokenId, liquidityToAdd, hookData); + } + + function _increaseLiquidity( + LiquidityRange memory _range, + uint256 tokenId, + uint256 liquidityToAdd, + bytes memory hookData + ) internal returns (BalanceDelta) { Planner.Plan memory planner = Planner.init(); planner = planner.add(Actions.INCREASE, abi.encode(tokenId, liquidityToAdd, hookData)); - (, LiquidityRange memory _range) = lpm.tokenPositions(tokenId); - planner = planner.finalize(_range); // Close the currencies. - lpm.modifyLiquidities(abi.encode(planner.actions, planner.params)); + bytes[] memory result = lpm.modifyLiquidities(planner.zip()); + return abi.decode(result[0], (BalanceDelta)); } function _decreaseLiquidity(uint256 tokenId, uint256 liquidityToRemove, bytes memory hookData) internal returns (BalanceDelta) { + (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); + + return _decreaseLiquidity(_range, tokenId, liquidityToRemove, hookData); + } + + // do not make external call before unlockAndExecute, allows us to test reverts + function _decreaseLiquidity( + LiquidityRange memory _range, + uint256 tokenId, + uint256 liquidityToRemove, + bytes memory hookData + ) internal returns (BalanceDelta) { Planner.Plan memory planner = Planner.init(); planner = planner.add(Actions.DECREASE, abi.encode(tokenId, liquidityToRemove, hookData)); - (, LiquidityRange memory _range) = lpm.tokenPositions(tokenId); - planner = planner.finalize(_range); // Close the currencies. - bytes[] memory result = lpm.modifyLiquidities(abi.encode(planner.actions, planner.params)); + bytes[] memory result = lpm.modifyLiquidities(planner.zip()); return abi.decode(result[0], (BalanceDelta)); } function _collect(uint256 tokenId, address recipient, bytes memory hookData) internal returns (BalanceDelta) { + (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); + return _collect(_range, tokenId, recipient, hookData); + } + + // do not make external call before unlockAndExecute, allows us to test reverts + function _collect(LiquidityRange memory _range, uint256 tokenId, address recipient, bytes memory hookData) + internal + returns (BalanceDelta) + { Planner.Plan memory planner = Planner.init(); planner = planner.add(Actions.DECREASE, abi.encode(tokenId, 0, hookData)); - (, LiquidityRange memory _range) = lpm.tokenPositions(tokenId); - planner = planner.finalize(_range); // Close the currencies. bytes[] memory result = lpm.modifyLiquidities(planner.zip()); @@ -70,4 +103,54 @@ contract LiquidityOperations { // No close needed on burn. lpm.modifyLiquidities(planner.zip()); } + + // TODO: organize somewhere else, or rename this file to NFTLiquidityHelpers? + function _permit(address signer, uint256 privateKey, uint256 tokenId, address operator, uint256 nonce) internal { + bytes32 digest = lpm.getDigest(operator, tokenId, 1, block.timestamp + 1); + + (uint8 v, bytes32 r, bytes32 s) = _vm1.sign(privateKey, digest); + + _vm1.prank(signer); + lpm.permit(operator, tokenId, block.timestamp + 1, nonce, v, r, s); + } + + // Helper functions for getting encoded calldata for .modifyLiquidities + function getIncreaseEncoded(uint256 tokenId, uint256 liquidityToAdd, bytes memory hookData) + internal + view + returns (bytes memory) + { + (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); + Planner.Plan memory planner = Planner.init(); + planner = planner.add(Actions.INCREASE, abi.encode(tokenId, liquidityToAdd, hookData)); + planner = planner.finalize(_range); + return planner.zip(); + } + + function getDecreaseEncoded(uint256 tokenId, uint256 liquidityToRemove, bytes memory hookData) + internal + view + returns (bytes memory) + { + (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); + Planner.Plan memory planner = Planner.init(); + planner = planner.add(Actions.DECREASE, abi.encode(tokenId, liquidityToRemove, hookData)); + planner = planner.finalize(_range); + return planner.zip(); + } + + function getCollectEncoded(uint256 tokenId, address recipient, bytes memory hookData) + internal + view + returns (bytes memory) + { + (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); + Planner.Plan memory planner = Planner.init(); + planner = planner.add(Actions.DECREASE, abi.encode(tokenId, 0, hookData)); + + // TODO: allow recipient when supported on CLOSE_CURRENCY? + planner = planner.add(Actions.CLOSE_CURRENCY, abi.encode(_range.poolKey.currency0)); + planner = planner.add(Actions.CLOSE_CURRENCY, abi.encode(_range.poolKey.currency1)); + return planner.zip(); + } }