v3-subgraph icon indicating copy to clipboard operation
v3-subgraph copied to clipboard

fix: math error in tick updates during swaps

Open mariorz opened this issue 3 years ago • 4 comments

Hello, It appears there's a bug in the tick section of handleSwap that result in ticks (and tickDayDatas) not getting updated when they should, causing a mismatch between subgraph and blockchain data, see [1].

Using small numbers to describe the issue, let's assume a pool's state in which the current tick is 19 and a swap occurs that pushes the current tick to 21, and let's also assume a tick spacing of 10 for this pool.

handleSwap in that example would compute modulo = newTick.mod(tickSpacing) == 1 and firstInitialized would thus be defined as firstInitialized = oldTick.plus(tickSpacing.minus(modulo)) == 28. The correct value for firstInitialized should of course be 20 instead.

I'm syncing a subgraph with the changes here, but it still has a long way to go.

[1] I've noticed bogus subgraph data while rendering charts of daily fees for positions, but I believe issue 44 is a good example. I wrote a test to compare subgraph data with blockchain data at the specified block, uploaded here. Use brownie to run it: brownie run tickstest.py --network mainnet.

Results:

Testing contract calls vs the graph values for
pool:0x4e68ccd3e89f51c3074ca5072bbac773960dfa36
tick:-195660
block:13010089
feeGrowthOutside0X128 from the blockchain: 1067292557275396938893002967958758719094
feeGrowthOutside0X128 from the graph     : 335883582520604181952009590658028448194028
Mismatch ✘

mariorz avatar Sep 29 '21 01:09 mariorz

@mariorz Thanks for looking into this - this is definitely the type of bug that would go unnoticed if no one tested against accurate data

When your subgraph syncs Would love to view results - if all looks good can merge and sync this change

ianlapham avatar Oct 14 '21 00:10 ianlapham

Thanks @ianlapham, so the subgraph is fully synced now at https://thegraph.com/hosted-service/subgraph/mariorz/uniswapv3

And running the above shared script does result in the test passing, results below. However I have still found mismatches with other parameters, e.g. (pool:0xc2e9f25be6257c210d7adf0d4cd6e3e881ba25f8, tick:-78060,block:12613933). So there seems to be some other issue. I'm going to investigate some more and hope to ping back later if that makes sense. Cheers.

Results:

-----------------------------
Testing contract calls vs the graph values for
pool:0x4e68ccd3e89f51c3074ca5072bbac773960dfa36
tick:-195660
block:13010089
feeGrowthOutside0X128 from the chain: 1067292557275396938893002967958758719094
feeGrowthOutside0X128 from the graph: 1067292557275396938893002967958758719094
Match ✓

mariorz avatar Oct 14 '21 16:10 mariorz

@mariorz Did you find what other issue was?

usgeeus avatar Sep 13 '23 04:09 usgeeus

Would it make sense to merge this fix regardless of the other issue? Fix one thing at a time in small discrete PRs.

ackvf avatar Jan 17 '24 20:01 ackvf