fix(rollapp): fix expensive iteration when setting canonical light cl…

…ient (#1575)

.gitignore

@@ -9,4 +9,5 @@ build/
 proposal.json
 .go-version
 **/testdata/rapid/
-**/__debug_bin*
+**/__debug_bin*
+.aider*

ibctesting/light_client_test.go

@@ -1,16 +1,18 @@
 package ibctesting_test
 
 import (
+	"slices"
 	"testing"
 
 	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	clienttypes "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/cosmos/ibc-go/v7/testing/simapp"
-
+	lightclientkeeper "github.com/dymensionxyz/dymension/v3/x/lightclient/keeper"
 	"github.com/dymensionxyz/dymension/v3/x/lightclient/types"
 	rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types"
+	"github.com/dymensionxyz/sdk-utils/utils/utest"
 
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
 	"github.com/stretchr/testify/suite"
@@ -41,13 +43,115 @@ func (s *lightClientSuite) TestSetCanonicalClient_FailsTrustRequirements() {
 	s.coordinator.SetupClients(s.path)
 
 	// Update rollapp state - this will trigger the check for prospective canonical client
-	currentRollappBlockHeight := uint64(s.rollappChain().App.LastBlockHeight())
-	s.updateRollappState(currentRollappBlockHeight)
+	_, err := s.lightclientMsgServer().SetCanonicalClient(s.hubCtx(),
+		&types.MsgSetCanonicalClient{
+			Signer: s.hubChain().SenderAccount.GetAddress().String(), ClientId: s.path.EndpointA.ClientID,
+		})
+	s.Require().Error(err)
 
 	_, found := s.hubApp().LightClientKeeper.GetCanonicalClient(s.hubCtx(), s.rollappChain().ChainID)
 	s.False(found)
 }
 
+func (s *lightClientSuite) TestSetCanonicalClient_ParamsMismatch() {
+	s.createRollapp(false, nil)
+	s.registerSequencer()
+	// create a custom tm client which matches the trust requirements of a canonical client
+	endpointA := ibctesting.NewEndpoint(s.hubChain(), &canonicalClientConfig, ibctesting.NewConnectionConfig(), ibctesting.NewChannelConfig())
+	endpointB := ibctesting.NewEndpoint(s.rollappChain(), ibctesting.NewTendermintConfig(), ibctesting.NewConnectionConfig(), ibctesting.NewChannelConfig())
+	endpointA.Counterparty = endpointB
+	endpointB.Counterparty = endpointA
+	s.path = &ibctesting.Path{EndpointA: endpointA, EndpointB: endpointB}
+
+	currentHeader := s.rollappChain().CurrentHeader
+	startHeight := uint64(currentHeader.Height)
+	bd := rollapptypes.BlockDescriptor{Height: startHeight, StateRoot: currentHeader.AppHash, Timestamp: currentHeader.Time}
+
+	// Creating the tm client - this will take us to the next block
+	s.NoError(s.path.EndpointA.CreateClient())
+
+	currentHeader = s.rollappChain().CurrentHeader
+	bdNext := rollapptypes.BlockDescriptor{Height: uint64(currentHeader.Height), StateRoot: currentHeader.AppHash, Timestamp: currentHeader.Time}
+
+	// Update the rollapp state - this will trigger the check for prospective canonical client
+	msgUpdateState := rollapptypes.NewMsgUpdateState(
+		s.hubChain().SenderAccount.GetAddress().String(),
+		rollappChainID(),
+		"mock-da-path",
+		startHeight,
+		2,
+		&rollapptypes.BlockDescriptors{BD: []rollapptypes.BlockDescriptor{bd, bdNext}},
+	)
+	_, err := s.rollappMsgServer().UpdateState(s.hubCtx(), msgUpdateState)
+	s.Require().NoError(err)
+
+	// now break the params
+	c, _ := s.hubChain().App.GetIBCKeeper().ClientKeeper.GetClientState(s.hubCtx(), s.path.EndpointA.ClientID)
+	tmClient, _ := c.(*ibctm.ClientState)
+	tmClient.MaxClockDrift = 0 // wrong
+	s.hubChain().App.GetIBCKeeper().ClientKeeper.SetClientState(s.hubCtx(), s.path.EndpointA.ClientID, tmClient)
+
+	// Update rollapp state - this will trigger the check for prospective canonical client
+	setCanonMsg := &types.MsgSetCanonicalClient{
+		Signer: s.hubChain().SenderAccount.GetAddress().String(), ClientId: s.path.EndpointA.ClientID,
+	}
+	_, err = s.lightclientMsgServer().SetCanonicalClient(s.hubCtx(), setCanonMsg)
+	utest.IsErr(s.Require(), err, lightclientkeeper.ErrParamsMismatch)
+
+	_, found := s.hubApp().LightClientKeeper.GetCanonicalClient(s.hubCtx(), s.rollappChain().ChainID)
+	s.False(found)
+}
+
+func (s *lightClientSuite) TestSetCanonicalClient_ConsStateMismatch() {
+	s.createRollapp(false, nil)
+	s.registerSequencer()
+	// create a custom tm client which matches the trust requirements of a canonical client
+	endpointA := ibctesting.NewEndpoint(s.hubChain(), &canonicalClientConfig, ibctesting.NewConnectionConfig(), ibctesting.NewChannelConfig())
+	endpointB := ibctesting.NewEndpoint(s.rollappChain(), ibctesting.NewTendermintConfig(), ibctesting.NewConnectionConfig(), ibctesting.NewChannelConfig())
+	endpointA.Counterparty = endpointB
+	endpointB.Counterparty = endpointA
+	s.path = &ibctesting.Path{EndpointA: endpointA, EndpointB: endpointB}
+
+	currentHeader := s.rollappChain().CurrentHeader
+	startHeight := uint64(currentHeader.Height)
+	bd := rollapptypes.BlockDescriptor{Height: startHeight, StateRoot: currentHeader.AppHash, Timestamp: currentHeader.Time}
+
+	// Creating the tm client - this will take us to the next block
+	s.NoError(s.path.EndpointA.CreateClient())
+
+	currentHeader = s.rollappChain().CurrentHeader
+	bdNext := rollapptypes.BlockDescriptor{Height: uint64(currentHeader.Height), StateRoot: currentHeader.AppHash, Timestamp: currentHeader.Time}
+
+	// It's too early, it should fail
+	setCanonMsg := &types.MsgSetCanonicalClient{
+		Signer: s.hubChain().SenderAccount.GetAddress().String(), ClientId: s.path.EndpointA.ClientID,
+	}
+	_, err := s.lightclientMsgServer().SetCanonicalClient(s.hubCtx(), setCanonMsg)
+	s.Require().Error(err)
+
+	// Update the rollapp state - this will trigger the check for prospective canonical client
+	msgUpdateState := rollapptypes.NewMsgUpdateState(
+		s.hubChain().SenderAccount.GetAddress().String(),
+		rollappChainID(),
+		"mock-da-path",
+		startHeight,
+		2,
+		&rollapptypes.BlockDescriptors{BD: []rollapptypes.BlockDescriptor{bd, bdNext}},
+	)
+	_, err = s.rollappMsgServer().UpdateState(s.hubCtx(), msgUpdateState)
+	s.Require().NoError(err)
+
+	c, _ := s.hubChain().App.GetIBCKeeper().ClientKeeper.GetClientState(s.hubCtx(), s.path.EndpointA.ClientID)
+	tmClient, _ := c.(*ibctm.ClientState)
+	cs, _ := s.hubChain().App.GetIBCKeeper().ClientKeeper.GetClientConsensusState(s.hubCtx(), s.path.EndpointA.ClientID, tmClient.GetLatestHeight())
+	tmConsState, _ := cs.(*ibctm.ConsensusState)
+	slices.Reverse(tmConsState.NextValidatorsHash) // make it wrong
+	s.hubChain().App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(s.hubCtx(), s.path.EndpointA.ClientID, tmClient.GetLatestHeight(), tmConsState)
+
+	_, err = s.lightclientMsgServer().SetCanonicalClient(s.hubCtx(), setCanonMsg)
+	utest.IsErr(s.Require(), err, lightclientkeeper.ErrMismatch)
+}
+
 func (s *lightClientSuite) TestSetCanonicalClient_FailsIncompatibleState() {
 	s.createRollapp(false, nil)
 	s.registerSequencer()
@@ -66,6 +170,12 @@ func (s *lightClientSuite) TestSetCanonicalClient_FailsIncompatibleState() {
 	currentRollappBlockHeight := uint64(s.rollappChain().App.LastBlockHeight())
 	s.updateRollappState(currentRollappBlockHeight)
 
+	_, err := s.lightclientMsgServer().SetCanonicalClient(s.hubCtx(),
+		&types.MsgSetCanonicalClient{
+			Signer: s.hubChain().SenderAccount.GetAddress().String(), ClientId: s.path.EndpointA.ClientID,
+		})
+	s.Require().Error(err)
+
 	_, found := s.hubApp().LightClientKeeper.GetCanonicalClient(s.hubCtx(), s.rollappChain().ChainID)
 	s.False(found)
 }
@@ -90,6 +200,13 @@ func (s *lightClientSuite) TestSetCanonicalClient_Succeeds() {
 	currentHeader = s.rollappChain().CurrentHeader
 	bdNext := rollapptypes.BlockDescriptor{Height: uint64(currentHeader.Height), StateRoot: currentHeader.AppHash, Timestamp: currentHeader.Time}
 
+	// It's too early, it should fail
+	setCanonMsg := &types.MsgSetCanonicalClient{
+		Signer: s.hubChain().SenderAccount.GetAddress().String(), ClientId: s.path.EndpointA.ClientID,
+	}
+	_, err := s.lightclientMsgServer().SetCanonicalClient(s.hubCtx(), setCanonMsg)
+	s.Require().Error(err)
+
 	// Update the rollapp state - this will trigger the check for prospective canonical client
 	msgUpdateState := rollapptypes.NewMsgUpdateState(
 		s.hubChain().SenderAccount.GetAddress().String(),
@@ -99,7 +216,10 @@ func (s *lightClientSuite) TestSetCanonicalClient_Succeeds() {
 		2,
 		&rollapptypes.BlockDescriptors{BD: []rollapptypes.BlockDescriptor{bd, bdNext}},
 	)
-	_, err := s.rollappMsgServer().UpdateState(s.hubCtx(), msgUpdateState)
+	_, err = s.rollappMsgServer().UpdateState(s.hubCtx(), msgUpdateState)
+	s.Require().NoError(err)
+
+	_, err = s.lightclientMsgServer().SetCanonicalClient(s.hubCtx(), setCanonMsg)
 	s.Require().NoError(err)
 
 	canonClientID, found := s.hubApp().LightClientKeeper.GetCanonicalClient(s.hubCtx(), s.rollappChain().ChainID)

ibctesting/utils_test.go

@@ -23,6 +23,8 @@ import (
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
 	"github.com/cosmos/ibc-go/v7/testing/mock"
 	"github.com/cosmos/ibc-go/v7/testing/simapp"
+	lightclientkeeper "github.com/dymensionxyz/dymension/v3/x/lightclient/keeper"
+	lightclienttypes "github.com/dymensionxyz/dymension/v3/x/lightclient/types"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
@@ -109,6 +111,10 @@ func (s *utilSuite) rollappMsgServer() rollapptypes.MsgServer {
 	return rollappkeeper.NewMsgServerImpl(s.hubApp().RollappKeeper)
 }
 
+func (s *utilSuite) lightclientMsgServer() lightclienttypes.MsgServer {
+	return lightclientkeeper.NewMsgServerImpl(&s.hubApp().LightClientKeeper)
+}
+
 // SetupTest creates a coordinator with 2 test chains.
 func (s *utilSuite) SetupTest() {
 	// this is used as default when creating blocks.

proto/dymensionxyz/dymension/lightclient/events.proto

@@ -10,3 +10,9 @@ message EventSetCanonicalChannel {
     string rollapp_id = 1;
     string channel_id = 2;
 }
+
+// When the canonical client of the rollapp is set
+message EventSetCanonicalClient {
+    string rollapp_id = 1;
+    string client_id = 2;
+}

proto/dymensionxyz/dymension/lightclient/tx.proto

@@ -0,0 +1,27 @@
+syntax = "proto3";
+package dymensionxyz.dymension.lightclient;
+
+import "cosmos/msg/v1/msg.proto";
+import "gogoproto/gogo.proto";
+import "google/api/annotations.proto";
+import "google/protobuf/any.proto";
+
+option go_package = "github.com/dymensionxyz/dymension/v3/x/lightclient/types";
+
+service Msg {
+  rpc SetCanonicalClient(MsgSetCanonicalClient) returns (MsgSetCanonicalClientResponse);
+}
+
+// verify a client state and its consensus states against the rollapp
+// if it matches, set the client as the canonical client for the rollapp
+// NOTE: this definition is also copied to the relayer code
+message MsgSetCanonicalClient {
+  option (cosmos.msg.v1.signer) = "signer";
+  string signer = 1;
+  // id of ibc client state
+  string client_id = 2;
+}
+
+message MsgSetCanonicalClientResponse {
+}
+

testutil/keeper/lightclient.go

@@ -182,6 +182,10 @@ func NewMockSequencerKeeper(sequencers map[string]*sequencertypes.Sequencer) *Mo
 
 type MockRollappKeeper struct{}
 
+func (m *MockRollappKeeper) GetLatestHeight(ctx sdk.Context, rollappId string) (uint64, bool) {
+	panic("implement me")
+}
+
 // GetLatestStateInfo implements types.RollappKeeperExpected.
 func (m *MockRollappKeeper) GetLatestStateInfo(ctx sdk.Context, rollappId string) (rollapptypes.StateInfo, bool) {
 	panic("unimplemented")

x/lightclient/ante/ibc_msgs_test.go

@@ -17,6 +17,11 @@ type MockRollappKeeper struct {
 	stateInfos map[string]map[uint64]rollapptypes.StateInfo
 }
 
+func (m *MockRollappKeeper) GetLatestHeight(ctx sdk.Context, rollappId string) (uint64, bool) {
+	// TODO implement me
+	panic("implement me")
+}
+
 // GetLatestStateInfo implements types.RollappKeeperExpected.
 func (m *MockRollappKeeper) GetLatestStateInfo(ctx sdk.Context, rollappId string) (rollapptypes.StateInfo, bool) {
 	return rollapptypes.StateInfo{}, false

x/lightclient/client/cli/query.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/cosmos/cosmos-sdk/client"
+	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/version"
 	ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
 	"github.com/spf13/cobra"
@@ -65,6 +66,7 @@ The other fields can take any value`,
 		},
 	}
 
+	flags.AddQueryFlagsToCmd(cmd)
 	return cmd
 }
 
@@ -95,5 +97,7 @@ func CmdGetLightClient() *cobra.Command {
 		},
 	}
 
+	flags.AddQueryFlagsToCmd(cmd)
+
 	return cmd
 }