From 3531ca2246db3474b5d1e61d18adc7e98725175b Mon Sep 17 00:00:00 2001
From: Martin Holst Swende <martin@swende.se>
Date: Tue, 28 Sep 2021 13:06:19 +0200
Subject: [PATCH] eth/tracers: avoid unsyncronized mutations on trie database
 (#23632)

This PR fixes an issue in traceChain, where the statedb Commit operation was performed asynchronously with dereference-operations agains the underlying trie.Database instance. Due to how the reference counting works within the trie database (where parent count is recursively updated when new parents are added), doing dereferencing in the middle of Commit can cause the refcount to become wrong, leading to an inconsistent state.

This was fixed by doing Commit/Deref from the same routine.
---
 eth/state_accessor.go |  3 ++-
 eth/tracers/api.go    | 23 +++++++++++++++++------
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/eth/state_accessor.go b/eth/state_accessor.go
index eb178311f3..ca2002b60d 100644
--- a/eth/state_accessor.go
+++ b/eth/state_accessor.go
@@ -119,7 +119,8 @@ func (eth *Ethereum) stateAtBlock(block *types.Block, reexec uint64, base *state
 		// Finalize the state so any modifications are written to the trie
 		root, err := statedb.Commit(eth.blockchain.Config().IsEIP158(current.Number()))
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("stateAtBlock commit failed, number %d root %v: %w",
+				current.NumberU64(), current.Root().Hex(), err)
 		}
 		statedb, err = state.New(root, database, nil)
 		if err != nil {
diff --git a/eth/tracers/api.go b/eth/tracers/api.go
index 39b95264d7..5019fb6f73 100644
--- a/eth/tracers/api.go
+++ b/eth/tracers/api.go
@@ -290,7 +290,11 @@ func (api *API) traceChain(ctx context.Context, start, end *types.Block, config
 		}()
 	}
 	// Start a goroutine to feed all the blocks into the tracers
-	begin := time.Now()
+	var (
+		begin     = time.Now()
+		derefTodo []common.Hash // list of hashes to dereference from the db
+		derefsMu  sync.Mutex    // mutex for the derefs
+	)
 
 	go func() {
 		var (
@@ -324,6 +328,14 @@ func (api *API) traceChain(ctx context.Context, start, end *types.Block, config
 				return
 			default:
 			}
+			// clean out any derefs
+			derefsMu.Lock()
+			for _, h := range derefTodo {
+				statedb.Database().TrieDB().Dereference(h)
+			}
+			derefTodo = derefTodo[:0]
+			derefsMu.Unlock()
+
 			// Print progress logs if long enough time elapsed
 			if time.Since(logged) > 8*time.Second {
 				logged = time.Now()
@@ -382,12 +394,11 @@ func (api *API) traceChain(ctx context.Context, start, end *types.Block, config
 				Hash:   res.block.Hash(),
 				Traces: res.results,
 			}
+			// Schedule any parent tries held in memory by this task for dereferencing
 			done[uint64(result.Block)] = result
-
-			// Dereference any parent tries held in memory by this task
-			if res.statedb.Database().TrieDB() != nil {
-				res.statedb.Database().TrieDB().Dereference(res.rootref)
-			}
+			derefsMu.Lock()
+			derefTodo = append(derefTodo, res.rootref)
+			derefsMu.Unlock()
 			// Stream completed traces to the user, aborting on the first error
 			for result, ok := done[next]; ok; result, ok = done[next] {
 				if len(result.Traces) > 0 || next == end.NumberU64() {