uniswap/v3-periphery

🦄 🦄 🦄 Peripheral smart contracts for interacting with Uniswap v3

Uniswap V3 Periphery

This repository contains the periphery smart contracts for the Uniswap V3 Protocol. For the lower level core contracts, see the uniswap-v3-core repository.

Bug bounty

This repository is subject to the Uniswap V3 bug bounty program, per the terms defined here.

Local deployment

In order to deploy this code to a local testnet, you should install the npm package @uniswap/v3-periphery and import bytecode imported from artifacts located at @uniswap/v3-periphery/artifacts/contracts/*/*.json. For example:

import {
  abi as SWAP_ROUTER_ABI,
  bytecode as SWAP_ROUTER_BYTECODE,
} from '@uniswap/v3-periphery/artifacts/contracts/SwapRouter.sol/SwapRouter.json'

// deploy the bytecode

This will ensure that you are testing against the same bytecode that is deployed to mainnet and public testnets, and all Uniswap code will correctly interoperate with your local deployment.

Using solidity interfaces

The Uniswap v3 periphery interfaces are available for import into solidity smart contracts via the npm artifact @uniswap/v3-periphery, e.g.:

import '@uniswap/v3-periphery/contracts/interfaces/ISwapRouter.sol';

contract MyContract {
  ISwapRouter router;

  function doSomethingWithSwapRouter() {
    // router.exactInput(...);
  }
}

v3-periphery's Issues

consider allowing partial migration, with the rest refunded

payer in multihop should be router except for first swap

https://github.com/Uniswap/uniswap-v3-periphery/blob/8ad273f45cd42e04b5f8f15b6f4da57827122273/contracts/SwapRouter.sol#L145

i guess tests are passing because all tokens are approved

support permit in swaps

missing liquidity burn check

https://github.com/Uniswap/uniswap-v3-periphery/blob/8ad273f45cd42e04b5f8f15b6f4da57827122273/contracts/NonfungiblePositionManager.sol#L330

can burn more liquidity than a position has

gasless swaps/mints

support swapping or minting without actually sending the transaction from the router

this would involve creating a new eip 712 domain for the different actions

potentially we want something generic like the OZ context module

end result is the router is perfectly usable with something like infura transactions
https://infura.io/docs/transactions

permit must be payable for add liquidity with eth and non-approved token in one tx

https://github.com/Uniswap/uniswap-v3-periphery/blob/e009adb581e75e2fc906e3ed1555f9b0145884d8/contracts/interfaces/IERC721Permit.sol#L20-L27

optimize out `IUniswapV3Pool#swap` extcodehash in swap router

add test to ensure non-pairs cannot call into callback functions without triggering revert

revert reasons will not be emitted by multicall

i think this eats them. need unit tests to show
https://github.com/Uniswap/uniswap-v3-periphery/blob/ede9e08bd4401160c5cc8d2b72d9be23cc269fa4/contracts/base/Multicall.sol#L14-L15

SelfPermit should support DAI permit

DAI permit has different function signature

create v3 migration contract

should allow uniswap v2-compatible LPs to safely migrate their liquidity to v3

use WETH{9,10} bytecode instead of compiling

that way, we are guaranteed that behavior is the same, and can just use the interfaces to interact

Oracle library

Need a library that returns TWAP for base currency/quote currency over some time period as a FixedPoint128 number

Considerations

• how to choose between multiple pools
• other kinds of averages (EMA)

explore opt-in fee on output tokens

we could either overload unwrapWETH9/sweepToken with extra methods to take a fee to a specified recipient

or, we could make another contract altogether

see if we can thread the error through in multicall

create example fee on input contract

some swapping integrations wish to collect a fee from the user input

we can create an example contract that enables this use case

mint could be separated from increaseLiquidity if token id is counterfactual

we use multicall everywhere for batch operations
mint contains duplicated logic with increaseLiquidity
however we cannot mint a token and then increase its liquidity because we do not know the minted token id until after the mint succeeds

we could reduce the duplication of logic if the token id were counterfactually computed from the sender address + mint nonce, but it would be costly and introduce mint ordering dependency

support `permit` for ERC721s

we should make sure that SelfPermit can also handle ERC721 permits for native positions

allow specifying price limits in swaps

• certainly need to be added to the quoter
• perhaps should be added to the router directly

considerations:

• to limit complexity, price limits would apply to the last/first swap of exact input/output trades, but this leads to a slightly strange API

INonfungiblePositionManager cannot be compiled without optimization

Attempting compilation of INonfungiblePositionManager with no optimization fails with:
"CompilerError: Stack too deep when compiling inline assembly: Variable headStart is 1 slot(s) too deep inside the stack.".

This is probably caused by the function positions(uint256 tokenId) returning too many variables and accessing a slot in the stack that is deeper than its 16th element.

That is a problem for code testing contracts which use the position manager - the solidity coverage plugin tests all code by compiling without optimization, and including the INonfungiblePositionManager in another contract causes the compilation to fail.

Possible solutions are refactoring the function positions(uint256 tokenId) by returning it's variables in a struct, or splitting it in two different functions.

npm version:
"@uniswap/v3-periphery": "^1.0.0-beta.21"

add WETH9<>WETH10 bridge to allow free chaining across pools

Accounting in NonfungiblePositionManager can bleed <1 wei of fees on every action

Per Uniswap/v3-core#424

Every time we trigger modifyPosition, e.g. in increaseLiquidity or decreaseLiquidity, we lose up to 1 wei of fees, exclusive, of both token0/token1

By the time we compute the fees owed to a specific NFT token ID, this loss of precision can accumulate to more than 1 wei of loss to rounding, i.e. is dependent on how many times that position has been poked and the aggregate loss due to rounding for each of those pokes

Thus the NFT contract can be 'insolvent' in that the sum of all positions' computed fees owed is greater than the total number of fees the NFT contract can collect, by greater than 1 wei * number of positions

This amount of loss is acceptable, but should be documented

support eth input in add liquidity

allow pool initialization outside of firstMint

i'm interested in exploring pool creation and initialization as two separate functions. this would let us neatly bundle things with multicall, and is more flexible. e.g. right now if a pool is created outside of the router but not initialized, we're stuck

eip 1271 support (verifying contract signatures)

need this support in the non fungible token contract. this will save us the trouble of doing some kind of batch call from argent

support ETH inputs to swap

add exact output check to Quoter

apply fix from #83

consider using `_safeMint` instead of `_mint`, or exposing it as a separate method

increaseLiquidity does not check for token ID exists

this will fail because pool key will be all 0 which will not ever be a valid pool

ensure `Position` struct is maximally packed

for gas savings

explore design space of additional modifier applied to `payable` functions

right now, the current one works quite well, but requires wrapping the entire ETH balance of the contract instead of just the required value. this imposes a small cost on exact output swappers who are paying in ETH in the case when the contract's ETH balance is greater than the value (anytime they're not pushed to their max slippage tolerance).

it's possible there are improvements to be made here.

current mint params make it difficult to mint the exact amount of desired liquidity

https://github.com/Uniswap/uniswap-v3-periphery/blob/33825d2570eb939ea44abc728e554634368dcd42/contracts/interfaces/INonfungiblePositionManager.sol#L46-L69

compared to the v2 router interface, which takes the desired amounts and min amounts then computes the liquidity amount to mint, this is interface is not as nice because it makes it difficult to spend the desired amount in the case of 0 slippage.

functions not marked payable in NFT cannot be combined via multicall/eth functions

https://github.com/Uniswap/uniswap-v3-periphery/blob/d76cda3befaef02198106ba5b1237fd3d349b500/contracts/NonfungiblePositionManager.sol#L231

e.g., you cannot mint, approve in a single transaction

Multicall being payable is seriously dangerous.

The Multicall.multicall pattern is getting a lot of love lately. I first saw it used by ENS and now in Uniswap-V3-periphery. Yet, I see that your implementation includes the payable keyword. I do believe this is potentially dangerous and should be removed. People might copy that and not see the side-effect.

Example:

pragma solidity >=0.8.2;

import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.0.0/contracts/token/ERC20/ERC20.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.0.0/contracts/utils/Address.sol";

abstract contract Multicall {
    function multicall(bytes[] calldata data) external payable 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) {
                // 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;
        }
    }
}

contract VulnerableWETH is ERC20, Multicall {
    constructor() ERC20('VulnerableWETH', 'VWETH') {}
    
    receive() external payable {
        _mint(msg.sender, msg.value);
    }
    
    function deposit() external payable {
        _mint(msg.sender, msg.value);
    }
    
    function withdraw(uint256 value) external {
        _burn(msg.sender, value);
        Address.sendValue(payable(msg.sender), value);
    }
    
    function withdrawAll() external {
        uint256 value = balanceOf(msg.sender);
        _burn(msg.sender, value);
        Address.sendValue(payable(msg.sender), value);
    }
}

Exploit:
If I batch multiple payable subcall, they will all consider the msg.value. So calling
instance.multicall(["0x","0x","0x","0x","0x"], {value : 10**18 }) will credit me 5eth, despite only having deposited one.

Thus I could drain 4eth from the contract in a single call by doing :
instance.multicall(["0x","0x","0x","0x","0x", "0x853828b6" ], {value : 10**18 })

Beyond this simple example, any contract that considers msg.value, without comparing it to address(this).balance - interbookkeppingBalance is sensible ... this include almost all contracts with payable functions

Advice:
Add extensive warnings in the Multicall contract, or remove payability from it.

PeripheryPayments contract allows any intermediately called contract to reenter and unwrap WETH/sweep tokens

The PeripheryPayments contract allows any intermediately called untrusted contract (e.g. an ERC777 token gets transferred) to call back into the SwapRouter or NFT contract, taking up to the difference between the current balance of WETH9/WETH10 token and the minimum amount expected by the multicall user

NFT Metadata improvements

• Display price/tokens in priority order for ETH & stablecoins (token/ETH, stablecoin/token) #60
• Submit final draft for description text (#94)
• Fuzz Test (#93)
• Account for token symbol with quotes 0_o would break JSON(#104)

investigate router gas overhead

even using the identical routing contract, best-case swaps in periphery cost ~15k more gas than in core. there's almost certainly some SSTORE in a slot that isn't being correctly initialized, let's investigate.

selfPermit and selfPermitIfNecessary are not marked payable

https://github.com/Uniswap/uniswap-v3-periphery/blob/41ed868731f74a4e0b15abe37d7ccb79eba50cb9/contracts/base/SelfPermit.sol#L13-L20

These functions will revert when combined with ETH methods

Reference Pool address

Hi all ,

I according to the core and periphery contracts, there consist of 2 methods of getting the pool address. First is through the factory, another method is through computing the pool address.

UniswapV3Factory.sol

            IUniswapV3Factory(factory).getPool(
                              params.token0,
                              params.token1,
                              params.fee
                          )

---

SwapRouter.sol

            PoolAddress.computeAddress(
                factory,
                PoolAddress.getPoolKey(tokenA, tokenB, fee))

---

However , through my observsation that the return addresses for both are different.

I gently twited the function in swapRouter to public and return the computedAddress for debugging purpose to find out the return address from computeAddress.

I found out that the Pool address return from the Library PoolAddress,compute is not providing the same address as the function getPool in the Factory contract. Therefore , the computeAddress in PoolAddress library is not usable ?
The pool address return from the Factory contract is the actual pool address that contains the bytecode.
Maybe I am wrong. Looking forward to your reply.

Thank you.

Please do initialise some pairs and an early documentation release would be really helpful as well

https://github.com/Uniswap/uniswap-v3-periphery/blob/main/testnet-deploys.md#kovan
The contracts on testnets have been redeployed 2 times and no documentation is to found.

NFT SVG image

Complete first iteration of NFT svg images.
Attributes of Note: Tokens, Fee, tick range (& liquidity?)

NFT metadata in tokenURI

mvp:

• include token names (see https://github.com/Uniswap/uniswap-lib/blob/v4.0.0-alpha/contracts/libraries/SafeERC20Namer.sol)
• include token addresses in additional keys (ideally checksummed)
• include token0/token1 owed
• include token0/token1 value of position liquidity as of current price
• compose a reasonable NFT description from the above information (think network effects of receiving an NFT, reading & learning about Uniswap)

fun ideas:

• hard code a base64 encoded image uri of a uniswap v3 logo (should be svg for file size)
• svgs per different fee level? special image for UNI positions?
• animate the svg

endless loop?

consider adding a balance-checked version of `exactInput` that supports FoT tokens

notes of decreaseLiquidity cause misunderstanding.

notes of decreaseLiquidity in
INonfungiblePositionManager.sol.

/// amount0Min The minimum amount of token0 that should be received in the burn,
/// amount1Min The minimum amount of token1 that should be received in the burn,

they are only added to tokenOwed, not sent to recipient, those notes may cause misunderstanding.
/// @return amount0 The amount of token0 sent to recipient
/// @return amount1 The amount of token1 sent to recipient
ditto.

add flash example contract

compile the swap router with the maximum amount of optimizer runs

the swap router will be called much more frequently and should get the maximum number of optimizer runs

we should configure hardhat to compile it with more runs
https://hardhat.org/config/

support permit in add liquidity

consider emitting events in NonfungiblePositionManager for mint, increaseLiquidity, decreaseLiquidity, collect, burn

while these would be redundant with core events, they would allow separating the liquidity of a particular nft from the aggregate liquidity of the position manager in a subgraph

Consider returning the original revert message with TransferHelper

TransferHelper overwrites the revert message, which makes it impossible to know whether the transaction failed because of insufficient balance, insufficient approval, or something else.

The code to return the revert message is here.

This issue applies as well to TransferHelper in the core repo.