diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 332cfbf76c4..83ec4d8fdb4 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -7,6 +7,7 @@ Special thanks to external contributors on this release: ### BREAKING CHANGES: * CLI/RPC/Config +- [types] consistent field order of `CanonicalVote` and `CanonicalProposal` * Apps diff --git a/docs/spec/blockchain/blockchain.md b/docs/spec/blockchain/blockchain.md index cda3232650b..f80c8c05f8f 100644 --- a/docs/spec/blockchain/blockchain.md +++ b/docs/spec/blockchain/blockchain.md @@ -109,10 +109,6 @@ Tendermint uses the [Google.Protobuf.WellKnownTypes.Timestamp](https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/timestamp) format, which uses two integers, one for Seconds and for Nanoseconds. -NOTE: there is currently a small divergence between Tendermint and the -Google.Protobuf.WellKnownTypes.Timestamp that should be resolved. See [this -issue](https://github.com/tendermint/go-amino/issues/223) for details. - ## Data Data is just a wrapper for a list of transactions, where transactions are diff --git a/docs/spec/blockchain/encoding.md b/docs/spec/blockchain/encoding.md index cb506739fe1..689ebbd6aa7 100644 --- a/docs/spec/blockchain/encoding.md +++ b/docs/spec/blockchain/encoding.md @@ -301,8 +301,8 @@ type CanonicalVote struct { Type byte Height int64 `binary:"fixed64"` Round int64 `binary:"fixed64"` - Timestamp time.Time BlockID CanonicalBlockID + Timestamp time.Time ChainID string } ``` diff --git a/docs/spec/consensus/signing.md b/docs/spec/consensus/signing.md index d1ee71a631a..f97df0c6596 100644 --- a/docs/spec/consensus/signing.md +++ b/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,7 +81,7 @@ 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 { @@ -118,8 +118,8 @@ type CanonicalVote struct { Type SignedMsgType // type alias for byte Height int64 `binary:"fixed64"` Round int64 `binary:"fixed64"` - Timestamp time.Time BlockID BlockID + Timestamp time.Time 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 diff --git a/types/block.go b/types/block.go index 15b88d81d8b..93315ade5c9 100644 --- a/types/block.go +++ b/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,19 @@ 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 len(blockID.Hash) == 0 && + blockID.PartsHeader.IsZero() +} + +// 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) diff --git a/types/canonical.go b/types/canonical.go index eabd7684887..47a8c817f1a 100644 --- a/types/canonical.go +++ b/types/canonical.go @@ -36,8 +36,8 @@ type CanonicalVote struct { Type SignedMsgType // type alias for byte Height int64 `binary:"fixed64"` Round int64 `binary:"fixed64"` - Timestamp time.Time BlockID CanonicalBlockID + Timestamp time.Time ChainID string } @@ -75,8 +75,8 @@ func CanonicalizeVote(chainID string, vote *Vote) CanonicalVote { Type: vote.Type, Height: vote.Height, Round: int64(vote.Round), // cast int->int64 to make amino encode it fixed64 (does not work for int) - Timestamp: vote.Timestamp, BlockID: CanonicalizeBlockID(vote.BlockID), + Timestamp: vote.Timestamp, ChainID: chainID, } } diff --git a/types/part_set.go b/types/part_set.go index a040258d1cb..7e1aa3c35d2 100644 --- a/types/part_set.go +++ b/types/part_set.go @@ -75,7 +75,7 @@ func (psh PartSetHeader) String() string { } func (psh PartSetHeader) IsZero() bool { - return psh.Total == 0 + return psh.Total == 0 && len(psh.Hash) == 0 } func (psh PartSetHeader) Equals(other PartSetHeader) bool { diff --git a/types/proposal.go b/types/proposal.go index f3b62aae7c5..97c06596eda 100644 --- a/types/proposal.go +++ b/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. diff --git a/types/vote.go b/types/vote.go index bf14d403bcb..8ff51d3c765 100644 --- a/types/vote.go +++ b/types/vote.go @@ -52,8 +52,8 @@ type Vote struct { Type SignedMsgType `json:"type"` Height int64 `json:"height"` Round int `json:"round"` - Timestamp time.Time `json:"timestamp"` BlockID BlockID `json:"block_id"` // zero if vote is nil. + Timestamp time.Time `json:"timestamp"` ValidatorAddress Address `json:"validator_address"` ValidatorIndex int `json:"validator_index"` Signature []byte `json:"signature"` @@ -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 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, diff --git a/types/vote_test.go b/types/vote_test.go index 942f2d6b9e4..aefa4fcfa56 100644 --- a/types/vote_test.go +++ b/types/vote_test.go @@ -63,8 +63,8 @@ func TestVoteSignableTestVectors(t *testing.T) { { CanonicalizeVote("", &Vote{}), // NOTE: Height and Round are skipped here. This case needs to be considered while parsing. - // []byte{0x22, 0x9, 0x9, 0x0, 0x9, 0x6e, 0x88, 0xf1, 0xff, 0xff, 0xff}, - []byte{0x22, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1}, + // []byte{0x2a, 0x9, 0x9, 0x0, 0x9, 0x6e, 0x88, 0xf1, 0xff, 0xff, 0xff}, + []byte{0x2a, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1}, }, // with proper (fixed size) height and round (PreCommit): { @@ -76,7 +76,7 @@ func TestVoteSignableTestVectors(t *testing.T) { 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // height 0x19, // (field_number << 3) | wire_type 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round - 0x22, // (field_number << 3) | wire_type + 0x2a, // (field_number << 3) | wire_type // remaining fields (timestamp): 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1}, }, @@ -90,7 +90,7 @@ func TestVoteSignableTestVectors(t *testing.T) { 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // height 0x19, // (field_number << 3) | wire_type 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round - 0x22, // (field_number << 3) | wire_type + 0x2a, // (field_number << 3) | wire_type // remaining fields (timestamp): 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1}, }, @@ -102,7 +102,7 @@ func TestVoteSignableTestVectors(t *testing.T) { 0x19, // (field_number << 3) | wire_type 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round // remaining fields (timestamp): - 0x22, + 0x2a, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1}, }, // containing non-empty chain_id: @@ -114,7 +114,7 @@ func TestVoteSignableTestVectors(t *testing.T) { 0x19, // (field_number << 3) | wire_type 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round // remaining fields: - 0x22, // (field_number << 3) | wire_type + 0x2a, // (field_number << 3) | wire_type 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, // timestamp 0x32, // (field_number << 3) | wire_type 0xd, 0x74, 0x65, 0x73, 0x74, 0x5f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x5f, 0x69, 0x64}, // chainID