diff --git a/trie/stackproof.go b/trie/stackproof.go index c84dd59631..c5b4eace9e 100644 --- a/trie/stackproof.go +++ b/trie/stackproof.go @@ -87,6 +87,7 @@ func newStackTrieFromProof(rootHash common.Hash, key []byte, proofDb ethdb.KeyVa if parent, err = resolveFromProof(proofDb, rootHash); err != nil { return nil, err } + var lastResolved common.Hash stParent = nodeToStNode(parent, key) stack.root = stParent // Now we pursue the given key downwards, and populate the stacktrie too @@ -94,9 +95,40 @@ func newStackTrieFromProof(rootHash common.Hash, key []byte, proofDb ethdb.KeyVa keyrest, child = get(parent, key, false) switch cld := child.(type) { case nil: - return nil, errors.New("no node at given path") + /* + If the parent is a shortnode, it means that the 'next' key is not + going in here (because then the parent would be a fullnode). We cannot + leave the parent shortnode dangling without value: either we + revert it back to hashed form, or we leave it as an empty node. + + There are a few cases to consider, this shortnode does prove the inexistence + of the 'origin', it does so by proving that the extension does not + point to 'origin'. However: it may point to + 1. An existing leaf to the left of the 'origin' + 2. An existing leaft to the right of the 'origin'. + + In the former case, we do not want it here, it does not belong. We need + to replace it with it's hash. + + In the latter case, we cannot replace it with a hash, because as soon + as we start feeding the leafs, the first one will hit the "trying to insert into hash" case. + So instead we must make it an emptyNode. + */ + + if sn, ok := parent.(*shortNode); ok { + if bytes.Compare(sn.Key, key) < 0 { + // This is on the lower side of the proof-border. We must replace + // this with a hash, not an empty node + stParent.typ = hashedNode + stParent.val = lastResolved[:] + } else { + stParent.typ = emptyNode + } + } + return stack, nil case hashNode: - child, err = resolveFromProof(proofDb, common.BytesToHash(cld)) + lastResolved = common.BytesToHash(cld) + child, err = resolveFromProof(proofDb, lastResolved) if err != nil { return nil, err } @@ -217,9 +249,15 @@ func iterateProof(rootHash common.Hash, path []byte, ascending bool, proof ethdb } if hn, ok := n.(hashNode); ok { n, _ = resolveFromProof(proof, common.Hash(hn)) - if n == nil { - return nil, fmt.Errorf("proof node (hash %064x) missing", rootHash) - } + } + if n == nil { + // n is nil! + // At this point, we are following a path of nonexistence. + // nothing more to iterate here + // However, while iterating towards a non-present end leaf, + // we may have encountered an actual leaf here. Must we + // restore the hash for that one? + break } } return paths, nil @@ -312,15 +350,30 @@ func VerifyRangeProofWithStack(rootHash common.Hash, firstKey []byte, keys [][]b if err != nil { return false, fmt.Errorf("could not initate stacktrie: %v", err) } - // Feed in the values, starting from 1 (do not re-add the proof-leaf) - for i := 1; i < len(keys); i++ { - if bytes.Compare(keys[i-1], keys[i]) >= 0 { + // Feed in the values + if bytes.Compare(firstKey, keys[0]) > 0 { + return false, errors.New("range is not monotonically increasing") + } + var pKey []byte + for i := 0; i < len(keys); i++ { + if bytes.Compare(pKey, keys[i]) >= 0 { return false, errors.New("range is not monotonically increasing") } + pKey = keys[i] if len(values[i]) == 0 { return false, errors.New("range contains deletion") } - stTrie.Update(keys[i], values[i]) + if err := stTrie.Update(keys[i], values[i]); err != nil { + return false, err + } + } + if !bytes.Equal(lastKey, keys[len(keys)-1]) { + // The method we have of inserting right-hand hashes only works + // if the proof indeed concerns the last element: otherwise it forces + // restructurings on the trie which result in insertion-into-hash. + // However, for snap-sync, we do not expect "right side proof-of-inexistence": + // the rhs proof should prove the last element that was sent along. + return false, fmt.Errorf("proofs must prove the last item") } // For the right-hand-side, we need a list of hashes ot inject hps, err := iterateProof(rootHash, lastKey, false, proof) @@ -330,7 +383,10 @@ func VerifyRangeProofWithStack(rootHash common.Hash, firstKey []byte, keys [][]b slices.Reverse(hps) // Insert into stacktrie for _, hp := range hps { - stTrie.insert(stTrie.root, hp.path, hp.hash[:], nil, newHashed) + //fmt.Printf("Inserting at %x: hash %x \n", hp.path, hp.hash) + if err := stTrie.insert(stTrie.root, hp.path, hp.hash[:], nil, newHashed); err != nil { + return false, err + } } if have := stTrie.Hash(); have != rootHash { return false, fmt.Errorf("invalid proof, want hash %x, got %x", rootHash, have) diff --git a/trie/stackproof_test.go b/trie/stackproof_test.go index 8b7d4fd084..f66ef5bdf6 100644 --- a/trie/stackproof_test.go +++ b/trie/stackproof_test.go @@ -170,7 +170,7 @@ func testStackRangeProof( var leftBorder = keybytesToHex(entries[0].k) var rightBorder = keybytesToHex(entries[len(entries)-1].k) - writeFn := func(_ common.Hash, path []byte, hash common.Hash, blob []byte) { + writeFn := func(path []byte, hash common.Hash, blob []byte) { if bytes.HasPrefix(leftBorder, path) { fmt.Printf("path %x tainted left (parent to %x)\n", path, leftBorder) return diff --git a/trie/stacktrie.go b/trie/stacktrie.go index b689a631d5..09e6a9a72e 100644 --- a/trie/stacktrie.go +++ b/trie/stacktrie.go @@ -17,9 +17,11 @@ package trie import ( + "bytes" "errors" "sync" + "fmt" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" @@ -59,8 +61,7 @@ func (t *StackTrie) Update(key, value []byte) error { if len(value) == 0 { panic("deletion not supported") } - t.insert(t.root, k[:len(k)-1], value, nil, newLeaf) - return nil + return t.insert(t.root, k[:len(k)-1], value, nil, newLeaf) } // MustUpdate is a wrapper of Update and will omit any encountered error but @@ -148,7 +149,7 @@ func (n *stNode) getDiffIndex(key []byte) int { // Helper function to that inserts a (key, value) pair into // the trie. -func (t *StackTrie) insert(st *stNode, key, value []byte, prefix []byte, leafCtor func([]byte, []byte) *stNode) { +func (t *StackTrie) insert(st *stNode, key, value []byte, prefix []byte, leafCtor func([]byte, []byte) *stNode) error { switch st.typ { case branchNode: /* Branch */ idx := int(key[0]) @@ -167,9 +168,8 @@ func (t *StackTrie) insert(st *stNode, key, value []byte, prefix []byte, leafCto if st.children[idx] == nil { st.children[idx] = leafCtor(key[1:], value) } else { - t.insert(st.children[idx], key[1:], value, append(prefix, key[0]), leafCtor) + return t.insert(st.children[idx], key[1:], value, append(prefix, key[0]), leafCtor) } - case extNode: /* Ext */ // Compare both key chunks and see where they differ diffidx := st.getDiffIndex(key) @@ -182,8 +182,7 @@ func (t *StackTrie) insert(st *stNode, key, value []byte, prefix []byte, leafCto if diffidx == len(st.key) { // Ext key and key segment are identical, recurse into // the child node. - t.insert(st.children[0], key[diffidx:], value, append(prefix, key[:diffidx]...), leafCtor) - return + return t.insert(st.children[0], key[diffidx:], value, append(prefix, key[:diffidx]...), leafCtor) } // Save the original part. Depending if the break is // at the extension's last byte or not, create an @@ -240,8 +239,16 @@ func (t *StackTrie) insert(st *stNode, key, value []byte, prefix []byte, leafCto // keys, 2) a fullnode selecting the path on which the // keys differ, and 3) one leaf for the differentiated // component of each key. + if diffidx >= len(st.key) { - panic("Trying to insert into existing key") + if diffidx == len(st.key) && bytes.Equal(st.val, value) { + // We can ignore this: it's not an overwrite, just reinsert. + // + // Sometimes a proof is initialized with a value, the + // same value is then fed in to the trie. + return nil + } + return errors.New("trying to insert into existing key") } // Check if the split occurs at the first nibble of the @@ -283,11 +290,12 @@ func (t *StackTrie) insert(st *stNode, key, value []byte, prefix []byte, leafCto st.val = value case hashedNode: - panic("trying to insert into hash") + return fmt.Errorf("trying to insert into hash %x path %x %x", st.val, key, value) default: panic("invalid type") } + return nil } // hash converts st into a 'hashedNode', if possible. Possible outcomes: