From 4ed3e87831c63397df69a0219fe1cc49bfcb7bfe Mon Sep 17 00:00:00 2001 From: Ismail Khoffi Date: Tue, 3 Jul 2018 02:56:39 +0100 Subject: [PATCH 01/13] Release/v0.9.1 (#79) * VerifyItem takes no index; Return keys/values in range; Fix * compute hash from rangeproof * Require Verify() before Verify*() * Review fixes from #75 * Bump version, update changelog --- CHANGELOG.md | 19 ++++ basic_test.go | 2 +- proof_path.go | 48 +++++++++- proof_range.go | 232 ++++++++++++++++++++++++++++++++----------------- proof_test.go | 126 ++++++++++++++++----------- tree_test.go | 8 +- version.go | 3 +- 7 files changed, 296 insertions(+), 142 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cb0117fd..0aa35a4cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## 0.9.1 (July 1, 2018) + +IMPROVEMENTS + +- RangeProof.ComputeRootHash() to compute root rather than provide as in Verify(hash) +- RangeProof.Verify\*() first require .Verify(root), which memoizes + +## 0.9.0 (July 1, 2018) + +BREAKING CHANGES + +- RangeProof.VerifyItem doesn't require an index. +- Only return values in range when getting proof. +- Return keys as well. + +BUG FIXES + +- traversal bugs in traverseRange. + ## 0.8.2 * Swap `tmlibs` for `tendermint/libs` diff --git a/basic_test.go b/basic_test.go index 500c72385..699177fde 100644 --- a/basic_test.go +++ b/basic_test.go @@ -450,7 +450,7 @@ func TestTreeProof(t *testing.T) { assert.Equal(t, key, value) err := proof.Verify(root) assert.NoError(t, err, "#### %v", proof.String()) - err = proof.VerifyItem(0, key, key) + err = proof.VerifyItem(key, key) assert.NoError(t, err, "#### %v", proof.String()) } } diff --git a/proof_path.go b/proof_path.go index c578b2af0..e865a3765 100644 --- a/proof_path.go +++ b/proof_path.go @@ -28,8 +28,19 @@ func (pwl pathWithLeaf) StringIndented(indent string) string { indent) } +// `verify` checks that the leaf node's hash + the inner nodes merkle-izes to +// the given root. If it returns an error, it means the leafHash or the +// PathToLeaf is incorrect. func (pwl pathWithLeaf) verify(root []byte) cmn.Error { - return pwl.Path.verify(pwl.Leaf.Hash(), root) + leafHash := pwl.Leaf.Hash() + return pwl.Path.verify(leafHash, root) +} + +// `computeRootHash` computes the root hash with leaf node. +// Does not verify the root hash. +func (pwl pathWithLeaf) computeRootHash() []byte { + leafHash := pwl.Leaf.Hash() + return pwl.Path.computeRootHash(leafHash) } //---------------------------------------- @@ -62,9 +73,9 @@ func (pl PathToLeaf) StringIndented(indent string) string { indent) } -// verify checks that the leaf node's hash + the inner nodes merkle-izes to the -// given root. If it returns an error, it means the leafHash or the PathToLeaf -// is incorrect. +// `verify` checks that the leaf node's hash + the inner nodes merkle-izes to +// the given root. If it returns an error, it means the leafHash or the +// PathToLeaf is incorrect. func (pl PathToLeaf) verify(leafHash []byte, root []byte) cmn.Error { hash := leafHash for i := len(pl) - 1; i >= 0; i-- { @@ -77,6 +88,17 @@ func (pl PathToLeaf) verify(leafHash []byte, root []byte) cmn.Error { return nil } +// `computeRootHash` computes the root hash assuming some leaf hash. +// Does not verify the root hash. +func (pl PathToLeaf) computeRootHash(leafHash []byte) []byte { + hash := leafHash + for i := len(pl) - 1; i >= 0; i-- { + pin := pl[i] + hash = pin.Hash(hash) + } + return hash +} + func (pl PathToLeaf) isLeftmost() bool { for _, node := range pl { if len(node.Left) > 0 { @@ -125,3 +147,21 @@ func (pl PathToLeaf) isLeftAdjacentTo(pl2 PathToLeaf) bool { return pl.isRightmost() && pl2.isLeftmost() } + +// returns -1 if invalid. +func (pl PathToLeaf) Index() (idx int64) { + for i, node := range pl { + if node.Left == nil { + continue + } else if node.Right == nil { + if i < len(pl)-1 { + idx += node.Size - pl[i+1].Size + } else { + idx += node.Size - 1 + } + } else { + return -1 + } + } + return idx +} diff --git a/proof_range.go b/proof_range.go index 6f7466984..ba9834084 100644 --- a/proof_range.go +++ b/proof_range.go @@ -3,6 +3,7 @@ package iavl import ( "bytes" "fmt" + "sort" "strings" "github.com/tendermint/tendermint/crypto/tmhash" @@ -12,16 +13,39 @@ import ( type RangeProof struct { // You don't need the right path because // it can be derived from what we have. - RootHash cmn.HexBytes `json:"root_hash"` LeftPath PathToLeaf `json:"left_path"` InnerNodes []PathToLeaf `json:"inner_nodes"` Leaves []proofLeafNode `json:"leaves"` - // temporary - treeEnd int // 0 if not set, 1 if true, -1 if false. + + // memoize + rootVerified bool + rootHash []byte // valid iff rootVerified is true + treeEnd bool // valid iff rootVerified is true + +} + +// Keys returns all the keys in the RangeProof. NOTE: The keys here may +// include more keys than provided by tree.GetRangeWithProof or +// VersionedTree.GetVersionedRangeWithProof. The keys returned there are only +// in the provided [startKey,endKey){limit} range. The keys returned here may +// include extra keys, such as: +// - the key before startKey if startKey is provided and doesn't exist; +// - the key after a queried key with tree.GetWithProof, when the key is absent. +func (proof *RangeProof) Keys() (keys [][]byte) { + if proof == nil { + return nil + } + for _, leaf := range proof.Leaves { + keys = append(keys, leaf.Key) + } + return keys } // String returns a string representation of the proof. func (proof *RangeProof) String() string { + if proof == nil { + return "" + } return proof.StringIndented("") } @@ -35,36 +59,54 @@ func (proof *RangeProof) StringIndented(indent string) string { lstrs = append(lstrs, leaf.StringIndented(indent+" ")) } return fmt.Sprintf(`RangeProof{ -%s RootHash: %X %s LeftPath: %v %s InnerNodes: %s %v %s Leaves: %s %v +%s (rootVerified): %v +%s (rootHash): %X %s (treeEnd): %v %s}`, - indent, proof.RootHash, indent, proof.LeftPath.StringIndented(indent+" "), indent, indent, strings.Join(istrs, "\n"+indent+" "), indent, indent, strings.Join(lstrs, "\n"+indent+" "), + indent, proof.rootVerified, + indent, proof.rootHash, indent, proof.treeEnd, indent) } -// Verify that a leaf is some value. -// Does not assume that the proof itself is value. -// For that, use Verify(root). -func (proof *RangeProof) VerifyItem(i int, key, value []byte) error { +// The index of the first leaf (of the whole tree). +// Returns -1 if the proof is nil. +func (proof *RangeProof) LeftIndex() int64 { + if proof == nil { + return -1 + } + return proof.LeftPath.Index() +} + +// Also see LeftIndex(). +// Verify that a key has some value. +// Does not assume that the proof itself is valid, call Verify() first. +func (proof *RangeProof) VerifyItem(key, value []byte) error { + leaves := proof.Leaves if proof == nil { return cmn.ErrorWrap(ErrInvalidProof, "proof is nil") } - if !bytes.Equal(proof.Leaves[i].Key, key) { - return cmn.ErrorWrap(ErrInvalidProof, "leaf key not same") + if !proof.rootVerified { + return cmn.NewError("must call Verify(root) first.") + } + i := sort.Search(len(leaves), func(i int) bool { + return bytes.Compare(key, leaves[i].Key) <= 0 + }) + if i >= len(leaves) || !bytes.Equal(leaves[i].Key, key) { + return cmn.ErrorWrap(ErrInvalidProof, "leaf key not found in proof") } valueHash := tmhash.Sum(value) - if !bytes.Equal(proof.Leaves[i].ValueHash, valueHash) { + if !bytes.Equal(leaves[i].ValueHash, valueHash) { return cmn.ErrorWrap(ErrInvalidProof, "leaf value hash not same") } return nil @@ -77,7 +119,7 @@ func (proof *RangeProof) VerifyAbsence(key []byte) error { if proof == nil { return cmn.ErrorWrap(ErrInvalidProof, "proof is nil") } - if proof.treeEnd == 0 { + if !proof.rootVerified { return cmn.NewError("must call Verify(root) first.") } cmp := bytes.Compare(key, proof.Leaves[0].Key) @@ -116,7 +158,7 @@ func (proof *RangeProof) VerifyAbsence(key []byte) error { } // It's still a valid proof if our last leaf is the rightmost child. - if proof.treeEnd == 1 { + if proof.treeEnd { return nil // OK! } @@ -133,26 +175,53 @@ func (proof *RangeProof) Verify(root []byte) error { if proof == nil { return cmn.ErrorWrap(ErrInvalidProof, "proof is nil") } - treeEnd, err := proof._verify(root) - if err == nil { - if treeEnd { - proof.treeEnd = 1 // memoize - } else { - proof.treeEnd = -1 // memoize + err := proof.verify(root) + return err +} + +func (proof *RangeProof) verify(root []byte) (err error) { + rootHash := proof.rootHash + if rootHash == nil { + derivedHash, err := proof.computeRootHash() + if err != nil { + return err } + rootHash = derivedHash } - return err + if !bytes.Equal(rootHash, root) { + return cmn.ErrorWrap(ErrInvalidRoot, "root hash doesn't match") + } else { + proof.rootVerified = true + } + return nil } -func (proof *RangeProof) _verify(root []byte) (treeEnd bool, err error) { - if !bytes.Equal(proof.RootHash, root) { - return false, cmn.ErrorWrap(ErrInvalidRoot, "root hash doesn't match") +// ComputeRootHash computes the root hash with leaves. +// Returns nil if error or proof is nil. +// Does not verify the root hash. +func (proof *RangeProof) ComputeRootHash() []byte { + if proof == nil { + return nil } + rootHash, _ := proof.computeRootHash() + return rootHash +} + +func (proof *RangeProof) computeRootHash() (rootHash []byte, err error) { + rootHash, treeEnd, err := proof._computeRootHash() + if err == nil { + proof.rootHash = rootHash // memoize + proof.treeEnd = treeEnd // memoize + } + return rootHash, err +} + +func (proof *RangeProof) _computeRootHash() (rootHash []byte, treeEnd bool, err error) { if len(proof.Leaves) == 0 { - return false, cmn.ErrorWrap(ErrInvalidProof, "no leaves") + return nil, false, cmn.ErrorWrap(ErrInvalidProof, "no leaves") } if len(proof.InnerNodes)+1 != len(proof.Leaves) { - return false, cmn.ErrorWrap(ErrInvalidProof, "InnerNodes vs Leaves length mismatch, leaves should be 1 more.") + return nil, false, cmn.ErrorWrap(ErrInvalidProof, "InnerNodes vs Leaves length mismatch, leaves should be 1 more.") } // Start from the left path and prove each leaf. @@ -160,28 +229,27 @@ func (proof *RangeProof) _verify(root []byte) (treeEnd bool, err error) { // shared across recursive calls var leaves = proof.Leaves var innersq = proof.InnerNodes - var VERIFY func(path PathToLeaf, root []byte, rightmost bool) (treeEnd bool, done bool, err error) + var COMPUTEHASH func(path PathToLeaf, rightmost bool) (hash []byte, treeEnd bool, done bool, err error) + // rightmost: is the root a rightmost child of the tree? // treeEnd: true iff the last leaf is the last item of the tree. - // NOTE: root doesn't necessarily mean root of the tree here. - VERIFY = func(path PathToLeaf, root []byte, rightmost bool) (treeEnd bool, done bool, err error) { + // Returns the (possibly intermediate, possibly root) hash. + COMPUTEHASH = func(path PathToLeaf, rightmost bool) (hash []byte, treeEnd bool, done bool, err error) { // Pop next leaf. nleaf, rleaves := leaves[0], leaves[1:] leaves = rleaves - // Verify leaf with path. - if err := (pathWithLeaf{ + // Compute hash. + hash = (pathWithLeaf{ Path: path, Leaf: nleaf, - }).verify(root); err != nil { - return false, false, err.Trace(0, "verifying some path to a leaf") - } + }).computeRootHash() // If we don't have any leaves left, we're done. if len(leaves) == 0 { rightmost = rightmost && path.isRightmost() - return rightmost, true, nil + return hash, rightmost, true, nil } // Prove along path (until we run out of leaves). @@ -203,31 +271,35 @@ func (proof *RangeProof) _verify(root []byte) (treeEnd bool, err error) { innersq = rinnersq // Recursively verify inners against remaining leaves. - treeEnd, done, err := VERIFY(inners, lpath.Right, rightmost && rpath.isRightmost()) + derivedRoot, treeEnd, done, err := COMPUTEHASH(inners, rightmost && rpath.isRightmost()) if err != nil { - return treeEnd, false, cmn.ErrorWrap(err, "recursive VERIFY call") - } else if done { - return treeEnd, true, nil + return nil, treeEnd, false, cmn.ErrorWrap(err, "recursive COMPUTEHASH call") + } + if !bytes.Equal(derivedRoot, lpath.Right) { + return nil, treeEnd, false, cmn.ErrorWrap(ErrInvalidRoot, "intermediate root hash %X doesn't match, got %X", lpath.Right, derivedRoot) + } + if done { + return hash, treeEnd, true, nil } } - // We're not done yet. No error, not done either. Technically if - // rightmost, we know there's an error "left over leaves -- malformed - // proof", but we return that at the top level, below. - return false, false, nil + // We're not done yet (leaves left over). No error, not done either. + // Technically if rightmost, we know there's an error "left over leaves + // -- malformed proof", but we return that at the top level, below. + return hash, false, false, nil } // Verify! path := proof.LeftPath - treeEnd, done, err := VERIFY(path, root, true) + rootHash, treeEnd, done, err := COMPUTEHASH(path, true) if err != nil { - return treeEnd, cmn.ErrorWrap(err, "root VERIFY call") + return nil, treeEnd, cmn.ErrorWrap(err, "root COMPUTEHASH call") } else if !done { - return treeEnd, cmn.ErrorWrap(ErrInvalidProof, "left over leaves -- malformed proof") + return nil, treeEnd, cmn.ErrorWrap(ErrInvalidProof, "left over leaves -- malformed proof") } // Ok! - return treeEnd, nil + return rootHash, treeEnd, nil } /////////////////////////////////////////////////////////////////////////////// @@ -236,13 +308,17 @@ func (proof *RangeProof) _verify(root []byte) (treeEnd bool, err error) { // If keyStart or keyEnd don't exist, the leaf before keyStart // or after keyEnd will also be included, but not be included in values. // If keyEnd-1 exists, no later leaves will be included. +// If keyStart >= keyEnd and both not nil, panics. // Limit is never exceeded. -func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangeProof, values [][]byte, err error) { +func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangeProof, keys, values [][]byte, err error) { + if keyStart != nil && keyEnd != nil && bytes.Compare(keyStart, keyEnd) >= 0 { + panic("if keyStart and keyEnd are present, need keyStart < keyEnd.") + } if limit < 0 { panic("limit must be greater or equal to 0 -- 0 means no limit") } if t.root == nil { - return nil, nil, cmn.ErrorWrap(ErrNilRoot, "") + return nil, nil, nil, cmn.ErrorWrap(ErrNilRoot, "") } t.root.hashWithCount() // Ensure that all hashes are calculated. @@ -250,10 +326,17 @@ func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangePr path, left, err := t.root.PathToLeaf(t, keyStart) if err != nil { // Key doesn't exist, but instead we got the prev leaf (or the - // first leaf), which provides proof of absence). + // first or last leaf), which provides proof of absence). err = nil } - values = append(values, left.value) + startOK := keyStart == nil || bytes.Compare(keyStart, left.key) <= 0 + endOK := keyEnd == nil || bytes.Compare(left.key, keyEnd) < 0 + // If left.key is in range, add it to key/values. + if startOK && endOK { + keys = append(keys, left.key) // == keyStart + values = append(values, left.value) + } + // Either way, add to proof leaves. var leaves = []proofLeafNode{proofLeafNode{ Key: left.key, ValueHash: tmhash.Sum(left.value), @@ -270,18 +353,9 @@ func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangePr } if _stop { return &RangeProof{ - RootHash: t.root.hash, LeftPath: path, Leaves: leaves, - }, values, nil - } - - if keyEnd != nil && bytes.Compare(cpIncr(left.key), keyEnd) >= 0 { - return &RangeProof{ - RootHash: t.root.hash, - LeftPath: path, - Leaves: leaves, - }, values, nil + }, keys, values, nil } // Get the key after left.key to iterate from. @@ -295,7 +369,7 @@ func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangePr var lastDepth uint8 = 0 var leafCount = 1 // from left above. var pathCount = 0 - // var values [][]byte defined as function outs. + // var keys, values [][]byte defined as function outs. t.root.traverseInRange(t, afterLeft, nil, true, false, 0, func(node *Node, depth uint8) (stop bool) { @@ -331,14 +405,20 @@ func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangePr ValueHash: tmhash.Sum(node.value), Version: node.version, }) - // Append value to values. - values = append(values, node.value) leafCount += 1 // Maybe terminate because we found enough leaves. if limit > 0 && limit <= leafCount { return true } - // Maybe terminate because we've found keyEnd-1 or after. + // Terminate if we've found keyEnd or after. + if keyEnd != nil && bytes.Compare(node.key, keyEnd) >= 0 { + return true + } + // Value is in range, append to keys and values. + keys = append(keys, node.key) + values = append(values, node.value) + // Terminate if we've found keyEnd-1 or after. + // We don't want to fetch any leaves for it. if keyEnd != nil && bytes.Compare(cpIncr(node.key), keyEnd) >= 0 { return true } @@ -362,11 +442,10 @@ func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangePr ) return &RangeProof{ - RootHash: t.root.hash, LeftPath: path, InnerNodes: innersq, Leaves: leaves, - }, values, nil + }, keys, values, nil } //---------------------------------------- @@ -374,7 +453,7 @@ func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangePr // GetWithProof gets the value under the key if it exists, or returns nil. // A proof of existence or absence is returned alongside the value. func (t *Tree) GetWithProof(key []byte) (value []byte, proof *RangeProof, err error) { - proof, values, err := t.getRangeProof(key, cpIncr(key), 2) + proof, _, values, err := t.getRangeProof(key, cpIncr(key), 2) if err == nil { if len(values) > 0 { if !bytes.Equal(proof.Leaves[0].Key, key) { @@ -390,18 +469,13 @@ func (t *Tree) GetWithProof(key []byte) (value []byte, proof *RangeProof, err er } // GetRangeWithProof gets key/value pairs within the specified range and limit. -// To specify a descending range, swap the start and end keys. func (t *Tree) GetRangeWithProof(startKey []byte, endKey []byte, limit int) (keys, values [][]byte, proof *RangeProof, err error) { - proof, values, err = t.getRangeProof(startKey, endKey, limit) - for _, leaf := range proof.Leaves { - keys = append(keys, leaf.Key) - } + proof, keys, values, err = t.getRangeProof(startKey, endKey, limit) return } // GetVersionedWithProof gets the value under the key at the specified version -// if it exists, or returns nil. A proof of existence or absence is returned -// alongside the value. +// if it exists, or returns nil. func (tree *VersionedTree) GetVersionedWithProof(key []byte, version int64) ([]byte, *RangeProof, error) { if t, ok := tree.versions[version]; ok { return t.GetWithProof(key) @@ -410,10 +484,10 @@ func (tree *VersionedTree) GetVersionedWithProof(key []byte, version int64) ([]b } // GetVersionedRangeWithProof gets key/value pairs within the specified range -// and limit. To specify a descending range, swap the start and end keys. -// -// Returns a list of values, a list of keys, and a proof. -func (tree *VersionedTree) GetVersionedRangeWithProof(startKey, endKey []byte, limit int, version int64) ([][]byte, [][]byte, *RangeProof, error) { +// and limit. +func (tree *VersionedTree) GetVersionedRangeWithProof(startKey, endKey []byte, limit int, version int64) ( + keys, values [][]byte, proof *RangeProof, err error) { + if t, ok := tree.versions[version]; ok { return t.GetRangeWithProof(startKey, endKey, limit) } diff --git a/proof_test.go b/proof_test.go index 45d5968a3..4410ac6aa 100644 --- a/proof_test.go +++ b/proof_test.go @@ -25,9 +25,11 @@ func TestTreeGetWithProof(t *testing.T) { require.NoError(err) require.NotEmpty(val) require.NotNil(proof) + err = proof.VerifyItem(key, val) + require.Error(err, "%+v", err) // Verifying item before calling Verify(root) err = proof.Verify(root) require.NoError(err, "%+v", err) - err = proof.VerifyItem(0, key, val) + err = proof.VerifyItem(key, val) require.NoError(err, "%+v", err) key = []byte{0x1} @@ -35,6 +37,8 @@ func TestTreeGetWithProof(t *testing.T) { require.NoError(err) require.Empty(val) require.NotNil(proof) + err = proof.VerifyAbsence(key) + require.Error(err, "%+v", err) // Verifying absence before calling Verify(root) err = proof.Verify(root) require.NoError(err, "%+v", err) err = proof.VerifyAbsence(key) @@ -46,52 +50,50 @@ func TestTreeKeyExistsProof(t *testing.T) { root := tree.Hash() // should get false for proof with nil root - proof, _, err := tree.getRangeProof([]byte("foo"), nil, 1) + proof, _, _, err := tree.getRangeProof([]byte("foo"), nil, 1) assert.NotNil(t, err) assert.NotNil(t, proof.Verify(root)) // insert lots of info and store the bytes - keys := make([][]byte, 200) + allkeys := make([][]byte, 200) for i := 0; i < 200; i++ { key := randstr(20) value := "value_for_" + key tree.Set([]byte(key), []byte(value)) - keys[i] = []byte(key) + allkeys[i] = []byte(key) } - sortByteSlices(keys) // Sort keys + sortByteSlices(allkeys) // Sort all keys root = tree.Hash() // query random key fails - proof, _, err = tree.getRangeProof([]byte("foo"), nil, 2) + proof, _, _, err = tree.getRangeProof([]byte("foo"), nil, 2) assert.Nil(t, err) assert.Nil(t, proof.Verify(root)) assert.Nil(t, proof.VerifyAbsence([]byte("foo")), proof.String()) // query min key fails - proof, _, err = tree.getRangeProof([]byte{0x00}, []byte{0x00}, 2) + proof, _, _, err = tree.getRangeProof([]byte{0x00}, []byte{0x01}, 2) assert.Nil(t, err) assert.Nil(t, proof.Verify(root)) assert.Nil(t, proof.VerifyAbsence([]byte{0x00})) // valid proof for real keys - for i, key := range keys { - var values [][]byte - proof, values, err = tree.getRangeProof(key, nil, 2) + for i, key := range allkeys { + var keys, values [][]byte + proof, keys, values, err = tree.getRangeProof(key, nil, 2) require.Nil(t, err) require.Equal(t, append([]byte("value_for_"), key...), values[0], ) + require.Equal(t, key, keys[0]) require.Nil(t, proof.Verify(root)) require.Nil(t, proof.VerifyAbsence(cpIncr(key))) - if i < len(keys)-1 { - // There should be 2 items as per limit. - if len(values) != 2 { - printNode(tree.ndb, tree.root, 0) - } - require.Equal(t, 2, len(values), proof.String()) - if i < len(keys)-2 { + require.Equal(t, 1, len(keys), proof.String()) + require.Equal(t, 1, len(values), proof.String()) + if i < len(allkeys)-1 { + if i < len(allkeys)-2 { // No last item... not a proof of absence of large key. require.NotNil(t, proof.VerifyAbsence(bytes.Repeat([]byte{0xFF}, 20)), proof.String()) } else { @@ -99,8 +101,6 @@ func TestTreeKeyExistsProof(t *testing.T) { require.Nil(t, proof.VerifyAbsence(bytes.Repeat([]byte{0xFF}, 20))) } } else { - // There should be 1 item since no more can be queried. - require.Equal(t, 1, len(values), values) // last item of tree... valid proof of absence of large key. require.Nil(t, proof.VerifyAbsence(bytes.Repeat([]byte{0xFF}, 20))) } @@ -118,29 +118,41 @@ func TestTreeKeyInRangeProofs(t *testing.T) { } root := tree.Hash() + // For spacing: + T := 10 + nil______ := []byte(nil) + cases := []struct { - startKey byte - endKey byte - keys []byte // one byte per key. + start byte + end byte + pkeys []byte // proof keys, one byte per key. + vals []byte // keys and values, one byte per key. + lidx int64 // proof left index (index of first proof key). + pnc bool // does panic }{ - {startKey: 0x0a, endKey: 0xf7, keys: keys[:10]}, - {startKey: 0x0a, endKey: 0xf8, keys: keys[:10]}, - {startKey: 0x0, endKey: 0xff, keys: keys[:]}, - {startKey: 0x14, endKey: 0xe4, keys: keys[1:9]}, - {startKey: 0x14, endKey: 0xe5, keys: keys[1:9]}, - {startKey: 0x14, endKey: 0xe6, keys: keys[1:10]}, - {startKey: 0x14, endKey: 0xf1, keys: keys[1:10]}, - {startKey: 0x14, endKey: 0xf7, keys: keys[1:10]}, - {startKey: 0x14, endKey: 0xff, keys: keys[1:10]}, - {startKey: 0x2e, endKey: 0x31, keys: keys[2:4]}, - {startKey: 0x2e, endKey: 0x32, keys: keys[2:4]}, - {startKey: 0x2f, endKey: 0x32, keys: keys[2:4]}, - {startKey: 0x2e, endKey: 0x31, keys: keys[2:4]}, - {startKey: 0x2e, endKey: 0x2f, keys: keys[2:3]}, - {startKey: 0x12, endKey: 0x31, keys: keys[1:4]}, - {startKey: 0xf8, endKey: 0xff, keys: keys[9:10]}, - {startKey: 0x12, endKey: 0x20, keys: keys[1:3]}, - {startKey: 0x0, endKey: 0x09, keys: keys[0:1]}, + {start: 0x0a, end: 0xf7, pkeys: keys[0:T], vals: keys[0:9], lidx: 0}, // #0 + {start: 0x0a, end: 0xf8, pkeys: keys[0:T], vals: keys[0:T], lidx: 0}, // #1 + {start: 0x00, end: 0xff, pkeys: keys[0:T], vals: keys[0:T], lidx: 0}, // #2 + {start: 0x14, end: 0xe4, pkeys: keys[1:9], vals: keys[2:8], lidx: 1}, // #3 + {start: 0x14, end: 0xe5, pkeys: keys[1:9], vals: keys[2:9], lidx: 1}, // #4 + {start: 0x14, end: 0xe6, pkeys: keys[1:T], vals: keys[2:9], lidx: 1}, // #5 + {start: 0x14, end: 0xf1, pkeys: keys[1:T], vals: keys[2:9], lidx: 1}, // #6 + {start: 0x14, end: 0xf7, pkeys: keys[1:T], vals: keys[2:9], lidx: 1}, // #7 + {start: 0x14, end: 0xff, pkeys: keys[1:T], vals: keys[2:T], lidx: 1}, // #8 + {start: 0x2e, end: 0x31, pkeys: keys[2:4], vals: keys[2:3], lidx: 2}, // #9 + {start: 0x2e, end: 0x32, pkeys: keys[2:4], vals: keys[2:3], lidx: 2}, // #10 + {start: 0x2f, end: 0x32, pkeys: keys[2:4], vals: nil______, lidx: 2}, // #11 + {start: 0x2e, end: 0x31, pkeys: keys[2:4], vals: keys[2:3], lidx: 2}, // #12 + {start: 0x2e, end: 0x2f, pkeys: keys[2:3], vals: keys[2:3], lidx: 2}, // #13 + {start: 0x12, end: 0x31, pkeys: keys[1:4], vals: keys[2:3], lidx: 1}, // #14 + {start: 0xf8, end: 0xff, pkeys: keys[9:T], vals: nil______, lidx: 9}, // #15 + {start: 0x12, end: 0x20, pkeys: keys[1:3], vals: nil______, lidx: 1}, // #16 + {start: 0x00, end: 0x09, pkeys: keys[0:1], vals: nil______, lidx: 0}, // #17 + {start: 0xf7, end: 0x00, pnc: true}, // #18 + {start: 0xf8, end: 0x00, pnc: true}, // #19 + {start: 0x10, end: 0x10, pnc: true}, // #20 + {start: 0x12, end: 0x12, pnc: true}, // #21 + {start: 0xff, end: 0xf7, pnc: true}, // #22 } // fmt.Println("PRINT TREE") @@ -149,24 +161,36 @@ func TestTreeKeyInRangeProofs(t *testing.T) { for i, c := range cases { t.Logf("case %v", i) - startKey := []byte{c.startKey} - endKey := []byte{c.endKey} + start := []byte{c.start} + end := []byte{c.end} + + if c.pnc { + require.Panics(func() { tree.GetRangeWithProof(start, end, 0) }) + continue + } // Compute range proof. - keys, values, proof, err := tree.GetRangeWithProof(startKey, endKey, 0) + keys, values, proof, err := tree.GetRangeWithProof(start, end, 0) require.NoError(err, "%+v", err) - require.Equal(c.keys, flatten(keys)) - require.Equal(c.keys, flatten(values)) + require.Equal(c.pkeys, flatten(proof.Keys())) + require.Equal(c.vals, flatten(keys)) + require.Equal(c.vals, flatten(values)) + require.Equal(c.lidx, proof.LeftIndex()) // Verify that proof is valid. err = proof.Verify(root) - require.NoError(err, "%+v", err) verifyProof(t, proof, root) - // Verify each value. - for i, key := range c.keys { - err := proof.VerifyItem(i, []byte{key}, []byte{key}) + // Verify each value of pkeys. + for _, key := range c.pkeys { + err := proof.VerifyItem([]byte{key}, []byte{key}) + require.NoError(err) + } + + // Verify each value of vals. + for _, key := range c.vals { + err := proof.VerifyItem([]byte{key}, []byte{key}) require.NoError(err) } } @@ -203,10 +227,6 @@ func verifyProof(t *testing.T, proof *RangeProof, root []byte) { proofBytes, badProofBytes) } } - - // targetted changes fails... - proof.RootHash = test.MutateByteSlice(proof.RootHash) - assert.Error(t, proof.Verify(root)) } //---------------------------------------- diff --git a/tree_test.go b/tree_test.go index ee5b13bd9..d4d0ff0d3 100644 --- a/tree_test.go +++ b/tree_test.go @@ -987,7 +987,7 @@ func TestVersionedTreeProofs(t *testing.T) { require.NoError(err) require.EqualValues(val, []byte("v1")) require.NoError(proof.Verify(root1), proof.String()) - require.NoError(proof.VerifyItem(0, []byte("k2"), val)) + require.NoError(proof.VerifyItem([]byte("k2"), val)) val, proof, err = tree.GetVersionedWithProof([]byte("k4"), 1) require.NoError(err) @@ -999,13 +999,13 @@ func TestVersionedTreeProofs(t *testing.T) { require.NoError(err) require.EqualValues(val, []byte("v2")) require.NoError(proof.Verify(root2), proof.String()) - require.NoError(proof.VerifyItem(0, []byte("k2"), val)) + require.NoError(proof.VerifyItem([]byte("k2"), val)) val, proof, err = tree.GetVersionedWithProof([]byte("k1"), 2) require.NoError(err) require.EqualValues(val, []byte("v1")) require.NoError(proof.Verify(root2)) - require.NoError(proof.VerifyItem(0, []byte("k1"), val)) + require.NoError(proof.VerifyItem([]byte("k1"), val)) val, proof, err = tree.GetVersionedWithProof([]byte("k2"), 3) @@ -1036,7 +1036,7 @@ func TestVersionedTreeHash(t *testing.T) { require.NoError(err) require.EqualValues(val, []byte("F")) require.NoError(proof.Verify(hash2)) - require.NoError(proof.VerifyItem(0, []byte("I"), val)) + require.NoError(proof.VerifyItem([]byte("I"), val)) } func TestNilValueSemantics(t *testing.T) { diff --git a/version.go b/version.go index 737b17fda..4417a9da3 100644 --- a/version.go +++ b/version.go @@ -1,3 +1,4 @@ package iavl -const Version = "0.8.2" + +const Version = "0.9.1" From 9f49e86db58fe9986325f1fdd42d91c041fc512e Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 14:42:26 +0100 Subject: [PATCH 02/13] appease the linters -ineffassign, misspell, (some) golint --- Gopkg.lock | 3 ++- basic_test.go | 10 +++++----- node.go | 5 ++--- proof_range.go | 10 ++++++---- version.go | 1 - 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index a4b4f637e..9a4174900 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -102,6 +102,7 @@ [[projects]] name = "github.com/tendermint/tendermint" packages = [ + "crypto/tmhash", "libs/common", "libs/db", "libs/log", @@ -122,6 +123,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "ad12afa101079a7f5bcd70176fbbab42fd52e844b6c74c0fcf81315d33053f90" + inputs-digest = "06b259c645a1a884f870ef81ab220a7302b156f6fc1f1500779d540a7eed0d49" solver-name = "gps-cdcl" solver-version = 1 diff --git a/basic_test.go b/basic_test.go index 699177fde..5dca7cb20 100644 --- a/basic_test.go +++ b/basic_test.go @@ -12,7 +12,7 @@ import ( ) func TestBasic(t *testing.T) { - var tree *Tree = NewTree(nil, 0) + tree := NewTree(nil, 0) up := tree.Set([]byte("1"), []byte("one")) if up { t.Error("Did not expect an update (should have been create)") @@ -220,7 +220,7 @@ func TestIntegration(t *testing.T) { } records := make([]*record, 400) - var tree *Tree = NewTree(nil, 0) + tree := NewTree(nil, 0) randomRecord := func() *record { return &record{randstr(20), randstr(20)} @@ -302,7 +302,7 @@ func TestIterateRange(t *testing.T) { } sort.Strings(keys) - var tree *Tree = NewTree(nil, 0) + tree := NewTree(nil, 0) // insert all the data for _, r := range records { @@ -394,7 +394,7 @@ func TestProof(t *testing.T) { // Construct some random tree db := db.NewMemDB() - var tree *VersionedTree = NewVersionedTree(db, 100) + tree := NewVersionedTree(db, 100) for i := 0; i < 1000; i++ { key, value := randstr(20), randstr(20) tree.Set([]byte(key), []byte(value)) @@ -423,7 +423,7 @@ func TestProof(t *testing.T) { func TestTreeProof(t *testing.T) { db := db.NewMemDB() - var tree *Tree = NewTree(db, 100) + tree := NewTree(db, 100) // should get false for proof with nil root _, _, err := tree.GetWithProof([]byte("foo")) diff --git a/node.go b/node.go index fd86eff57..9565e8bad 100644 --- a/node.go +++ b/node.go @@ -45,11 +45,10 @@ func NewNode(key []byte, value []byte, version int64) *Node { // afterwards. func MakeNode(buf []byte) (node *Node, err cmn.Error) { node = &Node{} - n := 0 - cause := error(nil) + var n int + var cause error // Read node header. - node.height, n, cause = amino.DecodeInt8(buf) if cause != nil { return nil, cmn.ErrorWrap(cause, "decoding node.height") diff --git a/proof_range.go b/proof_range.go index ba9834084..2378067f9 100644 --- a/proof_range.go +++ b/proof_range.go @@ -150,7 +150,7 @@ func (proof *RangeProof) VerifyAbsence(key []byte) error { } else { if i == len(proof.Leaves)-1 { // If last item, check whether - // it's the last item in teh tree. + // it's the last item in the tree. } continue @@ -305,12 +305,13 @@ func (proof *RangeProof) _computeRootHash() (rootHash []byte, treeEnd bool, err /////////////////////////////////////////////////////////////////////////////// // keyStart is inclusive and keyEnd is exclusive. +// Returns the range-proof and the included keys and values. // If keyStart or keyEnd don't exist, the leaf before keyStart // or after keyEnd will also be included, but not be included in values. // If keyEnd-1 exists, no later leaves will be included. // If keyStart >= keyEnd and both not nil, panics. // Limit is never exceeded. -func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangeProof, keys, values [][]byte, err error) { +func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (*RangeProof, [][]byte, [][]byte, error) { if keyStart != nil && keyEnd != nil && bytes.Compare(keyStart, keyEnd) >= 0 { panic("if keyStart and keyEnd are present, need keyStart < keyEnd.") } @@ -327,17 +328,18 @@ func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangePr if err != nil { // Key doesn't exist, but instead we got the prev leaf (or the // first or last leaf), which provides proof of absence). - err = nil + // err = nil isn't necessary as we do not use it in the returns below } startOK := keyStart == nil || bytes.Compare(keyStart, left.key) <= 0 endOK := keyEnd == nil || bytes.Compare(left.key, keyEnd) < 0 // If left.key is in range, add it to key/values. + var keys, values [][]byte if startOK && endOK { keys = append(keys, left.key) // == keyStart values = append(values, left.value) } // Either way, add to proof leaves. - var leaves = []proofLeafNode{proofLeafNode{ + var leaves = []proofLeafNode{{ Key: left.key, ValueHash: tmhash.Sum(left.value), Version: left.version, diff --git a/version.go b/version.go index 4417a9da3..4c734cb35 100644 --- a/version.go +++ b/version.go @@ -1,4 +1,3 @@ package iavl - const Version = "0.9.1" From 74c0d02676272b5c67869a2ecc4369c9a70f2508 Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 15:01:43 +0100 Subject: [PATCH 03/13] unexport helper function & godoc compatibility - _PathToLeaf -> _pathToLeaf - Package iavl description - remove obsolete proof-types from doc --- doc.go | 8 ++++++-- proof.go | 12 ++++++++---- versioned_tree.go | 1 + 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/doc.go b/doc.go index e35179273..7e4891bcb 100644 --- a/doc.go +++ b/doc.go @@ -1,3 +1,7 @@ +// Package iavl implements a versioned, snapshottable (immutable) AVL+ tree +// for persisting key-value pairs. +// +// // Basic usage of VersionedTree. // // import "github.com/tendermint/iavl" @@ -23,12 +27,12 @@ // Proof of existence: // // root := tree.Hash() -// val, proof, err := tree.GetVersionedWithProof([]byte("bob"), 2) // "xyz", KeyProof, nil +// val, proof, err := tree.GetVersionedWithProof([]byte("bob"), 2) // "xyz", RangeProof, nil // proof.Verify([]byte("bob"), val, root) // nil // // Proof of absence: // -// _, proof, err = tree.GetVersionedWithProof([]byte("tom"), 2) // nil, KeyProof, nil +// _, proof, err = tree.GetVersionedWithProof([]byte("tom"), 2) // nil, RangeProof, nil // proof.Verify([]byte("tom"), nil, root) // nil // // Now we delete an old version: diff --git a/proof.go b/proof.go index 7a600ce1a..29eb6e5f4 100644 --- a/proof.go +++ b/proof.go @@ -144,10 +144,14 @@ func (pln proofLeafNode) Hash() []byte { // a path to the least item. func (node *Node) PathToLeaf(t *Tree, key []byte) (PathToLeaf, *Node, error) { path := new(PathToLeaf) - val, err := node._PathToLeaf(t, key, path) + val, err := node._pathToLeaf(t, key, path) return *path, val, err } -func (node *Node) _PathToLeaf(t *Tree, key []byte, path *PathToLeaf) (*Node, error) { + +// _pathToLeaf is a helper which recursively constructs the PathToLeaf. +// As an optimization the already constructed path is passed in as an argument +// and is shared among recursive calls. +func (node *Node) _pathToLeaf(t *Tree, key []byte, path *PathToLeaf) (*Node, error) { if node.height == 0 { if bytes.Equal(node.key, key) { return node, nil @@ -165,7 +169,7 @@ func (node *Node) _PathToLeaf(t *Tree, key []byte, path *PathToLeaf) (*Node, err Right: node.getRightNode(t).hash, } *path = append(*path, pin) - n, err := node.getLeftNode(t)._PathToLeaf(t, key, path) + n, err := node.getLeftNode(t)._pathToLeaf(t, key, path) return n, err } else { // right side @@ -177,7 +181,7 @@ func (node *Node) _PathToLeaf(t *Tree, key []byte, path *PathToLeaf) (*Node, err Right: nil, } *path = append(*path, pin) - n, err := node.getRightNode(t)._PathToLeaf(t, key, path) + n, err := node.getRightNode(t)._pathToLeaf(t, key, path) return n, err } } diff --git a/versioned_tree.go b/versioned_tree.go index 3e399d11e..7d8108ea8 100644 --- a/versioned_tree.go +++ b/versioned_tree.go @@ -8,6 +8,7 @@ import ( dbm "github.com/tendermint/tendermint/libs/db" ) +// ErrVersionDoesNotExist is returned if a requested version does not exist. var ErrVersionDoesNotExist = fmt.Errorf("version does not exist") // VersionedTree is a persistent tree which keeps track of versions. From 645c77cd503ca8cc6133b9f5f6ab0c3538e470cb Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 15:02:50 +0100 Subject: [PATCH 04/13] remove unused exported method --- util.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/util.go b/util.go index fefb558f8..7560515f5 100644 --- a/util.go +++ b/util.go @@ -6,11 +6,6 @@ import ( "sort" ) -func PrintTree(tree *Tree) { - ndb, root := tree.ndb, tree.root - printNode(ndb, root, 0) -} - func printNode(ndb *nodeDB, node *Node, indent int) { indentPrefix := "" for i := 0; i < indent; i++ { From 66ff5849d4bead55be0fa1c6d23e8f7948542035 Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 15:09:12 +0100 Subject: [PATCH 05/13] comment exported method and unexport debug helper method - comment PrintTree - StringIndented -> stringIndented --- proof.go | 8 ++++---- proof_path.go | 4 ++-- proof_range.go | 2 +- util.go | 6 ++++++ 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/proof.go b/proof.go index 29eb6e5f4..cd18bcd40 100644 --- a/proof.go +++ b/proof.go @@ -34,10 +34,10 @@ type proofInnerNode struct { } func (pin proofInnerNode) String() string { - return pin.StringIndented("") + return pin.stringIndented("") } -func (pin proofInnerNode) StringIndented(indent string) string { +func (pin proofInnerNode) stringIndented(indent string) string { return fmt.Sprintf(`proofInnerNode{ %s Height: %v %s Size: %v @@ -97,10 +97,10 @@ type proofLeafNode struct { } func (pln proofLeafNode) String() string { - return pln.StringIndented("") + return pln.stringIndented("") } -func (pln proofLeafNode) StringIndented(indent string) string { +func (pln proofLeafNode) stringIndented(indent string) string { return fmt.Sprintf(`proofLeafNode{ %s Key: %v %s ValueHash: %X diff --git a/proof_path.go b/proof_path.go index e865a3765..8a3356d87 100644 --- a/proof_path.go +++ b/proof_path.go @@ -24,7 +24,7 @@ func (pwl pathWithLeaf) StringIndented(indent string) string { %s Leaf: %v %s}`, indent, pwl.Path.StringIndented(indent+" "), - indent, pwl.Leaf.StringIndented(indent+" "), + indent, pwl.Leaf.stringIndented(indent+" "), indent) } @@ -64,7 +64,7 @@ func (pl PathToLeaf) StringIndented(indent string) string { strs[i] = fmt.Sprintf("... (%v total)", len(pl)) break } - strs[i] = fmt.Sprintf("%v:%v", i, pin.StringIndented(indent+" ")) + strs[i] = fmt.Sprintf("%v:%v", i, pin.stringIndented(indent+" ")) } return fmt.Sprintf(`PathToLeaf{ %s %v diff --git a/proof_range.go b/proof_range.go index 2378067f9..da463e227 100644 --- a/proof_range.go +++ b/proof_range.go @@ -56,7 +56,7 @@ func (proof *RangeProof) StringIndented(indent string) string { } lstrs := make([]string, 0, len(proof.Leaves)) for _, leaf := range proof.Leaves { - lstrs = append(lstrs, leaf.StringIndented(indent+" ")) + lstrs = append(lstrs, leaf.stringIndented(indent+" ")) } return fmt.Sprintf(`RangeProof{ %s LeftPath: %v diff --git a/util.go b/util.go index 7560515f5..96f754189 100644 --- a/util.go +++ b/util.go @@ -6,6 +6,12 @@ import ( "sort" ) +// PrintTree prints the whole tree in an indented form. +func PrintTree(tree *Tree) { + ndb, root := tree.ndb, tree.root + printNode(ndb, root, 0) +} + func printNode(ndb *nodeDB, node *Node, indent int) { indentPrefix := "" for i := 0; i < indent; i++ { From b4076e44aff154efc29eb7951ab7016ffc531f34 Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 15:09:50 +0100 Subject: [PATCH 06/13] remove unnused private methods --- testutils_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/testutils_test.go b/testutils_test.go index d39dbe23d..77f04d8a1 100644 --- a/testutils_test.go +++ b/testutils_test.go @@ -13,18 +13,6 @@ import ( "github.com/tendermint/tendermint/libs/db" ) -func dummyPathToLeaf(t *Tree, key []byte) PathToLeaf { - path, _, err := t.root.PathToLeaf(t, key) - if err != nil { - panic(err) - } - return path -} - -func dummyLeafNode(key, val []byte) proofLeafNode { - return proofLeafNode{key, val, 1} -} - func randstr(length int) string { return cmn.RandStr(length) } From 1af65a62c25239dd13e8af2070eb976aae7b32ca Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 15:19:18 +0100 Subject: [PATCH 07/13] remove obsolete else blocks (golint): - if block ends with a return statement, so drop this else and outdent its block (golint) --- node.go | 73 +++++++++++++++++++++++--------------------------- proof.go | 23 ++++++++-------- proof_path.go | 6 ++--- proof_range.go | 4 +-- proof_test.go | 2 +- 5 files changed, 50 insertions(+), 58 deletions(-) diff --git a/node.go b/node.go index 9565e8bad..787c674de 100644 --- a/node.go +++ b/node.go @@ -144,9 +144,8 @@ func (node *Node) has(t *Tree, key []byte) (has bool) { } if bytes.Compare(key, node.key) < 0 { return node.getLeftNode(t).has(t, key) - } else { - return node.getRightNode(t).has(t, key) } + return node.getRightNode(t).has(t, key) } // Get a key under the node. @@ -164,31 +163,28 @@ func (node *Node) get(t *Tree, key []byte) (index int64, value []byte) { if bytes.Compare(key, node.key) < 0 { return node.getLeftNode(t).get(t, key) - } else { - rightNode := node.getRightNode(t) - index, value = rightNode.get(t, key) - index += node.size - rightNode.size - return index, value } + rightNode := node.getRightNode(t) + index, value = rightNode.get(t, key) + index += node.size - rightNode.size + return index, value } func (node *Node) getByIndex(t *Tree, index int64) (key []byte, value []byte) { if node.isLeaf() { if index == 0 { return node.key, node.value - } else { - return nil, nil - } - } else { - // TODO: could improve this by storing the - // sizes as well as left/right hash. - leftNode := node.getLeftNode(t) - if index < leftNode.size { - return leftNode.getByIndex(t, index) - } else { - return node.getRightNode(t).getByIndex(t, index-leftNode.size) } + return nil, nil + } + // TODO: could improve this by storing the + // sizes as well as left/right hash. + leftNode := node.getLeftNode(t) + + if index < leftNode.size { + return leftNode.getByIndex(t, index) } + return node.getRightNode(t).getByIndex(t, index-leftNode.size) } // Computes the hash of the node without computing its descendants. Must be @@ -386,11 +382,10 @@ func (node *Node) set(t *Tree, key []byte, value []byte) ( if updated { return node, updated, orphaned - } else { - node.calcHeightAndSize(t) - newNode, balanceOrphaned := node.balance(t) - return newNode, updated, append(orphaned, balanceOrphaned...) } + node.calcHeightAndSize(t) + newNode, balanceOrphaned := node.balance(t) + return newNode, updated, append(orphaned, balanceOrphaned...) } } @@ -530,34 +525,32 @@ func (node *Node) balance(t *Tree) (newSelf *Node, orphaned []*Node) { // Left Left Case newNode, orphaned := node.rotateRight(t) return newNode, []*Node{orphaned} - } else { - // Left Right Case - var leftOrphaned *Node + } + // Left Right Case + var leftOrphaned *Node - left := node.getLeftNode(t) - node.leftHash = nil - node.leftNode, leftOrphaned = left.rotateLeft(t) - newNode, rightOrphaned := node.rotateRight(t) + left := node.getLeftNode(t) + node.leftHash = nil + node.leftNode, leftOrphaned = left.rotateLeft(t) + newNode, rightOrphaned := node.rotateRight(t) - return newNode, []*Node{left, leftOrphaned, rightOrphaned} - } + return newNode, []*Node{left, leftOrphaned, rightOrphaned} } if balance < -1 { if node.getRightNode(t).calcBalance(t) <= 0 { // Right Right Case newNode, orphaned := node.rotateLeft(t) return newNode, []*Node{orphaned} - } else { - // Right Left Case - var rightOrphaned *Node + } + // Right Left Case + var rightOrphaned *Node - right := node.getRightNode(t) - node.rightHash = nil - node.rightNode, rightOrphaned = right.rotateRight(t) - newNode, leftOrphaned := node.rotateLeft(t) + right := node.getRightNode(t) + node.rightHash = nil + node.rightNode, rightOrphaned = right.rotateRight(t) + newNode, leftOrphaned := node.rotateLeft(t) - return newNode, []*Node{right, leftOrphaned, rightOrphaned} - } + return newNode, []*Node{right, leftOrphaned, rightOrphaned} } // Nothing changed return node, []*Node{} diff --git a/proof.go b/proof.go index cd18bcd40..1e3aa2658 100644 --- a/proof.go +++ b/proof.go @@ -171,17 +171,16 @@ func (node *Node) _pathToLeaf(t *Tree, key []byte, path *PathToLeaf) (*Node, err *path = append(*path, pin) n, err := node.getLeftNode(t)._pathToLeaf(t, key, path) return n, err - } else { - // right side - pin := proofInnerNode{ - Height: node.height, - Size: node.size, - Version: node.version, - Left: node.getLeftNode(t).hash, - Right: nil, - } - *path = append(*path, pin) - n, err := node.getRightNode(t)._pathToLeaf(t, key, path) - return n, err } + // right side + pin := proofInnerNode{ + Height: node.height, + Size: node.size, + Version: node.version, + Left: node.getLeftNode(t).hash, + Right: nil, + } + *path = append(*path, pin) + n, err := node.getRightNode(t)._pathToLeaf(t, key, path) + return n, err } diff --git a/proof_path.go b/proof_path.go index 8a3356d87..de366f338 100644 --- a/proof_path.go +++ b/proof_path.go @@ -23,7 +23,7 @@ func (pwl pathWithLeaf) StringIndented(indent string) string { %s Path: %v %s Leaf: %v %s}`, - indent, pwl.Path.StringIndented(indent+" "), + indent, pwl.Path.stringIndented(indent+" "), indent, pwl.Leaf.stringIndented(indent+" "), indent) } @@ -51,10 +51,10 @@ func (pwl pathWithLeaf) computeRootHash() []byte { type PathToLeaf []proofInnerNode func (pl PathToLeaf) String() string { - return pl.StringIndented("") + return pl.stringIndented("") } -func (pl PathToLeaf) StringIndented(indent string) string { +func (pl PathToLeaf) stringIndented(indent string) string { if len(pl) == 0 { return "empty-PathToLeaf" } diff --git a/proof_range.go b/proof_range.go index da463e227..6ee99c466 100644 --- a/proof_range.go +++ b/proof_range.go @@ -52,7 +52,7 @@ func (proof *RangeProof) String() string { func (proof *RangeProof) StringIndented(indent string) string { istrs := make([]string, 0, len(proof.InnerNodes)) for _, ptl := range proof.InnerNodes { - istrs = append(istrs, ptl.StringIndented(indent+" ")) + istrs = append(istrs, ptl.stringIndented(indent+" ")) } lstrs := make([]string, 0, len(proof.Leaves)) for _, leaf := range proof.Leaves { @@ -68,7 +68,7 @@ func (proof *RangeProof) StringIndented(indent string) string { %s (rootHash): %X %s (treeEnd): %v %s}`, - indent, proof.LeftPath.StringIndented(indent+" "), + indent, proof.LeftPath.stringIndented(indent+" "), indent, indent, strings.Join(istrs, "\n"+indent+" "), indent, diff --git a/proof_test.go b/proof_test.go index 4410ac6aa..248b497a3 100644 --- a/proof_test.go +++ b/proof_test.go @@ -222,7 +222,7 @@ func verifyProof(t *testing.T, proof *RangeProof, root []byte) { } // may be invalid... errors are okay if err == nil { - assert.Error(t, badProof.Verify(root), + assert.Errorf(t, badProof.Verify(root), "Proof was still valid after a random mutation:\n%X\n%X", proofBytes, badProofBytes) } From c5510883ba4aac764429857c177175811cbec316 Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 15:28:54 +0100 Subject: [PATCH 08/13] remove more obsolete else blocks (golint) and more lints - if block ends with a return statement, so drop this else and outdent its block (golint) - remove unused named returns - more idiomatic go code ( += 1 -> ++ etc) --- proof_range.go | 11 +++++------ testutils_test.go | 5 ++--- tree.go | 9 +++------ 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/proof_range.go b/proof_range.go index 6ee99c466..d86930642 100644 --- a/proof_range.go +++ b/proof_range.go @@ -179,7 +179,7 @@ func (proof *RangeProof) Verify(root []byte) error { return err } -func (proof *RangeProof) verify(root []byte) (err error) { +func (proof *RangeProof) verify(root []byte) (error) { rootHash := proof.rootHash if rootHash == nil { derivedHash, err := proof.computeRootHash() @@ -190,9 +190,8 @@ func (proof *RangeProof) verify(root []byte) (err error) { } if !bytes.Equal(rootHash, root) { return cmn.ErrorWrap(ErrInvalidRoot, "root hash doesn't match") - } else { - proof.rootVerified = true } + proof.rootVerified = true return nil } @@ -389,9 +388,9 @@ func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (*RangeProof, [ pn.Right != nil && !bytes.Equal(pn.Right, node.rightHash) { // We've diverged, so start appending to inners. - pathCount = -1 + pathCount-- } else { - pathCount += 1 + pathCount++ } } } @@ -407,7 +406,7 @@ func (t *Tree) getRangeProof(keyStart, keyEnd []byte, limit int) (*RangeProof, [ ValueHash: tmhash.Sum(node.value), Version: node.version, }) - leafCount += 1 + leafCount++ // Maybe terminate because we found enough leaves. if limit > 0 && limit <= leafCount { return true diff --git a/testutils_test.go b/testutils_test.go index 77f04d8a1..1e727fb52 100644 --- a/testutils_test.go +++ b/testutils_test.go @@ -66,9 +66,8 @@ func T(n *Node) *Tree { func P(n *Node) string { if n.height == 0 { return fmt.Sprintf("%v", b2i(n.key)) - } else { - return fmt.Sprintf("(%v %v)", P(n.leftNode), P(n.rightNode)) } + return fmt.Sprintf("(%v %v)", P(n.leftNode), P(n.rightNode)) } func randBytes(length int) []byte { @@ -89,7 +88,7 @@ func (t *traverser) view(key, value []byte) bool { t.first = string(key) } t.last = string(key) - t.count += 1 + t.count++ return false } diff --git a/tree.go b/tree.go index 676618b43..9353e24d4 100644 --- a/tree.go +++ b/tree.go @@ -175,9 +175,8 @@ func (t *Tree) Iterate(fn func(key []byte, value []byte) bool) (stopped bool) { return t.root.traverse(t, true, func(node *Node) bool { if node.height == 0 { return fn(node.key, node.value) - } else { - return false } + return false }) } @@ -190,9 +189,8 @@ func (t *Tree) IterateRange(start, end []byte, ascending bool, fn func(key []byt return t.root.traverseInRange(t, start, end, ascending, false, 0, func(node *Node, _ uint8) bool { if node.height == 0 { return fn(node.key, node.value) - } else { - return false } + return false }) } @@ -205,9 +203,8 @@ func (t *Tree) IterateRangeInclusive(start, end []byte, ascending bool, fn func( return t.root.traverseInRange(t, start, end, ascending, true, 0, func(node *Node, _ uint8) bool { if node.height == 0 { return fn(node.key, node.value, node.version) - } else { - return false } + return false }) } From e1d4252f8575bea5d3b20bee7461947810135fb5 Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 15:51:30 +0100 Subject: [PATCH 09/13] update comment and remove non-trivial obsolete else block (more lints) - if block ends with a return statement, so drop this else and outdent its block (golint) - remove unused named returns - updated/improved comment --- node.go | 61 +++++++++++++++++++++++---------------------------- proof_test.go | 2 ++ 2 files changed, 30 insertions(+), 33 deletions(-) diff --git a/node.go b/node.go index 787c674de..1b04e8c2e 100644 --- a/node.go +++ b/node.go @@ -389,27 +389,26 @@ func (node *Node) set(t *Tree, key []byte, value []byte) ( } } -// newHash/newNode: The new hash or node to replace node after remove. -// newKey: new leftmost leaf key for tree after successfully removing 'key' if changed. -// value: removed value. -func (node *Node) remove(t *Tree, key []byte) ( - newHash []byte, newNode *Node, newKey []byte, value []byte, orphaned []*Node, -) { +// removes the node corresponding to the passed key and balances the tree. +// It returns: +// - the hash of the new node (or nil if the node is the one removed) +// - the node that replaces the orig. node after remove +// - new leftmost leaf key for tree after successfully removing 'key' if changed. +// - the removed value +// - the orphaned nodes. +func (node *Node) remove(t *Tree, key []byte) ([]byte, *Node, []byte, []byte, []*Node) { version := t.version + 1 if node.isLeaf() { if bytes.Equal(key, node.key) { return nil, nil, nil, node.value, []*Node{node} } - return node.hash, node, nil, nil, orphaned + return node.hash, node, nil, nil, nil } + // node.key < key; we go to the left to find the key: if bytes.Compare(key, node.key) < 0 { - var newLeftHash []byte - var newLeftNode *Node - - newLeftHash, newLeftNode, newKey, value, orphaned = - node.getLeftNode(t).remove(t, key) + newLeftHash, newLeftNode, newKey, value, orphaned := node.getLeftNode(t).remove(t, key) if len(orphaned) == 0 { return node.hash, node, nil, value, orphaned @@ -424,30 +423,26 @@ func (node *Node) remove(t *Tree, key []byte) ( newNode, balanceOrphaned := newNode.balance(t) return newNode.hash, newNode, newKey, value, append(orphaned, balanceOrphaned...) - } else { - var newRightHash []byte - var newRightNode *Node - - newRightHash, newRightNode, newKey, value, orphaned = - node.getRightNode(t).remove(t, key) - - if len(orphaned) == 0 { - return node.hash, node, nil, value, orphaned - } else if newRightHash == nil && newRightNode == nil { // right node held value, was removed - return node.leftHash, node.leftNode, nil, value, orphaned - } - orphaned = append(orphaned, node) + } + // node.key >= key; either found or look to the right: + newRightHash, newRightNode, newKey, value, orphaned := node.getRightNode(t).remove(t, key) - newNode := node.clone(version) - newNode.rightHash, newNode.rightNode = newRightHash, newRightNode - if newKey != nil { - newNode.key = newKey - } - newNode.calcHeightAndSize(t) - newNode, balanceOrphaned := newNode.balance(t) + if len(orphaned) == 0 { + return node.hash, node, nil, value, orphaned + } else if newRightHash == nil && newRightNode == nil { // right node held value, was removed + return node.leftHash, node.leftNode, nil, value, orphaned + } + orphaned = append(orphaned, node) - return newNode.hash, newNode, nil, value, append(orphaned, balanceOrphaned...) + newNode := node.clone(version) + newNode.rightHash, newNode.rightNode = newRightHash, newRightNode + if newKey != nil { + newNode.key = newKey } + newNode.calcHeightAndSize(t) + newNode, balanceOrphaned := newNode.balance(t) + + return newNode.hash, newNode, nil, value, append(orphaned, balanceOrphaned...) } func (node *Node) getLeftNode(t *Tree) *Node { diff --git a/proof_test.go b/proof_test.go index 248b497a3..53cc5c977 100644 --- a/proof_test.go +++ b/proof_test.go @@ -120,6 +120,8 @@ func TestTreeKeyInRangeProofs(t *testing.T) { // For spacing: T := 10 + // disable: don't use underscores in Go names; var nil______ should be nil (golint) + // nolint nil______ := []byte(nil) cases := []struct { From 50a5c529929eab00a8af19187754a460ca3f62d0 Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 15:54:29 +0100 Subject: [PATCH 10/13] remove last obsolete else block (more lints) --- proof_range.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/proof_range.go b/proof_range.go index d86930642..c9901f975 100644 --- a/proof_range.go +++ b/proof_range.go @@ -126,9 +126,8 @@ func (proof *RangeProof) VerifyAbsence(key []byte) error { if cmp < 0 { if proof.LeftPath.isLeftmost() { return nil - } else { - return cmn.NewError("absence not proved by left path") } + return cmn.NewError("absence not proved by left path") } else if cmp == 0 { return cmn.NewError("absence disproved via first item #0") } @@ -165,9 +164,8 @@ func (proof *RangeProof) VerifyAbsence(key []byte) error { // It's not a valid absence proof. if len(proof.Leaves) < 2 { return cmn.NewError("absence not proved by right leaf (need another leaf?)") - } else { - return cmn.NewError("absence not proved by right leaf") } + return cmn.NewError("absence not proved by right leaf") } // Verify that proof is valid. @@ -459,12 +457,10 @@ func (t *Tree) GetWithProof(key []byte) (value []byte, proof *RangeProof, err er if len(values) > 0 { if !bytes.Equal(proof.Leaves[0].Key, key) { return nil, proof, nil - } else { - return values[0], proof, nil } - } else { - return nil, proof, nil + return values[0], proof, nil } + return nil, proof, nil } return nil, nil, cmn.ErrorWrap(err, "could not construct any proof") } From 95444241911aa6255ed80bffb9269c2e9c9f6564 Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 15:59:32 +0100 Subject: [PATCH 11/13] more idiomatic golang code: - consistent receiver names - omit type from declaration --- tree.go | 8 ++++---- tree_dotgraph_test.go | 2 +- tree_fuzz_test.go | 4 ++-- version.go | 1 + 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/tree.go b/tree.go index 9353e24d4..c92901532 100644 --- a/tree.go +++ b/tree.go @@ -210,11 +210,11 @@ func (t *Tree) IterateRangeInclusive(start, end []byte, ascending bool, fn func( // Clone creates a clone of the tree. // Used internally by VersionedTree. -func (tree *Tree) clone() *Tree { +func (t *Tree) clone() *Tree { return &Tree{ - root: tree.root, - ndb: tree.ndb, - version: tree.version, + root: t.root, + ndb: t.ndb, + version: t.version, } } diff --git a/tree_dotgraph_test.go b/tree_dotgraph_test.go index a2c80a3e9..02b245c5d 100644 --- a/tree_dotgraph_test.go +++ b/tree_dotgraph_test.go @@ -6,7 +6,7 @@ import ( ) func TestWriteDOTGraph(t *testing.T) { - var tree *Tree = NewTree(nil, 0) + tree := NewTree(nil, 0) for _, ikey := range []byte{ 0x0a, 0x11, 0x2e, 0x32, 0x50, 0x72, 0x99, 0xa1, 0xe4, 0xf7, } { diff --git a/tree_fuzz_test.go b/tree_fuzz_test.go index 5825f64d1..59d693153 100644 --- a/tree_fuzz_test.go +++ b/tree_fuzz_test.go @@ -45,8 +45,8 @@ func (p *program) addInstruction(i instruction) { p.instructions = append(p.instructions, i) } -func (prog *program) size() int { - return len(prog.instructions) +func (p *program) size() int { + return len(p.instructions) } type instruction struct { diff --git a/version.go b/version.go index 4c734cb35..526fcc68a 100644 --- a/version.go +++ b/version.go @@ -1,3 +1,4 @@ package iavl +// Version of iavl. const Version = "0.9.1" From 84d8eac03da88272ee7e197776f4c6ce0c49dc7b Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 16:28:10 +0100 Subject: [PATCH 12/13] remove unused named returns --- node.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/node.go b/node.go index 1b04e8c2e..307412c33 100644 --- a/node.go +++ b/node.go @@ -41,45 +41,50 @@ func NewNode(key []byte, value []byte, version int64) *Node { // MakeNode constructs an *Node from an encoded byte slice. // -// The new node doesn't have its hash saved or set. The caller must set it +// The new node doesn't have its hash saved or set. The caller must set it // afterwards. -func MakeNode(buf []byte) (node *Node, err cmn.Error) { - node = &Node{} - var n int - var cause error +func MakeNode(buf []byte) (*Node, cmn.Error) { - // Read node header. - node.height, n, cause = amino.DecodeInt8(buf) + // Read node header (height, size, version, key). + height, n, cause := amino.DecodeInt8(buf) if cause != nil { return nil, cmn.ErrorWrap(cause, "decoding node.height") } buf = buf[n:] - node.size, n, cause = amino.DecodeVarint(buf) + size, n, cause := amino.DecodeVarint(buf) if cause != nil { return nil, cmn.ErrorWrap(cause, "decoding node.size") } buf = buf[n:] - node.version, n, cause = amino.DecodeVarint(buf) + ver, n, cause := amino.DecodeVarint(buf) if cause != nil { return nil, cmn.ErrorWrap(cause, "decoding node.version") } buf = buf[n:] - node.key, n, cause = amino.DecodeByteSlice(buf) + key, n, cause := amino.DecodeByteSlice(buf) if cause != nil { return nil, cmn.ErrorWrap(cause, "decoding node.key") } buf = buf[n:] + node := &Node{ + height: height, + size: size, + version: ver, + key: key, + } + // Read node body. if node.isLeaf() { - node.value, _, cause = amino.DecodeByteSlice(buf) + val, _, cause := amino.DecodeByteSlice(buf) if cause != nil { return nil, cmn.ErrorWrap(cause, "decoding node.value") } + node.value = val } else { // Read children. leftHash, n, cause := amino.DecodeByteSlice(buf) if cause != nil { From e0c8fa12d5c4991676d19c89fab242ce6df4b184 Mon Sep 17 00:00:00 2001 From: Liamsi Date: Tue, 3 Jul 2018 17:09:31 +0100 Subject: [PATCH 13/13] review comments (thanks @melekes) - remove brackets from single return val - remove underscore from method name Signed-off-by: Liamsi --- proof.go | 10 +++++----- proof_range.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/proof.go b/proof.go index 1e3aa2658..a87877048 100644 --- a/proof.go +++ b/proof.go @@ -144,14 +144,14 @@ func (pln proofLeafNode) Hash() []byte { // a path to the least item. func (node *Node) PathToLeaf(t *Tree, key []byte) (PathToLeaf, *Node, error) { path := new(PathToLeaf) - val, err := node._pathToLeaf(t, key, path) + val, err := node.pathToLeaf(t, key, path) return *path, val, err } -// _pathToLeaf is a helper which recursively constructs the PathToLeaf. +// pathToLeaf is a helper which recursively constructs the PathToLeaf. // As an optimization the already constructed path is passed in as an argument // and is shared among recursive calls. -func (node *Node) _pathToLeaf(t *Tree, key []byte, path *PathToLeaf) (*Node, error) { +func (node *Node) pathToLeaf(t *Tree, key []byte, path *PathToLeaf) (*Node, error) { if node.height == 0 { if bytes.Equal(node.key, key) { return node, nil @@ -169,7 +169,7 @@ func (node *Node) _pathToLeaf(t *Tree, key []byte, path *PathToLeaf) (*Node, err Right: node.getRightNode(t).hash, } *path = append(*path, pin) - n, err := node.getLeftNode(t)._pathToLeaf(t, key, path) + n, err := node.getLeftNode(t).pathToLeaf(t, key, path) return n, err } // right side @@ -181,6 +181,6 @@ func (node *Node) _pathToLeaf(t *Tree, key []byte, path *PathToLeaf) (*Node, err Right: nil, } *path = append(*path, pin) - n, err := node.getRightNode(t)._pathToLeaf(t, key, path) + n, err := node.getRightNode(t).pathToLeaf(t, key, path) return n, err } diff --git a/proof_range.go b/proof_range.go index c9901f975..cc12618f9 100644 --- a/proof_range.go +++ b/proof_range.go @@ -177,7 +177,7 @@ func (proof *RangeProof) Verify(root []byte) error { return err } -func (proof *RangeProof) verify(root []byte) (error) { +func (proof *RangeProof) verify(root []byte) error { rootHash := proof.rootHash if rootHash == nil { derivedHash, err := proof.computeRootHash()