fix(gamm): SpotPrice keeper function (#3715)

* fix(gamm): SpotPrice keeper function

* fix amm mock

* changelog

* try fixing wasmbindings

* fix stableswap tests

* fix txfees tests

* fixes

* minor fixes

* comment

* Update x/gamm/keeper/grpc_query.go

* Update x/gamm/pool-models/balancer/pool.go

* fix comment

* nicolas's comment

CHANGELOG.md

@@ -59,6 +59,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
 

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

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.DenomOut
+	quoteAsset := spotPrice.Swap.DenomIn
 	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")
 	}

x/gamm/keeper/grpc_query.go

@@ -360,6 +360,8 @@ func (q Querier) SpotPrice(ctx context.Context, req *types.QuerySpotPriceRequest
 
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
 
+	// 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())

x/gamm/keeper/pool_service.go

@@ -23,8 +23,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 {
@@ -39,7 +39,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
 	}

x/gamm/keeper/pool_service_test.go

@@ -240,8 +240,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": {
@@ -264,7 +264,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)

x/gamm/pool-models/balancer/pool.go

@@ -611,14 +611,20 @@ 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
+// 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.
-// TODO: Come back and improve docs for this.
-func (p Pool) SpotPrice(ctx sdk.Context, baseAsset, quoteAsset string) (spotPrice sdk.Dec, err error) {
+func (p Pool) SpotPrice(ctx sdk.Context, quoteAsset, baseAsset string) (spotPrice sdk.Dec, err error) {
 	quote, base, err := p.parsePoolAssetsByDenoms(quoteAsset, baseAsset)
 	if err != nil {
 		return sdk.Dec{}, err
@@ -627,10 +633,11 @@ 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)
-	invWeightRatio := quote.Weight.ToDec().Quo(base.Weight.ToDec())
-	supplyRatio := base.Token.Amount.ToDec().Quo(quote.Token.Amount.ToDec())
+	// spot_price = (Quote Supply / Quote Weight) / (Base Supply / Base Weight)
+	//            = (Quote Supply / Quote Weight) * (Base Weight / 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)
 
 	return spotPrice, err

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,
 		},
 	}
@@ -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 {

x/gamm/pool-models/stableswap/amm.go

@@ -356,7 +356,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
@@ -372,8 +372,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
 }

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 {

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 (
@@ -1217,15 +1218,15 @@ 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)": {
 			baseDenom:      "bar",
 			quoteDenom:     "foo",
 			poolAssets:     twoUnevenStablePoolAssets,
 			scalingFactors: []uint64{10000, 20000},
-			expectedPrice:  sdk.NewDecWithPrec(5, 1),
+			expectedPrice:  sdk.NewDec(2),
 			expectPass:     true,
 		},
 		"uneven two-asset pool": {
@@ -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)
@@ -1337,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)
 				}
 

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)
 }

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 quote, denom1 base.
 	sp0, err0 := k.CalculateSpotPrice(ctx, poolId, denom0, denom1)
+	// sp1 = denom0 base, denom1 quote.
 	sp1, err1 := k.CalculateSpotPrice(ctx, poolId, denom1, denom0)
 	if err0 != nil || err1 != nil {
 		latestErrTime = ctx.BlockTime()

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)
 }

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)
 }

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"

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)
 			}

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.