fixed issue with example

[fasteater]

The fee charging example was incorrect because it was charging the fee in currency0 when swap zeroForOne, while it should take into account the amountSpecified together with zeroForOne to determine the specifiedAmount thus feeAmount is in currency0 or currency1, then it should charge the corresponding currency to the user.

[vercel]

@fasteater is attempting to deploy a commit to the Uniswap Team on Vercel.

A member of the Team first needs to authorize it.

[zolotokrylin]

We need clarity as soon as possible since we are using the original example. If it is incorrect, we must know before going to the mainnet.

🙏

[fasteater]

Hey I have validated this issue in the uniswap discord channel, I am sure this is a problem and I think it's important to fix this asap because I know some devs are using this example in live project.

However my proposed fix here might not be sufficient because it doesn't work to use beforeswap hook to charge a fee in tokenOut amt.

I think afterswap hook is needed here. I will propose further fix on this when I have it figured out.

[zolotokrylin]

I will contact the Uniswap team in X to speed up the response.

[lwtsn]

From @georgeciubotaru

the flag zeroForOne is for explicitly say which is the token input that will be taken by the Uniswap pool to pay for the swap.

struct SwapParams {
    /// Whether to swap token0 for token1 or vice versa
    bool zeroForOne;
    /// The desired input amount if negative (exactIn), or the desired output amount if positive (exactOut)
    int256 amountSpecified;
    /// The sqrt price at which, if reached, the swap will stop executing
    uint160 sqrtPriceLimitX96;
}

so amountSpecified > 0 means I want x token1 take y of token0 using conversion
amountSpecified <0 mean I give you x token0 and give me y of token1 using conversion.

so no matter of the value of amountSpecified the token will be taken for paying the swap is based on the zeroForOne flag.

[fasteater]

@lwtsn I dont think this is correct, as per your comment

so amountSpecified > 0 means I want x token1 take y of token0 using conversion -> in this case means amountSpecified = y in token1

amountSpecified <0 mean I give you x token0 and give me y of token1 using conversion. -> in this case amountSpecified = x in token0

So depending on amountSpecified being neg or positive, the value means differrent token type. Thus the fee based on this value is also in different token type.

so no matter of the value of amountSpecified the token will be taken for paying the swap is based on the zeroForOne flag. -> this is saying no matter amountSpecified being neg or positive, its value is alway in token0 if swapping ZeroForOne = true. This is simply not correct.

I believe this is a different pattern from normal uniswap fees which is always charged in token0. In case of amountSpecified is token1, it will applies the fee to the converted token0 amount. But here the fee is calculated directly based on amountSpecified.

[lwtsn]

@georgeciubotaru could you please follow up /w @fasteater

[zolotokrylin]

Is not the high-level logic the following?:

Given: Token A and Token B
OneToZero == true (exchange token A to token B)

If you want to spend exact amount (exactIn) of token A, then you will get in exchange the amount of B where B_amount=getAmount(exactIn) and where getAmount takes care of conversion rate and fee which is paid in token A.

If you want to receive exact amount of B (exactOut), then you will need to exchange the amount of A where A_amount=getAmount(exactOut) and where getAmount take care of the conversion rate and fee which is paid in token A.

---

If the above is correct understanding of the swap logic, then the current documentation example is correct, so is the @georgeciubotaru

[0xcoreblock]

Sorry, my previous comment went wrong.

I believe @zolotokrylin and @georgeciubotaru are correct.

In Uniswap, fees should be based on the input token, which is determined by the zeroForOne flag.

@fasteater — I think you're using amountSpecified to determine the fee token directly, which seems to be a misunderstanding.

The amountSpecified parameter is used to determine whether it’s an exact input or exact output swap, but not which token the fee is paid in.

[lwtsn]

@fasteater could you please weigh in - we are confident that the existing solution in the docs is correct. Please let me know your thoughts

[fasteater]

I am confident this issue is legit, I have also found evidence in the V4 core tests which does the same thing as I proposed here. When determine specifiedCurrency, it takes into account both ZeroForOne and specifiedAmount, we need both of these, not just one as in the example.

I have also updated the proposed fix to be more efficient, but the logic remains the same. And I have further looked into the issue and I can confirm the proposed changes is sufficient to deal with the fees here, we do not need afterSwapHook at all. This update fully fixes the issue, should be merged asap.

Also let me try to clear any confusion here

• Fee logic here is different from normal swap. Normal swap where it is always charged in input token(be careful with the difference, input token does not always equal to specifiedToken), because it is easier and normal swap we have access to both input and output amount, we can calculate backwards to get the input amount and charge fee always on input token. But the logic with the hook is different, here we are at beforeSwapHook, we do not have access to out put amount yet, so we have to charge fee on specifiedAmount, which can be both token0 or token1 when swapping zeroForOne, so we are taking fees sometimes in token0 sometimes in token1 depending on the actual swap.

• The beforeTokenSwap hook will handle the delta value deduction from the swap, if specified in token0, the delta will be deducted from the specified amount because amountSpecified < 0. if it is token 1, it will be added to the specified amount. Be aware here delta value is a positive value representing a fee to be deducted, if for some reason you want to add extra to the swap amount using the hook not charging a fee, this delta value needs to be negative.

• A detailed example of test and Contract example can be found in V4 core as linked.

[lwtsn]

@zolotokrylin @georgeciubotaru fyi