Skip to content

Commit

Permalink
chore: clean unnecessary member variables
Browse files Browse the repository at this point in the history
  • Loading branch information
@jaeseung-bae
jaeseung-bae committed Apr 30, 2024
1 parent be5f723 commit e2ac84c
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 101 deletions.
74 changes: 19 additions & 55 deletions x/fswap/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(),
}
}

Expand Down Expand Up @@ -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

Check warning on line 66 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L66 

Added line #L66 was not covered by tests
}
Expand Down Expand Up @@ -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

Check warning on line 163 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L163 

Added line #L163 was not covered by tests
}
Expand Down Expand Up @@ -225,15 +214,15 @@ 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

Check warning on line 219 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L216-L219 

Added lines #L216 - L219 were not covered by tests
}
swapped, err := k.getSwapped(ctx)
if err != nil {
return sdk.Coin{}, err

Check warning on line 223 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L221-L223 

Added lines #L221 - L223 were not covered by tests
}
denom, err := k.getToDenom(ctx)
denom, err := k.toDenom(ctx)
if err != nil {
return sdk.Coin{}, err

Check warning on line 227 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L225-L227 

Added lines #L225 - L227 were not covered by tests
}
Expand All @@ -243,74 +232,49 @@ func (k Keeper) getSwappableNewCoinAmount(ctx sdk.Context) (sdk.Coin, error) {
return sdk.NewCoin(denom, remainingAmount), nil

Check warning on line 232 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L232 

Added line #L232 was not covered by tests
}

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

Check failure on line 235 in x/fswap/keeper/keeper.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

func `Keeper.fromDenom` is unused (unused)
swapInit, err := k.swapInit(ctx)
if err != nil {
return "", err

Check warning on line 238 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L235-L238 

Added lines #L235 - L238 were not covered by tests
}

k.fromDenom = swapInit.GetFromDenom()
return k.fromDenom, nil
return swapInit.FromDenom, nil

Check warning on line 241 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L241 

Added line #L241 was not covered by tests
}

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

Check warning on line 247 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L247 

Added line #L247 was not covered by tests
}

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

Check failure on line 253 in x/fswap/keeper/keeper.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

func `Keeper.swapMultiple` is unused (unused)
swapInit, err := k.swapInit(ctx)
if err != nil {
return sdk.Int{}, err

Check warning on line 256 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L253-L256 

Added lines #L253 - L256 were not covered by tests
}

k.swapMultiple = swapInit.SwapMultiple
return k.swapMultiple, nil
return swapInit.SwapMultiple, nil

Check warning on line 259 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L259 

Added line #L259 was not covered by tests
}

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

Check warning on line 265 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L265 

Added line #L265 was not covered by tests
}

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

Check warning on line 274 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L274 

Added line #L274 was not covered by tests
}

k.swapInit = swapInits[0]
return k.swapInit, nil
return swapInits[0], nil
}

func (k Keeper) updateSwapped(ctx sdk.Context, fromAmount, toAmount sdk.Coin) error {
Expand Down Expand Up @@ -340,7 +304,7 @@ func (k Keeper) checkSwapCap(ctx sdk.Context, newCoinAmount sdk.Coin) error {
return err

Check warning on line 304 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L304 

Added line #L304 was not covered by tests
}

swapCap, err := k.getSwapCap(ctx)
swapCap, err := k.swapCap(ctx)
if err != nil {
return err

Check warning on line 309 in x/fswap/keeper/keeper.go

View check run for this annotation

Codecov / codecov/patch

x/fswap/keeper/keeper.go#L309 

Added line #L309 was not covered by tests
}
Expand Down
60 changes: 26 additions & 34 deletions x/fswap/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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))
}
Expand All @@ -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,
Expand All @@ -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)
})
}
Expand All @@ -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,
Expand All @@ -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)
})
}
Expand Down
24 changes: 12 additions & 12 deletions x/fswap/keeper/msg_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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)
})
}
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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)
})
}
Expand Down

0 comments on commit e2ac84c

Please sign in to comment.