From e2ac84c2c7bacec53fc989c3a027d354093f5fef Mon Sep 17 00:00:00 2001 From: "jaeseung.bae" Date: Tue, 30 Apr 2024 21:44:50 +0900 Subject: [PATCH] chore: clean unnecessary member variables --- x/fswap/keeper/keeper.go | 74 ++++++++----------------------- x/fswap/keeper/keeper_test.go | 60 +++++++++++-------------- x/fswap/keeper/msg_server_test.go | 24 +++++----- 3 files changed, 57 insertions(+), 101 deletions(-) diff --git a/x/fswap/keeper/keeper.go b/x/fswap/keeper/keeper.go index 450a43ce21..7706d5780c 100644 --- a/x/fswap/keeper/keeper.go +++ b/x/fswap/keeper/keeper.go @@ -19,12 +19,6 @@ type Keeper struct { AccountKeeper BankKeeper - - fromDenom string - toDenom string - swapInit types.SwapInit - swapMultiple sdk.Int - swapCap sdk.Int } func NewKeeper(cdc codec.BinaryCodec, storeKey storetypes.StoreKey, ak AccountKeeper, bk BankKeeper) Keeper { @@ -33,11 +27,6 @@ func NewKeeper(cdc codec.BinaryCodec, storeKey storetypes.StoreKey, ak AccountKe storeKey: storeKey, AccountKeeper: ak, BankKeeper: bk, - fromDenom: "", - toDenom: "", - swapInit: types.SwapInit{}, - swapMultiple: sdk.ZeroInt(), - swapCap: sdk.ZeroInt(), } } @@ -72,7 +61,7 @@ func (k Keeper) SwapInit(ctx sdk.Context, swapInit types.SwapInit) error { } func (k Keeper) Swap(ctx sdk.Context, addr sdk.AccAddress, fromCoinAmount sdk.Coin, toDenom string) error { - swapInit, err := k.getSwapInit(ctx) + swapInit, err := k.swapInit(ctx) if err != nil { return err } @@ -169,7 +158,7 @@ func (k Keeper) iterateAllSwapped(ctx sdk.Context, cb func(swapped types.Swapped } func (k Keeper) getSwapped(ctx sdk.Context) (types.Swapped, error) { - toDenom, err := k.getToDenom(ctx) + toDenom, err := k.toDenom(ctx) if err != nil { return types.Swapped{}, err } @@ -225,7 +214,7 @@ func (k Keeper) iterateAllSwapInits(ctx sdk.Context, cb func(swapped types.SwapI } func (k Keeper) getSwappableNewCoinAmount(ctx sdk.Context) (sdk.Coin, error) { - swapCap, err := k.getSwapCap(ctx) + swapCap, err := k.swapCap(ctx) if err != nil { return sdk.Coin{}, err } @@ -233,7 +222,7 @@ func (k Keeper) getSwappableNewCoinAmount(ctx sdk.Context) (sdk.Coin, error) { if err != nil { return sdk.Coin{}, err } - denom, err := k.getToDenom(ctx) + denom, err := k.toDenom(ctx) if err != nil { return sdk.Coin{}, err } @@ -243,74 +232,49 @@ func (k Keeper) getSwappableNewCoinAmount(ctx sdk.Context) (sdk.Coin, error) { return sdk.NewCoin(denom, remainingAmount), nil } -func (k Keeper) getFromDenom(ctx sdk.Context) (string, error) { - if len(k.fromDenom) > 0 { - return k.fromDenom, nil - } - - swapInit, err := k.getSwapInit(ctx) +func (k Keeper) fromDenom(ctx sdk.Context) (string, error) { + swapInit, err := k.swapInit(ctx) if err != nil { return "", err } - k.fromDenom = swapInit.GetFromDenom() - return k.fromDenom, nil + return swapInit.FromDenom, nil } -func (k Keeper) getToDenom(ctx sdk.Context) (string, error) { - if len(k.toDenom) > 0 { - return k.toDenom, nil - } - - swapInit, err := k.getSwapInit(ctx) +func (k Keeper) toDenom(ctx sdk.Context) (string, error) { + swapInit, err := k.swapInit(ctx) if err != nil { return "", err } - k.toDenom = swapInit.GetToDenom() - return k.toDenom, nil + return swapInit.GetToDenom(), nil } -func (k Keeper) getSwapMultiple(ctx sdk.Context) (sdk.Int, error) { - if k.swapMultiple.IsPositive() { - return k.swapMultiple, nil - } - - swapInit, err := k.getSwapInit(ctx) +func (k Keeper) swapMultiple(ctx sdk.Context) (sdk.Int, error) { + swapInit, err := k.swapInit(ctx) if err != nil { return sdk.Int{}, err } - k.swapMultiple = swapInit.SwapMultiple - return k.swapMultiple, nil + return swapInit.SwapMultiple, nil } -func (k Keeper) getSwapCap(ctx sdk.Context) (sdk.Int, error) { - if k.swapCap.IsPositive() { - return k.swapCap, nil - } - - swapInit, err := k.getSwapInit(ctx) +func (k Keeper) swapCap(ctx sdk.Context) (sdk.Int, error) { + swapInit, err := k.swapInit(ctx) if err != nil { return sdk.Int{}, err } - k.swapCap = swapInit.AmountCapForToDenom - return k.swapCap, nil + return swapInit.AmountCapForToDenom, nil } -func (k Keeper) getSwapInit(ctx sdk.Context) (types.SwapInit, error) { - if !k.swapInit.IsEmpty() { - return k.swapInit, nil - } - +func (k Keeper) swapInit(ctx sdk.Context) (types.SwapInit, error) { swapInits := k.getAllSwapInits(ctx) if len(swapInits) == 0 { return types.SwapInit{}, types.ErrSwapNotInitialized } - k.swapInit = swapInits[0] - return k.swapInit, nil + return swapInits[0], nil } func (k Keeper) updateSwapped(ctx sdk.Context, fromAmount, toAmount sdk.Coin) error { @@ -340,7 +304,7 @@ func (k Keeper) checkSwapCap(ctx sdk.Context, newCoinAmount sdk.Coin) error { return err } - swapCap, err := k.getSwapCap(ctx) + swapCap, err := k.swapCap(ctx) if err != nil { return err } diff --git a/x/fswap/keeper/keeper_test.go b/x/fswap/keeper/keeper_test.go index a49b42317c..9880b13e17 100644 --- a/x/fswap/keeper/keeper_test.go +++ b/x/fswap/keeper/keeper_test.go @@ -30,11 +30,7 @@ type KeeperTestSuite struct { accWithToCoin sdk.AccAddress initBalance sdk.Int - swapInit types.SwapInit - fromDenom string - toDenom string - swapMultiple sdk.Int - swapCap sdk.Int + swapInit types.SwapInit } func (s *KeeperTestSuite) createRandomAccounts(n int) []sdk.AccAddress { @@ -66,17 +62,13 @@ func (s *KeeperTestSuite) SetupTest() { s.queryServer = keeper.NewQueryServer(s.keeper) s.msgServer = keeper.NewMsgServer(s.keeper) - s.fromDenom = "fromdenom" - s.toDenom = "todenom" - s.swapMultiple = sdk.NewInt(1000) - s.initBalance = sdk.NewInt(123456789) numAcc := int64(2) - s.swapCap = s.initBalance.Mul(s.swapMultiple.Mul(sdk.NewInt(numAcc))) + s.initBalance = sdk.NewInt(123456789) s.swapInit = types.SwapInit{ - FromDenom: s.fromDenom, - ToDenom: s.toDenom, - AmountCapForToDenom: s.swapCap, - SwapMultiple: s.swapMultiple, + FromDenom: "fromdenom", + ToDenom: "todenom", + AmountCapForToDenom: s.initBalance.Mul(sdk.NewInt(1000).Mul(sdk.NewInt(numAcc))), + SwapMultiple: sdk.NewInt(1000), } s.createAccountsWithInitBalance(app) app.AccountKeeper.GetModuleAccount(s.ctx, types.ModuleName) @@ -91,11 +83,11 @@ func (s *KeeperTestSuite) createAccountsWithInitBalance(app *simapp.SimApp) { *addresses[i] = address } minter := app.AccountKeeper.GetModuleAccount(s.ctx, minttypes.ModuleName).GetAddress() - fromAmount := sdk.NewCoins(sdk.NewCoin(s.fromDenom, s.initBalance)) + fromAmount := sdk.NewCoins(sdk.NewCoin(s.swapInit.GetFromDenom(), s.initBalance)) s.Require().NoError(app.BankKeeper.MintCoins(s.ctx, minttypes.ModuleName, fromAmount)) s.Require().NoError(app.BankKeeper.SendCoins(s.ctx, minter, s.accWithFromCoin, fromAmount)) - toAmount := sdk.NewCoins(sdk.NewCoin(s.toDenom, s.initBalance)) + toAmount := sdk.NewCoins(sdk.NewCoin(s.swapInit.GetToDenom(), s.initBalance)) s.Require().NoError(app.BankKeeper.MintCoins(s.ctx, minttypes.ModuleName, toAmount)) s.Require().NoError(app.BankKeeper.SendCoins(s.ctx, minter, s.accWithToCoin, toAmount)) } @@ -115,40 +107,40 @@ func (s *KeeperTestSuite) TestSwap() { }{ "swap some": { s.accWithFromCoin, - sdk.NewCoin(s.fromDenom, sdk.NewInt(100)), - s.toDenom, + sdk.NewCoin(s.swapInit.GetFromDenom(), sdk.NewInt(100)), + s.swapInit.GetToDenom(), sdk.NewInt(100), false, nil, }, "swap all the balance": { s.accWithFromCoin, - sdk.NewCoin(s.fromDenom, s.initBalance), - s.toDenom, + sdk.NewCoin(s.swapInit.GetFromDenom(), s.initBalance), + s.swapInit.GetToDenom(), s.initBalance, false, nil, }, "swap without holding enough balance": { s.accWithFromCoin, - sdk.NewCoin(s.fromDenom, sdk.OneInt().Add(s.initBalance)), - s.toDenom, + sdk.NewCoin(s.swapInit.GetFromDenom(), sdk.OneInt().Add(s.initBalance)), + s.swapInit.GetToDenom(), sdk.ZeroInt(), true, sdkerrors.ErrInsufficientFunds, }, "account holding new coin only": { s.accWithToCoin, - sdk.NewCoin(s.fromDenom, sdk.NewInt(100)), - s.toDenom, + sdk.NewCoin(s.swapInit.GetFromDenom(), sdk.NewInt(100)), + s.swapInit.GetToDenom(), sdk.ZeroInt(), true, sdkerrors.ErrInsufficientFunds, }, "swap with the same from-denom and to-denom": { s.accWithFromCoin, - sdk.NewCoin(s.toDenom, s.initBalance), - s.toDenom, + sdk.NewCoin(s.swapInit.GetToDenom(), s.initBalance), + s.swapInit.GetToDenom(), sdk.ZeroInt(), true, sdkerrors.ErrInvalidRequest, @@ -167,8 +159,8 @@ func (s *KeeperTestSuite) TestSwap() { } s.Require().NoError(err) - actualAmount := s.keeper.GetBalance(ctx, tc.from, s.toDenom).Amount - expectedAmount := tc.expectedBalanceWithoutMultiply.Mul(s.swapMultiple) + actualAmount := s.keeper.GetBalance(ctx, tc.from, s.swapInit.GetToDenom()).Amount + expectedAmount := tc.expectedBalanceWithoutMultiply.Mul(s.swapInit.SwapMultiple) s.Require().Equal(expectedAmount, actualAmount) }) } @@ -185,16 +177,16 @@ func (s *KeeperTestSuite) TestSwapAll() { }{ "account holding from coin": { s.accWithFromCoin, - s.fromDenom, - s.toDenom, + s.swapInit.GetFromDenom(), + s.swapInit.GetToDenom(), s.initBalance, false, nil, }, "account holding to coin only": { s.accWithToCoin, - s.fromDenom, - s.toDenom, + s.swapInit.GetFromDenom(), + s.swapInit.GetToDenom(), s.initBalance, true, sdkerrors.ErrInsufficientFunds, @@ -213,8 +205,8 @@ func (s *KeeperTestSuite) TestSwapAll() { } s.Require().NoError(err) - actualAmount := s.keeper.GetBalance(ctx, tc.from, s.toDenom).Amount - expectedAmount := tc.expectedBalanceWithoutMultiply.Mul(s.swapMultiple) + actualAmount := s.keeper.GetBalance(ctx, tc.from, s.swapInit.GetToDenom()).Amount + expectedAmount := tc.expectedBalanceWithoutMultiply.Mul(s.swapInit.SwapMultiple) s.Require().Equal(expectedAmount, actualAmount) }) } diff --git a/x/fswap/keeper/msg_server_test.go b/x/fswap/keeper/msg_server_test.go index 8dde4f9b4b..dc0e4093f9 100644 --- a/x/fswap/keeper/msg_server_test.go +++ b/x/fswap/keeper/msg_server_test.go @@ -16,8 +16,8 @@ func (s *KeeperTestSuite) TestMsgSwap() { "swap some": { &types.MsgSwap{ FromAddress: s.accWithFromCoin.String(), - FromCoinAmount: sdk.NewCoin(s.fromDenom, sdk.NewInt(100)), - ToDenom: s.toDenom, + FromCoinAmount: sdk.NewCoin(s.swapInit.GetFromDenom(), sdk.NewInt(100)), + ToDenom: s.swapInit.GetToDenom(), }, sdk.NewInt(100), false, @@ -26,8 +26,8 @@ func (s *KeeperTestSuite) TestMsgSwap() { "swap all the balance": { &types.MsgSwap{ FromAddress: s.accWithFromCoin.String(), - FromCoinAmount: sdk.NewCoin(s.fromDenom, s.initBalance), - ToDenom: s.toDenom, + FromCoinAmount: sdk.NewCoin(s.swapInit.GetFromDenom(), s.initBalance), + ToDenom: s.swapInit.GetToDenom(), }, s.initBalance, false, @@ -36,8 +36,8 @@ func (s *KeeperTestSuite) TestMsgSwap() { "account holding new coin only": { &types.MsgSwap{ FromAddress: s.accWithToCoin.String(), - FromCoinAmount: sdk.NewCoin(s.fromDenom, sdk.NewInt(100)), - ToDenom: s.toDenom, + FromCoinAmount: sdk.NewCoin(s.swapInit.GetFromDenom(), sdk.NewInt(100)), + ToDenom: s.swapInit.GetToDenom(), }, s.initBalance, true, @@ -61,7 +61,7 @@ func (s *KeeperTestSuite) TestMsgSwap() { from, err := sdk.AccAddressFromBech32(tc.request.FromAddress) s.Require().NoError(err) actualAmount := s.keeper.GetBalance(ctx, from, tc.request.GetToDenom()).Amount - expectedAmount := tc.expectedBalanceWithoutMultiply.Mul(s.swapMultiple) + expectedAmount := tc.expectedBalanceWithoutMultiply.Mul(s.swapInit.SwapMultiple) s.Require().Equal(expectedAmount, actualAmount) }) } @@ -77,8 +77,8 @@ func (s *KeeperTestSuite) TestMsgSwapAll() { "swapAll": { &types.MsgSwapAll{ FromAddress: s.accWithFromCoin.String(), - FromDenom: s.fromDenom, - ToDenom: s.toDenom, + FromDenom: s.swapInit.GetFromDenom(), + ToDenom: s.swapInit.GetToDenom(), }, s.initBalance, false, @@ -87,8 +87,8 @@ func (s *KeeperTestSuite) TestMsgSwapAll() { "account holding new coin only": { &types.MsgSwapAll{ FromAddress: s.accWithToCoin.String(), - FromDenom: s.fromDenom, - ToDenom: s.toDenom, + FromDenom: s.swapInit.GetFromDenom(), + ToDenom: s.swapInit.GetToDenom(), }, s.initBalance, true, @@ -112,7 +112,7 @@ func (s *KeeperTestSuite) TestMsgSwapAll() { from, err := sdk.AccAddressFromBech32(tc.request.FromAddress) s.Require().NoError(err) actualAmount := s.keeper.GetBalance(ctx, from, tc.request.GetToDenom()).Amount - expectedAmount := tc.expectedBalanceWithoutMultiply.Mul(s.swapMultiple) + expectedAmount := tc.expectedBalanceWithoutMultiply.Mul(s.swapInit.SwapMultiple) s.Require().Equal(expectedAmount, actualAmount) }) }