Skip to content

Commit

Permalink
unordered nonces
Browse files Browse the repository at this point in the history
  • Loading branch information
saucepoint committed Jul 10, 2024
1 parent 8f50bdc commit ab74982
Show file tree
Hide file tree
Showing 13 changed files with 106 additions and 30 deletions.
2 changes: 1 addition & 1 deletion .forge-snapshots/autocompound_exactUnclaimedFees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
295470
295448
Original file line number Diff line number Diff line change
@@ -1 +1 @@
227829
227807
2 changes: 1 addition & 1 deletion .forge-snapshots/autocompound_excessFeesCredit.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
316009
315987
2 changes: 1 addition & 1 deletion .forge-snapshots/increaseLiquidity_erc20.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
199086
199064
2 changes: 1 addition & 1 deletion .forge-snapshots/increaseLiquidity_erc6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
199098
199076
2 changes: 1 addition & 1 deletion .forge-snapshots/permit.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
74708
75071
2 changes: 1 addition & 1 deletion .forge-snapshots/permit_secondPosition.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
57608
57959
2 changes: 1 addition & 1 deletion .forge-snapshots/permit_twice.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
40496
40871
26 changes: 23 additions & 3 deletions contracts/base/ERC721Permit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +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 {
mapping(address owner => uint256 nonce) public nonce;
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;
Expand Down Expand Up @@ -45,7 +45,7 @@ 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
Expand All @@ -55,7 +55,7 @@ abstract contract ERC721Permit is ERC721, IERC721Permit {
address owner = ownerOf(tokenId);
require(spender != owner, "ERC721Permit: approval to current owner");

bytes32 digest = getDigest(spender, tokenId, nonce[owner]++, deadline);
bytes32 digest = getDigest(spender, tokenId, nonce, deadline);

if (Address.isContract(owner)) {
require(IERC1271(owner).isValidSignature(digest, abi.encodePacked(r, s, v)) == 0x1626ba7e, "Unauthorized");
Expand All @@ -65,6 +65,7 @@ abstract contract ERC721Permit is ERC721, IERC721Permit {
require(recoveredAddress == owner, "Unauthorized");
}

_useUnorderedNonce(owner, nonce);
approve(spender, tokenId);
}

Expand All @@ -81,4 +82,23 @@ abstract contract ERC721Permit is ERC721, IERC721Permit {
)
);
}

/// @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();
}
}
4 changes: 3 additions & 1 deletion contracts/interfaces/IERC721Permit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
25 changes: 15 additions & 10 deletions test/position-managers/Gas.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -324,12 +324,13 @@ contract GasTest is Test, Deployers, GasSnapshot, LiquidityOperations {
uint256 tokenIdAlice = lpm.nextTokenId() - 1;

// alice gives operator permission to bob
bytes32 digest = lpm.getDigest(bob, tokenIdAlice, lpm.nonce(alice), block.timestamp + 1);
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, v, r, s);
lpm.permit(bob, tokenIdAlice, block.timestamp + 1, nonce, v, r, s);
snapLastCall("permit");
}

Expand All @@ -341,23 +342,25 @@ contract GasTest is Test, Deployers, GasSnapshot, LiquidityOperations {
uint256 tokenIdAlice = lpm.nextTokenId() - 1;

// alice gives operator permission to bob
bytes32 digest = lpm.getDigest(bob, tokenIdAlice, lpm.nonce(alice), block.timestamp + 1);
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, v, r, s);
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
digest = lpm.getDigest(bob, tokenIdAlice, lpm.nonce(alice), block.timestamp + 1);
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, v, r, s);
lpm.permit(bob, tokenIdAlice, block.timestamp + 1, nonce, v, r, s);
snapLastCall("permit_secondPosition");
}

Expand All @@ -371,18 +374,20 @@ contract GasTest is Test, Deployers, GasSnapshot, LiquidityOperations {
uint256 tokenIdAlice = lpm.nextTokenId() - 1;

// alice gives operator permission to bob
bytes32 digest = lpm.getDigest(bob, tokenIdAlice, lpm.nonce(alice), block.timestamp + 1);
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, v, r, s);
lpm.permit(bob, tokenIdAlice, block.timestamp + 1, nonce, v, r, s);

// alice gives operator permission to charlie
digest = lpm.getDigest(charlie, tokenIdAlice, lpm.nonce(alice), block.timestamp + 1);
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, v, r, s);
lpm.permit(charlie, tokenIdAlice, block.timestamp + 1, nonce, v, r, s);
snapLastCall("permit_twice");
}
}
59 changes: 54 additions & 5 deletions test/position-managers/Permit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ 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";
Expand Down Expand Up @@ -85,7 +86,7 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation
uint256 tokenIdAlice = lpm.nextTokenId() - 1;

// alice gives bob operator permissions
_permit(alice, alicePK, tokenIdAlice, bob);
_permit(alice, alicePK, tokenIdAlice, bob, 1);

// bob can increase liquidity on alice's token
uint256 newLiquidity = 2e18;
Expand All @@ -110,7 +111,7 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation
uint256 tokenIdAlice = lpm.nextTokenId() - 1;

// alice gives bob operator permissions
_permit(alice, alicePK, tokenIdAlice, bob);
_permit(alice, alicePK, tokenIdAlice, bob, 1);

// bob can decrease liquidity on alice's token
uint256 liquidityToRemove = 0.4444e18;
Expand All @@ -134,7 +135,7 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation
donateRouter.donate(key, currency0Revenue, currency1Revenue, ZERO_BYTES);

// alice gives bob operator permissions
_permit(alice, alicePK, tokenIdAlice, bob);
_permit(alice, alicePK, tokenIdAlice, bob, 1);

// TODO: enable once we fix recipient collection

Expand All @@ -158,13 +159,13 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation
uint256 tokenIdAlice = lpm.nextTokenId() - 1;

// bob cannot permit himself on alice's token
bytes32 digest = lpm.getDigest(bob, tokenIdAlice, lpm.nonce(bob), block.timestamp + 1);
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, v, r, s);
lpm.permit(bob, tokenIdAlice, block.timestamp + 1, 0, v, r, s);
vm.stopPrank();
}

Expand Down Expand Up @@ -219,4 +220,52 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation
_collect(range, tokenIdAlice, recipient, ZERO_BYTES, false);
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();
}
}
6 changes: 3 additions & 3 deletions test/shared/LiquidityOperations.sol
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,12 @@ contract LiquidityOperations {
}

// TODO: organize somewhere else, or rename this file to NFTLiquidityHelpers?
function _permit(address signer, uint256 privateKey, uint256 tokenId, address operator) internal {
bytes32 digest = lpm.getDigest(operator, tokenId, lpm.nonce(signer), block.timestamp + 1);
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, v, r, s);
lpm.permit(operator, tokenId, block.timestamp + 1, nonce, v, r, s);
}
}

0 comments on commit ab74982

Please sign in to comment.