Uniswap / v3-sdk

🛠 An SDK for building applications on top of Uniswap V3
MIT License
547 stars 425 forks source link

is `encodeRouteToPath` second param really `exactOutput` ? #103

Open Amxx opened 2 years ago

Amxx commented 2 years ago

Hello,

Lets start by the contract I'm writing. It is supposed to swap arbitrary user token to the one my system uses, before actually doing stuff.

function deposit(address receiver, uint256 amountIn, uint256 amountOutMinimum, bytes memory path) external payable {
    uint256 balanceBefore = USDC.balanceOf(receiver);

    if (value > 0) { 
        ...
    } else if (path.length == 0) {
        ...
    } else {
        // get first asset in path
        IERC20 assetIn = IERC20(path.toAddress(23));

        // take custody
        SafeERC20.safeTransferFrom(assetIn, msg.sender, address(this), amountIn);
        SafeERC20.safeApprove(assetIn, address(uniswapRouter), type(uint256).max);

        // start routing
        uniswapRouter.exactInput(ISwapRouter.ExactInputParams({
            path:              path,
            recipient:         owner,
            deadline:          block.timestamp,
            amountIn:          amountIn,
            amountOutMinimum:  amountOutMinimum
        }));
    }

    uint256 received = USDC.balanceOf(receiver) - balanceBefore;

    // DO STUFF
}

My issue is in building the path

So far I've been doing

const router = new AlphaRouter({ chainId: 1, provider });
const trade = await router.route(
    CurrencyAmount.fromRawAmount(TOKENS.USDT, amount),
    TOKENS.USDC,
    TradeType.EXACT_IN,
).then(({ trade }) => encodeRouteToPath(trade.routes[0]));

the resulting trade is USDC.address | fee | USDT.address ... hence the path.toAddress(23) in the contract. → but that fails with STF.

So what I did is changing encodeRouteToPath(trade.routes[0]) to encodeRouteToPath(trade.routes[0], true) and path.toAddress(23) to path.toAddress(0) → this works!

So I'm confused, because the true params is supposed to be for exactOutput ... and I'm using exactInput. I'm afraid this might break with longer paths ...

Am I doing things wrong, or is the docs mistaken?

marktoda commented 2 years ago

Hey @Amxx - apologies for the long delay on this! Hopefully you've figured out this issue by now, but I'll see if I can help in case not :)

The exactOutput boolean switches between whether path is listed in forward or reverse order (since exactOutput swaps are executed in reverse). The thing that stands out to me that may be causing your issue is this line in your contract:

    IERC20 assetIn = IERC20(path.toAddress(23));

In exactInput swaps the input asset is assumed to be the first token of the first pool in the route (i.e. see https://github.com/Uniswap/v3-periphery/blob/2880364dcde105b903d4bf1bea94105b14cbbd35/contracts/SwapRouter.sol#L96 and https://github.com/Uniswap/v3-periphery/blob/2880364dcde105b903d4bf1bea94105b14cbbd35/contracts/libraries/Path.sol#L51). So I would guess that if you used offset 0 as tokenIn it would solve the issue.

Let me know if helpful or any other way we can help!