From 16eb3e55e3712cffdde9e26c5b22e056b586704a Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 05:22:19 -0800 Subject: [PATCH 01/13] fix(gamm): SpotPrice keeper function --- app/apptesting/test_suite.go | 2 +- wasmbinding/queries.go | 6 +-- x/gamm/keeper/grpc_query.go | 2 + x/gamm/keeper/pool_service.go | 4 +- x/gamm/keeper/pool_service_test.go | 6 +-- x/gamm/pool-models/balancer/pool.go | 20 ++++---- .../pool-models/balancer/pool_suite_test.go | 46 +++++++++---------- x/gamm/pool-models/stableswap/amm.go | 5 +- x/gamm/pool-models/stableswap/pool.go | 4 +- x/gamm/pool-models/stableswap/pool_test.go | 5 +- x/swaprouter/types/pool.go | 2 +- x/twap/logic.go | 2 + x/twap/types/expected_interfaces.go | 2 +- x/txfees/keeper/feetokens.go | 3 +- x/txfees/types/expected_keepers.go | 2 +- 15 files changed, 60 insertions(+), 51 deletions(-) diff --git a/app/apptesting/test_suite.go b/app/apptesting/test_suite.go index d2b3337b0ca..c6c525c73bf 100644 --- a/app/apptesting/test_suite.go +++ b/app/apptesting/test_suite.go @@ -333,7 +333,7 @@ func (s *KeeperTestHelper) SwapAndSetSpotPrice(poolId uint64, fromAsset sdk.Coin ) s.Require().NoError(err) - spotPrice, err := s.App.GAMMKeeper.CalculateSpotPrice(s.Ctx, poolId, toAsset.Denom, fromAsset.Denom) + spotPrice, err := s.App.GAMMKeeper.CalculateSpotPrice(s.Ctx, poolId, fromAsset.Denom, toAsset.Denom) s.Require().NoError(err) return spotPrice diff --git a/wasmbinding/queries.go b/wasmbinding/queries.go index 59df2e89161..a020b12ef57 100644 --- a/wasmbinding/queries.go +++ b/wasmbinding/queries.go @@ -63,11 +63,11 @@ func (qp QueryPlugin) GetSpotPrice(ctx sdk.Context, spotPrice *bindings.SpotPric } poolId := spotPrice.Swap.PoolId - denomIn := spotPrice.Swap.DenomIn - denomOut := spotPrice.Swap.DenomOut + baseAsset := spotPrice.Swap.DenomIn + quoteAsset := spotPrice.Swap.DenomOut withSwapFee := spotPrice.WithSwapFee - price, err := qp.gammKeeper.CalculateSpotPrice(ctx, poolId, denomIn, denomOut) + price, err := qp.gammKeeper.CalculateSpotPrice(ctx, poolId, quoteAsset, baseAsset) if err != nil { return nil, sdkerrors.Wrap(err, "gamm get spot price") } diff --git a/x/gamm/keeper/grpc_query.go b/x/gamm/keeper/grpc_query.go index 159fc948d7d..d163d0dd663 100644 --- a/x/gamm/keeper/grpc_query.go +++ b/x/gamm/keeper/grpc_query.go @@ -349,6 +349,8 @@ func (q Querier) SpotPrice(ctx context.Context, req *types.QuerySpotPriceRequest sdkCtx := sdk.UnwrapSDKContext(ctx) + // Note: the base and quote asset are provided as argument incorrectly intentionally. + // due to the historic bug in the original implementation. sp, err := q.Keeper.CalculateSpotPrice(sdkCtx, req.PoolId, req.BaseAssetDenom, req.QuoteAssetDenom) if err != nil { return nil, status.Error(codes.Internal, err.Error()) diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index 14a6e50857e..a7a819eba17 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -24,8 +24,8 @@ import ( func (k Keeper) CalculateSpotPrice( ctx sdk.Context, poolID uint64, - baseAssetDenom string, quoteAssetDenom string, + baseAssetDenom string, ) (spotPrice sdk.Dec, err error) { pool, err := k.GetPoolAndPoke(ctx, poolID) if err != nil { @@ -40,7 +40,7 @@ func (k Keeper) CalculateSpotPrice( } }() - spotPrice, err = pool.SpotPrice(ctx, baseAssetDenom, quoteAssetDenom) + spotPrice, err = pool.SpotPrice(ctx, quoteAssetDenom, baseAssetDenom) if err != nil { return sdk.Dec{}, err } diff --git a/x/gamm/keeper/pool_service_test.go b/x/gamm/keeper/pool_service_test.go index ba1462507e1..8804322328d 100644 --- a/x/gamm/keeper/pool_service_test.go +++ b/x/gamm/keeper/pool_service_test.go @@ -339,8 +339,8 @@ func (suite *KeeperTestSuite) TestSpotPriceOverflow() { poolLiquidity: sdk.NewCoins(sdk.NewCoin(denomA, types.MaxSpotPrice.TruncateInt().Add(sdk.OneInt())), sdk.NewCoin(denomB, sdk.OneInt())), poolWeights: []int64{1, 1}, - quoteAssetDenom: denomB, - baseAssetDenom: denomA, + quoteAssetDenom: denomA, + baseAssetDenom: denomB, overflows: true, }, "uniV2 internal error": { @@ -363,7 +363,7 @@ func (suite *KeeperTestSuite) TestSpotPriceOverflow() { osmoassert.ConditionalPanic(suite.T(), tc.panics, func() { poolSpotPrice, poolErr = pool.SpotPrice(suite.Ctx, tc.baseAssetDenom, tc.quoteAssetDenom) }) - keeperSpotPrice, keeperErr := suite.App.GAMMKeeper.CalculateSpotPrice(suite.Ctx, poolId, tc.baseAssetDenom, tc.quoteAssetDenom) + keeperSpotPrice, keeperErr := suite.App.GAMMKeeper.CalculateSpotPrice(suite.Ctx, poolId, tc.quoteAssetDenom, tc.baseAssetDenom) if tc.overflows { suite.Require().NoError(poolErr) suite.Require().ErrorIs(keeperErr, types.ErrSpotPriceOverflow) diff --git a/x/gamm/pool-models/balancer/pool.go b/x/gamm/pool-models/balancer/pool.go index 9d2196f382e..907aa695618 100644 --- a/x/gamm/pool-models/balancer/pool.go +++ b/x/gamm/pool-models/balancer/pool.go @@ -611,15 +611,19 @@ func (p *Pool) applySwap(ctx sdk.Context, tokensIn sdk.Coins, tokensOut sdk.Coin // SpotPrice returns the spot price of the pool // This is the weight-adjusted balance of the tokens in the pool. -// In order reduce the propagated effect of incorrect trailing digits, +// To reduce the propagated effect of incorrect trailing digits, // we take the ratio of weights and divide this by ratio of supplies -// this is equivalent to spot_price = (Base_supply / Weight_base) / (Quote_supply / Weight_quote) -// but cancels out the common term in weight. +// this is equivalent to spot_price = (Quote Supply / Quote Weight) / (Base Supply / Base Weight) +// +// As an example, assume equal weights. uosmo supply of 2 and uatom supply of 4. +// +// Case 1: base = uosmo, quote = uatom -> for one uosmo, get 2 uatom = 4 / 2 = 2 +// +// Case 2: base = uatom, quote = uosmo -> for one uatom, get 0.5 uosmo = 2 / 4 = 0.5 // // panics if the pool in state is incorrect, and has any weight that is 0. -// TODO: Come back and improve docs for this. -func (p Pool) SpotPrice(ctx sdk.Context, baseAsset, quoteAsset string) (spotPrice sdk.Dec, err error) { - quote, base, err := p.parsePoolAssetsByDenoms(quoteAsset, baseAsset) +func (p Pool) SpotPrice(ctx sdk.Context, quoteAsset, baseAsset string) (spotPrice sdk.Dec, err error) { + quote, base, err := p.parsePoolAssetsByDenoms(baseAsset, quoteAsset) if err != nil { return sdk.Dec{}, err } @@ -627,8 +631,8 @@ func (p Pool) SpotPrice(ctx sdk.Context, baseAsset, quoteAsset string) (spotPric return sdk.Dec{}, errors.New("pool is misconfigured, got 0 weight") } - // spot_price = (Base_supply / Weight_base) / (Quote_supply / Weight_quote) - // spot_price = (weight_quote / weight_base) * (base_supply / quote_supply) + // spot_price = (Quote Supply / Quote Weight) / (Base Supply / Base Weight) + // spot_price = (Quote Weight / Base Weight) * (Quote Supply / Base Supply) invWeightRatio := quote.Weight.ToDec().Quo(base.Weight.ToDec()) supplyRatio := base.Token.Amount.ToDec().Quo(quote.Token.Amount.ToDec()) spotPrice = supplyRatio.Mul(invWeightRatio) diff --git a/x/gamm/pool-models/balancer/pool_suite_test.go b/x/gamm/pool-models/balancer/pool_suite_test.go index fdce7c02161..4db7ce47d4f 100644 --- a/x/gamm/pool-models/balancer/pool_suite_test.go +++ b/x/gamm/pool-models/balancer/pool_suite_test.go @@ -713,57 +713,57 @@ func (suite *KeeperTestSuite) TestBalancerSpotPriceBounds() { tests := []struct { name string - baseDenomPoolInput sdk.Coin - baseDenomWeight sdk.Int quoteDenomPoolInput sdk.Coin quoteDenomWeight sdk.Int + baseDenomPoolInput sdk.Coin + baseDenomWeight sdk.Int expectError bool expectedOutput sdk.Dec }{ { name: "spot price check at max bitlen supply", // 2^196, as >= 2^197 trips max bitlen of 256 - baseDenomPoolInput: sdk.NewCoin(baseDenom, sdk.MustNewDecFromStr("100433627766186892221372630771322662657637687111424552206336").TruncateInt()), - baseDenomWeight: sdk.NewInt(100), - quoteDenomPoolInput: sdk.NewCoin(quoteDenom, sdk.MustNewDecFromStr("100433627766186892221372630771322662657637687111424552206337").TruncateInt()), + quoteDenomPoolInput: sdk.NewCoin(baseDenom, sdk.MustNewDecFromStr("100433627766186892221372630771322662657637687111424552206336").TruncateInt()), quoteDenomWeight: sdk.NewInt(100), + baseDenomPoolInput: sdk.NewCoin(quoteDenom, sdk.MustNewDecFromStr("100433627766186892221372630771322662657637687111424552206337").TruncateInt()), + baseDenomWeight: sdk.NewInt(100), expectError: false, expectedOutput: sdk.MustNewDecFromStr("1.000000000000000000"), }, { name: "spot price check at min supply", - baseDenomPoolInput: sdk.NewCoin(baseDenom, sdk.OneInt()), - baseDenomWeight: sdk.NewInt(100), - quoteDenomPoolInput: sdk.NewCoin(quoteDenom, sdk.OneInt()), + quoteDenomPoolInput: sdk.NewCoin(baseDenom, sdk.OneInt()), quoteDenomWeight: sdk.NewInt(100), + baseDenomPoolInput: sdk.NewCoin(quoteDenom, sdk.OneInt()), + baseDenomWeight: sdk.NewInt(100), expectError: false, expectedOutput: sdk.MustNewDecFromStr("1.000000000000000000"), }, { name: "max spot price with equal weights", - baseDenomPoolInput: sdk.NewCoin(baseDenom, types.MaxSpotPrice.TruncateInt()), - baseDenomWeight: sdk.NewInt(100), - quoteDenomPoolInput: sdk.NewCoin(quoteDenom, sdk.OneInt()), + quoteDenomPoolInput: sdk.NewCoin(baseDenom, types.MaxSpotPrice.TruncateInt()), quoteDenomWeight: sdk.NewInt(100), + baseDenomPoolInput: sdk.NewCoin(quoteDenom, sdk.OneInt()), + baseDenomWeight: sdk.NewInt(100), expectError: false, expectedOutput: types.MaxSpotPrice, }, { // test int overflows name: "max spot price with extreme weights", - baseDenomPoolInput: sdk.NewCoin(baseDenom, types.MaxSpotPrice.TruncateInt()), - baseDenomWeight: sdk.OneInt(), - quoteDenomPoolInput: sdk.NewCoin(quoteDenom, sdk.OneInt()), - quoteDenomWeight: sdk.NewInt(1 << 19), + quoteDenomPoolInput: sdk.NewCoin(baseDenom, types.MaxSpotPrice.TruncateInt()), + quoteDenomWeight: sdk.OneInt(), + baseDenomPoolInput: sdk.NewCoin(quoteDenom, sdk.OneInt()), + baseDenomWeight: sdk.NewInt(1 << 19), expectError: true, }, { name: "greater than max spot price with equal weights", // Max spot price capped at 2^160 - baseDenomPoolInput: sdk.NewCoin(baseDenom, types.MaxSpotPrice.TruncateInt().Add(sdk.OneInt())), - baseDenomWeight: sdk.NewInt(100), - quoteDenomPoolInput: sdk.NewCoin(quoteDenom, sdk.OneInt()), + quoteDenomPoolInput: sdk.NewCoin(baseDenom, types.MaxSpotPrice.TruncateInt().Add(sdk.OneInt())), quoteDenomWeight: sdk.NewInt(100), + baseDenomPoolInput: sdk.NewCoin(quoteDenom, sdk.OneInt()), + baseDenomWeight: sdk.NewInt(100), expectError: true, }, } @@ -773,13 +773,13 @@ func (suite *KeeperTestSuite) TestBalancerSpotPriceBounds() { suite.Run(tc.name, func() { // pool assets defaultBaseAsset := balancer.PoolAsset{ - Weight: tc.baseDenomWeight, - Token: tc.baseDenomPoolInput, - } - defaultQuoteAsset := balancer.PoolAsset{ Weight: tc.quoteDenomWeight, Token: tc.quoteDenomPoolInput, } + defaultQuoteAsset := balancer.PoolAsset{ + Weight: tc.baseDenomWeight, + Token: tc.baseDenomPoolInput, + } poolAssets := []balancer.PoolAsset{defaultBaseAsset, defaultQuoteAsset} poolId := suite.PrepareBalancerPoolWithPoolAsset(poolAssets) @@ -790,7 +790,7 @@ func (suite *KeeperTestSuite) TestBalancerSpotPriceBounds() { sut := func() { spotPrice, err := suite.App.GAMMKeeper.CalculateSpotPrice(suite.Ctx, - poolId, tc.baseDenomPoolInput.Denom, tc.quoteDenomPoolInput.Denom) + poolId, tc.quoteDenomPoolInput.Denom, tc.baseDenomPoolInput.Denom) if tc.expectError { suite.Require().Error(err, "test: %s", tc.name) } else { diff --git a/x/gamm/pool-models/stableswap/amm.go b/x/gamm/pool-models/stableswap/amm.go index cdbdbf5a24c..b42cfdcbf06 100644 --- a/x/gamm/pool-models/stableswap/amm.go +++ b/x/gamm/pool-models/stableswap/amm.go @@ -357,7 +357,7 @@ func solveCFMMBinarySearchMulti(xReserve, yReserve, wSumSquares, yIn osmomath.Bi return xOut } -func (p Pool) spotPrice(baseDenom, quoteDenom string) (spotPrice sdk.Dec, err error) { +func (p Pool) spotPrice(quoteDenom, baseDenom string) (spotPrice sdk.Dec, err error) { // Define f_{y -> x}(a) as the function that outputs the amount of tokens X you'd get by // trading "a" units of Y against the pool, assuming 0 swap fee, at the current liquidity. // The spot price of the pool is then lim a -> 0, f_{y -> x}(a) / a @@ -373,8 +373,7 @@ func (p Pool) spotPrice(baseDenom, quoteDenom string) (spotPrice sdk.Dec, err er // xReserve & yReserve. a := sdk.OneInt() - // We swap quoteDenom and baseDenom intentionally, due to the odd issue needed for balancer v1 query compat - res, err := p.calcOutAmtGivenIn(sdk.NewCoin(quoteDenom, a), baseDenom, sdk.ZeroDec()) + res, err := p.calcOutAmtGivenIn(sdk.NewCoin(baseDenom, a), quoteDenom, sdk.ZeroDec()) // fmt.Println("spot price res", res) return res, err } diff --git a/x/gamm/pool-models/stableswap/pool.go b/x/gamm/pool-models/stableswap/pool.go index 6f03b08a5ec..60b7db553f1 100644 --- a/x/gamm/pool-models/stableswap/pool.go +++ b/x/gamm/pool-models/stableswap/pool.go @@ -331,8 +331,8 @@ func (p *Pool) SwapInAmtGivenOut(ctx sdk.Context, tokenOut sdk.Coins, tokenInDen // SpotPrice calculates the approximate amount of `baseDenom` one would receive for // an input dx of `quoteDenom` (to simplify calculations, we approximate dx = 1) -func (p Pool) SpotPrice(ctx sdk.Context, baseAssetDenom string, quoteAssetDenom string) (sdk.Dec, error) { - return p.spotPrice(baseAssetDenom, quoteAssetDenom) +func (p Pool) SpotPrice(ctx sdk.Context, quoteAssetDenom string, baseAssetDenom string) (sdk.Dec, error) { + return p.spotPrice(quoteAssetDenom, baseAssetDenom) } func (p Pool) Copy() Pool { diff --git a/x/gamm/pool-models/stableswap/pool_test.go b/x/gamm/pool-models/stableswap/pool_test.go index 8200c81f63f..d3db4f8ba2f 100644 --- a/x/gamm/pool-models/stableswap/pool_test.go +++ b/x/gamm/pool-models/stableswap/pool_test.go @@ -7,11 +7,12 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto/ed25519" + "github.com/osmosis-labs/osmosis/v13/app/apptesting/osmoassert" "github.com/osmosis-labs/osmosis/v13/osmomath" "github.com/osmosis-labs/osmosis/v13/x/gamm/pool-models/internal/cfmm_common" "github.com/osmosis-labs/osmosis/v13/x/gamm/types" - "github.com/tendermint/tendermint/crypto/ed25519" ) var ( @@ -1328,7 +1329,7 @@ func TestStableswapSpotPrice(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := sdk.Context{} p := poolStructFromAssets(tc.poolAssets, tc.scalingFactors) - spotPrice, err := p.SpotPrice(ctx, tc.baseDenom, tc.quoteDenom) + spotPrice, err := p.SpotPrice(ctx, tc.quoteDenom, tc.baseDenom) if tc.expectPass { require.NoError(t, err) diff --git a/x/swaprouter/types/pool.go b/x/swaprouter/types/pool.go index 13be3d40763..c3b2d8ea254 100644 --- a/x/swaprouter/types/pool.go +++ b/x/swaprouter/types/pool.go @@ -31,5 +31,5 @@ type PoolI interface { // errors if either baseAssetDenom, or quoteAssetDenom does not exist. // For example, if this was a UniV2 50-50 pool, with 2 ETH, and 8000 UST // pool.SpotPrice(ctx, "eth", "ust") = 4000.00 - SpotPrice(ctx sdk.Context, baseAssetDenom string, quoteAssetDenom string) (sdk.Dec, error) + SpotPrice(ctx sdk.Context, quoteAssetDenom string, baseAssetDenom string) (sdk.Dec, error) } diff --git a/x/twap/logic.go b/x/twap/logic.go index 35a707936cb..648fe227980 100644 --- a/x/twap/logic.go +++ b/x/twap/logic.go @@ -55,7 +55,9 @@ func getSpotPrices( previousErrorTime time.Time, ) (sp0 sdk.Dec, sp1 sdk.Dec, latestErrTime time.Time) { latestErrTime = previousErrorTime + // sp0 = denom0 base, denom1 quote. sp0, err0 := k.CalculateSpotPrice(ctx, poolId, denom0, denom1) + // sp1 = denom0 quote, denom1 base. sp1, err1 := k.CalculateSpotPrice(ctx, poolId, denom1, denom0) if err0 != nil || err1 != nil { latestErrTime = ctx.BlockTime() diff --git a/x/twap/types/expected_interfaces.go b/x/twap/types/expected_interfaces.go index 8ed0bfe4e58..1a8dadb6d78 100644 --- a/x/twap/types/expected_interfaces.go +++ b/x/twap/types/expected_interfaces.go @@ -12,7 +12,7 @@ type AmmInterface interface { CalculateSpotPrice( ctx sdk.Context, poolID uint64, - baseAssetDenom string, quoteAssetDenom string, + baseAssetDenom string, ) (price sdk.Dec, err error) } diff --git a/x/txfees/keeper/feetokens.go b/x/txfees/keeper/feetokens.go index 12581a6b8dd..ac97f8572cf 100644 --- a/x/txfees/keeper/feetokens.go +++ b/x/txfees/keeper/feetokens.go @@ -2,6 +2,7 @@ package keeper import ( "github.com/gogo/protobuf/proto" + "github.com/osmosis-labs/osmosis/v13/x/txfees/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -47,7 +48,7 @@ func (k Keeper) CalcFeeSpotPrice(ctx sdk.Context, inputDenom string) (sdk.Dec, e return sdk.Dec{}, err } - spotPrice, err := k.spotPriceCalculator.CalculateSpotPrice(ctx, feeToken.PoolID, baseDenom, feeToken.Denom) + spotPrice, err := k.spotPriceCalculator.CalculateSpotPrice(ctx, feeToken.PoolID, feeToken.Denom, baseDenom) if err != nil { return sdk.Dec{}, err } diff --git a/x/txfees/types/expected_keepers.go b/x/txfees/types/expected_keepers.go index b4f050e75cb..e0eb6c0ed72 100644 --- a/x/txfees/types/expected_keepers.go +++ b/x/txfees/types/expected_keepers.go @@ -8,7 +8,7 @@ import ( // SpotPriceCalculator defines the contract that must be fulfilled by a spot price calculator // The x/gamm keeper is expected to satisfy this interface. type SpotPriceCalculator interface { - CalculateSpotPrice(ctx sdk.Context, poolId uint64, tokenInDenom, tokenOutDenom string) (sdk.Dec, error) + CalculateSpotPrice(ctx sdk.Context, poolId uint64, quoteDenom, baseDenom string) (sdk.Dec, error) } // GammKeeper defines the contract needed for AccountKeeper related APIs. From a96cf584c34858c557a6d56a59473625b84749f8 Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 05:31:35 -0800 Subject: [PATCH 02/13] fix amm mock --- x/twap/types/twapmock/amminterface.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x/twap/types/twapmock/amminterface.go b/x/twap/types/twapmock/amminterface.go index 4b7b408d306..3f2ba2df9ab 100644 --- a/x/twap/types/twapmock/amminterface.go +++ b/x/twap/types/twapmock/amminterface.go @@ -52,7 +52,7 @@ func (p *ProgrammedAmmInterface) ProgramPoolDenomsOverride(poolId uint64, overri } func (p *ProgrammedAmmInterface) ProgramPoolSpotPriceOverride(poolId uint64, - baseDenom, quoteDenom string, overrideSp sdk.Dec, overrideErr error, + quoteDenom, baseDenom string, overrideSp sdk.Dec, overrideErr error, ) { input := SpotPriceInput{poolId, baseDenom, quoteDenom} p.programmedSpotPrice[input] = SpotPriceResult{overrideSp, overrideErr} @@ -71,12 +71,12 @@ func (p *ProgrammedAmmInterface) GetPoolDenoms(ctx sdk.Context, poolId uint64) ( func (p *ProgrammedAmmInterface) CalculateSpotPrice(ctx sdk.Context, poolId uint64, - baseDenom, - quoteDenom string, + quoteDenom, + baseDenom string, ) (price sdk.Dec, err error) { input := SpotPriceInput{poolId, baseDenom, quoteDenom} if res, ok := p.programmedSpotPrice[input]; ok { return res.Sp, res.Err } - return p.underlyingKeeper.CalculateSpotPrice(ctx, poolId, baseDenom, quoteDenom) + return p.underlyingKeeper.CalculateSpotPrice(ctx, poolId, quoteDenom, baseDenom) } From 09e0ce27ac3e2a70eb185f0a2591cf9fe2d6e3f0 Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 05:33:10 -0800 Subject: [PATCH 03/13] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c23240a067..5a945131ac2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Bug fixes * [#3608](https://github.com/osmosis-labs/osmosis/pull/3608) Make it possible to state export from any directory. +* [#3715](https://github.com/osmosis-labs/osmosis/pull/3715) Fix x/gamm CalculateSpotPrice, balancer.SpotPrice and Stableswap.SpotPrice base and quote asset. ### Misc Improvements From f9d1f582534f2e1fc19e220092c896e34a69eae7 Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 09:04:14 -0800 Subject: [PATCH 04/13] try fixing wasmbindings --- wasmbinding/queries.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wasmbinding/queries.go b/wasmbinding/queries.go index a020b12ef57..41882d0a7c5 100644 --- a/wasmbinding/queries.go +++ b/wasmbinding/queries.go @@ -63,8 +63,8 @@ func (qp QueryPlugin) GetSpotPrice(ctx sdk.Context, spotPrice *bindings.SpotPric } poolId := spotPrice.Swap.PoolId - baseAsset := spotPrice.Swap.DenomIn - quoteAsset := spotPrice.Swap.DenomOut + baseAsset := spotPrice.Swap.DenomOut + quoteAsset := spotPrice.Swap.DenomIn withSwapFee := spotPrice.WithSwapFee price, err := qp.gammKeeper.CalculateSpotPrice(ctx, poolId, quoteAsset, baseAsset) From d40e20b5110ef66fe537c470aac3867498d5763a Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 09:21:57 -0800 Subject: [PATCH 05/13] fix stableswap tests --- x/gamm/pool-models/stableswap/pool_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/gamm/pool-models/stableswap/pool_test.go b/x/gamm/pool-models/stableswap/pool_test.go index d3db4f8ba2f..4d1b9475d6f 100644 --- a/x/gamm/pool-models/stableswap/pool_test.go +++ b/x/gamm/pool-models/stableswap/pool_test.go @@ -1218,7 +1218,7 @@ func TestStableswapSpotPrice(t *testing.T) { quoteDenom: "bar", poolAssets: twoUnevenStablePoolAssets, scalingFactors: []uint64{10000, 20000}, - expectedPrice: sdk.NewDec(2), + expectedPrice: sdk.NewDecWithPrec(5, 1), expectPass: true, }, "even two-asset pool with different scaling factors (bar -> foo)": { @@ -1226,7 +1226,7 @@ func TestStableswapSpotPrice(t *testing.T) { quoteDenom: "foo", poolAssets: twoUnevenStablePoolAssets, scalingFactors: []uint64{10000, 20000}, - expectedPrice: sdk.NewDecWithPrec(5, 1), + expectedPrice: sdk.NewDec(2), expectPass: true, }, "uneven two-asset pool": { @@ -1338,7 +1338,7 @@ func TestStableswapSpotPrice(t *testing.T) { if (tc.expectedPrice != sdk.Dec{}) { expectedSpotPrice = tc.expectedPrice } else { - expectedSpotPrice, err = p.calcOutAmtGivenIn(sdk.NewInt64Coin(tc.quoteDenom, 1), tc.baseDenom, sdk.ZeroDec()) + expectedSpotPrice, err = p.calcOutAmtGivenIn(sdk.NewInt64Coin(tc.baseDenom, 1), tc.quoteDenom, sdk.ZeroDec()) require.NoError(t, err) } From 88ff590b0577bdc0daf82d79a11cf89d2ec40c62 Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 09:39:46 -0800 Subject: [PATCH 06/13] fix txfees tests --- x/txfees/keeper/feetokens.go | 2 +- x/txfees/keeper/feetokens_test.go | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/x/txfees/keeper/feetokens.go b/x/txfees/keeper/feetokens.go index ac97f8572cf..54f10280997 100644 --- a/x/txfees/keeper/feetokens.go +++ b/x/txfees/keeper/feetokens.go @@ -48,7 +48,7 @@ func (k Keeper) CalcFeeSpotPrice(ctx sdk.Context, inputDenom string) (sdk.Dec, e return sdk.Dec{}, err } - spotPrice, err := k.spotPriceCalculator.CalculateSpotPrice(ctx, feeToken.PoolID, feeToken.Denom, baseDenom) + spotPrice, err := k.spotPriceCalculator.CalculateSpotPrice(ctx, feeToken.PoolID, baseDenom, feeToken.Denom) if err != nil { return sdk.Dec{}, err } diff --git a/x/txfees/keeper/feetokens_test.go b/x/txfees/keeper/feetokens_test.go index fb6430f7b86..3cd091adc3e 100644 --- a/x/txfees/keeper/feetokens_test.go +++ b/x/txfees/keeper/feetokens_test.go @@ -179,7 +179,9 @@ func (suite *KeeperTestSuite) TestFeeTokenConversions() { baseDenomPoolInput: sdk.NewInt64Coin(baseDenom, 100), feeTokenPoolInput: sdk.NewInt64Coin("foo", 200), inputFee: sdk.NewInt64Coin("foo", 10), - // expected to get 5.000000000005368710 baseDenom without rounding + // expected to get approximately 5 base denom + // foo supply / stake supply = 200 / 100 = 2 foo for 1 stake + // 10 foo in / 2 foo for 1 stake = 5 base denom expectedOutput: sdk.NewInt64Coin(baseDenom, 5), expectedConvertable: true, }, @@ -215,7 +217,7 @@ func (suite *KeeperTestSuite) TestFeeTokenConversions() { converted, err := suite.App.TxFeesKeeper.ConvertToBaseToken(suite.Ctx, tc.inputFee) if tc.expectedConvertable { suite.Require().NoError(err, "test: %s", tc.name) - suite.Require().True(converted.IsEqual(tc.expectedOutput), "test: %s", tc.name) + suite.Require().Equal(tc.expectedOutput, converted) } else { suite.Require().Error(err, "test: %s", tc.name) } From 2b4174e057700a8501dafddaf026317edf34ee60 Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 10:03:23 -0800 Subject: [PATCH 07/13] fixes --- x/gamm/pool-models/balancer/pool.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x/gamm/pool-models/balancer/pool.go b/x/gamm/pool-models/balancer/pool.go index 907aa695618..e981c02a593 100644 --- a/x/gamm/pool-models/balancer/pool.go +++ b/x/gamm/pool-models/balancer/pool.go @@ -623,7 +623,7 @@ func (p *Pool) applySwap(ctx sdk.Context, tokensIn sdk.Coins, tokensOut sdk.Coin // // panics if the pool in state is incorrect, and has any weight that is 0. func (p Pool) SpotPrice(ctx sdk.Context, quoteAsset, baseAsset string) (spotPrice sdk.Dec, err error) { - quote, base, err := p.parsePoolAssetsByDenoms(baseAsset, quoteAsset) + quote, base, err := p.parsePoolAssetsByDenoms(quoteAsset, baseAsset) if err != nil { return sdk.Dec{}, err } @@ -632,9 +632,10 @@ func (p Pool) SpotPrice(ctx sdk.Context, quoteAsset, baseAsset string) (spotPric } // spot_price = (Quote Supply / Quote Weight) / (Base Supply / Base Weight) + // = (Quote Supply / Quote Weight) * (Base Weight / Base Supply) // spot_price = (Quote Weight / Base Weight) * (Quote Supply / Base Supply) - invWeightRatio := quote.Weight.ToDec().Quo(base.Weight.ToDec()) - supplyRatio := base.Token.Amount.ToDec().Quo(quote.Token.Amount.ToDec()) + invWeightRatio := base.Weight.ToDec().Quo(quote.Weight.ToDec()) + supplyRatio := quote.Token.Amount.ToDec().Quo(base.Token.Amount.ToDec()) spotPrice = supplyRatio.Mul(invWeightRatio) return spotPrice, err From 63dee1bef2d9f5398fa6d26731261cd2c11d676f Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 10:16:35 -0800 Subject: [PATCH 08/13] minor fixes --- x/gamm/pool-models/balancer/pool_suite_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x/gamm/pool-models/balancer/pool_suite_test.go b/x/gamm/pool-models/balancer/pool_suite_test.go index 4db7ce47d4f..a4269818291 100644 --- a/x/gamm/pool-models/balancer/pool_suite_test.go +++ b/x/gamm/pool-models/balancer/pool_suite_test.go @@ -773,13 +773,13 @@ func (suite *KeeperTestSuite) TestBalancerSpotPriceBounds() { suite.Run(tc.name, func() { // pool assets defaultBaseAsset := balancer.PoolAsset{ - Weight: tc.quoteDenomWeight, - Token: tc.quoteDenomPoolInput, - } - defaultQuoteAsset := balancer.PoolAsset{ Weight: tc.baseDenomWeight, Token: tc.baseDenomPoolInput, } + defaultQuoteAsset := balancer.PoolAsset{ + Weight: tc.quoteDenomWeight, + Token: tc.quoteDenomPoolInput, + } poolAssets := []balancer.PoolAsset{defaultBaseAsset, defaultQuoteAsset} poolId := suite.PrepareBalancerPoolWithPoolAsset(poolAssets) From 4f8cfebae5d53248e009eb1f4fe5c7ee811686da Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 10:19:25 -0800 Subject: [PATCH 09/13] comment --- x/twap/logic.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/twap/logic.go b/x/twap/logic.go index 648fe227980..20d7c6c338f 100644 --- a/x/twap/logic.go +++ b/x/twap/logic.go @@ -55,9 +55,9 @@ func getSpotPrices( previousErrorTime time.Time, ) (sp0 sdk.Dec, sp1 sdk.Dec, latestErrTime time.Time) { latestErrTime = previousErrorTime - // sp0 = denom0 base, denom1 quote. + // sp0 = denom0 quote, denom1 base. sp0, err0 := k.CalculateSpotPrice(ctx, poolId, denom0, denom1) - // sp1 = denom0 quote, denom1 base. + // sp1 = denom0 base, denom1 quote. sp1, err1 := k.CalculateSpotPrice(ctx, poolId, denom1, denom0) if err0 != nil || err1 != nil { latestErrTime = ctx.BlockTime() From b258cfa14c2da739bb48b716893772cadcb31c82 Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 13:21:34 -0500 Subject: [PATCH 10/13] Update x/gamm/keeper/grpc_query.go --- x/gamm/keeper/grpc_query.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gamm/keeper/grpc_query.go b/x/gamm/keeper/grpc_query.go index d163d0dd663..82114bcfd0c 100644 --- a/x/gamm/keeper/grpc_query.go +++ b/x/gamm/keeper/grpc_query.go @@ -349,8 +349,8 @@ func (q Querier) SpotPrice(ctx context.Context, req *types.QuerySpotPriceRequest sdkCtx := sdk.UnwrapSDKContext(ctx) - // Note: the base and quote asset are provided as argument incorrectly intentionally. - // due to the historic bug in the original implementation. + // Note: the base and quote asset argument order is intentionally incorrect + // due to a historic bug in the original implementation. sp, err := q.Keeper.CalculateSpotPrice(sdkCtx, req.PoolId, req.BaseAssetDenom, req.QuoteAssetDenom) if err != nil { return nil, status.Error(codes.Internal, err.Error()) From 6652e9a236a0e72f64aa5e3c0720ab35cbd280e9 Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 13:21:41 -0500 Subject: [PATCH 11/13] Update x/gamm/pool-models/balancer/pool.go --- x/gamm/pool-models/balancer/pool.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gamm/pool-models/balancer/pool.go b/x/gamm/pool-models/balancer/pool.go index e981c02a593..9d61ffa9974 100644 --- a/x/gamm/pool-models/balancer/pool.go +++ b/x/gamm/pool-models/balancer/pool.go @@ -633,7 +633,7 @@ func (p Pool) SpotPrice(ctx sdk.Context, quoteAsset, baseAsset string) (spotPric // spot_price = (Quote Supply / Quote Weight) / (Base Supply / Base Weight) // = (Quote Supply / Quote Weight) * (Base Weight / Base Supply) - // spot_price = (Quote Weight / Base Weight) * (Quote Supply / Base Supply) + // = (Quote Weight / Base Weight) * (Quote Supply / Base Supply) invWeightRatio := base.Weight.ToDec().Quo(quote.Weight.ToDec()) supplyRatio := quote.Token.Amount.ToDec().Quo(base.Token.Amount.ToDec()) spotPrice = supplyRatio.Mul(invWeightRatio) From d8de9b84a8720b30a7a61aa3a9e2052e6c6a9286 Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 14 Dec 2022 10:25:32 -0800 Subject: [PATCH 12/13] fix comment --- x/gamm/pool-models/balancer/pool.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gamm/pool-models/balancer/pool.go b/x/gamm/pool-models/balancer/pool.go index 9d61ffa9974..a3930216ccb 100644 --- a/x/gamm/pool-models/balancer/pool.go +++ b/x/gamm/pool-models/balancer/pool.go @@ -633,7 +633,7 @@ func (p Pool) SpotPrice(ctx sdk.Context, quoteAsset, baseAsset string) (spotPric // spot_price = (Quote Supply / Quote Weight) / (Base Supply / Base Weight) // = (Quote Supply / Quote Weight) * (Base Weight / Base Supply) - // = (Quote Weight / Base Weight) * (Quote Supply / Base Supply) + // = (Base Weight / Quote Weight) * (Quote Supply / Base Supply) invWeightRatio := base.Weight.ToDec().Quo(quote.Weight.ToDec()) supplyRatio := quote.Token.Amount.ToDec().Quo(base.Token.Amount.ToDec()) spotPrice = supplyRatio.Mul(invWeightRatio) From cdfff9fa2b128421a6b36eac7f01b3760c81480a Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 16 Dec 2022 15:06:39 -0800 Subject: [PATCH 13/13] nicolas's comment --- x/gamm/pool-models/balancer/pool.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/gamm/pool-models/balancer/pool.go b/x/gamm/pool-models/balancer/pool.go index a3930216ccb..ac2bd624412 100644 --- a/x/gamm/pool-models/balancer/pool.go +++ b/x/gamm/pool-models/balancer/pool.go @@ -618,8 +618,10 @@ func (p *Pool) applySwap(ctx sdk.Context, tokensIn sdk.Coins, tokensOut sdk.Coin // As an example, assume equal weights. uosmo supply of 2 and uatom supply of 4. // // Case 1: base = uosmo, quote = uatom -> for one uosmo, get 2 uatom = 4 / 2 = 2 +// In other words, it costs 2 uatom to get one uosmo. // // Case 2: base = uatom, quote = uosmo -> for one uatom, get 0.5 uosmo = 2 / 4 = 0.5 +// In other words, it costs 0.5 uosmo to get one uatom. // // panics if the pool in state is incorrect, and has any weight that is 0. func (p Pool) SpotPrice(ctx sdk.Context, quoteAsset, baseAsset string) (spotPrice sdk.Dec, err error) {