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

Routing #58

Merged
merged 64 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
b6f8d3b
start routing
ewilz Aug 17, 2023
d5dc31a
start routing contract
ewilz Aug 23, 2023
2ca710a
naive swapExactIn impl
ewilz Aug 24, 2023
664d2c2
lint + bytecode snapshot
ewilz Aug 30, 2023
8b687b8
change concept of hops to token hops
ewilz Aug 31, 2023
1717039
UniswapV4Routing --> Routing
ewilz Sep 1, 2023
2549c3a
use PathKey
ewilz Sep 1, 2023
e1c46fd
exactInputSingle
ewilz Sep 1, 2023
8aae01b
save DRY progress
ewilz Sep 1, 2023
5823d2f
no sqrtPriceLimit for multipool hops
ewilz Sep 5, 2023
7e25390
exactOut implemented w awkward loops/int conversions
ewilz Sep 5, 2023
607ea3b
gas savings from not doing double negative number
ewilz Sep 5, 2023
cf82527
gas savings from unchecked math
ewilz Sep 5, 2023
9ebc9d5
add swapExactOuputSingle
ewilz Sep 5, 2023
54a48e5
break out structs into interface
ewilz Sep 5, 2023
01a5db0
PR comments
ewilz Sep 25, 2023
31cbfe5
pass hook data along
ewilz Sep 26, 2023
d50f18d
gas and coherency optimization
ewilz Oct 2, 2023
4b010ff
updated lib/v4-core submodule to main branch
dianakocsis Mar 19, 2024
7c6675b
Merge branch 'main' into routing
hensha256 Jun 28, 2024
aa148b3
feat: abstract router (#86)
ConjunctiveNormalForm Jun 28, 2024
1586c0d
allow payer and recipient to be different
hensha256 Jul 2, 2024
06961bf
try to fix ci
hensha256 Jul 5, 2024
528f15d
improve casting
hensha256 Jul 5, 2024
0ac638a
reverting 3 commits to make branch
hensha256 Jul 17, 2024
1844d70
remove unused recipient param
hensha256 Jul 17, 2024
86039e4
remove payment logic from swap logic
hensha256 Jul 17, 2024
90f3f35
factor out payment functions
hensha256 Jul 17, 2024
50372c7
gas opt removing struct
hensha256 Jul 17, 2024
64f4645
gas opt: decode in assembly
hensha256 Jul 17, 2024
e92b023
Merge branch 'main' into routing
hensha256 Jul 18, 2024
8283fb6
merge error
hensha256 Jul 18, 2024
7aacd94
merge main
hensha256 Jul 18, 2024
f2a3dd8
merge main
hensha256 Jul 23, 2024
4792d1c
merge routing
hensha256 Jul 23, 2024
903d5fa
use base actions router in v4router
hensha256 Jul 23, 2024
f1c4746
use isolate
hensha256 Jul 23, 2024
4b7d019
Merge pull request #155 from Uniswap/v4-routing
hensha256 Jul 23, 2024
f555329
Merge branch 'main' into routing
hensha256 Jul 23, 2024
9c11de4
merge conflicts
hensha256 Jul 23, 2024
b2fdb51
removing submodules
hensha256 Jul 23, 2024
963c360
renaming to stop clashes in UR
hensha256 Jul 24, 2024
eb801c7
Merge branch 'main' into routing
hensha256 Jul 24, 2024
86fd91a
Merge branch 'main' into routing
hensha256 Jul 25, 2024
22c0879
PR comments and gas optimisations
hensha256 Jul 25, 2024
3a4c36d
add _getLocker
hensha256 Jul 25, 2024
e86036b
Amount field on settle and take
hensha256 Jul 25, 2024
2236812
rename snaps
hensha256 Jul 25, 2024
456d0c8
correcy casting order
hensha256 Jul 25, 2024
9e5e418
Merge branch 'main' into routing
hensha256 Jul 25, 2024
5ceaadf
Add deltaresolver to router
hensha256 Jul 25, 2024
20224d7
take_all command, remove from delta resolver
hensha256 Jul 26, 2024
d02049b
Routing test helper
hensha256 Jul 26, 2024
ff206ec
Separate gas tests and regular tests
hensha256 Jul 26, 2024
09b2e23
remove duplicate snapshot name
hensha256 Jul 26, 2024
b51f0ea
another gas test
hensha256 Jul 26, 2024
b625d74
merge main
hensha256 Jul 26, 2024
d1d66ec
Merge branch 'main' into routing
hensha256 Jul 26, 2024
e2ae6d3
Merge branch 'main' into routing
hensha256 Jul 29, 2024
7094a75
PR comments
hensha256 Jul 29, 2024
9ee5768
remove unused function
hensha256 Jul 29, 2024
f14c6f0
Merge branch 'main' into routing
hensha256 Jul 29, 2024
7545fa3
PR cmments
hensha256 Jul 29, 2024
a92da59
handle hook edgecase
hensha256 Jul 29, 2024
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
2 changes: 1 addition & 1 deletion .forge-snapshots/BaseActionsRouter_mock10commands.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
62824
62960
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_collect.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
162252
162329
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_collect_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
153467
153544
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_collect_sameRange.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
162252
162329
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_decreaseLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
127630
127707
Original file line number Diff line number Diff line change
@@ -1 +1 @@
119010
119087
Original file line number Diff line number Diff line change
@@ -1 +1 @@
140511
140588
Original file line number Diff line number Diff line change
@@ -1 +1 @@
156808
156885
Original file line number Diff line number Diff line change
@@ -1 +1 @@
142246
142323
Original file line number Diff line number Diff line change
@@ -1 +1 @@
148668
148711
Original file line number Diff line number Diff line change
@@ -1 +1 @@
184822
184899
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
417817
417894
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
366027
366104
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_nativeWithSweep.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
372965
373042
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_onSameTickLower.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
360499
360576
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_onSameTickUpper.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
361141
361218
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_sameRange.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
286723
286800
Original file line number Diff line number Diff line change
@@ -1 +1 @@
366517
366594
Original file line number Diff line number Diff line change
@@ -1 +1 @@
462307
462384
1 change: 1 addition & 0 deletions .forge-snapshots/V4Router_Bytecode.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
6189
1 change: 1 addition & 0 deletions .forge-snapshots/V4Router_ExactIn1Hop.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
136315
1 change: 1 addition & 0 deletions .forge-snapshots/V4Router_ExactIn2Hops.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
187572
1 change: 1 addition & 0 deletions .forge-snapshots/V4Router_ExactIn3Hops.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
245746
1 change: 1 addition & 0 deletions .forge-snapshots/V4Router_ExactInputSingle.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
135175
1 change: 1 addition & 0 deletions .forge-snapshots/V4Router_ExactOut1Hop.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
135133
1 change: 1 addition & 0 deletions .forge-snapshots/V4Router_ExactOut2Hops.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
187005
1 change: 1 addition & 0 deletions .forge-snapshots/V4Router_ExactOut3Hops.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
243778
1 change: 1 addition & 0 deletions .forge-snapshots/V4Router_ExactOutputSingle.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
133709
160 changes: 160 additions & 0 deletions src/V4Router.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;

import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol";
import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol";
import {TickMath} from "@uniswap/v4-core/src/libraries/TickMath.sol";

import {PathKey, PathKeyLib} from "./libraries/PathKey.sol";
import {CalldataBytesLib} from "./libraries/CalldataBytesLib.sol";
import {IV4Router} from "./interfaces/IV4Router.sol";
import {BaseActionsRouter} from "./base/BaseActionsRouter.sol";
import {DeltaResolver} from "./base/DeltaResolver.sol";
import {Actions} from "./libraries/Actions.sol";

/// @title UniswapV4Router
/// @notice Abstract contract that contains all internal logic needed for routing through Uniswap V4 pools
/// @dev the entry point to executing actions in this contract is calling `BaseActionsRouter._executeActions`
/// An inheriting contract should call _executeActions at the point that they wish actions to be executed
abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
using PathKeyLib for PathKey;
using CalldataBytesLib for bytes;

constructor(IPoolManager poolManager) BaseActionsRouter(poolManager) {}
hensha256 marked this conversation as resolved.
Show resolved Hide resolved

// TODO native support !!
function _handleAction(uint256 action, bytes calldata params) internal override {
// swap actions and payment actions in different blocks for gas efficiency
if (action < Actions.SETTLE) {
if (action == Actions.SWAP_EXACT_IN) {
_swapExactInput(abi.decode(params, (IV4Router.ExactInputParams)));
hensha256 marked this conversation as resolved.
Show resolved Hide resolved
} else if (action == Actions.SWAP_EXACT_IN_SINGLE) {
_swapExactInputSingle(abi.decode(params, (IV4Router.ExactInputSingleParams)));
} else if (action == Actions.SWAP_EXACT_OUT) {
_swapExactOutput(abi.decode(params, (IV4Router.ExactOutputParams)));
} else if (action == Actions.SWAP_EXACT_OUT_SINGLE) {
_swapExactOutputSingle(abi.decode(params, (IV4Router.ExactOutputSingleParams)));
} else {
revert UnsupportedAction(action);
}
} else {
if (action == Actions.SETTLE) {
// equivalent: abi.decode(params, (Currency, uint256))
Currency currency;
uint256 amount;
assembly ("memory-safe") {
currency := calldataload(params.offset)
amount := calldataload(add(params.offset, 0x20))
}

// TODO support address(this) paying too
hensha256 marked this conversation as resolved.
Show resolved Hide resolved
// TODO should it have a maxAmountOut added slippage protection?
Copy link
Member

Choose a reason for hiding this comment

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

my thought is that slippage protection should be with the swap commands? why do we also need slippage protection with settle commands?

i think we can make settle just take an amount, and if you're ok with the full settle amount then you pass in a constant/sentinel

Copy link
Contributor

Choose a reason for hiding this comment

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

Its to support split route trades. Lets say youre trading 1000 DAI for USDC, where you want at least 998 USDC

  • 500 DAI -> USDT -> USDC
  • 500 DAI -> USDC

Then you would do a command thats like "take all USDC, and check we're receiving at least 998".

Copy link
Contributor

Choose a reason for hiding this comment

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

We implement the same style of logic in UR using the sweep command e.g. here

Copy link
Contributor

Choose a reason for hiding this comment

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

For v3 split routes it trades both routes, with the recipient of the UR, and then calls SWEEP(USDC, 998) which will revert the trade if 998 weren't received

Copy link
Contributor

Choose a reason for hiding this comment

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

And we do it with the unwrapETH command to do the same check for ETH output trades e.g. here

Copy link
Member

Choose a reason for hiding this comment

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

but surely you're also checking slippage from USDT to USDC and from DAI to USDC? or are you ignoring those checks until the end? My concern is that we're doing lots of slippage checks and the ones in the middle then dont even matter?

Copy link
Member

Choose a reason for hiding this comment

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

My other thought process then also is just if posm & routers take/settle commands looks different now bc I was not planning on doing extra slippage checks on take/settle. (IK this is a future TODO but just adding my thoughts in this thread. Not blocking on getting this PR merged)

Copy link
Contributor

Choose a reason for hiding this comment

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

but surely you're also checking slippage from USDT to USDC and from DAI to USDC? or are you ignoring those checks until the end? My concern is that we're doing lots of slippage checks and the ones in the middle then dont even matter?

yeah the ones in the middle dont matter. the user doesnt care if one half of the trade gets hit with more slippage than the other - theres just an output amount that they want from the trade in total and thats it

Copy link
Contributor

Choose a reason for hiding this comment

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

for split route trades we pass in 0 as minAmountOut at each stage - and then do aggregated slippage checks on total output

Copy link
Contributor

Choose a reason for hiding this comment

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

My other thought process then also is just if posm & routers take/settle commands looks different now

yeah i could do it as a different cmmand or something depending how the other commands end up

_settle(currency, _msgSender(), amount);
} else if (action == Actions.TAKE) {
// equivalent: abi.decode(params, (Currency, address, uint256))
Currency currency;
address recipient;
uint256 amount;
assembly ("memory-safe") {
Copy link
Member

Choose a reason for hiding this comment

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

#199 just tagging that 199 should also put these in a lib so we can use in both contracts

currency := calldataload(params.offset)
recipient := calldataload(add(params.offset, 0x20))
amount := calldataload(add(params.offset, 0x40))
}

// TODO should _take have a minAmountOut added slippage check?
_take(currency, recipient, amount);
} else {
revert UnsupportedAction(action);
}
}
}

function _swapExactInputSingle(IV4Router.ExactInputSingleParams memory params) private {
_swap(
params.poolKey,
params.zeroForOne,
int256(-int128(params.amountIn)),
params.sqrtPriceLimitX96,
params.hookData
);
}

function _swapExactInput(IV4Router.ExactInputParams memory params) private {
unchecked {
// Caching for gas savings
uint256 pathLength = params.path.length;
uint128 amountOut;
uint128 amountIn = params.amountIn;
Currency currencyIn = params.currencyIn;
PathKey memory pathKey;

for (uint256 i = 0; i < pathLength; i++) {
hensha256 marked this conversation as resolved.
Show resolved Hide resolved
pathKey = params.path[i];
(PoolKey memory poolKey, bool zeroForOne) = pathKey.getPoolAndSwapDirection(currencyIn);
amountOut = uint128(_swap(poolKey, zeroForOne, -int256(uint256(amountIn)), 0, pathKey.hookData));

amountIn = amountOut;
currencyIn = pathKey.intermediateCurrency;
}

if (amountOut < params.amountOutMinimum) revert TooLittleReceived();
}
}

function _swapExactOutputSingle(IV4Router.ExactOutputSingleParams memory params) private {
_swap(
params.poolKey,
params.zeroForOne,
int256(int128(params.amountOut)),
params.sqrtPriceLimitX96,
params.hookData
);
}

function _swapExactOutput(IV4Router.ExactOutputParams memory params) private {
unchecked {
// Caching for gas savings
uint256 pathLength = params.path.length;
uint128 amountIn;
uint128 amountOut = params.amountOut;
Currency currencyOut = params.currencyOut;
PathKey memory pathKey;

for (uint256 i = pathLength; i > 0; i--) {
pathKey = params.path[i - 1];
(PoolKey memory poolKey, bool oneForZero) = pathKey.getPoolAndSwapDirection(currencyOut);
amountIn = uint128(-_swap(poolKey, !oneForZero, int256(uint256(amountOut)), 0, pathKey.hookData));
Copy link
Member

Choose a reason for hiding this comment

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

I feel like I commented at some point about documenting/adding a little comment about the return signs but can't find it anywhere.. Anyways I do think it would be nice explaining that the output is guaranteed to be negative for exactOutput in the reciprocal token.

Copy link
Contributor

Choose a reason for hiding this comment

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

have actually added a safecast on this - i think with custom accounting hooks on low liquidity pools its technically not guaranteed


amountOut = amountIn;
currencyOut = pathKey.intermediateCurrency;
}
if (amountIn > params.amountInMaximum) revert TooMuchRequested();
}
}

function _swap(
PoolKey memory poolKey,
bool zeroForOne,
int256 amountSpecified,
uint160 sqrtPriceLimitX96,
bytes memory hookData
) private returns (int128 reciprocalAmount) {
unchecked {
BalanceDelta delta = poolManager.swap(
poolKey,
IPoolManager.SwapParams(
zeroForOne,
amountSpecified,
sqrtPriceLimitX96 == 0
? (zeroForOne ? TickMath.MIN_SQRT_PRICE + 1 : TickMath.MAX_SQRT_PRICE - 1)
: sqrtPriceLimitX96
),
hookData
);

reciprocalAmount = (zeroForOne == amountSpecified < 0) ? delta.amount1() : delta.amount0();
}
}
}
10 changes: 5 additions & 5 deletions src/base/BaseActionsRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ pragma solidity ^0.8.24;

import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {SafeCallback} from "./SafeCallback.sol";
import {BytesLib} from "../libraries/BytesLib.sol";
import {CalldataBytesLib} from "../libraries/CalldataBytesLib.sol";

/// @notice Abstract contract for performing a combination of actions on Uniswap v4.
/// @dev Suggested uint256 action values are defined in Actions.sol, however any definition can be used
abstract contract BaseActionsRouter is SafeCallback {
using BytesLib for bytes;
using CalldataBytesLib for bytes;

/// @notice emitted when different numbers of parameters and actions are provided
error LengthMismatch();
error InputLengthMismatch();

/// @notice emitted when an inheriting contract does not support an action
error UnsupportedAction(uint256 action);
Expand All @@ -30,7 +30,7 @@ abstract contract BaseActionsRouter is SafeCallback {
(uint256[] calldata actions, bytes[] calldata params) = data.decodeInCalldata();

uint256 numActions = actions.length;
if (numActions != params.length) revert LengthMismatch();
if (numActions != params.length) revert InputLengthMismatch();

for (uint256 actionIndex = 0; actionIndex < numActions; actionIndex++) {
uint256 action = actions[actionIndex];
Expand All @@ -49,6 +49,6 @@ abstract contract BaseActionsRouter is SafeCallback {
/// @dev The other context functions, _msgData and _msgValue, are not supported by this contract
/// In many contracts this will be the address that calls the initial entry point that calls `_executeActions`
/// `msg.sender` shouldnt be used, as this will be the v4 pool manager contract that calls `unlockCallback`
/// If using ReentrancyLock.sol, this function can return Locker.get() - locker of the contract
/// If using ReentrancyLock.sol, this function can return _getLocker()
function _msgSender() internal view virtual returns (address);
}
22 changes: 22 additions & 0 deletions src/base/DeltaResolver.sol
Original file line number Diff line number Diff line change
@@ -1,17 +1,33 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.24;

import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
import {ImmutableState} from "./ImmutableState.sol";
import {TransientStateLibrary} from "@uniswap/v4-core/src/libraries/TransientStateLibrary.sol";

/// @notice Abstract contract used to sync, send, and settle funds to the pool manager
/// @dev Note that sync() is called before any erc-20 transfer in `settle`.
abstract contract DeltaResolver is ImmutableState {
using TransientStateLibrary for IPoolManager;

/// @notice Value used to signal that an entire open delta should be taken/settled
uint256 public constant ENTIRE_OPEN_DELTA = 0;

/// @notice Emitted trying to settle a positive delta, or take a negative delta
Copy link
Member

Choose a reason for hiding this comment

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

should we make this type(uint256).max so its super explicit? Also matches other patterns like approving with type(uint256).max to mean my entire balance

Copy link
Member

Choose a reason for hiding this comment

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

Oh weird.. on codespaces the code I was reviewing had the changes where you kept the flag for checking to use the contract balance.. I guess we can do that in a different PR!

error InvalidDeltaForAction();

/// @notice Take an amount of currency out of the PoolManager
/// @param currency Currency to take
/// @param recipient Address to receive the currency
/// @param amount Amount to take
function _take(Currency currency, address recipient, uint256 amount) internal {
if (amount == ENTIRE_OPEN_DELTA) {
hensha256 marked this conversation as resolved.
Show resolved Hide resolved
int256 delta = poolManager.currencyDelta(address(this), currency);
if (delta < 0) revert InvalidDeltaForAction();
amount = uint256(delta);
}

poolManager.take(currency, recipient, amount);
}

Expand All @@ -21,6 +37,12 @@ abstract contract DeltaResolver is ImmutableState {
/// @param payer Address of the payer
/// @param amount Amount to send
function _settle(Currency currency, address payer, uint256 amount) internal {
if (amount == ENTIRE_OPEN_DELTA) {
int256 delta = poolManager.currencyDelta(address(this), currency);
if (delta > 0) revert InvalidDeltaForAction();
amount = uint256(-delta);
}

if (currency.isNative()) {
poolManager.settle{value: amount}();
} else {
Expand Down
4 changes: 4 additions & 0 deletions src/base/ReentrancyLock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@ contract ReentrancyLock {
_;
Locker.set(address(0));
}

function _getLocker() internal view returns (address) {
return Locker.get();
}
}
Loading
Loading