Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adding ConsensusHost interface for custom self client/consensus state validation #6055

Merged
merged 14 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/core/02-client/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
117 changes: 20 additions & 97 deletions modules/core/02-client/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package keeper
import (
"errors"
"fmt"
"reflect"
"strings"

errorsmod "cosmossdk.io/errors"
Expand All @@ -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"
Expand All @@ -32,8 +27,8 @@ type Keeper struct {
storeKey storetypes.StoreKey
cdc codec.BinaryCodec
router *types.Router
consensusHost types.ConsensusHost
legacySubspace types.ParamSubspace
stakingKeeper types.StakingKeeper
upgradeKeeper types.UpgradeKeeper
}

Expand All @@ -47,8 +42,8 @@ func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace ty
storeKey: key,
cdc: cdc,
router: router,
consensusHost: ibctm.NewConsensusHost(sk),
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
legacySubspace: legacySubspace,
stakingKeeper: sk,
upgradeKeeper: uk,
}
}
Expand Down Expand Up @@ -88,6 +83,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
}
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

// GenerateClientIdentifier returns the next client identifier.
func (k Keeper) GenerateClientIdentifier(ctx sdk.Context, clientType string) string {
nextClientSeq := k.GetNextClientSequence(ctx)
Expand All @@ -99,7 +103,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 {
Expand All @@ -111,13 +115,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 {
Expand Down Expand Up @@ -308,96 +312,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.
Expand Down
118 changes: 2 additions & 116 deletions modules/core/02-client/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions modules/core/02-client/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}

Expand Down
4 changes: 2 additions & 2 deletions modules/core/02-client/migrations/v7/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion modules/core/02-client/proposal_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading
Loading