Skip to content

Commit

Permalink
trie: use explicit errors in stacktrie (instead of panic) (#28361)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
holiman authored Oct 25, 2023
1 parent 300df87 commit 96b7503
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 4 deletions.
4 changes: 3 additions & 1 deletion core/state/snapshot/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ func (dl *diskLayer) proveRange(ctx *generatorContext, trieId *trie.ID, prefix [
if origin == nil && !diskMore {
stackTr := trie.NewStackTrie(nil)
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 {
return &proofResult{
Expand Down
9 changes: 6 additions & 3 deletions trie/stacktrie.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package trie

import (
"bytes"
"errors"
"sync"

"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -92,12 +93,14 @@ func NewStackTrie(options *StackTrieOptions) *StackTrie {

// Update inserts a (key, value) pair into the stack trie.
func (t *StackTrie) Update(key, value []byte) error {
k := keybytesToHex(key)
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

if bytes.Compare(t.last, k) >= 0 {
return errors.New("non-ascending key order")
}
// track the first and last inserted entries.
if t.first == nil {
t.first = append([]byte{}, k...)
Expand Down
22 changes: 22 additions & 0 deletions trie/stacktrie_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/ethereum/go-ethereum/core/rawdb"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/trie/testutil"
"github.com/stretchr/testify/assert"
"golang.org/x/exp/slices"
)

Expand Down Expand Up @@ -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")
}

0 comments on commit 96b7503

Please sign in to comment.