Migrate IBC tendermint header to proto (#7120)

* gen header proto file

* fix tm type to proto conversion issues

* fix tendermint type tests

* fix remaining tests

* fix lint

* Update x/ibc/07-tendermint/types/header.go

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Update x/ibc/07-tendermint/types/header.go

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* apply @fedekunze review suggestions

* fix build

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

Makefile

@@ -371,6 +371,7 @@ proto-update-deps:
 	@curl -sSL $(TM_URL)/types/types.proto > $(TM_TYPES)/types.proto
 	@curl -sSL $(TM_URL)/types/evidence.proto > $(TM_TYPES)/evidence.proto
 	@curl -sSL $(TM_URL)/types/params.proto > $(TM_TYPES)/params.proto
+	@curl -sSL $(TM_URL)/types/validator.proto > $(TM_TYPES)/validator.proto
 
 	@mkdir -p $(TM_CRYPTO_TYPES)
 	@curl -sSL $(TM_URL)/crypto/proof.proto > $(TM_CRYPTO_TYPES)/proof.proto

proto/ibc/tendermint/tendermint.proto

@@ -3,6 +3,8 @@ package ibc.tendermint;
 
 option go_package = "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types";
 
+import "tendermint/types/validator.proto";
+import "tendermint/types/types.proto";
 import "confio/proofs.proto";
 import "google/protobuf/duration.proto";
 import "google/protobuf/timestamp.proto";
@@ -14,29 +16,29 @@ import "gogoproto/gogo.proto";
 message ClientState {
   option (gogoproto.goproto_getters) = false;
 
-  string chain_id = 1;
+  string   chain_id    = 1;
   Fraction trust_level = 2 [
     (gogoproto.nullable) = false,
     (gogoproto.moretags) = "yaml:\"trust_level\""
   ];
   // duration of the period since the LastestTimestamp during which the
   // submitted headers are valid for upgrade
   google.protobuf.Duration trusting_period = 3 [
-    (gogoproto.nullable) = false,
+    (gogoproto.nullable)    = false,
     (gogoproto.stdduration) = true,
-    (gogoproto.moretags) = "yaml:\"trusting_period\""
+    (gogoproto.moretags)    = "yaml:\"trusting_period\""
   ];
   // duration of the staking unbonding period
   google.protobuf.Duration unbonding_period = 4 [
-    (gogoproto.nullable) = false,
+    (gogoproto.nullable)    = false,
     (gogoproto.stdduration) = true,
-    (gogoproto.moretags) = "yaml:\"unbonding_period\""
+    (gogoproto.moretags)    = "yaml:\"unbonding_period\""
   ];
   // defines how much new (untrusted) header's Time can drift into the future.
   google.protobuf.Duration max_clock_drift = 5 [
-    (gogoproto.nullable) = false,
+    (gogoproto.nullable)    = false,
     (gogoproto.stdduration) = true,
-    (gogoproto.moretags) = "yaml:\"max_clock_drift\""
+    (gogoproto.moretags)    = "yaml:\"max_clock_drift\""
   ];
   // Block height when the client was frozen due to a misbehaviour
   uint64 frozen_height = 6 [(gogoproto.moretags) = "yaml:\"frozen_height\""];
@@ -58,15 +60,42 @@ message ConsensusState {
   // commitment root (i.e app hash)
   ibc.commitment.MerkleRoot root = 2 [(gogoproto.nullable) = false];
   // height at which the consensus state was stored.
-  uint64 height = 3;
-  bytes next_validators_hash = 4 [
-    (gogoproto.casttype) = "github.com/tendermint/tendermint/libs/bytes.HexBytes",
+  uint64 height               = 3;
+  bytes  next_validators_hash = 4 [
+    (gogoproto.casttype) =
+        "github.com/tendermint/tendermint/libs/bytes.HexBytes",
     (gogoproto.moretags) = "yaml:\"next_validators_hash\""
   ];
 }
 
+// Header defines the Tendermint client consensus Header.
+// It encapsulates all the information necessary to update from a trusted
+// Tendermint ConsensusState. The inclusion of TrustedHeight and
+// TrustedValidators allows this update to process correctly, so long as the
+// ConsensusState for the TrustedHeight exists, this removes race conditions
+// among relayers The SignedHeader and ValidatorSet are the new untrusted update
+// fields for the client. The TrustedHeight is the height of a stored
+// ConsensusState on the client that will be used to verify the new untrusted
+// header. The Trusted ConsensusState must be within the unbonding period of
+// current time in order to correctly verify, and the TrustedValidators must
+// hash to TrustedConsensusState.NextValidatorsHash since that is the last
+// trusted validator set at the TrustedHeight.
+message Header {
+  .tendermint.types.SignedHeader signed_header = 1 [
+    (gogoproto.embed)    = true,
+    (gogoproto.nullable) = false,
+    (gogoproto.moretags) = "yaml:\"signed_header\""
+  ];
+
+  .tendermint.types.ValidatorSet validator_set = 2
+      [(gogoproto.moretags) = "yaml:\"validator_set\""];
+  uint64 trusted_height = 3 [(gogoproto.moretags) = "yaml:\"trusted_height\""];
+  .tendermint.types.ValidatorSet trusted_validators = 4
+      [(gogoproto.moretags) = "yaml:\"trusted_validators\""];
+}
+
 // Fraction defines the protobuf message type for tmmath.Fraction
 message Fraction {
-  int64 numerator = 1;
+  int64 numerator   = 1;
   int64 denominator = 2;
 }

third_party/proto/tendermint/types/validator.proto

@@ -0,0 +1,25 @@
+syntax = "proto3";
+package tendermint.types;
+
+option go_package = "github.com/tendermint/tendermint/proto/tendermint/types";
+
+import "gogoproto/gogo.proto";
+import "tendermint/crypto/keys.proto";
+
+message ValidatorSet {
+  repeated Validator validators         = 1;
+  Validator          proposer           = 2;
+  int64              total_voting_power = 3;
+}
+
+message Validator {
+  bytes                       address           = 1;
+  tendermint.crypto.PublicKey pub_key           = 2 [(gogoproto.nullable) = false];
+  int64                       voting_power      = 3;
+  int64                       proposer_priority = 4;
+}
+
+message SimpleValidator {
+  tendermint.crypto.PublicKey pub_key      = 1;
+  int64                       voting_power = 2;
+}

x/ibc/02-client/client/utils/utils.go

@@ -148,9 +148,15 @@ func QueryTendermintHeader(clientCtx client.Context) (ibctmtypes.Header, int64,
 		return ibctmtypes.Header{}, 0, err
 	}
 
+	protoCommit := commit.SignedHeader.ToProto()
+	protoValset, err := tmtypes.NewValidatorSet(validators.Validators).ToProto()
+	if err != nil {
+		return ibctmtypes.Header{}, 0, err
+	}
+
 	header := ibctmtypes.Header{
-		SignedHeader: commit.SignedHeader,
-		ValidatorSet: tmtypes.NewValidatorSet(validators.Validators),
+		SignedHeader: *protoCommit,
+		ValidatorSet: protoValset,
 	}
 
 	return header, height, nil

x/ibc/02-client/keeper/client_test.go

@@ -57,11 +57,11 @@ func (suite *KeeperTestSuite) TestCreateClient() {
 func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
 	// Must create header creation functions since suite.header gets recreated on each test case
 	createFutureUpdateFn := func(s *KeeperTestSuite) ibctmtypes.Header {
-		return ibctmtypes.CreateTestHeader(testChainID, suite.header.Height+3, suite.header.Height, suite.header.Time.Add(time.Hour),
+		return ibctmtypes.CreateTestHeader(testChainID, int64(suite.header.GetHeight()+3), int64(suite.header.GetHeight()), suite.header.Header.Time.Add(time.Hour),
 			suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal})
 	}
 	createPastUpdateFn := func(s *KeeperTestSuite) ibctmtypes.Header {
-		return ibctmtypes.CreateTestHeader(testChainID, suite.header.Height-2, suite.header.Height-4, suite.header.Time,
+		return ibctmtypes.CreateTestHeader(testChainID, int64(suite.header.GetHeight()-2), int64(suite.header.GetHeight())-4, suite.header.Header.Time,
 			suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal})
 	}
 	var (
@@ -189,7 +189,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
 			err := tc.malleate()
 			suite.Require().NoError(err)
 
-			suite.ctx = suite.ctx.WithBlockTime(updateHeader.Time.Add(time.Minute))
+			suite.ctx = suite.ctx.WithBlockTime(updateHeader.Header.Time.Add(time.Minute))
 
 			updatedClientState, err := suite.keeper.UpdateClient(suite.ctx, testClientID, updateHeader)
 
@@ -198,9 +198,9 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
 
 				expConsensusState := &ibctmtypes.ConsensusState{
 					Height:             updateHeader.GetHeight(),
-					Timestamp:          updateHeader.Time,
-					Root:               commitmenttypes.NewMerkleRoot(updateHeader.AppHash),
-					NextValidatorsHash: updateHeader.NextValidatorsHash,
+					Timestamp:          updateHeader.GetTime(),
+					Root:               commitmenttypes.NewMerkleRoot(updateHeader.Header.GetAppHash()),
+					NextValidatorsHash: updateHeader.Header.NextValidatorsHash,
 				}
 
 				newClientState, found := suite.keeper.GetClientState(suite.ctx, testClientID)
@@ -212,9 +212,9 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
 				suite.Require().Equal(updatedClientState, newClientState, "updatedClient state not persisted correctly")
 
 				// Determine if clientState should be updated or not
-				if uint64(updateHeader.Height) > clientState.GetLatestHeight() {
-					// Header Height is greater than clientState latest Height, clientState should be updated with header.Height
-					suite.Require().Equal(uint64(updateHeader.Height), updatedClientState.GetLatestHeight(), "clientstate height did not update")
+				if uint64(updateHeader.GetHeight()) > clientState.GetLatestHeight() {
+					// Header Height is greater than clientState latest Height, clientState should be updated with header.GetHeight()
+					suite.Require().Equal(uint64(updateHeader.GetHeight()), updatedClientState.GetLatestHeight(), "clientstate height did not update")
 				} else {
 					// Update will add past consensus state, clientState should not be updated at all
 					suite.Require().Equal(clientState.GetLatestHeight(), updatedClientState.GetLatestHeight(), "client state height updated for past header")

x/ibc/02-client/keeper/keeper.go

@@ -191,7 +191,7 @@ func (k Keeper) GetSelfConsensusState(ctx sdk.Context, height uint64) (exported.
 	consensusState := &ibctmtypes.ConsensusState{
 		Height:             height,
 		Timestamp:          histInfo.Header.Time,
-		Root:               commitmenttypes.NewMerkleRoot(histInfo.Header.AppHash),
+		Root:               commitmenttypes.NewMerkleRoot(histInfo.Header.GetAppHash()),
 		NextValidatorsHash: histInfo.Header.NextValidatorsHash,
 	}
 	return consensusState, true

x/ibc/02-client/keeper/keeper_test.go

@@ -212,11 +212,11 @@ func (suite KeeperTestSuite) TestConsensusStateHelpers() {
 
 	nextState := ibctmtypes.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot([]byte("next")), testClientHeight+5, suite.valSetHash)
 
-	header := ibctmtypes.CreateTestHeader(testClientID, testClientHeight+5, testClientHeight, suite.header.Time.Add(time.Minute),
+	header := ibctmtypes.CreateTestHeader(testClientID, testClientHeight+5, testClientHeight, suite.header.Header.Time.Add(time.Minute),
 		suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal})
 
 	// mock update functionality
-	clientState.LatestHeight = uint64(header.Height)
+	clientState.LatestHeight = header.GetHeight()
 	suite.keeper.SetClientConsensusState(suite.ctx, testClientID, testClientHeight+5, nextState)
 	suite.keeper.SetClientState(suite.ctx, testClientID, clientState)
 

x/ibc/02-client/types/genesis_test.go

@@ -60,7 +60,7 @@ func TestValidateGenesis(t *testing.T) {
 						clientID,
 						[]exported.ConsensusState{
 							ibctmtypes.NewConsensusState(
-								header.Time, commitmenttypes.NewMerkleRoot(header.AppHash), header.GetHeight(), header.NextValidatorsHash,
+								header.GetTime(), commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), header.GetHeight(), header.Header.NextValidatorsHash,
 							),
 						},
 					),
@@ -85,7 +85,7 @@ func TestValidateGenesis(t *testing.T) {
 						clientID,
 						[]exported.ConsensusState{
 							ibctmtypes.NewConsensusState(
-								header.Time, commitmenttypes.NewMerkleRoot(header.AppHash), header.GetHeight(), header.NextValidatorsHash,
+								header.GetTime(), commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), header.GetHeight(), header.Header.NextValidatorsHash,
 							),
 						},
 					),
@@ -124,7 +124,7 @@ func TestValidateGenesis(t *testing.T) {
 						"(CLIENTID2)",
 						[]exported.ConsensusState{
 							ibctmtypes.NewConsensusState(
-								header.Time, commitmenttypes.NewMerkleRoot(header.AppHash), 0, header.NextValidatorsHash,
+								header.GetTime(), commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), 0, header.Header.NextValidatorsHash,
 							),
 						},
 					),
@@ -149,7 +149,7 @@ func TestValidateGenesis(t *testing.T) {
 						clientID,
 						[]exported.ConsensusState{
 							ibctmtypes.NewConsensusState(
-								header.Time, commitmenttypes.NewMerkleRoot(header.AppHash), 0, header.NextValidatorsHash,
+								header.GetTime(), commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), 0, header.Header.NextValidatorsHash,
 							),
 						},
 					),

x/ibc/03-connection/keeper/keeper_test.go

@@ -118,12 +118,12 @@ func (suite *KeeperTestSuite) TestGetTimestampAtHeight() {
 			tc.malleate()
 
 			actualTimestamp, err := suite.chainA.App.IBCKeeper.ConnectionKeeper.GetTimestampAtHeight(
-				suite.chainA.GetContext(), connection, uint64(suite.chainB.LastHeader.Height),
+				suite.chainA.GetContext(), connection, uint64(suite.chainB.LastHeader.GetHeight()),
 			)
 
 			if tc.expPass {
 				suite.Require().NoError(err)
-				suite.Require().EqualValues(uint64(suite.chainB.LastHeader.Time.UnixNano()), actualTimestamp)
+				suite.Require().EqualValues(uint64(suite.chainB.LastHeader.GetTime().UnixNano()), actualTimestamp)
 			} else {
 				suite.Require().Error(err)
 			}

x/ibc/07-tendermint/types/client_state_test.go

@@ -106,7 +106,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() {
 		// 	name:        "successful verification",
 		// 	clientState: ibctmtypes.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 		// 	consensusState: ibctmtypes.ConsensusState{
-		// 		Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash),
+		// 		Root: commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()),
 		// 	},
 		// 	prefix:  commitmenttypes.NewMerklePrefix([]byte("ibc")),
 		// 	expPass: true,
@@ -115,7 +115,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() {
 			name:        "ApplyPrefix failed",
 			clientState: ibctmtypes.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			consensusState: ibctmtypes.ConsensusState{
-				Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash),
+				Root: commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()),
 			},
 			prefix:  commitmenttypes.MerklePrefix{},
 			expPass: false,
@@ -124,7 +124,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() {
 			name:        "latest client height < height",
 			clientState: ibctmtypes.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			consensusState: ibctmtypes.ConsensusState{
-				Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash),
+				Root: commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()),
 			},
 			prefix:  commitmenttypes.NewMerklePrefix([]byte("ibc")),
 			expPass: false,
@@ -133,7 +133,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() {
 			name:        "client is frozen",
 			clientState: &ibctmtypes.ClientState{LatestHeight: height, FrozenHeight: height - 1},
 			consensusState: ibctmtypes.ConsensusState{
-				Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash),
+				Root: commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()),
 			},
 			prefix:  commitmenttypes.NewMerklePrefix([]byte("ibc")),
 			expPass: false,
@@ -142,7 +142,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() {
 			name:        "proof verification failed",
 			clientState: ibctmtypes.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			consensusState: ibctmtypes.ConsensusState{
-				Root:               commitmenttypes.NewMerkleRoot(suite.header.AppHash),
+				Root:               commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()),
 				NextValidatorsHash: suite.valsHash,
 			},
 			prefix:  commitmenttypes.NewMerklePrefix([]byte("ibc")),

x/ibc/07-tendermint/types/evidence.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/tendermint/tendermint/crypto/tmhash"
 	tmbytes "github.com/tendermint/tendermint/libs/bytes"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	tmtypes "github.com/tendermint/tendermint/types"
 
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
@@ -72,14 +73,14 @@ func (ev Evidence) Hash() tmbytes.HexBytes {
 //
 // NOTE: assumes that evidence headers have the same height
 func (ev Evidence) GetHeight() int64 {
-	return int64(math.Min(float64(ev.Header1.Height), float64(ev.Header2.Height)))
+	return int64(math.Min(float64(ev.Header1.GetHeight()), float64(ev.Header2.GetHeight())))
 }
 
 // GetTime returns the timestamp at which misbehaviour occurred. It uses the
 // maximum value from both headers to prevent producing an invalid header outside
 // of the evidence age range.
 func (ev Evidence) GetTime() time.Time {
-	minTime := int64(math.Max(float64(ev.Header1.Time.UnixNano()), float64(ev.Header2.Time.UnixNano())))
+	minTime := int64(math.Max(float64(ev.Header1.GetTime().UnixNano()), float64(ev.Header2.GetTime().UnixNano())))
 	return time.Unix(0, minTime)
 }
 
@@ -116,12 +117,22 @@ func (ev Evidence) ValidateBasic() error {
 		)
 	}
 	// Ensure that Heights are the same
-	if ev.Header1.Height != ev.Header2.Height {
-		return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "headers in evidence are on different heights (%d ≠ %d)", ev.Header1.Height, ev.Header2.Height)
+	if ev.Header1.GetHeight() != ev.Header2.GetHeight() {
+		return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "headers in evidence are on different heights (%d ≠ %d)", ev.Header1.GetHeight(), ev.Header2.GetHeight())
 	}
+
+	blockID1, err := tmtypes.BlockIDFromProto(&ev.Header1.SignedHeader.Commit.BlockID)
+	if err != nil {
+		return sdkerrors.Wrap(err, "invalid block ID from header 1 in evidence")
+	}
+	blockID2, err := tmtypes.BlockIDFromProto(&ev.Header2.SignedHeader.Commit.BlockID)
+	if err != nil {
+		return sdkerrors.Wrap(err, "invalid block ID from header 2 in evidence")
+	}
+
 	// Ensure that Commit Hashes are different
-	if ev.Header1.Commit.BlockID.Equals(ev.Header2.Commit.BlockID) {
-		return sdkerrors.Wrap(clienttypes.ErrInvalidEvidence, "headers commit to same blockID")
+	if blockID1.Equals(*blockID2) {
+		return sdkerrors.Wrap(clienttypes.ErrInvalidEvidence, "headers blockIDs are not equal")
 	}
 	if err := ValidCommit(ev.ChainID, ev.Header1.Commit, ev.Header1.ValidatorSet); err != nil {
 		return err
@@ -137,21 +148,30 @@ func (ev Evidence) ValidateBasic() error {
 // CommitToVoteSet will panic if the commit cannot be converted to a valid voteset given the validatorset
 // This implies that someone tried to submit evidence that wasn't actually committed by the validatorset
 // thus we should return an error here and reject the evidence rather than panicing.
-func ValidCommit(chainID string, commit *tmtypes.Commit, valSet *tmtypes.ValidatorSet) (err error) {
+func ValidCommit(chainID string, commit *tmproto.Commit, valSet *tmproto.ValidatorSet) (err error) {
 	defer func() {
 		if r := recover(); r != nil {
 			err = sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "invalid commit: %v", r)
 		}
 	}()
 
+	tmCommit, err := tmtypes.CommitFromProto(commit)
+	if err != nil {
+		return sdkerrors.Wrap(err, "commit is not tendermint commit type")
+	}
+	tmValset, err := tmtypes.ValidatorSetFromProto(valSet)
+	if err != nil {
+		return sdkerrors.Wrap(err, "validator set is not tendermint validator set type")
+	}
+
 	// Convert commits to vote-sets given the validator set so we can check if they both have 2/3 power
-	voteSet := tmtypes.CommitToVoteSet(chainID, commit, valSet)
+	voteSet := tmtypes.CommitToVoteSet(chainID, tmCommit, tmValset)
 
 	blockID, ok := voteSet.TwoThirdsMajority()
 
 	// Check that ValidatorSet did indeed commit to blockID in Commit
-	if !ok || !blockID.Equals(commit.BlockID) {
-		return sdkerrors.Wrap(clienttypes.ErrInvalidEvidence, "validator set did not commit to header 1")
+	if !ok || !blockID.Equals(tmCommit.BlockID) {
+		return sdkerrors.Wrap(clienttypes.ErrInvalidEvidence, "validator set did not commit to header")
 	}
 
 	return nil