diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index 1e5743f7490..cb6a89d13b7 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -321,7 +321,8 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() { packetFees := types.NewPacketFees([]types.PacketFee{types.NewPacketFee(fee, "invalid refund address", nil)}) suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) - suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + suite.Require().NoError(err) }, false, }, @@ -342,7 +343,9 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() { refundAcc = suite.chainA.SenderAccount.GetAddress() packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{}) - err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) + + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee})) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) suite.Require().NoError(err) tc.malleate() @@ -395,7 +398,8 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() { packetFees := types.NewPacketFees([]types.PacketFee{types.NewPacketFee(fee, "invalid refund address", nil)}) suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) - suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + suite.Require().NoError(err) }, false, }, @@ -417,7 +421,9 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() { refundAcc = suite.chainA.SenderAccount.GetAddress() packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{}) - err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) + + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee})) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) suite.Require().NoError(err) tc.malleate() @@ -657,7 +663,9 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { suite.chainA.SenderAccount.GetAddress().String(), []string{}, ) - err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) + + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee})) + err = suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), types.ModuleName, packetFee.Fee.Total()) suite.Require().NoError(err) relayerAddr := suite.chainB.SenderAccount.GetAddress() @@ -789,7 +797,8 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { []string{}, ) - err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee})) + err = suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), types.ModuleName, packetFee.Fee.Total()) suite.Require().NoError(err) // log original sender balance diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index e2b29a439cc..0ca84684440 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -11,13 +11,8 @@ import ( channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" ) -// EscrowPacketFee sends the packet fee to the 29-fee module account to hold in escrow -func (k Keeper) EscrowPacketFee(ctx sdk.Context, packetID channeltypes.PacketId, packetFee types.PacketFee) error { - if !k.IsFeeEnabled(ctx, packetID.PortId, packetID.ChannelId) { - // users may not escrow fees on this channel. Must send packets without a fee message - return sdkerrors.Wrap(types.ErrFeeNotEnabled, "cannot escrow fee for packet") - } - +// escrowPacketFee sends the packet fee to the 29-fee module account to hold in escrow +func (k Keeper) escrowPacketFee(ctx sdk.Context, packetID channeltypes.PacketId, packetFee types.PacketFee) error { // check if the refund address is valid refundAddr, err := sdk.AccAddressFromBech32(packetFee.RefundAddress) if err != nil { diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index ea915beedf6..e2a43afe586 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -9,117 +9,7 @@ import ( channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" ) -func (suite *KeeperTestSuite) TestEscrowPacketFee() { - var ( - err error - refundAcc sdk.AccAddress - ackFee sdk.Coins - receiveFee sdk.Coins - timeoutFee sdk.Coins - packetID channeltypes.PacketId - ) - - testCases := []struct { - name string - malleate func() - expPass bool - }{ - { - "success", func() {}, true, - }, - { - "success with existing packet fee", func() { - fee := types.Fee{ - RecvFee: receiveFee, - AckFee: ackFee, - TimeoutFee: timeoutFee, - } - - packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{}) - feesInEscrow := types.NewPacketFees([]types.PacketFee{packetFee}) - - suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, feesInEscrow) - }, true, - }, - { - "fee not enabled on this channel", func() { - packetID.ChannelId = "disabled_channel" - }, false, - }, - { - "refundAcc does not exist", func() { - // this acc does not exist on chainA - refundAcc = suite.chainB.SenderAccount.GetAddress() - }, false, - }, - { - "ackFee balance not found", func() { - ackFee = invalidCoins - }, false, - }, - { - "receive balance not found", func() { - receiveFee = invalidCoins - }, false, - }, - { - "timeout balance not found", func() { - timeoutFee = invalidCoins - }, false, - }, - } - - for _, tc := range testCases { - tc := tc - - suite.Run(tc.name, func() { - suite.SetupTest() // reset - suite.coordinator.Setup(suite.path) // setup channel - - // setup - refundAcc = suite.chainA.SenderAccount.GetAddress() - receiveFee = defaultRecvFee - ackFee = defaultAckFee - timeoutFee = defaultTimeoutFee - packetID = channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(1)) - - tc.malleate() - fee := types.Fee{ - RecvFee: receiveFee, - AckFee: ackFee, - TimeoutFee: timeoutFee, - } - packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{}) - - // refundAcc balance before escrow - originalBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) - - // escrow the packet fee - err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) - - if tc.expPass { - feesInEscrow, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID) - suite.Require().True(found) - // check if the escrowed fee is set in state - suite.Require().True(feesInEscrow.PacketFees[0].Fee.AckFee.IsEqual(fee.AckFee)) - suite.Require().True(feesInEscrow.PacketFees[0].Fee.RecvFee.IsEqual(fee.RecvFee)) - suite.Require().True(feesInEscrow.PacketFees[0].Fee.TimeoutFee.IsEqual(fee.TimeoutFee)) - // check if the fee is escrowed correctly - hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(600)}) - suite.Require().True(hasBalance) - expectedBal := originalBal.Amount.Sub(sdk.NewInt(600)) - // check if the refund acc has sent the fee - hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), refundAcc, sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: expectedBal}) - suite.Require().True(hasBalance) - suite.Require().NoError(err) - } else { - suite.Require().Error(err) - } - }) - } -} - -func (suite *KeeperTestSuite) TestDistributePacketFeesOnAcknowledgement() { +func (suite *KeeperTestSuite) TestDistributeFee() { var ( forwardRelayer string forwardRelayerBal sdk.Coin @@ -243,14 +133,12 @@ func (suite *KeeperTestSuite) TestDistributePacketFeesOnAcknowledgement() { packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) - // escrow the packet fee & store the fee in state + // escrow the packet fees & store the fees in state packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) packetFees = []types.PacketFee{packetFee, packetFee} - err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) - suite.Require().NoError(err) - // escrow a second packet fee to test with multiple fees distributed - err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees)) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, packetFee.Fee.Total().Add(packetFee.Fee.Total()...)) suite.Require().NoError(err) tc.malleate() @@ -357,14 +245,12 @@ func (suite *KeeperTestSuite) TestDistributePacketFeesOnTimeout() { packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) - // escrow the packet fee & store the fee in state + // escrow the packet fees & store the fees in state packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) packetFees = []types.PacketFee{packetFee, packetFee} - err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) - suite.Require().NoError(err) - // escrow a second packet fee to test with multiple fees distributed - err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees)) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, packetFee.Fee.Total().Add(packetFee.Fee.Total()...)) suite.Require().NoError(err) tc.malleate() @@ -405,7 +291,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) - suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + suite.Require().NoError(err) expIdentifiedPacketFees = append(expIdentifiedPacketFees, identifiedPacketFees) } @@ -421,7 +308,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) - suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + suite.Require().NoError(err) expIdentifiedPacketFees = append(expIdentifiedPacketFees, identifiedPacketFees) } @@ -432,7 +320,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, "channel-1") suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) - suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + suite.Require().NoError(err) expEscrowBal = fee.Total() expRefundBal = expRefundBal.Sub(fee.Total()) @@ -468,7 +357,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID2, packetFees) // update escrow account balance for 1 fee - suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + suite.Require().NoError(err) expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFee1, identifiedPacketFee2} }, true, @@ -482,7 +372,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) - suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + suite.Require().NoError(err) expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFees} }, false, @@ -498,7 +389,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) - suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) + suite.Require().NoError(err) expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFees} diff --git a/modules/apps/29-fee/keeper/grpc_query_test.go b/modules/apps/29-fee/keeper/grpc_query_test.go index 43935490f2a..d665c655ccf 100644 --- a/modules/apps/29-fee/keeper/grpc_query_test.go +++ b/modules/apps/29-fee/keeper/grpc_query_test.go @@ -34,7 +34,7 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPackets() { for i := 0; i < 3; i++ { // escrow packet fees for three different packets packetID := channeltypes.NewPacketId(ibctesting.MockFeePort, ibctesting.FirstChannelID, uint64(i+1)) - suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee})) expectedPackets = append(expectedPackets, types.NewIdentifiedPacketFees(packetID, []types.PacketFee{packetFee})) } @@ -116,11 +116,8 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacket() { fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), []string(nil)) - for i := 0; i < 3; i++ { - // escrow three packet fees for the same packet - err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) - suite.Require().NoError(err) - } + packetFees := []types.PacketFee{packetFee, packetFee, packetFee} + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees)) req = &types.QueryIncentivizedPacketRequest{ PacketId: packetID, @@ -272,11 +269,8 @@ func (suite *KeeperTestSuite) TestQueryTotalRecvFees() { fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), []string(nil)) - for i := 0; i < 3; i++ { - // escrow three packet fees for the same packet - err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) - suite.Require().NoError(err) - } + packetFees := []types.PacketFee{packetFee, packetFee, packetFee} + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees)) req = &types.QueryTotalRecvFeesRequest{ PacketId: packetID, @@ -336,11 +330,8 @@ func (suite *KeeperTestSuite) TestQueryTotalAckFees() { fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), []string(nil)) - for i := 0; i < 3; i++ { - // escrow three packet fees for the same packet - err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) - suite.Require().NoError(err) - } + packetFees := []types.PacketFee{packetFee, packetFee, packetFee} + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees)) req = &types.QueryTotalAckFeesRequest{ PacketId: packetID, @@ -400,11 +391,8 @@ func (suite *KeeperTestSuite) TestQueryTotalTimeoutFees() { fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), []string(nil)) - for i := 0; i < 3; i++ { - // escrow three packet fees for the same packet - err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) - suite.Require().NoError(err) - } + packetFees := []types.PacketFee{packetFee, packetFee, packetFee} + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees)) req = &types.QueryTotalTimeoutFeesRequest{ PacketId: packetID, diff --git a/modules/apps/29-fee/keeper/keeper_test.go b/modules/apps/29-fee/keeper/keeper_test.go index c4be4553c5a..7446ecbab64 100644 --- a/modules/apps/29-fee/keeper/keeper_test.go +++ b/modules/apps/29-fee/keeper/keeper_test.go @@ -98,10 +98,10 @@ func (suite *KeeperTestSuite) TestFeesInEscrow() { packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) - for i := 1; i < 6; i++ { - packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), nil) - suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee) - } + packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), nil) + packetFees := []types.PacketFee{packetFee, packetFee, packetFee, packetFee, packetFee} + + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees)) // retrieve the fees in escrow and assert the length of PacketFees feesInEscrow, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID) diff --git a/modules/apps/29-fee/keeper/msg_server.go b/modules/apps/29-fee/keeper/msg_server.go index 40a7fb4df9c..69f6520c759 100644 --- a/modules/apps/29-fee/keeper/msg_server.go +++ b/modules/apps/29-fee/keeper/msg_server.go @@ -31,6 +31,11 @@ func (k Keeper) RegisterCounterpartyAddress(goCtx context.Context, msg *types.Ms func (k Keeper) PayPacketFee(goCtx context.Context, msg *types.MsgPayPacketFee) (*types.MsgPayPacketFeeResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) + if !k.IsFeeEnabled(ctx, msg.SourcePortId, msg.SourceChannelId) { + // users may not escrow fees on this channel. Must send packets without a fee message + return nil, types.ErrFeeNotEnabled + } + if k.IsLocked(ctx) { return nil, types.ErrFeeModuleLocked } @@ -41,14 +46,10 @@ func (k Keeper) PayPacketFee(goCtx context.Context, msg *types.MsgPayPacketFee) return nil, channeltypes.ErrSequenceSendNotFound } - packetID := channeltypes.NewPacketId( - msg.SourcePortId, - msg.SourceChannelId, - sequence, - ) - + packetID := channeltypes.NewPacketId(msg.SourcePortId, msg.SourceChannelId, sequence) packetFee := types.NewPacketFee(msg.Fee, msg.Signer, msg.Relayers) - if err := k.EscrowPacketFee(ctx, packetID, packetFee); err != nil { + + if err := k.escrowPacketFee(ctx, packetID, packetFee); err != nil { return nil, err } @@ -61,11 +62,16 @@ func (k Keeper) PayPacketFee(goCtx context.Context, msg *types.MsgPayPacketFee) func (k Keeper) PayPacketFeeAsync(goCtx context.Context, msg *types.MsgPayPacketFeeAsync) (*types.MsgPayPacketFeeAsyncResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) + if !k.IsFeeEnabled(ctx, msg.PacketId.PortId, msg.PacketId.ChannelId) { + // users may not escrow fees on this channel. Must send packets without a fee message + return nil, types.ErrFeeNotEnabled + } + if k.IsLocked(ctx) { return nil, types.ErrFeeModuleLocked } - if err := k.EscrowPacketFee(ctx, msg.PacketId, msg.PacketFee); err != nil { + if err := k.escrowPacketFee(ctx, msg.PacketId, msg.PacketFee); err != nil { return nil, err } diff --git a/modules/apps/29-fee/keeper/msg_server_test.go b/modules/apps/29-fee/keeper/msg_server_test.go index 2954ebcebaf..8a927559a51 100644 --- a/modules/apps/29-fee/keeper/msg_server_test.go +++ b/modules/apps/29-fee/keeper/msg_server_test.go @@ -54,100 +54,257 @@ func (suite *KeeperTestSuite) TestRegisterCounterpartyAddress() { } func (suite *KeeperTestSuite) TestPayPacketFee() { + var ( + expEscrowBalance sdk.Coins + expFeesInEscrow []types.PacketFee + msg *types.MsgPayPacketFee + ) + testCases := []struct { name string - expPass bool malleate func() + expPass bool }{ { "success", - true, func() {}, + true, + }, + { + "success with existing packet fees in escrow", + func() { + fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + + packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) + packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), nil) + feesInEscrow := types.NewPacketFees([]types.PacketFee{packetFee}) + + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, feesInEscrow) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), types.ModuleName, fee.Total()) + suite.Require().NoError(err) + + expEscrowBalance = expEscrowBalance.Add(fee.Total()...) + expFeesInEscrow = append(expFeesInEscrow, packetFee) + }, + true, }, { "fee module is locked", - false, func() { lockFeeModule(suite.chainA) }, + false, + }, + { + "fee module disabled on channel", + func() { + msg.SourcePortId = "invalid-port" + msg.SourceChannelId = "invalid-channel" + }, + false, + }, + { + "invalid refund address", + func() { + msg.Signer = "invalid-address" + }, + false, + }, + { + "refund account does not exist", + func() { + msg.Signer = suite.chainB.SenderAccount.GetAddress().String() + }, + false, + }, + { + "acknowledgement fee balance not found", + func() { + msg.Fee.AckFee = invalidCoins + }, + false, + }, + { + "receive fee balance not found", + func() { + msg.Fee.RecvFee = invalidCoins + }, + false, + }, + { + "timeout fee balance not found", + func() { + msg.Fee.TimeoutFee = invalidCoins + }, + false, }, } for _, tc := range testCases { - suite.SetupTest() - suite.coordinator.Setup(suite.path) // setup channel - - refundAcc := suite.chainA.SenderAccount.GetAddress() - channelID := suite.path.EndpointA.ChannelID - fee := types.Fee{ - RecvFee: defaultRecvFee, - AckFee: defaultAckFee, - TimeoutFee: defaultTimeoutFee, - } - msg := types.NewMsgPayPacketFee(fee, suite.path.EndpointA.ChannelConfig.PortID, channelID, refundAcc.String(), []string{}) + tc := tc - tc.malleate() + suite.Run(tc.name, func() { + suite.SetupTest() + suite.coordinator.Setup(suite.path) // setup channel - _, err := suite.chainA.GetSimApp().IBCFeeKeeper.PayPacketFee(sdk.WrapSDKContext(suite.chainA.GetContext()), msg) + fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + msg = types.NewMsgPayPacketFee( + fee, + suite.path.EndpointA.ChannelConfig.PortID, + suite.path.EndpointA.ChannelID, + suite.chainA.SenderAccount.GetAddress().String(), + nil, + ) - if tc.expPass { - suite.Require().NoError(err) // message committed - } else { - suite.Require().Error(err) - } + expEscrowBalance = fee.Total() + expPacketFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), nil) + expFeesInEscrow = []types.PacketFee{expPacketFee} + + tc.malleate() + + _, err := suite.chainA.GetSimApp().IBCFeeKeeper.PayPacketFee(sdk.WrapSDKContext(suite.chainA.GetContext()), msg) + + if tc.expPass { + suite.Require().NoError(err) // message committed + + packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) + feesInEscrow, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID) + suite.Require().True(found) + suite.Require().Equal(expFeesInEscrow, feesInEscrow.PacketFees) + + escrowBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.DefaultBondDenom) + suite.Require().Equal(expEscrowBalance.AmountOf(sdk.DefaultBondDenom), escrowBalance.Amount) + } else { + suite.Require().Error(err) + + escrowBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.DefaultBondDenom) + suite.Require().Equal(sdk.NewInt(0), escrowBalance.Amount) + } + }) } } func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { + var ( + expEscrowBalance sdk.Coins + expFeesInEscrow []types.PacketFee + msg *types.MsgPayPacketFeeAsync + ) + testCases := []struct { name string - expPass bool malleate func() + expPass bool }{ { "success", - true, func() {}, + true, + }, + { + "success with existing packet fees in escrow", + func() { + fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + + packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) + packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), nil) + feesInEscrow := types.NewPacketFees([]types.PacketFee{packetFee}) + + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, feesInEscrow) + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), types.ModuleName, fee.Total()) + suite.Require().NoError(err) + + expEscrowBalance = expEscrowBalance.Add(fee.Total()...) + expFeesInEscrow = append(expFeesInEscrow, packetFee) + }, + true, }, { "fee module is locked", - false, func() { lockFeeModule(suite.chainA) }, + false, + }, + { + "fee module disabled on channel", + func() { + msg.PacketId.PortId = "invalid-port" + msg.PacketId.ChannelId = "invalid-channel" + }, + false, + }, + { + "invalid refund address", + func() { + msg.PacketFee.RefundAddress = "invalid-address" + }, + false, + }, + { + "refund account does not exist", + func() { + msg.PacketFee.RefundAddress = suite.chainB.SenderAccount.GetAddress().String() + }, + false, + }, + { + "acknowledgement fee balance not found", + func() { + msg.PacketFee.Fee.AckFee = invalidCoins + }, + false, + }, + { + "receive fee balance not found", + func() { + msg.PacketFee.Fee.RecvFee = invalidCoins + }, + false, + }, + { + "timeout fee balance not found", + func() { + msg.PacketFee.Fee.TimeoutFee = invalidCoins + }, + false, }, } for _, tc := range testCases { - suite.SetupTest() - suite.coordinator.Setup(suite.path) // setup channel + tc := tc - ctxA := suite.chainA.GetContext() + suite.Run(tc.name, func() { + suite.SetupTest() + suite.coordinator.Setup(suite.path) // setup channel - refundAcc := suite.chainA.SenderAccount.GetAddress() + packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) + fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), nil) - // build packetID - channelID := suite.path.EndpointA.ChannelID - fee := types.Fee{ - RecvFee: defaultRecvFee, - AckFee: defaultAckFee, - TimeoutFee: defaultTimeoutFee, - } - seq, _ := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceSend(ctxA, suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + expEscrowBalance = fee.Total() + expFeesInEscrow = []types.PacketFee{packetFee} + msg = types.NewMsgPayPacketFeeAsync(packetID, packetFee) - // build fee - packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, channelID, seq) - packetFee := types.NewPacketFee(fee, refundAcc.String(), nil) + tc.malleate() - tc.malleate() + _, err := suite.chainA.GetSimApp().IBCFeeKeeper.PayPacketFeeAsync(sdk.WrapSDKContext(suite.chainA.GetContext()), msg) - msg := types.NewMsgPayPacketFeeAsync(packetID, packetFee) - _, err := suite.chainA.GetSimApp().IBCFeeKeeper.PayPacketFeeAsync(sdk.WrapSDKContext(suite.chainA.GetContext()), msg) + if tc.expPass { + suite.Require().NoError(err) // message committed - if tc.expPass { - suite.Require().NoError(err) // message committed - } else { - suite.Require().Error(err) - } + feesInEscrow, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID) + suite.Require().True(found) + suite.Require().Equal(expFeesInEscrow, feesInEscrow.PacketFees) + + escrowBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.DefaultBondDenom) + suite.Require().Equal(expEscrowBalance.AmountOf(sdk.DefaultBondDenom), escrowBalance.Amount) + } else { + suite.Require().Error(err) + + escrowBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.DefaultBondDenom) + suite.Require().Equal(sdk.NewInt(0), escrowBalance.Amount) + } + }) } }