add NewPoolSimulatorV2 for uniswapv3 #306
Conversation
| p := &PoolSimulatorV2{} | ||
| err := InitPoolSimulatorV2(entityPool, p, chainID) |
There was a problem hiding this comment.
looks weird, since we are not pre-loc p. Why do we have to write this type of pattern.
There was a problem hiding this comment.
um this is for caller (for example combine-path lib) that want to simply create a simulator without worrying about pre-allocating simulator instance
(router-service will try to call InitPoolSimulatorV2 directly on the re-used instance)
| // TickGob should be fully compatible with v3Entities.Tick (GobBigInt is just a wrapper around bigInt) | ||
| // this should always be checked with a unit test | ||
| // also, uninitialized tick should be ignored at pool-tracker/tick-based-worker already, so no need to check here | ||
| v3Ticks := *(*v3TickList)(unsafe.Pointer(&p.extra.Ticks)) |
There was a problem hiding this comment.
will this assertion panic if the p.extra.Ticks is empty?
There was a problem hiding this comment.
've added a testcase for empty list
| @@ -59,6 +61,12 @@ type Tick struct { | |||
| LiquidityNet *big.Int `json:"liquidityNet"` | |||
| } | |||
|
|
|||
| type TickGob struct { | |||
| Index int `json:"index"` | |||
There was a problem hiding this comment.
maybe we don't need to use json struct tag when using gob anymore?
There was a problem hiding this comment.
um in this PR we only use Gob for 2 bigInt (liq gross and liq net), not the whole struct, so we'll still need these tags
Benchmark:
Before:
BenchmarkNewPoolSimulator-8 2350 511776 ns/op 242664 B/op 3778 allocs/opAfter:
If we don't reuse PoolSimulator instance
BenchmarkNewPoolSimulatorV2-8 8212 146271 ns/op 157368 B/op 2673 allocs/opBest case: if we keep reusing the same PoolSimulator (don't need to alloc new tick list array)
BenchmarkNewPoolSimulatorV2ReuseBestcase-8 12393 95657 ns/op 87663 B/op 88 allocs/opWhy did we need it?
Related Issue
Release Note
How Has This Been Tested?
Screenshots (if appropriate):