From f11db42e4d066077f9f6d0b986061956cd25c695 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 7 May 2024 10:03:41 +0200 Subject: [PATCH 1/6] chore: disable failing grandpa tests due to hyperspace event attr parsing (#6259) --- .github/workflows/e2e-wasm.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/e2e-wasm.yaml b/.github/workflows/e2e-wasm.yaml index e75da52a156..c6cd042584d 100644 --- a/.github/workflows/e2e-wasm.yaml +++ b/.github/workflows/e2e-wasm.yaml @@ -65,3 +65,5 @@ jobs: chain-binary: 'simd' # only run the grandpa test suite for wasm tests. test-entry-point: 'TestGrandpaTestSuite' + # exclude transfer tests which rely on removed packet event attributes: # https://github.com/cosmos/ibc-go/issues/6243 + test-exclusions: 'TestMsgTransfer_Succeeds_GrandpaContract,TestMsgTransfer_TimesOut_GrandpaContract' From d34bf44b8636136cfdd559e87587b86a122b3409 Mon Sep 17 00:00:00 2001 From: Hoa Nguyen Date: Tue, 7 May 2024 15:13:47 +0700 Subject: [PATCH 2/6] chore: reduce allocations (#6233) * refactor: improve perf * review comment * rollback --------- Co-authored-by: Carlos Rodriguez Co-authored-by: Damian Nolan --- modules/apps/transfer/types/transfer_authorization.go | 2 +- modules/light-clients/08-wasm/keeper/keeper.go | 2 +- modules/light-clients/08-wasm/light_client_module.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 242bc86e801..0683166cc8b 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -188,7 +188,7 @@ func validateMemo(ctx sdk.Context, memo string, allowedPacketDataList []string) } } - var keys []string + keys := make([]string, 0, len(jsonObject)) for k := range jsonObject { keys = append(keys, k) } diff --git a/modules/light-clients/08-wasm/keeper/keeper.go b/modules/light-clients/08-wasm/keeper/keeper.go index 69ac2e2a2cd..3c3995dafb9 100644 --- a/modules/light-clients/08-wasm/keeper/keeper.go +++ b/modules/light-clients/08-wasm/keeper/keeper.go @@ -213,7 +213,7 @@ func (k Keeper) GetAllChecksums(ctx context.Context) ([]types.Checksum, error) { return nil, err } - checksums := []types.Checksum{} + checksums := make([]types.Checksum, 0, len(keys)) for _, key := range keys { checksums = append(checksums, key) } diff --git a/modules/light-clients/08-wasm/light_client_module.go b/modules/light-clients/08-wasm/light_client_module.go index 1467a6834ed..8e44931ef15 100644 --- a/modules/light-clients/08-wasm/light_client_module.go +++ b/modules/light-clients/08-wasm/light_client_module.go @@ -205,7 +205,7 @@ func (l LightClientModule) UpdateState(ctx sdk.Context, clientID string, clientM panic(errorsmod.Wrap(types.ErrWasmInvalidResponseData, err.Error())) } - heights := []exported.Height{} + heights := make([]exported.Height, 0, len(result.Heights)) for _, height := range result.Heights { heights = append(heights, height) } From 7f274d5a4f1ddf998c3acf8fd822e5c4a1884f05 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Tue, 7 May 2024 15:50:45 +0700 Subject: [PATCH 3/6] chore(api!): move additional methods to light client module (#6230) * move methods to light client module * remove status function from client state * update changelog + migration docs * godoc: update godoc of solomachine status method --------- Co-authored-by: Carlos Rodriguez Co-authored-by: Damian Nolan --- CHANGELOG.md | 1 + docs/docs/05-migrations/13-v8-to-v9.md | 2 +- .../06-solomachine/client_state.go | 22 ------------------ .../06-solomachine/light_client_module.go | 23 ++++++++++++++----- .../light-clients/06-solomachine/update.go | 8 ------- 5 files changed, 19 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 385550813bf..34a61ca83da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (core/02-client, light-clients) [\#5806](https://github.com/cosmos/ibc-go/pull/5806) Decouple light client routing from their encoding structure. * (core/04-channel) [\#5991](https://github.com/cosmos/ibc-go/pull/5991) The client CLI `QueryLatestConsensusState` has been removed. * (light-clients/06-solomachine) [\#6037](https://github.com/cosmos/ibc-go/pull/6037) Remove `Initialize` function from `ClientState` and move logic to `Initialize` function of `LightClientModule`. +* (light-clients/06-solomachine) [\#6230](https://github.com/cosmos/ibc-go/pull/6230) Remove `GetTimestampAtHeight`, `Status` and `UpdateStateOnMisbehaviour` functions from `ClientState` and move logic to functions of `LightClientModule`. * (core/02-client) [\#6084](https://github.com/cosmos/ibc-go/pull/6084) Removed `stakingKeeper` as an argument to `NewKeeper` and replaced with a `ConsensusHost` implementation. * (testing) [\#6070](https://github.com/cosmos/ibc-go/pull/6070) Remove `AssertEventsLegacy` function. * (core) [\#6138](https://github.com/cosmos/ibc-go/pull/6138) Remove `Router` reference from IBC core keeper and use instead the router on the existing `PortKeeper` reference. diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md index 0012683f295..b3e03747167 100644 --- a/docs/docs/05-migrations/13-v8-to-v9.md +++ b/docs/docs/05-migrations/13-v8-to-v9.md @@ -118,7 +118,7 @@ Please check also the [Light client developer guide](../03-light-clients/01-deve ### 06-solomachine -The `Initialize` function in `ClientState` has been removed and all its logic has been moved to the implementation of the `LightClientModule` interface `Initialize` function. +The `Initialize`, `Status`, `GetTimestampAtHeight` and `UpdateStateOnMisbehaviour` functions in `ClientState` have been removed and all their logic has been moved to functions of the `LightClientModule`. ### 07-tendermint diff --git a/modules/light-clients/06-solomachine/client_state.go b/modules/light-clients/06-solomachine/client_state.go index 2b816a9dfb7..43d30fc4577 100644 --- a/modules/light-clients/06-solomachine/client_state.go +++ b/modules/light-clients/06-solomachine/client_state.go @@ -32,28 +32,6 @@ func (ClientState) ClientType() string { return exported.Solomachine } -// GetTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height. -func (cs ClientState) GetTimestampAtHeight( - _ sdk.Context, - clientStore storetypes.KVStore, - cdc codec.BinaryCodec, - height exported.Height, -) (uint64, error) { - return cs.ConsensusState.Timestamp, nil -} - -// Status returns the status of the solo machine client. -// The client may be: -// - Active: if frozen sequence is 0 -// - Frozen: otherwise solo machine is frozen -func (cs ClientState) Status(_ sdk.Context, _ storetypes.KVStore, _ codec.BinaryCodec) exported.Status { - if cs.IsFrozen { - return exported.Frozen - } - - return exported.Active -} - // Validate performs basic validation of the client state fields. func (cs ClientState) Validate() error { if cs.Sequence == 0 { diff --git a/modules/light-clients/06-solomachine/light_client_module.go b/modules/light-clients/06-solomachine/light_client_module.go index 35682f9b911..2e8f2d9fc99 100644 --- a/modules/light-clients/06-solomachine/light_client_module.go +++ b/modules/light-clients/06-solomachine/light_client_module.go @@ -95,7 +95,9 @@ func (l LightClientModule) CheckForMisbehaviour(ctx sdk.Context, clientID string return clientState.CheckForMisbehaviour(ctx, l.cdc, clientStore, clientMsg) } -// UpdateStateOnMisbehaviour obtains the client state associated with the client identifier and calls into the clientState.UpdateStateOnMisbehaviour method. +// UpdateStateOnMisbehaviour updates state upon misbehaviour, freezing the ClientState. +// This method should only be called when misbehaviour is detected as it does not perform +// any misbehaviour checks. // // CONTRACT: clientID is validated in 02-client router, thus clientID is assumed here to have the format 06-solomachine-{n}. func (l LightClientModule) UpdateStateOnMisbehaviour(ctx sdk.Context, clientID string, clientMsg exported.ClientMessage) { @@ -105,7 +107,8 @@ func (l LightClientModule) UpdateStateOnMisbehaviour(ctx sdk.Context, clientID s panic(errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)) } - clientState.UpdateStateOnMisbehaviour(ctx, l.cdc, clientStore, clientMsg) + clientState.IsFrozen = true + setClientState(clientStore, l.cdc, clientState) } // UpdateState obtains the client state associated with the client identifier and calls into the clientState.UpdateState method. @@ -164,7 +167,11 @@ func (l LightClientModule) VerifyNonMembership( return clientState.VerifyNonMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path) } -// Status obtains the client state associated with the client identifier and calls into the clientState.Status method. +// Status returns the status of the solo machine client. +// The client may be: +// - Active: if `IsFrozen` is false. +// - Frozen: if `IsFrozen` is true. +// - Unknown: if the client state associated with the provided client identifier is not found. // // CONTRACT: clientID is validated in 02-client router, thus clientID is assumed here to have the format 06-solomachine-{n}. func (l LightClientModule) Status(ctx sdk.Context, clientID string) exported.Status { @@ -174,7 +181,11 @@ func (l LightClientModule) Status(ctx sdk.Context, clientID string) exported.Sta return exported.Unknown } - return clientState.Status(ctx, clientStore, l.cdc) + if clientState.IsFrozen { + return exported.Frozen + } + + return exported.Active } // LatestHeight returns the latest height for the client state for the given client identifier. @@ -193,7 +204,7 @@ func (l LightClientModule) LatestHeight(ctx sdk.Context, clientID string) export return clienttypes.NewHeight(0, clientState.Sequence) } -// TimestampAtHeight obtains the client state associated with the client identifier and calls into the clientState.GetTimestampAtHeight method. +// TimestampAtHeight obtains the client state associated with the client identifier and returns the timestamp in nanoseconds of the consensus state at the given height. // // CONTRACT: clientID is validated in 02-client router, thus clientID is assumed here to have the format 06-solomachine-{n}. func (l LightClientModule) TimestampAtHeight(ctx sdk.Context, clientID string, height exported.Height) (uint64, error) { @@ -203,7 +214,7 @@ func (l LightClientModule) TimestampAtHeight(ctx sdk.Context, clientID string, h return 0, errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID) } - return clientState.GetTimestampAtHeight(ctx, clientStore, l.cdc, height) + return clientState.ConsensusState.Timestamp, nil } // RecoverClient asserts that the substitute client is a solo machine client. It obtains the client state associated with the diff --git a/modules/light-clients/06-solomachine/update.go b/modules/light-clients/06-solomachine/update.go index d013031ba48..359bb118be4 100644 --- a/modules/light-clients/06-solomachine/update.go +++ b/modules/light-clients/06-solomachine/update.go @@ -99,11 +99,3 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client return []exported.Height{clienttypes.NewHeight(0, cs.Sequence)} } - -// UpdateStateOnMisbehaviour updates state upon misbehaviour. This method should only be called on misbehaviour -// as it does not perform any misbehaviour checks. -func (cs ClientState) UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore storetypes.KVStore, _ exported.ClientMessage) { - cs.IsFrozen = true - - setClientState(clientStore, cdc, &cs) -} From fd2ec0dd702d62cfa2b57b475a095e53558e1918 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 7 May 2024 16:14:51 +0200 Subject: [PATCH 4/6] build(deps): Bump golangci/golangci-lint-action from 5.3.0 to 6.0.0 (#6266) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 5.3.0 to 6.0.0. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](https://github.com/golangci/golangci-lint-action/compare/v5.3.0...v6.0.0) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/callbacks.yml | 2 +- .github/workflows/e2emodule.yml | 2 +- .github/workflows/golangci-feature.yml | 2 +- .github/workflows/golangci.yml | 2 +- .github/workflows/wasm-client.yml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/callbacks.yml b/.github/workflows/callbacks.yml index ba5deb99eac..031c7cbd146 100644 --- a/.github/workflows/callbacks.yml +++ b/.github/workflows/callbacks.yml @@ -16,7 +16,7 @@ jobs: with: go-version: '1.21' - uses: actions/checkout@v4 - - uses: golangci/golangci-lint-action@v5.3.0 + - uses: golangci/golangci-lint-action@v6.0.0 with: version: v1.57.2 args: --timeout 5m diff --git a/.github/workflows/e2emodule.yml b/.github/workflows/e2emodule.yml index 5b03d2b7d4a..1c7c275c7a7 100644 --- a/.github/workflows/e2emodule.yml +++ b/.github/workflows/e2emodule.yml @@ -14,7 +14,7 @@ jobs: with: go-version: '1.21' - uses: actions/checkout@v4 - - uses: golangci/golangci-lint-action@v5.3.0 + - uses: golangci/golangci-lint-action@v6.0.0 with: version: v1.57.2 args: --timeout 5m diff --git a/.github/workflows/golangci-feature.yml b/.github/workflows/golangci-feature.yml index f769b7384f3..edc6650f3a7 100644 --- a/.github/workflows/golangci-feature.yml +++ b/.github/workflows/golangci-feature.yml @@ -25,7 +25,7 @@ jobs: go-version: '1.21' - uses: actions/checkout@v4 - name: golangci-lint - uses: golangci/golangci-lint-action@v5.3.0 + uses: golangci/golangci-lint-action@v6.0.0 with: version: v1.57.2 args: --timeout 10m diff --git a/.github/workflows/golangci.yml b/.github/workflows/golangci.yml index f24bb6f2abd..04419821bcb 100644 --- a/.github/workflows/golangci.yml +++ b/.github/workflows/golangci.yml @@ -20,7 +20,7 @@ jobs: go-version: '1.21' - uses: actions/checkout@v4 - name: golangci-lint - uses: golangci/golangci-lint-action@v5.3.0 + uses: golangci/golangci-lint-action@v6.0.0 with: version: v1.57.2 args: --timeout 10m diff --git a/.github/workflows/wasm-client.yml b/.github/workflows/wasm-client.yml index 800b7757ed0..a53ae22cb57 100644 --- a/.github/workflows/wasm-client.yml +++ b/.github/workflows/wasm-client.yml @@ -16,7 +16,7 @@ jobs: with: go-version: '1.21' - uses: actions/checkout@v4 - - uses: golangci/golangci-lint-action@v5.3.0 + - uses: golangci/golangci-lint-action@v6.0.0 with: version: v1.57.2 args: --timeout 10m From 9413b4e71ed3cd31de83cbea05925c08b9d1a403 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 7 May 2024 17:29:31 +0200 Subject: [PATCH 5/6] build(deps): Bump google.golang.org/protobuf from 1.34.0 to 1.34.1 (#6265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * build(deps): Bump google.golang.org/protobuf from 1.34.0 to 1.34.1 Bumps google.golang.org/protobuf from 1.34.0 to 1.34.1. --- updated-dependencies: - dependency-name: google.golang.org/protobuf dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] * chore: make tidy-all --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Damian Nolan Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- e2e/go.mod | 2 +- e2e/go.sum | 4 ++-- go.mod | 2 +- go.sum | 4 ++-- modules/apps/callbacks/go.mod | 2 +- modules/apps/callbacks/go.sum | 4 ++-- modules/capability/go.sum | 2 -- modules/light-clients/08-wasm/go.mod | 2 +- modules/light-clients/08-wasm/go.sum | 4 ++-- 9 files changed, 12 insertions(+), 14 deletions(-) diff --git a/e2e/go.mod b/e2e/go.mod index c6d207ba292..ee60f18d4d4 100644 --- a/e2e/go.mod +++ b/e2e/go.mod @@ -242,7 +242,7 @@ require ( google.golang.org/genproto v0.0.0-20240227224415-6ceb2ff114de // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240227224415-6ceb2ff114de // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda // indirect - google.golang.org/protobuf v1.34.0 // indirect + google.golang.org/protobuf v1.34.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/e2e/go.sum b/e2e/go.sum index f3752830089..fdbda98c386 100644 --- a/e2e/go.sum +++ b/e2e/go.sum @@ -1749,8 +1749,8 @@ google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQ google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= google.golang.org/protobuf v1.28.1/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= -google.golang.org/protobuf v1.34.0 h1:Qo/qEd2RZPCf2nKuorzksSknv0d3ERwp1vFG38gSmH4= -google.golang.org/protobuf v1.34.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= +google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg= +google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/go.mod b/go.mod index e4352f51ace..213e9beef33 100644 --- a/go.mod +++ b/go.mod @@ -32,7 +32,7 @@ require ( github.com/stretchr/testify v1.9.0 google.golang.org/genproto/googleapis/api v0.0.0-20240227224415-6ceb2ff114de google.golang.org/grpc v1.63.2 - google.golang.org/protobuf v1.34.0 + google.golang.org/protobuf v1.34.1 gopkg.in/yaml.v2 v2.4.0 ) diff --git a/go.sum b/go.sum index d58148a5502..ac6eaaded6f 100644 --- a/go.sum +++ b/go.sum @@ -1664,8 +1664,8 @@ google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQ google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= google.golang.org/protobuf v1.28.1/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= -google.golang.org/protobuf v1.34.0 h1:Qo/qEd2RZPCf2nKuorzksSknv0d3ERwp1vFG38gSmH4= -google.golang.org/protobuf v1.34.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= +google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg= +google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/modules/apps/callbacks/go.mod b/modules/apps/callbacks/go.mod index 863552939b3..9402fe75c6e 100644 --- a/modules/apps/callbacks/go.mod +++ b/modules/apps/callbacks/go.mod @@ -196,7 +196,7 @@ require ( google.golang.org/genproto/googleapis/api v0.0.0-20240227224415-6ceb2ff114de // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda // indirect google.golang.org/grpc v1.63.2 // indirect - google.golang.org/protobuf v1.34.0 // indirect + google.golang.org/protobuf v1.34.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/modules/apps/callbacks/go.sum b/modules/apps/callbacks/go.sum index d58148a5502..ac6eaaded6f 100644 --- a/modules/apps/callbacks/go.sum +++ b/modules/apps/callbacks/go.sum @@ -1664,8 +1664,8 @@ google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQ google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= google.golang.org/protobuf v1.28.1/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= -google.golang.org/protobuf v1.34.0 h1:Qo/qEd2RZPCf2nKuorzksSknv0d3ERwp1vFG38gSmH4= -google.golang.org/protobuf v1.34.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= +google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg= +google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/modules/capability/go.sum b/modules/capability/go.sum index ab4a2d16eb6..5cede605910 100644 --- a/modules/capability/go.sum +++ b/modules/capability/go.sum @@ -111,8 +111,6 @@ github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZ github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 h1:zuQyyAKVxetITBuuhv3BI9cMrmStnpT18zmgmTxunpo= github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06/go.mod h1:7nc4anLGjupUW/PeY5qiNYsdNXj7zopG+eqsS7To5IQ= github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd/go.mod h1:sE/e/2PUdi/liOCUjSTXgM1o87ZssimdTWN964YiIeI= -github.com/cometbft/cometbft v0.38.6 h1:QSgpCzrGWJ2KUq1qpw+FCfASRpE27T6LQbfEHscdyOk= -github.com/cometbft/cometbft v0.38.6/go.mod h1:8rSPxzUJYquCN8uuBgbUHOMg2KAwvr7CyUw+6ukO4nw= github.com/cometbft/cometbft v0.38.7 h1:ULhIOJ9+LgSy6nLekhq9ae3juX3NnQUMMPyVdhZV6Hk= github.com/cometbft/cometbft v0.38.7/go.mod h1:HIyf811dFMI73IE0F7RrnY/Fr+d1+HuJAgtkEpQjCMY= github.com/cometbft/cometbft-db v0.9.1 h1:MIhVX5ja5bXNHF8EYrThkG9F7r9kSfv8BX4LWaxWJ4M= diff --git a/modules/light-clients/08-wasm/go.mod b/modules/light-clients/08-wasm/go.mod index af3031e8d2a..449c23f731b 100644 --- a/modules/light-clients/08-wasm/go.mod +++ b/modules/light-clients/08-wasm/go.mod @@ -195,7 +195,7 @@ require ( google.golang.org/appengine v1.6.8 // indirect google.golang.org/genproto v0.0.0-20240227224415-6ceb2ff114de // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda // indirect - google.golang.org/protobuf v1.34.0 // indirect + google.golang.org/protobuf v1.34.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/modules/light-clients/08-wasm/go.sum b/modules/light-clients/08-wasm/go.sum index 1002e5c96fb..1e3f204762a 100644 --- a/modules/light-clients/08-wasm/go.sum +++ b/modules/light-clients/08-wasm/go.sum @@ -1666,8 +1666,8 @@ google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQ google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= google.golang.org/protobuf v1.28.1/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= -google.golang.org/protobuf v1.34.0 h1:Qo/qEd2RZPCf2nKuorzksSknv0d3ERwp1vFG38gSmH4= -google.golang.org/protobuf v1.34.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= +google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg= +google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= From 500765e43a2059a80ebac2d596adeaf12d1883c5 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 7 May 2024 23:12:23 +0200 Subject: [PATCH 6/6] 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 --- CHANGELOG.md | 1 + modules/apps/29-fee/keeper/escrow.go | 12 ++-- modules/apps/29-fee/keeper/escrow_test.go | 86 +++++++++-------------- 3 files changed, 41 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34a61ca83da..9df5e0f62a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,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 diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index c3bb9c8fefa..1ebbeb95caa 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 f0d6233d23f..8aa0840784c 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -395,19 +395,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() { @@ -415,16 +413,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() { @@ -432,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 @@ -453,12 +444,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 +456,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)) @@ -494,42 +480,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, }, } @@ -539,10 +529,8 @@ 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{} - locked = false - expectEscrowFeesToBeDeleted = true // setup refundAcc = suite.chainA.SenderAccount.GetAddress() @@ -565,32 +553,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) } }) }