diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index 1b54981d7ea..ac4ea21ca52 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -56,6 +56,8 @@ func (im IBCModule) OnChanOpenInit( } // OnChanOpenTry implements the IBCModule interface +// If the channel is not fee enabled the underlying application version will be returned +// If the channel is fee enabled we merge the underlying application version with the ics29 version func (im IBCModule) OnChanOpenTry( ctx sdk.Context, order channeltypes.Order, @@ -86,11 +88,11 @@ func (im IBCModule) OnChanOpenTry( return "", err } - if im.keeper.IsFeeEnabled(ctx, portID, channelID) { - return channeltypes.MergeChannelVersions(types.Version, appVersion), nil + if !im.keeper.IsFeeEnabled(ctx, portID, channelID) { + return appVersion, nil } - return appVersion, nil + return channeltypes.MergeChannelVersions(types.Version, appVersion), nil } // OnChanOpenAck implements the IBCModule interface diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index 33ee6a67eec..f01e56ac249 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -23,7 +23,7 @@ func (k Keeper) EscrowPacketFee(ctx sdk.Context, identifiedFee types.IdentifiedP hasRefundAcc := k.authKeeper.GetAccount(ctx, refundAcc) if hasRefundAcc == nil { - return sdkerrors.Wrap(types.ErrRefundAccNotFound, fmt.Sprintf("Account with address: %s not found", refundAcc)) + return sdkerrors.Wrapf(types.ErrRefundAccNotFound, "account with address: %s not found", refundAcc) } coins := identifiedFee.Fee.RecvFee @@ -122,6 +122,7 @@ func (k Keeper) RefundFeesOnChannel(ctx sdk.Context, portID, channelID string) e refundErr = err return true } + err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.AckFee) if err != nil { refundErr = err diff --git a/modules/apps/29-fee/types/msgs.go b/modules/apps/29-fee/types/msgs.go index 4f176de82f5..2d5ba7a42d0 100644 --- a/modules/apps/29-fee/types/msgs.go +++ b/modules/apps/29-fee/types/msgs.go @@ -1,6 +1,8 @@ package types import ( + "strings" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -28,7 +30,7 @@ func (msg MsgRegisterCounterpartyAddress) ValidateBasic() error { return sdkerrors.Wrap(err, "failed to convert msg.Address into sdk.AccAddress") } - if msg.CounterpartyAddress == "" { + if strings.TrimSpace(msg.CounterpartyAddress) == "" { return ErrCounterpartyAddressEmpty } @@ -82,12 +84,12 @@ func (msg MsgPayPacketFee) ValidateBasic() error { // if any of the fee's are invalid return an error if !msg.Fee.AckFee.IsValid() || !msg.Fee.RecvFee.IsValid() || !msg.Fee.TimeoutFee.IsValid() { - return sdkerrors.ErrInvalidCoins + return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, "contains one or more invalid fees") } // if all three fee's are zero or empty return an error if msg.Fee.AckFee.IsZero() && msg.Fee.RecvFee.IsZero() && msg.Fee.TimeoutFee.IsZero() { - return sdkerrors.ErrInvalidCoins + return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, "contains one or more invalid fees") } return nil @@ -119,7 +121,7 @@ func (msg MsgPayPacketFeeAsync) ValidateBasic() error { err = msg.IdentifiedPacketFee.Validate() if err != nil { - return sdkerrors.Wrap(err, "Invalid IdentifiedPacketFee") + return sdkerrors.Wrap(err, "invalid IdentifiedPacketFee") } return nil @@ -144,11 +146,12 @@ func NewIdentifiedPacketFee(packetId channeltypes.PacketId, fee Fee, refundAddr } } +// Validate performs a stateless check of the IdentifiedPacketFee fields func (fee IdentifiedPacketFee) Validate() error { // validate PacketId err := fee.PacketId.Validate() if err != nil { - return sdkerrors.Wrap(err, "Invalid PacketId") + return sdkerrors.Wrap(err, "invalid PacketId") } _, err = sdk.AccAddressFromBech32(fee.RefundAddress) diff --git a/modules/apps/29-fee/types/msgs_test.go b/modules/apps/29-fee/types/msgs_test.go index 8671694bca0..8f7a0dc4cd7 100644 --- a/modules/apps/29-fee/types/msgs_test.go +++ b/modules/apps/29-fee/types/msgs_test.go @@ -29,6 +29,7 @@ func TestMsgRegisterCountepartyAddressValidation(t *testing.T) { {"validate with correct sdk.AccAddress", NewMsgRegisterCounterpartyAddress(validAddr, validAddr), true}, {"validate with incorrect destination relayer address", NewMsgRegisterCounterpartyAddress(invalidAddr, validAddr), false}, {"invalid counterparty address", NewMsgRegisterCounterpartyAddress(validAddr, ""), false}, + {"invalid counterparty address: whitespaced empty string", NewMsgRegisterCounterpartyAddress(validAddr, " "), false}, } for i, tc := range testCases {