From 61cfb64df780087b59f1b2441fd17cc323567c24 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Tue, 22 Mar 2022 15:26:17 +0100 Subject: [PATCH] Custom queries unit tests (#10) * Add full denom unit tests * Add get pool state unit tests * Add get spot price unit tests * Add estimate price extra checks * Add estimate price unit tests * Add tests for zero and negative amounts * Fix: null spot price * Fix: Avoid panic on negative amounts Fix rebase errors / missing null checks * Fix rebase errors Add negative amount checks --- app/wasm/message_plugin.go | 10 +- app/wasm/queries.go | 17 ++ app/wasm/test/queries_test.go | 485 ++++++++++++++++++++++++++++++++++ 3 files changed, 510 insertions(+), 2 deletions(-) create mode 100644 app/wasm/test/queries_test.go diff --git a/app/wasm/message_plugin.go b/app/wasm/message_plugin.go index 5a44e7db8ba..669663424d8 100644 --- a/app/wasm/message_plugin.go +++ b/app/wasm/message_plugin.go @@ -93,6 +93,9 @@ func performSwap(keeper *gammkeeper.Keeper, ctx sdk.Context, contractAddr sdk.Ac TokenOutDenom: step.DenomOut, }) } + if swap.Amount.ExactIn.Input.IsNegative() { + return nil, wasmvmtypes.InvalidRequest{Err: "gamm perform swap negative amount in"} + } tokenIn := sdk.Coin{ Denom: swap.First.DenomIn, Amount: swap.Amount.ExactIn.Input, @@ -100,7 +103,7 @@ func performSwap(keeper *gammkeeper.Keeper, ctx sdk.Context, contractAddr sdk.Ac tokenOutMinAmount := swap.Amount.ExactIn.MinOutput estimatedAmount, err := keeper.MultihopSwapExactAmountIn(ctx, contractAddr, routes, tokenIn, tokenOutMinAmount) if err != nil { - return nil, sdkerrors.Wrap(err, "gamm estimate price exact amount in") + return nil, sdkerrors.Wrap(err, "gamm perform swap exact amount in") } return &wasmbindings.SwapAmount{Out: &estimatedAmount}, nil } else if swap.Amount.ExactOut != nil { @@ -117,13 +120,16 @@ func performSwap(keeper *gammkeeper.Keeper, ctx sdk.Context, contractAddr sdk.Ac output = step.DenomOut } tokenInMaxAmount := swap.Amount.ExactOut.MaxInput + if swap.Amount.ExactOut.Output.IsNegative() { + return nil, wasmvmtypes.InvalidRequest{Err: "gamm perform swap negative amount out"} + } tokenOut := sdk.Coin{ Denom: output, Amount: swap.Amount.ExactOut.Output, } estimatedAmount, err := keeper.MultihopSwapExactAmountOut(ctx, contractAddr, routes, tokenInMaxAmount, tokenOut) if err != nil { - return nil, sdkerrors.Wrap(err, "gamm estimate price exact amount out") + return nil, sdkerrors.Wrap(err, "gamm perform swap exact amount out") } return &wasmbindings.SwapAmount{In: &estimatedAmount}, nil } else { diff --git a/app/wasm/queries.go b/app/wasm/queries.go index 1a952e2c838..024ab1a8620 100644 --- a/app/wasm/queries.go +++ b/app/wasm/queries.go @@ -1,6 +1,7 @@ package wasm import ( + wasmvmtypes "github.com/CosmWasm/wasmvm/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -38,6 +39,9 @@ func (qp QueryPlugin) GetPoolState(ctx sdk.Context, poolId uint64) (*types.PoolS } func (qp QueryPlugin) GetSpotPrice(ctx sdk.Context, spotPrice *wasmbindings.SpotPrice) (*sdk.Dec, error) { + if spotPrice == nil { + return nil, wasmvmtypes.InvalidRequest{Err: "gamm spot price null"} + } poolId := spotPrice.Swap.PoolId denomIn := spotPrice.Swap.DenomIn denomOut := spotPrice.Swap.DenomOut @@ -56,11 +60,24 @@ func (qp QueryPlugin) GetSpotPrice(ctx sdk.Context, spotPrice *wasmbindings.Spot } func (qp QueryPlugin) EstimatePrice(ctx sdk.Context, estimatePrice *wasmbindings.EstimatePrice) (*wasmbindings.SwapAmount, error) { + if estimatePrice == nil { + return nil, wasmvmtypes.InvalidRequest{Err: "gamm estimate price null"} + } + if err := sdk.ValidateDenom(estimatePrice.First.DenomIn); err != nil { + return nil, sdkerrors.Wrap(err, "gamm estimate price denom in") + } + if err := sdk.ValidateDenom(estimatePrice.First.DenomOut); err != nil { + return nil, sdkerrors.Wrap(err, "gamm estimate price denom out") + } contractAddr, err := sdk.AccAddressFromBech32(estimatePrice.Contract) if err != nil { return nil, sdkerrors.Wrap(err, "gamm estimate price sender address") } + if estimatePrice.Amount == (wasmbindings.SwapAmount{}) { + return nil, wasmvmtypes.InvalidRequest{Err: "gamm estimate price empty swap"} + } + estimate, err := performSwap(qp.gammKeeper, ctx, contractAddr, estimatePrice.ToSwapMsg()) return estimate, err } diff --git a/app/wasm/test/queries_test.go b/app/wasm/test/queries_test.go new file mode 100644 index 00000000000..64cdabf11bf --- /dev/null +++ b/app/wasm/test/queries_test.go @@ -0,0 +1,485 @@ +package wasm + +import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" + wasmbindings "github.com/osmosis-labs/osmosis/v7/app/wasm/bindings" + "github.com/osmosis-labs/osmosis/v7/app/wasm/types" + "github.com/stretchr/testify/assert" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/osmosis-labs/osmosis/v7/app/wasm" +) + +func TestFullDenom(t *testing.T) { + actor := RandomAccountAddress() + + specs := map[string]struct { + addr string + subDenom string + expFullDenom string + expErr bool + }{ + "valid address": { + addr: actor.String(), + subDenom: "sub_denom_1", + expFullDenom: fmt.Sprintf("cw/%s/sub_denom_1", actor.String()), + }, + "empty address": { + addr: "", + subDenom: "sub_denom_1", + expErr: true, + }, + "invalid address": { + addr: "invalid", + subDenom: "sub_denom_1", + expErr: true, + }, + "empty sub-denom": { + addr: actor.String(), + subDenom: "", + expErr: true, + }, + } + for name, spec := range specs { + t.Run(name, func(t *testing.T) { + // when + gotFullDenom, gotErr := wasm.GetFullDenom(spec.addr, spec.subDenom) + // then + if spec.expErr { + require.Error(t, gotErr) + return + } + require.NoError(t, gotErr) + assert.Equal(t, spec.expFullDenom, gotFullDenom, "exp %s but got %s", spec.expFullDenom, gotFullDenom) + }) + } +} + +func TestPoolState(t *testing.T) { + actor := RandomAccountAddress() + osmosis, ctx := SetupCustomApp(t, actor) + + fundAccount(t, ctx, osmosis, actor, defaultFunds) + + poolFunds := []sdk.Coin{ + sdk.NewInt64Coin("uosmo", 12000000), + sdk.NewInt64Coin("ustar", 240000000), + } + // 20 star to 1 osmo + starPool := preparePool(t, ctx, osmosis, actor, poolFunds) + + // FIXME: Derive / obtain these values + starSharesDenom := fmt.Sprintf("gamm/pool/%d", starPool) + starSharedAmount, _ := sdk.NewIntFromString("100_000_000_000_000_000_000") + + queryPlugin := wasm.NewQueryPlugin(osmosis.GAMMKeeper) + + specs := map[string]struct { + poolId uint64 + expPoolState *types.PoolState + expErr bool + }{ + "existent pool id": { + poolId: starPool, + expPoolState: &types.PoolState{ + Assets: poolFunds, + Shares: sdk.NewCoin(starSharesDenom, starSharedAmount), + }, + }, + "non-existent pool id": { + poolId: starPool + 1, + expErr: true, + }, + "zero pool id": { + poolId: 0, + expErr: true, + }, + } + for name, spec := range specs { + t.Run(name, func(t *testing.T) { + // when + gotPoolState, gotErr := queryPlugin.GetPoolState(ctx, spec.poolId) + // then + if spec.expErr { + require.Error(t, gotErr) + return + } + require.NoError(t, gotErr) + assert.Equal(t, spec.expPoolState, gotPoolState, "exp %s but got %s", spec.expPoolState, gotPoolState) + }) + } +} + +func TestSpotPrice(t *testing.T) { + actor := RandomAccountAddress() + swapFee := 0. // FIXME: Set / support an actual fee + epsilon := 1e-6 + osmosis, ctx := SetupCustomApp(t, actor) + + fundAccount(t, ctx, osmosis, actor, defaultFunds) + + poolFunds := []sdk.Coin{ + sdk.NewInt64Coin("uosmo", 12000000), + sdk.NewInt64Coin("ustar", 240000000), + } + // 20 star to 1 osmo + starPool := preparePool(t, ctx, osmosis, actor, poolFunds) + + uosmo := poolFunds[0].Amount.ToDec().MustFloat64() + ustar := poolFunds[1].Amount.ToDec().MustFloat64() + + starPrice := sdk.MustNewDecFromStr(fmt.Sprintf("%f", uosmo/ustar)) + starFee := sdk.MustNewDecFromStr(fmt.Sprintf("%f", swapFee)) + starPriceWithFee := starPrice.Add(starFee) + + queryPlugin := wasm.NewQueryPlugin(osmosis.GAMMKeeper) + + specs := map[string]struct { + spotPrice *wasmbindings.SpotPrice + expPrice *sdk.Dec + expErr bool + }{ + "valid spot price": { + spotPrice: &wasmbindings.SpotPrice{ + Swap: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + WithSwapFee: false, + }, + expPrice: &starPrice, + }, + "valid spot price with fee": { + spotPrice: &wasmbindings.SpotPrice{ + Swap: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + WithSwapFee: true, + }, + expPrice: &starPriceWithFee, + }, + "non-existent pool id": { + spotPrice: &wasmbindings.SpotPrice{ + Swap: wasmbindings.Swap{ + PoolId: starPool + 2, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + WithSwapFee: false, + }, + expErr: true, + }, + "zero pool id": { + spotPrice: &wasmbindings.SpotPrice{ + Swap: wasmbindings.Swap{ + PoolId: 0, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + WithSwapFee: false, + }, + expErr: true, + }, + "invalid denom in": { + spotPrice: &wasmbindings.SpotPrice{ + Swap: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "invalid", + DenomOut: "ustar", + }, + WithSwapFee: false, + }, + expErr: true, + }, + "empty denom in": { + spotPrice: &wasmbindings.SpotPrice{ + Swap: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "", + DenomOut: "ustar", + }, + WithSwapFee: false, + }, + expErr: true, + }, + "invalid denom out": { + spotPrice: &wasmbindings.SpotPrice{ + Swap: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "uosmo", + DenomOut: "invalid", + }, + WithSwapFee: false, + }, + expErr: true, + }, + "empty denom out": { + spotPrice: &wasmbindings.SpotPrice{ + Swap: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "uosmo", + DenomOut: "", + }, + WithSwapFee: false, + }, + expErr: true, + }, + "null spot price": { + spotPrice: nil, + expErr: true, + }, + } + for name, spec := range specs { + t.Run(name, func(t *testing.T) { + // when + gotPrice, gotErr := queryPlugin.GetSpotPrice(ctx, spec.spotPrice) + // then + if spec.expErr { + require.Error(t, gotErr) + return + } + require.NoError(t, gotErr) + assert.InEpsilonf(t, spec.expPrice.MustFloat64(), gotPrice.MustFloat64(), epsilon, "exp %s but got %s", spec.expPrice.String(), gotPrice.String()) + }) + } +} + +func TestEstimatePrice(t *testing.T) { + actor := RandomAccountAddress() + osmosis, ctx := SetupCustomApp(t, actor) + epsilon := 1e-3 + + fundAccount(t, ctx, osmosis, actor, defaultFunds) + + poolFunds := []sdk.Coin{ + sdk.NewInt64Coin("uosmo", 12000000), + sdk.NewInt64Coin("ustar", 240000000), + } + // 20 star to 1 osmo + starPool := preparePool(t, ctx, osmosis, actor, poolFunds) + + // Estimate swap rate + uosmo := poolFunds[0].Amount.ToDec().MustFloat64() + ustar := poolFunds[1].Amount.ToDec().MustFloat64() + swapRate := ustar / uosmo + + amountIn := sdk.NewInt(10000) + zeroAmount := sdk.ZeroInt() + negativeAmount := amountIn.Neg() + + amount := amountIn.ToDec().MustFloat64() + starAmount := sdk.NewInt(int64(amount * swapRate)) + + starSwapAmount := wasmbindings.SwapAmount{Out: &starAmount} + + queryPlugin := wasm.NewQueryPlugin(osmosis.GAMMKeeper) + + specs := map[string]struct { + estimatePrice *wasmbindings.EstimatePrice + expCost *wasmbindings.SwapAmount + expErr bool + }{ + "valid estimate price (exact in)": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + In: &amountIn, + }, + }, + expCost: &starSwapAmount, + }, + "non-existent pool id": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool + 3, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + In: &amountIn, + }, + }, + expErr: true, + }, + "zero pool id": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: 0, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + In: &amountIn, + }, + }, + expErr: true, + }, + "invalid denom in": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "invalid", + DenomOut: "ustar", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + In: &amountIn, + }, + }, + expErr: true, + }, + "empty denom in": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "", + DenomOut: "ustar", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + In: &amountIn, + }, + }, + expErr: true, + }, + "invalid denom out": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "ustar", + DenomOut: "invalid", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + In: &amountIn, + }, + }, + expErr: true, + }, + "empty denom out": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "ustar", + DenomOut: "", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + In: &amountIn, + }, + }, + expErr: true, + }, + "null estimate price": { + estimatePrice: nil, + expErr: true, + }, + "empty swap amount": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{}, + }, + expErr: true, + }, + "zero amount in": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + In: &zeroAmount, + }, + }, + expErr: true, + }, + "zero amount out": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + Out: &zeroAmount, + }, + }, + expErr: true, + }, + "negative amount in": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + In: &negativeAmount, + }, + }, + expErr: true, + }, + "negative amount out": { + estimatePrice: &wasmbindings.EstimatePrice{ + Contract: actor.String(), + First: wasmbindings.Swap{ + PoolId: starPool, + DenomIn: "uosmo", + DenomOut: "ustar", + }, + Route: nil, + Amount: wasmbindings.SwapAmount{ + Out: &negativeAmount, + }, + }, + expErr: true, + }, + } + for name, spec := range specs { + t.Run(name, func(t *testing.T) { + // when + gotCost, gotErr := queryPlugin.EstimatePrice(ctx, spec.estimatePrice) + // then + if spec.expErr { + require.Error(t, gotErr) + return + } + require.NoError(t, gotErr) + assert.InEpsilonf(t, (*spec.expCost.Out).ToDec().MustFloat64(), (*gotCost.Out).ToDec().MustFloat64(), epsilon, "exp %s but got %s", spec.expCost.Out.String(), gotCost.Out.String()) + }) + } + +}