Enforce solo machine signature type uniqueness

proto/ibc/lightclients/solomachine/v1/solomachine.proto

@@ -60,8 +60,9 @@ message Misbehaviour {
 // signature.
 message SignatureAndData {
   option (gogoproto.goproto_getters) = false;
-  bytes signature                    = 1;
-  bytes data                         = 2;
+  bytes    signature                 = 1;
+  DataType data_type                 = 2 [(gogoproto.moretags) = "yaml:\"data_type\""];
+  bytes    data                      = 3;
 }
 
 // TimestampedSignature contains the signature and the timestamp of the
@@ -79,11 +80,40 @@ message SignBytes {
   uint64 sequence    = 1;
   uint64 timestamp   = 2;
   string diversifier = 3;
+  // type of the data used
+  DataType data_type = 4 [(gogoproto.moretags) = "yaml:\"data_type\""];
   // marshaled data
-  bytes data = 4;
-}
-
-// HeaderData returns the SignBytes data for misbehaviour verification.
+  bytes data = 5;
+}
+
+// DataType defines the type of solo machine proof being created. This is done to preserve uniqueness of different
+// data sign byte encodings.
+enum DataType {
+  option (gogoproto.goproto_enum_prefix) = false;
+
+  // Default State
+  DATA_TYPE_UNINITIALIZED_UNSPECIFIED = 0 [(gogoproto.enumvalue_customname) = "UNSPECIFIED"];
+  // Data type for client state verification
+  DATA_TYPE_CLIENT_STATE = 1 [(gogoproto.enumvalue_customname) = "CLIENT"];
+  // Data type for consensus state verification
+  DATA_TYPE_CONSENSUS_STATE = 2 [(gogoproto.enumvalue_customname) = "CONSENSUS"];
+  // Data type for connection state verification
+  DATA_TYPE_CONNECTION_STATE = 3 [(gogoproto.enumvalue_customname) = "CONNECTION"];
+  // Data type for channel state verification
+  DATA_TYPE_CHANNEL_STATE = 4 [(gogoproto.enumvalue_customname) = "CHANNEL"];
+  // Data type for packet commitment verification
+  DATA_TYPE_PACKET_COMMITMENT = 5 [(gogoproto.enumvalue_customname) = "PACKETCOMMITMENT"];
+  // Data type for packet acknowledgement verification
+  DATA_TYPE_PACKET_ACKNOWLEDGEMENT = 6 [(gogoproto.enumvalue_customname) = "PACKETACKNOWLEDGEMENT"];
+  // Data type for packet receipt absence verification
+  DATA_TYPE_PACKET_RECEIPT_ABSENCE = 7 [(gogoproto.enumvalue_customname) = "PACKETRECEIPTABSENCE"];
+  // Data type for next sequence recv verification
+  DATA_TYPE_NEXT_SEQUENCE_RECV = 8 [(gogoproto.enumvalue_customname) = "NEXTSEQUENCERECV"];
+  // Data type for header verification
+  DATA_TYPE_HEADER = 9 [(gogoproto.enumvalue_customname) = "HEADER"];
+}
+
+// HeaderData returns the SignBytes data for update verification.
 message HeaderData {
   option (gogoproto.goproto_getters) = false;
 
@@ -101,7 +131,7 @@ message ClientStateData {
   google.protobuf.Any client_state = 2 [(gogoproto.moretags) = "yaml:\"client_state\""];
 }
 
-// ConsensusStateSignBytes returns the SignBytes data for consensus state
+// ConsensusStateData returns the SignBytes data for consensus state
 // verification.
 message ConsensusStateData {
   option (gogoproto.goproto_getters) = false;
@@ -110,7 +140,7 @@ message ConsensusStateData {
   google.protobuf.Any consensus_state = 2 [(gogoproto.moretags) = "yaml:\"consensus_state\""];
 }
 
-// ConnectionStateSignBytes returns the SignBytes data for connection state
+// ConnectionStateData returns the SignBytes data for connection state
 // verification.
 message ConnectionStateData {
   option (gogoproto.goproto_getters) = false;
@@ -119,7 +149,7 @@ message ConnectionStateData {
   ibc.connection.ConnectionEnd connection = 2;
 }
 
-// ChannelStateSignBytes returns the SignBytes data for channel state
+// ChannelStateData returns the SignBytes data for channel state
 // verification.
 message ChannelStateData {
   option (gogoproto.goproto_getters) = false;
@@ -128,27 +158,27 @@ message ChannelStateData {
   ibc.channel.Channel channel = 2;
 }
 
-// PacketCommitmentSignBytes returns the SignBytes data for packet commitment
+// PacketCommitmentData returns the SignBytes data for packet commitment
 // verification.
 message PacketCommitmentData {
   bytes path       = 1;
   bytes commitment = 2;
 }
 
-// PacketAcknowledgementSignBytes returns the SignBytes data for acknowledgement
+// PacketAcknowledgementData returns the SignBytes data for acknowledgement
 // verification.
 message PacketAcknowledgementData {
   bytes path            = 1;
   bytes acknowledgement = 2;
 }
 
-// PacketReceiptAbsenceSignBytes returns the SignBytes data for
+// PacketReceiptAbsenceData returns the SignBytes data for
 // packet receipt absence verification.
-message PacketReceiptAbsenseData {
+message PacketReceiptAbsenceData {
   bytes path = 1;
 }
 
-// NextSequenceRecv returns the SignBytes data for verification of the next
+// NextSequenceRecvData returns the SignBytes data for verification of the next
 // sequence to be received.
 message NextSequenceRecvData {
   bytes  path          = 1;

x/ibc/light-clients/06-solomachine/spec/01_concepts.md

@@ -6,17 +6,18 @@ order: 1
 
 ## Proofs
 
-A solo machine proof should verify that the solomachine public key signed 
+A solo machine proof should verify that the solomachine public key signed
 over some specified data. The format for generating marshaled proofs for
 the SDK's implementation of solo machine is as follows:
 
 Construct the data using the associated protobuf definition and marshal it.
 
 For example:
+
 ```go
 data := &ClientStateData{
-	Path:        []byte(path.String()),
-	ClientState: any,
+  Path:        []byte(path.String()),
+  ClientState: any,
 }
 
 dataBz, err := cdc.MarshalBinaryBare(data)
@@ -25,27 +26,32 @@ dataBz, err := cdc.MarshalBinaryBare(data)
 Construct the `SignBytes` and marshal it.
 
 For example:
+
 ```go
 signBytes := &SignBytes{
-	Sequence:    sequence,
-	Timestamp:   timestamp,
-	Diversifier: diversifier,
-	Data:        dataBz,
+  Sequence:    sequence,
+  Timestamp:   timestamp,
+  Diversifier: diversifier,
+  DataType:    CLIENT,
+  Data:        dataBz,
 }
 
 signBz, err := cdc.MarshalBinaryBare(signBytes)
 ```
 
 The helper functions in [proofs.go](../types/proofs.go) handle the above actions.
+The `DataType` field is used to disambiguate what type of data was signed because
+of potential proto encoding overlap.
 
 Sign the sign bytes. Embed the signatures into either `SingleSignatureData` or
-`MultiSignatureData`. Convert the `SignatureData` to proto and marshal it. 
+`MultiSignatureData`. Convert the `SignatureData` to proto and marshal it
 
 For example:
+
 ```go
 sig, err := key.Sign(signBz)
 sigData := &signing.SingleSignatureData{
-	Signature: sig,
+  Signature: sig,
 }
 
 protoSigData := signing.SignatureDataToProto(sigData)
@@ -56,10 +62,11 @@ Construct a `TimestampedSignature` and marshal it. The marshaled result can be
 passed in as the proof parameter to the verification functions.
 
 For example:
+
 ```go
 timestampedSignature := &types.TimestampedSignature{
-	Signature: sig,
-	Timestamp: solomachine.Time,
+  Signature: sig,
+  Timestamp: solomachine.Time,
 }
 
 proof, err := cdc.MarshalBinaryBare(timestampedSignature)

x/ibc/light-clients/06-solomachine/types/client_state_test.go

@@ -5,7 +5,6 @@ import (
 	connectiontypes "github.com/cosmos/cosmos-sdk/x/ibc/03-connection/types"
 	channeltypes "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/types"
 	commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types"
-	host "github.com/cosmos/cosmos-sdk/x/ibc/24-host"
 	"github.com/cosmos/cosmos-sdk/x/ibc/exported"
 	"github.com/cosmos/cosmos-sdk/x/ibc/light-clients/06-solomachine/types"
 	ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing"
@@ -85,10 +84,7 @@ func (suite *SoloMachineTestSuite) TestVerifyClientState() {
 	// create client for tendermint so we can use client state for verification
 	clientA, _ := suite.coordinator.SetupClients(suite.chainA, suite.chainB, ibctesting.Tendermint)
 	clientState := suite.chainA.GetClientState(clientA)
-
-	clientPrefixedPath := "clients/" + counterpartyClientIdentifier + "/" + host.ClientStatePath()
-	path, err := commitmenttypes.ApplyPrefix(prefix, clientPrefixedPath)
-	suite.Require().NoError(err)
+	path := suite.solomachine.GetClientStatePath(counterpartyClientIdentifier)
 
 	// test singlesig and multisig public keys
 	for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
@@ -217,9 +213,7 @@ func (suite *SoloMachineTestSuite) TestVerifyClientConsensusState() {
 	consensusState, found := suite.chainA.GetConsensusState(clientA, clientState.GetLatestHeight())
 	suite.Require().True(found)
 
-	clientPrefixedPath := "clients/" + counterpartyClientIdentifier + "/" + host.ConsensusStatePath(consensusHeight)
-	path, err := commitmenttypes.ApplyPrefix(prefix, clientPrefixedPath)
-	suite.Require().NoError(err)
+	path := suite.solomachine.GetConsensusStatePath(counterpartyClientIdentifier, consensusHeight)
 
 	// test singlesig and multisig public keys
 	for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
@@ -344,8 +338,7 @@ func (suite *SoloMachineTestSuite) TestVerifyConnectionState() {
 	counterparty := connectiontypes.NewCounterparty("clientB", testConnectionID, prefix)
 	conn := connectiontypes.NewConnectionEnd(connectiontypes.OPEN, "clientA", counterparty, []string{"1.0.0"})
 
-	path, err := commitmenttypes.ApplyPrefix(prefix, host.ConnectionPath(testConnectionID))
-	suite.Require().NoError(err)
+	path := suite.solomachine.GetConnectionStatePath(testConnectionID)
 
 	// test singlesig and multisig public keys
 	for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
@@ -434,8 +427,7 @@ func (suite *SoloMachineTestSuite) TestVerifyChannelState() {
 	counterparty := channeltypes.NewCounterparty(testPortID, testChannelID)
 	ch := channeltypes.NewChannel(channeltypes.OPEN, channeltypes.ORDERED, counterparty, []string{testConnectionID}, "1.0.0")
 
-	path, err := commitmenttypes.ApplyPrefix(prefix, host.ChannelPath(testPortID, testChannelID))
-	suite.Require().NoError(err)
+	path := suite.solomachine.GetChannelStatePath(testPortID, testChannelID)
 
 	// test singlesig and multisig public keys
 	for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
@@ -526,8 +518,7 @@ func (suite *SoloMachineTestSuite) TestVerifyPacketCommitment() {
 	// test singlesig and multisig public keys
 	for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
 
-		path, err := commitmenttypes.ApplyPrefix(prefix, host.PacketCommitmentPath(testPortID, testChannelID, solomachine.Sequence))
-		suite.Require().NoError(err)
+		path := solomachine.GetPacketCommitmentPath(testPortID, testChannelID)
 
 		value, err := types.PacketCommitmentSignBytes(suite.chainA.Codec, solomachine.Sequence, solomachine.Time, solomachine.Diversifier, path, commitmentBytes)
 		suite.Require().NoError(err)
@@ -614,8 +605,7 @@ func (suite *SoloMachineTestSuite) TestVerifyPacketAcknowledgement() {
 	// test singlesig and multisig public keys
 	for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
 
-		path, err := commitmenttypes.ApplyPrefix(prefix, host.PacketAcknowledgementPath(testPortID, testChannelID, solomachine.Sequence))
-		suite.Require().NoError(err)
+		path := solomachine.GetPacketAcknowledgementPath(testPortID, testChannelID)
 
 		value, err := types.PacketAcknowledgementSignBytes(suite.chainA.Codec, solomachine.Sequence, solomachine.Time, solomachine.Diversifier, path, ack)
 		suite.Require().NoError(err)
@@ -701,8 +691,8 @@ func (suite *SoloMachineTestSuite) TestVerifyPacketReceiptAbsence() {
 	// test singlesig and multisig public keys
 	for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
 
-		path, err := commitmenttypes.ApplyPrefix(prefix, host.PacketReceiptPath(testPortID, testChannelID, solomachine.Sequence))
-		suite.Require().NoError(err)
+		// absence uses receipt path as well
+		path := solomachine.GetPacketReceiptPath(testPortID, testChannelID)
 
 		value, err := types.PacketReceiptAbsenceSignBytes(suite.chainA.Codec, solomachine.Sequence, solomachine.Time, solomachine.Diversifier, path)
 		suite.Require().NoError(err)
@@ -789,8 +779,7 @@ func (suite *SoloMachineTestSuite) TestVerifyNextSeqRecv() {
 	for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
 
 		nextSeqRecv := solomachine.Sequence + 1
-		path, err := commitmenttypes.ApplyPrefix(prefix, host.NextSequenceRecvPath(testPortID, testChannelID))
-		suite.Require().NoError(err)
+		path := solomachine.GetNextSequenceRecvPath(testPortID, testChannelID)
 
 		value, err := types.NextSequenceRecvSignBytes(suite.chainA.Codec, solomachine.Sequence, solomachine.Time, solomachine.Diversifier, path, nextSeqRecv)
 		suite.Require().NoError(err)

x/ibc/light-clients/06-solomachine/types/codec.go

@@ -5,6 +5,7 @@ import (
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/types/tx/signing"
+	clienttypes "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
 	"github.com/cosmos/cosmos-sdk/x/ibc/exported"
 )
 
@@ -48,3 +49,91 @@ func UnmarshalSignatureData(cdc codec.BinaryMarshaler, data []byte) (signing.Sig
 
 	return sigData, nil
 }
+
+// UnmarshalDataByType attempts to unmarshal the data to the specified type. An error is
+// return if it fails.
+func UnmarshalDataByType(cdc codec.BinaryMarshaler, dataType DataType, data []byte) (Data, error) {
+	if len(data) == 0 {
+		return nil, sdkerrors.Wrap(ErrInvalidSignatureAndData, "data cannot be empty")
+	}
+
+	switch dataType {
+	case UNSPECIFIED:
+		return nil, sdkerrors.Wrap(ErrInvalidDataType, "data type cannot be UNSPECIFIED")
+
+	case CLIENT:
+		clientData := &ClientStateData{}
+		if err := cdc.UnmarshalBinaryBare(data, clientData); err != nil {
+			return nil, err
+		}
+
+		// unpack any
+		if _, err := clienttypes.UnpackClientState(clientData.ClientState); err != nil {
+			return nil, err
+		}
+		return clientData, nil
+
+	case CONSENSUS:
+		consensusData := &ConsensusStateData{}
+		if err := cdc.UnmarshalBinaryBare(data, consensusData); err != nil {
+			return nil, err
+		}
+
+		// unpack any
+		if _, err := clienttypes.UnpackConsensusState(consensusData.ConsensusState); err != nil {
+			return nil, err
+		}
+		return consensusData, nil
+
+	case CONNECTION:
+		connectionData := &ConnectionStateData{}
+		if err := cdc.UnmarshalBinaryBare(data, connectionData); err != nil {
+			return nil, err
+		}
+
+		return connectionData, nil
+
+	case CHANNEL:
+		channelData := &ChannelStateData{}
+		if err := cdc.UnmarshalBinaryBare(data, channelData); err != nil {
+			return nil, err
+		}
+
+		return channelData, nil
+
+	case PACKETCOMMITMENT:
+		commitmentData := &PacketCommitmentData{}
+		if err := cdc.UnmarshalBinaryBare(data, commitmentData); err != nil {
+			return nil, err
+		}
+
+		return commitmentData, nil
+
+	case PACKETACKNOWLEDGEMENT:
+		ackData := &PacketAcknowledgementData{}
+		if err := cdc.UnmarshalBinaryBare(data, ackData); err != nil {
+			return nil, err
+		}
+
+		return ackData, nil
+
+	case PACKETRECEIPTABSENCE:
+		receiptAbsenceData := &PacketReceiptAbsenceData{}
+		if err := cdc.UnmarshalBinaryBare(data, receiptAbsenceData); err != nil {
+			return nil, err
+		}
+
+		return receiptAbsenceData, nil
+
+	case NEXTSEQUENCERECV:
+		nextSeqRecvData := &NextSequenceRecvData{}
+		if err := cdc.UnmarshalBinaryBare(data, nextSeqRecvData); err != nil {
+			return nil, err
+		}
+
+		return nextSeqRecvData, nil
+
+	default:
+		return nil, sdkerrors.Wrapf(ErrInvalidDataType, "unsupported data type %T", dataType)
+	}
+}