trie: use explicit errors in stacktrie (instead of panic) (#28361)

This PR removes panics from stacktrie (mostly), and makes the Update return errors instead. While adding tests for this, I also found that one case of possible corruption was not caught, which is now fixed.
This commit is contained in:
Martin Holst Swende 2023-10-25 14:53:50 +02:00 committed by GitHub
parent 300df874d7
commit 96b75033c0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 31 additions and 4 deletions

View File

@ -230,7 +230,9 @@ func (dl *diskLayer) proveRange(ctx *generatorContext, trieId *trie.ID, prefix [
if origin == nil && !diskMore { if origin == nil && !diskMore {
stackTr := trie.NewStackTrie(nil) stackTr := trie.NewStackTrie(nil)
for i, key := range keys { for i, key := range keys {
stackTr.Update(key, vals[i]) if err := stackTr.Update(key, vals[i]); err != nil {
return nil, err
}
} }
if gotRoot := stackTr.Hash(); gotRoot != root { if gotRoot := stackTr.Hash(); gotRoot != root {
return &proofResult{ return &proofResult{

View File

@ -18,6 +18,7 @@ package trie
import ( import (
"bytes" "bytes"
"errors"
"sync" "sync"
"github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common"
@ -92,12 +93,14 @@ func NewStackTrie(options *StackTrieOptions) *StackTrie {
// Update inserts a (key, value) pair into the stack trie. // Update inserts a (key, value) pair into the stack trie.
func (t *StackTrie) Update(key, value []byte) error { func (t *StackTrie) Update(key, value []byte) error {
k := keybytesToHex(key)
if len(value) == 0 { if len(value) == 0 {
panic("deletion not supported") return errors.New("trying to insert empty (deletion)")
} }
k := keybytesToHex(key)
k = k[:len(k)-1] // chop the termination flag k = k[:len(k)-1] // chop the termination flag
if bytes.Compare(t.last, k) >= 0 {
return errors.New("non-ascending key order")
}
// track the first and last inserted entries. // track the first and last inserted entries.
if t.first == nil { if t.first == nil {
t.first = append([]byte{}, k...) t.first = append([]byte{}, k...)

View File

@ -26,6 +26,7 @@ import (
"github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/rawdb"
"github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/trie/testutil" "github.com/ethereum/go-ethereum/trie/testutil"
"github.com/stretchr/testify/assert"
"golang.org/x/exp/slices" "golang.org/x/exp/slices"
) )
@ -463,3 +464,24 @@ func TestPartialStackTrie(t *testing.T) {
} }
} }
} }
func TestStackTrieErrors(t *testing.T) {
s := NewStackTrie(nil)
// Deletion
if err := s.Update(nil, nil); err == nil {
t.Fatal("expected error")
}
if err := s.Update(nil, []byte{}); err == nil {
t.Fatal("expected error")
}
if err := s.Update([]byte{0xa}, []byte{}); err == nil {
t.Fatal("expected error")
}
// Non-ascending keys (going backwards or repeating)
assert.Nil(t, s.Update([]byte{0xaa}, []byte{0xa}))
assert.NotNil(t, s.Update([]byte{0xaa}, []byte{0xa}), "repeat insert same key")
assert.NotNil(t, s.Update([]byte{0xaa}, []byte{0xb}), "repeat insert same key")
assert.Nil(t, s.Update([]byte{0xab}, []byte{0xa}))
assert.NotNil(t, s.Update([]byte{0x10}, []byte{0xb}), "out of order insert")
assert.NotNil(t, s.Update([]byte{0xaa}, []byte{0xb}), "repeat insert same key")
}