Skip to content

Commit

Permalink
don't check bank balance for estimateXX queries
Browse files Browse the repository at this point in the history
also general cleanup for estimateXX
  • Loading branch information
jcompagni10 committed May 29, 2024
1 parent 063805a commit faddb43
Show file tree
Hide file tree
Showing 8 changed files with 288 additions and 471 deletions.
28 changes: 11 additions & 17 deletions proto/neutron/dex/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -260,25 +260,22 @@ message QueryGetPoolReservesResponse {
}

message QueryEstimateMultiHopSwapRequest {
string creator = 1;
string receiver = 2;
repeated MultiHopRoute routes = 3;
string amount_in = 4 [
repeated MultiHopRoute routes = 1;
string amount_in = 2 [
(gogoproto.moretags) = "yaml:\"amount_in\"",
(gogoproto.customtype) = "cosmossdk.io/math.Int",
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "amount_in"
];
string exit_limit_price = 5 [
string exit_limit_price = 3 [
(gogoproto.moretags) = "yaml:\"exit_limit_price\"",
(gogoproto.customtype) = "github.com/neutron-org/neutron/v4/utils/math.PrecDec",
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "exit_limit_price"
];

// If pickBestRoute == true then all routes are run and the route with the
// best price is chosen otherwise, the first succesful route is used.
bool pick_best_route = 6;
bool pick_best_route = 4;
}

message QueryEstimateMultiHopSwapResponse {
Expand All @@ -290,25 +287,22 @@ message QueryEstimateMultiHopSwapResponse {
}

message QueryEstimatePlaceLimitOrderRequest {
string creator = 1;
string receiver = 2;
string token_in = 3;
string token_out = 4;
int64 tick_index_in_to_out = 5;
string amount_in = 6 [
string token_in = 1;
string token_out = 2;
int64 tick_index_in_to_out = 3;
string amount_in = 4 [
(gogoproto.moretags) = "yaml:\"amount_in\"",
(gogoproto.customtype) = "cosmossdk.io/math.Int",
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "amount_in"
];
LimitOrderType order_type = 7;

LimitOrderType order_type = 5;
// expirationTime is only valid iff orderType == GOOD_TIL_TIME.
google.protobuf.Timestamp expiration_time = 8 [
google.protobuf.Timestamp expiration_time = 9 [
(gogoproto.stdtime) = true,
(gogoproto.nullable) = true
];
string maxAmount_out = 9 [
string max_amount_out = 6 [
(gogoproto.moretags) = "yaml:\"max_amount_out\"",
(gogoproto.customtype) = "cosmossdk.io/math.Int",
(gogoproto.nullable) = true,
Expand Down
2 changes: 0 additions & 2 deletions wasmbinding/queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ func (qp *QueryPlugin) DexQuery(ctx sdk.Context, query bindings.DexQuery) (data
data, err = dexQuery(ctx, query.EstimateMultiHopSwap, qp.dexKeeper.EstimateMultiHopSwap)
case query.EstimatePlaceLimitOrder != nil:
q := dextypes.QueryEstimatePlaceLimitOrderRequest{
Creator: query.EstimatePlaceLimitOrder.Creator,
Receiver: query.EstimatePlaceLimitOrder.Receiver,
TokenIn: query.EstimatePlaceLimitOrder.TokenIn,
TokenOut: query.EstimatePlaceLimitOrder.TokenOut,
TickIndexInToOut: query.EstimatePlaceLimitOrder.TickIndexInToOut,
Expand Down
17 changes: 8 additions & 9 deletions x/dex/keeper/grpc_query_estimate_multi_hop_swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@ import (
"github.com/neutron-org/neutron/v4/x/dex/types"
)

// TODO: This doesn't run ValidateBasic() checks.
func (k Keeper) EstimateMultiHopSwap(
goCtx context.Context,
req *types.QueryEstimateMultiHopSwapRequest,
) (*types.QueryEstimateMultiHopSwapResponse, error) {
msg := types.MsgMultiHopSwap{
Creator: req.Creator,
Receiver: req.Receiver,
Creator: "neutron1dft8nwxzr0u27wvr2cknpermjkreqvp9fdy0uz",
Receiver: "neutron1dft8nwxzr0u27wvr2cknpermjkreqvp9fdy0uz",
Routes: req.Routes,
AmountIn: req.AmountIn,
ExitLimitPrice: req.ExitLimitPrice,
Expand All @@ -28,23 +27,23 @@ func (k Keeper) EstimateMultiHopSwap(
ctx := sdk.UnwrapSDKContext(goCtx)
cacheCtx, _ := ctx.CacheContext()

callerAddr := sdk.MustAccAddressFromBech32(req.Creator)
receiverAddr := sdk.MustAccAddressFromBech32(req.Receiver)

oldBK := k.bankKeeper
k.bankKeeper = NewSimulationBankKeeper(k.bankKeeper)
coinOut, err := k.MultiHopSwapCore(
cacheCtx,
req.AmountIn,
req.Routes,
req.ExitLimitPrice,
req.PickBestRoute,
callerAddr,
receiverAddr,
[]byte("caller"),
[]byte("receiver"),
)
if err != nil {
return nil, err
}

// NB: Critically, we do not write the best route's buffered state context since this is only an estimate.
//nolint:staticcheck // Should be unnecessary but out of an abundance of caution we restore the old bankkeeper
k.bankKeeper = oldBK

return &types.QueryEstimateMultiHopSwapResponse{CoinOut: coinOut}, nil
}
27 changes: 14 additions & 13 deletions x/dex/keeper/grpc_query_estimate_multi_hop_swap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ package keeper_test
import (
"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

math_utils "github.com/neutron-org/neutron/v4/utils/math"
"github.com/neutron-org/neutron/v4/x/dex/types"
)

func (s *DexTestSuite) TestEstimateMultiHopSwapSingleRoute() {
s.fundAliceBalances(100, 0)

// GIVEN liquidity in pools A<>B, B<>C, C<>D,
s.SetupMultiplePools(
NewPoolSetup("TokenA", "TokenB", 0, 100, 0, 1),
Expand All @@ -20,17 +19,19 @@ func (s *DexTestSuite) TestEstimateMultiHopSwapSingleRoute() {

// WHEN alice multihopswaps A<>B => B<>C => C<>D,
route := [][]string{{"TokenA", "TokenB", "TokenC", "TokenD"}}
coinOut := s.aliceEstimatesMultiHopSwap(route, 100, math_utils.MustNewPrecDecFromStr("0.9"), false)
coinOut := s.estimatesMultiHopSwap(route, 100, math_utils.MustNewPrecDecFromStr("0.9"), false)

// THEN alice would get out ~99 BIGTokenD
s.Assert().Equal(math.NewInt(99970003), coinOut.Amount)
s.assertAccountBalanceWithDenom(s.alice, "TokenA", 100)
s.assertAccountBalanceWithDenom(s.alice, "TokenD", 0)

// AND state is not altered
s.assertAccountBalanceWithDenom(s.alice, "TokenD", 0)
s.assertDexBalanceWithDenom("TokenA", 0)
s.assertDexBalanceWithDenom("TokenB", 100)
s.assertDexBalanceWithDenom("TokenC", 100)
s.assertDexBalanceWithDenom("TokenD", 100)

s.assertBobLimitSellFails(sdkerrors.ErrInsufficientFunds, "TokenA", -400_000, 100_000_000)
}

func (s *DexTestSuite) TestEstimateMultiHopSwapInsufficientLiquiditySingleRoute() {
Expand All @@ -45,7 +46,7 @@ func (s *DexTestSuite) TestEstimateMultiHopSwapInsufficientLiquiditySingleRoute(

// THEN estimate multihopswap fails
route := [][]string{{"TokenA", "TokenB", "TokenC", "TokenD"}}
s.aliceEstimatesMultiHopSwapFails(
s.estimatesMultiHopSwapFails(
types.ErrLimitPriceNotSatisfied,
route,
100,
Expand All @@ -66,7 +67,7 @@ func (s *DexTestSuite) TestEstimateMultiHopSwapLimitPriceNotMetSingleRoute() {

// THEN estimate multihopswap fails
route := [][]string{{"TokenA", "TokenB", "TokenC", "TokenD"}}
s.aliceEstimatesMultiHopSwapFails(
s.estimatesMultiHopSwapFails(
types.ErrLimitPriceNotSatisfied,
route,
50,
Expand Down Expand Up @@ -106,7 +107,7 @@ func (s *DexTestSuite) TestEstimateMultiHopSwapMultiRouteOneGood() {
1,
)

coinOut := s.aliceEstimatesMultiHopSwap(routes, 100, math_utils.MustNewPrecDecFromStr("0.91"), false)
coinOut := s.estimatesMultiHopSwap(routes, 100, math_utils.MustNewPrecDecFromStr("0.91"), false)

// THEN swap estimation succeeds through route A<>B, B<>E, E<>X

Expand Down Expand Up @@ -205,7 +206,7 @@ func (s *DexTestSuite) TestEstimateMultiHopSwapMultiRouteAllFail() {
}

// Then fails with findBestRoute
s.aliceEstimatesMultiHopSwapFails(
s.estimatesMultiHopSwapFails(
types.ErrLimitPriceNotSatisfied,
routes,
100,
Expand All @@ -215,7 +216,7 @@ func (s *DexTestSuite) TestEstimateMultiHopSwapMultiRouteAllFail() {

// and with findFirstRoute

s.aliceEstimatesMultiHopSwapFails(
s.estimatesMultiHopSwapFails(
types.ErrLimitPriceNotSatisfied,
routes,
100,
Expand Down Expand Up @@ -244,7 +245,7 @@ func (s *DexTestSuite) TestEstimateMultiHopSwapMultiRouteFindBestRoute() {
{"TokenA", "TokenB", "TokenD", "TokenX"},
{"TokenA", "TokenB", "TokenE", "TokenX"},
}
coinOut := s.aliceEstimatesMultiHopSwap(routes, 100, math_utils.MustNewPrecDecFromStr("0.9"), true)
coinOut := s.estimatesMultiHopSwap(routes, 100, math_utils.MustNewPrecDecFromStr("0.9"), true)

// THEN swap succeeds through route A<>B, B<>E, E<>X

Expand Down Expand Up @@ -336,7 +337,7 @@ func (s *DexTestSuite) TestEstimateMultiHopSwapLongRouteWithCache() {
"TokenG", "TokenH", "TokenI", "TokenJ", "TokenK", "TokenM", "TokenX",
},
}
coinOut := s.aliceEstimatesMultiHopSwap(routes, 100, math_utils.MustNewPrecDecFromStr("0.8"), true)
coinOut := s.estimatesMultiHopSwap(routes, 100, math_utils.MustNewPrecDecFromStr("0.8"), true)

// THEN swap succeeds with second route
s.Assert().Equal(coinOut, sdk.NewCoin("TokenX", math.NewInt(99880066)))
Expand All @@ -359,7 +360,7 @@ func (s *DexTestSuite) TestEstimateMultiHopSwapEventsEmitted() {
)

route := [][]string{{"TokenA", "TokenB", "TokenC"}}
_ = s.aliceEstimatesMultiHopSwap(route, 100, math_utils.MustNewPrecDecFromStr("0.9"), false)
_ = s.estimatesMultiHopSwap(route, 100, math_utils.MustNewPrecDecFromStr("0.9"), false)

// 8 tickUpdateEvents are emitted 4x for pool setup 4x for two swaps
s.AssertEventValueNotEmitted(types.TickUpdateEventKey, "Expected no events")
Expand Down
28 changes: 11 additions & 17 deletions x/dex/keeper/grpc_query_estimate_place_limit_order.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,18 @@ package keeper
import (
"context"

sdkerrors "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/neutron-org/neutron/v4/x/dex/types"
)

// TODO: This doesn't run ValidateBasic() checks.
func (k Keeper) EstimatePlaceLimitOrder(
goCtx context.Context,
req *types.QueryEstimatePlaceLimitOrderRequest,
) (*types.QueryEstimatePlaceLimitOrderResponse, error) {
msg := types.MsgPlaceLimitOrder{
Creator: req.Creator,
Receiver: req.Receiver,
Creator: "neutron1dft8nwxzr0u27wvr2cknpermjkreqvp9fdy0uz",
Receiver: "neutron1dft8nwxzr0u27wvr2cknpermjkreqvp9fdy0uz",
TokenIn: req.TokenIn,
TokenOut: req.TokenOut,
TickIndexInToOut: req.TickIndexInToOut,
Expand All @@ -32,18 +30,13 @@ func (k Keeper) EstimatePlaceLimitOrder(
ctx := sdk.UnwrapSDKContext(goCtx)
cacheCtx, _ := ctx.CacheContext()

callerAddr := sdk.MustAccAddressFromBech32(req.Creator)
receiverAddr := sdk.MustAccAddressFromBech32(req.Receiver)

blockTime := cacheCtx.BlockTime()
if req.OrderType.IsGoodTil() && !req.ExpirationTime.After(blockTime) {
return nil, sdkerrors.Wrapf(types.ErrExpirationTimeInPast,
"Current BlockTime: %s; Provided ExpirationTime: %s",
blockTime.String(),
req.ExpirationTime.String(),
)
err := msg.ValidateGoodTilExpiration(ctx.BlockTime())
if err != nil {
return nil, err
}

oldBK := k.bankKeeper
k.bankKeeper = NewSimulationBankKeeper(k.bankKeeper)
_, totalInCoin, swapInCoin, swapOutCoin, err := k.PlaceLimitOrderCore(
cacheCtx,
req.TokenIn,
Expand All @@ -53,14 +46,15 @@ func (k Keeper) EstimatePlaceLimitOrder(
req.OrderType,
req.ExpirationTime,
req.MaxAmountOut,
callerAddr,
receiverAddr,
[]byte("caller"),
[]byte("receiver"),
)
if err != nil {
return nil, err
}

// NB: We're only using a cache context so we don't expect any writes to happen.
//nolint:staticcheck // Should be unnecessary but out of an abundance of caution we restore the old bankkeeper
k.bankKeeper = oldBK

return &types.QueryEstimatePlaceLimitOrderResponse{
TotalInCoin: totalInCoin,
Expand Down
8 changes: 2 additions & 6 deletions x/dex/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ func (s *DexTestSuite) multiHopSwaps(
s.Assert().Nil(err)
}

func (s *DexTestSuite) aliceEstimatesMultiHopSwap(
func (s *DexTestSuite) estimatesMultiHopSwap(
routes [][]string,
amountIn int,
exitLimitPrice math_utils.PrecDec,
Expand All @@ -929,8 +929,6 @@ func (s *DexTestSuite) aliceEstimatesMultiHopSwap(
multiHopRoutes[i] = &types.MultiHopRoute{Hops: hops}
}
msg := &types.QueryEstimateMultiHopSwapRequest{
Creator: s.alice.String(),
Receiver: s.alice.String(),
Routes: multiHopRoutes,
AmountIn: sdkmath.NewInt(int64(amountIn)).Mul(denomMultiple),
ExitLimitPrice: exitLimitPrice,
Expand All @@ -941,7 +939,7 @@ func (s *DexTestSuite) aliceEstimatesMultiHopSwap(
return res.CoinOut
}

func (s *DexTestSuite) aliceEstimatesMultiHopSwapFails(
func (s *DexTestSuite) estimatesMultiHopSwapFails(
expectedErr error,
routes [][]string,
amountIn int,
Expand All @@ -953,8 +951,6 @@ func (s *DexTestSuite) aliceEstimatesMultiHopSwapFails(
multiHopRoutes[i] = &types.MultiHopRoute{Hops: hops}
}
msg := &types.QueryEstimateMultiHopSwapRequest{
Creator: s.alice.String(),
Receiver: s.alice.String(),
Routes: multiHopRoutes,
AmountIn: sdkmath.NewInt(int64(amountIn)).Mul(denomMultiple),
ExitLimitPrice: exitLimitPrice,
Expand Down
41 changes: 41 additions & 0 deletions x/dex/keeper/simulation_bank_keeper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package keeper

import (
"context"

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

"github.com/neutron-org/neutron/v4/x/dex/types"
)

type SimulationBankKeeper struct {
originalBankKeeper types.BankKeeper
}

func (s SimulationBankKeeper) SendCoinsFromAccountToModule(_ context.Context, _ sdk.AccAddress, _ string, _ sdk.Coins) error {
return nil
}

func (s SimulationBankKeeper) SendCoinsFromModuleToAccount(_ context.Context, _ string, _ sdk.AccAddress, _ sdk.Coins) error {
return nil
}

func (s SimulationBankKeeper) MintCoins(_ context.Context, _ string, _ sdk.Coins) error {
return nil
}

func (s SimulationBankKeeper) BurnCoins(_ context.Context, _ string, _ sdk.Coins) error {
return nil
}

func (s SimulationBankKeeper) IterateAccountBalances(ctx context.Context, addr sdk.AccAddress, cb func(sdk.Coin) bool) {
s.originalBankKeeper.IterateAccountBalances(ctx, addr, cb)
}

func (s SimulationBankKeeper) GetSupply(ctx context.Context, denom string) sdk.Coin {
return s.originalBankKeeper.GetSupply(ctx, denom)
}

func NewSimulationBankKeeper(bk types.BankKeeper) types.BankKeeper {
return SimulationBankKeeper{originalBankKeeper: bk}
}
Loading

0 comments on commit faddb43

Please sign in to comment.