From e165abe9510b5e17a84636f17d50fa45c0e4633a Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Wed, 1 May 2024 23:59:33 +0200 Subject: [PATCH 1/6] delete the refunded fees in case an error happens in the loop that refunds fees on channel closure --- modules/apps/29-fee/keeper/escrow.go | 12 ++++- modules/apps/29-fee/keeper/escrow_test.go | 59 +++++++++++------------ 2 files changed, 37 insertions(+), 34 deletions(-) diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index c3bb9c8fefa..07e158432d0 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -190,7 +190,8 @@ func (k Keeper) RefundFeesOnChannelClosure(ctx sdk.Context, portID, channelID st for _, identifiedPacketFee := range identifiedPacketFees { var failedToSendCoins bool - for _, packetFee := range identifiedPacketFee.PacketFees { + var unRefundedFees []types.PacketFee + for i, packetFee := range identifiedPacketFee.PacketFees { if !k.EscrowAccountHasBalance(cacheCtx, packetFee.Fee.Total()) { // if the escrow account does not have sufficient funds then there must exist a severe bug @@ -205,6 +206,9 @@ func (k Keeper) RefundFeesOnChannelClosure(ctx sdk.Context, portID, channelID st return nil } + // pre-empitively store the unrefunded fees in case of failure + unRefundedFees = identifiedPacketFee.PacketFees[i:] + refundAddr, err := sdk.AccAddressFromBech32(packetFee.RefundAddress) if err != nil { failedToSendCoins = true @@ -218,7 +222,11 @@ func (k Keeper) RefundFeesOnChannelClosure(ctx sdk.Context, portID, channelID st } } - if !failedToSendCoins { + if failedToSendCoins { + // update packet fees to keep only the unrefunded fees + packetFees := types.NewPacketFees(unRefundedFees) + k.SetFeesInEscrow(cacheCtx, identifiedPacketFee.PacketId, packetFees) + } else { k.DeleteFeesInEscrow(cacheCtx, identifiedPacketFee.PacketId) } } diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index f0d6233d23f..4a20b7da01e 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -400,14 +400,13 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { expRefundBal sdk.Coins refundAcc sdk.AccAddress fee types.Fee - locked bool expectEscrowFeesToBeDeleted bool ) testCases := []struct { name string malleate func() - expPass bool + locked bool }{ { "success", func() { @@ -424,7 +423,7 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { expIdentifiedPacketFees = append(expIdentifiedPacketFees, identifiedPacketFees) } - }, true, + }, false, }, { "success with undistributed packet fees on a different channel", func() { @@ -453,12 +452,10 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { expEscrowBal = fee.Total() expRefundBal = expRefundBal.Sub(fee.Total()...) - }, true, + }, false, }, { "escrow account empty, module should become locked", func() { - locked = true - // store the fee in state without updating escrow account balance packetID := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(1)) packetFees := types.NewPacketFees([]types.PacketFee{types.NewPacketFee(fee, refundAcc.String(), nil)}) @@ -467,13 +464,10 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFees} - }, - true, + }, true, }, { "escrow account goes negative on second packet, module should become locked", func() { - locked = true - // store 2 fees in state packetID1 := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(1)) packetID2 := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(2)) @@ -496,19 +490,20 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { // store the fee in state & update escrow account balance expectEscrowFeesToBeDeleted = false packetID := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(1)) - packetFees := types.NewPacketFees([]types.PacketFee{types.NewPacketFee(fee, "invalid refund address", nil)}) - identifiedPacketFees := types.NewIdentifiedPacketFees(packetID, packetFees.PacketFees) - + packetFees := types.NewPacketFees([]types.PacketFee{ + types.NewPacketFee(fee, refundAcc.String(), nil), // this packet fee will be refunded, and will be deleted from state + types.NewPacketFee(fee, "invalid refund address", nil), + }) suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) - err := 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().MulInt(sdkmath.NewInt(2))) suite.Require().NoError(err) - expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFees} + expIdentifiedPacketFees = []types.IdentifiedPacketFees{types.NewIdentifiedPacketFees(packetID, packetFees.PacketFees[1:])} expEscrowBal = fee.Total() expRefundBal = expRefundBal.Sub(fee.Total()...) - }, true, + }, false, }, { "distributing to blocked address is skipped", func() { @@ -517,19 +512,20 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { // store the fee in state & update escrow account balance packetID := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(1)) - packetFees := types.NewPacketFees([]types.PacketFee{types.NewPacketFee(fee, blockedAddr, nil)}) - identifiedPacketFees := types.NewIdentifiedPacketFees(packetID, packetFees.PacketFees) - + packetFees := types.NewPacketFees([]types.PacketFee{ + types.NewPacketFee(fee, refundAcc.String(), nil), // this packet fee will be refunded, and will be deleted from state + types.NewPacketFee(fee, blockedAddr, nil), + }) suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) - err := 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().MulInt(sdkmath.NewInt(2))) suite.Require().NoError(err) - expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFees} + expIdentifiedPacketFees = []types.IdentifiedPacketFees{types.NewIdentifiedPacketFees(packetID, packetFees.PacketFees[1:])} expEscrowBal = fee.Total() expRefundBal = expRefundBal.Sub(fee.Total()...) - }, true, + }, false, }, } @@ -541,7 +537,6 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.path.Setup() // setup channel expIdentifiedPacketFees = []types.IdentifiedPacketFees{} expEscrowBal = sdk.Coins{} - locked = false expectEscrowFeesToBeDeleted = true // setup @@ -565,20 +560,15 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { originalEscrowBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), moduleAcc) err := suite.chainA.GetSimApp().IBCFeeKeeper.RefundFeesOnChannelClosure(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + suite.Require().NoError(err) // refundAcc balance after RefundFeesOnChannelClosure refundBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), refundAcc) escrowBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), moduleAcc) - if tc.expPass { - suite.Require().NoError(err) - } else { - suite.Require().Error(err) - } + suite.Require().Equal(tc.locked, suite.chainA.GetSimApp().IBCFeeKeeper.IsLocked(suite.chainA.GetContext())) - suite.Require().Equal(locked, suite.chainA.GetSimApp().IBCFeeKeeper.IsLocked(suite.chainA.GetContext())) - - if locked || !tc.expPass { + if tc.locked { // refund account and escrow account balances should remain unchanged suite.Require().Equal(originalRefundBal, refundBal) suite.Require().Equal(originalEscrowBal, escrowBal) @@ -590,7 +580,12 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.Require().Equal(expRefundBal, refundBal) // all packets should have been refunded // all fees in escrow should be deleted if expected for this channel - suite.Require().Equal(expectEscrowFeesToBeDeleted, len(suite.chainA.GetSimApp().IBCFeeKeeper.GetIdentifiedPacketFeesForChannel(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)) == 0) + // otherwise, the fees should remain in escrow + if expectEscrowFeesToBeDeleted { + suite.Require().Len(suite.chainA.GetSimApp().IBCFeeKeeper.GetIdentifiedPacketFeesForChannel(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID), 0) + } else { + suite.Require().Equal(expIdentifiedPacketFees, suite.chainA.GetSimApp().IBCFeeKeeper.GetIdentifiedPacketFeesForChannel(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)) + } } }) } From 24e40bd026ae02bca83047ce0c68f51680e5ccef Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Thu, 2 May 2024 09:13:55 +0200 Subject: [PATCH 2/6] test simplifications --- modules/apps/29-fee/keeper/escrow_test.go | 42 ++++++++--------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index 4a20b7da01e..36a945c2495 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -395,12 +395,11 @@ func (suite *KeeperTestSuite) TestDistributePacketFeesOnTimeout() { func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { var ( - expIdentifiedPacketFees []types.IdentifiedPacketFees - expEscrowBal sdk.Coins - expRefundBal sdk.Coins - refundAcc sdk.AccAddress - fee types.Fee - expectEscrowFeesToBeDeleted bool + expIdentifiedPacketFees []types.IdentifiedPacketFees + expEscrowBal sdk.Coins + expRefundBal sdk.Coins + refundAcc sdk.AccAddress + fee types.Fee ) testCases := []struct { @@ -414,14 +413,11 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { // store the fee in state & update escrow account balance packetID := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(i)) packetFees := types.NewPacketFees([]types.PacketFee{types.NewPacketFee(fee, refundAcc.String(), nil)}) - identifiedPacketFees := types.NewIdentifiedPacketFees(packetID, packetFees.PacketFees) suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) suite.Require().NoError(err) - - expIdentifiedPacketFees = append(expIdentifiedPacketFees, identifiedPacketFees) } }, false, }, @@ -431,14 +427,10 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { // store the fee in state & update escrow account balance packetID := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(i)) packetFees := types.NewPacketFees([]types.PacketFee{types.NewPacketFee(fee, refundAcc.String(), nil)}) - identifiedPacketFees := types.NewIdentifiedPacketFees(packetID, packetFees.PacketFees) - suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) suite.Require().NoError(err) - - expIdentifiedPacketFees = append(expIdentifiedPacketFees, identifiedPacketFees) } // set packet fee for a different channel @@ -488,7 +480,7 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { { "invalid refund acc address", func() { // store the fee in state & update escrow account balance - expectEscrowFeesToBeDeleted = false + //expectEscrowFeesToBeDeleted = false packetID := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(1)) packetFees := types.NewPacketFees([]types.PacketFee{ types.NewPacketFee(fee, refundAcc.String(), nil), // this packet fee will be refunded, and will be deleted from state @@ -496,9 +488,11 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { }) suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) + // escrow twice the fee amount to account for the packet to have been incentivized twice err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total().MulInt(sdkmath.NewInt(2))) suite.Require().NoError(err) + // only the first packet fee should be refunded, the second should remain in state expIdentifiedPacketFees = []types.IdentifiedPacketFees{types.NewIdentifiedPacketFees(packetID, packetFees.PacketFees[1:])} expEscrowBal = fee.Total() @@ -507,7 +501,7 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { }, { "distributing to blocked address is skipped", func() { - expectEscrowFeesToBeDeleted = false + //expectEscrowFeesToBeDeleted = false blockedAddr := suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress().String() // store the fee in state & update escrow account balance @@ -518,9 +512,11 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { }) suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees) + // escrow twice the fee amount to account for the packet to have been incentivized twice err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total().MulInt(sdkmath.NewInt(2))) suite.Require().NoError(err) + // only the first packet fee should be refunded, the second should remain in state expIdentifiedPacketFees = []types.IdentifiedPacketFees{types.NewIdentifiedPacketFees(packetID, packetFees.PacketFees[1:])} expEscrowBal = fee.Total() @@ -535,9 +531,9 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.Run(tc.name, func() { suite.SetupTest() // reset suite.path.Setup() // setup channel - expIdentifiedPacketFees = []types.IdentifiedPacketFees{} + expIdentifiedPacketFees = []types.IdentifiedPacketFees(nil) expEscrowBal = sdk.Coins{} - expectEscrowFeesToBeDeleted = true + //expectEscrowFeesToBeDeleted = true // setup refundAcc = suite.chainA.SenderAccount.GetAddress() @@ -567,25 +563,15 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { escrowBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), moduleAcc) suite.Require().Equal(tc.locked, suite.chainA.GetSimApp().IBCFeeKeeper.IsLocked(suite.chainA.GetContext())) + suite.Require().Equal(expIdentifiedPacketFees, suite.chainA.GetSimApp().IBCFeeKeeper.GetIdentifiedPacketFeesForChannel(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)) if tc.locked { // refund account and escrow account balances should remain unchanged suite.Require().Equal(originalRefundBal, refundBal) suite.Require().Equal(originalEscrowBal, escrowBal) - - // ensure none of the fees were deleted - suite.Require().Equal(expIdentifiedPacketFees, suite.chainA.GetSimApp().IBCFeeKeeper.GetIdentifiedPacketFeesForChannel(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)) } else { suite.Require().Equal(expEscrowBal, escrowBal) // escrow balance should be empty suite.Require().Equal(expRefundBal, refundBal) // all packets should have been refunded - - // all fees in escrow should be deleted if expected for this channel - // otherwise, the fees should remain in escrow - if expectEscrowFeesToBeDeleted { - suite.Require().Len(suite.chainA.GetSimApp().IBCFeeKeeper.GetIdentifiedPacketFeesForChannel(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID), 0) - } else { - suite.Require().Equal(expIdentifiedPacketFees, suite.chainA.GetSimApp().IBCFeeKeeper.GetIdentifiedPacketFeesForChannel(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)) - } } }) } From 6c3b67470e7380be529acec98b831f7f7d847d2a Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Sun, 5 May 2024 23:09:11 +0200 Subject: [PATCH 3/6] fix typo --- modules/apps/29-fee/keeper/escrow.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index 07e158432d0..f1761d4e854 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -206,7 +206,7 @@ func (k Keeper) RefundFeesOnChannelClosure(ctx sdk.Context, portID, channelID st return nil } - // pre-empitively store the unrefunded fees in case of failure + // preemptively store the unrefunded fees in case of failure unRefundedFees = identifiedPacketFee.PacketFees[i:] refundAddr, err := sdk.AccAddressFromBech32(packetFee.RefundAddress) From 9eb3bd48c1d73f0b56905b433eb0c62f262f9f4c Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Sun, 5 May 2024 23:10:26 +0200 Subject: [PATCH 4/6] clean up code --- modules/apps/29-fee/keeper/escrow_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index 36a945c2495..8aa0840784c 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -480,7 +480,6 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { { "invalid refund acc address", func() { // store the fee in state & update escrow account balance - //expectEscrowFeesToBeDeleted = false packetID := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(1)) packetFees := types.NewPacketFees([]types.PacketFee{ types.NewPacketFee(fee, refundAcc.String(), nil), // this packet fee will be refunded, and will be deleted from state @@ -501,7 +500,6 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { }, { "distributing to blocked address is skipped", func() { - //expectEscrowFeesToBeDeleted = false blockedAddr := suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress().String() // store the fee in state & update escrow account balance @@ -533,7 +531,6 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { suite.path.Setup() // setup channel expIdentifiedPacketFees = []types.IdentifiedPacketFees(nil) expEscrowBal = sdk.Coins{} - //expectEscrowFeesToBeDeleted = true // setup refundAcc = suite.chainA.SenderAccount.GetAddress() From 9027b6ebf1d8645ba160eb118d6ed68c74076150 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Sun, 5 May 2024 23:23:25 +0200 Subject: [PATCH 5/6] fix logic --- modules/apps/29-fee/keeper/escrow.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index f1761d4e854..1ebbeb95caa 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -189,9 +189,8 @@ func (k Keeper) RefundFeesOnChannelClosure(ctx sdk.Context, portID, channelID st cacheCtx, writeFn := ctx.CacheContext() for _, identifiedPacketFee := range identifiedPacketFees { - var failedToSendCoins bool var unRefundedFees []types.PacketFee - for i, packetFee := range identifiedPacketFee.PacketFees { + for _, packetFee := range identifiedPacketFee.PacketFees { if !k.EscrowAccountHasBalance(cacheCtx, packetFee.Fee.Total()) { // if the escrow account does not have sufficient funds then there must exist a severe bug @@ -206,23 +205,20 @@ func (k Keeper) RefundFeesOnChannelClosure(ctx sdk.Context, portID, channelID st return nil } - // preemptively store the unrefunded fees in case of failure - unRefundedFees = identifiedPacketFee.PacketFees[i:] - refundAddr, err := sdk.AccAddressFromBech32(packetFee.RefundAddress) if err != nil { - failedToSendCoins = true + unRefundedFees = append(unRefundedFees, packetFee) continue } // refund all fees to refund address if err = k.bankKeeper.SendCoinsFromModuleToAccount(cacheCtx, types.ModuleName, refundAddr, packetFee.Fee.Total()); err != nil { - failedToSendCoins = true + unRefundedFees = append(unRefundedFees, packetFee) continue } } - if failedToSendCoins { + if len(unRefundedFees) > 0 { // update packet fees to keep only the unrefunded fees packetFees := types.NewPacketFees(unRefundedFees) k.SetFeesInEscrow(cacheCtx, identifiedPacketFee.PacketId, packetFees) From 5389f1ee907bc84e611bfaffb16e180525e2cf8c Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Sun, 5 May 2024 23:30:06 +0200 Subject: [PATCH 6/6] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c25ce8d2ae1..9b1b69d48e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes * (apps/27-interchain-accounts) [\#6167](https://github.com/cosmos/ibc-go/pull/6167) Fixed an edge case bug where migrating params for a pre-existing ica module which implemented controller functionality only could panic when migrating params for newly added host, and align controller param migration with host. +* (app/29-fee) [\#6255](https://github.com/cosmos/ibc-go/pull/6255) Delete refunded fees from state if some fee(s) cannot be refunded on channel closure. ## [v8.2.0](https://github.com/cosmos/ibc-go/releases/tag/v8.2.0) - 2024-04-05