fix order of BlockID and Timestamp in Vote and Proposal

CHANGELOG_PENDING.md

@@ -12,6 +12,7 @@ Special thanks to external contributors on this release:
 - [config] \#2992 `allow_duplicate_ip` is now set to false
 
 - [privval] \#2926 split up `PubKeyMsg` into `PubKeyRequest` and `PubKeyResponse` to be consistent with other message types
+- [types] consistent field order of `CanonicalVote` and `CanonicalProposal` 
 
 * Apps
 

docs/spec/consensus/signing.md

@@ -59,9 +59,9 @@ type PartSetHeader struct {
 ```
 
 To be included in a valid vote or proposal, BlockID must either represent a `nil` block, or a complete one.
-We introduce two methods, `BlockID.IsNil()` and `BlockID.IsComplete()` for these cases, respectively.
+We introduce two methods, `BlockID.IsZero()` and `BlockID.IsComplete()` for these cases, respectively.
 
-`BlockID.IsNil()` returns true for BlockID `b` if each of the following
+`BlockID.IsZero()` returns true for BlockID `b` if each of the following
 are true:
 
 ```
@@ -81,16 +81,16 @@ len(b.PartsHeader.Hash) == 32
 
 ## Proposals
 
-The structure of a propsal for signing looks like:
+The structure of a proposal for signing looks like:
 
 ```
 type CanonicalProposal struct {
 	Type      SignedMsgType // type alias for byte
 	Height    int64         `binary:"fixed64"`
 	Round     int64         `binary:"fixed64"`
 	POLRound  int64         `binary:"fixed64"`
-	BlockID   BlockID
 	Timestamp time.Time
+	BlockID   BlockID
 	ChainID   string
 }
 ```
@@ -130,7 +130,7 @@ A vote is valid if each of the following lines evaluates to true for vote `v`:
 v.Type == 0x1 || v.Type == 0x2
 v.Height > 0
 v.Round >= 0
-v.BlockID.IsNil() || v.BlockID.IsValid()
+v.BlockID.IsZero() || v.BlockID.IsComplete()
 ```
 
 In other words, a vote is valid for signing if it contains the type of a Prevote

types/block.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/crypto/merkle"
+	"github.com/tendermint/tendermint/crypto/tmhash"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/version"
 )
@@ -788,11 +789,6 @@ type BlockID struct {
 	PartsHeader PartSetHeader `json:"parts"`
 }
 
-// IsZero returns true if this is the BlockID for a nil-block
-func (blockID BlockID) IsZero() bool {
-	return len(blockID.Hash) == 0 && blockID.PartsHeader.IsZero()
-}
-
 // Equals returns true if the BlockID matches the given BlockID
 func (blockID BlockID) Equals(other BlockID) bool {
 	return bytes.Equal(blockID.Hash, other.Hash) &&
@@ -820,6 +816,20 @@ func (blockID BlockID) ValidateBasic() error {
 	return nil
 }
 
+// IsZero returns true if this is the BlockID of a nil block.
+func (blockID BlockID) IsZero() bool {
+	return blockID.Hash == nil &&
+		blockID.PartsHeader.Total == 0 &&
+		blockID.PartsHeader.Hash == nil
+}
+
+// IsComplete returns true if this is a valid BlockID of a non-nil block.
+func (blockID BlockID) IsComplete() bool {
+	return len(blockID.Hash) == tmhash.Size &&
+		blockID.PartsHeader.Total > 0 &&
+		len(blockID.PartsHeader.Hash) == tmhash.Size
+}
+
 // String returns a human readable string representation of the BlockID
 func (blockID BlockID) String() string {
 	return fmt.Sprintf(`%v:%v`, blockID.Hash, blockID.PartsHeader)

types/canonical.go

@@ -27,8 +27,8 @@ type CanonicalProposal struct {
 	Height    int64         `binary:"fixed64"`
 	Round     int64         `binary:"fixed64"`
 	POLRound  int64         `binary:"fixed64"`
-	BlockID   CanonicalBlockID
 	Timestamp time.Time
+	BlockID   CanonicalBlockID
 	ChainID   string
 }
 
@@ -64,8 +64,8 @@ func CanonicalizeProposal(chainID string, proposal *Proposal) CanonicalProposal
 		Height:    proposal.Height,
 		Round:     int64(proposal.Round), // cast int->int64 to make amino encode it fixed64 (does not work for int)
 		POLRound:  int64(proposal.POLRound),
-		BlockID:   CanonicalizeBlockID(proposal.BlockID),
 		Timestamp: proposal.Timestamp,
+		BlockID:   CanonicalizeBlockID(proposal.BlockID),
 		ChainID:   chainID,
 	}
 }

types/proposal.go

@@ -60,6 +60,10 @@ func (p *Proposal) ValidateBasic() error {
 	if err := p.BlockID.ValidateBasic(); err != nil {
 		return fmt.Errorf("Wrong BlockID: %v", err)
 	}
+	// ValidateBasic above would pass even if the blockId was empty:
+	if !p.BlockID.IsComplete() {
+		return fmt.Errorf("Expected a complete, non-empty BlockID, got: %v", p.BlockID)
+	}
 
 	// NOTE: Timestamp validation is subtle and handled elsewhere.
 

types/vote.go

@@ -127,6 +127,11 @@ func (vote *Vote) ValidateBasic() error {
 	if err := vote.BlockID.ValidateBasic(); err != nil {
 		return fmt.Errorf("Wrong BlockID: %v", err)
 	}
+	// BlockID.ValidateBasic would not err if we for instance we have an empty hash but a
+	// non-empty PartsSetHeader:
+	if !vote.BlockID.IsZero() && !vote.BlockID.IsComplete() {
+		return fmt.Errorf("BlockID must be either empty or complete, got: %v", vote.BlockID)
+	}
 	if len(vote.ValidatorAddress) != crypto.AddressSize {
 		return fmt.Errorf("Expected ValidatorAddress size to be %d bytes, got %d bytes",
 			crypto.AddressSize,