trie: fix delete bug for values contained in fullNode

Delete crashed if a fullNode contained a valueNode directly. This bug is
very unlikely to occur with SecureTrie, but can happen with regular
tries. This commit also introduces a randomised test which triggers all
trie operations, which should prevent such bugs in the future.

Credit for finding this bug goes to Github user @rjl493456442.
This commit is contained in:
Felix Lange 2016-09-29 16:51:32 +02:00
parent ba8c4c6b1a
commit c3a77d6268
2 changed files with 127 additions and 35 deletions

View File

@ -377,6 +377,9 @@ func (t *Trie) delete(n node, prefix, key []byte) (bool, node, error) {
// n still contains at least two values and cannot be reduced. // n still contains at least two values and cannot be reduced.
return true, n, nil return true, n, nil
case valueNode:
return true, nil, nil
case nil: case nil:
return false, nil, nil return false, nil, nil

View File

@ -21,8 +21,11 @@ import (
"encoding/binary" "encoding/binary"
"fmt" "fmt"
"io/ioutil" "io/ioutil"
"math/rand"
"os" "os"
"reflect"
"testing" "testing"
"testing/quick"
"github.com/davecgh/go-spew/spew" "github.com/davecgh/go-spew/spew"
"github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common"
@ -297,41 +300,6 @@ func TestReplication(t *testing.T) {
} }
} }
func paranoiaCheck(t1 *Trie) (bool, *Trie) {
t2 := new(Trie)
it := NewIterator(t1)
for it.Next() {
t2.Update(it.Key, it.Value)
}
return t2.Hash() == t1.Hash(), t2
}
func TestParanoia(t *testing.T) {
t.Skip()
trie := newEmpty()
vals := []struct{ k, v string }{
{"do", "verb"},
{"ether", "wookiedoo"},
{"horse", "stallion"},
{"shaman", "horse"},
{"doge", "coin"},
{"ether", ""},
{"dog", "puppy"},
{"shaman", ""},
{"somethingveryoddindeedthis is", "myothernodedata"},
}
for _, val := range vals {
updateString(trie, val.k, val.v)
}
trie.Commit()
ok, t2 := paranoiaCheck(trie)
if !ok {
t.Errorf("trie paranoia check failed %x %x", trie.Hash(), t2.Hash())
}
}
// Not an actual test // Not an actual test
func TestOutput(t *testing.T) { func TestOutput(t *testing.T) {
t.Skip() t.Skip()
@ -356,7 +324,128 @@ func TestLargeValue(t *testing.T) {
trie.Update([]byte("key1"), []byte{99, 99, 99, 99}) trie.Update([]byte("key1"), []byte{99, 99, 99, 99})
trie.Update([]byte("key2"), bytes.Repeat([]byte{1}, 32)) trie.Update([]byte("key2"), bytes.Repeat([]byte{1}, 32))
trie.Hash() trie.Hash()
}
type randTestStep struct {
op int
key []byte // for opUpdate, opDelete, opGet
value []byte // for opUpdate
}
type randTest []randTestStep
const (
opUpdate = iota
opDelete
opGet
opCommit
opHash
opReset
opItercheckhash
opMax // boundary value, not an actual op
)
func (randTest) Generate(r *rand.Rand, size int) reflect.Value {
var allKeys [][]byte
genKey := func() []byte {
if len(allKeys) < 2 || r.Intn(100) < 10 {
// new key
key := make([]byte, r.Intn(50))
randRead(r, key)
allKeys = append(allKeys, key)
return key
}
// use existing key
return allKeys[r.Intn(len(allKeys))]
}
var steps randTest
for i := 0; i < size; i++ {
step := randTestStep{op: r.Intn(opMax)}
switch step.op {
case opUpdate:
step.key = genKey()
step.value = make([]byte, 8)
binary.BigEndian.PutUint64(step.value, uint64(i))
case opGet, opDelete:
step.key = genKey()
}
steps = append(steps, step)
}
return reflect.ValueOf(steps)
}
// rand.Rand provides a Read method in Go 1.7 and later, but
// we can't use it yet.
func randRead(r *rand.Rand, b []byte) {
pos := 0
val := 0
for n := 0; n < len(b); n++ {
if pos == 0 {
val = r.Int()
pos = 7
}
b[n] = byte(val)
val >>= 8
pos--
}
}
func runRandTest(rt randTest) bool {
db, _ := ethdb.NewMemDatabase()
tr, _ := New(common.Hash{}, db)
values := make(map[string]string) // tracks content of the trie
for _, step := range rt {
switch step.op {
case opUpdate:
tr.Update(step.key, step.value)
values[string(step.key)] = string(step.value)
case opDelete:
tr.Delete(step.key)
delete(values, string(step.key))
case opGet:
v := tr.Get(step.key)
want := values[string(step.key)]
if string(v) != want {
fmt.Printf("mismatch for key 0x%x, got 0x%x want 0x%x", step.key, v, want)
return false
}
case opCommit:
if _, err := tr.Commit(); err != nil {
panic(err)
}
case opHash:
tr.Hash()
case opReset:
hash, err := tr.Commit()
if err != nil {
panic(err)
}
newtr, err := New(hash, db)
if err != nil {
panic(err)
}
tr = newtr
case opItercheckhash:
checktr, _ := New(common.Hash{}, nil)
it := tr.Iterator()
for it.Next() {
checktr.Update(it.Key, it.Value)
}
if tr.Hash() != checktr.Hash() {
fmt.Println("hashes not equal")
return false
}
}
}
return true
}
func TestRandom(t *testing.T) {
if err := quick.Check(runRandTest, nil); err != nil {
t.Fatal(err)
}
} }
func BenchmarkGet(b *testing.B) { benchGet(b, false) } func BenchmarkGet(b *testing.B) { benchGet(b, false) }