curveresearch / curvesim

Simulates Curve Finance pools
MIT License
156 stars 32 forks source link

_tweak_price Raises "Loss" Due to Neglible Numerical Error #224

Closed nagakingg closed 1 year ago

nagakingg commented 1 year ago

Version Information

curvesim 0.5.0.b1, CPython 3.10.12 on Linux

What's your issue about?

Simulation is occasionally halted by this assertion failing: https://github.com/curveresearch/curvesim/blob/1bddb535b692be8d56850abf7b40e4c4b4f9ebb6/curvesim/pool/cryptoswap/pool.py#L383

Exploring this under a number of conditions, I only came across cases where new_virtual_price was 1 less than old_virtual price.

How can it be fixed?

Either or both:

I think the second option (only) is better to preserve closeness to the original code. But one option would be to always raise a warning, but raise an exception if the difference is greater than some tolerance.

chanhosuh commented 1 year ago

if you allow a 1 wei loss, then couldn't this accumulate?

nagakingg commented 1 year ago

if you allow a 1 wei loss, then couldn't this accumulate?

Ya, that also makes me think I should look more closely into this. In theory, any trade should increase xcp because fees are collected, so its unclear to me how this is even happening. It's likely occurring under some obscure circumstances, so I can try to determine what those are.

chanhosuh commented 1 year ago

tldr, i recall our discussion concluded it's that we need to also recalculate tokens. Since we recalculate balances to match current xcp, integer truncation means when we compute the xcp from that, that it's off by 1. Thus virtual_price, which is 10**18 * xcp / tokens is off by 1 potentially.

nagakingg commented 1 year ago

Closed by #246