From f66b7a8e321630cf95296cad6c8fa3b1c33b0016 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 11 Aug 2020 12:31:05 +0200 Subject: [PATCH] merkle: return hashes for empty merkle trees (#5193) Fixes #5192. @liamsi Can you verify that the test vectors match the Rust implementation? I updated `ProofsFromByteSlices()` as well, anything else that should be updated? --- CHANGELOG_PENDING.md | 9 ++++++--- UPGRADING.md | 8 ++++++++ blockchain/msgs_test.go | 21 +++++++++++++-------- consensus/replay.go | 3 +++ crypto/merkle/hash.go | 5 +++++ crypto/merkle/proof.go | 2 +- crypto/merkle/rfc6962_test.go | 11 +++++++++-- crypto/merkle/tree.go | 6 +++--- crypto/merkle/tree_test.go | 33 ++++++++++++++++++++++++++++++++- types/block_test.go | 13 +++++++++---- types/part_set.go | 2 +- types/validator_set.go | 3 --- types/validator_set_test.go | 5 +++-- 13 files changed, 93 insertions(+), 28 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 71f5cfab8..c5a010eb5 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,4 +1,4 @@ -## v0.34.1 +## v0.34.0-rc3 Special thanks to external contributors on this release: @@ -6,9 +6,12 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi ### BREAKING CHANGES -- Go API +- Blockchain Protocol + - [\#5193](https://github.com/tendermint/tendermint/pull/5193) Header hashes are no longer empty for empty inputs, notably `DataHash`, `EvidenceHash`, and `LastResultsHash` (@erikgrinaker) - - [evidence] [\#5181](https://github.com/tendermint/tendermint/pull/5181) Phantom validator evidence was removed (also from abci) (@cmwaters) +- Go API + - [evidence] [\#5181](https://github.com/tendermint/tendermint/pull/5181) Phantom validator evidence was removed (also from abci) (@cmwaters) + - [merkle] [\#5193](https://github.com/tendermint/tendermint/pull/5193) `HashFromByteSlices` and `ProofsFromByteSlices` now return a hash for empty inputs, following RFC6962 (@erikgrinaker) - [crypto] [\#5214] Change `GenPrivKeySecp256k1` to `GenPrivKeyFromSecret` to be consistent with other keys ### FEATURES: diff --git a/UPGRADING.md b/UPGRADING.md index bb10866de..d4626e3f3 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -48,6 +48,11 @@ Merkle tree built from: GasWanted, GasUsed, Events)` responses; - `BeginBlock#Events`. +Merkle hashes of empty trees previously returned nothing, but now return the hash of an empty input, +to conform with RFC-6962. This mainly affects `Header#DataHash`, `Header#LastResultsHash`, and +`Header#EvidenceHash`, which are often empty. Non-empty hashes can also be affected, e.g. if their +inputs depend on other (empty) Merkle hashes, giving different results. + ### Tx Indexing - Tendermint will now rely on the application entirely to tell it what txs to index. This means that in the `config.toml`, @@ -100,6 +105,9 @@ The multisig that was previously located in Tendermint has now migrated to a new From the merkle package `SimpleHashFromMap()` and `SimpleProofsFromMap()` were removed along with all the prefixes of `Simple`. If you are looking for `SimpleProof` it has been renamed to `Proof` within the merkle pkg. Previously there were protobuf messages located in the merkle pkg, these have since been moved to the `/proto` directory. The protobuf message `Proof` that contained multiple ProofOp's has been renamed to `ProofOps`. This change effects the ABCI type `ResponseQuery`, the field that was named Proof is now named `ProofOps`. +`HashFromByteSlices` and `ProofsFromByteSlices` now return a hash for empty inputs, to conform with +RFC-6962. + ### Libs The Bech32 pkg has been migrated to a new home, you can find it in the [Cosmos-SDK](https://github.com/cosmos/cosmos-sdk/tree/4173ea5ebad906dd9b45325bed69b9c655504867/types/bech32) diff --git a/blockchain/msgs_test.go b/blockchain/msgs_test.go index 899bc6217..c984e4fe9 100644 --- a/blockchain/msgs_test.go +++ b/blockchain/msgs_test.go @@ -95,15 +95,20 @@ func TestBlockchainMessageVectors(t *testing.T) { BlockRequest: &bcproto.BlockRequest{Height: math.MaxInt64}}}, []byte{0xa, 0xa, 0x8, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f}}, {"BlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_BlockResponse{ - BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, []byte{0x1a, 0x6e, 0xa, 0x6c, - 0xa, 0x37, 0xa, 0x0, 0x18, 0x3, 0x22, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, - 0xff, 0xff, 0xff, 0x1, 0x2a, 0x2, 0x12, 0x0, 0x3a, 0x20, 0xc4, 0xda, 0x88, 0xe8, + BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, []byte{0x1a, 0xb3, 0x1, 0xa, + 0xb0, 0x1, 0xa, 0x59, 0xa, 0x0, 0x18, 0x3, 0x22, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, + 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, 0x2a, 0x2, 0x12, 0x0, 0x3a, 0x20, 0xc4, 0xda, + 0x88, 0xe8, 0x76, 0x6, 0x2a, 0xa1, 0x54, 0x34, 0x0, 0xd5, 0xd, 0xe, 0xaa, 0xd, 0xac, + 0x88, 0x9, 0x60, 0x57, 0x94, 0x9c, 0xfb, 0x7b, 0xca, 0x7f, 0x3a, 0x48, 0xc0, 0x4b, + 0xf9, 0x6a, 0x20, 0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, + 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, + 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55, 0x12, 0x2f, 0xa, 0xb, 0x48, 0x65, 0x6c, + 0x6c, 0x6f, 0x20, 0x57, 0x6f, 0x72, 0x6c, 0x64, 0x12, 0x20, 0xc4, 0xda, 0x88, 0xe8, 0x76, 0x6, 0x2a, 0xa1, 0x54, 0x34, 0x0, 0xd5, 0xd, 0xe, 0xaa, 0xd, 0xac, 0x88, 0x9, - 0x60, 0x57, 0x94, 0x9c, 0xfb, 0x7b, 0xca, 0x7f, 0x3a, 0x48, 0xc0, 0x4b, 0xf9, 0x12, - 0x2f, 0xa, 0xb, 0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x57, 0x6f, 0x72, 0x6c, 0x64, - 0x12, 0x20, 0xc4, 0xda, 0x88, 0xe8, 0x76, 0x6, 0x2a, 0xa1, 0x54, 0x34, 0x0, 0xd5, - 0xd, 0xe, 0xaa, 0xd, 0xac, 0x88, 0x9, 0x60, 0x57, 0x94, 0x9c, 0xfb, 0x7b, 0xca, - 0x7f, 0x3a, 0x48, 0xc0, 0x4b, 0xf9, 0x1a, 0x0}}, + 0x60, 0x57, 0x94, 0x9c, 0xfb, 0x7b, 0xca, 0x7f, 0x3a, 0x48, 0xc0, 0x4b, 0xf9, 0x1a, + 0x22, 0x12, 0x20, 0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, + 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, + 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55}}, {"NoBlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_NoBlockResponse{ NoBlockResponse: &bcproto.NoBlockResponse{Height: 1}}}, []byte{0x12, 0x2, 0x8, 0x1}}, {"NoBlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_NoBlockResponse{ diff --git a/consensus/replay.go b/consensus/replay.go index dc4ab9ab3..c27d66130 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -11,6 +11,7 @@ import ( dbm "github.com/tendermint/tm-db" abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/proxy" sm "github.com/tendermint/tendermint/state" @@ -332,6 +333,8 @@ func (h *Handshaker) ReplayBlocks( state.ConsensusParams = types.UpdateConsensusParams(state.ConsensusParams, res.ConsensusParams) state.Version.Consensus.App = state.ConsensusParams.Version.AppVersion } + // We update the last results hash with the empty hash, to conform with RFC-6962. + state.LastResultsHash = merkle.HashFromByteSlices(nil) sm.SaveState(h.stateDB, state) } } diff --git a/crypto/merkle/hash.go b/crypto/merkle/hash.go index 4e24046ac..d45130fe5 100644 --- a/crypto/merkle/hash.go +++ b/crypto/merkle/hash.go @@ -10,6 +10,11 @@ var ( innerPrefix = []byte{1} ) +// returns tmhash() +func emptyHash() []byte { + return tmhash.Sum([]byte{}) +} + // returns tmhash(0x00 || leaf) func leafHash(leaf []byte) []byte { return tmhash.Sum(append(leafPrefix, leaf...)) diff --git a/crypto/merkle/proof.go b/crypto/merkle/proof.go index f9a42fd02..ab43f30e7 100644 --- a/crypto/merkle/proof.go +++ b/crypto/merkle/proof.go @@ -217,7 +217,7 @@ func trailsFromByteSlices(items [][]byte) (trails []*ProofNode, root *ProofNode) // Recursive impl. switch len(items) { case 0: - return nil, nil + return []*ProofNode{}, &ProofNode{emptyHash(), nil, nil, nil} case 1: trail := &ProofNode{leafHash(items[0]), nil, nil, nil} return []*ProofNode{trail}, trail diff --git a/crypto/merkle/rfc6962_test.go b/crypto/merkle/rfc6962_test.go index 6c508164a..c762cda56 100644 --- a/crypto/merkle/rfc6962_test.go +++ b/crypto/merkle/rfc6962_test.go @@ -28,13 +28,20 @@ func TestRFC6962Hasher(t *testing.T) { leafHash := leafHashTrail.Hash _, leafHashTrail = trailsFromByteSlices([][]byte{{}}) emptyLeafHash := leafHashTrail.Hash + _, emptyHashTrail := trailsFromByteSlices([][]byte{}) + emptyTreeHash := emptyHashTrail.Hash for _, tc := range []struct { desc string got []byte want string }{ - // Since creating a merkle tree of no leaves is unsupported here, we skip - // the corresponding trillian test vector. + // Check that empty trees return the hash of an empty string. + // echo -n '' | sha256sum + { + desc: "RFC6962 Empty Tree", + want: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"[:tmhash.Size*2], + got: emptyTreeHash, + }, // Check that the empty hash is not the same as the hash of an empty leaf. // echo -n 00 | xxd -r -p | sha256sum diff --git a/crypto/merkle/tree.go b/crypto/merkle/tree.go index cbd51ca3a..466c43482 100644 --- a/crypto/merkle/tree.go +++ b/crypto/merkle/tree.go @@ -5,11 +5,11 @@ import ( ) // HashFromByteSlices computes a Merkle tree where the leaves are the byte slice, -// in the provided order. +// in the provided order. It follows RFC-6962. func HashFromByteSlices(items [][]byte) []byte { switch len(items) { case 0: - return nil + return emptyHash() case 1: return leafHash(items[0]) default: @@ -70,7 +70,7 @@ func HashFromByteSlicesIterative(input [][]byte) []byte { for { switch size { case 0: - return nil + return emptyHash() case 1: return items[0] default: diff --git a/crypto/merkle/tree_test.go b/crypto/merkle/tree_test.go index b8679256c..bdf469836 100644 --- a/crypto/merkle/tree_test.go +++ b/crypto/merkle/tree_test.go @@ -1,8 +1,10 @@ package merkle import ( + "encoding/hex" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" tmrand "github.com/tendermint/tendermint/libs/rand" @@ -17,8 +19,37 @@ func (tI testItem) Hash() []byte { return []byte(tI) } +func TestHashFromByteSlices(t *testing.T) { + testcases := map[string]struct { + slices [][]byte + expectHash string // in hex format + }{ + "nil": {nil, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + "empty": {[][]byte{}, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + "single": {[][]byte{{1, 2, 3}}, "054edec1d0211f624fed0cbca9d4f9400b0e491c43742af2c5b0abebf0c990d8"}, + "single blank": {[][]byte{{}}, "6e340b9cffb37a989ca544e6bb780a2c78901d3fb33738768511a30617afa01d"}, + "two": {[][]byte{{1, 2, 3}, {4, 5, 6}}, "82e6cfce00453804379b53962939eaa7906b39904be0813fcadd31b100773c4b"}, + "many": { + [][]byte{{1, 2}, {3, 4}, {5, 6}, {7, 8}, {9, 10}}, + "f326493eceab4f2d9ffbc78c59432a0a005d6ea98392045c74df5d14a113be18", + }, + } + for name, tc := range testcases { + tc := tc + t.Run(name, func(t *testing.T) { + hash := HashFromByteSlices(tc.slices) + assert.Equal(t, tc.expectHash, hex.EncodeToString(hash)) + }) + } +} + func TestProof(t *testing.T) { + // Try an empty proof first + rootHash, proofs := ProofsFromByteSlices([][]byte{}) + require.Equal(t, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", hex.EncodeToString(rootHash)) + require.Empty(t, proofs) + total := 100 items := make([][]byte, total) @@ -26,7 +57,7 @@ func TestProof(t *testing.T) { items[i] = testItem(tmrand.Bytes(tmhash.Size)) } - rootHash := HashFromByteSlices(items) + rootHash = HashFromByteSlices(items) rootHash2, proofs := ProofsFromByteSlices(items) diff --git a/types/block_test.go b/types/block_test.go index 19b56aee8..7accf5b66 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -202,14 +202,19 @@ func makeBlockID(hash []byte, partSetSize uint32, partSetHash []byte) BlockID { var nilBytes []byte +// This follows RFC-6962, i.e. `echo -n '' | sha256sum` +var emptyBytes = []byte{0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, + 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, + 0x78, 0x52, 0xb8, 0x55} + func TestNilHeaderHashDoesntCrash(t *testing.T) { - assert.Equal(t, []byte((*Header)(nil).Hash()), nilBytes) - assert.Equal(t, []byte((new(Header)).Hash()), nilBytes) + assert.Equal(t, nilBytes, []byte((*Header)(nil).Hash())) + assert.Equal(t, nilBytes, []byte((new(Header)).Hash())) } func TestNilDataHashDoesntCrash(t *testing.T) { - assert.Equal(t, []byte((*Data)(nil).Hash()), nilBytes) - assert.Equal(t, []byte(new(Data).Hash()), nilBytes) + assert.Equal(t, emptyBytes, []byte((*Data)(nil).Hash())) + assert.Equal(t, emptyBytes, []byte(new(Data).Hash())) } func TestCommit(t *testing.T) { diff --git a/types/part_set.go b/types/part_set.go index 6b00a5f78..ca93b2b1a 100644 --- a/types/part_set.go +++ b/types/part_set.go @@ -225,7 +225,7 @@ func (ps *PartSet) BitArray() *bits.BitArray { func (ps *PartSet) Hash() []byte { if ps == nil { - return nil + return merkle.HashFromByteSlices(nil) } return ps.hash } diff --git a/types/validator_set.go b/types/validator_set.go index c1abb44e4..1d8c0b756 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -345,9 +345,6 @@ func (vals *ValidatorSet) findProposer() *Validator { // Hash returns the Merkle root hash build using validators (as leaves) in the // set. func (vals *ValidatorSet) Hash() []byte { - if len(vals.Validators) == 0 { - return nil - } bzs := make([][]byte, len(vals.Validators)) for i, val := range vals.Validators { bzs[i] = val.Bytes() diff --git a/types/validator_set_test.go b/types/validator_set_test.go index f45288484..20a3ce2f7 100644 --- a/types/validator_set_test.go +++ b/types/validator_set_test.go @@ -46,8 +46,9 @@ func TestValidatorSetBasic(t *testing.T) { assert.Zero(t, vset.Size()) assert.Equal(t, int64(0), vset.TotalVotingPower()) assert.Nil(t, vset.GetProposer()) - assert.Nil(t, vset.Hash()) - + assert.Equal(t, []byte{0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, + 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, + 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55}, vset.Hash()) // add val = randValidator(vset.TotalVotingPower()) assert.NoError(t, vset.UpdateWithChangeSet([]*Validator{val}))