From 010c4c5803adbf622d320697b88e30639a7c531a Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 7 May 2024 23:12:23 +0200 Subject: [PATCH 1/3] fix: delete already refunded fees from state if some fee cannot be refunded on channel closure (#6255) * delete the refunded fees in case an error happens in the loop that refunds fees on channel closure * test simplifications * fix typo * clean up code * fix logic * add changelog (cherry picked from commit 500765e43a2059a80ebac2d596adeaf12d1883c5) # Conflicts: # CHANGELOG.md # modules/apps/29-fee/keeper/escrow_test.go --- CHANGELOG.md | 145 ++++++++++++++++++++++ modules/apps/29-fee/keeper/escrow.go | 12 +- modules/apps/29-fee/keeper/escrow_test.go | 90 ++++++-------- 3 files changed, 190 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ca36a43c73..6eff8546002 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,151 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +<<<<<<< HEAD +======= +* (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 + +### Dependencies + +* [\#5975](https://github.com/cosmos/ibc-go/pull/5975) Bump Cosmos SDK to v0.50.5. + +### Improvements + +* (proto) [\#5987](https://github.com/cosmos/ibc-go/pull/5987) Add wasm proto files. + +## [v8.1.0](https://github.com/cosmos/ibc-go/releases/tag/v8.1.0) - 2024-01-31 + +### Dependencies + +* [\#5663](https://github.com/cosmos/ibc-go/pull/5663) Bump Cosmos SDK to v0.50.3 and CometBFT to v0.38.2. + +### State Machine Breaking + +* (apps/27-interchain-accounts) [\#5442](https://github.com/cosmos/ibc-go/pull/5442) Increase the maximum allowed length for the memo field of `InterchainAccountPacketData`. + +### Improvements + +* (core/02-client) [\#5429](https://github.com/cosmos/ibc-go/pull/5429) Add wildcard `"*"` to allow all clients in `AllowedClients` param. +* (core) [\#5541](https://github.com/cosmos/ibc-go/pull/5541) Enable emission of events on erroneous IBC application callbacks by appending a prefix to all event type and attribute keys. + +### Features + +* (core/04-channel) [\#1613](https://github.com/cosmos/ibc-go/pull/1613) Channel upgradability. +* (apps/transfer) [\#5280](https://github.com/cosmos/ibc-go/pull/5280) Add list of allowed packet data keys to `Allocation` of `TransferAuthorization`. +* (apps/27-interchain-accounts) [\#5633](https://github.com/cosmos/ibc-go/pull/5633) Allow setting new and upgrading existing ICA (ordered) channels to use unordered ordering. + +### Bug Fixes + +* (apps/27-interchain-accounts) [\#5343](https://github.com/cosmos/ibc-go/pull/5343) Add check if controller is enabled in `sendTx` before sending packet to host. +* (apps/29-fee) [\#5441](https://github.com/cosmos/ibc-go/pull/5441) Allow setting the relayer address as payee. + +## [v8.0.1](https://github.com/cosmos/ibc-go/releases/tag/v8.0.1) - 2024-01-31 + +### Dependencies + +* [\#5718](https://github.com/cosmos/ibc-go/pull/5718) Update Cosmos SDK to v0.50.3 and CometBFT to v0.38.2. + +### Improvements + +* (core) [\#5541](https://github.com/cosmos/ibc-go/pull/5541) Enable emission of events on erroneous IBC application callbacks by appending a prefix to all event type and attribute keys. + +## [v8.0.0](https://github.com/cosmos/ibc-go/releases/tag/v8.0.0) - 2023-11-10 + +### Dependencies + +* [\#5038](https://github.com/cosmos/ibc-go/pull/5038) Bump SDK v0.50.1 and cometBFT v0.38. +* [\#4398](https://github.com/cosmos/ibc-go/pull/4398) Update all modules to go 1.21. + +### API Breaking + +* (core) [\#4703](https://github.com/cosmos/ibc-go/pull/4703) Make `PortKeeper` field of `IBCKeeper` a pointer. +* (core/23-commitment) [\#4459](https://github.com/cosmos/ibc-go/pull/4459) Remove `Pretty` and `String` custom implementations of `MerklePath`. +* [\#3205](https://github.com/cosmos/ibc-go/pull/3205) Make event emission functions unexported. +* (apps/27-interchain-accounts, apps/transfer) [\#3253](https://github.com/cosmos/ibc-go/pull/3253) Rename `IsBound` to `HasCapability`. +* (apps/27-interchain-accounts, apps/transfer) [\#3303](https://github.com/cosmos/ibc-go/pull/3303) Make `HasCapability` private. +* [\#3303](https://github.com/cosmos/ibc-go/pull/3856) Add missing/remove unnecessary gogoproto directive. +* (apps/27-interchain-accounts) [\#3967](https://github.com/cosmos/ibc-go/pull/3967) Add encoding type as argument to ICA encoding/decoding functions. +* (core) [\#3867](https://github.com/cosmos/ibc-go/pull/3867) Remove unnecessary event attribute from INIT handshake msgs. +* (core/04-channel) [\#3806](https://github.com/cosmos/ibc-go/pull/3806) Remove unused `EventTypeTimeoutPacketOnClose`. +* (testing) [\#4018](https://github.com/cosmos/ibc-go/pull/4018) Allow failure expectations when using `chain.SendMsgs`. + +### State Machine Breaking + +* (apps/transfer, apps/27-interchain-accounts, app/29-fee) [\#4992](https://github.com/cosmos/ibc-go/pull/4992) Set validation for length of string fields. + +### Improvements + +* [\#3304](https://github.com/cosmos/ibc-go/pull/3304) Remove unnecessary defer func statements. +* (apps/29-fee) [\#3054](https://github.com/cosmos/ibc-go/pull/3054) Add page result to ics29-fee queries. +* (apps/27-interchain-accounts, apps/transfer) [\#3077](https://github.com/cosmos/ibc-go/pull/3077) Add debug level logging for the error message which is discarded when generating a failed acknowledgement. +* (core/03-connection) [\#3244](https://github.com/cosmos/ibc-go/pull/3244) Cleanup 03-connection msg validate basic test. +* (core/02-client) [\#3514](https://github.com/cosmos/ibc-go/pull/3514) Add check for the client status in `CreateClient`. +* (apps/29-fee) [\#4100](https://github.com/cosmos/ibc-go/pull/4100) Adding `MetadataFromVersion` to `29-fee` package `types`. +* (apps/29-fee) [\#4290](https://github.com/cosmos/ibc-go/pull/4290) Use `types.MetadataFromVersion` helper function for callback handlers. +* (core/04-channel) [\#4155](https://github.com/cosmos/ibc-go/pull/4155) Adding `IsOpen` and `IsClosed` methods to `Channel` type. +* (core/03-connection) [\#4110](https://github.com/cosmos/ibc-go/pull/4110) Remove `Version` interface and casting functions from 03-connection. +* (core) [\#4835](https://github.com/cosmos/ibc-go/pull/4835) Use expected interface for legacy params subspace parameter of keeper constructor functions. + +### Features + +* (capability) [\#3097](https://github.com/cosmos/ibc-go/pull/3097) Migrate capability module from Cosmos SDK to ibc-go. +* (core/02-client) [\#3640](https://github.com/cosmos/ibc-go/pull/3640) Migrate client params to be self managed. +* (core/03-connection) [\#3650](https://github.com/cosmos/ibc-go/pull/3650) Migrate connection params to be self managed. +* (apps/transfer) [\#3553](https://github.com/cosmos/ibc-go/pull/3553) Migrate transfer parameters to be self managed (#3553) +* (apps/27-interchain-accounts) [\#3520](https://github.com/cosmos/ibc-go/pull/3590) Migrate ica/controller parameters to be self managed (#3590) +* (apps/27-interchain-accounts) [\#3520](https://github.com/cosmos/ibc-go/pull/3520) Migrate ica/host to params to be self managed. +* (apps/transfer) [\#3104](https://github.com/cosmos/ibc-go/pull/3104) Add metadata for IBC tokens. +* [\#4620](https://github.com/cosmos/ibc-go/pull/4620) Migrate to gov v1 via the additions of `MsgRecoverClient` and `MsgIBCSoftwareUpgrade`. The legacy proposal types `ClientUpdateProposal` and `UpgradeProposal` have been deprecated and will be removed in the next major release. + +### Bug Fixes + +* (apps/transfer) [\#4709](https://github.com/cosmos/ibc-go/pull/4709) Order query service RPCs to fix availability of denom traces endpoint when no args are provided. +* (core/04-channel) [\#3357](https://github.com/cosmos/ibc-go/pull/3357) Handle unordered channels in `NextSequenceReceive` query. +* (e2e) [\#3402](https://github.com/cosmos/ibc-go/pull/3402 Allow retries for messages signed by relayer. +* (core/04-channel) [\#3417](https://github.com/cosmos/ibc-go/pull/3417) Add missing query for next sequence send. +* (testing) [\#4630](https://github.com/cosmos/ibc-go/pull/4630) Update `testconfig` to use revision formatted chain IDs. +* (core/04-channel) [\#4706](https://github.com/cosmos/ibc-go/pull/4706) Retrieve correct next send sequence for packets in unordered channels. +* (core/02-client) [\#4746](https://github.com/cosmos/ibc-go/pull/4746) Register implementations against `govtypes.Content` interface. +* (apps/27-interchain-accounts) [\#4944](https://github.com/cosmos/ibc-go/pull/4944) Add missing proto interface registration. +* (core/02-client) [\#5020](https://github.com/cosmos/ibc-go/pull/5020) Fix expect pointer error when unmarshalling misbehaviour file. + +### Documentation + +* [\#3133](https://github.com/cosmos/ibc-go/pull/3133) Add linter for markdown documents. +* [\#4693](https://github.com/cosmos/ibc-go/pull/4693) Migrate docs to docusaurus. + +### Testing + +* [\#3138](https://github.com/cosmos/ibc-go/pull/3138) Use `testing.TB` instead of `testing.T` to support benchmarks and fuzz tests. +* [\#3980](https://github.com/cosmos/ibc-go/pull/3980) Change `sdk.Events` usage to `[]abci.Event` in the testing package. +* [\#3986](https://github.com/cosmos/ibc-go/pull/3986) Add function `RelayPacketWithResults`. +* [\#4182](https://github.com/cosmos/ibc-go/pull/4182) Return current validator set when requesting current height in `GetValsAtHeight`. +* [\#4319](https://github.com/cosmos/ibc-go/pull/4319) Fix in `TimeoutPacket` function to use counterparty `portID`/`channelID` in `GetNextSequenceRecv` query. +* [\#4180](https://github.com/cosmos/ibc-go/pull/4180) Remove unused function `simapp.SetupWithGenesisAccounts`. + +### Miscellaneous Tasks + +* (apps/27-interchain-accounts) [\#4677](https://github.com/cosmos/ibc-go/pull/4677) Remove ica store key. +* [\#4724](https://github.com/cosmos/ibc-go/pull/4724) Add `HasValidateBasic` compiler assertions to messages. +* [\#4725](https://github.com/cosmos/ibc-go/pull/4725) Add fzf selection for config files. +* [\#4741](https://github.com/cosmos/ibc-go/pull/4741) Panic with error. +* [\#3186](https://github.com/cosmos/ibc-go/pull/3186) Migrate all SDK errors to the new errors go module. +* [\#3216](https://github.com/cosmos/ibc-go/pull/3216) Modify `simapp` to fulfill the SDK `runtime.AppI` interface. +* [\#3290](https://github.com/cosmos/ibc-go/pull/3290) Remove `gogoproto` yaml tags from proto files. +* [\#3439](https://github.com/cosmos/ibc-go/pull/3439) Use nil pointer pattern to check for interface compliance. +* [\#3433](https://github.com/cosmos/ibc-go/pull/3433) Add tests for `acknowledgement.Acknowledgement()`. +* (core, apps/29-fee) [\#3462](https://github.com/cosmos/ibc-go/pull/3462) Add missing `nil` check and corresponding tests for query handlers. +* (light-clients/07-tendermint, light-clients/06-solomachine) [\#3571](https://github.com/cosmos/ibc-go/pull/3571) Delete unused `GetProofSpecs` functions. +* (core) [\#3616](https://github.com/cosmos/ibc-go/pull/3616) Add debug log for redundant relay. +* (core) [\#3892](https://github.com/cosmos/ibc-go/pull/3892) Add deprecated option to `create_localhost` field. +* (core) [\#3893](https://github.com/cosmos/ibc-go/pull/3893) Add deprecated option to `MsgSubmitMisbehaviour`. +* (apps/transfer, apps/29-fee) [\#4570](https://github.com/cosmos/ibc-go/pull/4570) Remove `GetSignBytes` from 29-fee and transfer msgs. +* [\#3630](https://github.com/cosmos/ibc-go/pull/3630) Add annotation to Msg service. + +>>>>>>> 500765e4 (fix: delete already refunded fees from state if some fee cannot be refunded on channel closure (#6255)) ## [v7.4.0](https://github.com/cosmos/ibc-go/releases/tag/v7.4.0) - 2024-04-05 ## [v7.3.2](https://github.com/cosmos/ibc-go/releases/tag/v7.3.2) - 2024-01-31 diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index d32ec6baae4..5976b58728d 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -189,7 +189,7 @@ 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 _, packetFee := range identifiedPacketFee.PacketFees { if !k.EscrowAccountHasBalance(cacheCtx, packetFee.Fee.Total()) { @@ -207,18 +207,22 @@ func (k Keeper) RefundFeesOnChannelClosure(ctx sdk.Context, portID, channelID st 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) + } 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 bcb6c473596..20b02876568 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -313,19 +313,17 @@ func (suite *KeeperTestSuite) TestDistributePacketFeesOnTimeout() { func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { var ( - expIdentifiedPacketFees []types.IdentifiedPacketFees - expEscrowBal sdk.Coins - expRefundBal sdk.Coins - refundAcc sdk.AccAddress - fee types.Fee - locked bool - expectEscrowFeesToBeDeleted bool + expIdentifiedPacketFees []types.IdentifiedPacketFees + expEscrowBal sdk.Coins + expRefundBal sdk.Coins + refundAcc sdk.AccAddress + fee types.Fee ) testCases := []struct { name string malleate func() - expPass bool + locked bool }{ { "success", func() { @@ -333,16 +331,13 @@ 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) } - }, true, + }, false, }, { "success with undistributed packet fees on a different channel", func() { @@ -350,14 +345,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 @@ -371,12 +362,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)}) @@ -385,13 +374,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)) @@ -412,42 +398,46 @@ 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, "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()) + // 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) - expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFees} + // 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() expRefundBal = expRefundBal.Sub(fee.Total()...) - }, true, + }, false, }, { "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 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()) + // 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) - expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFees} + // 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() expRefundBal = expRefundBal.Sub(fee.Total()...) - }, true, + }, false, }, } @@ -455,12 +445,16 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { tc := tc suite.Run(tc.name, func() { +<<<<<<< HEAD suite.SetupTest() // reset suite.coordinator.Setup(suite.path) // setup channel expIdentifiedPacketFees = []types.IdentifiedPacketFees{} +======= + suite.SetupTest() // reset + suite.path.Setup() // setup channel + expIdentifiedPacketFees = []types.IdentifiedPacketFees(nil) +>>>>>>> 500765e4 (fix: delete already refunded fees from state if some fee cannot be refunded on channel closure (#6255)) expEscrowBal = sdk.Coins{} - locked = false - expectEscrowFeesToBeDeleted = true // setup refundAcc = suite.chainA.SenderAccount.GetAddress() @@ -483,32 +477,22 @@ 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(expIdentifiedPacketFees, suite.chainA.GetSimApp().IBCFeeKeeper.GetIdentifiedPacketFeesForChannel(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)) - 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) - - // 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 - suite.Require().Equal(expectEscrowFeesToBeDeleted, len(suite.chainA.GetSimApp().IBCFeeKeeper.GetIdentifiedPacketFeesForChannel(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)) == 0) } }) } From 47a57618879c2ef98c8462e6313ca791013442bb Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 7 May 2024 23:29:46 +0200 Subject: [PATCH 2/3] fix conflicts --- CHANGELOG.md | 145 ---------------------- modules/apps/29-fee/keeper/escrow_test.go | 6 - 2 files changed, 151 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6eff8546002..4ca36a43c73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,151 +57,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes -<<<<<<< HEAD -======= -* (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 - -### Dependencies - -* [\#5975](https://github.com/cosmos/ibc-go/pull/5975) Bump Cosmos SDK to v0.50.5. - -### Improvements - -* (proto) [\#5987](https://github.com/cosmos/ibc-go/pull/5987) Add wasm proto files. - -## [v8.1.0](https://github.com/cosmos/ibc-go/releases/tag/v8.1.0) - 2024-01-31 - -### Dependencies - -* [\#5663](https://github.com/cosmos/ibc-go/pull/5663) Bump Cosmos SDK to v0.50.3 and CometBFT to v0.38.2. - -### State Machine Breaking - -* (apps/27-interchain-accounts) [\#5442](https://github.com/cosmos/ibc-go/pull/5442) Increase the maximum allowed length for the memo field of `InterchainAccountPacketData`. - -### Improvements - -* (core/02-client) [\#5429](https://github.com/cosmos/ibc-go/pull/5429) Add wildcard `"*"` to allow all clients in `AllowedClients` param. -* (core) [\#5541](https://github.com/cosmos/ibc-go/pull/5541) Enable emission of events on erroneous IBC application callbacks by appending a prefix to all event type and attribute keys. - -### Features - -* (core/04-channel) [\#1613](https://github.com/cosmos/ibc-go/pull/1613) Channel upgradability. -* (apps/transfer) [\#5280](https://github.com/cosmos/ibc-go/pull/5280) Add list of allowed packet data keys to `Allocation` of `TransferAuthorization`. -* (apps/27-interchain-accounts) [\#5633](https://github.com/cosmos/ibc-go/pull/5633) Allow setting new and upgrading existing ICA (ordered) channels to use unordered ordering. - -### Bug Fixes - -* (apps/27-interchain-accounts) [\#5343](https://github.com/cosmos/ibc-go/pull/5343) Add check if controller is enabled in `sendTx` before sending packet to host. -* (apps/29-fee) [\#5441](https://github.com/cosmos/ibc-go/pull/5441) Allow setting the relayer address as payee. - -## [v8.0.1](https://github.com/cosmos/ibc-go/releases/tag/v8.0.1) - 2024-01-31 - -### Dependencies - -* [\#5718](https://github.com/cosmos/ibc-go/pull/5718) Update Cosmos SDK to v0.50.3 and CometBFT to v0.38.2. - -### Improvements - -* (core) [\#5541](https://github.com/cosmos/ibc-go/pull/5541) Enable emission of events on erroneous IBC application callbacks by appending a prefix to all event type and attribute keys. - -## [v8.0.0](https://github.com/cosmos/ibc-go/releases/tag/v8.0.0) - 2023-11-10 - -### Dependencies - -* [\#5038](https://github.com/cosmos/ibc-go/pull/5038) Bump SDK v0.50.1 and cometBFT v0.38. -* [\#4398](https://github.com/cosmos/ibc-go/pull/4398) Update all modules to go 1.21. - -### API Breaking - -* (core) [\#4703](https://github.com/cosmos/ibc-go/pull/4703) Make `PortKeeper` field of `IBCKeeper` a pointer. -* (core/23-commitment) [\#4459](https://github.com/cosmos/ibc-go/pull/4459) Remove `Pretty` and `String` custom implementations of `MerklePath`. -* [\#3205](https://github.com/cosmos/ibc-go/pull/3205) Make event emission functions unexported. -* (apps/27-interchain-accounts, apps/transfer) [\#3253](https://github.com/cosmos/ibc-go/pull/3253) Rename `IsBound` to `HasCapability`. -* (apps/27-interchain-accounts, apps/transfer) [\#3303](https://github.com/cosmos/ibc-go/pull/3303) Make `HasCapability` private. -* [\#3303](https://github.com/cosmos/ibc-go/pull/3856) Add missing/remove unnecessary gogoproto directive. -* (apps/27-interchain-accounts) [\#3967](https://github.com/cosmos/ibc-go/pull/3967) Add encoding type as argument to ICA encoding/decoding functions. -* (core) [\#3867](https://github.com/cosmos/ibc-go/pull/3867) Remove unnecessary event attribute from INIT handshake msgs. -* (core/04-channel) [\#3806](https://github.com/cosmos/ibc-go/pull/3806) Remove unused `EventTypeTimeoutPacketOnClose`. -* (testing) [\#4018](https://github.com/cosmos/ibc-go/pull/4018) Allow failure expectations when using `chain.SendMsgs`. - -### State Machine Breaking - -* (apps/transfer, apps/27-interchain-accounts, app/29-fee) [\#4992](https://github.com/cosmos/ibc-go/pull/4992) Set validation for length of string fields. - -### Improvements - -* [\#3304](https://github.com/cosmos/ibc-go/pull/3304) Remove unnecessary defer func statements. -* (apps/29-fee) [\#3054](https://github.com/cosmos/ibc-go/pull/3054) Add page result to ics29-fee queries. -* (apps/27-interchain-accounts, apps/transfer) [\#3077](https://github.com/cosmos/ibc-go/pull/3077) Add debug level logging for the error message which is discarded when generating a failed acknowledgement. -* (core/03-connection) [\#3244](https://github.com/cosmos/ibc-go/pull/3244) Cleanup 03-connection msg validate basic test. -* (core/02-client) [\#3514](https://github.com/cosmos/ibc-go/pull/3514) Add check for the client status in `CreateClient`. -* (apps/29-fee) [\#4100](https://github.com/cosmos/ibc-go/pull/4100) Adding `MetadataFromVersion` to `29-fee` package `types`. -* (apps/29-fee) [\#4290](https://github.com/cosmos/ibc-go/pull/4290) Use `types.MetadataFromVersion` helper function for callback handlers. -* (core/04-channel) [\#4155](https://github.com/cosmos/ibc-go/pull/4155) Adding `IsOpen` and `IsClosed` methods to `Channel` type. -* (core/03-connection) [\#4110](https://github.com/cosmos/ibc-go/pull/4110) Remove `Version` interface and casting functions from 03-connection. -* (core) [\#4835](https://github.com/cosmos/ibc-go/pull/4835) Use expected interface for legacy params subspace parameter of keeper constructor functions. - -### Features - -* (capability) [\#3097](https://github.com/cosmos/ibc-go/pull/3097) Migrate capability module from Cosmos SDK to ibc-go. -* (core/02-client) [\#3640](https://github.com/cosmos/ibc-go/pull/3640) Migrate client params to be self managed. -* (core/03-connection) [\#3650](https://github.com/cosmos/ibc-go/pull/3650) Migrate connection params to be self managed. -* (apps/transfer) [\#3553](https://github.com/cosmos/ibc-go/pull/3553) Migrate transfer parameters to be self managed (#3553) -* (apps/27-interchain-accounts) [\#3520](https://github.com/cosmos/ibc-go/pull/3590) Migrate ica/controller parameters to be self managed (#3590) -* (apps/27-interchain-accounts) [\#3520](https://github.com/cosmos/ibc-go/pull/3520) Migrate ica/host to params to be self managed. -* (apps/transfer) [\#3104](https://github.com/cosmos/ibc-go/pull/3104) Add metadata for IBC tokens. -* [\#4620](https://github.com/cosmos/ibc-go/pull/4620) Migrate to gov v1 via the additions of `MsgRecoverClient` and `MsgIBCSoftwareUpgrade`. The legacy proposal types `ClientUpdateProposal` and `UpgradeProposal` have been deprecated and will be removed in the next major release. - -### Bug Fixes - -* (apps/transfer) [\#4709](https://github.com/cosmos/ibc-go/pull/4709) Order query service RPCs to fix availability of denom traces endpoint when no args are provided. -* (core/04-channel) [\#3357](https://github.com/cosmos/ibc-go/pull/3357) Handle unordered channels in `NextSequenceReceive` query. -* (e2e) [\#3402](https://github.com/cosmos/ibc-go/pull/3402 Allow retries for messages signed by relayer. -* (core/04-channel) [\#3417](https://github.com/cosmos/ibc-go/pull/3417) Add missing query for next sequence send. -* (testing) [\#4630](https://github.com/cosmos/ibc-go/pull/4630) Update `testconfig` to use revision formatted chain IDs. -* (core/04-channel) [\#4706](https://github.com/cosmos/ibc-go/pull/4706) Retrieve correct next send sequence for packets in unordered channels. -* (core/02-client) [\#4746](https://github.com/cosmos/ibc-go/pull/4746) Register implementations against `govtypes.Content` interface. -* (apps/27-interchain-accounts) [\#4944](https://github.com/cosmos/ibc-go/pull/4944) Add missing proto interface registration. -* (core/02-client) [\#5020](https://github.com/cosmos/ibc-go/pull/5020) Fix expect pointer error when unmarshalling misbehaviour file. - -### Documentation - -* [\#3133](https://github.com/cosmos/ibc-go/pull/3133) Add linter for markdown documents. -* [\#4693](https://github.com/cosmos/ibc-go/pull/4693) Migrate docs to docusaurus. - -### Testing - -* [\#3138](https://github.com/cosmos/ibc-go/pull/3138) Use `testing.TB` instead of `testing.T` to support benchmarks and fuzz tests. -* [\#3980](https://github.com/cosmos/ibc-go/pull/3980) Change `sdk.Events` usage to `[]abci.Event` in the testing package. -* [\#3986](https://github.com/cosmos/ibc-go/pull/3986) Add function `RelayPacketWithResults`. -* [\#4182](https://github.com/cosmos/ibc-go/pull/4182) Return current validator set when requesting current height in `GetValsAtHeight`. -* [\#4319](https://github.com/cosmos/ibc-go/pull/4319) Fix in `TimeoutPacket` function to use counterparty `portID`/`channelID` in `GetNextSequenceRecv` query. -* [\#4180](https://github.com/cosmos/ibc-go/pull/4180) Remove unused function `simapp.SetupWithGenesisAccounts`. - -### Miscellaneous Tasks - -* (apps/27-interchain-accounts) [\#4677](https://github.com/cosmos/ibc-go/pull/4677) Remove ica store key. -* [\#4724](https://github.com/cosmos/ibc-go/pull/4724) Add `HasValidateBasic` compiler assertions to messages. -* [\#4725](https://github.com/cosmos/ibc-go/pull/4725) Add fzf selection for config files. -* [\#4741](https://github.com/cosmos/ibc-go/pull/4741) Panic with error. -* [\#3186](https://github.com/cosmos/ibc-go/pull/3186) Migrate all SDK errors to the new errors go module. -* [\#3216](https://github.com/cosmos/ibc-go/pull/3216) Modify `simapp` to fulfill the SDK `runtime.AppI` interface. -* [\#3290](https://github.com/cosmos/ibc-go/pull/3290) Remove `gogoproto` yaml tags from proto files. -* [\#3439](https://github.com/cosmos/ibc-go/pull/3439) Use nil pointer pattern to check for interface compliance. -* [\#3433](https://github.com/cosmos/ibc-go/pull/3433) Add tests for `acknowledgement.Acknowledgement()`. -* (core, apps/29-fee) [\#3462](https://github.com/cosmos/ibc-go/pull/3462) Add missing `nil` check and corresponding tests for query handlers. -* (light-clients/07-tendermint, light-clients/06-solomachine) [\#3571](https://github.com/cosmos/ibc-go/pull/3571) Delete unused `GetProofSpecs` functions. -* (core) [\#3616](https://github.com/cosmos/ibc-go/pull/3616) Add debug log for redundant relay. -* (core) [\#3892](https://github.com/cosmos/ibc-go/pull/3892) Add deprecated option to `create_localhost` field. -* (core) [\#3893](https://github.com/cosmos/ibc-go/pull/3893) Add deprecated option to `MsgSubmitMisbehaviour`. -* (apps/transfer, apps/29-fee) [\#4570](https://github.com/cosmos/ibc-go/pull/4570) Remove `GetSignBytes` from 29-fee and transfer msgs. -* [\#3630](https://github.com/cosmos/ibc-go/pull/3630) Add annotation to Msg service. - ->>>>>>> 500765e4 (fix: delete already refunded fees from state if some fee cannot be refunded on channel closure (#6255)) ## [v7.4.0](https://github.com/cosmos/ibc-go/releases/tag/v7.4.0) - 2024-04-05 ## [v7.3.2](https://github.com/cosmos/ibc-go/releases/tag/v7.3.2) - 2024-01-31 diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index 20b02876568..543b0b2d881 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -445,15 +445,9 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() { tc := tc suite.Run(tc.name, func() { -<<<<<<< HEAD suite.SetupTest() // reset suite.coordinator.Setup(suite.path) // setup channel - expIdentifiedPacketFees = []types.IdentifiedPacketFees{} -======= - suite.SetupTest() // reset - suite.path.Setup() // setup channel expIdentifiedPacketFees = []types.IdentifiedPacketFees(nil) ->>>>>>> 500765e4 (fix: delete already refunded fees from state if some fee cannot be refunded on channel closure (#6255)) expEscrowBal = sdk.Coins{} // setup From 4f5c1821ac850a77d0ccd77fd48c61767a64e2ae Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 7 May 2024 23:37:41 +0200 Subject: [PATCH 3/3] add import --- modules/apps/29-fee/keeper/escrow_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index 543b0b2d881..eceb2ffb113 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + sdkmath "cosmossdk.io/math" "github.com/cometbft/cometbft/crypto/secp256k1" sdk "github.com/cosmos/cosmos-sdk/types"