Minor code improvements (#119)

* refactor: minor code improvements

* refactor: namming and remove unused code

---------

Co-authored-by: Duong Minh Ngoc <[email protected]>

proto/feeabstraction/absfee/v1beta1/query.proto

@@ -25,21 +25,21 @@ service Query {
   }
 
   rpc HostChainConfig(QueryHostChainConfigRequest)
-      returns (QueryHostChainConfigRespone) {
+      returns (QueryHostChainConfigResponse) {
     option (google.api.http).get =
         "/fee-abstraction/feeabs/v1/host-chain-config/{ibc_denom}";
   }
 
   rpc AllHostChainConfig(AllQueryHostChainConfigRequest)
-      returns (AllQueryHostChainConfigRespone) {
+      returns (AllQueryHostChainConfigResponse) {
     option (google.api.http).get =
         "/fee-abstraction/feeabs/v1/all-host-chain-config";
   }
 }
 
 message QueryHostChainConfigRequest { string ibc_denom = 1; }
 
-message QueryHostChainConfigRespone {
+message QueryHostChainConfigResponse {
   HostChainFeeAbsConfig host_chain_config = 1 [
     (gogoproto.moretags) = "yaml:\"host_chain_config\"",
     (gogoproto.nullable) = false
@@ -73,7 +73,7 @@ message QueryFeeabsModuleBalacesResponse {
 
 message AllQueryHostChainConfigRequest {}
 
-message AllQueryHostChainConfigRespone {
+message AllQueryHostChainConfigResponse {
   repeated HostChainFeeAbsConfig all_host_chain_config = 1 [
     (gogoproto.moretags) = "yaml:\"all_host_chain_config\"",
     (gogoproto.nullable) = false

tests/interchaintest/packet_foward_test.go

@@ -550,17 +550,17 @@ func TestPacketForwardMiddleware(t *testing.T) {
 	})
 }
 
-func QueryFeeabsHostZoneConfig(c *cosmos.CosmosChain, ctx context.Context) (*QueryHostChainConfigRespone, error) {
+func QueryFeeabsHostZoneConfig(c *cosmos.CosmosChain, ctx context.Context) (*QueryHostChainConfigResponse, error) {
 	cmd := []string{"feeabs", "all-host-chain-config"}
 	stdout, _, err := c.ExecQuery(ctx, cmd)
 	if err != nil {
-		return &QueryHostChainConfigRespone{}, err
+		return &QueryHostChainConfigResponse{}, err
 	}
 
-	var hostZoneConfig QueryHostChainConfigRespone
+	var hostZoneConfig QueryHostChainConfigResponse
 	err = json.Unmarshal(stdout, &hostZoneConfig)
 	if err != nil {
-		return &QueryHostChainConfigRespone{}, err
+		return &QueryHostChainConfigResponse{}, err
 	}
 
 	return &hostZoneConfig, nil

tests/interchaintest/setup.go

@@ -23,7 +23,7 @@ type QueryFeeabsModuleBalacesResponse struct {
 	Address  string
 }
 
-type QueryHostChainConfigRespone struct {
+type QueryHostChainConfigResponse struct {
 	HostChainConfig cosmos.HostChainFeeAbsConfig `protobuf:"bytes,1,opt,name=host_chain_config,json=hostChainConfig,proto3" json:"host_chain_config" yaml:"host_chain_config"`
 }
 

x/feeabs/ante/decorate.go

@@ -83,15 +83,15 @@ func (fadfd FeeAbstractionDeductFeeDecorate) normalDeductFeeAnteHandle(ctx sdk.C
 	}
 
 	// deduct the fees
-	if !feeTx.GetFee().IsZero() {
-		err = DeductFees(fadfd.bankKeeper, ctx, deductFeesFrom, feeTx.GetFee())
+	if !fee.IsZero() {
+		err = DeductFees(fadfd.bankKeeper, ctx, deductFeesFrom, fee)
 		if err != nil {
 			return ctx, err
 		}
 	}
 
 	events := sdk.Events{sdk.NewEvent(sdk.EventTypeTx,
-		sdk.NewAttribute(sdk.AttributeKeyFee, feeTx.GetFee().String()),
+		sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()),
 	)}
 	ctx.EventManager().EmitEvents(events)
 
@@ -126,19 +126,18 @@ func (fadfd FeeAbstractionDeductFeeDecorate) abstractionDeductFeeHandler(ctx sdk
 	}
 
 	// calculate the native token can be swapped from ibc token
-	ibcFees := feeTx.GetFee()
-	if len(ibcFees) != 1 {
-		return ctx, sdkerrors.Wrapf(errorstypes.ErrInvalidCoins, "invalid ibc token: %s", ibcFees)
+	if len(fee) != 1 {
+		return ctx, sdkerrors.Wrapf(errorstypes.ErrInvalidCoins, "invalid ibc token: %s", fee)
 	}
 
-	nativeFees, err := fadfd.feeabsKeeper.CalculateNativeFromIBCCoins(ctx, ibcFees, hostChainConfig)
+	nativeFees, err := fadfd.feeabsKeeper.CalculateNativeFromIBCCoins(ctx, fee, hostChainConfig)
 	if err != nil {
 		return ctx, err
 	}
 
 	// deduct the fees
 	if !feeTx.GetFee().IsZero() {
-		err = fadfd.bankKeeper.SendCoinsFromAccountToModule(ctx, feeAbstractionPayer, feeabstypes.ModuleName, ibcFees)
+		err = fadfd.bankKeeper.SendCoinsFromAccountToModule(ctx, feeAbstractionPayer, feeabstypes.ModuleName, fee)
 		if err != nil {
 			return ctx, err
 		}
@@ -150,7 +149,7 @@ func (fadfd FeeAbstractionDeductFeeDecorate) abstractionDeductFeeHandler(ctx sdk
 	}
 
 	events := sdk.Events{sdk.NewEvent(sdk.EventTypeTx,
-		sdk.NewAttribute(sdk.AttributeKeyFee, feeTx.GetFee().String()),
+		sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()),
 	)}
 	ctx.EventManager().EmitEvents(events)
 
@@ -159,12 +158,11 @@ func (fadfd FeeAbstractionDeductFeeDecorate) abstractionDeductFeeHandler(ctx sdk
 
 // DeductFees deducts fees from the given account.
 func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, accAddress sdk.AccAddress, fees sdk.Coins) error {
-	if !fees.IsValid() {
+	if err := fees.Validate(); err != nil {
 		return sdkerrors.Wrapf(errorstypes.ErrInsufficientFee, "invalid fee amount: %s", fees)
 	}
 
-	err := bankKeeper.SendCoinsFromAccountToModule(ctx, accAddress, types.FeeCollectorName, fees)
-	if err != nil {
+	if err := bankKeeper.SendCoinsFromAccountToModule(ctx, accAddress, types.FeeCollectorName, fees); err != nil {
 		return sdkerrors.Wrapf(errorstypes.ErrInsufficientFunds, err.Error())
 	}
 
@@ -266,27 +264,20 @@ func (famfd FeeAbstrationMempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk
 			return ctx, sdkerrors.Wrapf(errorstypes.ErrInsufficientFee, "fee is not a subset of required fees; got %s, required: %s", feeCoins.String(), feeRequired.String())
 		}
 
-		if byPass {
-			return next(ctx, tx, simulate)
-		}
-
 		// if the msg does not satisfy bypass condition and the feeCoins denoms are subset of fezeRequired,
 		// check the feeCoins amount against feeRequired
 		//
 		// when feeCoins=[]
-		// special case: and there is zero coin in fee requirement, pass,
-		// otherwise, err
-		if feeCoinsLen == 0 {
-			if len(zeroCoinFeesDenomReq) != 0 {
-				return next(ctx, tx, simulate)
-			}
-			return ctx, sdkerrors.Wrapf(errorstypes.ErrInsufficientFee, "insufficient fees; got: %s required 12: %s", feeCoins, feeRequired)
-		}
+		// special case: and there is zero coin in fee requirement, pass, otherwise, err
 		// when feeCoins != []
 		// special case: if TX has at least one of the zeroCoinFeesDenomReq, then it should pass
-		if len(feeCoinsZeroDenom) > 0 {
+		if byPass || (feeCoinsLen == 0 && len(zeroCoinFeesDenomReq) != 0) || len(feeCoinsZeroDenom) > 0 {
 			return next(ctx, tx, simulate)
 		}
+
+		if feeCoinsLen == 0 {
+			return ctx, sdkerrors.Wrapf(errorstypes.ErrInsufficientFee, "insufficient fees; got: %s required 12: %s", feeCoins, feeRequired)
+		}
 		// After all the checks, the tx is confirmed:
 		// feeCoins denoms subset off feeRequired (or replaced with fee-abstraction)
 		// Not bypass

x/feeabs/keeper/config.go

@@ -81,15 +81,10 @@ func (k Keeper) GetAllHostZoneConfig(ctx sdk.Context) (allChainConfigs []types.H
 	return allChainConfigs, nil
 }
 
-func (k Keeper) IteratorHostZone(ctx sdk.Context) sdk.Iterator {
-	store := ctx.KVStore(k.storeKey)
-	return sdk.KVStorePrefixIterator(store, types.KeyHostChainChainConfigByFeeAbs)
-}
-
 // IterateHostZone iterates over the hostzone .
 func (k Keeper) IterateHostZone(ctx sdk.Context, cb func(hostZoneConfig types.HostChainFeeAbsConfig) (stop bool)) {
 	store := ctx.KVStore(k.storeKey)
-	iterator := sdk.KVStorePrefixIterator(store, types.KeyHostChainChainConfigByFeeAbs)
+	iterator := sdk.KVStorePrefixIterator(store, types.KeyHostChainConfigByFeeAbs)
 
 	defer iterator.Close()
 	for ; iterator.Valid(); iterator.Next() {
@@ -101,7 +96,7 @@ func (k Keeper) IterateHostZone(ctx sdk.Context, cb func(hostZoneConfig types.Ho
 	}
 }
 
-func (k Keeper) FrozenHostZoneByIBCDenom(ctx sdk.Context, ibcDenom string) error {
+func (k Keeper) FreezeHostZoneByIBCDenom(ctx sdk.Context, ibcDenom string) error {
 	hostChainConfig, found := k.GetHostZoneConfig(ctx, ibcDenom)
 	if !found {
 		return types.ErrHostZoneConfigNotFound
@@ -115,7 +110,7 @@ func (k Keeper) FrozenHostZoneByIBCDenom(ctx sdk.Context, ibcDenom string) error
 	return nil
 }
 
-func (k Keeper) UnFrozenHostZoneByIBCDenom(ctx sdk.Context, ibcDenom string) error {
+func (k Keeper) UnFreezeHostZoneByIBCDenom(ctx sdk.Context, ibcDenom string) error {
 	hostChainConfig, found := k.GetHostZoneConfig(ctx, ibcDenom)
 	if !found {
 		return types.ErrHostZoneConfigNotFound

x/feeabs/keeper/epoch.go

@@ -64,12 +64,6 @@ func (k Keeper) setEpochInfo(ctx sdk.Context, epoch types.EpochInfo) {
 	store.Set(append(types.KeyPrefixEpoch, []byte(epoch.Identifier)...), value)
 }
 
-// DeleteEpochInfo delete epoch info.
-func (k Keeper) DeleteEpochInfo(ctx sdk.Context, identifier string) {
-	store := ctx.KVStore(k.storeKey)
-	store.Delete(append(types.KeyPrefixEpoch, []byte(identifier)...))
-}
-
 // IterateEpochInfo iterate through epochs.
 func (k Keeper) IterateEpochInfo(ctx sdk.Context, fn func(index int64, epochInfo types.EpochInfo) (stop bool)) {
 	store := ctx.KVStore(k.storeKey)
@@ -104,18 +98,6 @@ func (k Keeper) AllEpochInfos(ctx sdk.Context) []types.EpochInfo {
 	return epochs
 }
 
-// NumBlocksSinceEpochStart returns the number of blocks since the epoch started.
-// if the epoch started on block N, then calling this during block N (after BeforeEpochStart)
-// would return 0.
-// Calling it any point in block N+1 (assuming the epoch doesn't increment) would return 1.
-func (k Keeper) NumBlocksSinceEpochStart(ctx sdk.Context, identifier string) (int64, error) {
-	epoch := k.GetEpochInfo(ctx, identifier)
-	if (epoch == types.EpochInfo{}) {
-		return 0, fmt.Errorf("epoch with identifier %s not found", identifier)
-	}
-	return ctx.BlockHeight() - epoch.CurrentEpochStartHeight, nil
-}
-
 func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string) {
 	switch epochIdentifier {
 	case types.DefaultQueryEpochIdentifier:

x/feeabs/keeper/grpc_query.go

@@ -58,7 +58,7 @@ func (q Querier) FeeabsModuleBalances(goCtx context.Context, req *types.QueryFee
 	}, nil
 }
 
-func (q Querier) HostChainConfig(goCtx context.Context, req *types.QueryHostChainConfigRequest) (*types.QueryHostChainConfigRespone, error) {
+func (q Querier) HostChainConfig(goCtx context.Context, req *types.QueryHostChainConfigRequest) (*types.QueryHostChainConfigResponse, error) {
 	if req == nil {
 		return nil, status.Error(codes.InvalidArgument, "empty request")
 	}
@@ -70,12 +70,12 @@ func (q Querier) HostChainConfig(goCtx context.Context, req *types.QueryHostChai
 		return nil, types.ErrHostZoneConfigNotFound
 	}
 
-	return &types.QueryHostChainConfigRespone{
+	return &types.QueryHostChainConfigResponse{
 		HostChainConfig: hostChainConfig,
 	}, nil
 }
 
-func (q Querier) AllHostChainConfig(goCtx context.Context, req *types.AllQueryHostChainConfigRequest) (*types.AllQueryHostChainConfigRespone, error) {
+func (q Querier) AllHostChainConfig(goCtx context.Context, req *types.AllQueryHostChainConfigRequest) (*types.AllQueryHostChainConfigResponse, error) {
 	if req == nil {
 		return nil, status.Error(codes.InvalidArgument, "empty request")
 	}
@@ -87,7 +87,7 @@ func (q Querier) AllHostChainConfig(goCtx context.Context, req *types.AllQueryHo
 		return nil, err
 	}
 
-	return &types.AllQueryHostChainConfigRespone{
+	return &types.AllQueryHostChainConfigResponse{
 		AllHostChainConfig: allHostChainConfig,
 	}, nil
 }

x/feeabs/keeper/grpc_query_test.go

@@ -70,15 +70,15 @@ func (s *KeeperTestSuite) TestHostChainConfig() {
 	for _, tc := range []struct {
 		desc      string
 		req       *types.QueryHostChainConfigRequest
-		res       *types.QueryHostChainConfigRespone
+		res       *types.QueryHostChainConfigResponse
 		shouldErr bool
 	}{
 		{
 			desc: "Success",
 			req: &types.QueryHostChainConfigRequest{
 				IbcDenom: chainConfig.IbcDenom,
 			},
-			res: &types.QueryHostChainConfigRespone{
+			res: &types.QueryHostChainConfigResponse{
 				HostChainConfig: chainConfig,
 			},
 			shouldErr: false,
@@ -88,7 +88,7 @@ func (s *KeeperTestSuite) TestHostChainConfig() {
 			req: &types.QueryHostChainConfigRequest{
 				IbcDenom: "Invalid",
 			},
-			res: &types.QueryHostChainConfigRespone{
+			res: &types.QueryHostChainConfigResponse{
 				HostChainConfig: chainConfig,
 			},
 			shouldErr: true,

x/feeabs/keeper/ibc.go

@@ -19,6 +19,10 @@ import (
 	"github.com/osmosis-labs/fee-abstraction/v7/x/feeabs/types"
 )
 
+const (
+	timeoutDuration = 5 * time.Minute
+)
+
 // GetPort returns the portID for the module. Used in ExportGenesis.
 func (k Keeper) GetPort(ctx sdk.Context) string {
 	store := ctx.KVStore(k.storeKey)
@@ -84,7 +88,7 @@ func (k Keeper) SendInterchainQuery(
 	sourcePort string,
 	sourceChannel string,
 ) (uint64, error) {
-	timeoutTimestamp := ctx.BlockTime().Add(time.Minute * 5).UnixNano()
+	timeoutTimestamp := ctx.BlockTime().Add(timeoutDuration).UnixNano()
 	channelCap, ok := k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(sourcePort, sourceChannel))
 	if !ok {
 		return 0, sdkerrors.Wrap(channeltypes.ErrChannelCapabilityNotFound, "module does not own channel capability")
@@ -135,7 +139,7 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, ack channeltypes.Acknow
 
 			if icqRes.Code != 0 {
 				k.Logger(ctx).Error(fmt.Sprintf("Failed to send interchain query code %d", icqRes.Code))
-				err := k.FrozenHostZoneByIBCDenom(ctx, hostZoneConfig.IbcDenom)
+				err := k.FreezeHostZoneByIBCDenom(ctx, hostZoneConfig.IbcDenom)
 				if err != nil {
 					// should never happen
 					k.Logger(ctx).Error(fmt.Sprintf("Failed to frozen host zone %s", err.Error()))
@@ -151,7 +155,7 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, ack channeltypes.Acknow
 			k.Logger(ctx).Info(fmt.Sprintf("TwapRate %v", twapRate))
 			k.SetTwapRate(ctx, hostZoneConfig.IbcDenom, twapRate)
 
-			err = k.UnFrozenHostZoneByIBCDenom(ctx, hostZoneConfig.IbcDenom)
+			err = k.UnFreezeHostZoneByIBCDenom(ctx, hostZoneConfig.IbcDenom)
 			if err != nil {
 				// should never happen
 				k.Logger(ctx).Error(fmt.Sprintf("Failed to frozen host zone %s", err.Error()))
@@ -168,7 +172,7 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, ack channeltypes.Acknow
 		)
 	case *channeltypes.Acknowledgement_Error:
 		k.IterateHostZone(ctx, func(hostZoneConfig types.HostChainFeeAbsConfig) (stop bool) {
-			err := k.FrozenHostZoneByIBCDenom(ctx, hostZoneConfig.IbcDenom)
+			err := k.FreezeHostZoneByIBCDenom(ctx, hostZoneConfig.IbcDenom)
 			if err != nil {
 				k.Logger(ctx).Error(fmt.Sprintf("Failed to frozen host zone %s", err.Error()))
 			}
@@ -224,7 +228,7 @@ func (k Keeper) transferOsmosisCrosschainSwap(ctx sdk.Context, hostChainConfig t
 		return err
 	}
 
-	timeoutTimestamp := ctx.BlockTime().Add(time.Minute * 5).UnixNano()
+	timeoutTimestamp := ctx.BlockTime().Add(timeoutDuration).UnixNano()
 
 	transferMsg := transfertypes.MsgTransfer{
 		SourcePort:       transfertypes.PortID,
@@ -306,7 +310,7 @@ func (k Keeper) executeAllHostChainSwap(ctx sdk.Context) {
 
 		if err != nil {
 			k.Logger(ctx).Error(fmt.Sprintf("Failed to transfer IBC token %s", err.Error()))
-			err = k.FrozenHostZoneByIBCDenom(ctx, hostZoneConfig.IbcDenom)
+			err = k.FreezeHostZoneByIBCDenom(ctx, hostZoneConfig.IbcDenom)
 			if err != nil {
 				k.Logger(ctx).Error(fmt.Sprintf("Failed to frozem host zone %s", err.Error()))
 			}

x/feeabs/keeper/keeper.go

@@ -107,10 +107,6 @@ func (k Keeper) SendAbstractionFeeToModuleAccount(ctx sdk.Context, ibcCoins sdk.
 
 // return err if IBC token isn't in allowed_list
 func (k Keeper) verifyIBCCoins(ctx sdk.Context, ibcCoins sdk.Coins) error {
-	if ibcCoins.Len() != 1 {
-		return types.ErrInvalidIBCFees
-	}
-
 	if k.HasHostZoneConfig(ctx, ibcCoins[0].Denom) {
 		return nil
 	}

x/feeabs/types/keys.go

@@ -27,20 +27,20 @@ type (
 )
 
 var (
-	OsmosisTwapExchangeRate          = []byte{0x01} // Key for the exchange rate of osmosis (to native token)
-	KeyChannelID                     = []byte{0x02} // Key for IBC channel to osmosis
-	KeyHostChainChainConfigByFeeAbs  = []byte{0x03} // Key for IBC channel to osmosis
-	KeyHostChainChainConfigByOsmosis = []byte{0x04} // Key for IBC channel to osmosis
-	KeyPrefixEpoch                   = []byte{0x05} // KeyPrefixEpoch defines prefix key for storing epochs.
-	KeyTokenDenomPair                = []byte{0x06} // Key store token denom pair on feeabs and osmosis
+	OsmosisTwapExchangeRate     = []byte{0x01} // Key for the exchange rate of osmosis (to native token)
+	KeyChannelID                = []byte{0x02} // Key for IBC channel to osmosis
+	KeyHostChainConfigByFeeAbs  = []byte{0x03} // Key for IBC channel to osmosis
+	KeyHostChainConfigByOsmosis = []byte{0x04} // Key for IBC channel to osmosis
+	KeyPrefixEpoch              = []byte{0x05} // KeyPrefixEpoch defines prefix key for storing epochs.
+	KeyTokenDenomPair           = []byte{0x06} // Key store token denom pair on feeabs and osmosis
 )
 
 func GetKeyHostZoneConfigByFeeabsIBCDenom(feeabsIbcDenom string) []byte {
-	return append(KeyHostChainChainConfigByFeeAbs, []byte(feeabsIbcDenom)...)
+	return append(KeyHostChainConfigByFeeAbs, []byte(feeabsIbcDenom)...)
 }
 
 func GetKeyHostZoneConfigByOsmosisIBCDenom(osmosisIbcDenom string) []byte {
-	return append(KeyHostChainChainConfigByOsmosis, []byte(osmosisIbcDenom)...)
+	return append(KeyHostChainConfigByOsmosis, []byte(osmosisIbcDenom)...)
 }
 
 func GetKeyTwapExchangeRate(ibcDenom string) []byte {