From ba0e3025dd161a09df0a3b57d52b08bd06a723a3 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 25 Mar 2024 17:29:44 +0100 Subject: [PATCH 01/12] feat: adding consensus host interface for custom self client/consensus state validation --- modules/core/02-client/abci_test.go | 8 +- modules/core/02-client/keeper/keeper.go | 115 ++------- modules/core/02-client/keeper/keeper_test.go | 118 +-------- modules/core/02-client/keeper/migrations.go | 4 +- .../core/02-client/keeper/migrations_test.go | 2 +- .../02-client/migrations/v7/genesis_test.go | 4 +- .../core/02-client/proposal_handler_test.go | 2 +- modules/core/02-client/types/client.go | 7 + modules/core/02-client/types/errors.go | 1 + modules/core/02-client/types/genesis_test.go | 2 +- .../03-connection/keeper/handshake_test.go | 18 ++ modules/core/genesis.go | 4 +- modules/core/keeper/keeper.go | 8 +- modules/core/keeper/migrations.go | 2 +- modules/core/migrations/v7/genesis_test.go | 4 +- modules/core/module.go | 4 +- .../07-tendermint/consensus_host.go | 125 +++++++++ .../07-tendermint/consensus_host_test.go | 241 ++++++++++++++++++ .../08-wasm/testing/simapp/app.go | 3 +- .../08-wasm/types/consensus_host.go | 75 ++++++ testing/mock/consensus_host.go | 31 +++ testing/simapp/app.go | 2 +- testing/simapp/upgrades.go | 4 +- testing/simapp/upgrades/upgrades.go | 2 +- 24 files changed, 548 insertions(+), 238 deletions(-) create mode 100644 modules/light-clients/07-tendermint/consensus_host.go create mode 100644 modules/light-clients/07-tendermint/consensus_host_test.go create mode 100644 modules/light-clients/08-wasm/types/consensus_host.go create mode 100644 testing/mock/consensus_host.go diff --git a/modules/core/02-client/abci_test.go b/modules/core/02-client/abci_test.go index b5a78320ad1..fbc36465b34 100644 --- a/modules/core/02-client/abci_test.go +++ b/modules/core/02-client/abci_test.go @@ -44,7 +44,7 @@ func (suite *ClientTestSuite) TestBeginBlocker() { suite.coordinator.CommitBlock(suite.chainA, suite.chainB) suite.Require().NotPanics(func() { - client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) + client.BeginBlocker(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper) }, "BeginBlocker shouldn't panic") } } @@ -69,7 +69,7 @@ func (suite *ClientTestSuite) TestBeginBlockerConsensusState() { err := suite.chainA.GetSimApp().UpgradeKeeper.SetUpgradedClient(newCtx, plan.Height, []byte("client state")) suite.Require().NoError(err) - client.BeginBlocker(newCtx, suite.chainA.App.GetIBCKeeper().ClientKeeper) + client.BeginBlocker(newCtx, *suite.chainA.App.GetIBCKeeper().ClientKeeper) // plan Height is at ctx.BlockHeight+1 consState, err := suite.chainA.GetSimApp().UpgradeKeeper.GetUpgradedConsensusState(newCtx, plan.Height) @@ -101,7 +101,7 @@ func (suite *ClientTestSuite) TestBeginBlockerUpgradeEvents() { cacheCtx, writeCache := suite.chainA.GetContext().CacheContext() - client.BeginBlocker(cacheCtx, suite.chainA.App.GetIBCKeeper().ClientKeeper) + client.BeginBlocker(cacheCtx, *suite.chainA.App.GetIBCKeeper().ClientKeeper) writeCache() suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, true) @@ -109,7 +109,7 @@ func (suite *ClientTestSuite) TestBeginBlockerUpgradeEvents() { func (suite *ClientTestSuite) TestBeginBlockerUpgradeEventsAbsence() { cacheCtx, writeCache := suite.chainA.GetContext().CacheContext() - client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) + client.BeginBlocker(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper) writeCache() suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, false) } diff --git a/modules/core/02-client/keeper/keeper.go b/modules/core/02-client/keeper/keeper.go index 3a73cfa9588..1ab1619f1cb 100644 --- a/modules/core/02-client/keeper/keeper.go +++ b/modules/core/02-client/keeper/keeper.go @@ -3,7 +3,6 @@ package keeper import ( "errors" "fmt" - "reflect" "strings" errorsmod "cosmossdk.io/errors" @@ -15,12 +14,8 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cometbft/cometbft/light" - "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" - ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" localhost "github.com/cosmos/ibc-go/v8/modules/light-clients/09-localhost" @@ -32,6 +27,7 @@ type Keeper struct { storeKey storetypes.StoreKey cdc codec.BinaryCodec router *types.Router + consensusHost types.ConsensusHost legacySubspace types.ParamSubspace stakingKeeper types.StakingKeeper upgradeKeeper types.UpgradeKeeper @@ -47,6 +43,7 @@ func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace ty storeKey: key, cdc: cdc, router: router, + consensusHost: ibctm.NewConsensusHost(sk), legacySubspace: legacySubspace, stakingKeeper: sk, upgradeKeeper: uk, @@ -88,6 +85,15 @@ func (k Keeper) UpdateLocalhostClient(ctx sdk.Context, clientState exported.Clie return clientModule.UpdateState(ctx, exported.LocalhostClientID, nil) } +// SetSelfConsensusHost sets a custom ConsensusHost for self client state and consensus state validation. +func (k *Keeper) SetSelfConsensusHost(consensusHost types.ConsensusHost) { + if consensusHost == nil { + panic(fmt.Errorf("cannot set a nil self consensus host")) + } + + k.consensusHost = consensusHost +} + // GenerateClientIdentifier returns the next client identifier. func (k Keeper) GenerateClientIdentifier(ctx sdk.Context, clientType string) string { nextClientSeq := k.GetNextClientSequence(ctx) @@ -99,7 +105,7 @@ func (k Keeper) GenerateClientIdentifier(ctx sdk.Context, clientType string) str } // GetClientState gets a particular client from the store -func (k Keeper) GetClientState(ctx sdk.Context, clientID string) (exported.ClientState, bool) { +func (k *Keeper) GetClientState(ctx sdk.Context, clientID string) (exported.ClientState, bool) { store := k.ClientStore(ctx, clientID) bz := store.Get(host.ClientStateKey()) if len(bz) == 0 { @@ -111,13 +117,13 @@ func (k Keeper) GetClientState(ctx sdk.Context, clientID string) (exported.Clien } // SetClientState sets a particular Client to the store -func (k Keeper) SetClientState(ctx sdk.Context, clientID string, clientState exported.ClientState) { +func (k *Keeper) SetClientState(ctx sdk.Context, clientID string, clientState exported.ClientState) { store := k.ClientStore(ctx, clientID) store.Set(host.ClientStateKey(), k.MustMarshalClientState(clientState)) } // GetClientConsensusState gets the stored consensus state from a client at a given height. -func (k Keeper) GetClientConsensusState(ctx sdk.Context, clientID string, height exported.Height) (exported.ConsensusState, bool) { +func (k *Keeper) GetClientConsensusState(ctx sdk.Context, clientID string, height exported.Height) (exported.ConsensusState, bool) { store := k.ClientStore(ctx, clientID) bz := store.Get(host.ConsensusStateKey(height)) if len(bz) == 0 { @@ -308,96 +314,15 @@ func (k Keeper) GetLatestClientConsensusState(ctx sdk.Context, clientID string) // and returns the expected consensus state at that height. // For now, can only retrieve self consensus states for the current revision func (k Keeper) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { - selfHeight, ok := height.(types.Height) - if !ok { - return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", types.Height{}, height) - } - // check that height revision matches chainID revision - revision := types.ParseChainID(ctx.ChainID()) - if revision != height.GetRevisionNumber() { - return nil, errorsmod.Wrapf(types.ErrInvalidHeight, "chainID revision number does not match height revision number: expected %d, got %d", revision, height.GetRevisionNumber()) - } - histInfo, err := k.stakingKeeper.GetHistoricalInfo(ctx, int64(selfHeight.RevisionHeight)) - if err != nil { - return nil, errorsmod.Wrapf(err, "height %d", selfHeight.RevisionHeight) - } - - consensusState := &ibctm.ConsensusState{ - Timestamp: histInfo.Header.Time, - Root: commitmenttypes.NewMerkleRoot(histInfo.Header.GetAppHash()), - NextValidatorsHash: histInfo.Header.NextValidatorsHash, - } - - return consensusState, nil + return k.consensusHost.GetSelfConsensusState(ctx, height) } -// ValidateSelfClient validates the client parameters for a client of the running chain -// This function is only used to validate the client state the counterparty stores for this chain -// Client must be in same revision as the executing chain +// ValidateSelfClient validates the client parameters for a client of the running chain. +// This function is only used to validate the client state the counterparty stores for this chain. +// NOTE: If the client type is not of type Tendermint then delegate to a custom client validator function. +// This allows support for non-Tendermint clients, for example 08-wasm clients. func (k Keeper) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error { - tmClient, ok := clientState.(*ibctm.ClientState) - if !ok { - return errorsmod.Wrapf(types.ErrInvalidClient, "client must be a Tendermint client, expected: %T, got: %T", - &ibctm.ClientState{}, tmClient) - } - - if !tmClient.FrozenHeight.IsZero() { - return types.ErrClientFrozen - } - - if ctx.ChainID() != tmClient.ChainId { - return errorsmod.Wrapf(types.ErrInvalidClient, "invalid chain-id. expected: %s, got: %s", - ctx.ChainID(), tmClient.ChainId) - } - - revision := types.ParseChainID(ctx.ChainID()) - - // client must be in the same revision as executing chain - if tmClient.LatestHeight.RevisionNumber != revision { - return errorsmod.Wrapf(types.ErrInvalidClient, "client is not in the same revision as the chain. expected revision: %d, got: %d", - tmClient.LatestHeight.RevisionNumber, revision) - } - - selfHeight := types.NewHeight(revision, uint64(ctx.BlockHeight())) - if tmClient.LatestHeight.GTE(selfHeight) { - return errorsmod.Wrapf(types.ErrInvalidClient, "client has LatestHeight %d greater than or equal to chain height %d", - tmClient.LatestHeight, selfHeight) - } - - expectedProofSpecs := commitmenttypes.GetSDKSpecs() - if !reflect.DeepEqual(expectedProofSpecs, tmClient.ProofSpecs) { - return errorsmod.Wrapf(types.ErrInvalidClient, "client has invalid proof specs. expected: %v got: %v", - expectedProofSpecs, tmClient.ProofSpecs) - } - - if err := light.ValidateTrustLevel(tmClient.TrustLevel.ToTendermint()); err != nil { - return errorsmod.Wrapf(types.ErrInvalidClient, "trust-level invalid: %v", err) - } - - expectedUbdPeriod, err := k.stakingKeeper.UnbondingTime(ctx) - if err != nil { - return errorsmod.Wrapf(err, "failed to retrieve unbonding period") - } - - if expectedUbdPeriod != tmClient.UnbondingPeriod { - return errorsmod.Wrapf(types.ErrInvalidClient, "invalid unbonding period. expected: %s, got: %s", - expectedUbdPeriod, tmClient.UnbondingPeriod) - } - - if tmClient.UnbondingPeriod < tmClient.TrustingPeriod { - return errorsmod.Wrapf(types.ErrInvalidClient, "unbonding period must be greater than trusting period. unbonding period (%d) < trusting period (%d)", - tmClient.UnbondingPeriod, tmClient.TrustingPeriod) - } - - if len(tmClient.UpgradePath) != 0 { - // For now, SDK IBC implementation assumes that upgrade path (if defined) is defined by SDK upgrade module - expectedUpgradePath := []string{upgradetypes.StoreKey, upgradetypes.KeyUpgradedIBCState} - if !reflect.DeepEqual(expectedUpgradePath, tmClient.UpgradePath) { - return errorsmod.Wrapf(types.ErrInvalidClient, "upgrade path must be the upgrade path defined by upgrade module. expected %v, got %v", - expectedUpgradePath, tmClient.UpgradePath) - } - } - return nil + return k.consensusHost.ValidateSelfClient(ctx, clientState) } // GetUpgradePlan executes the upgrade keeper GetUpgradePlan function. diff --git a/modules/core/02-client/keeper/keeper_test.go b/modules/core/02-client/keeper/keeper_test.go index 9c9d61e76f2..fd120c12b54 100644 --- a/modules/core/02-client/keeper/keeper_test.go +++ b/modules/core/02-client/keeper/keeper_test.go @@ -25,7 +25,6 @@ import ( commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" "github.com/cosmos/ibc-go/v8/modules/core/exported" - solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" localhost "github.com/cosmos/ibc-go/v8/modules/light-clients/09-localhost" ibctesting "github.com/cosmos/ibc-go/v8/testing" @@ -45,10 +44,7 @@ const ( maxClockDrift time.Duration = time.Second * 10 ) -var ( - testClientHeight = types.NewHeight(0, 5) - testClientHeightRevision1 = types.NewHeight(1, 5) -) +var testClientHeight = types.NewHeight(0, 5) type KeeperTestSuite struct { testifysuite.Suite @@ -85,7 +81,7 @@ func (suite *KeeperTestSuite) SetupTest() { suite.cdc = app.AppCodec() suite.ctx = app.BaseApp.NewContext(isCheckTx) - suite.keeper = &app.IBCKeeper.ClientKeeper + suite.keeper = app.IBCKeeper.ClientKeeper suite.privVal = cmttypes.NewMockPV() pubKey, err := suite.privVal.GetPubKey() suite.Require().NoError(err) @@ -145,90 +141,6 @@ func (suite *KeeperTestSuite) TestSetClientConsensusState() { suite.Require().Equal(suite.consensusState, tmConsState, "ConsensusState not stored correctly") } -func (suite *KeeperTestSuite) TestValidateSelfClient() { - testClientHeight := types.GetSelfHeight(suite.chainA.GetContext()) - testClientHeight.RevisionHeight-- - - testCases := []struct { - name string - clientState exported.ClientState - expPass bool - }{ - { - "success", - ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), - true, - }, - { - "success with nil UpgradePath", - ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), nil), - true, - }, - { - "frozen client", - &ibctm.ClientState{ChainId: suite.chainA.ChainID, TrustLevel: ibctm.DefaultTrustLevel, TrustingPeriod: trustingPeriod, UnbondingPeriod: ubdPeriod, MaxClockDrift: maxClockDrift, FrozenHeight: testClientHeight, LatestHeight: testClientHeight, ProofSpecs: commitmenttypes.GetSDKSpecs(), UpgradePath: ibctesting.UpgradePath}, - false, - }, - { - "incorrect chainID", - ibctm.NewClientState("gaiatestnet", ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), - false, - }, - { - "invalid client height", - ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, types.GetSelfHeight(suite.chainA.GetContext()).Increment().(types.Height), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), - false, - }, - { - "invalid client type", - solomachine.NewClientState(0, &solomachine.ConsensusState{PublicKey: suite.solomachine.ConsensusState().PublicKey, Diversifier: suite.solomachine.Diversifier, Timestamp: suite.solomachine.Time}), - false, - }, - { - "invalid client revision", - ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeightRevision1, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), - false, - }, - { - "invalid proof specs", - ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, nil, ibctesting.UpgradePath), - false, - }, - { - "invalid trust level", - ibctm.NewClientState(suite.chainA.ChainID, ibctm.Fraction{Numerator: 0, Denominator: 1}, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), false, - }, - { - "invalid unbonding period", - ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+10, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), - false, - }, - { - "invalid trusting period", - ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, ubdPeriod+10, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), - false, - }, - { - "invalid upgrade path", - ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), []string{"bad", "upgrade", "path"}), - false, - }, - } - - for _, tc := range testCases { - tc := tc - - suite.Run(tc.name, func() { - err := suite.chainA.App.GetIBCKeeper().ClientKeeper.ValidateSelfClient(suite.chainA.GetContext(), tc.clientState) - if tc.expPass { - suite.Require().NoError(err, "expected valid client for case: %s", tc.name) - } else { - suite.Require().Error(err, "expected invalid client for case: %s", tc.name) - } - }) - } -} - func (suite *KeeperTestSuite) TestGetAllGenesisClients() { clientIDs := []string{ exported.LocalhostClientID, testClientID2, testClientID3, testClientID, @@ -308,32 +220,6 @@ func (suite *KeeperTestSuite) TestGetAllGenesisMetadata() { }) } -func (suite *KeeperTestSuite) TestGetConsensusState() { - suite.ctx = suite.ctx.WithBlockHeight(10) - cases := []struct { - name string - height types.Height - expPass bool - }{ - {"zero height", types.ZeroHeight(), false}, - {"height > latest height", types.NewHeight(0, uint64(suite.ctx.BlockHeight())+1), false}, - {"latest height - 1", types.NewHeight(0, uint64(suite.ctx.BlockHeight())-1), true}, - {"latest height", types.GetSelfHeight(suite.ctx), true}, - } - - for i, tc := range cases { - tc := tc - cs, err := suite.keeper.GetSelfConsensusState(suite.ctx, tc.height) - if tc.expPass { - suite.Require().NoError(err, "Case %d should have passed: %s", i, tc.name) - suite.Require().NotNil(cs, "Case %d should have passed: %s", i, tc.name) - } else { - suite.Require().Error(err, "Case %d should have failed: %s", i, tc.name) - suite.Require().Nil(cs, "Case %d should have failed: %s", i, tc.name) - } - } -} - // 2 clients in total are created on chainA. The first client is updated so it contains an initial consensus state // and a consensus state at the update height. func (suite *KeeperTestSuite) TestGetAllConsensusStates() { diff --git a/modules/core/02-client/keeper/migrations.go b/modules/core/02-client/keeper/migrations.go index 54620867928..818883248c4 100644 --- a/modules/core/02-client/keeper/migrations.go +++ b/modules/core/02-client/keeper/migrations.go @@ -24,13 +24,13 @@ func NewMigrator(keeper Keeper) Migrator { // - removes the localhost client // - asserts that existing tendermint clients are properly registered on the chain codec func (m Migrator) Migrate2to3(ctx sdk.Context) error { - return v7.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc, m.keeper) + return v7.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc, &m.keeper) } // Migrate3to4 migrates from consensus version 3 to 4. // This migration enables the localhost client. func (m Migrator) Migrate3to4(ctx sdk.Context) error { - return v7.MigrateLocalhostClient(ctx, m.keeper) + return v7.MigrateLocalhostClient(ctx, &m.keeper) } // MigrateParams migrates from consensus version 4 to 5. diff --git a/modules/core/02-client/keeper/migrations_test.go b/modules/core/02-client/keeper/migrations_test.go index 7bdd26b8a12..cf8abb2d312 100644 --- a/modules/core/02-client/keeper/migrations_test.go +++ b/modules/core/02-client/keeper/migrations_test.go @@ -32,7 +32,7 @@ func (suite *KeeperTestSuite) TestMigrateParams() { tc.malleate() ctx := suite.chainA.GetContext() - migrator := keeper.NewMigrator(suite.chainA.GetSimApp().IBCKeeper.ClientKeeper) + migrator := keeper.NewMigrator(*suite.chainA.GetSimApp().IBCKeeper.ClientKeeper) err := migrator.MigrateParams(ctx) suite.Require().NoError(err) diff --git a/modules/core/02-client/migrations/v7/genesis_test.go b/modules/core/02-client/migrations/v7/genesis_test.go index 4abbb646f0c..944e579892f 100644 --- a/modules/core/02-client/migrations/v7/genesis_test.go +++ b/modules/core/02-client/migrations/v7/genesis_test.go @@ -33,7 +33,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() { solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, ibctesting.DefaultSolomachineClientID, "testing", 1) solomachineMulti := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-1", "testing", 4) - clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) + clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper) // manually generate old proto buf definitions and set in genesis // NOTE: we cannot use 'ExportGenesis' for the solo machines since we are @@ -108,7 +108,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() { // NOTE: tendermint clients are not pruned in genesis so the test should not have expired tendermint clients err := v7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(ibcexported.StoreKey), suite.chainA.App.AppCodec(), suite.chainA.GetSimApp().IBCKeeper.ClientKeeper) suite.Require().NoError(err) - expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) + expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper) cdc, ok := suite.chainA.App.AppCodec().(codec.ProtoCodecMarshaler) suite.Require().True(ok) diff --git a/modules/core/02-client/proposal_handler_test.go b/modules/core/02-client/proposal_handler_test.go index 108b886add6..2d6a963e4d8 100644 --- a/modules/core/02-client/proposal_handler_test.go +++ b/modules/core/02-client/proposal_handler_test.go @@ -80,7 +80,7 @@ func (suite *ClientTestSuite) TestNewClientUpdateProposalHandler() { tc.malleate() - proposalHandler := client.NewClientProposalHandler(suite.chainA.App.GetIBCKeeper().ClientKeeper) + proposalHandler := client.NewClientProposalHandler(*suite.chainA.App.GetIBCKeeper().ClientKeeper) err = proposalHandler(suite.chainA.GetContext(), content) diff --git a/modules/core/02-client/types/client.go b/modules/core/02-client/types/client.go index 31da1a54e70..2b1cb965b85 100644 --- a/modules/core/02-client/types/client.go +++ b/modules/core/02-client/types/client.go @@ -11,6 +11,7 @@ import ( errorsmod "cosmossdk.io/errors" codectypes "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" "github.com/cosmos/ibc-go/v8/modules/core/exported" @@ -21,6 +22,12 @@ var ( _ codectypes.UnpackInterfacesMessage = (*ConsensusStateWithHeight)(nil) ) +// ConsensusHost defines an interface used to validate an IBC ClientState and ConsensusState against the host chain's underlying consensus parameters. +type ConsensusHost interface { + GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) + ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error +} + // NewIdentifiedClientState creates a new IdentifiedClientState instance func NewIdentifiedClientState(clientID string, clientState exported.ClientState) IdentifiedClientState { msg, ok := clientState.(proto.Message) diff --git a/modules/core/02-client/types/errors.go b/modules/core/02-client/types/errors.go index 98c4a502236..62d906bc40c 100644 --- a/modules/core/02-client/types/errors.go +++ b/modules/core/02-client/types/errors.go @@ -37,4 +37,5 @@ var ( ErrFailedMembershipVerification = errorsmod.Register(SubModuleName, 30, "membership verification failed") ErrFailedNonMembershipVerification = errorsmod.Register(SubModuleName, 31, "non-membership verification failed") ErrRouteNotFound = errorsmod.Register(SubModuleName, 32, "light client module route not found") + ErrClientTypeNotSupported = errorsmod.Register(SubModuleName, 33, "client type not supported") ) diff --git a/modules/core/02-client/types/genesis_test.go b/modules/core/02-client/types/genesis_test.go index ab9a5c18150..2a91d94b764 100644 --- a/modules/core/02-client/types/genesis_test.go +++ b/modules/core/02-client/types/genesis_test.go @@ -32,7 +32,7 @@ func (suite *TypesTestSuite) TestMarshalGenesisState() { err := path.EndpointA.UpdateClient() suite.Require().NoError(err) - genesis := client.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) + genesis := client.ExportGenesis(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper) bz, err := cdc.MarshalJSON(&genesis) suite.Require().NoError(err) diff --git a/modules/core/03-connection/keeper/handshake_test.go b/modules/core/03-connection/keeper/handshake_test.go index 8636ed5f591..4b27757e50e 100644 --- a/modules/core/03-connection/keeper/handshake_test.go +++ b/modules/core/03-connection/keeper/handshake_test.go @@ -3,12 +3,15 @@ package keeper_test import ( "time" + sdk "github.com/cosmos/cosmos-sdk/types" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" "github.com/cosmos/ibc-go/v8/modules/core/exported" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v8/testing" + "github.com/cosmos/ibc-go/v8/testing/mock" ) // TestConnOpenInit - chainA initializes (INIT state) a connection with @@ -216,6 +219,21 @@ func (suite *KeeperTestSuite) TestConnOpenTry() { err := path.EndpointA.ConnOpenInit() suite.Require().NoError(err) }, false}, + {"override self consensus host", func() { + err := path.EndpointA.ConnOpenInit() + suite.Require().NoError(err) + + // retrieve client state of chainA to pass as counterpartyClient + counterpartyClient = suite.chainA.GetClientState(path.EndpointA.ClientID) + + mockValidator := mock.ConsensusHost{ + ValidateSelfClientFn: func(ctx sdk.Context, clientState exported.ClientState) error { + return mock.MockApplicationCallbackError + }, + } + + suite.chainB.App.GetIBCKeeper().ClientKeeper.SetSelfConsensusHost(&mockValidator) + }, false}, } for _, tc := range testCases { diff --git a/modules/core/genesis.go b/modules/core/genesis.go index 3eaada47208..d1379de4b84 100644 --- a/modules/core/genesis.go +++ b/modules/core/genesis.go @@ -13,7 +13,7 @@ import ( // InitGenesis initializes the ibc state from a provided genesis // state. func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs *types.GenesisState) { - client.InitGenesis(ctx, k.ClientKeeper, gs.ClientGenesis) + client.InitGenesis(ctx, *k.ClientKeeper, gs.ClientGenesis) connection.InitGenesis(ctx, k.ConnectionKeeper, gs.ConnectionGenesis) channel.InitGenesis(ctx, k.ChannelKeeper, gs.ChannelGenesis) } @@ -21,7 +21,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs *types.GenesisState) { // ExportGenesis returns the ibc exported genesis. func ExportGenesis(ctx sdk.Context, k keeper.Keeper) *types.GenesisState { return &types.GenesisState{ - ClientGenesis: client.ExportGenesis(ctx, k.ClientKeeper), + ClientGenesis: client.ExportGenesis(ctx, *k.ClientKeeper), ConnectionGenesis: connection.ExportGenesis(ctx, k.ConnectionKeeper), ChannelGenesis: channel.ExportGenesis(ctx, k.ChannelKeeper), } diff --git a/modules/core/keeper/keeper.go b/modules/core/keeper/keeper.go index 08ee5dc027e..d5982a6faa6 100644 --- a/modules/core/keeper/keeper.go +++ b/modules/core/keeper/keeper.go @@ -28,7 +28,7 @@ type Keeper struct { cdc codec.BinaryCodec - ClientKeeper clientkeeper.Keeper + ClientKeeper *clientkeeper.Keeper ConnectionKeeper connectionkeeper.Keeper ChannelKeeper channelkeeper.Keeper PortKeeper *portkeeper.Keeper @@ -60,13 +60,13 @@ func NewKeeper( } clientKeeper := clientkeeper.NewKeeper(cdc, key, paramSpace, stakingKeeper, upgradeKeeper) - connectionKeeper := connectionkeeper.NewKeeper(cdc, key, paramSpace, clientKeeper) + connectionKeeper := connectionkeeper.NewKeeper(cdc, key, paramSpace, &clientKeeper) portKeeper := portkeeper.NewKeeper(scopedKeeper) - channelKeeper := channelkeeper.NewKeeper(cdc, key, clientKeeper, connectionKeeper, &portKeeper, scopedKeeper) + channelKeeper := channelkeeper.NewKeeper(cdc, key, &clientKeeper, &connectionKeeper, &portKeeper, scopedKeeper) return &Keeper{ cdc: cdc, - ClientKeeper: clientKeeper, + ClientKeeper: &clientKeeper, ConnectionKeeper: connectionKeeper, ChannelKeeper: channelKeeper, PortKeeper: &portKeeper, diff --git a/modules/core/keeper/migrations.go b/modules/core/keeper/migrations.go index 9afaddcb3c7..82aa3e10ac4 100644 --- a/modules/core/keeper/migrations.go +++ b/modules/core/keeper/migrations.go @@ -18,6 +18,6 @@ func NewMigrator(keeper Keeper) Migrator { // Migrate2to3 migrates from version 2 to 3. See 02-client keeper function Migrate2to3. func (m Migrator) Migrate2to3(ctx sdk.Context) error { - clientMigrator := clientkeeper.NewMigrator(m.keeper.ClientKeeper) + clientMigrator := clientkeeper.NewMigrator(*m.keeper.ClientKeeper) return clientMigrator.Migrate2to3(ctx) } diff --git a/modules/core/migrations/v7/genesis_test.go b/modules/core/migrations/v7/genesis_test.go index de941f3a2d2..781d4f594ba 100644 --- a/modules/core/migrations/v7/genesis_test.go +++ b/modules/core/migrations/v7/genesis_test.go @@ -61,7 +61,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() { solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, ibctesting.DefaultSolomachineClientID, "testing", 1) solomachineMulti := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-1", "testing", 4) - clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) + clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper) // manually generate old proto buf definitions and set in genesis // NOTE: we cannot use 'ExportGenesis' for the solo machines since we are @@ -135,7 +135,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() { // NOTE: tendermint clients are not pruned in genesis so the test should not have expired tendermint clients err := clientv7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(ibcexported.StoreKey), suite.chainA.App.AppCodec(), suite.chainA.GetSimApp().IBCKeeper.ClientKeeper) suite.Require().NoError(err) - expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) + expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper) cdc := suite.chainA.App.AppCodec().(*codec.ProtoCodec) diff --git a/modules/core/module.go b/modules/core/module.go index e7f6ebd4a94..374bfaeda92 100644 --- a/modules/core/module.go +++ b/modules/core/module.go @@ -133,7 +133,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { channeltypes.RegisterMsgServer(cfg.MsgServer(), am.keeper) types.RegisterQueryService(cfg.QueryServer(), am.keeper) - clientMigrator := clientkeeper.NewMigrator(am.keeper.ClientKeeper) + clientMigrator := clientkeeper.NewMigrator(*am.keeper.ClientKeeper) if err := cfg.RegisterMigration(exported.ModuleName, 2, clientMigrator.Migrate2to3); err != nil { panic(err) } @@ -188,7 +188,7 @@ func (AppModule) ConsensusVersion() uint64 { return 6 } // BeginBlock returns the begin blocker for the ibc module. func (am AppModule) BeginBlock(ctx context.Context) error { - ibcclient.BeginBlocker(sdk.UnwrapSDKContext(ctx), am.keeper.ClientKeeper) + ibcclient.BeginBlocker(sdk.UnwrapSDKContext(ctx), *am.keeper.ClientKeeper) return nil } diff --git a/modules/light-clients/07-tendermint/consensus_host.go b/modules/light-clients/07-tendermint/consensus_host.go new file mode 100644 index 00000000000..d0c62c75c08 --- /dev/null +++ b/modules/light-clients/07-tendermint/consensus_host.go @@ -0,0 +1,125 @@ +package tendermint + +import ( + "reflect" + + errorsmod "cosmossdk.io/errors" + upgradetypes "cosmossdk.io/x/upgrade/types" + + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/cometbft/cometbft/light" + + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" + "github.com/cosmos/ibc-go/v8/modules/core/exported" +) + +var _ clienttypes.ConsensusHost = (*ConsensusHost)(nil) + +// ConsensusHost implements the 02-client clienttypes.ConsensusHost interface +type ConsensusHost struct { + stakingKeeper clienttypes.StakingKeeper +} + +// NewConsensusHost creates and returns a new ConsensusHost for tendermint consensus. +func NewConsensusHost(stakingKeeper clienttypes.StakingKeeper) clienttypes.ConsensusHost { + return &ConsensusHost{ + stakingKeeper: stakingKeeper, + } +} + +// GetSelfConsensusState implements the 02-client clienttypes.ConsensusHost interface. +func (c *ConsensusHost) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { + selfHeight, ok := height.(clienttypes.Height) + if !ok { + return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", clienttypes.Height{}, height) + } + + // check that height revision matches chainID revision + revision := clienttypes.ParseChainID(ctx.ChainID()) + if revision != height.GetRevisionNumber() { + return nil, errorsmod.Wrapf(clienttypes.ErrInvalidHeight, "chainID revision number does not match height revision number: expected %d, got %d", revision, height.GetRevisionNumber()) + } + + histInfo, err := c.stakingKeeper.GetHistoricalInfo(ctx, int64(selfHeight.RevisionHeight)) + if err != nil { + return nil, errorsmod.Wrapf(err, "height %d", selfHeight.RevisionHeight) + } + + consensusState := &ConsensusState{ + Timestamp: histInfo.Header.Time, + Root: commitmenttypes.NewMerkleRoot(histInfo.Header.GetAppHash()), + NextValidatorsHash: histInfo.Header.NextValidatorsHash, + } + + return consensusState, nil +} + +// ValidateSelfClient implements the 02-client clienttypes.ConsensusHost interface. +func (c *ConsensusHost) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error { + tmClient, ok := clientState.(*ClientState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client must be a Tendermint client, expected: %T, got: %T", &ClientState{}, tmClient) + } + + if !tmClient.FrozenHeight.IsZero() { + return clienttypes.ErrClientFrozen + } + + if ctx.ChainID() != tmClient.ChainId { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "invalid chain-id. expected: %s, got: %s", + ctx.ChainID(), tmClient.ChainId) + } + + revision := clienttypes.ParseChainID(ctx.ChainID()) + + // client must be in the same revision as executing chain + if tmClient.LatestHeight.RevisionNumber != revision { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client is not in the same revision as the chain. expected revision: %d, got: %d", + tmClient.LatestHeight.RevisionNumber, revision) + } + + selfHeight := clienttypes.NewHeight(revision, uint64(ctx.BlockHeight())) + if tmClient.LatestHeight.GTE(selfHeight) { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client has LatestHeight %d greater than or equal to chain height %d", + tmClient.LatestHeight, selfHeight) + } + + expectedProofSpecs := commitmenttypes.GetSDKSpecs() + if !reflect.DeepEqual(expectedProofSpecs, tmClient.ProofSpecs) { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client has invalid proof specs. expected: %v got: %v", + expectedProofSpecs, tmClient.ProofSpecs) + } + + if err := light.ValidateTrustLevel(tmClient.TrustLevel.ToTendermint()); err != nil { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "trust-level invalid: %v", err) + } + + expectedUbdPeriod, err := c.stakingKeeper.UnbondingTime(ctx) + if err != nil { + return errorsmod.Wrapf(err, "failed to retrieve unbonding period") + } + + if expectedUbdPeriod != tmClient.UnbondingPeriod { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "invalid unbonding period. expected: %s, got: %s", + expectedUbdPeriod, tmClient.UnbondingPeriod) + } + + if tmClient.UnbondingPeriod < tmClient.TrustingPeriod { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "unbonding period must be greater than trusting period. unbonding period (%d) < trusting period (%d)", + tmClient.UnbondingPeriod, tmClient.TrustingPeriod) + } + + if len(tmClient.UpgradePath) != 0 { + // For now, SDK IBC implementation assumes that upgrade path (if defined) is defined by SDK upgrade module + expectedUpgradePath := []string{upgradetypes.StoreKey, upgradetypes.KeyUpgradedIBCState} + if !reflect.DeepEqual(expectedUpgradePath, tmClient.UpgradePath) { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "upgrade path must be the upgrade path defined by upgrade module. expected %v, got %v", + expectedUpgradePath, tmClient.UpgradePath) + } + } + + return nil +} diff --git a/modules/light-clients/07-tendermint/consensus_host_test.go b/modules/light-clients/07-tendermint/consensus_host_test.go new file mode 100644 index 00000000000..21e390cbf03 --- /dev/null +++ b/modules/light-clients/07-tendermint/consensus_host_test.go @@ -0,0 +1,241 @@ +package tendermint_test + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" + "github.com/cosmos/ibc-go/v8/modules/core/exported" + solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine" + ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" + ibctesting "github.com/cosmos/ibc-go/v8/testing" + "github.com/cosmos/ibc-go/v8/testing/mock" +) + +func (suite *TendermintTestSuite) TestGetSelfConsensusState() { + var height clienttypes.Height + + cases := []struct { + name string + malleate func() + expError error + }{ + { + name: "zero height", + malleate: func() { + height = clienttypes.ZeroHeight() + }, + expError: clienttypes.ErrInvalidHeight, + }, + { + name: "height > latest height", + malleate: func() { + height = clienttypes.NewHeight(1, uint64(suite.chainA.GetContext().BlockHeight())+1) + }, + expError: stakingtypes.ErrNoHistoricalInfo, + }, + { + name: "custom consensus host: failure", + malleate: func() { + consensusHost := &mock.ConsensusHost{ + GetSelfConsensusStateFn: func(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { + return nil, mock.MockApplicationCallbackError + }, + } + suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.SetSelfConsensusHost(consensusHost) + }, + expError: mock.MockApplicationCallbackError, + }, + { + name: "custom consensus host: success", + malleate: func() { + consensusHost := &mock.ConsensusHost{ + GetSelfConsensusStateFn: func(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { + return &solomachine.ConsensusState{}, nil + }, + } + suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.SetSelfConsensusHost(consensusHost) + }, + expError: nil, + }, + { + name: "latest height - 1", + malleate: func() { + height = clienttypes.NewHeight(1, uint64(suite.chainA.GetContext().BlockHeight())-1) + }, + expError: nil, + }, + { + name: "latest height", + malleate: func() { + // historical info is set on BeginBlock in x/staking, which is now encapsulated within the FinalizeBlock abci method, + // thus, we do not have historical info for current height due to how the ibctesting library operates. + // ibctesting calls app.Commit() as a final step on NextBlock and we invoke test code before FinalizeBlock is called at the current height once again. + err := suite.chainA.GetSimApp().StakingKeeper.TrackHistoricalInfo(suite.chainA.GetContext()) + suite.Require().NoError(err) + + height = clienttypes.GetSelfHeight(suite.chainA.GetContext()) + }, + expError: nil, + }, + } + + for i, tc := range cases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() + + height = clienttypes.ZeroHeight() + + tc.malleate() + + cs, err := suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.GetSelfConsensusState(suite.chainA.GetContext(), height) + + expPass := tc.expError == nil + if expPass { + suite.Require().NoError(err, "Case %d should have passed: %s", i, tc.name) + suite.Require().NotNil(cs, "Case %d should have passed: %s", i, tc.name) + } else { + suite.Require().ErrorIs(err, tc.expError, "Case %d should have failed: %s", i, tc.name) + suite.Require().Nil(cs, "Case %d should have failed: %s", i, tc.name) + } + }) + } +} + +func (suite *TendermintTestSuite) TestValidateSelfClient() { + testClientHeight := clienttypes.GetSelfHeight(suite.chainA.GetContext()) + testClientHeight.RevisionHeight-- + + var clientState exported.ClientState + + testCases := []struct { + name string + malleate func() + expError error + }{ + { + name: "success", + malleate: func() { + clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + }, + expError: nil, + }, + { + name: "success with nil UpgradePath", + malleate: func() { + clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), nil) + }, + expError: nil, + }, + { + name: "success with custom self validator: solomachine", + malleate: func() { + clientState = solomachine.NewClientState(1, &solomachine.ConsensusState{}) + + smConsensusHost := &mock.ConsensusHost{ + ValidateSelfClientFn: func(ctx sdk.Context, clientState exported.ClientState) error { + smClientState, ok := clientState.(*solomachine.ClientState) + suite.Require().True(ok) + suite.Require().Equal(uint64(1), smClientState.Sequence) + + return nil + }, + } + + // add mock validation logic + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetSelfConsensusHost(smConsensusHost) + }, + expError: nil, + }, + { + name: "frozen client", + malleate: func() { + clientState = &ibctm.ClientState{ChainId: suite.chainA.ChainID, TrustLevel: ibctm.DefaultTrustLevel, TrustingPeriod: trustingPeriod, UnbondingPeriod: ubdPeriod, MaxClockDrift: maxClockDrift, FrozenHeight: testClientHeight, LatestHeight: testClientHeight, ProofSpecs: commitmenttypes.GetSDKSpecs(), UpgradePath: ibctesting.UpgradePath} + }, + expError: clienttypes.ErrClientFrozen, + }, + { + name: "incorrect chainID", + malleate: func() { + clientState = ibctm.NewClientState("gaiatestnet", ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "invalid client height", + malleate: func() { + clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, clienttypes.GetSelfHeight(suite.chainA.GetContext()).Increment().(clienttypes.Height), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "invalid client type", + malleate: func() { + clientState = solomachine.NewClientState(0, &solomachine.ConsensusState{}) + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "invalid client revision", + malleate: func() { + clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, clienttypes.NewHeight(1, 5), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "invalid proof specs", + malleate: func() { + clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, nil, ibctesting.UpgradePath) + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "invalid trust level", + malleate: func() { + clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.Fraction{Numerator: 0, Denominator: 1}, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "invalid unbonding period", + malleate: func() { + clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+10, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "invalid trusting period", + malleate: func() { + clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, ubdPeriod+10, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "invalid upgrade path", + malleate: func() { + clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), []string{"bad", "upgrade", "path"}) + }, + expError: clienttypes.ErrInvalidClient, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() + + tc.malleate() + + err := suite.chainA.App.GetIBCKeeper().ClientKeeper.ValidateSelfClient(suite.chainA.GetContext(), clientState) + + expPass := tc.expError == nil + if expPass { + suite.Require().NoError(err, "expected valid client for case: %s", tc.name) + } else { + suite.Require().Error(err, "expected invalid client for case: %s", tc.name) + } + }) + } +} diff --git a/modules/light-clients/08-wasm/testing/simapp/app.go b/modules/light-clients/08-wasm/testing/simapp/app.go index 7b53ab827f8..df3e653a5ea 100644 --- a/modules/light-clients/08-wasm/testing/simapp/app.go +++ b/modules/light-clients/08-wasm/testing/simapp/app.go @@ -418,6 +418,7 @@ func NewSimApp( app.IBCKeeper = ibckeeper.NewKeeper( appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(), ) + // Register the proposal types // Deprecated: Avoid adding new handlers, instead use the new proposal flow // by granting the governance module the right to execute the message. @@ -425,7 +426,7 @@ func NewSimApp( 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(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(*app.IBCKeeper.ClientKeeper)) govConfig := govtypes.DefaultConfig() /* Example of setting gov params: diff --git a/modules/light-clients/08-wasm/types/consensus_host.go b/modules/light-clients/08-wasm/types/consensus_host.go new file mode 100644 index 00000000000..3325fdbabc8 --- /dev/null +++ b/modules/light-clients/08-wasm/types/consensus_host.go @@ -0,0 +1,75 @@ +package types + +import ( + errorsmod "cosmossdk.io/errors" + + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + "github.com/cosmos/ibc-go/v8/modules/core/exported" +) + +// WasmConsensusHost implements the 02-client types.ConsensusHost interface. +type WasmConsensusHost struct { + cdc codec.BinaryCodec + delegate clienttypes.ConsensusHost +} + +var _ clienttypes.ConsensusHost = (*WasmConsensusHost)(nil) + +// NewWasmConsensusHost creates and returns a new ConsensusHost for wasm wrapped consensus client state and consensus state self validation. +func NewWasmConsensusHost(cdc codec.BinaryCodec, delegate clienttypes.ConsensusHost) *WasmConsensusHost { + return &WasmConsensusHost{ + cdc: cdc, + delegate: delegate, + } +} + +// GetSelfConsensusState implements the 02-client types.ConsensusHost interface. +func (w *WasmConsensusHost) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { + consensusState, err := w.delegate.GetSelfConsensusState(ctx, height) + if err != nil { + return nil, err + } + + // encode consensusState to wasm.ConsensusState.Data + bz, err := w.cdc.MarshalInterface(consensusState) + if err != nil { + return nil, err + } + + wasmConsensusState := &ConsensusState{ + Data: bz, + } + + return wasmConsensusState, nil +} + +// ValidateSelfClient implements the 02-client types.ConsensusHost interface. +func (w *WasmConsensusHost) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error { + wasmClientState, ok := clientState.(*ClientState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client must be a wasm client, expected: %T, got: %T", ClientState{}, wasmClientState) + } + + if w == nil { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm consensus host is nil") + } + + if w.cdc == nil { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm consensus host cdc is nil") + } + + if wasmClientState.Data == nil { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm client state data is nil") + } + + // unmarshal the wasmClientState bytes into the ClientState interface and call self validation + var unwrappedClientState exported.ClientState + if err := w.cdc.UnmarshalInterface(wasmClientState.Data, &unwrappedClientState); err != nil { + return err + } + + return w.delegate.ValidateSelfClient(ctx, unwrappedClientState) +} diff --git a/testing/mock/consensus_host.go b/testing/mock/consensus_host.go new file mode 100644 index 00000000000..4ce25373a53 --- /dev/null +++ b/testing/mock/consensus_host.go @@ -0,0 +1,31 @@ +package mock + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + "github.com/cosmos/ibc-go/v8/modules/core/exported" +) + +var _ clienttypes.ConsensusHost = (*ConsensusHost)(nil) + +type ConsensusHost struct { + GetSelfConsensusStateFn func(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) + ValidateSelfClientFn func(ctx sdk.Context, clientState exported.ClientState) error +} + +func (cv *ConsensusHost) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { + if cv.GetSelfConsensusStateFn == nil { + return nil, nil + } + + return cv.GetSelfConsensusStateFn(ctx, height) +} + +func (cv *ConsensusHost) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error { + if cv.ValidateSelfClientFn == nil { + return nil + } + + return cv.ValidateSelfClientFn(ctx, clientState) +} diff --git a/testing/simapp/app.go b/testing/simapp/app.go index a451470e401..5e402d39c32 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -417,7 +417,7 @@ func NewSimApp( 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(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(*app.IBCKeeper.ClientKeeper)) govConfig := govtypes.DefaultConfig() /* Example of setting gov params: diff --git a/testing/simapp/upgrades.go b/testing/simapp/upgrades.go index 58139ee6271..2c0ddaf1071 100644 --- a/testing/simapp/upgrades.go +++ b/testing/simapp/upgrades.go @@ -42,7 +42,7 @@ func (app *SimApp) registerUpgradeHandlers() { app.ModuleManager, app.configurator, app.appCodec, - app.IBCKeeper.ClientKeeper, + *app.IBCKeeper.ClientKeeper, app.ConsensusParamsKeeper, app.ParamsKeeper, ), @@ -50,7 +50,7 @@ func (app *SimApp) registerUpgradeHandlers() { app.UpgradeKeeper.SetUpgradeHandler( upgrades.V7_1, - upgrades.CreateV7LocalhostUpgradeHandler(app.ModuleManager, app.configurator, app.IBCKeeper.ClientKeeper), + upgrades.CreateV7LocalhostUpgradeHandler(app.ModuleManager, app.configurator, *app.IBCKeeper.ClientKeeper), ) app.UpgradeKeeper.SetUpgradeHandler( diff --git a/testing/simapp/upgrades/upgrades.go b/testing/simapp/upgrades/upgrades.go index 03ddbb75502..0509ae15466 100644 --- a/testing/simapp/upgrades/upgrades.go +++ b/testing/simapp/upgrades/upgrades.go @@ -79,7 +79,7 @@ func CreateV7UpgradeHandler( return func(ctx context.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) { sdkCtx := sdk.UnwrapSDKContext(ctx) // OPTIONAL: prune expired tendermint consensus states to save storage space - if _, err := ibctmmigrations.PruneExpiredConsensusStates(sdkCtx, cdc, clientKeeper); err != nil { + if _, err := ibctmmigrations.PruneExpiredConsensusStates(sdkCtx, cdc, &clientKeeper); err != nil { return nil, err } From 335812d165fe603de6fc1ea1f8140a37eb9fec81 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 25 Mar 2024 17:58:04 +0100 Subject: [PATCH 02/12] chore: rm stakingKeeper field from clientKeeper --- modules/core/02-client/keeper/keeper.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/core/02-client/keeper/keeper.go b/modules/core/02-client/keeper/keeper.go index 1ab1619f1cb..1e5c6f0ca50 100644 --- a/modules/core/02-client/keeper/keeper.go +++ b/modules/core/02-client/keeper/keeper.go @@ -29,7 +29,6 @@ type Keeper struct { router *types.Router consensusHost types.ConsensusHost legacySubspace types.ParamSubspace - stakingKeeper types.StakingKeeper upgradeKeeper types.UpgradeKeeper } @@ -45,7 +44,6 @@ func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace ty router: router, consensusHost: ibctm.NewConsensusHost(sk), legacySubspace: legacySubspace, - stakingKeeper: sk, upgradeKeeper: uk, } } From 25ca20855a6b034ca6797e80973d529ccd21cf85 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 26 Mar 2024 10:42:47 +0100 Subject: [PATCH 03/12] imp: tidy up pointer dereferencing --- modules/core/02-client/abci.go | 2 +- modules/core/02-client/abci_test.go | 8 +++---- modules/core/02-client/keeper/migrations.go | 8 +++---- .../core/02-client/keeper/migrations_test.go | 2 +- modules/core/02-client/proposal_handler.go | 2 +- .../core/02-client/proposal_handler_test.go | 2 +- modules/core/keeper/migrations.go | 23 ------------------- modules/core/module.go | 4 ++-- .../08-wasm/testing/simapp/app.go | 2 +- testing/simapp/app.go | 2 +- 10 files changed, 16 insertions(+), 39 deletions(-) delete mode 100644 modules/core/keeper/migrations.go diff --git a/modules/core/02-client/abci.go b/modules/core/02-client/abci.go index 4fbfc7fec1c..44dfca138b2 100644 --- a/modules/core/02-client/abci.go +++ b/modules/core/02-client/abci.go @@ -9,7 +9,7 @@ import ( ) // BeginBlocker is used to perform IBC client upgrades -func BeginBlocker(ctx sdk.Context, k keeper.Keeper) { +func BeginBlocker(ctx sdk.Context, k *keeper.Keeper) { plan, err := k.GetUpgradePlan(ctx) if err == nil { // Once we are at the last block this chain will commit, set the upgraded consensus state diff --git a/modules/core/02-client/abci_test.go b/modules/core/02-client/abci_test.go index fbc36465b34..b5a78320ad1 100644 --- a/modules/core/02-client/abci_test.go +++ b/modules/core/02-client/abci_test.go @@ -44,7 +44,7 @@ func (suite *ClientTestSuite) TestBeginBlocker() { suite.coordinator.CommitBlock(suite.chainA, suite.chainB) suite.Require().NotPanics(func() { - client.BeginBlocker(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper) + client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) }, "BeginBlocker shouldn't panic") } } @@ -69,7 +69,7 @@ func (suite *ClientTestSuite) TestBeginBlockerConsensusState() { err := suite.chainA.GetSimApp().UpgradeKeeper.SetUpgradedClient(newCtx, plan.Height, []byte("client state")) suite.Require().NoError(err) - client.BeginBlocker(newCtx, *suite.chainA.App.GetIBCKeeper().ClientKeeper) + client.BeginBlocker(newCtx, suite.chainA.App.GetIBCKeeper().ClientKeeper) // plan Height is at ctx.BlockHeight+1 consState, err := suite.chainA.GetSimApp().UpgradeKeeper.GetUpgradedConsensusState(newCtx, plan.Height) @@ -101,7 +101,7 @@ func (suite *ClientTestSuite) TestBeginBlockerUpgradeEvents() { cacheCtx, writeCache := suite.chainA.GetContext().CacheContext() - client.BeginBlocker(cacheCtx, *suite.chainA.App.GetIBCKeeper().ClientKeeper) + client.BeginBlocker(cacheCtx, suite.chainA.App.GetIBCKeeper().ClientKeeper) writeCache() suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, true) @@ -109,7 +109,7 @@ func (suite *ClientTestSuite) TestBeginBlockerUpgradeEvents() { func (suite *ClientTestSuite) TestBeginBlockerUpgradeEventsAbsence() { cacheCtx, writeCache := suite.chainA.GetContext().CacheContext() - client.BeginBlocker(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper) + client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) writeCache() suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, false) } diff --git a/modules/core/02-client/keeper/migrations.go b/modules/core/02-client/keeper/migrations.go index 818883248c4..73bee079231 100644 --- a/modules/core/02-client/keeper/migrations.go +++ b/modules/core/02-client/keeper/migrations.go @@ -9,11 +9,11 @@ import ( // Migrator is a struct for handling in-place store migrations. type Migrator struct { - keeper Keeper + keeper *Keeper } // NewMigrator returns a new Migrator. -func NewMigrator(keeper Keeper) Migrator { +func NewMigrator(keeper *Keeper) Migrator { return Migrator{keeper: keeper} } @@ -24,13 +24,13 @@ func NewMigrator(keeper Keeper) Migrator { // - removes the localhost client // - asserts that existing tendermint clients are properly registered on the chain codec func (m Migrator) Migrate2to3(ctx sdk.Context) error { - return v7.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc, &m.keeper) + return v7.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc, m.keeper) } // Migrate3to4 migrates from consensus version 3 to 4. // This migration enables the localhost client. func (m Migrator) Migrate3to4(ctx sdk.Context) error { - return v7.MigrateLocalhostClient(ctx, &m.keeper) + return v7.MigrateLocalhostClient(ctx, m.keeper) } // MigrateParams migrates from consensus version 4 to 5. diff --git a/modules/core/02-client/keeper/migrations_test.go b/modules/core/02-client/keeper/migrations_test.go index cf8abb2d312..7bdd26b8a12 100644 --- a/modules/core/02-client/keeper/migrations_test.go +++ b/modules/core/02-client/keeper/migrations_test.go @@ -32,7 +32,7 @@ func (suite *KeeperTestSuite) TestMigrateParams() { tc.malleate() ctx := suite.chainA.GetContext() - migrator := keeper.NewMigrator(*suite.chainA.GetSimApp().IBCKeeper.ClientKeeper) + migrator := keeper.NewMigrator(suite.chainA.GetSimApp().IBCKeeper.ClientKeeper) err := migrator.MigrateParams(ctx) suite.Require().NoError(err) diff --git a/modules/core/02-client/proposal_handler.go b/modules/core/02-client/proposal_handler.go index a1044bf33fc..279f9a0be41 100644 --- a/modules/core/02-client/proposal_handler.go +++ b/modules/core/02-client/proposal_handler.go @@ -15,7 +15,7 @@ import ( // // 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 +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: diff --git a/modules/core/02-client/proposal_handler_test.go b/modules/core/02-client/proposal_handler_test.go index 2d6a963e4d8..108b886add6 100644 --- a/modules/core/02-client/proposal_handler_test.go +++ b/modules/core/02-client/proposal_handler_test.go @@ -80,7 +80,7 @@ func (suite *ClientTestSuite) TestNewClientUpdateProposalHandler() { tc.malleate() - proposalHandler := client.NewClientProposalHandler(*suite.chainA.App.GetIBCKeeper().ClientKeeper) + proposalHandler := client.NewClientProposalHandler(suite.chainA.App.GetIBCKeeper().ClientKeeper) err = proposalHandler(suite.chainA.GetContext(), content) diff --git a/modules/core/keeper/migrations.go b/modules/core/keeper/migrations.go deleted file mode 100644 index 82aa3e10ac4..00000000000 --- a/modules/core/keeper/migrations.go +++ /dev/null @@ -1,23 +0,0 @@ -package keeper - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - - clientkeeper "github.com/cosmos/ibc-go/v8/modules/core/02-client/keeper" -) - -// Migrator is a struct for handling in-place store migrations. -type Migrator struct { - keeper Keeper -} - -// NewMigrator returns a new Migrator. -func NewMigrator(keeper Keeper) Migrator { - return Migrator{keeper: keeper} -} - -// Migrate2to3 migrates from version 2 to 3. See 02-client keeper function Migrate2to3. -func (m Migrator) Migrate2to3(ctx sdk.Context) error { - clientMigrator := clientkeeper.NewMigrator(*m.keeper.ClientKeeper) - return clientMigrator.Migrate2to3(ctx) -} diff --git a/modules/core/module.go b/modules/core/module.go index 374bfaeda92..e7f6ebd4a94 100644 --- a/modules/core/module.go +++ b/modules/core/module.go @@ -133,7 +133,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { channeltypes.RegisterMsgServer(cfg.MsgServer(), am.keeper) types.RegisterQueryService(cfg.QueryServer(), am.keeper) - clientMigrator := clientkeeper.NewMigrator(*am.keeper.ClientKeeper) + clientMigrator := clientkeeper.NewMigrator(am.keeper.ClientKeeper) if err := cfg.RegisterMigration(exported.ModuleName, 2, clientMigrator.Migrate2to3); err != nil { panic(err) } @@ -188,7 +188,7 @@ func (AppModule) ConsensusVersion() uint64 { return 6 } // BeginBlock returns the begin blocker for the ibc module. func (am AppModule) BeginBlock(ctx context.Context) error { - ibcclient.BeginBlocker(sdk.UnwrapSDKContext(ctx), *am.keeper.ClientKeeper) + ibcclient.BeginBlocker(sdk.UnwrapSDKContext(ctx), am.keeper.ClientKeeper) return nil } diff --git a/modules/light-clients/08-wasm/testing/simapp/app.go b/modules/light-clients/08-wasm/testing/simapp/app.go index df3e653a5ea..4ce02317f39 100644 --- a/modules/light-clients/08-wasm/testing/simapp/app.go +++ b/modules/light-clients/08-wasm/testing/simapp/app.go @@ -426,7 +426,7 @@ func NewSimApp( 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(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper)) govConfig := govtypes.DefaultConfig() /* Example of setting gov params: diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 5e402d39c32..a451470e401 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -417,7 +417,7 @@ func NewSimApp( 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(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper)) govConfig := govtypes.DefaultConfig() /* Example of setting gov params: From 002d4502edc6ffa78d239840bfbc8115596dda35 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 26 Mar 2024 13:36:16 +0100 Subject: [PATCH 04/12] chore: define StakingKeeper expected interface in 07-tendermint --- .../light-clients/07-tendermint/consensus_host.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/modules/light-clients/07-tendermint/consensus_host.go b/modules/light-clients/07-tendermint/consensus_host.go index d0c62c75c08..0ce7bb4e802 100644 --- a/modules/light-clients/07-tendermint/consensus_host.go +++ b/modules/light-clients/07-tendermint/consensus_host.go @@ -1,12 +1,15 @@ package tendermint import ( + "context" "reflect" + "time" errorsmod "cosmossdk.io/errors" upgradetypes "cosmossdk.io/x/upgrade/types" sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/cometbft/cometbft/light" @@ -18,9 +21,15 @@ import ( var _ clienttypes.ConsensusHost = (*ConsensusHost)(nil) -// ConsensusHost implements the 02-client clienttypes.ConsensusHost interface +// ConsensusHost implements the 02-client clienttypes.ConsensusHost interface. type ConsensusHost struct { - stakingKeeper clienttypes.StakingKeeper + stakingKeeper StakingKeeper +} + +// StakingKeeper defines an expected interface for the tendermint ConsensusHost. +type StakingKeeper interface { + GetHistoricalInfo(ctx context.Context, height int64) (stakingtypes.HistoricalInfo, error) + UnbondingTime(ctx context.Context) (time.Duration, error) } // NewConsensusHost creates and returns a new ConsensusHost for tendermint consensus. From ef6c6250ffde05d2cc7b27858d6fc606e85c46b2 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 26 Mar 2024 15:34:44 +0100 Subject: [PATCH 05/12] chore: move nil checks to top of methods in 08-wasm --- .../08-wasm/types/consensus_host.go | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/modules/light-clients/08-wasm/types/consensus_host.go b/modules/light-clients/08-wasm/types/consensus_host.go index 3325fdbabc8..185a6f9065e 100644 --- a/modules/light-clients/08-wasm/types/consensus_host.go +++ b/modules/light-clients/08-wasm/types/consensus_host.go @@ -28,6 +28,14 @@ func NewWasmConsensusHost(cdc codec.BinaryCodec, delegate clienttypes.ConsensusH // GetSelfConsensusState implements the 02-client types.ConsensusHost interface. func (w *WasmConsensusHost) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { + if w.cdc == nil { + return nil, errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm consensus host codec is nil") + } + + if w.delegate == nil { + return nil, errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm delegate consensus host is nil") + } + consensusState, err := w.delegate.GetSelfConsensusState(ctx, height) if err != nil { return nil, err @@ -48,17 +56,17 @@ func (w *WasmConsensusHost) GetSelfConsensusState(ctx sdk.Context, height export // ValidateSelfClient implements the 02-client types.ConsensusHost interface. func (w *WasmConsensusHost) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error { - wasmClientState, ok := clientState.(*ClientState) - if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client must be a wasm client, expected: %T, got: %T", ClientState{}, wasmClientState) + if w.cdc == nil { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm consensus host codec is nil") } - if w == nil { - return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm consensus host is nil") + if w.delegate == nil { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm delegate consensus host is nil") } - if w.cdc == nil { - return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm consensus host cdc is nil") + wasmClientState, ok := clientState.(*ClientState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client must be a wasm client, expected: %T, got: %T", ClientState{}, wasmClientState) } if wasmClientState.Data == nil { From fc4d3495144a61cb0a8f9ddcea99e4c406fd86be Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 2 Apr 2024 16:37:47 +0200 Subject: [PATCH 06/12] test: add pruned historical info testcase to tendermint consensus host --- .../light-clients/07-tendermint/consensus_host_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/modules/light-clients/07-tendermint/consensus_host_test.go b/modules/light-clients/07-tendermint/consensus_host_test.go index 21e390cbf03..efcebea756a 100644 --- a/modules/light-clients/07-tendermint/consensus_host_test.go +++ b/modules/light-clients/07-tendermint/consensus_host_test.go @@ -35,6 +35,15 @@ func (suite *TendermintTestSuite) TestGetSelfConsensusState() { }, expError: stakingtypes.ErrNoHistoricalInfo, }, + { + name: "pruned historical info", + malleate: func() { + height = clienttypes.NewHeight(1, uint64(suite.chainA.GetContext().BlockHeight())-1) + + suite.chainA.GetSimApp().StakingKeeper.DeleteHistoricalInfo(suite.chainA.GetContext(), int64(height.GetRevisionHeight())) + }, + expError: stakingtypes.ErrNoHistoricalInfo, + }, { name: "custom consensus host: failure", malleate: func() { From 437adda21626a2efc37e4b87eb2fde190cd93ae6 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 2 Apr 2024 19:29:24 +0200 Subject: [PATCH 07/12] lint: check err return in test Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- modules/light-clients/07-tendermint/consensus_host_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/light-clients/07-tendermint/consensus_host_test.go b/modules/light-clients/07-tendermint/consensus_host_test.go index efcebea756a..860e63fffb5 100644 --- a/modules/light-clients/07-tendermint/consensus_host_test.go +++ b/modules/light-clients/07-tendermint/consensus_host_test.go @@ -40,7 +40,8 @@ func (suite *TendermintTestSuite) TestGetSelfConsensusState() { malleate: func() { height = clienttypes.NewHeight(1, uint64(suite.chainA.GetContext().BlockHeight())-1) - suite.chainA.GetSimApp().StakingKeeper.DeleteHistoricalInfo(suite.chainA.GetContext(), int64(height.GetRevisionHeight())) + err := suite.chainA.GetSimApp().StakingKeeper.DeleteHistoricalInfo(suite.chainA.GetContext(), int64(height.GetRevisionHeight())) + suite.Require().NoError(err) }, expError: stakingtypes.ErrNoHistoricalInfo, }, From 81394e7b58ca3fd8f2746161bff6344c30609814 Mon Sep 17 00:00:00 2001 From: chatton Date: Wed, 3 Apr 2024 10:52:05 +0100 Subject: [PATCH 08/12] test: add tests for TestValidateSelfClient --- .../08-wasm/types/consensus_host_test.go | 196 ++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100644 modules/light-clients/08-wasm/types/consensus_host_test.go diff --git a/modules/light-clients/08-wasm/types/consensus_host_test.go b/modules/light-clients/08-wasm/types/consensus_host_test.go new file mode 100644 index 00000000000..d8edb8b82bb --- /dev/null +++ b/modules/light-clients/08-wasm/types/consensus_host_test.go @@ -0,0 +1,196 @@ +package types_test + +import ( + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" + "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + "github.com/cosmos/ibc-go/v8/modules/core/exported" + solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine" + ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" + "github.com/cosmos/ibc-go/v8/testing/mock" +) + +func (suite *TypesTestSuite) TestGetSelfConsensusState() { + var height clienttypes.Height + + cases := []struct { + name string + malleate func() + expError error + }{ + { + name: "zero height", + malleate: func() { + height = clienttypes.ZeroHeight() + }, + expError: clienttypes.ErrInvalidHeight, + }, + { + name: "height > latest height", + malleate: func() { + height = clienttypes.NewHeight(1, uint64(suite.chainA.GetContext().BlockHeight())+1) + }, + expError: stakingtypes.ErrNoHistoricalInfo, + }, + { + name: "pruned historical info", + malleate: func() { + height = clienttypes.NewHeight(1, uint64(suite.chainA.GetContext().BlockHeight())-1) + + err := suite.chainA.GetSimApp().StakingKeeper.DeleteHistoricalInfo(suite.chainA.GetContext(), int64(height.GetRevisionHeight())) + suite.Require().NoError(err) + }, + expError: stakingtypes.ErrNoHistoricalInfo, + }, + { + name: "custom consensus host: failure", + malleate: func() { + consensusHost := &mock.ConsensusHost{ + GetSelfConsensusStateFn: func(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { + return nil, mock.MockApplicationCallbackError + }, + } + suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.SetSelfConsensusHost(consensusHost) + }, + expError: mock.MockApplicationCallbackError, + }, + { + name: "custom consensus host: success", + malleate: func() { + consensusHost := &mock.ConsensusHost{ + GetSelfConsensusStateFn: func(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { + return &solomachine.ConsensusState{}, nil + }, + } + suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.SetSelfConsensusHost(consensusHost) + }, + expError: nil, + }, + { + name: "latest height - 1", + malleate: func() { + height = clienttypes.NewHeight(1, uint64(suite.chainA.GetContext().BlockHeight())-1) + }, + expError: nil, + }, + { + name: "latest height", + malleate: func() { + // historical info is set on BeginBlock in x/staking, which is now encapsulated within the FinalizeBlock abci method, + // thus, we do not have historical info for current height due to how the ibctesting library operates. + // ibctesting calls app.Commit() as a final step on NextBlock and we invoke test code before FinalizeBlock is called at the current height once again. + err := suite.chainA.GetSimApp().StakingKeeper.TrackHistoricalInfo(suite.chainA.GetContext()) + suite.Require().NoError(err) + + height = clienttypes.GetSelfHeight(suite.chainA.GetContext()) + }, + expError: nil, + }, + } + + for i, tc := range cases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() + + height = clienttypes.ZeroHeight() + + tc.malleate() + + cs, err := suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.GetSelfConsensusState(suite.chainA.GetContext(), height) + + expPass := tc.expError == nil + if expPass { + suite.Require().NoError(err, "Case %d should have passed: %s", i, tc.name) + suite.Require().NotNil(cs, "Case %d should have passed: %s", i, tc.name) + } else { + suite.Require().ErrorIs(err, tc.expError, "Case %d should have failed: %s", i, tc.name) + suite.Require().Nil(cs, "Case %d should have failed: %s", i, tc.name) + } + }) + } +} + +func (suite *TypesTestSuite) TestValidateSelfClient() { + var clientState exported.ClientState + var consensusHost *mock.ConsensusHost + var cdc codec.BinaryCodec + + testCases := []struct { + name string + malleate func() + expError error + }{ + { + name: "success", + malleate: func() {}, + expError: nil, + }, + { + name: "failure: invalid data", + malleate: func() { + clientState = types.NewClientState(nil, wasmtesting.Code, clienttypes.ZeroHeight()) + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "failure: invalid delegate", + malleate: func() { + consensusHost = nil + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "failure: invalid codec", + malleate: func() { + cdc = nil + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "failure: invalid clientstate type", + malleate: func() { + clientState = &ibctm.ClientState{} + }, + expError: clienttypes.ErrInvalidClient, + }, + { + name: "failure: delegate error propagates", + malleate: func() { + consensusHost.ValidateSelfClientFn = func(ctx sdk.Context, clientState exported.ClientState) error { + return mock.MockApplicationCallbackError + } + }, + expError: mock.MockApplicationCallbackError, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() + + clientState = types.NewClientState(wasmtesting.CreateMockClientStateBz(suite.chainA.Codec, suite.checksum), wasmtesting.Code, clienttypes.ZeroHeight()) + consensusHost = &mock.ConsensusHost{} + cdc = suite.chainA.Codec + + tc.malleate() + + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetSelfConsensusHost( + types.NewWasmConsensusHost(cdc, consensusHost), + ) + + err := suite.chainA.App.GetIBCKeeper().ClientKeeper.ValidateSelfClient(suite.chainA.GetContext(), clientState) + + expPass := tc.expError == nil + if expPass { + suite.Require().NoError(err, "expected valid client for case: %s", tc.name) + } else { + suite.Require().ErrorIs(err, tc.expError) + } + }) + } +} From cf239f58424c773363b5fddbae879cb7eba6d15a Mon Sep 17 00:00:00 2001 From: chatton Date: Wed, 3 Apr 2024 11:00:26 +0100 Subject: [PATCH 09/12] test: corrected invalid delegate test --- .../08-wasm/types/consensus_host_test.go | 105 +----------------- 1 file changed, 3 insertions(+), 102 deletions(-) diff --git a/modules/light-clients/08-wasm/types/consensus_host_test.go b/modules/light-clients/08-wasm/types/consensus_host_test.go index d8edb8b82bb..e5a1f59ebdb 100644 --- a/modules/light-clients/08-wasm/types/consensus_host_test.go +++ b/modules/light-clients/08-wasm/types/consensus_host_test.go @@ -3,120 +3,21 @@ package types_test import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" "github.com/cosmos/ibc-go/v8/modules/core/exported" - solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" "github.com/cosmos/ibc-go/v8/testing/mock" ) func (suite *TypesTestSuite) TestGetSelfConsensusState() { - var height clienttypes.Height - cases := []struct { - name string - malleate func() - expError error - }{ - { - name: "zero height", - malleate: func() { - height = clienttypes.ZeroHeight() - }, - expError: clienttypes.ErrInvalidHeight, - }, - { - name: "height > latest height", - malleate: func() { - height = clienttypes.NewHeight(1, uint64(suite.chainA.GetContext().BlockHeight())+1) - }, - expError: stakingtypes.ErrNoHistoricalInfo, - }, - { - name: "pruned historical info", - malleate: func() { - height = clienttypes.NewHeight(1, uint64(suite.chainA.GetContext().BlockHeight())-1) - - err := suite.chainA.GetSimApp().StakingKeeper.DeleteHistoricalInfo(suite.chainA.GetContext(), int64(height.GetRevisionHeight())) - suite.Require().NoError(err) - }, - expError: stakingtypes.ErrNoHistoricalInfo, - }, - { - name: "custom consensus host: failure", - malleate: func() { - consensusHost := &mock.ConsensusHost{ - GetSelfConsensusStateFn: func(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { - return nil, mock.MockApplicationCallbackError - }, - } - suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.SetSelfConsensusHost(consensusHost) - }, - expError: mock.MockApplicationCallbackError, - }, - { - name: "custom consensus host: success", - malleate: func() { - consensusHost := &mock.ConsensusHost{ - GetSelfConsensusStateFn: func(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { - return &solomachine.ConsensusState{}, nil - }, - } - suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.SetSelfConsensusHost(consensusHost) - }, - expError: nil, - }, - { - name: "latest height - 1", - malleate: func() { - height = clienttypes.NewHeight(1, uint64(suite.chainA.GetContext().BlockHeight())-1) - }, - expError: nil, - }, - { - name: "latest height", - malleate: func() { - // historical info is set on BeginBlock in x/staking, which is now encapsulated within the FinalizeBlock abci method, - // thus, we do not have historical info for current height due to how the ibctesting library operates. - // ibctesting calls app.Commit() as a final step on NextBlock and we invoke test code before FinalizeBlock is called at the current height once again. - err := suite.chainA.GetSimApp().StakingKeeper.TrackHistoricalInfo(suite.chainA.GetContext()) - suite.Require().NoError(err) - - height = clienttypes.GetSelfHeight(suite.chainA.GetContext()) - }, - expError: nil, - }, - } - - for i, tc := range cases { - tc := tc - suite.Run(tc.name, func() { - suite.SetupTest() - - height = clienttypes.ZeroHeight() - - tc.malleate() - - cs, err := suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.GetSelfConsensusState(suite.chainA.GetContext(), height) - - expPass := tc.expError == nil - if expPass { - suite.Require().NoError(err, "Case %d should have passed: %s", i, tc.name) - suite.Require().NotNil(cs, "Case %d should have passed: %s", i, tc.name) - } else { - suite.Require().ErrorIs(err, tc.expError, "Case %d should have failed: %s", i, tc.name) - suite.Require().Nil(cs, "Case %d should have failed: %s", i, tc.name) - } - }) - } } func (suite *TypesTestSuite) TestValidateSelfClient() { var clientState exported.ClientState - var consensusHost *mock.ConsensusHost + var consensusHost clienttypes.ConsensusHost var cdc codec.BinaryCodec testCases := []struct { @@ -160,7 +61,7 @@ func (suite *TypesTestSuite) TestValidateSelfClient() { { name: "failure: delegate error propagates", malleate: func() { - consensusHost.ValidateSelfClientFn = func(ctx sdk.Context, clientState exported.ClientState) error { + consensusHost.(*mock.ConsensusHost).ValidateSelfClientFn = func(ctx sdk.Context, clientState exported.ClientState) error { return mock.MockApplicationCallbackError } }, @@ -189,7 +90,7 @@ func (suite *TypesTestSuite) TestValidateSelfClient() { if expPass { suite.Require().NoError(err, "expected valid client for case: %s", tc.name) } else { - suite.Require().ErrorIs(err, tc.expError) + suite.Require().ErrorIs(err, tc.expError, "expected %s got %s", tc.expError, err) } }) } From afb3de25c70b58439e0c870033de1fc89e5f75d6 Mon Sep 17 00:00:00 2001 From: chatton Date: Wed, 3 Apr 2024 12:27:24 +0100 Subject: [PATCH 10/12] chore: updating tests for TestGetSelfConsensusState and TestValidateSelfClient --- .../08-wasm/types/consensus_host.go | 22 ++--- .../08-wasm/types/consensus_host_test.go | 88 +++++++++++++++---- 2 files changed, 81 insertions(+), 29 deletions(-) diff --git a/modules/light-clients/08-wasm/types/consensus_host.go b/modules/light-clients/08-wasm/types/consensus_host.go index 185a6f9065e..c0a936efce0 100644 --- a/modules/light-clients/08-wasm/types/consensus_host.go +++ b/modules/light-clients/08-wasm/types/consensus_host.go @@ -1,6 +1,8 @@ package types import ( + "fmt" + errorsmod "cosmossdk.io/errors" "github.com/cosmos/cosmos-sdk/codec" @@ -19,23 +21,23 @@ type WasmConsensusHost struct { var _ clienttypes.ConsensusHost = (*WasmConsensusHost)(nil) // NewWasmConsensusHost creates and returns a new ConsensusHost for wasm wrapped consensus client state and consensus state self validation. -func NewWasmConsensusHost(cdc codec.BinaryCodec, delegate clienttypes.ConsensusHost) *WasmConsensusHost { +func NewWasmConsensusHost(cdc codec.BinaryCodec, delegate clienttypes.ConsensusHost) (*WasmConsensusHost, error) { + if cdc == nil { + return nil, fmt.Errorf("wasm consensus host codec is nil") + } + + if delegate == nil { + return nil, fmt.Errorf("wasm delegate consensus host is nil") + } + return &WasmConsensusHost{ cdc: cdc, delegate: delegate, - } + }, nil } // GetSelfConsensusState implements the 02-client types.ConsensusHost interface. func (w *WasmConsensusHost) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { - if w.cdc == nil { - return nil, errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm consensus host codec is nil") - } - - if w.delegate == nil { - return nil, errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm delegate consensus host is nil") - } - consensusState, err := w.delegate.GetSelfConsensusState(ctx, height) if err != nil { return nil, err diff --git a/modules/light-clients/08-wasm/types/consensus_host_test.go b/modules/light-clients/08-wasm/types/consensus_host_test.go index e5a1f59ebdb..e6a43e95b97 100644 --- a/modules/light-clients/08-wasm/types/consensus_host_test.go +++ b/modules/light-clients/08-wasm/types/consensus_host_test.go @@ -1,8 +1,8 @@ package types_test import ( - "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" @@ -12,13 +12,74 @@ import ( ) func (suite *TypesTestSuite) TestGetSelfConsensusState() { + var height clienttypes.Height + var consensusHost clienttypes.ConsensusHost + var consensusState exported.ConsensusState + + cases := []struct { + name string + malleate func() + expError error + }{ + { + name: "success", + malleate: func() {}, + expError: nil, + }, + { + name: "failure: delegate error", + malleate: func() { + consensusHost.(*mock.ConsensusHost).GetSelfConsensusStateFn = func(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { + return nil, mock.MockApplicationCallbackError + } + }, + expError: mock.MockApplicationCallbackError, + }, + } + + for i, tc := range cases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() + height = clienttypes.ZeroHeight() + + wrappedClientConsensusStateBz := clienttypes.MustMarshalConsensusState(suite.chainA.App.AppCodec(), wasmtesting.MockTendermintClientConsensusState) + consensusState = types.NewConsensusState(wrappedClientConsensusStateBz) + + consensusHost = &mock.ConsensusHost{ + GetSelfConsensusStateFn: func(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) { + return consensusState, nil + }, + } + + tc.malleate() + + var err error + consensusHost, err = types.NewWasmConsensusHost(suite.chainA.Codec, consensusHost) + suite.Require().NoError(err) + + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetSelfConsensusHost( + consensusHost, + ) + cs, err := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetSelfConsensusState(suite.chainA.GetContext(), height) + + expPass := tc.expError == nil + if expPass { + suite.Require().NoError(err, "Case %d should have passed: %s", i, tc.name) + suite.Require().NotNil(cs, "Case %d should have passed: %s", i, tc.name) + suite.Require().NotNil(cs.(*types.ConsensusState).Data, "Case %d should have passed: %s", i, tc.name) + } else { + suite.Require().ErrorIs(err, tc.expError, "Case %d should have failed: %s", i, tc.name) + suite.Require().Nil(cs, "Case %d should have failed: %s", i, tc.name) + } + }) + } } func (suite *TypesTestSuite) TestValidateSelfClient() { var clientState exported.ClientState var consensusHost clienttypes.ConsensusHost - var cdc codec.BinaryCodec testCases := []struct { name string @@ -37,20 +98,6 @@ func (suite *TypesTestSuite) TestValidateSelfClient() { }, expError: clienttypes.ErrInvalidClient, }, - { - name: "failure: invalid delegate", - malleate: func() { - consensusHost = nil - }, - expError: clienttypes.ErrInvalidClient, - }, - { - name: "failure: invalid codec", - malleate: func() { - cdc = nil - }, - expError: clienttypes.ErrInvalidClient, - }, { name: "failure: invalid clientstate type", malleate: func() { @@ -76,15 +123,18 @@ func (suite *TypesTestSuite) TestValidateSelfClient() { clientState = types.NewClientState(wasmtesting.CreateMockClientStateBz(suite.chainA.Codec, suite.checksum), wasmtesting.Code, clienttypes.ZeroHeight()) consensusHost = &mock.ConsensusHost{} - cdc = suite.chainA.Codec tc.malleate() + var err error + consensusHost, err = types.NewWasmConsensusHost(suite.chainA.Codec, consensusHost) + suite.Require().NoError(err) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetSelfConsensusHost( - types.NewWasmConsensusHost(cdc, consensusHost), + consensusHost, ) - err := suite.chainA.App.GetIBCKeeper().ClientKeeper.ValidateSelfClient(suite.chainA.GetContext(), clientState) + err = suite.chainA.App.GetIBCKeeper().ClientKeeper.ValidateSelfClient(suite.chainA.GetContext(), clientState) expPass := tc.expError == nil if expPass { From 7ac83fd1c8e689ee3f1c0060790f094d4a519ce1 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 3 Apr 2024 14:22:09 +0200 Subject: [PATCH 11/12] chore: vanity nits --- .../07-tendermint/consensus_host_test.go | 6 ++---- .../light-clients/08-wasm/types/consensus_host.go | 8 -------- .../08-wasm/types/consensus_host_test.go | 14 +++++++++----- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/modules/light-clients/07-tendermint/consensus_host_test.go b/modules/light-clients/07-tendermint/consensus_host_test.go index 860e63fffb5..7c79815ea22 100644 --- a/modules/light-clients/07-tendermint/consensus_host_test.go +++ b/modules/light-clients/07-tendermint/consensus_host_test.go @@ -22,10 +22,8 @@ func (suite *TendermintTestSuite) TestGetSelfConsensusState() { expError error }{ { - name: "zero height", - malleate: func() { - height = clienttypes.ZeroHeight() - }, + name: "zero height", + malleate: func() {}, expError: clienttypes.ErrInvalidHeight, }, { diff --git a/modules/light-clients/08-wasm/types/consensus_host.go b/modules/light-clients/08-wasm/types/consensus_host.go index c0a936efce0..fde71695656 100644 --- a/modules/light-clients/08-wasm/types/consensus_host.go +++ b/modules/light-clients/08-wasm/types/consensus_host.go @@ -58,14 +58,6 @@ func (w *WasmConsensusHost) GetSelfConsensusState(ctx sdk.Context, height export // ValidateSelfClient implements the 02-client types.ConsensusHost interface. func (w *WasmConsensusHost) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error { - if w.cdc == nil { - return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm consensus host codec is nil") - } - - if w.delegate == nil { - return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm delegate consensus host is nil") - } - wasmClientState, ok := clientState.(*ClientState) if !ok { return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client must be a wasm client, expected: %T, got: %T", ClientState{}, wasmClientState) diff --git a/modules/light-clients/08-wasm/types/consensus_host_test.go b/modules/light-clients/08-wasm/types/consensus_host_test.go index e6a43e95b97..f6249f382cf 100644 --- a/modules/light-clients/08-wasm/types/consensus_host_test.go +++ b/modules/light-clients/08-wasm/types/consensus_host_test.go @@ -12,9 +12,11 @@ import ( ) func (suite *TypesTestSuite) TestGetSelfConsensusState() { - var height clienttypes.Height - var consensusHost clienttypes.ConsensusHost - var consensusState exported.ConsensusState + var ( + consensusHost clienttypes.ConsensusHost + consensusState exported.ConsensusState + height clienttypes.Height + ) cases := []struct { name string @@ -78,8 +80,10 @@ func (suite *TypesTestSuite) TestGetSelfConsensusState() { } func (suite *TypesTestSuite) TestValidateSelfClient() { - var clientState exported.ClientState - var consensusHost clienttypes.ConsensusHost + var ( + clientState exported.ClientState + consensusHost clienttypes.ConsensusHost + ) testCases := []struct { name string From 26cd5d192accf2a6f79066c55cafbc69a6ccc658 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 3 Apr 2024 14:55:14 +0200 Subject: [PATCH 12/12] chore: add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1ac73885eb..e7037ac65c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features * (apps/27-interchain-accounts) [\#5785](https://github.com/cosmos/ibc-go/pull/5785) Introduce a new tx message that ICA host submodule can use to query the chain (only those marked with `module_query_safe`) and write the responses to the acknowledgement. +* (core) [\#6055](https://github.com/cosmos/ibc-go/pull/6055) Introduce a new interface `ConsensusHost` used to validate an IBC `ClientState` and `ConsensusState` against the host chain's underlying consensus parameters. ### Bug Fixes