From 3b063453c0683a52475374e3e278ab5dc2ab501c Mon Sep 17 00:00:00 2001 From: jelysn Date: Fri, 1 Sep 2023 11:10:12 +0800 Subject: [PATCH] Resolve comments on swap txs sorting mechanism PR --- x/amm/keeper/abci.go | 66 +++----------------------------- x/amm/keeper/abci_test.go | 4 +- x/amm/keeper/batch_processing.go | 6 +-- x/amm/types/errors.go | 3 +- x/amm/types/key_batch_txs.go | 16 +++----- 5 files changed, 16 insertions(+), 79 deletions(-) diff --git a/x/amm/keeper/abci.go b/x/amm/keeper/abci.go index c89b46b91..48b49f084 100644 --- a/x/amm/keeper/abci.go +++ b/x/amm/keeper/abci.go @@ -1,7 +1,6 @@ package keeper import ( - "fmt" "strings" "time" @@ -22,9 +21,8 @@ func (k Keeper) GetStackedSlippage(ctx sdk.Context, poolId uint64) sdk.Dec { } func (k Keeper) ApplySwapRequest(ctx sdk.Context, msg sdk.Msg) error { - switch msg.(type) { + switch msg := msg.(type) { case *types.MsgSwapExactAmountIn: - msg := msg.(*types.MsgSwapExactAmountIn) sender, err := sdk.AccAddressFromBech32(msg.Sender) if err != nil { return err @@ -35,7 +33,6 @@ func (k Keeper) ApplySwapRequest(ctx sdk.Context, msg sdk.Msg) error { } return nil case *types.MsgSwapExactAmountOut: - msg := msg.(*types.MsgSwapExactAmountOut) sender, err := sdk.AccAddressFromBech32(msg.Sender) if err != nil { return err @@ -46,17 +43,15 @@ func (k Keeper) ApplySwapRequest(ctx sdk.Context, msg sdk.Msg) error { } return nil default: - return fmt.Errorf("unexpected swap message") + return types.ErrInvalidSwapMsgType } } func (k Keeper) DeleteSwapRequest(ctx sdk.Context, msg sdk.Msg, index uint64) { - switch msg.(type) { + switch msg := msg.(type) { case *types.MsgSwapExactAmountIn: - msg := msg.(*types.MsgSwapExactAmountIn) k.DeleteSwapExactAmountInRequest(ctx, msg, index) case *types.MsgSwapExactAmountOut: - msg := msg.(*types.MsgSwapExactAmountOut) k.DeleteSwapExactAmountOutRequest(ctx, msg, index) } } @@ -72,12 +67,10 @@ func (k Keeper) SelectOneSwapRequest(ctx sdk.Context, sprefix []byte) (sdk.Msg, func (k Keeper) SelectReverseSwapRequest(ctx sdk.Context, msg sdk.Msg) (sdk.Msg, uint64) { sprefix := []byte{} - switch msg.(type) { + switch msg := msg.(type) { case *types.MsgSwapExactAmountIn: - msg := msg.(*types.MsgSwapExactAmountIn) sprefix = types.TKeyPrefixSwapExactAmountInPrefix(msg) case *types.MsgSwapExactAmountOut: - msg := msg.(*types.MsgSwapExactAmountOut) sprefix = types.TKeyPrefixSwapExactAmountOutPrefix(msg) } @@ -90,12 +83,10 @@ func (k Keeper) SelectReverseSwapRequest(ctx sdk.Context, msg sdk.Msg) (sdk.Msg, } func (k Keeper) FirstPoolId(msg sdk.Msg) uint64 { - switch msg.(type) { + switch msg := msg.(type) { case *types.MsgSwapExactAmountIn: - msg := msg.(*types.MsgSwapExactAmountIn) return types.FirstPoolIdFromSwapExactAmountIn(msg) case *types.MsgSwapExactAmountOut: - msg := msg.(*types.MsgSwapExactAmountOut) return types.FirstPoolIdFromSwapExactAmountOut(msg) } return 0 @@ -176,51 +167,4 @@ func (k Keeper) EndBlocker(ctx sdk.Context) { defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) k.ExecuteSwapRequests(ctx) - // swapInRequests := k.GetAllSwapExactAmountInRequests(ctx) - // for _, msg := range swapInRequests { - // sender, err := sdk.AccAddressFromBech32(msg.Sender) - // if err != nil { - // continue - // } - - // cacheCtx, write := ctx.CacheContext() - // _, err = k.RouteExactAmountIn(cacheCtx, sender, msg.Routes, msg.TokenIn, math.Int(msg.TokenOutMinAmount)) - // if err != nil { - // continue - // } - // write() - - // // Swap event is handled elsewhere - // ctx.EventManager().EmitEvents(sdk.Events{ - // sdk.NewEvent( - // sdk.EventTypeMessage, - // sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory), - // sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender), - // ), - // }) - - // } - // swapOutRequests := k.GetAllSwapExactAmountOutRequests(ctx) - // for _, msg := range swapOutRequests { - // sender, err := sdk.AccAddressFromBech32(msg.Sender) - // if err != nil { - // continue - // } - - // cacheCtx, write := ctx.CacheContext() - // _, err = k.RouteExactAmountOut(cacheCtx, sender, msg.Routes, msg.TokenInMaxAmount, msg.TokenOut) - // if err != nil { - // continue - // } - // write() - - // // Swap event is handled elsewhere - // ctx.EventManager().EmitEvents(sdk.Events{ - // sdk.NewEvent( - // sdk.EventTypeMessage, - // sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory), - // sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender), - // ), - // }) - // } } diff --git a/x/amm/keeper/abci_test.go b/x/amm/keeper/abci_test.go index 777613b73..6f5b978fc 100644 --- a/x/amm/keeper/abci_test.go +++ b/x/amm/keeper/abci_test.go @@ -276,16 +276,14 @@ func (suite *KeeperTestSuite) TestExecuteSwapRequests() { msgServer := keeper.NewMsgServerImpl(suite.app.AmmKeeper) for _, msg := range tc.swapMsgs { - switch msg.(type) { + switch msg := msg.(type) { case *types.MsgSwapExactAmountIn: - msg := msg.(*types.MsgSwapExactAmountIn) _, err := msgServer.SwapExactAmountIn( sdk.WrapSDKContext(suite.ctx), msg, ) suite.Require().NoError(err) case *types.MsgSwapExactAmountOut: - msg := msg.(*types.MsgSwapExactAmountOut) _, err := msgServer.SwapExactAmountOut( sdk.WrapSDKContext(suite.ctx), msg, diff --git a/x/amm/keeper/batch_processing.go b/x/amm/keeper/batch_processing.go index efd32a263..7ce70cdeb 100644 --- a/x/amm/keeper/batch_processing.go +++ b/x/amm/keeper/batch_processing.go @@ -21,11 +21,10 @@ func (k Keeper) GetLastSwapRequestIndex(ctx sdk.Context) uint64 { } // SetSwapExactAmountInRequests stores swap exact amount in request -func (k Keeper) SetSwapExactAmountInRequests(ctx sdk.Context, msg *types.MsgSwapExactAmountIn, index uint64) error { +func (k Keeper) SetSwapExactAmountInRequests(ctx sdk.Context, msg *types.MsgSwapExactAmountIn, index uint64) { store := prefix.NewStore(ctx.TransientStore(k.transientStoreKey), types.KeyPrefix(types.TSwapExactAmountInKey)) b := k.cdc.MustMarshal(msg) store.Set(types.TKeyPrefixSwapExactAmountIn(msg, index), b) - return nil } // DeleteSwapExactAmountInRequest removes a swap exact amount in request @@ -65,11 +64,10 @@ func (k Keeper) GetFirstSwapExactAmountInRequest(ctx sdk.Context, sprefix []byte } // SetSwapExactAmountInRequests stores swap exact amount out request -func (k Keeper) SetSwapExactAmountOutRequests(ctx sdk.Context, msg *types.MsgSwapExactAmountOut, index uint64) error { +func (k Keeper) SetSwapExactAmountOutRequests(ctx sdk.Context, msg *types.MsgSwapExactAmountOut, index uint64) { store := prefix.NewStore(ctx.TransientStore(k.transientStoreKey), types.KeyPrefix(types.TSwapExactAmountOutKey)) b := k.cdc.MustMarshal(msg) store.Set(types.TKeyPrefixSwapExactAmountOut(msg, index), b) - return nil } // DeleteSwapExactAmountOutRequest deletes a swap exact amount out request diff --git a/x/amm/types/errors.go b/x/amm/types/errors.go index 3e7b9b4ab..042cc833d 100644 --- a/x/amm/types/errors.go +++ b/x/amm/types/errors.go @@ -22,7 +22,8 @@ var ( ErrTooManyTokensOut = sdkerrors.Register(ModuleName, 31, "tx is trying to get more tokens out of the pool than exist") - ErrInvalidPoolId = sdkerrors.Register(ModuleName, 91, "invalid pool id") + ErrInvalidPoolId = sdkerrors.Register(ModuleName, 91, "invalid pool id") + ErrInvalidSwapMsgType = sdkerrors.Register(ModuleName, 92, "unexpected swap message type") ) const ( diff --git a/x/amm/types/key_batch_txs.go b/x/amm/types/key_batch_txs.go index e34d0c9db..0777d0a53 100644 --- a/x/amm/types/key_batch_txs.go +++ b/x/amm/types/key_batch_txs.go @@ -1,15 +1,12 @@ package types import ( - "encoding/binary" fmt "fmt" "strings" sdk "github.com/cosmos/cosmos-sdk/types" ) -var _ binary.ByteOrder - const ( TLastSwapRequestIndex = "last-swap-request-index" TSwapExactAmountInKey = "batch/swap-exact-amount-in" @@ -17,7 +14,7 @@ const ( ) func TKeyPrefixSwapExactAmountInPrefix(m *MsgSwapExactAmountIn) []byte { - prefix := []byte(m.TokenIn.Denom + "/") + prefix := []byte(fmt.Sprintf("%s/", m.TokenIn.Denom)) routeKeys := []string{} for _, route := range m.Routes[:1] { routeKeys = append(routeKeys, fmt.Sprintf("%d/%s", route.PoolId, route.TokenOutDenom)) @@ -27,8 +24,8 @@ func TKeyPrefixSwapExactAmountInPrefix(m *MsgSwapExactAmountIn) []byte { } func FirstPoolIdFromSwapExactAmountIn(m *MsgSwapExactAmountIn) uint64 { - for _, route := range m.Routes { - return route.PoolId + if len(m.Routes) > 0 { + return m.Routes[0].PoolId } return 0 } @@ -39,12 +36,11 @@ func TKeyPrefixSwapExactAmountIn(m *MsgSwapExactAmountIn, index uint64) []byte { } func TKeyPrefixSwapExactAmountOutPrefix(m *MsgSwapExactAmountOut) []byte { - prefix := []byte("/" + m.TokenOut.Denom) + prefix := []byte(fmt.Sprintf("/%s", m.TokenOut.Denom)) routeKeys := []string{} - for i := len(m.Routes) - 1; i >= 0; i-- { - route := m.Routes[i] + if len(m.Routes) > 0 { + route := m.Routes[len(m.Routes)-1] routeKeys = append(routeKeys, fmt.Sprintf("%s/%d", route.TokenInDenom, route.PoolId)) - break } prefix = append([]byte(strings.Join(routeKeys, "/")), prefix...) return prefix