search

Uniswap / v3-core-optimism

Optimism fork of the V3 core contracts
Other
32 stars 41 forks source link

binarySearch may have bug? #6

Open zgfzgf opened 3 years ago

zgfzgf commented 3 years ago

for example: old cardinality 10 new cardinality 100

when index=3 self blockTimestamp is 10,11,12,13,4,5,6,7,8,9,1,1,1,....1 call(self, 13, 7, 3, 100) l=4, r=103 first l=53 r=103 ... not find 7
https://github.com/Uniswap/uniswap-v3-core-optimism/blob/d99ea89a1f29e46bede8fab47e8e737190a6b6fa/contracts/libraries/Oracle.sol#L153-L184

moodysalem commented 3 years ago

is this a difference between v3-core and v3-core-optimism or the code is the same b/t the two?

NoahZinsmeister commented 3 years ago

@zgfzgf do you have a test case reproduction of this bug? in your example, i'm already seeing a potential issue, index would never be 3 for the given self blockTimestamps, index always represents the most recently updated entry, in this case 9

zgfzgf commented 3 years ago

when old cardinality 10. may index 3 because first 0-9 then start 0

when index=3 update cardinality =100

https://github.com/Uniswap/uniswap-v3-core-optimism/blob/da89f9b88a1616be1ef075ad4313e8aeb3532906/contracts/libraries/Oracle.sol#L78-L101

zgfzgf commented 3 years ago

if it is bug. createPool and then increaseObservationCardinalityNext(65535). no problem. or modify contract code. @NoahZinsmeister