From 8e83ba27fd139decd96c5cad65f3b98700154158 Mon Sep 17 00:00:00 2001 From: Charly Date: Fri, 26 Nov 2021 11:58:45 +0100 Subject: [PATCH 1/7] proto file --- proto/ibc/applications/fee/v1/ack.proto | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 proto/ibc/applications/fee/v1/ack.proto diff --git a/proto/ibc/applications/fee/v1/ack.proto b/proto/ibc/applications/fee/v1/ack.proto new file mode 100644 index 00000000000..03efed89464 --- /dev/null +++ b/proto/ibc/applications/fee/v1/ack.proto @@ -0,0 +1,15 @@ +syntax = "proto3"; + +package ibc.applications.fee.v1; + +message IncentivizedAcknowledgement { + oneof response { + IncentivizedAck result = 21; + string error = 22; + } +} + +message IncentivizedAck { + bytes result = 1; + string forward_relayer_address = 2 [(gogoproto.moretags) = "yaml:\"forward_relayer_address\""]; +} \ No newline at end of file From 5d0b5ccda68ffde054f41bc82ecc7e80007e560b Mon Sep 17 00:00:00 2001 From: Charly Date: Wed, 22 Dec 2021 14:55:37 +0100 Subject: [PATCH 2/7] initial impl --- modules/apps/29-fee/ibc_module.go | 31 +---- modules/apps/29-fee/ibc_module_test.go | 1 + modules/apps/29-fee/keeper/escrow.go | 133 +++++++++++++++++----- modules/apps/29-fee/keeper/escrow_test.go | 40 +++++-- modules/apps/29-fee/types/errors.go | 1 + 5 files changed, 143 insertions(+), 63 deletions(-) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index 22af7709cde..cbb0b56705f 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -219,21 +219,9 @@ func (im IBCModule) OnAcknowledgementPacket( return im.app.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer) } - // cache context before trying to distribute the fee - cacheCtx, writeFn := ctx.CacheContext() + im.keeper.DistributeFee(ctx, identifiedPacketFee.RefundAddress, ack.ForwardRelayerAddress, relayer.String(), packetId) - forwardRelayer, _ := sdk.AccAddressFromBech32(ack.ForwardRelayerAddress) - refundAcc, _ := sdk.AccAddressFromBech32(identifiedPacketFee.RefundAddress) - - err := im.keeper.DistributeFee(cacheCtx, refundAcc, forwardRelayer, relayer, packetId) - - if err == nil { - // write the cache and then call underlying callback - writeFn() - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - } - // otherwise discard cache and call underlying callback + // call underlying callback return im.app.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer) } @@ -257,19 +245,8 @@ func (im IBCModule) OnTimeoutPacket( return im.app.OnTimeoutPacket(ctx, packet, relayer) } - // cache context before trying to distribute the fee - cacheCtx, writeFn := ctx.CacheContext() - - refundAcc, _ := sdk.AccAddressFromBech32(identifiedPacketFee.RefundAddress) - err := im.keeper.DistributeFeeTimeout(cacheCtx, refundAcc, relayer, packetId) - - if err == nil { - // write the cache and then call underlying callback - writeFn() - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - } + im.keeper.DistributeFeeTimeout(ctx, identifiedPacketFee.RefundAddress, relayer.String(), packetId) - // otherwise discard cache and call underlying callback + // call underlying callback return im.app.OnTimeoutPacket(ctx, packet, relayer) } diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index f95617a3dd0..f58002c139b 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -653,6 +653,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { tc.malleate() err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, ack, suite.chainA.SenderAccount.GetAddress()) + fmt.Println(err) if tc.expPass { suite.Require().NoError(err, "unexpected error for case: %s", tc.name) diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index 096d1f0ce3b..cddde2b1ef5 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -44,66 +44,143 @@ func (k Keeper) EscrowPacketFee(ctx sdk.Context, identifiedFee *types.Identified } // DistributeFee pays the acknowledgement fee & receive fee for a given packetId while refunding the timeout fee to the refund account associated with the Fee -func (k Keeper) DistributeFee(ctx sdk.Context, refundAcc, forwardRelayer, reverseRelayer sdk.AccAddress, packetID *channeltypes.PacketId) error { +func (k Keeper) DistributeFee(ctx sdk.Context, refundAcc, forwardRelayer, reverseRelayer string, packetID *channeltypes.PacketId) error { + var packetIdErr error + var distributeFwdErr error + var distributeReverseErr error + var refundAccErr error + // check if there is a Fee in escrow for the given packetId feeInEscrow, found := k.GetFeeInEscrow(ctx, packetID) if !found { - return sdkerrors.Wrapf(types.ErrFeeNotFound, "with channelID %s, sequence %d", packetID.ChannelId, packetID.Sequence) + packetIdErr = sdkerrors.Wrapf(types.ErrFeeNotFound, "with channelID %s, sequence %d", packetID.ChannelId, packetID.Sequence) } - // send receive fee to forward relayer - err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, forwardRelayer, feeInEscrow.Fee.ReceiveFee) - if err != nil { - return sdkerrors.Wrap(err, "failed to send fee to forward relayer") + // cache context before trying to send to forward relayer + cacheCtx, writeFn := ctx.CacheContext() + + fwd, err := sdk.AccAddressFromBech32(forwardRelayer) + if err == nil { + err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, fwd, feeInEscrow.Fee.ReceiveFee) + if err == nil { + // write the cache + writeFn() + // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. + ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) + } + } else { + distributeFwdErr = sdkerrors.Wrap(err, "failed to send fee to forward relayer") } - // send ack fee to reverse relayer - err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, reverseRelayer, feeInEscrow.Fee.AckFee) - if err != nil { - return sdkerrors.Wrap(err, "error sending fee to reverse relayer") + // cache context before trying to send to reverse relayer + cacheCtx, writeFn = ctx.CacheContext() + + reverse, err := sdk.AccAddressFromBech32(reverseRelayer) + if err == nil { + err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, reverse, feeInEscrow.Fee.AckFee) + if err == nil { + // write the cache + writeFn() + // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. + ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) + } + } else { + distributeReverseErr = sdkerrors.Wrap(err, "failed to send fee to reverse relayer") } - // refund timeout fee to refundAddr - err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAcc, feeInEscrow.Fee.TimeoutFee) - if err != nil { - return sdkerrors.Wrap(err, "error refunding timeout fee") + // cache context before trying to send timeout fee to refund address + cacheCtx, writeFn = ctx.CacheContext() + + refund, err := sdk.AccAddressFromBech32(refundAcc) + if err == nil { + err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, refund, feeInEscrow.Fee.TimeoutFee) + if err == nil { + // write the cache + writeFn() + // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. + ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) + } + } else { + refundAccErr = sdkerrors.Wrap(err, "refunding timeout fee") } // removes the fee from the store as fee is now paid k.DeleteFeeInEscrow(ctx, packetID) + // check if there are any errors, otherwise return nil + if packetIdErr != nil || distributeFwdErr != nil || distributeReverseErr != nil || refundAccErr != nil { + return sdkerrors.Wrapf(types.ErrFeeDistribution, "error in distributing fee: %s %s %s %s", packetIdErr, distributeFwdErr, distributeReverseErr, refundAccErr) + } return nil } // DistributeFeeTimeout pays the timeout fee for a given packetId while refunding the acknowledgement fee & receive fee to the refund account associated with the Fee -func (k Keeper) DistributeFeeTimeout(ctx sdk.Context, refundAcc, timeoutRelayer sdk.AccAddress, packetID *channeltypes.PacketId) error { +func (k Keeper) DistributeFeeTimeout(ctx sdk.Context, refundAcc, timeoutRelayer string, packetID *channeltypes.PacketId) error { + var packetIdErr error + var refundAccErr error + var distributeTimeoutErr error + // check if there is a Fee in escrow for the given packetId feeInEscrow, found := k.GetFeeInEscrow(ctx, packetID) if !found { - return sdkerrors.Wrapf(types.ErrFeeNotFound, "for packetID %s", packetID) + packetIdErr = sdkerrors.Wrapf(types.ErrFeeNotFound, "for packetID %s", packetID) } - // refund the receive fee - err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAcc, feeInEscrow.Fee.ReceiveFee) - if err != nil { - return sdkerrors.Wrap(err, "error refunding receive fee") - } + // cache context before trying to refund the receive fee + cacheCtx, writeFn := ctx.CacheContext() + + // check if refundAcc address works + refund, err := sdk.AccAddressFromBech32(refundAcc) + if err == nil { + // first try to refund receive fee + err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, refund, feeInEscrow.Fee.ReceiveFee) + if err == nil { + // write the cache + writeFn() + // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. + ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) + } else { + // set refundAccErr to error resulting from failed refund + refundAccErr = sdkerrors.Wrap(err, "error refunding receive fee") + } - // refund the ack fee - err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAcc, feeInEscrow.Fee.AckFee) - if err != nil { - return sdkerrors.Wrap(err, "error refunding ack fee") + // then try to refund ack fee + err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, refund, feeInEscrow.Fee.AckFee) + if err == nil { + // write the cache + writeFn() + // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. + ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) + } else { + // set refundAccErr to error resulting from failed refund + refundAccErr = sdkerrors.Wrap(err, "error refunding ack fee") + } + } else { + refundAccErr = sdkerrors.Wrap(err, "failed to parse refund account address") } - // pay the timeout fee to the timeout relayer - err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, timeoutRelayer, feeInEscrow.Fee.TimeoutFee) + // parse timeout relayer address + cacheCtx, writeFn = ctx.CacheContext() + timeout, err := sdk.AccAddressFromBech32(timeoutRelayer) if err != nil { - return sdkerrors.Wrap(err, "error sending fee to timeout relayer") + err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, timeout, feeInEscrow.Fee.TimeoutFee) + if err == nil { + // write the cache + writeFn() + // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. + ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) + } + } else { + distributeTimeoutErr = sdkerrors.Wrap(err, "error sending fee to timeout relayer") } // removes the fee from the store as fee is now paid k.DeleteFeeInEscrow(ctx, packetID) + // check if there are any errors, otherwise return nil + if packetIdErr != nil || refundAccErr != nil || distributeTimeoutErr != nil { + return sdkerrors.Wrapf(types.ErrFeeDistribution, "error in distributing fee: %s %s %s", packetIdErr, refundAccErr, distributeTimeoutErr) + } return nil } diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index 103e3904f24..c7be3742f85 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -70,7 +70,11 @@ func (suite *KeeperTestSuite) TestEscrowPacketFee() { packetId = &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: uint64(1)} tc.malleate() - fee := types.Fee{ackFee, receiveFee, timeoutFee} + fee := types.Fee{ + ReceiveFee: receiveFee, + AckFee: ackFee, + TimeoutFee: timeoutFee, + } identifiedPacketFee := &types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} // refundAcc balance before escrow @@ -146,7 +150,11 @@ func (suite *KeeperTestSuite) TestDistributeFee() { receiveFee = validCoins2 timeoutFee = validCoins3 packetId = &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: validSeq} - fee := types.Fee{receiveFee, ackFee, timeoutFee} + fee := types.Fee{ + ReceiveFee: receiveFee, + AckFee: ackFee, + TimeoutFee: timeoutFee, + } // escrow the packet fee & store the fee in state identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} @@ -159,7 +167,7 @@ func (suite *KeeperTestSuite) TestDistributeFee() { // refundAcc balance after escrow refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) - err = suite.chainA.GetSimApp().IBCFeeKeeper.DistributeFee(suite.chainA.GetContext(), refundAcc, forwardRelayer, reverseRelayer, packetId) + err = suite.chainA.GetSimApp().IBCFeeKeeper.DistributeFee(suite.chainA.GetContext(), refundAcc.String(), forwardRelayer.String(), reverseRelayer.String(), packetId) if tc.expPass { suite.Require().NoError(err) @@ -236,7 +244,11 @@ func (suite *KeeperTestSuite) TestDistributeTimeoutFee() { receiveFee = validCoins2 timeoutFee = validCoins3 packetId = &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: validSeq} - fee := types.Fee{receiveFee, ackFee, timeoutFee} + fee := types.Fee{ + ReceiveFee: receiveFee, + AckFee: ackFee, + TimeoutFee: timeoutFee, + } // escrow the packet fee & store the fee in state identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} @@ -248,7 +260,7 @@ func (suite *KeeperTestSuite) TestDistributeTimeoutFee() { // refundAcc balance after escrow refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) - err = suite.chainA.GetSimApp().IBCFeeKeeper.DistributeFeeTimeout(suite.chainA.GetContext(), refundAcc, timeoutRelayer, packetId) + err = suite.chainA.GetSimApp().IBCFeeKeeper.DistributeFeeTimeout(suite.chainA.GetContext(), refundAcc.String(), timeoutRelayer.String(), packetId) if tc.expPass { suite.Require().NoError(err) @@ -291,7 +303,11 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannel() { for i := 0; i < 5; i++ { packetId := &channeltypes.PacketId{ChannelId: "channel-0", PortId: transfertypes.PortID, Sequence: uint64(i)} - fee := types.Fee{receiveFee, ackFee, timeoutFee} + fee := types.Fee{ + ReceiveFee: receiveFee, + AckFee: ackFee, + TimeoutFee: timeoutFee, + } identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), transfertypes.PortID, "channel-0") @@ -301,7 +317,11 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannel() { // send a packet over a different channel to ensure this fee is not refunded packetId := &channeltypes.PacketId{ChannelId: "channel-1", PortId: transfertypes.PortID, Sequence: 1} - fee := types.Fee{receiveFee, ackFee, timeoutFee} + fee := types.Fee{ + ReceiveFee: receiveFee, + AckFee: ackFee, + TimeoutFee: timeoutFee, + } identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), transfertypes.PortID, "channel-1") @@ -318,7 +338,11 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannel() { // create escrow and then change module account balance to cause error on refund packetId = &channeltypes.PacketId{ChannelId: "channel-0", PortId: transfertypes.PortID, Sequence: uint64(6)} - fee = types.Fee{receiveFee, ackFee, timeoutFee} + fee = types.Fee{ + ReceiveFee: receiveFee, + AckFee: ackFee, + TimeoutFee: timeoutFee, + } identifiedPacketFee = types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee) diff --git a/modules/apps/29-fee/types/errors.go b/modules/apps/29-fee/types/errors.go index 7891d6e18b8..8c4070e4dfc 100644 --- a/modules/apps/29-fee/types/errors.go +++ b/modules/apps/29-fee/types/errors.go @@ -13,4 +13,5 @@ var ( ErrRelayersNotNil = sdkerrors.Register(ModuleName, 6, "relayers must be nil. This feature is not supported") ErrCounterpartyAddressEmpty = sdkerrors.Register(ModuleName, 7, "counterparty address must not be empty") ErrFeeNotEnabled = sdkerrors.Register(ModuleName, 8, "fee module is not enabled for this channel. If this error occurs after channel setup, fee module may not be enabled") + ErrFeeDistribution = sdkerrors.Register(ModuleName, 9, "error on distributing packet fee") ) From 9d2742d77ae9a53a7997397ab46756d543c6a606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Fri, 7 Jan 2022 12:08:39 +0100 Subject: [PATCH 3/7] apply self review suggestions Deduplicate fee distribution code. Rename DistributeFee to DistributePacketFees. Rename DistributeFeeTimeout to DistributePacketFeesOnTimeout --- modules/apps/29-fee/ibc_module.go | 7 +- modules/apps/29-fee/keeper/escrow.go | 163 +++++++-------------------- 2 files changed, 45 insertions(+), 125 deletions(-) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index cbb0b56705f..3e0f1653298 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -212,14 +212,14 @@ func (im IBCModule) OnAcknowledgementPacket( } packetId := channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence) - identifiedPacketFee, found := im.keeper.GetFeeInEscrow(ctx, packetId) + identifiedPacketFee, found := im.keeper.GetFeeInEscrow(ctx, packetId) if !found { // return underlying callback if no fee found for given packetID return im.app.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer) } - im.keeper.DistributeFee(ctx, identifiedPacketFee.RefundAddress, ack.ForwardRelayerAddress, relayer.String(), packetId) + im.keeper.DistributePacketFees(ctx, identifiedPacketFee.RefundAddress, ack.ForwardRelayerAddress, relayer, identifiedPacketFee) // call underlying callback return im.app.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer) @@ -239,13 +239,12 @@ func (im IBCModule) OnTimeoutPacket( packetId := channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence) identifiedPacketFee, found := im.keeper.GetFeeInEscrow(ctx, packetId) - if !found { // return underlying callback if fee not found for given packetID return im.app.OnTimeoutPacket(ctx, packet, relayer) } - im.keeper.DistributeFeeTimeout(ctx, identifiedPacketFee.RefundAddress, relayer.String(), packetId) + im.keeper.DistributePacketFeesOnTimeout(ctx, identifiedPacketFee.RefundAddress, relayer, identifiedPacketFee) // call underlying callback return im.app.OnTimeoutPacket(ctx, packet, relayer) diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index cddde2b1ef5..1e9c65ea3d3 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -43,145 +43,66 @@ func (k Keeper) EscrowPacketFee(ctx sdk.Context, identifiedFee *types.Identified return nil } -// DistributeFee pays the acknowledgement fee & receive fee for a given packetId while refunding the timeout fee to the refund account associated with the Fee -func (k Keeper) DistributeFee(ctx sdk.Context, refundAcc, forwardRelayer, reverseRelayer string, packetID *channeltypes.PacketId) error { - var packetIdErr error - var distributeFwdErr error - var distributeReverseErr error - var refundAccErr error - - // check if there is a Fee in escrow for the given packetId - feeInEscrow, found := k.GetFeeInEscrow(ctx, packetID) - if !found { - packetIdErr = sdkerrors.Wrapf(types.ErrFeeNotFound, "with channelID %s, sequence %d", packetID.ChannelId, packetID.Sequence) - } - - // cache context before trying to send to forward relayer - cacheCtx, writeFn := ctx.CacheContext() - - fwd, err := sdk.AccAddressFromBech32(forwardRelayer) +// DistributePacketFees pays the acknowledgement fee & receive fee for a given packetId while refunding the timeout fee to the refund account associated with the Fee. +func (k Keeper) DistributePacketFees(ctx sdk.Context, refundAcc, forwardRelayer string, reverseRelayer sdk.AccAddress, feeInEscrow types.IdentifiedPacketFee) { + // distribute fee for forward relaying + forward, err := sdk.AccAddressFromBech32(forwardRelayer) if err == nil { - err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, fwd, feeInEscrow.Fee.ReceiveFee) - if err == nil { - // write the cache - writeFn() - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - } - } else { - distributeFwdErr = sdkerrors.Wrap(err, "failed to send fee to forward relayer") + k.distributeFee(ctx, forward, feeInEscrow.Fee.ReceiveFee) } - // cache context before trying to send to reverse relayer - cacheCtx, writeFn = ctx.CacheContext() + // distribute fee for reverse relaying + k.distributeFee(ctx, reverseRelayer, feeInEscrow.Fee.AckFee) - reverse, err := sdk.AccAddressFromBech32(reverseRelayer) - if err == nil { - err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, reverse, feeInEscrow.Fee.AckFee) - if err == nil { - // write the cache - writeFn() - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - } - } else { - distributeReverseErr = sdkerrors.Wrap(err, "failed to send fee to reverse relayer") + // refund timeout fee refund, + refundAddr, err := sdk.AccAddressFromBech32(refundAcc) + if err != nil { + panic(fmt.Sprintf("could not parse refundAcc %s to sdk.AccAddress", refundAcc)) } - // cache context before trying to send timeout fee to refund address - cacheCtx, writeFn = ctx.CacheContext() - - refund, err := sdk.AccAddressFromBech32(refundAcc) - if err == nil { - err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, refund, feeInEscrow.Fee.TimeoutFee) - if err == nil { - // write the cache - writeFn() - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - } - } else { - refundAccErr = sdkerrors.Wrap(err, "refunding timeout fee") - } + // refund timeout fee for unused timeout + k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.TimeoutFee) // removes the fee from the store as fee is now paid - k.DeleteFeeInEscrow(ctx, packetID) - - // check if there are any errors, otherwise return nil - if packetIdErr != nil || distributeFwdErr != nil || distributeReverseErr != nil || refundAccErr != nil { - return sdkerrors.Wrapf(types.ErrFeeDistribution, "error in distributing fee: %s %s %s %s", packetIdErr, distributeFwdErr, distributeReverseErr, refundAccErr) - } - return nil + k.DeleteFeeInEscrow(ctx, feeInEscrow.PacketId) } -// DistributeFeeTimeout pays the timeout fee for a given packetId while refunding the acknowledgement fee & receive fee to the refund account associated with the Fee -func (k Keeper) DistributeFeeTimeout(ctx sdk.Context, refundAcc, timeoutRelayer string, packetID *channeltypes.PacketId) error { - var packetIdErr error - var refundAccErr error - var distributeTimeoutErr error - - // check if there is a Fee in escrow for the given packetId - feeInEscrow, found := k.GetFeeInEscrow(ctx, packetID) - if !found { - packetIdErr = sdkerrors.Wrapf(types.ErrFeeNotFound, "for packetID %s", packetID) +// DistributePacketsFeesTimeout pays the timeout fee for a given packetId while refunding the acknowledgement fee & receive fee to the refund account associated with the Fee +func (k Keeper) DistributePacketFeesOnTimeout(ctx sdk.Context, refundAcc string, timeoutRelayer sdk.AccAddress, feeInEscrow types.IdentifiedPacketFee) { + // check if refundAcc address works + refundAddr, err := sdk.AccAddressFromBech32(refundAcc) + if err != nil { + panic(fmt.Sprintf("could not parse refundAcc %s to sdk.AccAddress", refundAcc)) } - // cache context before trying to refund the receive fee - cacheCtx, writeFn := ctx.CacheContext() + // refund receive fee for unused forward relaying + k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.ReceiveFee) - // check if refundAcc address works - refund, err := sdk.AccAddressFromBech32(refundAcc) - if err == nil { - // first try to refund receive fee - err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, refund, feeInEscrow.Fee.ReceiveFee) - if err == nil { - // write the cache - writeFn() - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - } else { - // set refundAccErr to error resulting from failed refund - refundAccErr = sdkerrors.Wrap(err, "error refunding receive fee") - } + // refund ack fee for unused reverse relaying + k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.AckFee) - // then try to refund ack fee - err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, refund, feeInEscrow.Fee.AckFee) - if err == nil { - // write the cache - writeFn() - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - } else { - // set refundAccErr to error resulting from failed refund - refundAccErr = sdkerrors.Wrap(err, "error refunding ack fee") - } - } else { - refundAccErr = sdkerrors.Wrap(err, "failed to parse refund account address") - } - - // parse timeout relayer address - cacheCtx, writeFn = ctx.CacheContext() - timeout, err := sdk.AccAddressFromBech32(timeoutRelayer) - if err != nil { - err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, timeout, feeInEscrow.Fee.TimeoutFee) - if err == nil { - // write the cache - writeFn() - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - } - } else { - distributeTimeoutErr = sdkerrors.Wrap(err, "error sending fee to timeout relayer") - } + // distribute fee for timeout relaying + k.distributeFee(ctx, timeoutRelayer, feeInEscrow.Fee.TimeoutFee) // removes the fee from the store as fee is now paid - k.DeleteFeeInEscrow(ctx, packetID) + k.DeleteFeeInEscrow(ctx, feeInEscrow.PacketId) +} - // check if there are any errors, otherwise return nil - if packetIdErr != nil || refundAccErr != nil || distributeTimeoutErr != nil { - return sdkerrors.Wrapf(types.ErrFeeDistribution, "error in distributing fee: %s %s %s", packetIdErr, refundAccErr, distributeTimeoutErr) +// distributeFee will attempt to distribute the escrowed fee to the receiver address. +// If the distribution fails for any reason (such as the receiving address being blocked), +// the state changes will be discarded. +func (k Keeper) distributeFee(ctx sdk.Context, receiver sdk.AccAddress, fee sdk.Coins) { + // cache context before trying to send to reverse relayer + cacheCtx, writeFn := ctx.CacheContext() + + err := k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, receiver, fee) + if err == nil { + // write the cache + writeFn() + + // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. + ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) } - return nil } func (k Keeper) RefundFeesOnChannel(ctx sdk.Context, portID, channelID string) error { From c450d49b4f47c081dc2bee34c15d6a09e83d88d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Fri, 7 Jan 2022 12:38:44 +0100 Subject: [PATCH 4/7] fixup tests rename validCoins. DistributePacketFeesOnTimeout no longer has a valid error case Add test case for invalid forward relayer address on DistributePacketFees. --- modules/apps/29-fee/ibc_module_test.go | 1 - modules/apps/29-fee/keeper/escrow.go | 1 - modules/apps/29-fee/keeper/escrow_test.go | 208 +++++++----------- modules/apps/29-fee/keeper/genesis_test.go | 12 +- modules/apps/29-fee/keeper/keeper_test.go | 18 +- modules/apps/29-fee/keeper/msg_server_test.go | 12 +- modules/apps/29-fee/types/errors.go | 1 - 7 files changed, 98 insertions(+), 155 deletions(-) diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index f58002c139b..f95617a3dd0 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -653,7 +653,6 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { tc.malleate() err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, ack, suite.chainA.SenderAccount.GetAddress()) - fmt.Println(err) if tc.expPass { suite.Require().NoError(err, "unexpected error for case: %s", tc.name) diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index 1e9c65ea3d3..b379bac6dd2 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -7,7 +7,6 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/ibc-go/modules/apps/29-fee/types" - channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" ) // EscrowPacketFee sends the packet fee to the 29-fee module account to hold in escrow diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index c7be3742f85..0bc940959d1 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -64,9 +64,9 @@ func (suite *KeeperTestSuite) TestEscrowPacketFee() { // setup refundAcc = suite.chainA.SenderAccount.GetAddress() - ackFee = validCoins - receiveFee = validCoins2 - timeoutFee = validCoins3 + receiveFee = defaultReceiveFee + ackFee = defaultAckFee + timeoutFee = defaultTimeoutFee packetId = &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: uint64(1)} tc.malleate() @@ -106,13 +106,8 @@ func (suite *KeeperTestSuite) TestEscrowPacketFee() { func (suite *KeeperTestSuite) TestDistributeFee() { var ( - err error - ackFee sdk.Coins - receiveFee sdk.Coins - timeoutFee sdk.Coins - packetId *channeltypes.PacketId reverseRelayer sdk.AccAddress - forwardRelayer sdk.AccAddress + forwardRelayer string refundAcc sdk.AccAddress ) @@ -127,9 +122,8 @@ func (suite *KeeperTestSuite) TestDistributeFee() { "success", func() {}, true, }, { - "fee not found for packet", func() { - // setting packetId with an invalid sequence of 2 - packetId = &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: uint64(2)} + "invalid forward address", func() { + forwardRelayer = "invalid address" }, false, }, } @@ -144,22 +138,24 @@ func (suite *KeeperTestSuite) TestDistributeFee() { // setup refundAcc = suite.chainA.SenderAccount.GetAddress() reverseRelayer = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) - forwardRelayer = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + forwardRelayer = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() - ackFee = validCoins - receiveFee = validCoins2 - timeoutFee = validCoins3 - packetId = &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: validSeq} + packetId := &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: validSeq} fee := types.Fee{ - ReceiveFee: receiveFee, - AckFee: ackFee, - TimeoutFee: timeoutFee, + ReceiveFee: defaultReceiveFee, + AckFee: defaultAckFee, + TimeoutFee: defaultTimeoutFee, } // escrow the packet fee & store the fee in state - identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} + identifiedPacketFee := types.IdentifiedPacketFee{ + PacketId: packetId, + Fee: fee, + RefundAddress: refundAcc.String(), + Relayers: []string{}, + } - err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee) + err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee) suite.Require().NoError(err) tc.malleate() @@ -167,127 +163,92 @@ func (suite *KeeperTestSuite) TestDistributeFee() { // refundAcc balance after escrow refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) - err = suite.chainA.GetSimApp().IBCFeeKeeper.DistributeFee(suite.chainA.GetContext(), refundAcc.String(), forwardRelayer.String(), reverseRelayer.String(), packetId) + suite.chainA.GetSimApp().IBCFeeKeeper.DistributePacketFees(suite.chainA.GetContext(), refundAcc.String(), forwardRelayer, reverseRelayer, identifiedPacketFee) if tc.expPass { - suite.Require().NoError(err) - hasFeeInEscrow := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeeInEscrow(suite.chainA.GetContext(), packetId) // there should no longer be a fee in escrow for this packet - suite.Require().False(hasFeeInEscrow) + found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeeInEscrow(suite.chainA.GetContext(), packetId) + suite.Require().False(found) + // check if the reverse relayer is paid - hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), reverseRelayer, ackFee[0]) + hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), reverseRelayer, fee.AckFee[0]) suite.Require().True(hasBalance) + // check if the forward relayer is paid - hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), forwardRelayer, receiveFee[0]) + forward, err := sdk.AccAddressFromBech32(forwardRelayer) + suite.Require().NoError(err) + hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), forward, fee.ReceiveFee[0]) suite.Require().True(hasBalance) + // check if the refund acc has been refunded the timeoutFee - expectedRefundAccBal := refundAccBal.Add(timeoutFee[0]) + expectedRefundAccBal := refundAccBal.Add(fee.TimeoutFee[0]) hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), refundAcc, expectedRefundAccBal) suite.Require().True(hasBalance) + // check the module acc wallet is now empty hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(0)}) suite.Require().True(hasBalance) - - suite.Require().NoError(err) - } else { - suite.Require().Error(err) - invalidPacketID := &channeltypes.PacketId{PortId: transfertypes.PortID, ChannelId: suite.path.EndpointA.ChannelID, Sequence: 1} - hasFeeInEscrow := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeeInEscrow(suite.chainA.GetContext(), invalidPacketID) - // there should still be a fee in escrow for this packet - suite.Require().True(hasFeeInEscrow) + // check the module acc wallet still has forward relaying balance + hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), fee.ReceiveFee[0]) + suite.Require().True(hasBalance) } }) } } func (suite *KeeperTestSuite) TestDistributeTimeoutFee() { - var ( - err error - ackFee sdk.Coins - receiveFee sdk.Coins - timeoutFee sdk.Coins - packetId *channeltypes.PacketId - refundAcc sdk.AccAddress - ) + suite.coordinator.Setup(suite.path) // setup channel - validSeq := uint64(1) + // setup + refundAcc := suite.chainA.SenderAccount.GetAddress() + timeoutRelayer := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) - testCases := []struct { - name string - malleate func() - expPass bool - }{ - { - "success", func() {}, true, - }, - { - "fee not found for packet", func() { - // setting packetId with an invalid sequence of 2 - packetId = &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: uint64(2)} - }, false, - }, + packetId := &channeltypes.PacketId{ + ChannelId: suite.path.EndpointA.ChannelID, + PortId: transfertypes.PortID, + Sequence: 1, } - for _, tc := range testCases { - tc := tc - - suite.Run(tc.name, func() { - suite.SetupTest() // reset - suite.coordinator.Setup(suite.path) // setup channel + fee := types.Fee{ + ReceiveFee: defaultReceiveFee, + AckFee: defaultAckFee, + TimeoutFee: defaultTimeoutFee, + } - // setup - refundAcc = suite.chainA.SenderAccount.GetAddress() - timeoutRelayer := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + // escrow the packet fee & store the fee in state + identifiedPacketFee := types.IdentifiedPacketFee{ + PacketId: packetId, + Fee: fee, + RefundAddress: refundAcc.String(), + Relayers: []string{}, + } - ackFee = validCoins - receiveFee = validCoins2 - timeoutFee = validCoins3 - packetId = &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: validSeq} - fee := types.Fee{ - ReceiveFee: receiveFee, - AckFee: ackFee, - TimeoutFee: timeoutFee, - } + err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee) + suite.Require().NoError(err) - // escrow the packet fee & store the fee in state - identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} - err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee) - suite.Require().NoError(err) + // refundAcc balance after escrow + refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) - tc.malleate() + suite.chainA.GetSimApp().IBCFeeKeeper.DistributePacketFeesOnTimeout(suite.chainA.GetContext(), refundAcc.String(), timeoutRelayer, identifiedPacketFee) - // refundAcc balance after escrow - refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) + // there should no longer be a fee in escrow for this packet + found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeeInEscrow(suite.chainA.GetContext(), packetId) + suite.Require().False(found) - err = suite.chainA.GetSimApp().IBCFeeKeeper.DistributeFeeTimeout(suite.chainA.GetContext(), refundAcc.String(), timeoutRelayer.String(), packetId) + // check if the timeoutRelayer has been paid + hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), timeoutRelayer, fee.TimeoutFee[0]) + suite.Require().True(hasBalance) - if tc.expPass { - suite.Require().NoError(err) - hasFeeInEscrow := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeeInEscrow(suite.chainA.GetContext(), packetId) - // there should no longer be a fee in escrow for this packet - suite.Require().False(hasFeeInEscrow) - // check if the timeoutRelayer has been paid - hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), timeoutRelayer, timeoutFee[0]) - suite.Require().True(hasBalance) - // check if the refund acc has been refunded the recv & ack fees - expectedRefundAccBal := refundAccBal.Add(ackFee[0]) - expectedRefundAccBal = refundAccBal.Add(receiveFee[0]) - hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), refundAcc, expectedRefundAccBal) - suite.Require().True(hasBalance) - // check the module acc wallet is now empty - hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(0)}) - suite.Require().True(hasBalance) + // check if the refund acc has been refunded the recv & ack fees + expectedRefundAccBal := refundAccBal.Add(fee.AckFee[0]) + expectedRefundAccBal = refundAccBal.Add(fee.ReceiveFee[0]) + hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), refundAcc, expectedRefundAccBal) + suite.Require().True(hasBalance) - } else { - suite.Require().Error(err) - invalidPacketID := &channeltypes.PacketId{PortId: transfertypes.PortID, ChannelId: suite.path.EndpointA.ChannelID, Sequence: 1} - hasFeeInEscrow := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeeInEscrow(suite.chainA.GetContext(), invalidPacketID) - // there should still be a fee in escrow for this packet - suite.Require().True(hasFeeInEscrow) - } - }) - } + // check the module acc wallet is now empty + hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(0)}) + suite.Require().True(hasBalance) } func (suite *KeeperTestSuite) TestRefundFeesOnChannel() { @@ -297,16 +258,12 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannel() { // refundAcc balance before escrow prevBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), refundAcc) - ackFee := validCoins - receiveFee := validCoins2 - timeoutFee := validCoins3 - for i := 0; i < 5; i++ { packetId := &channeltypes.PacketId{ChannelId: "channel-0", PortId: transfertypes.PortID, Sequence: uint64(i)} fee := types.Fee{ - ReceiveFee: receiveFee, - AckFee: ackFee, - TimeoutFee: timeoutFee, + ReceiveFee: defaultReceiveFee, + AckFee: defaultAckFee, + TimeoutFee: defaultTimeoutFee, } identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} @@ -318,9 +275,9 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannel() { // send a packet over a different channel to ensure this fee is not refunded packetId := &channeltypes.PacketId{ChannelId: "channel-1", PortId: transfertypes.PortID, Sequence: 1} fee := types.Fee{ - ReceiveFee: receiveFee, - AckFee: ackFee, - TimeoutFee: timeoutFee, + ReceiveFee: defaultReceiveFee, + AckFee: defaultAckFee, + TimeoutFee: defaultTimeoutFee, } identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} @@ -334,21 +291,16 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannel() { // add fee sent to channel-1 to after balance to recover original balance afterBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), refundAcc) - suite.Require().Equal(prevBal, afterBal.Add(validCoins...).Add(validCoins2...).Add(validCoins3...), "refund account not back to original balance after refunding all tokens") + suite.Require().Equal(prevBal, afterBal.Add(fee.ReceiveFee...).Add(fee.AckFee...).Add(fee.TimeoutFee...), "refund account not back to original balance after refunding all tokens") // create escrow and then change module account balance to cause error on refund packetId = &channeltypes.PacketId{ChannelId: "channel-0", PortId: transfertypes.PortID, Sequence: uint64(6)} - fee = types.Fee{ - ReceiveFee: receiveFee, - AckFee: ackFee, - TimeoutFee: timeoutFee, - } identifiedPacketFee = types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee) suite.Require().NoError(err) - suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, refundAcc, validCoins3) + suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, refundAcc, fee.TimeoutFee) err = suite.chainA.GetSimApp().IBCFeeKeeper.RefundFeesOnChannel(suite.chainA.GetContext(), transfertypes.PortID, "channel-0") suite.Require().Error(err, "refund fees returned no error with insufficient balance on module account") diff --git a/modules/apps/29-fee/keeper/genesis_test.go b/modules/apps/29-fee/keeper/genesis_test.go index 3fe9a3db38d..ea6984625b2 100644 --- a/modules/apps/29-fee/keeper/genesis_test.go +++ b/modules/apps/29-fee/keeper/genesis_test.go @@ -18,9 +18,9 @@ func (suite *KeeperTestSuite) TestInitGenesis() { uint64(1), ) fee := types.Fee{ - validCoins, - validCoins2, - validCoins3, + defaultReceiveFee, + defaultAckFee, + defaultTimeoutFee, } // relayer addresses @@ -80,9 +80,9 @@ func (suite *KeeperTestSuite) TestExportGenesis() { uint64(1), ) fee := types.Fee{ - validCoins, - validCoins2, - validCoins3, + defaultReceiveFee, + defaultAckFee, + defaultTimeoutFee, } identifiedPacketFee := &types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedPacketFee) diff --git a/modules/apps/29-fee/keeper/keeper_test.go b/modules/apps/29-fee/keeper/keeper_test.go index 7675bb127e3..ad626d3dc0c 100644 --- a/modules/apps/29-fee/keeper/keeper_test.go +++ b/modules/apps/29-fee/keeper/keeper_test.go @@ -15,10 +15,10 @@ import ( ) var ( - validCoins = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}} - validCoins2 = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(200)}} - validCoins3 = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(300)}} - invalidCoins = sdk.Coins{sdk.Coin{Denom: "invalidDenom", Amount: sdk.NewInt(100)}} + defaultReceiveFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}} + defaultAckFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(200)}} + defaultTimeoutFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(300)}} + invalidCoins = sdk.Coins{sdk.Coin{Denom: "invalidDenom", Amount: sdk.NewInt(100)}} ) type KeeperTestSuite struct { @@ -57,10 +57,7 @@ func TestKeeperTestSuite(t *testing.T) { } func (suite *KeeperTestSuite) TestFeeInEscrow() { - ackFee := validCoins - receiveFee := validCoins2 - timeoutFee := validCoins3 - fee := types.Fee{ReceiveFee: receiveFee, AckFee: ackFee, TimeoutFee: timeoutFee} + fee := types.Fee{ReceiveFee: defaultReceiveFee, AckFee: defaultAckFee, TimeoutFee: defaultTimeoutFee} // set some fees for i := 1; i < 6; i++ { @@ -104,11 +101,8 @@ func (suite *KeeperTestSuite) TestGetAllIdentifiedPacketFees() { // escrow a fee refundAcc := suite.chainA.SenderAccount.GetAddress() - ackFee := validCoins - receiveFee := validCoins2 - timeoutFee := validCoins3 packetId := &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: uint64(1)} - fee := types.Fee{ackFee, receiveFee, timeoutFee} + fee := types.Fee{defaultAckFee, defaultReceiveFee, defaultTimeoutFee} identifiedPacketFee := &types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} // escrow the packet fee diff --git a/modules/apps/29-fee/keeper/msg_server_test.go b/modules/apps/29-fee/keeper/msg_server_test.go index f7d5d8f8c59..7daec96c829 100644 --- a/modules/apps/29-fee/keeper/msg_server_test.go +++ b/modules/apps/29-fee/keeper/msg_server_test.go @@ -70,9 +70,9 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { refundAcc := suite.chainA.SenderAccount.GetAddress() channelID := suite.path.EndpointA.ChannelID fee := types.Fee{ - ReceiveFee: validCoins, - AckFee: validCoins, - TimeoutFee: validCoins, + ReceiveFee: defaultReceiveFee, + AckFee: defaultAckFee, + TimeoutFee: defaultTimeoutFee, } msg := types.NewMsgPayPacketFee(fee, suite.path.EndpointA.ChannelConfig.PortID, channelID, refundAcc.String(), []string{}) @@ -111,9 +111,9 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { // build packetId channelID := suite.path.EndpointA.ChannelID fee := types.Fee{ - ReceiveFee: validCoins, - AckFee: validCoins, - TimeoutFee: validCoins, + ReceiveFee: defaultReceiveFee, + AckFee: defaultAckFee, + TimeoutFee: defaultTimeoutFee, } seq, _ := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceSend(ctxA, suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) diff --git a/modules/apps/29-fee/types/errors.go b/modules/apps/29-fee/types/errors.go index 8c4070e4dfc..7891d6e18b8 100644 --- a/modules/apps/29-fee/types/errors.go +++ b/modules/apps/29-fee/types/errors.go @@ -13,5 +13,4 @@ var ( ErrRelayersNotNil = sdkerrors.Register(ModuleName, 6, "relayers must be nil. This feature is not supported") ErrCounterpartyAddressEmpty = sdkerrors.Register(ModuleName, 7, "counterparty address must not be empty") ErrFeeNotEnabled = sdkerrors.Register(ModuleName, 8, "fee module is not enabled for this channel. If this error occurs after channel setup, fee module may not be enabled") - ErrFeeDistribution = sdkerrors.Register(ModuleName, 9, "error on distributing packet fee") ) From 9727fc9239dfef10722a8d33096c5472b2ebb4c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Tue, 11 Jan 2022 17:38:38 +0100 Subject: [PATCH 5/7] partially fix tests timeout fee is still being distributed depsite WriteFn() not being called --- modules/apps/29-fee/ibc_module_test.go | 148 +++++++++++++++++-------- 1 file changed, 99 insertions(+), 49 deletions(-) diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index f95617a3dd0..6d0591a87b4 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -556,7 +556,14 @@ func (suite *FeeTestSuite) TestOnRecvPacket() { // different channel than sending chain func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { - var ack []byte + var ( + ack []byte + identifiedFee *types.IdentifiedPacketFee + originalBalance sdk.Coins + expectedBalance sdk.Coins + expectedRelayerBalance sdk.Coins + ) + testCases := []struct { name string malleate func() @@ -564,7 +571,9 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { }{ { "success", - func() {}, + func() { + expectedRelayerBalance = identifiedFee.Fee.ReceiveFee.Add(identifiedFee.Fee.AckFee[0]) + }, true, }, { @@ -577,13 +586,17 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { Result: channeltypes.NewResultAcknowledgement([]byte{1}).Acknowledgement(), ForwardRelayerAddress: suite.chainA.SenderAccount.GetAddress().String(), }.Acknowledgement() + + expectedBalance = originalBalance }, - false, + true, }, { "ack wrong format", func() { ack = []byte("unsupported acknowledgement format") + + expectedBalance = originalBalance }, false, }, @@ -592,11 +605,13 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { func() { suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) ack = channeltypes.NewResultAcknowledgement([]byte{1}).Acknowledgement() + + expectedBalance = originalBalance }, - false, + true, }, { - "error on distribute fee (blocked address)", + "fail on distribute receive fee (blocked address)", func() { blockedAddr := suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), types.ModuleName).GetAddress() @@ -604,8 +619,10 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { Result: channeltypes.NewResultAcknowledgement([]byte{1}).Acknowledgement(), ForwardRelayerAddress: blockedAddr.String(), }.Acknowledgement() + + expectedBalance = originalBalance.Add(identifiedFee.Fee.AckFee[0]) }, - false, + true, }, } @@ -613,6 +630,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { tc := tc suite.Run(tc.name, func() { suite.SetupTest() + expectedRelayerBalance = sdk.Coins{} // reset // open incentivized channel suite.coordinator.Setup(suite.path) @@ -630,7 +648,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { // escrow the packet fee packetId := channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, suite.chainA.SenderAccount.GetSequence()) - identifiedFee := types.NewIdentifiedPacketFee( + identifiedFee = types.NewIdentifiedPacketFee( packetId, types.Fee{ ReceiveFee: validCoins, @@ -643,45 +661,59 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedFee) suite.Require().NoError(err) + relayerAddr := suite.chainB.SenderAccount.GetAddress() + // must be changed explicitly ack = types.IncentivizedAcknowledgement{ Result: channeltypes.NewResultAcknowledgement([]byte{1}).Acknowledgement(), - ForwardRelayerAddress: suite.chainA.SenderAccount.GetAddress().String(), + ForwardRelayerAddress: relayerAddr.String(), }.Acknowledgement() + // log original sender balance + // NOTE: balance is logged after escrowing tokens + originalBalance = sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) + + // default to success case + expectedBalance = originalBalance. + Add(identifiedFee.Fee.TimeoutFee[0]) + // malleate test case tc.malleate() - err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, ack, suite.chainA.SenderAccount.GetAddress()) + err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, ack, relayerAddr) if tc.expPass { - suite.Require().NoError(err, "unexpected error for case: %s", tc.name) - suite.Require().Equal( - sdk.Coin{ - Denom: ibctesting.TestCoin.Denom, - Amount: sdk.NewInt(100000000000000), - }, - suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) + suite.Require().NoError(err) } else { - suite.Require().Equal( - sdk.Coin{ - Denom: ibctesting.TestCoin.Denom, - Amount: sdk.NewInt(99999999999400), - }, - suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) + suite.Require().Error(err) } + + suite.Require().Equal( + expectedBalance, + sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)), + ) + + relayerBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, ibctesting.TestCoin.Denom)) + suite.Require().Equal( + expectedRelayerBalance, + relayerBalance, + ) + }) } } func (suite *FeeTestSuite) TestOnTimeoutPacket() { var ( - relayerAddr sdk.AccAddress + relayerAddr sdk.AccAddress + identifiedFee *types.IdentifiedPacketFee + originalBalance sdk.Coins + expectedBalance sdk.Coins ) testCases := []struct { - name string - malleate func() - expPass bool + name string + malleate func() + expFeeDistributed bool }{ { "success", @@ -689,25 +721,34 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { true, }, { - "no op if identified packet fee doesn't exist", + "fee not enabled", func() { - // delete packet fee - packetId := channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, suite.chainA.SenderAccount.GetSequence()) - suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeInEscrow(suite.chainA.GetContext(), packetId) + suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + + expectedBalance = originalBalance.Add(ibctesting.TestCoin) // timeout refund for ics20 transfer }, false, }, { - "error on distribute fee (blocked address)", + "no op if identified packet fee doesn't exist", func() { - relayerAddr = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), types.ModuleName).GetAddress() + // delete packet fee + packetId := channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, suite.chainA.SenderAccount.GetSequence()) + suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeInEscrow(suite.chainA.GetContext(), packetId) + + expectedBalance = originalBalance.Add(ibctesting.TestCoin) // timeout refund for ics20 transfer }, false, }, { - "fee not enabled", + "distribute fee fails for timeout fee (blocked address)", func() { - suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + relayerAddr = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), types.ModuleName).GetAddress() + + expectedBalance = originalBalance. + Add(identifiedFee.Fee.ReceiveFee[0]). + Add(identifiedFee.Fee.AckFee[0]). + Add(ibctesting.TestCoin) // timeout refund for ics20 transfer }, false, }, @@ -739,9 +780,9 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { packetId := channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, suite.chainA.SenderAccount.GetSequence()) // must be explicitly changed - relayerAddr = suite.chainA.SenderAccount.GetAddress() + relayerAddr = suite.chainB.SenderAccount.GetAddress() - identifiedFee := types.NewIdentifiedPacketFee( + identifiedFee = types.NewIdentifiedPacketFee( packetId, types.Fee{ ReceiveFee: validCoins, @@ -755,26 +796,35 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedFee) suite.Require().NoError(err) + // log original sender balance + // NOTE: balance is logged after escrowing tokens + originalBalance = sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) + + // default to success case + expectedBalance = originalBalance. + Add(identifiedFee.Fee.ReceiveFee[0]). + Add(identifiedFee.Fee.AckFee[0]). + Add(coin) // timeout refund from ics20 transfer + // malleate test case tc.malleate() err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, relayerAddr) + suite.Require().NoError(err) - if tc.expPass { - suite.Require().NoError(err, "unexpected error for case: %s", tc.name) + suite.Require().Equal( + expectedBalance, + sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)), + ) + + relayerBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, ibctesting.TestCoin.Denom)) + if tc.expFeeDistributed { suite.Require().Equal( - sdk.Coin{ - Denom: ibctesting.TestCoin.Denom, - Amount: sdk.NewInt(100000000000100), - }, - suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) + identifiedFee.Fee.TimeoutFee, + relayerBalance, + ) } else { - suite.Require().Equal( - sdk.Coin{ - Denom: ibctesting.TestCoin.Denom, - Amount: sdk.NewInt(99999999999500), - }, - suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) + suite.Require().Empty(relayerBalance) } }) } From 4c4ffae90bcbea2ddd07fb71599cf866e3be144f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 12 Jan 2022 14:52:27 +0100 Subject: [PATCH 6/7] fix tests --- modules/apps/29-fee/ibc_module_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index f11f8c79f6f..41234d7468a 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -565,14 +565,14 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { { "fail on distribute receive fee (blocked address)", func() { - blockedAddr := suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), types.ModuleName).GetAddress() + blockedAddr := suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress() ack = types.IncentivizedAcknowledgement{ Result: channeltypes.NewResultAcknowledgement([]byte{1}).Acknowledgement(), ForwardRelayerAddress: blockedAddr.String(), }.Acknowledgement() - expectedBalance = originalBalance.Add(identifiedFee.Fee.AckFee[0]) + expectedRelayerBalance = identifiedFee.Fee.AckFee }, true, }, @@ -695,7 +695,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { { "distribute fee fails for timeout fee (blocked address)", func() { - relayerAddr = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), types.ModuleName).GetAddress() + relayerAddr = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress() expectedBalance = originalBalance. Add(identifiedFee.Fee.ReceiveFee[0]). From 2524b52d158fb8c2cac59d1db694d5e4a8a83149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 12 Jan 2022 14:56:00 +0100 Subject: [PATCH 7/7] address code nit --- modules/apps/29-fee/keeper/escrow_test.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index ba0e9311735..3b25f03d101 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -148,14 +148,9 @@ func (suite *KeeperTestSuite) TestDistributeFee() { } // escrow the packet fee & store the fee in state - identifiedPacketFee := types.IdentifiedPacketFee{ - PacketId: packetId, - Fee: fee, - RefundAddress: refundAcc.String(), - Relayers: []string{}, - } + identifiedPacketFee := types.NewIdentifiedPacketFee(packetId, fee, refundAcc.String(), []string{}) - err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee) + err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedPacketFee) suite.Require().NoError(err) tc.malleate() @@ -163,7 +158,7 @@ func (suite *KeeperTestSuite) TestDistributeFee() { // refundAcc balance after escrow refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) - suite.chainA.GetSimApp().IBCFeeKeeper.DistributePacketFees(suite.chainA.GetContext(), refundAcc.String(), forwardRelayer, reverseRelayer, identifiedPacketFee) + suite.chainA.GetSimApp().IBCFeeKeeper.DistributePacketFees(suite.chainA.GetContext(), refundAcc.String(), forwardRelayer, reverseRelayer, *identifiedPacketFee) if tc.expPass { // there should no longer be a fee in escrow for this packet