Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Dec 23, 2022
Merged

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

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d84e4cf
test(x/twap): add randomized test with a balancer pool
p0mvn Dec 23, 2022
ba42404
comments
p0mvn Dec 23, 2022
f21f32b
Merge branch 'main' into roman/geom-randomized
p0mvn Dec 23, 2022
046d945
multiplicative tolerance, fewer retries and larger initial supply range
p0mvn Dec 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions x/twap/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@ package twap_test
import (
"errors"
"fmt"
"math/rand"
"time"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/osmosis-labs/osmosis/osmomath"
sdkrand "github.com/osmosis-labs/osmosis/v13/simulation/simtypes/random"
"github.com/osmosis-labs/osmosis/v13/x/gamm/pool-models/balancer"
"github.com/osmosis-labs/osmosis/v13/x/twap"
"github.com/osmosis-labs/osmosis/v13/x/twap/types"
)
Expand Down Expand Up @@ -837,3 +841,75 @@ func (s *TestSuite) TestGetArithmeticTwapToNow_ThreeAsset() {
})
}
}

// TestGeometricTwapToNow_BalancerPool_Randomized the goal of this test case is to validate
// that no internal panics occur when computing geometric twap. It also sanity checks
// that geometric twap is roughly close to spot price.
func (s *TestSuite) TestGeometricTwapToNow_BalancerPool_Randomized() {
seed := int64(1)
r := rand.New(rand.NewSource(seed))
retries := 1000

type randTestCase struct {
elapsedTimeMs int
weightA sdk.Int
tokenASupply sdk.Int
weightB sdk.Int
tokenBSupply sdk.Int
}

maxUint64 := ^uint(0)

for i := 0; i < retries; i++ {
elapsedTimeMs := sdkrand.RandIntBetween(r, 1, int(maxUint64>>1))
weightA := sdk.NewInt(int64(sdkrand.RandIntBetween(r, 1, 1000)))
tokenASupply := sdk.NewInt(int64(sdkrand.RandIntBetween(r, 10_000, 1_000_000_000)))

tokenBSupply := sdk.NewInt(int64(sdkrand.RandIntBetween(r, 10_000, 1_000_000_000)))
weightB := sdk.NewInt(int64(sdkrand.RandIntBetween(r, 1, 1000)))

s.Run(fmt.Sprintf("elapsedTimeMs=%d, weightA=%d, tokenASupply=%d, weightB=%d, tokenBSupply=%d", elapsedTimeMs, weightA, tokenASupply, weightB, tokenBSupply), func() {
s.SetupTest()

ctx := s.Ctx
app := s.App

assets := []balancer.PoolAsset{
{
Token: sdk.NewCoin(denom0, tokenASupply),
Weight: weightA,
},
{
Token: sdk.NewCoin(denom1, tokenBSupply),
Weight: weightB,
},
}

s.PrepareCustomBalancerPool(assets, balancer.PoolParams{
SwapFee: sdk.ZeroDec(),
ExitFee: sdk.ZeroDec(),
})

// We add 1ms to avoid always landing on the same block time
// In that case, the most recent spot price would be used
// instead of interpolation.
oldTime := ctx.BlockTime().Add(1 * time.Millisecond)
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
newTime := oldTime.Add(time.Duration(elapsedTimeMs))

ctx = ctx.WithBlockTime(newTime)

spotPrice, err := app.GAMMKeeper.CalculateSpotPrice(ctx, 1, denom1, denom0)
s.Require().NoError(err)

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

osmomath.ErrTolerance{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why not check multiplicative tolerance too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

})
}
}