Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add v4 NonfungiblePositionManager skeleton #76

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
fe9305e
add helper contracts and interfaces for position manager
tinaszheng Nov 21, 2023
0b01a0a
run forge fmt
tinaszheng Nov 21, 2023
74ad1a2
remove custom timestamp getter
tinaszheng Nov 21, 2023
30479c6
use custom errors and IER1271.isValidSignature.selector
tinaszheng Nov 21, 2023
349ddb8
use using .. for syntax for TransferHelper
tinaszheng Nov 21, 2023
906fb65
add erics custom error handler
tinaszheng Nov 21, 2023
0152e5c
remove ChainId library
tinaszheng Nov 21, 2023
e5fc9dd
use currency library to combine sweepETH and sweepToken
tinaszheng Nov 21, 2023
54d55fd
use Currency library for pay method
tinaszheng Nov 21, 2023
a2200ac
use solmate SafeTransferLib instead
tinaszheng Nov 21, 2023
b4f84ee
remove erc721permit for now
tinaszheng Nov 21, 2023
ad5c503
revert changes to TransferHelper
tinaszheng Nov 22, 2023
04a7a1d
put using .. for in contract header
tinaszheng Nov 22, 2023
f9d830e
move custom errors inside contracts
tinaszheng Nov 22, 2023
0d87708
run format
tinaszheng Nov 22, 2023
02a5cd2
update version
tinaszheng Nov 22, 2023
d261d58
add v4 position manager contracts
tinaszheng Nov 21, 2023
7f544b1
add Currency class and fix override error
tinaszheng Nov 21, 2023
7cc8c66
remove reference to erc721enumerable from concrete
tinaszheng Nov 21, 2023
c52f05a
use custom errors
tinaszheng Nov 21, 2023
05c9543
use ERC721 for now
tinaszheng Nov 21, 2023
ca01507
move custom errors inside contract + inherit from ILockCallback
tinaszheng Nov 28, 2023
4a7d82a
use poolkey in mint params
tinaszheng Nov 28, 2023
6e82a2e
rename variables
tinaszheng Nov 28, 2023
3166045
add periphery immutable state
tinaszheng Nov 29, 2023
ad34755
update position manager skeleton with liquidity management file
tinaszheng Nov 29, 2023
aad107a
fix inheritance order
tinaszheng Nov 29, 2023
87e0b2f
move stuff around
tinaszheng Nov 29, 2023
c64e82e
move addLiquidity functions to liquiditymanagement
tinaszheng Nov 29, 2023
b72452b
update comment + delete unused file
tinaszheng Nov 29, 2023
5d8a888
remove stuff for initializing pools for now
tinaszheng Nov 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
211 changes: 211 additions & 0 deletions contracts/NonfungiblePositionManagerV4.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;

import {IERC721Metadata} from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import {PoolKey} from "@uniswap/v4-core/contracts/types/PoolKey.sol";
import {PoolId, PoolIdLibrary} from "@uniswap/v4-core/contracts/types/PoolId.sol";
import {IPoolManager} from "@uniswap/v4-core/contracts/interfaces/IPoolManager.sol";
import {Currency} from "@uniswap/v4-core/contracts/types/Currency.sol";

import {INonfungiblePositionManagerV4} from "./interfaces/INonfungiblePositionManagerV4.sol";
import {PeripheryValidation} from "./base/PeripheryValidation.sol";
import {PeripheryPayments} from "./base/PeripheryPayments.sol";
import {PeripheryImmutableState} from "./base/PeripheryImmutableState.sol";
import {SelfPermit} from "./base/SelfPermit.sol";
import {LiquidityManagement} from "./base/LiquidityManagement.sol";
import {Multicall} from "./base/Multicall.sol";

contract NonfungiblePositionManagerV4 is
INonfungiblePositionManagerV4,
ERC721,
PeripheryImmutableState,
PeripheryValidation,
PeripheryPayments,
LiquidityManagement,
SelfPermit,
Multicall
{
using PoolIdLibrary for PoolKey;

error InvalidTokenID();
error NotApproved();
error NotCleared();
error NonexistentToken();

// details about the Uniswap position
struct TokenPosition {
// the nonce for permits
uint96 nonce;
// the address that is approved for spending this token
address operator;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm this is kind of strange because you can have more than 1 operator with the 721 spec... so not sure why we save just one. did we do the same in v3? if so.. why?

// the hashed poolKey of the pool with which this token is connected
PoolId poolId;
// the tick range of the position
int24 tickLower;
int24 tickUpper;
// the liquidity of the position
uint128 liquidity;
// the fee growth of the aggregate position as of the last action on the individual position
uint256 feeGrowthInside0LastX128;
uint256 feeGrowthInside1LastX128;
// how many uncollected tokens are owed to the position, as of the last computation
uint128 tokensOwed0;
uint128 tokensOwed1;
}

/// @dev Pool keys by poolIds
mapping(bytes32 => PoolKey) private _poolIdToPoolKey;

/// @dev The token ID position data
mapping(uint256 => TokenPosition) private _positions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason for this to be private? im down for this to be public i can imagine other contracts or interfaces wanting to look up position info

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking they probably want to use the positions getter for getting position data, since it includes helpers for unwrapping poolId - otherwise they're kind of exposed to our internal representation of how we store pools right? they'll need both _poolIdToPoolKey and _positions to get useful position data

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah good point

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Would just add to the natspec that they key is specifically the tokenId (tokenId) ie mapping from tokenId to position data


/// @dev The ID of the next token that will be minted. Skips 0
uint176 private _nextId = 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we call this _nextTokenId? We have tokenIds and poolIds so just want to be clear when we have id var names


/// @dev The address of the token descriptor contract, which handles generating token URIs for position tokens
address private immutable _tokenDescriptor;

constructor(IPoolManager _poolManager, address _tokenDescriptor_)
PeripheryImmutableState(_poolManager)
ERC721("Uniswap V4 Positions NFT-V1", "UNI-V4-POS")
{
_tokenDescriptor = _tokenDescriptor_;
}

/// @inheritdoc INonfungiblePositionManagerV4
function positions(uint256 tokenId)
external
view
override
returns (
uint96 nonce,
address operator,
Currency currency0,
Currency currency1,
uint24 fee,
int24 tickLower,
int24 tickUpper,
uint128 liquidity,
uint256 feeGrowthInside0LastX128,
uint256 feeGrowthInside1LastX128,
uint128 tokensOwed0,
uint128 tokensOwed1
)
{
TokenPosition memory position = _positions[tokenId];
if (PoolId.unwrap(position.poolId) == 0) revert InvalidTokenID();
PoolKey memory poolKey = _poolIdToPoolKey[PoolId.unwrap(position.poolId)];
return (
position.nonce,
position.operator,
poolKey.currency0,
poolKey.currency1,
poolKey.fee,
position.tickLower,
position.tickUpper,
position.liquidity,
position.feeGrowthInside0LastX128,
position.feeGrowthInside1LastX128,
position.tokensOwed0,
position.tokensOwed1
);
}

/// @inheritdoc INonfungiblePositionManagerV4
function createAndInitializePoolIfNecessary(PoolKey memory poolkey, uint160 sqrtPriceX96, bytes memory initData)
external
payable
{
// TODO: implement this
}

/// @inheritdoc INonfungiblePositionManagerV4
function mint(MintParams calldata params)
external
payable
override
checkDeadline(params.deadline)
returns (uint256 tokenId, uint128 liquidity, uint256 amount0, uint256 amount1)
{
// TODO: implement this
// will do something like return mintEntry(params)
}

modifier isAuthorizedForToken(uint256 tokenId) {
if (!_isApprovedOrOwner(msg.sender, tokenId)) revert NotApproved();
_;
}

function tokenURI(uint256 tokenId) public view override(ERC721, IERC721Metadata) returns (string memory) {
// TODO: implement this
}

/// @inheritdoc INonfungiblePositionManagerV4
function increaseLiquidity(IncreaseLiquidityParams calldata params)
external
payable
override
checkDeadline(params.deadline)
returns (uint128 liquidity, uint256 amount0, uint256 amount1)
{
// TODO: implement this
}

/// @inheritdoc INonfungiblePositionManagerV4
function decreaseLiquidity(DecreaseLiquidityParams calldata params)
external
payable
override
isAuthorizedForToken(params.tokenId)
checkDeadline(params.deadline)
returns (uint256 amount0, uint256 amount1)
{
// TODO: implement this
}

/// @inheritdoc INonfungiblePositionManagerV4
function collect(CollectParams calldata params)
external
payable
override
isAuthorizedForToken(params.tokenId)
returns (uint256 amount0, uint256 amount1)
{
// TODO: implement this
}

/// @inheritdoc INonfungiblePositionManagerV4
function burn(uint256 tokenId) external payable override isAuthorizedForToken(tokenId) {
TokenPosition storage position = _positions[tokenId];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the checks below will all be sloads. I think you can copy to memory, do the checks (mloads) below and then delete the storage var with delete _positions[tokenId];

if (position.liquidity != 0 || position.tokensOwed0 != 0 || position.tokensOwed1 != 0) revert NotCleared();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to make sure theyve collected all their rewards before they can burn? or does this already check that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok ive dug into it and these checks should be enough for burn

delete _positions[tokenId];
_burn(tokenId);
}

/// @inheritdoc IERC721
function getApproved(uint256 tokenId) public view override(ERC721, IERC721) returns (address) {
if (!_exists(tokenId)) revert NonexistentToken();

return _positions[tokenId].operator;
}

/// @dev Overrides _approve to use the operator in the position, which is packed with the position permit nonce
function _approve(address to, uint256 tokenId) internal override(ERC721) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm I'm kinda confused with the approval/auth scheme we are defining. It sounds like we are trying to override the traditional 721 spec but we are still using(not dis-allowing) the old 721 interfaces.

and we are using the old traditional 721 checks like _isApprovedOrOwner which doesnt look at the operator in the TokenPositions struct

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it seems like we want to allow permits... but where is all that sig verification?

_positions[tokenId].operator = to;
emit Approval(ownerOf(tokenId), to, tokenId);
}

function tokenByIndex(uint256 index) external view returns (uint256) {
// TODO: implement this
}

function tokenOfOwnerByIndex(address owner, uint256 index) external view returns (uint256) {
// TODO: implement this
}

function totalSupply() external view returns (uint256) {
// TODO: implement this
}
}
22 changes: 22 additions & 0 deletions contracts/base/LiquidityManagement.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;

import {BalanceDelta} from "@uniswap/v4-core/contracts/types/BalanceDelta.sol";
import {ILockCallback} from "@uniswap/v4-core/contracts/interfaces/callback//ILockCallback.sol";
import {IPeripheryPayments} from "../interfaces/IPeripheryPayments.sol";
import {ILiquidityManagement} from "../interfaces/ILiquidityManagement.sol";
import {PeripheryImmutableState} from "./PeripheryImmutableState.sol";

abstract contract LiquidityManagement is ILockCallback, ILiquidityManagement, PeripheryImmutableState {
function mintEntry(MintParams memory params)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just an example of how LiquidityManagement can be used with contracts/NonfungiblePositionManagerV4.sol - mintEntry calls the lock, then lockAcquired handles rest of the codepath within this file

internal
returns (uint256 tokenId, uint128 liquidity, BalanceDelta delta)
{
// poolManager.lock call here
}

function lockAcquired(bytes calldata rawData) external override returns (bytes memory) {
// TODO: handle mint/add/decrease liquidity here
return abi.encode(0);
}
}
33 changes: 33 additions & 0 deletions contracts/base/Multicall.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.19;

import {IMulticall} from "../interfaces/IMulticall.sol";

/// @title Multicall
/// @notice Enables calling multiple methods in a single call to the contract
abstract contract Multicall is IMulticall {
/// @inheritdoc IMulticall
function multicall(bytes[] calldata data) public payable override returns (bytes[] memory results) {
results = new bytes[](data.length);
for (uint256 i = 0; i < data.length; i++) {
(bool success, bytes memory result) = address(this).delegatecall(data[i]);

if (!success) {
// handle custom errors
if (result.length == 4) {
assembly {
revert(add(result, 0x20), mload(result))
}
}
Comment on lines +17 to +21

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops mark pointed out that custom errors can revert with params so this doesn't necessarily catch all custom errors 😅 , we could probably just revert with the reason again or we can look for a fix

// Next 5 lines from https://ethereum.stackexchange.com/a/83577
if (result.length < 68) revert();
assembly {
result := add(result, 0x04)
}
revert(abi.decode(result, (string)));
}

results[i] = result;
}
}
}
15 changes: 15 additions & 0 deletions contracts/base/PeripheryImmutableState.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;

import {IPoolManager} from "@uniswap/v4-core/contracts/interfaces/IPoolManager.sol";
import {IPeripheryImmutableState} from "../interfaces/IPeripheryImmutableState.sol";

/// @title Immutable state
/// @notice Immutable state used by periphery contracts
abstract contract PeripheryImmutableState is IPeripheryImmutableState {
IPoolManager public immutable override poolManager;

constructor(IPoolManager _poolManager) {
poolManager = _poolManager;
}
}
41 changes: 41 additions & 0 deletions contracts/base/PeripheryPayments.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;

import {ERC20} from "solmate/tokens/ERC20.sol";
import {Currency, CurrencyLibrary} from "@uniswap/v4-core/contracts/types/Currency.sol";
import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";
import {IPeripheryPayments} from "../interfaces/IPeripheryPayments.sol";

abstract contract PeripheryPayments is IPeripheryPayments {
using CurrencyLibrary for Currency;
using SafeTransferLib for address;
using SafeTransferLib for ERC20;

error InsufficientToken();
error NativeTokenTransferFrom();

/// @inheritdoc IPeripheryPayments
function sweepToken(Currency currency, uint256 amountMinimum, address recipient) public payable override {
uint256 balanceCurrency = currency.balanceOfSelf();
if (balanceCurrency < amountMinimum) revert InsufficientToken();

if (balanceCurrency > 0) {
currency.transfer(recipient, balanceCurrency);
}
}

/// @param currency The currency to pay
/// @param payer The entity that must pay
/// @param recipient The entity that will receive payment
/// @param value The amount to pay
function pay(Currency currency, address payer, address recipient, uint256 value) internal {
if (payer == address(this)) {
// pay with tokens already in the contract (for the exact input multihop case)
currency.transfer(recipient, value);
} else {
if (currency.isNative()) revert NativeTokenTransferFrom();
// pull payment
ERC20(Currency.unwrap(currency)).safeTransferFrom(payer, recipient, value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to use Permit2 for this? or are we good with approve and transferFrom?

}
}
}
11 changes: 11 additions & 0 deletions contracts/base/PeripheryValidation.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;

abstract contract PeripheryValidation {
error TransactionTooOld();

modifier checkDeadline(uint256 deadline) {
if (block.timestamp > deadline) revert TransactionTooOld();
_;
}
}
52 changes: 52 additions & 0 deletions contracts/base/SelfPermit.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;

import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {IERC20Permit} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Permit.sol";

import {IERC20PermitAllowed} from "../interfaces/external/IERC20PermitAllowed.sol";
import {ISelfPermit} from "../interfaces/ISelfPermit.sol";

/// @title Self Permit
/// @notice Functionality to call permit on any EIP-2612-compliant token for use in the route
/// @dev These functions are expected to be embedded in multicalls to allow EOAs to approve a contract and call a function
/// that requires an approval in a single transaction.
abstract contract SelfPermit is ISelfPermit {
/// @inheritdoc ISelfPermit
function selfPermit(address token, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s)
public
payable
override
{
IERC20Permit(token).permit(msg.sender, address(this), value, deadline, v, r, s);
}

/// @inheritdoc ISelfPermit
function selfPermitIfNecessary(address token, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s)
external
payable
override
{
if (IERC20(token).allowance(msg.sender, address(this)) < value) selfPermit(token, value, deadline, v, r, s);
}

/// @inheritdoc ISelfPermit
function selfPermitAllowed(address token, uint256 nonce, uint256 expiry, uint8 v, bytes32 r, bytes32 s)
public
payable
override
{
IERC20PermitAllowed(token).permit(msg.sender, address(this), nonce, expiry, true, v, r, s);
}

/// @inheritdoc ISelfPermit
function selfPermitAllowedIfNecessary(address token, uint256 nonce, uint256 expiry, uint8 v, bytes32 r, bytes32 s)
external
payable
override
{
if (IERC20(token).allowance(msg.sender, address(this)) < type(uint256).max) {
selfPermitAllowed(token, nonce, expiry, v, r, s);
}
}
}
Loading