refactor(lightclient): removed the need for next stateInfo for valiadition

go.mod

@@ -246,6 +246,5 @@ 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 - this will trigger the check for prospective canonical client
+	// Update the rollapp state so we could attempt to set the canonical client
 	msgUpdateState := rollapptypes.NewMsgUpdateState(
 		s.hubChain().SenderAccount.GetAddress().String(),
 		rollappChainID(),

testutil/keeper/lightclient.go

@@ -97,6 +97,11 @@ 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")
@@ -182,6 +187,11 @@ 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,11 +10,15 @@ 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
@@ -32,6 +36,7 @@ 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) {
@@ -70,26 +75,27 @@ func (i IBCMessagesDecorator) HandleMsgUpdateClient(ctx sdk.Context, msg *ibccli
 	}
 
 	h := header.GetHeight().GetRevisionHeight()
-	stateInfos, err := i.getStateInfos(ctx, rollapp.RollappId, h)
+	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
+	}
 	if err != nil {
-		return errorsmod.Wrap(err, "get state infos")
+		return errorsmod.Wrap(err, "find state info by height")
 	}
 
-	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")
+	err = i.k.ValidateHeaderAgainstStateInfo(ctx, sInfo, header.ConsensusState(), h)
+	if err != nil {
+		return errorsmod.Wrap(err, "validate pessimistic")
 	}
 
-	// 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")
+	return nil
 }
 
-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
@@ -115,45 +121,3 @@ 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

@@ -17,6 +17,11 @@ 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")
 }
@@ -75,6 +80,11 @@ 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,13 +5,18 @@ 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
@@ -39,14 +44,9 @@ func (k *Keeper) TrySetCanonicalClient(ctx sdk.Context, clientID string) error {
 		return gerrc.ErrAlreadyExists.Wrap("canonical client for rollapp")
 	}
 
-	latestHeight, ok := k.rollappKeeper.GetLatestHeight(ctx, rollappID)
-	if !ok {
-		return gerrc.ErrNotFound.Wrap("latest rollapp height")
-	}
-
-	err := k.validClient(ctx, clientID, clientState, rollappID, latestHeight)
+	err := k.validClient(ctx, clientID, clientState, rollappID)
 	if err != nil {
-		return errorsmod.Wrap(err, "unsafe to mark client canonical: check that sequencer has posted a recent state update")
+		return errorsmod.Wrap(err, "unsafe to mark client canonical")
 	}
 
 	k.SetCanonicalClient(ctx, rollappID, clientID)
@@ -93,69 +93,43 @@ 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, 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())
-
+func (k Keeper) validClient(ctx sdk.Context, clientID string, cs *ibctm.ClientState, rollappId string) error {
 	expClient := k.expectedClient()
-
 	if err := types.IsCanonicalClientParamsValid(cs, &expClient); err != nil {
 		return errors.Join(err, ErrParamsMismatch)
 	}
 
-	// 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")
+	sinfo, ok := k.rollappKeeper.GetLatestStateInfoIndex(ctx, rollappId)
+	if !ok {
+		return gerrc.ErrNotFound.Wrap("latest state info index")
 	}
+
+	baseHeight := k.GetFirstConsensusStateHeight(ctx, clientID)
 	atLeastOneMatch := false
-	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
+	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")
 		}
-		consensusState, _ := k.ibcClientKeeper.GetClientConsensusState(ctx, clientID, consensusHeight)
-		tmConsensusState, _ := consensusState.(*ibctm.ConsensusState)
-		stateInfoH, err := k.rollappKeeper.FindStateInfoByHeight(ctx, rollappId, h)
+		matched, err := k.ValidateStateInfoAgainstConsensusStates(ctx, clientID, &sInfo)
 		if err != nil {
-			return errorsmod.Wrapf(err, "find state info by height h: %d", h)
+			return errors.Join(ErrMismatch, err)
 		}
-		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)
 
-		nextSeq, err := k.SeqK.RealSequencer(ctx, stateInfoHplus1.Sequencer)
-		if err != nil {
-			return errorsmod.Wrap(err, "get sequencer")
+		if matched {
+			atLeastOneMatch = true
 		}
-		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)
+
+		// break point with the lowest height of the consensus states
+		if sInfo.StartHeight > baseHeight {
+			break
 		}
-		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.)
@@ -164,3 +138,21 @@ 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,6 +9,21 @@ 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
 
@@ -91,3 +106,13 @@ 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()
+}