Fix recording of deleted Merkle values

- Record Merkle values deleted only if they were not changed since the last trie snapshot
- Record Merkle values deleted when one or more nodes are deleted from the trie
- Do NOT record Merkle values of inlined nodes in the deleted set of Merkle values (since they are not stored to database)
- Lazy calculate Merkle values if they are missing (especially for delete operations)
- Update tests to have full trie.go coverage again
- Change `PopulateMerkleValues` to take a control boolean `excludeInlined`
- Fix preparing branch for mutation only once
- Fix merkle value calculation for future-to-be root node

dot/state/offline_pruner.go

@@ -121,7 +121,8 @@ func (p *OfflinePruner) SetBloomFilter() (err error) {
 			return err
 		}
 
-		trie.PopulateMerkleValues(tr.RootNode(), merkleValues)
+		const excludeInlined = false
+		trie.PopulateMerkleValues(tr.RootNode(), merkleValues, excludeInlined)
 
 		// get parent header of current block
 		header, err = p.blockState.GetHeader(header.ParentHash)

dot/state/test_helpers.go

@@ -244,7 +244,8 @@ func generateBlockWithRandomTrie(t *testing.T, serv *Service,
 	rand := time.Now().UnixNano()
 	key := []byte("testKey" + fmt.Sprint(rand))
 	value := []byte("testValue" + fmt.Sprint(rand))
-	trieState.Set(key, value)
+	err = trieState.Set(key, value)
+	require.NoError(t, err)
 
 	trieStateRoot, err := trieState.Root()
 	require.NoError(t, err)

lib/runtime/interface.go

@@ -50,22 +50,24 @@ type Instance interface {
 
 // Storage interface
 type Storage interface {
-	Set(key []byte, value []byte)
+	Set(key []byte, value []byte) (err error)
 	Get(key []byte) []byte
 	Root() (common.Hash, error)
 	SetChild(keyToChild []byte, child *trie.Trie) error
 	SetChildStorage(keyToChild, key, value []byte) error
 	GetChildStorage(keyToChild, key []byte) ([]byte, error)
-	Delete(key []byte)
-	DeleteChild(keyToChild []byte)
-	DeleteChildLimit(keyToChild []byte, limit *[]byte) (uint32, bool, error)
+	Delete(key []byte) (err error)
+	DeleteChild(keyToChild []byte) (err error)
+	DeleteChildLimit(keyToChild []byte, limit *[]byte) (
+		deleted uint32, allDeleted bool, err error)
 	ClearChildStorage(keyToChild, key []byte) error
 	NextKey([]byte) []byte
 	ClearPrefixInChild(keyToChild, prefix []byte) error
 	GetChildNextKey(keyToChild, key []byte) ([]byte, error)
 	GetChild(keyToChild []byte) (*trie.Trie, error)
-	ClearPrefix(prefix []byte)
-	ClearPrefixLimit(prefix []byte, limit uint32) (uint32, bool)
+	ClearPrefix(prefix []byte) (err error)
+	ClearPrefixLimit(prefix []byte, limit uint32) (
+		deleted uint32, allDeleted bool, err error)
 	BeginStorageTransaction()
 	CommitStorageTransaction()
 	RollbackStorageTransaction()

lib/runtime/storage/trie.go

@@ -5,6 +5,7 @@ package storage
 
 import (
 	"encoding/binary"
+	"fmt"
 	"sort"
 	"sync"
 
@@ -68,10 +69,10 @@ func (s *TrieState) RollbackStorageTransaction() {
 }
 
 // Set sets a key-value pair in the trie
-func (s *TrieState) Set(key, value []byte) {
+func (s *TrieState) Set(key, value []byte) (err error) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
-	s.t.Put(key, value)
+	return s.t.Put(key, value)
 }
 
 // Get gets a value from the trie
@@ -97,15 +98,20 @@ func (s *TrieState) Has(key []byte) bool {
 }
 
 // Delete deletes a key from the trie
-func (s *TrieState) Delete(key []byte) {
+func (s *TrieState) Delete(key []byte) (err error) {
 	val := s.t.Get(key)
 	if val == nil {
-		return
+		return nil
 	}
 
 	s.lock.Lock()
 	defer s.lock.Unlock()
-	s.t.Delete(key)
+	err = s.t.Delete(key)
+	if err != nil {
+		return err
+	}
+
+	return nil
 }
 
 // NextKey returns the next key in the trie in lexicographical order. If it does not exist, it returns nil.
@@ -116,19 +122,19 @@ func (s *TrieState) NextKey(key []byte) []byte {
 }
 
 // ClearPrefix deletes all key-value pairs from the trie where the key starts with the given prefix
-func (s *TrieState) ClearPrefix(prefix []byte) {
+func (s *TrieState) ClearPrefix(prefix []byte) (err error) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
-	s.t.ClearPrefix(prefix)
+	return s.t.ClearPrefix(prefix)
 }
 
 // ClearPrefixLimit deletes key-value pairs from the trie where the key starts with the given prefix till limit reached
-func (s *TrieState) ClearPrefixLimit(prefix []byte, limit uint32) (uint32, bool) {
+func (s *TrieState) ClearPrefixLimit(prefix []byte, limit uint32) (
+	deleted uint32, allDeleted bool, err error) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
-	num, del := s.t.ClearPrefixLimit(prefix, limit)
-	return num, del
+	return s.t.ClearPrefixLimit(prefix, limit)
 }
 
 // TrieEntries returns every key-value pair in the trie
@@ -167,24 +173,29 @@ func (s *TrieState) GetChildStorage(keyToChild, key []byte) ([]byte, error) {
 }
 
 // DeleteChild deletes a child trie from the main trie
-func (s *TrieState) DeleteChild(key []byte) {
+func (s *TrieState) DeleteChild(key []byte) (err error) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
-	s.t.DeleteChild(key)
+	return s.t.DeleteChild(key)
 }
 
-// DeleteChildLimit deletes up to limit of database entries by lexicographic order, return number
-//  deleted, true if all delete otherwise false
-func (s *TrieState) DeleteChildLimit(key []byte, limit *[]byte) (uint32, bool, error) {
+// DeleteChildLimit deletes up to limit of database entries by lexicographic order.
+func (s *TrieState) DeleteChildLimit(key []byte, limit *[]byte) (
+	deleted uint32, allDeleted bool, err error) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 	tr, err := s.t.GetChild(key)
 	if err != nil {
 		return 0, false, err
 	}
+
 	qtyEntries := uint32(len(tr.Entries()))
 	if limit == nil {
-		s.t.DeleteChild(key)
+		err = s.t.DeleteChild(key)
+		if err != nil {
+			return 0, false, err
+		}
+
 		return qtyEntries, true, nil
 	}
 	limitUint := binary.LittleEndian.Uint32(*limit)
@@ -194,20 +205,24 @@ func (s *TrieState) DeleteChildLimit(key []byte, limit *[]byte) (uint32, bool, e
 		keys = append(keys, k)
 	}
 	sort.Strings(keys)
-	deleted := uint32(0)
 	for _, k := range keys {
-		tr.Delete([]byte(k))
+		// TODO have a transactional/atomic way to delete multiple keys in trie.
+		// If one deletion fails, the child trie and its parent trie are then in
+		// a bad intermediary state. Take also care of the caching of deleted Merkle
+		// values within the tries, which is used for online pruning.
+		err = tr.Delete([]byte(k))
+		if err != nil {
+			return deleted, allDeleted, fmt.Errorf("deleting from child trie at key 0x%x: %w", key, err)
+		}
+
 		deleted++
 		if deleted == limitUint {
 			break
 		}
 	}
 
-	if deleted == qtyEntries {
-		return deleted, true, nil
-	}
-
-	return deleted, false, nil
+	allDeleted = deleted == qtyEntries
+	return deleted, allDeleted, nil
 }
 
 // ClearChildStorage removes the child storage entry from the trie
@@ -230,7 +245,11 @@ func (s *TrieState) ClearPrefixInChild(keyToChild, prefix []byte) error {
 		return nil
 	}
 
-	child.ClearPrefix(prefix)
+	err = child.ClearPrefix(prefix)
+	if err != nil {
+		return fmt.Errorf("clearing prefix in child trie at key 0x%x: %w", keyToChild, err)
+	}
+
 	return nil
 }
 

lib/runtime/wasmer/helpers.go

@@ -159,7 +159,7 @@ func toWasmMemoryFixedSizeOptional(context wasmer.InstanceContext, data []byte)
 	return toWasmMemory(context, encodedOptionalFixedSize)
 }
 
-func storageAppend(storage runtime.Storage, key, valueToAppend []byte) error {
+func storageAppend(storage runtime.Storage, key, valueToAppend []byte) (err error) {
 	// this function assumes the item in storage is a SCALE encoded array of items
 	// the valueToAppend is a new item, so it appends the item and increases the length prefix by 1
 	currentValue := storage.Get(key)
@@ -210,7 +210,16 @@ func storageAppend(storage runtime.Storage, key, valueToAppend []byte) error {
 		}
 	}
 
-	logger.Debugf("resulting value: 0x%x", value)
-	storage.Set(key, value)
+	err = storage.Set(key, value)
+	if err != nil {
+		return fmt.Errorf("settings key and value in storage: %w", err)
+	}
+
 	return nil
 }
+
+func panicOnError(err error) {
+	if err != nil {
+		panic(err)
+	}
+}

lib/runtime/wasmer/helpers_test.go

@@ -5,6 +5,7 @@ package wasmer
 
 import (
 	"encoding/json"
+	"errors"
 	"os"
 	"path/filepath"
 	"testing"
@@ -66,3 +67,13 @@ func Test_pointerSize(t *testing.T) {
 		})
 	}
 }
+
+func Test_panicOnError(t *testing.T) {
+	t.Parallel()
+
+	err := (error)(nil)
+	assert.NotPanics(t, func() { panicOnError(err) })
+
+	err = errors.New("test error")
+	assert.PanicsWithValue(t, err, func() { panicOnError(err) })
+}

lib/runtime/wasmer/imports.go

@@ -823,7 +823,12 @@ func ext_trie_blake2_256_root_version_1(context unsafe.Pointer, dataSpan C.int64
 	}
 
 	for _, kv := range kvs {
-		t.Put(kv.Key, kv.Value)
+		err := t.Put(kv.Key, kv.Value)
+		if err != nil {
+			logger.Errorf("failed putting key 0x%x and value 0x%x into trie: %s",
+				kv.Key, kv.Value, err)
+			return 0
+		}
 	}
 
 	// allocate memory for value and copy value to memory
@@ -871,7 +876,12 @@ func ext_trie_blake2_256_ordered_root_version_1(context unsafe.Pointer, dataSpan
 			"put key=0x%x and value=0x%x",
 			key, value)
 
-		t.Put(key, value)
+		err = t.Put(key, value)
+		if err != nil {
+			logger.Errorf("failed putting key 0x%x and value 0x%x into trie: %s",
+				key, value, err)
+			return 0
+		}
 	}
 
 	// allocate memory for value and copy value to memory
@@ -1187,7 +1197,8 @@ func ext_default_child_storage_storage_kill_version_1(context unsafe.Pointer, ch
 	storage := ctx.Storage
 
 	childStorageKey := asMemorySlice(instanceContext, childStorageKeySpan)
-	storage.DeleteChild(childStorageKey)
+	err := storage.DeleteChild(childStorageKey)
+	panicOnError(err)
 }
 
 //export ext_default_child_storage_storage_kill_version_2
@@ -1850,7 +1861,8 @@ func ext_storage_clear_version_1(context unsafe.Pointer, keySpan C.int64_t) {
 	key := asMemorySlice(instanceContext, keySpan)
 
 	logger.Debugf("key: 0x%x", key)
-	storage.Delete(key)
+	err := storage.Delete(key)
+	panicOnError(err)
 }
 
 //export ext_storage_clear_prefix_version_1
@@ -1863,7 +1875,8 @@ func ext_storage_clear_prefix_version_1(context unsafe.Pointer, prefixSpan C.int
 	prefix := asMemorySlice(instanceContext, prefixSpan)
 	logger.Debugf("prefix: 0x%x", prefix)
 
-	storage.ClearPrefix(prefix)
+	err := storage.ClearPrefix(prefix)
+	panicOnError(err)
 }
 
 //export ext_storage_clear_prefix_version_2
@@ -1893,7 +1906,13 @@ func ext_storage_clear_prefix_version_2(context unsafe.Pointer, prefixSpan, lim
 	}
 
 	limitUint := binary.LittleEndian.Uint32(limit)
-	numRemoved, all := storage.ClearPrefixLimit(prefix, limitUint)
+	numRemoved, all, err := storage.ClearPrefixLimit(prefix, limitUint)
+	if err != nil {
+		logger.Errorf("failed to clear prefix limit: %s", err)
+		ret, _ := toWasmMemory(instanceContext, nil)
+		return C.int64_t(ret)
+	}
+
 	encBytes, err := toKillStorageResultEnum(all, numRemoved)
 	if err != nil {
 		logger.Errorf("failed to allocate memory: %s", err)
@@ -2056,7 +2075,8 @@ func ext_storage_set_version_1(context unsafe.Pointer, keySpan, valueSpan C.int6
 	logger.Debugf(
 		"key 0x%x has value 0x%x",
 		key, value)
-	storage.Set(key, cp)
+	err := storage.Set(key, cp)
+	panicOnError(err)
 }
 
 //export ext_storage_start_transaction_version_1

lib/trie/child_storage.go

@@ -28,7 +28,11 @@ func (t *Trie) PutChild(keyToChild []byte, child *Trie) error {
 	copy(key, ChildStorageKeyPrefix)
 	copy(key[len(ChildStorageKeyPrefix):], keyToChild)
 
-	t.Put(key, childHash.ToBytes())
+	err = t.Put(key, childHash.ToBytes())
+	if err != nil {
+		return fmt.Errorf("putting child trie root hash %s in main trie: %w", childHash, err)
+	}
+
 	t.childTries[childHash] = child
 	return nil
 }
@@ -59,7 +63,11 @@ func (t *Trie) PutIntoChild(keyToChild, key, value []byte) error {
 		return err
 	}
 
-	child.Put(key, value)
+	err = child.Put(key, value)
+	if err != nil {
+		return fmt.Errorf("putting into child trie at key 0x%x: %w", keyToChild, err)
+	}
+
 	childHash, err := child.Hash()
 	if err != nil {
 		return err
@@ -88,12 +96,16 @@ func (t *Trie) GetFromChild(keyToChild, key []byte) ([]byte, error) {
 }
 
 // DeleteChild deletes the child storage trie
-func (t *Trie) DeleteChild(keyToChild []byte) {
+func (t *Trie) DeleteChild(keyToChild []byte) (err error) {
 	key := make([]byte, len(ChildStorageKeyPrefix)+len(keyToChild))
 	copy(key, ChildStorageKeyPrefix)
 	copy(key[len(ChildStorageKeyPrefix):], keyToChild)
 
-	t.Delete(key)
+	err = t.Delete(key)
+	if err != nil {
+		return fmt.Errorf("deleting child trie at key 0x%x: %w", key, err)
+	}
+	return nil
 }
 
 // ClearFromChild removes the child storage entry
@@ -105,6 +117,11 @@ func (t *Trie) ClearFromChild(keyToChild, key []byte) error {
 	if child == nil {
 		return fmt.Errorf("%w at key 0x%x%x", ErrChildTrieDoesNotExist, ChildStorageKeyPrefix, keyToChild)
 	}
-	child.Delete(key)
+
+	err = child.Delete(key)
+	if err != nil {
+		return fmt.Errorf("deleting from child trie at key 0x%x: %w", keyToChild, err)
+	}
+
 	return nil
 }