How do you handle rounding errors between querySwap and swap?

[jamescarter-le]

It's only a small issue, especially when used for back testing and your not going to make any live transactions from the results, and you query the chain for its state at the time, however I've noticed an issue with regards to rounding with querySwap vs swap when sqrtPrice is applied, however I would expect it to resolve to the same as the Amounts in the EventLog of Swap?

This is how you determine amountRequested from your tests: https://github.com/Bella-DeFinTech/uniswap-v3-simulator/blob/dev/scripts/Events.ts

Sorry its not in your unit test format, I just wanted to verify it was working for my desired pool before hooking it up:

import JSBI from 'jsbi';
import { CorePool } from './src/core/CorePool';
import { FeeAmount } from './src/enum/FeeAmount';

function main()
{
    // Pool is placed in the Initialized state, with no operations yet invoked apart from it being Initialized.
    var pool = new CorePool("", "", FeeAmount.MEDIUM, 60, JSBI.BigInt(0), JSBI.BigInt(0), JSBI.BigInt("2505290050365003892876723467"),
    JSBI.BigInt(0), -69082, JSBI.BigInt(0), JSBI.BigInt(0));

    // Block 13578816   Tx 0x2f0461509914dbd045ae6946a26f69850b914f97a67dd6078fa5d871959fb105  LogIndex  16
    pool.mint("0xC36442b4a4522E871399CD717aBDD847Ab11FE88", 49800, 64020, JSBI.BigInt("556973545490136947176"));
    // Block 13578904   Tx 0x28fe258a62cf142fced434c5eb3ff051970470333b5770118c8df225e374849b  LogIndex  460
    pool.burn("0xC36442b4a4522E871399CD717aBDD847Ab11FE88", 49800, 64020, JSBI.BigInt("529124868215630099817"));
    // Block 13578904   Tx 0x28fe258a62cf142fced434c5eb3ff051970470333b5770118c8df225e374849b  LogIndex  462
    pool.burn("0xC36442b4a4522E871399CD717aBDD847Ab11FE88", 49800, 64020, JSBI.BigInt("0"));

    // Block 13578932   Tx 0x4d8a042cae977aa124cda85419461093fb87b04814126e024a05dc4f8252e27a  LogIndex  187
    var amount0 = JSBI.BigInt("-927248711787417535");
    var amount1 = JSBI.BigInt("226019397190954581334");
    var sqrtPrice = JSBI.BigInt("1596559182082899146010277864392");

    var zeroForOne: boolean = JSBI.greaterThan(amount0, JSBI.BigInt("0")) ? true : false;

    var queryResult = pool.querySwap(zeroForOne, amount1);
    if(!JSBI.equal(queryResult.amount0, amount0))
        console.log("Amount1: Amount 0 Wrong");
    if(!JSBI.equal(queryResult.amount1, amount1))
        console.log("Amount1: Amount 1 Wrong");

    var applyResult = pool.swap(zeroForOne, amount0, sqrtPrice);
    if(!JSBI.equal(applyResult.amount0, amount0))
        console.log("Apply: Amount 0 Wrong");
    if(!JSBI.equal(applyResult.amount1, amount1))
        console.log("Apply: Amount 1 Wrong: " + applyResult.amount1.toString() + "  vs expected  " + amount1.toString());// <---- Error, is out by 1 LSB.
}

main();

[jamescarter-le]

It only checks as below, and so test passes, but the actual result is off by 1 LSB (at least in 18 decimal token pools):

https://github.com/Bella-DeFinTech/uniswap-v3-simulator/blob/cf3bc45418e48c70d3c947aeaed3e418cde5f9f6/test/ConfigurableCorePool.test.ts#L139-L143

It should check as per try run:

https://github.com/Bella-DeFinTech/uniswap-v3-simulator/blob/cf3bc45418e48c70d3c947aeaed3e418cde5f9f6/test/ConfigurableCorePool.test.ts#L115-L118

Are you saying with that test, that the amount outs don't matter, the Pool and therefore Position and Tick liquidities are all correct?

[kafeikui]

@jamescarter-le Good question. Actually check on sqrtPriceX96 after one swap is stricter than check on amount outs. If two attempts of dry run(querySwap) fail, swap will not be executed, instead it will throw an error.

I agree that we should also provide the sqrtPrice as EventLog records in querySwap, but just to clarify so far with USDC-WETH pool we haven't got an error by that. Because we give the value of amountSpecified from amount outs of the EventLog, the influence of sqrtPrice should not be obvious to amount outs. But if we don't specify that in following swap, the result of sqrtPrice can differ from EventLog. In fact I'm quite interested in 1 LSB error(in amount outs right?) you mentioned. Could you please reproduce that more specifically?

[kafeikui]

Thank you @jamescarter-le . I see your point. Let me check that.

[jamescarter-le]

Thanks @kafeikui If I do not use the sqrtPrice when doing the swap (letting the pool execute the best swap) for when running through a Back Test (as we wouldn't be able to use that Price, as the tokens locked in the pool will change, hence the price the user got will change), it also ends up with an LSB error.

I was under the impression that assuming you make no change to the order of operations, taken directly from the chain, it should wrap up to the current state of the chain?

Here is an example when you go further down that ETH/ENS pool: 0x92560C178cE069CC014138eD3C2F5221Ba71f58a

https://gist.github.com/jamescarter-le/0f815e6134bd8f299e5c4a9e3a34ae52

[jamescarter-le]

I think I'll pull down the USDC/ETH pool so I'm testing against the same data set as you for validation.

Am I correct in thinking it is this pool? https://etherscan.io/address/0x8ad599c3a0ff1de082011efddc58f1908eb6e6d8

[jamescarter-le]

So the first rounding issue I found when opening the issue looks like a rounding issue with sqrtRatioTarget.

When you pass no limit onto HandleSwap, it uses the full step.sqrtPriceNextX96 for computeStepSwap (because each step price will never be greater than MAX_RATIO_SQRT), however when you have a price that is between the steps, it uses the supplied price limit for this function.

Then with this limit rather that MAX_RATIO_SQRT, it ends up missing a rounding on the Amount.

Here is the handleSwap with the price limit for the original top issue.:

This gives us the following AmountIn and Fee's calculated:

Whereas when we pass MAX_RATIO_SQRT into this, we get into the ""//we didn't reach the target, so take the remainder of the maximum input as fee"" section. Resulting in this:

This is the correct amount for the EventLog Swap.

Now on the Limit version, it knows it is exactIn, but we also have reached the sqrtRatioNextX96 and sqrtRatioTargetX96 (because we input these, and they've been precalculated by Uniswap already as we are re-processing).

It runs the FeeAmount with the mulDiv rounding, I assume to ensure the user doesn't over spend on fees?

I'm not a math genius and so I'm having trouble to figure what to change here, but mulDivRoundingUp calculates 2 LSB off, then rounds up one. We end up being one out at the end.

Hopefully we can get this sorted? I'm hoping this error is compounding through the chain, as if I ignore it I can process quite a few events before it blows up again on this pool.

TLDR: When passing the matching SqrtPriceLimitX96 to ComputeSwap, it will in some circumstances provide the wrong Fee amount, hence the output not matching the Ethereum Event Log.

[jamescarter-le]

Also, ran this on your ETH/USDC 3000 pool, and got this rounding problem at this point:

Swap Block 12403791 Tx 150 Log 297 0x3d9dd52a8e903b0fc02b2c393dc975b0c726d556410f20a690746086575d9cd5

[neptune-v]

@jamescarter-le Thanks for the detailed explanation of your findings, let me check with @kafeikui to see what exactly we can do to fix this. Please note tho, we had rounding/precision issues before, and we had to fix some logic, so what you are seeing might be because you are on a branch without the fix (needs confirmation), but we will get back to you soon. Thanks again!!

[kafeikui]

Impressive. You're right @jamescarter-le . The problem has been verified with our test data set and I have commited a fix by refining try-error process. Please see #53 . Besides, you know what we are doing is to reproduce logic and result exactly as Uniswap v3 contract do, so I do think the rounding problem is against that but will have little influence in strategy backtesting. When doing backtesting, we only pass amountSpecified(chosen from amount0/amount1 in event record) to swap because the actions from user's strategy might lead swap to fail(for example, it can't meet sqrtPriceLimitX96 condition if we pass it). That is why that from my perspective maybe swap inputs decided by try-error swap events is better than using input by parsing swap trxs directly. And of course, if you are doing events-replay work directly from the chain, try this fix and let me know if you have any problem.

Thanks a lot!

BTW, your effort and understanding on Uniswap v3 is highly appreciated. Have you considered participating in our project? We can make things better.

[jamescarter-le]

Hello both, many thanks for responding to my raised issue.

Yes I agree that the literal price a user gets is not too important from a backtesting point of view (apart from events like arbitrage and MEV).

The main issue was that I was not able to sync the full state of the Pool from the Event Logs on chain, I want to be able to do this to verify this code is working correctly before I allow automated transactions from this. (This is not a criticism, trust but verify 👍 ).

I think your approach of validating first using the Transaction call data through the full data set is a good one, and would serve to ensure that the code replicates exactly the UniswapV3Pool behavior, then we can confirm we can 'fast-forward' through this with the Event Logs, and then fork or snapshot as you say, the Pool state to make alterations to pool state in time.

I'm happy to contribute any bugs I can find, however TypeScript isn't my preferred environment (I'm a .NET person really, hence my scaffold code in this issue and not using your TS test setup) and so I was using uniswap-v3-simulator to validate my own simulation of the Pool, I get similar errors, and so thought to fix uniswap-v3-simulator with a team already experienced and use this to validate my dotnet version.

I was attempting to set up an EVM environment to debug the UniswapV3Pool directly to verify, but maybe pulling Tx calldata would be simpler as an initial goto. I've not done very much work with EVM, although I do have significant Blockchain experience from BTC.

[jamescarter-le]

Also, I was on the dev branch, as linked to on my previous 'help wanted' issue, can you recommend me the best branch?

[jamescarter-le]

I did manage to get the UniswapV3Pool deployed to test, and the 'bug' is also present there, so I think your suggestion of #53 is probably the best option unfortunately. When we pass the PriceLimit into the Pool, it will return a different value (in some circumstances) than if you do not, even though the PriceLimit is the same as that returned by Uniswap itself.

I've created an issue on v3 core for their feedback: https://github.com/Uniswap/v3-core/issues/501

[kafeikui]

Also, I was on the dev branch, as linked to on my previous 'help wanted' issue, can you recommend me the best branch?

Just use main branch. We have merged dev into main.

[kafeikui]

I see, I'm not very good at .NET but I think @neptune-v is an expert ha-ha. We will pay attention to the issue on v3 core. Anything else about Uniswap v3 guess we can talk about it :D

[neptune-v]

I did manage to get the UniswapV3Pool deployed to test, and the 'bug' is also present there, so I think your suggestion of #53 is probably the best option unfortunately. When we pass the PriceLimit into the Pool, it will return a different value (in some circumstances) than if you do not, even though the PriceLimit is the same as that returned by Uniswap itself.

I've created an issue on v3 core for their feedback: Uniswap/v3-core#501

https://github.com/Bella-DeFinTech/uniswap-v3-simulator/pull/53 The fix has not yet been merged. We will merge it into main shortly.

[jamescarter-le]

Last one guys:

When I was syncing the original Pool I need (WETH/ENS) I ran into the Sqrt error still. This is because in this pool, all the Liquidity was drained by a MEV transaction, and it does not return the same Sqrt as UniswapV3Pool.sol in this instance, however when liquidity is restored the value corrects.

Hence I suggest adding the following to your scripts to protect against this scenario, not sure what pools you are investigating but low liquidity pools will definitely run into this issue.

      let dryRunRes =
        JSBI.equal(amount0, param.amount0) &&
        JSBI.equal(amount1, param.amount1) &&
        JSBI.equal(liquidity, param.liquidity) && 
        (JSBI.equal(liquidity, JSBI.BigInt("0")) || JSBI.equal(sqrtPriceX96, param.sqrtPriceX96));

Note that you will need to add liquidity to the return tuple of querySwap & swap. I've not done this as I'm not using the TS code, but this bug will be present in your repo, especially on the WETH/ENS pool with this data set:

        { block: 13578816, type: "mint", tickLower: 49800, tickUpper: 64020, amount: JSBI.BigInt("556973545490136947176"), owner: "0xC36442b4a4522E871399CD717aBDD847Ab11FE88" },
        { block: 13578904, type: "burn", tickLower: 49800, tickUpper: 64020, amount: JSBI.BigInt("529124868215630099817"), owner: "0xC36442b4a4522E871399CD717aBDD847Ab11FE88" },
        { block: 13578904, type: "burn", tickLower: 49800, tickUpper: 64020, amount: JSBI.BigInt("0"), owner: "0xC36442b4a4522E871399CD717aBDD847Ab11FE88" },
        { block: 13578932, type: "swap", amount0: JSBI.BigInt("-927248711787417535"), amount1: JSBI.BigInt("226019397190954581334"), sqrtPriceX96: JSBI.BigInt("1596559182082899146010277864392"), liquidity: JSBI.BigInt("27848677274506847359"), tick: 60068 },
        { block: 13578934, type: "swap", amount0: JSBI.BigInt("920000000000000000"), amount1: JSBI.BigInt("-223879353725590078001"), sqrtPriceX96: JSBI.BigInt("959633067218665487871404249200"), liquidity: JSBI.BigInt("27848677274506847359"), tick: 49886 },
        { block: 13578935, type: "swap", amount0: JSBI.BigInt("-959865922027525865"), amount1: JSBI.BigInt("242467151826027761469"), sqrtPriceX96: JSBI.BigInt("1647371161194432686455871832571"), liquidity: JSBI.BigInt("27848677274506847359"), tick: 60695 },
        { block: 13578936, type: "swap", amount0: JSBI.BigInt("450000000000000000"), amount1: JSBI.BigInt("-145297332972233388668"), sqrtPriceX96: JSBI.BigInt("1234007158439758347475901738156"), liquidity: JSBI.BigInt("27848677274506847359"), tick: 54916 },
        { block: 13578936, type: "swap", amount0: JSBI.BigInt("-354496843525935494"), amount1: JSBI.BigInt("107587674962888456320"), sqrtPriceX96: JSBI.BigInt("1539170731967240812097888883834"), liquidity: JSBI.BigInt("27848677274506847359"), tick: 59336 },
        { block: 13578936, type: "swap", amount0: JSBI.BigInt("99075117070465334"), amount1: JSBI.BigInt("-34876614258170369519"), sqrtPriceX96: JSBI.BigInt("1439948422944933305299128962054"), liquidity: JSBI.BigInt("27848677274506847359"), tick: 58003 },
        { block: 13578941, type: "swap", amount0: JSBI.BigInt("-200964582387892645"), amount1: JSBI.BigInt("76633097773913642784"), sqrtPriceX96: JSBI.BigInt("1657311889301420165775927484336"), liquidity: JSBI.BigInt("27848677274506847359"), tick: 60815 },
        { block: 13578941, type: "swap", amount0: JSBI.BigInt("100000000000000000"), amount1: JSBI.BigInt("-40586449235886312908"), sqrtPriceX96: JSBI.BigInt("1541845371296224062163342553694"), liquidity: JSBI.BigInt("27848677274506847359"), tick: 59371 },
        { block: 13578941, type: "swap", amount0: JSBI.BigInt("60000000000000000"), amount1: JSBI.BigInt("-21746203898771549812"), sqrtPriceX96: JSBI.BigInt("1479978455335465530025234166091"), liquidity: JSBI.BigInt("27848677274506847359"), tick: 58551 },
        { block: 13578943, type: "swap", amount0: JSBI.BigInt("820850720170027688"), amount1: JSBI.BigInt("-184363245697871389667"), sqrtPriceX96: JSBI.BigInt("4295128740"), liquidity: JSBI.BigInt("0"), tick: -887272 },

[neptune-v]

Much appreciated, I have created a new issue for this. We will see what we can do. @jamescarter-le

[neptune-v]

Last one guys:

And it doesn't have to be the last one, if you run into more issues and have time, please let us know 😉

[amd705]

@jamescarter-le This is almost a year old now, but curious .. did the commit in #56 fix this for you? We were having the exact same problem (i.e. re-building a pool state from the beginning in order to replicate current state), and for many pools, over the course of hundreds of thousands/millions of transactions, there is frequently at least one where the entire liquidity is drained by an MEV transaction and it still errors out even with latest build. (could be our issue, but looks like one example would be 0xe76fe511e54d9163a6ef420246a161c972df8e5f2923e0c2510aa71f53a676e5 on Polygon USDC-WETH 3000) Notably, in this case, the resulting Liquidity from the event seems to be -1 instead of 0. Wondering if the fix only covers the scenario of Liquidity = 0 as opposed to Liquidity = -1.

I also noticed that the return tuple of swap and querySwap still doesn't return Liquidity per your suggestion above, which would be an easy fix, just wasn't sure if it ended up actually mattering for you...

[jamescarter-le]

I did fix the majority of times when this occurs, I cant remember the actual fix because I ported to c# and it's a long time ago.

Some instances still gave me trouble, and for those I resorted to querying the chain at that block for the true state and resumed simulation from there.

Good luck

On Wed, Oct 26, 2022, 20:36 Adam Duritza @.***> wrote:

@jamescarter-le https://github.com/jamescarter-le This is almost a year old now, but curious .. did the commit in #56 https://github.com/Bella-DeFinTech/uniswap-v3-simulator/pull/56 fix this for you? We were having the exact same problem (i.e. re-building a pool state from the beginning in order to replicate current state), and for many pools, over the course of hundreds of thousands/millions of transactions, there is frequently at least one where the entire liquidity is drained by an MEV transaction and it still errors out even with latest build. (could be our issue, but looks like one example would be 0xe76fe511e54d9163a6ef420246a161c972df8e5f2923e0c2510aa71f53a676e5 on Polygon USDC-WETH 3000) Notably, in this case, the resulting Liquidity from the event seems to be -1 instead of 0. Wondering if the fix only covers the scenario of Liquidity = 0 as opposed to Liquidity = -1.

I also noticed that the return tuple of swap and querySwap still doesn't return Liquidity per your suggestion above, which would be an easy fix, just wasn't sure if it ended up actually mattering for you...

— Reply to this email directly, view it on GitHub https://github.com/Bella-DeFinTech/uniswap-v3-simulator/issues/51#issuecomment-1292550055, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMRJLCQO6TIUWN5MPPGUE3WFGB33ANCNFSM5IOAOKWA . You are receiving this because you were mentioned.Message ID: @.***>

[amd705]

Thanks James. All good - That's the direction I was going as well. Not sure if you remember, but did you iterate through the tick bitmap or something and just do lots of RPC calls to the core pool contract at that block number in order to fully rebuild the state including all ticks/positions at that block? I don't know of an easy way to fully grab the entire state/storage of a UniV3 pool ( including all ticks/positions, which I think would be needed) at a historical block number, but I'm hoping there is a way to do it that isn't that painful.

[jamescarter-le]

You can access the Tick Bitmap to know what ticks you're looking for and pull the data directly from the Ethereum storage layer.

On Wed, Oct 26, 2022, 21:16 Adam Duritza @.***> wrote:

Thanks James. All good - That's the direction I was going as well. Not sure if you remember, but did you iterate through the tick bitmap or something and just do lots of RPC calls to the core pool contract at that block number in order to fully rebuild the state including all ticks/positions at that block? I don't know of an easy way to fully grab the entire state/storage of a UniV3 pool ( including all ticks/positions, which I think would be needed) at a historical block number, but I'm hoping there is a way to do it that isn't that painful.

— Reply to this email directly, view it on GitHub https://github.com/Bella-DeFinTech/uniswap-v3-simulator/issues/51#issuecomment-1292604001, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMRJLBIPNRYQSGVZY3ADNDWFGGRHANCNFSM5IOAOKWA . You are receiving this because you were mentioned.Message ID: @.***>

[amd705]

Thanks @jamescarter-le that's what I figured. @neptune-v @kafeikui - I'm not sure if you guys are still maintaining this library at all, but if we get in place a durable solution that covers all of the edge cases, we are happy to share back/contribute here. Could be wrong, but I don't think the latest build covers all of the edge cases. And, while these are technically edge cases, over a long enough time horizon and with enough pools, we think it can affect as much as ~50% of pools on a protocol/chain.

[neptune-v]

Hi @amd705 , thanks for the interest and feedback on Tuner. And sorry for the late reply we have missed your comments since the issue has been closed. We have been working on different projects at the moment. But if you are still actively using Tuner for your own project and find more edge cases or bugs in the code, definitely feel free to discuss your findings or directly submit PR to help us improve Tuner!