Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simple merkle rfc compatibility #2713

Merged
merged 14 commits into from
Jan 13, 2019
1 change: 1 addition & 0 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
* Go API

* Blockchain Protocol
* Merkle trees now match the RFC 6962 specification

* P2P Protocol

Expand Down
2 changes: 1 addition & 1 deletion blockchain/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ func TestLoadBlockPart(t *testing.T) {
gotPart, _, panicErr := doFn(loadPart)
require.Nil(t, panicErr, "an existent and proper block should not panic")
require.Nil(t, res, "a properly saved block should return a proper block")
require.Equal(t, gotPart.(*types.Part).Hash(), part1.Hash(),
require.Equal(t, gotPart.(*types.Part), part1,
"expecting successful retrieval of previously saved block")
}

Expand Down
21 changes: 21 additions & 0 deletions crypto/merkle/hash.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package merkle

import (
"github.com/tendermint/tendermint/crypto/tmhash"
)

// TODO: make these have a large predefined capacity
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
var (
leafPrefix = []byte{0}
innerPrefix = []byte{1}
)

// returns tmhash(0x00 || leaf)
func leafHash(leaf []byte) []byte {
return tmhash.Sum(append(leafPrefix, leaf...))
}

// returns tmhash(0x01 || left || right)
func innerHash(left []byte, right []byte) []byte {
return tmhash.Sum(append(innerPrefix, append(left, right...)...))
}
8 changes: 4 additions & 4 deletions crypto/merkle/proof_simple_value.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ func (op SimpleValueOp) Run(args [][]byte) ([][]byte, error) {
hasher.Write(value) // does not error
vhash := hasher.Sum(nil)

bz := bytes.Buffer{}
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
// Wrap <op.Key, vhash> to hash the KVPair.
hasher = tmhash.New()
encodeByteSlice(hasher, []byte(op.key)) // does not error
encodeByteSlice(hasher, []byte(vhash)) // does not error
kvhash := hasher.Sum(nil)
encodeByteSlice(&bz, []byte(op.key)) // does not error
encodeByteSlice(&bz, []byte(vhash)) // does not error
kvhash := leafHash(bz.Bytes())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should indicate in the general merkle docs that the SimpleProofOp uses the rfc hashed version for .LeafHash


if !bytes.Equal(kvhash, op.Proof.LeafHash) {
return nil, cmn.NewError("leaf hash mismatch: want %X got %X", op.Proof.LeafHash, kvhash)
Expand Down
94 changes: 94 additions & 0 deletions crypto/merkle/rfc6962_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package merkle

// Copyright 2016 Google Inc. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// These tests were taken from https://github.com/google/trillian/blob/master/merkle/rfc6962/rfc6962.go,
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
// and consequently fall under the above license.
import (
"bytes"
"encoding/hex"
"testing"

"github.com/tendermint/tendermint/crypto/tmhash"
)

func TestRFC6962Hasher(t *testing.T) {
_, leafHashTrail := trailsFromByteSlices([][]byte{[]byte("L123456")})
leafHash := leafHashTrail.Hash
_, leafHashTrail = trailsFromByteSlices([][]byte{[]byte{}})
emptyLeafHash := leafHashTrail.Hash
for _, tc := range []struct {
desc string
got []byte
want string
}{
// Check that the empty hash is not the same as the hash of an empty leaf.
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
// echo -n 00 | xxd -r -p | sha256sum
{
desc: "RFC6962 Empty Leaf",
want: "6e340b9cffb37a989ca544e6bb780a2c78901d3fb33738768511a30617afa01d"[:tmhash.Size*2],
got: emptyLeafHash,
},
// echo -n 004C313233343536 | xxd -r -p | sha256sum
{
desc: "RFC6962 Leaf",
want: "395aa064aa4c29f7010acfe3f25db9485bbd4b91897b6ad7ad547639252b4d56"[:tmhash.Size*2],
got: leafHash,
},
// echo -n 014E3132334E343536 | xxd -r -p | sha256sum
{
desc: "RFC6962 Node",
want: "aa217fe888e47007fa15edab33c2b492a722cb106c64667fc2b044444de66bbb"[:tmhash.Size*2],
got: innerHash([]byte("N123"), []byte("N456")),
},
} {
t.Run(tc.desc, func(t *testing.T) {
wantBytes, err := hex.DecodeString(tc.want)
if err != nil {
t.Fatalf("hex.DecodeString(%x): %v", tc.want, err)
}
if got, want := tc.got, wantBytes; !bytes.Equal(got, want) {
t.Errorf("got %x, want %x", got, want)
}
})
}
}

func TestRFC6962HasherCollisions(t *testing.T) {
// Check that different leaves have different hashes.
leaf1, leaf2 := []byte("Hello"), []byte("World")
_, leafHashTrail := trailsFromByteSlices([][]byte{leaf1})
hash1 := leafHashTrail.Hash
_, leafHashTrail = trailsFromByteSlices([][]byte{leaf2})
hash2 := leafHashTrail.Hash
if bytes.Equal(hash1, hash2) {
t.Errorf("Leaf hashes should differ, but both are %x", hash1)
}
// Compute an intermediate subtree hash.
_, subHash1Trail := trailsFromByteSlices([][]byte{hash1, hash2})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if shouldn't also split up trailsFromByteSlices into hashChildren and hashLeaf or so. Orthogonal to this PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good idea to me, but lets open up in a separate issue

subHash1 := subHash1Trail.Hash
// Check that this is not the same as a leaf hash of their concatenation.
preimage := append(hash1, hash2...)
_, forgedHashTrail := trailsFromByteSlices([][]byte{preimage})
forgedHash := forgedHashTrail.Hash
if bytes.Equal(subHash1, forgedHash) {
t.Errorf("Hasher is not second-preimage resistant")
}
// Swap the order of nodes and check that the hash is different.
_, subHash2Trail := trailsFromByteSlices([][]byte{hash2, hash1})
subHash2 := subHash2Trail.Hash
if bytes.Equal(subHash1, subHash2) {
t.Errorf("Subtree hash does not depend on the order of leaves")
}
}
12 changes: 6 additions & 6 deletions crypto/merkle/simple_map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ func TestSimpleMap(t *testing.T) {
values []string // each string gets converted to []byte in test
want string
}{
{[]string{"key1"}, []string{"value1"}, "321d150de16dceb51c72981b432b115045383259b1a550adf8dc80f927508967"},
{[]string{"key1"}, []string{"value2"}, "2a9e4baf321eac99f6eecc3406603c14bc5e85bb7b80483cbfc75b3382d24a2f"},
{[]string{"key1"}, []string{"value1"}, "a44d3cc7daba1a4600b00a2434b30f8b970652169810d6dfa9fb1793a2189324"},
{[]string{"key1"}, []string{"value2"}, "0638e99b3445caec9d95c05e1a3fc1487b4ddec6a952ff337080360b0dcc078c"},
// swap order with 2 keys
{[]string{"key1", "key2"}, []string{"value1", "value2"}, "c4d8913ab543ba26aa970646d4c99a150fd641298e3367cf68ca45fb45a49881"},
{[]string{"key2", "key1"}, []string{"value2", "value1"}, "c4d8913ab543ba26aa970646d4c99a150fd641298e3367cf68ca45fb45a49881"},
{[]string{"key1", "key2"}, []string{"value1", "value2"}, "8fd19b19e7bb3f2b3ee0574027d8a5a4cec370464ea2db2fbfa5c7d35bb0cff3"},
{[]string{"key2", "key1"}, []string{"value2", "value1"}, "8fd19b19e7bb3f2b3ee0574027d8a5a4cec370464ea2db2fbfa5c7d35bb0cff3"},
// swap order with 3 keys
{[]string{"key1", "key2", "key3"}, []string{"value1", "value2", "value3"}, "b23cef00eda5af4548a213a43793f2752d8d9013b3f2b64bc0523a4791196268"},
{[]string{"key1", "key3", "key2"}, []string{"value1", "value3", "value2"}, "b23cef00eda5af4548a213a43793f2752d8d9013b3f2b64bc0523a4791196268"},
{[]string{"key1", "key2", "key3"}, []string{"value1", "value2", "value3"}, "1dd674ec6782a0d586a903c9c63326a41cbe56b3bba33ed6ff5b527af6efb3dc"},
{[]string{"key1", "key3", "key2"}, []string{"value1", "value3", "value2"}, "1dd674ec6782a0d586a903c9c63326a41cbe56b3bba33ed6ff5b527af6efb3dc"},
}
for i, tc := range tests {
db := newSimpleMap()
Expand Down
19 changes: 10 additions & 9 deletions crypto/merkle/simple_proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"errors"
"fmt"

"github.com/tendermint/tendermint/crypto/tmhash"
cmn "github.com/tendermint/tendermint/libs/common"
)

Expand Down Expand Up @@ -67,7 +66,8 @@ func SimpleProofsFromMap(m map[string][]byte) (rootHash []byte, proofs map[strin

// Verify that the SimpleProof proves the root hash.
// Check sp.Index/sp.Total manually if needed
func (sp *SimpleProof) Verify(rootHash []byte, leafHash []byte) error {
func (sp *SimpleProof) Verify(rootHash []byte, leaf []byte) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we change the semantics of Verify here? It used to take the hash of the thing passed to SimpleMerkle, and now it takes the pre-image bytes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we had to so that everything that uses it doesn't have to be aware of the 0 byte prefix.

leafHash := leafHash(leaf)
if sp.Total < 0 {
return errors.New("Proof total must be positive")
}
Expand Down Expand Up @@ -128,19 +128,19 @@ func computeHashFromAunts(index int, total int, leafHash []byte, innerHashes [][
if len(innerHashes) == 0 {
return nil
}
numLeft := (total + 1) / 2
numLeft := getSplitPoint(total)
if index < numLeft {
leftHash := computeHashFromAunts(index, numLeft, leafHash, innerHashes[:len(innerHashes)-1])
if leftHash == nil {
return nil
}
return simpleHashFromTwoHashes(leftHash, innerHashes[len(innerHashes)-1])
return innerHash(leftHash, innerHashes[len(innerHashes)-1])
}
rightHash := computeHashFromAunts(index-numLeft, total-numLeft, leafHash, innerHashes[:len(innerHashes)-1])
if rightHash == nil {
return nil
}
return simpleHashFromTwoHashes(innerHashes[len(innerHashes)-1], rightHash)
return innerHash(innerHashes[len(innerHashes)-1], rightHash)
}
}

Expand Down Expand Up @@ -182,12 +182,13 @@ func trailsFromByteSlices(items [][]byte) (trails []*SimpleProofNode, root *Simp
case 0:
return nil, nil
case 1:
trail := &SimpleProofNode{tmhash.Sum(items[0]), nil, nil, nil}
trail := &SimpleProofNode{leafHash(items[0]), nil, nil, nil}
return []*SimpleProofNode{trail}, trail
default:
lefts, leftRoot := trailsFromByteSlices(items[:(len(items)+1)/2])
rights, rightRoot := trailsFromByteSlices(items[(len(items)+1)/2:])
rootHash := simpleHashFromTwoHashes(leftRoot.Hash, rightRoot.Hash)
k := getSplitPoint(len(items))
lefts, leftRoot := trailsFromByteSlices(items[:k])
rights, rightRoot := trailsFromByteSlices(items[k:])
rootHash := innerHash(leftRoot.Hash, rightRoot.Hash)
root := &SimpleProofNode{rootHash, nil, nil, nil}
leftRoot.Parent = root
leftRoot.Right = rightRoot
Expand Down
38 changes: 19 additions & 19 deletions crypto/merkle/simple_tree.go
Original file line number Diff line number Diff line change
@@ -1,35 +1,22 @@
package merkle

import (
"github.com/tendermint/tendermint/crypto/tmhash"
"math/bits"
)

// simpleHashFromTwoHashes is the basic operation of the Merkle tree: Hash(left | right).
func simpleHashFromTwoHashes(left, right []byte) []byte {
var hasher = tmhash.New()
err := encodeByteSlice(hasher, left)
if err != nil {
panic(err)
}
err = encodeByteSlice(hasher, right)
if err != nil {
panic(err)
}
return hasher.Sum(nil)
}

// SimpleHashFromByteSlices computes a Merkle tree where the leaves are the byte slice,
// in the provided order.
func SimpleHashFromByteSlices(items [][]byte) []byte {
switch len(items) {
case 0:
return nil
case 1:
return tmhash.Sum(items[0])
return leafHash(items[0])
default:
left := SimpleHashFromByteSlices(items[:(len(items)+1)/2])
right := SimpleHashFromByteSlices(items[(len(items)+1)/2:])
return simpleHashFromTwoHashes(left, right)
k := getSplitPoint(len(items))
left := SimpleHashFromByteSlices(items[:k])
right := SimpleHashFromByteSlices(items[k:])
return innerHash(left, right)
}
}

Expand All @@ -44,3 +31,16 @@ func SimpleHashFromMap(m map[string][]byte) []byte {
}
return sm.Hash()
}

func getSplitPoint(length int) int {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments are helpful when we start doing bit magic.

if length < 1 {
panic("Trying to split a tree with size < 1")
}
uLength := uint(length)
bitlen := bits.Len(uLength)
k := 1 << uint(bitlen-1)
if k == length {
k >>= 1
}
return k
}
36 changes: 29 additions & 7 deletions crypto/merkle/simple_tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func TestSimpleProof(t *testing.T) {

// For each item, check the trail.
for i, item := range items {
itemHash := tmhash.Sum(item)
proof := proofs[i]

// Check total/index
Expand All @@ -43,30 +42,53 @@ func TestSimpleProof(t *testing.T) {
require.Equal(t, proof.Total, total, "Unmatched totals: %d vs %d", proof.Total, total)

// Verify success
err := proof.Verify(rootHash, itemHash)
require.NoError(t, err, "Verificatior failed: %v.", err)
err := proof.Verify(rootHash, item)
require.NoError(t, err, "Verificatiom failed: %v.", err)
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved

// Trail too long should make it fail
origAunts := proof.Aunts
proof.Aunts = append(proof.Aunts, cmn.RandBytes(32))
err = proof.Verify(rootHash, itemHash)
err = proof.Verify(rootHash, item)
require.Error(t, err, "Expected verification to fail for wrong trail length")

proof.Aunts = origAunts

// Trail too short should make it fail
proof.Aunts = proof.Aunts[0 : len(proof.Aunts)-1]
err = proof.Verify(rootHash, itemHash)
err = proof.Verify(rootHash, item)
require.Error(t, err, "Expected verification to fail for wrong trail length")

proof.Aunts = origAunts

// Mutating the itemHash should make it fail.
err = proof.Verify(rootHash, MutateByteSlice(itemHash))
err = proof.Verify(rootHash, MutateByteSlice(item))
require.Error(t, err, "Expected verification to fail for mutated leaf hash")

// Mutating the rootHash should make it fail.
err = proof.Verify(MutateByteSlice(rootHash), itemHash)
err = proof.Verify(MutateByteSlice(rootHash), item)
require.Error(t, err, "Expected verification to fail for mutated root hash")
}
}

func Test_getSplitPoint(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the _?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use https://github.com/cweill/gotests to automate alot of the boiler plate of table driven tests. I forgot to change the template to remove the underscore though!

tests := []struct {
length int
want int
}{
{1, 0},
{2, 1},
{3, 2},
{4, 2},
{5, 4},
{10, 8},
{20, 16},
{100, 64},
{255, 128},
{256, 128},
{257, 256},
}
for _, tt := range tests {
got := getSplitPoint(tt.length)
require.Equal(t, tt.want, got, "getSplitPoint(%d) = %v, want %v", tt.length, got, tt.want)
}
}