types: dont use SimpleHashFromMap for header. closes #1841 (#2670)

* types: dont use SimpleHashFromMap for header. closes #1841

* changelog and spec

* comments

CHANGELOG_PENDING.md

@@ -45,6 +45,8 @@ BREAKING CHANGES:
   * [types] \#2644 Add Version struct to Header
   * [state] \#2587 Require block.Time of the fist block to be genesis time
   * [state] \#2644 Require block.Version to match state.Version
+  * [types] \#2670 Header.Hash() builds Merkle tree out of fields in the same
+    order they appear in the header, instead of sorting by field name
 
 * P2P Protocol
   * [p2p] \#2654 Add `ProtocolVersion` struct with protocol versions to top of

docs/spec/blockchain/encoding.md

@@ -216,7 +216,7 @@ prefix) before being concatenated together and hashed.
 
 Note: we will abuse notion and invoke `SimpleMerkleRoot` with arguments of type `struct` or type `[]struct`.
 For `struct` arguments, we compute a `[][]byte` containing the hash of each
-field in the struct sorted by the hash of the field name.
+field in the struct, in the same order the fields appear in the struct.
 For `[]struct` arguments, we compute a `[][]byte` by hashing the individual `struct` elements.
 
 ### Simple Merkle Proof

types/block.go

@@ -258,8 +258,10 @@ func MaxDataBytesUnknownEvidence(maxBytes int64, valsCount int) int64 {
 //-----------------------------------------------------------------------------
 
 // Header defines the structure of a Tendermint block header
-// NOTE: changes to the Header should be duplicated in the abci Header
-// and in /docs/spec/blockchain/blockchain.md
+// NOTE: changes to the Header should be duplicated in:
+//  - header.Hash()
+// 	- abci.Header
+//  - /docs/spec/blockchain/blockchain.md
 type Header struct {
 	// basic block info
 	Version  version.Consensus `json:"version"`
@@ -289,6 +291,8 @@ type Header struct {
 }
 
 // Hash returns the hash of the header.
+// It computes a Merkle tree from the header fields
+// ordered as they appear in the Header.
 // Returns nil if ValidatorHash is missing,
 // since a Header is not valid unless there is
 // a ValidatorsHash (corresponding to the validator set).
@@ -296,23 +300,23 @@ func (h *Header) Hash() cmn.HexBytes {
 	if h == nil || len(h.ValidatorsHash) == 0 {
 		return nil
 	}
-	return merkle.SimpleHashFromMap(map[string][]byte{
-		"Version":            cdcEncode(h.Version),
-		"ChainID":            cdcEncode(h.ChainID),
-		"Height":             cdcEncode(h.Height),
-		"Time":               cdcEncode(h.Time),
-		"NumTxs":             cdcEncode(h.NumTxs),
-		"TotalTxs":           cdcEncode(h.TotalTxs),
-		"LastBlockID":        cdcEncode(h.LastBlockID),
-		"LastCommitHash":     cdcEncode(h.LastCommitHash),
-		"DataHash":           cdcEncode(h.DataHash),
-		"ValidatorsHash":     cdcEncode(h.ValidatorsHash),
-		"NextValidatorsHash": cdcEncode(h.NextValidatorsHash),
-		"AppHash":            cdcEncode(h.AppHash),
-		"ConsensusHash":      cdcEncode(h.ConsensusHash),
-		"LastResultsHash":    cdcEncode(h.LastResultsHash),
-		"EvidenceHash":       cdcEncode(h.EvidenceHash),
-		"ProposerAddress":    cdcEncode(h.ProposerAddress),
+	return merkle.SimpleHashFromByteSlices([][]byte{
+		cdcEncode(h.Version),
+		cdcEncode(h.ChainID),
+		cdcEncode(h.Height),
+		cdcEncode(h.Time),
+		cdcEncode(h.NumTxs),
+		cdcEncode(h.TotalTxs),
+		cdcEncode(h.LastBlockID),
+		cdcEncode(h.LastCommitHash),
+		cdcEncode(h.DataHash),
+		cdcEncode(h.ValidatorsHash),
+		cdcEncode(h.NextValidatorsHash),
+		cdcEncode(h.ConsensusHash),
+		cdcEncode(h.AppHash),
+		cdcEncode(h.LastResultsHash),
+		cdcEncode(h.EvidenceHash),
+		cdcEncode(h.ProposerAddress),
 	})
 }