From 5b004f5957ad99ccd759ab6fb30c2a1660de4b0c Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Tue, 9 Jul 2024 13:14:31 +0300 Subject: [PATCH] chore(core/02-client): remove the legacy handler for 02-client. (#6777) * chore(core/02-client): remove the legacy handler for 02-client. * chore: add to changelog, docs. * Update docs/docs/05-migrations/13-v8-to-v9.md Co-authored-by: Carlos Rodriguez --------- Co-authored-by: Carlos Rodriguez --- CHANGELOG.md | 1 + docs/docs/05-migrations/13-v8-to-v9.md | 11 ++- e2e/tests/core/02-client/client_test.go | 80 ---------------- modules/core/02-client/proposal_handler.go | 28 ------ .../core/02-client/proposal_handler_test.go | 94 ------------------- .../08-wasm/testing/simapp/app.go | 4 +- testing/simapp/app.go | 4 +- 7 files changed, 13 insertions(+), 209 deletions(-) delete mode 100644 modules/core/02-client/proposal_handler.go delete mode 100644 modules/core/02-client/proposal_handler_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index cb89d8ab90c..a1b08292819 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (23-commmitment) [\#6644](https://github.com/cosmos/ibc-go/pull/6644) Introduce commitment/v2 `MerklePath` to include `repeated bytes` in favour of `repeated string`. This supports using merkle path keys which include non UTF-8 encoded runes. * (23-commmitment) [\#6633](https://github.com/cosmos/ibc-go/pull/6633) MerklePath has been changed to use `repeated bytes` in favour of `repeated strings`. * (apps/27-interchain-accounts) [\#6749](https://github.com/cosmos/ibc-go/pull/6749) The ICA controller `NewIBCMiddleware` constructor function sets by default the auth module to nil. +* (core/02-client) [\#6777](https://github.com/cosmos/ibc-go/pull/6777) The `NewClientProposalHandler` of `02-client` has been removed. ### State Machine Breaking diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md index d7028f20fec..4c059015cac 100644 --- a/docs/docs/05-migrations/13-v8-to-v9.md +++ b/docs/docs/05-migrations/13-v8-to-v9.md @@ -20,7 +20,15 @@ There are four sections based on the four potential user groups of this document ## Chains -- No relevant changes were made in this release. +Chains will need to remove the route for the legacy proposal handler for `02-client` from their `app/app.go`: + +```diff +// app.go +govRouter.AddRoute(govtypes.RouterKey, govv1beta1.ProposalHandler). +- AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)). +- AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper)) ++ AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)) +``` ## IBC Apps @@ -114,6 +122,7 @@ The base application is then set by default to nil and thus authentication is as - `UnmarshalPacketData` now takes in the context, portID, and channelID. This allows the packet data to be unmarshaled based on the channel version. - `Router` reference has been removed from IBC core keeper: [#6138](https://github.com/cosmos/ibc-go/pull/6138). Please use `PortKeeper.Router` instead. - The function `CreateLocalhostClient` has been removed. The localhost client is now stateless. +- The function `NewClientProposalHandler` has been removed. [#6777](https://github.com/cosmos/ibc-go/pull/6777). ### 02-client diff --git a/e2e/tests/core/02-client/client_test.go b/e2e/tests/core/02-client/client_test.go index 100ed215ce0..72f12d14ddb 100644 --- a/e2e/tests/core/02-client/client_test.go +++ b/e2e/tests/core/02-client/client_test.go @@ -160,86 +160,6 @@ func (s *ClientTestSuite) TestScheduleIBCUpgrade_Succeeds() { }) } -func (s *ClientTestSuite) TestClientUpdateProposal_Succeeds() { - t := s.T() - ctx := context.TODO() - - var ( - pathName string - subjectClientID string - substituteClientID string - // set the trusting period to a value which will still be valid upon client creation, but invalid before the first update - badTrustingPeriod = time.Second * 10 - ) - - testName := t.Name() - relayer := s.CreateDefaultPaths(testName) - - t.Run("create substitute client with correct trusting period", func(t *testing.T) { - // TODO: update when client identifier created is accessible - // currently assumes first client is 07-tendermint-0 - substituteClientID = clienttypes.FormatClientIdentifier(ibcexported.Tendermint, 0) - - pathName = s.GetPaths(testName)[0] - }) - - chainA, chainB := s.GetChains() - chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount) - - t.Run("create subject client with bad trusting period", func(t *testing.T) { - createClientOptions := ibc.CreateClientOptions{ - TrustingPeriod: badTrustingPeriod.String(), - } - - s.SetupClients(ctx, relayer, createClientOptions) - - // TODO: update when client identifier created is accessible - // currently assumes second client is 07-tendermint-1 - subjectClientID = clienttypes.FormatClientIdentifier(ibcexported.Tendermint, 1) - }) - - time.Sleep(badTrustingPeriod) - - t.Run("update substitute client", func(t *testing.T) { - s.UpdateClients(ctx, relayer, pathName) - }) - - s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks") - - t.Run("check status of each client", func(t *testing.T) { - t.Run("substitute should be active", func(t *testing.T) { - status, err := query.ClientStatus(ctx, chainA, substituteClientID) - s.Require().NoError(err) - s.Require().Equal(ibcexported.Active.String(), status) - }) - - t.Run("subject should be expired", func(t *testing.T) { - status, err := query.ClientStatus(ctx, chainA, subjectClientID) - s.Require().NoError(err) - s.Require().Equal(ibcexported.Expired.String(), status) - }) - }) - - t.Run("pass client update proposal", func(t *testing.T) { - proposal := clienttypes.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subjectClientID, substituteClientID) - s.ExecuteAndPassGovV1Beta1Proposal(ctx, chainA, chainAWallet, proposal) - }) - - t.Run("check status of each client", func(t *testing.T) { - t.Run("substitute should be active", func(t *testing.T) { - status, err := query.ClientStatus(ctx, chainA, substituteClientID) - s.Require().NoError(err) - s.Require().Equal(ibcexported.Active.String(), status) - }) - - t.Run("subject should be active", func(t *testing.T) { - status, err := query.ClientStatus(ctx, chainA, subjectClientID) - s.Require().NoError(err) - s.Require().Equal(ibcexported.Active.String(), status) - }) - }) -} - // TestRecoverClient_Succeeds tests that a governance proposal to recover a client using a MsgRecoverClient is successful. func (s *ClientTestSuite) TestRecoverClient_Succeeds() { t := s.T() diff --git a/modules/core/02-client/proposal_handler.go b/modules/core/02-client/proposal_handler.go deleted file mode 100644 index 279f9a0be41..00000000000 --- a/modules/core/02-client/proposal_handler.go +++ /dev/null @@ -1,28 +0,0 @@ -package client - -import ( - errorsmod "cosmossdk.io/errors" - - sdk "github.com/cosmos/cosmos-sdk/types" - govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" - - "github.com/cosmos/ibc-go/v8/modules/core/02-client/keeper" - "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" -) - -// NewClientProposalHandler defines the 02-client legacy proposal handler. -// -// Deprecated: This function is deprecated and will be removed in a future release. -// Please use MsgRecoverClient and MsgIBCSoftwareUpgrade in favour of this legacy Handler. -func NewClientProposalHandler(k *keeper.Keeper) govtypes.Handler { //nolint:staticcheck - return func(ctx sdk.Context, content govtypes.Content) error { - switch c := content.(type) { - case *types.ClientUpdateProposal: - // NOTE: RecoverClient is called in favour of the deprecated ClientUpdateProposal function. - return k.RecoverClient(ctx, c.SubjectClientId, c.SubstituteClientId) - default: - return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "unrecognized ibc proposal content type: %T", c) - } - } -} diff --git a/modules/core/02-client/proposal_handler_test.go b/modules/core/02-client/proposal_handler_test.go deleted file mode 100644 index 108b886add6..00000000000 --- a/modules/core/02-client/proposal_handler_test.go +++ /dev/null @@ -1,94 +0,0 @@ -package client_test - -import ( - sdkmath "cosmossdk.io/math" - - sdk "github.com/cosmos/cosmos-sdk/types" - distributiontypes "github.com/cosmos/cosmos-sdk/x/distribution/types" - govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" - - client "github.com/cosmos/ibc-go/v8/modules/core/02-client" - clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" - ibctesting "github.com/cosmos/ibc-go/v8/testing" -) - -func (suite *ClientTestSuite) TestNewClientUpdateProposalHandler() { - var ( - content govtypes.Content - err error - ) - - testCases := []struct { - name string - malleate func() - expPass bool - }{ - { - "valid update client proposal", func() { - subjectPath := ibctesting.NewPath(suite.chainA, suite.chainB) - subjectPath.SetupClients() - subjectClientState := suite.chainA.GetClientState(subjectPath.EndpointA.ClientID) - - substitutePath := ibctesting.NewPath(suite.chainA, suite.chainB) - substitutePath.SetupClients() - - // update substitute twice - err = substitutePath.EndpointA.UpdateClient() - suite.Require().NoError(err) - err = substitutePath.EndpointA.UpdateClient() - suite.Require().NoError(err) - substituteClientState := suite.chainA.GetClientState(substitutePath.EndpointA.ClientID) - - tmClientState, ok := subjectClientState.(*ibctm.ClientState) - suite.Require().True(ok) - tmClientState.AllowUpdateAfterMisbehaviour = true - tmClientState.FrozenHeight = tmClientState.LatestHeight - suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID, tmClientState) - - // replicate changes to substitute (they must match) - tmClientState, ok = substituteClientState.(*ibctm.ClientState) - suite.Require().True(ok) - tmClientState.AllowUpdateAfterMisbehaviour = true - suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), substitutePath.EndpointA.ClientID, tmClientState) - - content = clienttypes.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subjectPath.EndpointA.ClientID, substitutePath.EndpointA.ClientID) - }, true, - }, - { - "nil proposal", func() { - content = nil - }, false, - }, - { - "unsupported proposal type", func() { - content = &distributiontypes.CommunityPoolSpendProposal{ //nolint:staticcheck - Title: ibctesting.Title, - Description: ibctesting.Description, - Recipient: suite.chainA.SenderAccount.GetAddress().String(), - Amount: sdk.NewCoins(sdk.NewCoin("communityfunds", sdkmath.NewInt(10))), - } - }, false, - }, - } - - for _, tc := range testCases { - tc := tc - - suite.Run(tc.name, func() { - suite.SetupTest() // reset - - tc.malleate() - - proposalHandler := client.NewClientProposalHandler(suite.chainA.App.GetIBCKeeper().ClientKeeper) - - err = proposalHandler(suite.chainA.GetContext(), content) - - if tc.expPass { - suite.Require().NoError(err) - } else { - suite.Require().Error(err) - } - }) - } -} diff --git a/modules/light-clients/08-wasm/testing/simapp/app.go b/modules/light-clients/08-wasm/testing/simapp/app.go index 1d41543aeb0..b4ce8b04ce7 100644 --- a/modules/light-clients/08-wasm/testing/simapp/app.go +++ b/modules/light-clients/08-wasm/testing/simapp/app.go @@ -129,7 +129,6 @@ import ( ibctransferkeeper "github.com/cosmos/ibc-go/v8/modules/apps/transfer/keeper" ibctransfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" ibc "github.com/cosmos/ibc-go/v8/modules/core" - ibcclient "github.com/cosmos/ibc-go/v8/modules/core/02-client" ibcclienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" ibcconnectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types" porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" @@ -425,8 +424,7 @@ func NewSimApp( // See: https://docs.cosmos.network/main/modules/gov#proposal-messages govRouter := govv1beta1.NewRouter() govRouter.AddRoute(govtypes.RouterKey, govv1beta1.ProposalHandler). - AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)). - AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper)) + AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)) govConfig := govtypes.DefaultConfig() /* Example of setting gov params: diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 5fdd714d43a..32c6afab1e2 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -121,7 +121,6 @@ import ( ibctransferkeeper "github.com/cosmos/ibc-go/v8/modules/apps/transfer/keeper" ibctransfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" ibc "github.com/cosmos/ibc-go/v8/modules/core" - ibcclient "github.com/cosmos/ibc-go/v8/modules/core/02-client" ibcclienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" ibcconnectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types" porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" @@ -416,8 +415,7 @@ func NewSimApp( // See: https://docs.cosmos.network/main/modules/gov#proposal-messages govRouter := govv1beta1.NewRouter() govRouter.AddRoute(govtypes.RouterKey, govv1beta1.ProposalHandler). - AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)). - AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper)) + AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)) govConfig := govtypes.DefaultConfig() /* Example of setting gov params: