Revert "refactor(lightclient): removed the need for next stateInfo fo…

…r valiadition (#1467)"

This reverts commit 200dd62.

go.mod

@@ -246,5 +246,6 @@ replace (
 
 	//  broken goleveldb
 	github.com/syndtr/goleveldb => github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7
+	github.com/tendermint/tendermint => github.com/cometbft/cometbft v0.34.29
 	golang.org/x/exp => golang.org/x/exp v0.0.0-20230711153332-06a737ee72cb
 )

ibctesting/light_client_test.go

@@ -129,7 +129,7 @@ func (s *lightClientSuite) TestSetCanonicalClient_ConsStateMismatch() {
 	_, err := s.lightclientMsgServer().SetCanonicalClient(s.hubCtx(), setCanonMsg)
 	s.Require().Error(err)
 
-	// Update the rollapp state so we could attempt to set the canonical client
+	// Update the rollapp state - this will trigger the check for prospective canonical client
 	msgUpdateState := rollapptypes.NewMsgUpdateState(
 		s.hubChain().SenderAccount.GetAddress().String(),
 		rollappChainID(),

testutil/keeper/lightclient.go

@@ -98,11 +98,6 @@ type MockIBCCLientKeeper struct {
 	clientStates         map[string]exported.ClientState
 }
 
-// IterateConsensusStates implements types.IBCClientKeeperExpected.
-func (m *MockIBCCLientKeeper) IterateConsensusStates(ctx sdk.Context, cb func(clientID string, cs ibcclienttypes.ConsensusStateWithHeight) bool) {
-	panic("unimplemented")
-}
-
 // ClientStore implements types.IBCClientKeeperExpected.
 func (m *MockIBCCLientKeeper) ClientStore(ctx sdk.Context, clientID string) storetypes.KVStore {
 	panic("unimplemented")
@@ -188,11 +183,6 @@ func NewMockSequencerKeeper(sequencers map[string]*sequencertypes.Sequencer) *Mo
 
 type MockRollappKeeper struct{}
 
-// GetLatestStateInfoIndex implements types.RollappKeeperExpected.
-func (m *MockRollappKeeper) GetLatestStateInfoIndex(ctx sdk.Context, rollappId string) (rollapptypes.StateInfoIndex, bool) {
-	panic("unimplemented")
-}
-
 func (m *MockRollappKeeper) IsFirstHeightOfLatestFork(ctx sdk.Context, rollappId string, revision, height uint64) bool {
 	return false
 }

x/lightclient/ante/ibc_msg_update_client.go

@@ -10,15 +10,11 @@ import (
 	ibctm "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint"
 	"github.com/dymensionxyz/gerr-cosmos/gerrc"
 
+	"github.com/dymensionxyz/dymension/v3/x/lightclient/types"
+	rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types"
 	sequencertypes "github.com/dymensionxyz/dymension/v3/x/sequencer/types"
 )
 
-var (
-	errIsMisbehaviour   = errorsmod.Wrap(gerrc.ErrFailedPrecondition, "misbehavior evidence is disabled for canonical clients")
-	errNoHeader         = errors.New("message does not contain header")
-	errProposerMismatch = errorsmod.Wrap(gerrc.ErrInvalidArgument, "validator set proposer not equal header proposer field")
-)
-
 func (i IBCMessagesDecorator) HandleMsgUpdateClient(ctx sdk.Context, msg *ibcclienttypes.MsgUpdateClient) error {
 	if !i.k.Enabled() {
 		return nil
@@ -36,7 +32,6 @@ func (i IBCMessagesDecorator) HandleMsgUpdateClient(ctx sdk.Context, msg *ibccli
 	if err != nil {
 		return errorsmod.Wrap(err, "get header")
 	}
-
 	seq, err := i.getSequencer(ctx, header)
 	err = errorsmod.Wrap(err, "get sequencer")
 	if errorsmod.IsOf(err, errProposerMismatch) {
@@ -75,27 +70,26 @@ func (i IBCMessagesDecorator) HandleMsgUpdateClient(ctx sdk.Context, msg *ibccli
 	}
 
 	h := header.GetHeight().GetRevisionHeight()
-	sInfo, err := i.raK.FindStateInfoByHeight(ctx, rollapp.RollappId, h)
-	if errorsmod.IsOf(err, gerrc.ErrNotFound) {
-		// the header is optimistic: the state update has not yet been received, so we save optimistically
-		err := i.k.SaveSigner(ctx, seq.Address, msg.ClientId, h)
-		if err != nil {
-			return errorsmod.Wrap(err, "save signer")
-		}
-		return nil
-	}
+	stateInfos, err := i.getStateInfos(ctx, rollapp.RollappId, h)
 	if err != nil {
-		return errorsmod.Wrap(err, "find state info by height")
+		return errorsmod.Wrap(err, "get state infos")
 	}
 
-	err = i.k.ValidateHeaderAgainstStateInfo(ctx, sInfo, header.ConsensusState(), h)
-	if err != nil {
-		return errorsmod.Wrap(err, "validate pessimistic")
+	if stateInfos.containingHPlus1 != nil {
+		// the header is pessimistic: the state update has already been received, so we check the header doesn't mismatch
+		return errorsmod.Wrap(i.validateUpdatePessimistically(ctx, stateInfos, header.ConsensusState(), h), "validate pessimistic")
 	}
 
-	return nil
+	// the header is optimistic: the state update has not yet been received, so we save optimistically
+	return errorsmod.Wrap(i.k.SaveSigner(ctx, seq.Address, msg.ClientId, h), "save updater")
 }
 
+var (
+	errIsMisbehaviour   = errorsmod.Wrap(gerrc.ErrFailedPrecondition, "misbehavior evidence is disabled for canonical clients")
+	errNoHeader         = errors.New("message does not contain header")
+	errProposerMismatch = errorsmod.Wrap(gerrc.ErrInvalidArgument, "validator set proposer not equal header proposer field")
+)
+
 func (i IBCMessagesDecorator) getSequencer(ctx sdk.Context, header *ibctm.Header) (sequencertypes.Sequencer, error) {
 	proposerBySignature := header.ValidatorSet.Proposer.GetAddress()
 	proposerByData := header.Header.ProposerAddress
@@ -121,3 +115,45 @@ func getHeader(msg *ibcclienttypes.MsgUpdateClient) (*ibctm.Header, error) {
 	}
 	return header, nil
 }
+
+// if containingHPlus1 is not nil then containingH also guaranteed to not be nil
+type stateInfos struct {
+	containingH      *rollapptypes.StateInfo
+	containingHPlus1 *rollapptypes.StateInfo
+}
+
+// getStateInfos gets state infos for h and h+1
+func (i IBCMessagesDecorator) getStateInfos(ctx sdk.Context, rollapp string, h uint64) (stateInfos, error) {
+	// Check if there are existing block descriptors for the given height of client state
+	s0, err := i.raK.FindStateInfoByHeight(ctx, rollapp, h)
+	if errorsmod.IsOf(err, gerrc.ErrNotFound) {
+		return stateInfos{}, nil
+	}
+	if err != nil {
+		return stateInfos{}, err
+	}
+	s1 := s0
+	if !s1.ContainsHeight(h + 1) {
+		s1, err = i.raK.FindStateInfoByHeight(ctx, rollapp, h+1)
+		if errorsmod.IsOf(err, gerrc.ErrNotFound) {
+			return stateInfos{s0, nil}, nil
+		}
+		if err != nil {
+			return stateInfos{}, err
+		}
+	}
+	return stateInfos{s0, s1}, nil
+}
+
+func (i IBCMessagesDecorator) validateUpdatePessimistically(ctx sdk.Context, infos stateInfos, consState *ibctm.ConsensusState, h uint64) error {
+	bd, _ := infos.containingH.GetBlockDescriptor(h)
+	seq, err := i.k.SeqK.RealSequencer(ctx, infos.containingHPlus1.Sequencer)
+	if err != nil {
+		return errorsmod.Wrap(errors.Join(err, gerrc.ErrInternal), "get sequencer of state info")
+	}
+	rollappState := types.RollappState{
+		BlockDescriptor:    bd,
+		NextBlockSequencer: seq,
+	}
+	return errorsmod.Wrap(types.CheckCompatibility(*consState, rollappState), "check compatibility")
+}

x/lightclient/ante/ibc_msgs_test.go

@@ -18,11 +18,6 @@ type MockRollappKeeper struct {
 	stateInfos map[string]map[uint64]rollapptypes.StateInfo
 }
 
-// GetLatestStateInfoIndex implements types.RollappKeeperExpected.
-func (m *MockRollappKeeper) GetLatestStateInfoIndex(ctx sdk.Context, rollappId string) (rollapptypes.StateInfoIndex, bool) {
-	panic("unimplemented")
-}
-
 func (m *MockRollappKeeper) IsFirstHeightOfLatestFork(ctx sdk.Context, rollappId string, revision, height uint64) bool {
 	panic("implement me")
 }
@@ -81,11 +76,6 @@ type MockIBCClientKeeper struct {
 	clientStates map[string]exported.ClientState
 }
 
-// IterateConsensusStates implements types.IBCClientKeeperExpected.
-func (m *MockIBCClientKeeper) IterateConsensusStates(ctx sdk.Context, cb func(clientID string, cs ibcclienttypes.ConsensusStateWithHeight) bool) {
-	panic("unimplemented")
-}
-
 // ClientStore implements types.IBCClientKeeperExpected.
 func (m *MockIBCClientKeeper) ClientStore(ctx sdk.Context, clientID string) types.KVStore {
 	panic("unimplemented")

x/lightclient/keeper/canonical_client.go

@@ -5,18 +5,13 @@ import (
 
 	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/query"
+	ibcclienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
 	ibctm "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint"
 	"github.com/dymensionxyz/gerr-cosmos/gerrc"
 	"github.com/dymensionxyz/sdk-utils/utils/uevent"
 
 	"github.com/dymensionxyz/dymension/v3/x/lightclient/types"
-	rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types"
-)
-
-var (
-	ErrNoMatch        = gerrc.ErrFailedPrecondition.Wrap("not at least one cons state matches the rollapp state")
-	ErrMismatch       = gerrc.ErrInvalidArgument.Wrap("consensus state mismatch")
-	ErrParamsMismatch = gerrc.ErrInvalidArgument.Wrap("params")
 )
 
 // intended to be called by relayer, but can be called by anyone
@@ -44,9 +39,14 @@ func (k *Keeper) TrySetCanonicalClient(ctx sdk.Context, clientID string) error {
 		return gerrc.ErrAlreadyExists.Wrap("canonical client for rollapp")
 	}
 
-	err := k.validClient(ctx, clientID, clientState, rollappID)
+	latestHeight, ok := k.rollappKeeper.GetLatestHeight(ctx, rollappID)
+	if !ok {
+		return gerrc.ErrNotFound.Wrap("latest rollapp height")
+	}
+
+	err := k.validClient(ctx, clientID, clientState, rollappID, latestHeight)
 	if err != nil {
-		return errorsmod.Wrap(err, "unsafe to mark client canonical")
+		return errorsmod.Wrap(err, "unsafe to mark client canonical: check that sequencer has posted a recent state update")
 	}
 
 	k.SetCanonicalClient(ctx, rollappID, clientID)
@@ -93,43 +93,69 @@ func (k Keeper) expectedClient() ibctm.ClientState {
 	return types.DefaultExpectedCanonicalClientParams()
 }
 
+var (
+	ErrNoMatch        = gerrc.ErrFailedPrecondition.Wrap("not at least one cons state matches the rollapp state")
+	ErrMismatch       = gerrc.ErrInvalidArgument.Wrap("consensus state mismatch")
+	ErrParamsMismatch = gerrc.ErrInvalidArgument.Wrap("params")
+)
+
 // The canonical client criteria are:
 // 1. The client must be a tendermint client.
 // 2. The client state must match the expected client params as configured by the module
 // 3. All the existing consensus states much match the corresponding height rollapp block descriptors
-func (k Keeper) validClient(ctx sdk.Context, clientID string, cs *ibctm.ClientState, rollappId string) error {
+func (k Keeper) validClient(ctx sdk.Context, clientID string, cs *ibctm.ClientState, rollappId string, maxHeight uint64) error {
+	log := k.Logger(ctx).With("component", "valid client func", "rollapp", rollappId, "client", clientID)
+
+	log.Debug("top of func", "max height", maxHeight, "gas", ctx.GasMeter().GasConsumed())
+
 	expClient := k.expectedClient()
+
 	if err := types.IsCanonicalClientParamsValid(cs, &expClient); err != nil {
 		return errors.Join(err, ErrParamsMismatch)
 	}
 
-	sinfo, ok := k.rollappKeeper.GetLatestStateInfoIndex(ctx, rollappId)
-	if !ok {
-		return gerrc.ErrNotFound.Wrap("latest state info index")
+	// FIXME: No need to get all consensus states. should iterate over the consensus states
+	res, err := k.ibcClientKeeper.ConsensusStateHeights(ctx, &ibcclienttypes.QueryConsensusStateHeightsRequest{
+		ClientId:   clientID,
+		Pagination: &query.PageRequest{Limit: maxHeight},
+	})
+	log.Debug("after fetch heights", "max height", maxHeight, "gas", ctx.GasMeter().GasConsumed())
+	if err != nil {
+		return errorsmod.Wrap(err, "cons state heights")
 	}
-
-	baseHeight := k.GetFirstConsensusStateHeight(ctx, clientID)
 	atLeastOneMatch := false
-	for i := sinfo.Index; i > 0; i-- {
-		sInfo, ok := k.rollappKeeper.GetStateInfo(ctx, rollappId, i)
-		if !ok {
-			return errorsmod.Wrap(gerrc.ErrInternal, "get state info")
+	for _, consensusHeight := range res.ConsensusStateHeights {
+		log.Debug("after fetch heights", "cons state height", consensusHeight.RevisionHeight, "gas", ctx.GasMeter().GasConsumed())
+		h := consensusHeight.GetRevisionHeight()
+		if maxHeight <= h {
+			break
 		}
-		matched, err := k.ValidateStateInfoAgainstConsensusStates(ctx, clientID, &sInfo)
+		consensusState, _ := k.ibcClientKeeper.GetClientConsensusState(ctx, clientID, consensusHeight)
+		tmConsensusState, _ := consensusState.(*ibctm.ConsensusState)
+		stateInfoH, err := k.rollappKeeper.FindStateInfoByHeight(ctx, rollappId, h)
 		if err != nil {
-			return errors.Join(ErrMismatch, err)
+			return errorsmod.Wrapf(err, "find state info by height h: %d", h)
 		}
-
-		if matched {
-			atLeastOneMatch = true
+		stateInfoHplus1, err := k.rollappKeeper.FindStateInfoByHeight(ctx, rollappId, h+1)
+		if err != nil {
+			return errorsmod.Wrapf(err, "find state info by height h+1: %d", h+1)
 		}
+		bd, _ := stateInfoH.GetBlockDescriptor(h)
 
-		// break point with the lowest height of the consensus states
-		if sInfo.StartHeight > baseHeight {
-			break
+		nextSeq, err := k.SeqK.RealSequencer(ctx, stateInfoHplus1.Sequencer)
+		if err != nil {
+			return errorsmod.Wrap(err, "get sequencer")
+		}
+		rollappState := types.RollappState{
+			BlockDescriptor:    bd,
+			NextBlockSequencer: nextSeq,
 		}
+		err = types.CheckCompatibility(*tmConsensusState, rollappState)
+		if err != nil {
+			return errorsmod.Wrapf(errors.Join(ErrMismatch, err), "check compatibility: height: %d", h)
+		}
+		atLeastOneMatch = true
 	}
-
 	// Need to be sure that at least one consensus state agrees with a state update
 	// (There are also no disagreeing consensus states. There may be some consensus states
 	// for future state updates, which will incur a fraud if they disagree.)
@@ -138,21 +164,3 @@ func (k Keeper) validClient(ctx sdk.Context, clientID string, cs *ibctm.ClientSt
 	}
 	return nil
 }
-
-func (k Keeper) ValidateHeaderAgainstStateInfo(ctx sdk.Context, sInfo *rollapptypes.StateInfo, consState *ibctm.ConsensusState, h uint64) error {
-	bd, ok := sInfo.GetBlockDescriptor(h)
-	if !ok {
-		return errorsmod.Wrapf(gerrc.ErrInternal, "no block descriptor found for height %d", h)
-	}
-
-	nextSeq, err := k.SeqK.RealSequencer(ctx, sInfo.NextSequencerForHeight(h))
-	if err != nil {
-		return errorsmod.Wrap(errors.Join(err, gerrc.ErrInternal), "get sequencer of state info")
-	}
-
-	rollappState := types.RollappState{
-		BlockDescriptor:    bd,
-		NextBlockSequencer: nextSeq,
-	}
-	return errorsmod.Wrap(types.CheckCompatibility(*consState, rollappState), "check compatibility")
-}

x/lightclient/keeper/client_store.go

@@ -9,21 +9,6 @@ import (
 	ibctm "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint"
 )
 
-// IterateConsensusStateDescending iterates through all consensus states in descending order
-// until cb returns true.
-func IterateConsensusStateDescending(clientStore sdk.KVStore, cb func(height exported.Height) (stop bool)) {
-	iterator := sdk.KVStoreReversePrefixIterator(clientStore, []byte(ibctm.KeyIterateConsensusStatePrefix))
-	defer iterator.Close() // nolint: errcheck
-
-	for ; iterator.Valid(); iterator.Next() {
-		iterKey := iterator.Key()
-		height := ibctm.GetHeightFromIterationKey(iterKey)
-		if cb(height) {
-			break
-		}
-	}
-}
-
 // functions here copied from ibc-go/modules/core/02-client/keeper/
 // as we need direct access to the client store
 
@@ -106,13 +91,3 @@ func deleteIterationKey(clientStore sdk.KVStore, height exported.Height) {
 	key := ibctm.IterationKey(height)
 	clientStore.Delete(key)
 }
-
-// GetFirstHeight returns the lowest height available for a client.
-func (k Keeper) GetFirstConsensusStateHeight(ctx sdk.Context, clientID string) uint64 {
-	height := clienttypes.Height{}
-	k.ibcClientKeeper.IterateConsensusStates(ctx, func(clientID string, cs clienttypes.ConsensusStateWithHeight) bool {
-		height = cs.Height
-		return true
-	})
-	return height.GetRevisionHeight()
-}