test(x/twap): add randomized geometric twap test on a balancer pool #3844

p0mvn · 2022-12-23T03:13:42Z

Closes: #XXX

What is the purpose of the change

This PR adds a geometric twap to now randomized test against a balancer pool. It creates 1000 pools with randomized supply and executes twap over random duration for these pools.

The result is asserted to be within "0.0001" relative to spot price.

This test serves as a sanity check that we do not encounter any internal panics in math calculations

Testing and Verifying

This change added tests.

Documentation and Release Note

Does this pull request introduce a new feature or user-facing behavior changes? no
Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
How is the feature or change documented? not applicable

x/twap/api_test.go

stackman27 · 2022-12-23T03:39:13Z

x/twap/api_test.go

+			twap, err := app.TwapKeeper.GetGeometricTwapToNow(ctx, 1, denom0, denom1, oldTime)
+			s.Require().NoError(err)
+
+			osmomath.ErrTolerance{


Out of curiosity, why not check multiplicative tolerance too?

Additive just gives a better idea of how many precision points there are here for all test cases. The goal of this test is just to see that twap is roughly close to spot price. So any reasonable tolerance would suffice.

sounds good

Hrmm, I think the guarantee we want is actually the multiplicative tolerance though. Since spot prices can differ in orders of magnitude, this may be the wrong order of magnitude for checking many of the pools.

Yeah but the largest spot price possible in this setup is 1_000_000_000 / 10_000 = 100_000 which should not cause an extremely high additive difference.

I think I will adjust the parameters to allow for even greater spot prices and switch to multiplicative tolerance

czarcas7ic

Despite me not having a ton of context on geo twap, the test was easy to follow. Nicely done 👍

czarcas7ic · 2022-12-23T18:17:32Z

x/twap/api_test.go

+				AdditiveTolerance: sdk.NewDecWithPrec(1, 4),
+			}.CompareBigDec(
+				osmomath.BigDecFromSDKDec(spotPrice),
+				osmomath.BigDecFromSDKDec(twap),
+			)


Do you expect the additive tolerance to go both ways? My guess is yes, but I just wanted to be sure.

There are no guarantees on the direction of the tolerance because exp2 function does not guarantee it. However, log always rounds down.

So I would say it is more likely to be smaller than higher but I don't think we can claim that for all test cases

p0mvn · 2022-12-23T20:56:33Z

Addressed comments.

The initial supply range of valid values has been increased. Multiplicative tolerance is now used. Also, reduced the number of retries to 200 only to avoid increasing CI time by over 2 seconds

…3844) * test(x/twap): add randomized test with a balancer pool * comments * multiplicative tolerance, fewer retries and larger initial supply range

…14 (#3925) * Upgrade IBC to v4.2.0 (#3838) * initial changes to migrate to ibc v4 * added checksum to proposal * begin and end block are now being called inside nextBlock * added changelog * linked pr on changelog * remove local replace * using error acks from osmoutils * osmoutils tagged * go sum * added checksum * feat(x/twap): modify cli to add geometric option (#3812) * feat(x/twap): geometric twap code gen query boilerplate * revert cli change * query gen * wire up API * test * fix * fixes * cli * lint * refactor via flag * refactor * refactor * fixes * lint * add arithmetic twap alias * Make wasm hooks importable (#3850) * moved ibc-hooks to its own go.mod * updated ibc hooks version * go sum * add ics23 patch into x/ibc-hooks * Fix wasm import version conflict * Update x/ibc-hooks to osmoutils v0.0.2 * Update versions Co-authored-by: Dev Ojha <dojha@berkeley.edu> * refactor(x/twap): handle spot price error case in the context of geometric twap (#3845) * refactor(x/twap): handle spot price error case * supporting test cases * table-driven log tests * test(x/twap): add randomized geometric twap test on a balancer pool (#3844) * test(x/twap): add randomized test with a balancer pool * comments * multiplicative tolerance, fewer retries and larger initial supply range * Basic geometric twap e2e test (#3835) * feat(x/twap): geometric twap code gen query boilerplate * revert cli change * query gen * wire up API * test * fix * fixes * add geometric queries * create pool.json * add test * resolve conflict * fix: swap uosmo in * fix problem with wallet creation * updates * simplify and add comments * Update tests/e2e/e2e_test.go * Update tests/e2e/e2e_test.go * Update tests/e2e/configurer/chain/queries.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Update tests/e2e/configurer/chain/queries.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Update tests/e2e/e2e_test.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Update tests/e2e/e2e_test.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Update tests/e2e/e2e_test.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Update tests/e2e/e2e_test.go Co-authored-by: Adam Tucker <adam@osmosis.team> Co-authored-by: Roman <ackhtariev@gmail.com> Co-authored-by: Roman <roman@osmosis.team> Co-authored-by: Adam Tucker <adam@osmosis.team> * feat(x/twap): whitelist GeometricTwap and GeometricTwapToNow (#3852) * feat(x/twap): GeometricTwap and GeometricTwapToNow queries added to Stargate whitelist * update docs * fix(scripts): proto gen for osmoutils (#3854) * fix(scripts): proto gen for osmoutils * Update scripts/protocgen.sh * fix(scripts): proto gen osmoutils path (#3859) * added packet timeouts to wasm hooks (#3862) * add negative to cli (#3888) * Making osmoutils compile on chains that don't use our SDK fork (#3899) * making osmoutils compile on chains that don't use osmosis' fork of the cosmos sdk * updated imports for e2e tests * go fumpt * updated version everywhere * added changelog entry * remove deprecation from arithmetic & geometric twap to now query (#3917) * Add types & boilerplate for the Downtime detector module (#3609) Sub-component of #3603 Adds types for the thin module intended for downtime detection - Add downtime detection module types No tests added - Does this pull request introduce a new feature or user-facing behavior changes? somewhat - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? yes - How is the feature or change documented? In its spec * Add downtime detector module (#3688) * WIP * Switch to enum * Remove params query * Add query * Add wiring, add import/export test * Start begin block test * Finish keeper tests * Add CLI * Wire downtime detector CLI + queries * more module wiring * add types test * Fix state alteration test * Fix superfluid test * Add stargate whitelist support * Apply code comment * Rename folder * Add requested godoc * Update x/downtime-detector/genesis.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Apply adam suggestion for having a `-` * move genesis test to own file Co-authored-by: Adam Tucker <adam@osmosis.team> * Initial by hand fixes * feat(osmomath): Exp2 function (#3708) * feat(osmomath): exp2 function * export exp2 * changelog * refactor ErrTolerance to use Dec instead of Int for additive tolerance * Update osmomath/exp2.go * Update osmomath/exp2.go * Update osmomath/exp2.go * Update osmomath/exp2_test.go * Update osmomath/exp2_test.go * do bit shift instead of multiplication * godoc about error bounds * comment about bit shift equivalency * merge conflict * improve godoc * typo * remove TODOs - confirmed obsolete * Runge's phenomenon comment * [x/TWAP] Expose a geometric TWAP API (#3529) * refactored twap api.go for geometric TWAP * added barebon docs * romans feedback * new * fix * nichola feedback * final roman comments * fix twap by hand * change to gamm * fix balancer test * bump to v14 upgrade * e2e fix * add remaining diff from main to ibc-rate-limit * update contracts test * osmomath: `AddMut` and `QuoMut` (#3779) * mut add * test add mut * quo mut * test quo mut/ remove want from test struct * refactor exp * change mutatives code * change * not allocaing * exp change to quomut * remove file * refactor quo * refactor ad * refactor tests * Modify CHANGELOG * Whitelist EstimateSwapExactAmountOut (#3693) * whitelist EstimateSwapExactAmountOut * doc: changelog * updated rate limit contract * Fix rust checks (#3576) * added cargo.lock * added Cargo.lock as an artifact * added new bytecode with lock file Co-authored-by: Nicolas Lara <nicolaslara@gmail.com> Co-authored-by: Roman <roman@osmosis.team> Co-authored-by: Dev Ojha <dojha@berkeley.edu> Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Co-authored-by: Roman <ackhtariev@gmail.com> Co-authored-by: Supanat <supanat.ptk@gmail.com> Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Sishir Giri <sis1001@berkeley.edu> Co-authored-by: Ruslan Akhtariev <46343690+RusAkh@users.noreply.github.com> Co-authored-by: mattverse <mattpark1028@gmail.com> Co-authored-by: ByeongSu Hong <frostornge@gmail.com>

test(x/twap): add randomized test with a balancer pool

d84e4cf

p0mvn added the F: geometric-twap label Dec 23, 2022

github-actions bot added the C:x/twap Changes to the twap module label Dec 23, 2022

p0mvn added V:state/compatible/no_backport State machine compatible PR, depends on prior breaks and removed C:x/twap Changes to the twap module labels Dec 23, 2022

p0mvn marked this pull request as ready for review December 23, 2022 03:27

p0mvn requested review from czarcas7ic, ValarDragon, nicolaslara and stackman27 December 23, 2022 03:27

p0mvn changed the title ~~test(x/twap): add randomized gaometrict twap test on a balancer pool~~ test(x/twap): add randomized geometric twap test on a balancer pool Dec 23, 2022

stackman27 reviewed Dec 23, 2022

View reviewed changes

comments

ba42404

github-actions bot added the C:x/twap Changes to the twap module label Dec 23, 2022

czarcas7ic approved these changes Dec 23, 2022

View reviewed changes

p0mvn added 2 commits December 23, 2022 15:35

Merge branch 'main' into roman/geom-randomized

f21f32b

multiplicative tolerance, fewer retries and larger initial supply range

046d945

p0mvn merged commit b3b9904 into main Dec 23, 2022

p0mvn deleted the roman/geom-randomized branch December 23, 2022 21:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(x/twap): add randomized geometric twap test on a balancer pool #3844

test(x/twap): add randomized geometric twap test on a balancer pool #3844

p0mvn commented Dec 23, 2022 •

edited

Loading

stackman27 Dec 23, 2022

p0mvn Dec 23, 2022

stackman27 Dec 23, 2022

ValarDragon Dec 23, 2022

p0mvn Dec 23, 2022

czarcas7ic left a comment

czarcas7ic Dec 23, 2022

p0mvn Dec 23, 2022

p0mvn commented Dec 23, 2022

test(x/twap): add randomized geometric twap test on a balancer pool #3844

test(x/twap): add randomized geometric twap test on a balancer pool #3844

Conversation

p0mvn commented Dec 23, 2022 • edited Loading

What is the purpose of the change

Testing and Verifying

Documentation and Release Note

stackman27 Dec 23, 2022

Choose a reason for hiding this comment

p0mvn Dec 23, 2022

Choose a reason for hiding this comment

stackman27 Dec 23, 2022

Choose a reason for hiding this comment

ValarDragon Dec 23, 2022

Choose a reason for hiding this comment

p0mvn Dec 23, 2022

Choose a reason for hiding this comment

czarcas7ic left a comment

Choose a reason for hiding this comment

czarcas7ic Dec 23, 2022

Choose a reason for hiding this comment

p0mvn Dec 23, 2022

Choose a reason for hiding this comment

p0mvn commented Dec 23, 2022

p0mvn commented Dec 23, 2022 •

edited

Loading