Fix concurrent map access error when tracing blocks (#1822)

Fixes  #1799

This commit ensures that state.StateDB instances are not
accessed concurrently when tracing blocks and also adds
an e2e test to verify the fix.

It also disables logging from the e2e tests, because the logs
slowed the tests down and cluttered the output of test runs.

It also updates test node instances to ensure that all rpc api
modules are enabled.

e2e_test/e2e_test.go

@@ -5,12 +5,15 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
+	"sync"
 	"testing"
 	"time"
 
 	"github.com/celo-org/celo-blockchain/common/hexutil"
 	"github.com/celo-org/celo-blockchain/core/types"
+	"github.com/celo-org/celo-blockchain/log"
 	"github.com/celo-org/celo-blockchain/node"
+	"github.com/celo-org/celo-blockchain/rpc"
 	"github.com/celo-org/celo-blockchain/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -20,7 +23,10 @@ func init() {
 	// This statement is commented out but left here since its very useful for
 	// debugging problems and its non trivial to construct.
 	//
-	// log.Root().SetHandler(log.LvlFilterHandler(log.LvlDebug, log.StreamHandler(os.Stdout, log.TerminalFormat(true))))
+	// log.Root().SetHandler(log.LvlFilterHandler(log.LvlTrace, log.StreamHandler(os.Stdout, log.TerminalFormat(true))))
+
+	// This disables all logging which in general we want, because there is a lot
+	log.Root().SetHandler(log.DiscardHandler())
 }
 
 // This test starts a network submits a transaction and waits for the whole
@@ -192,6 +198,59 @@ func TestStartStopValidators(t *testing.T) {
 
 }
 
+// This test was created to reproduce the concurrent map access error in
+// https://github.com/celo-org/celo-blockchain/issues/1799
+//
+// It does this by calling debug_traceBlockByNumber a number of times since the
+// trace block code was the source of the concurrent map access.
+func TestBlockTracingConcurrentMapAccess(t *testing.T) {
+	ac := test.AccountConfig(1, 2)
+	gc, ec, err := test.BuildConfig(ac)
+	require.NoError(t, err)
+	network, shutdown, err := test.NewNetwork(ac, gc, ec)
+	require.NoError(t, err)
+	defer shutdown()
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*60)
+	defer cancel()
+
+	n := network[0]
+
+	accounts := test.Accounts(ac.DeveloperAccounts(), gc.ChainConfig())
+
+	var txs []*types.Transaction
+	// Send one celo from external account 0 to 1 via node 0.
+	for i := 0; i < 10; i++ {
+		tx, err := accounts[0].SendCelo(ctx, accounts[1].Address, 1, n)
+		require.NoError(t, err)
+		txs = append(txs, tx)
+	}
+
+	// Wait for the whole network to process the transactions.
+	err = network.AwaitTransactions(ctx, txs...)
+	require.NoError(t, err)
+
+	lastTx := txs[len(txs)-1]
+
+	b := n.Tracker.GetProcessedBlockForTx(lastTx.Hash())
+
+	var wg sync.WaitGroup
+	for i := 1; i < +int(b.NumberU64()); i++ {
+		wg.Add(1)
+		num := i
+		go func() {
+			defer wg.Done()
+			c, err := rpc.DialContext(ctx, n.WSEndpoint())
+			require.NoError(t, err)
+
+			var result []interface{}
+			err = c.CallContext(ctx, &result, "debug_traceBlockByNumber", hexutil.EncodeUint64(uint64(num)))
+			require.NoError(t, err)
+		}()
+
+	}
+	wg.Wait()
+}
+
 type rpcCustomTransaction struct {
 	BlockNumber *hexutil.Big `json:"blockNumber"`
 	GasPrice    *hexutil.Big `json:"gasPrice"`

eth/tracers/api.go

@@ -529,19 +529,18 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac
 	}
 	blockCtx := core.NewEVMBlockContext(block.Header(), api.chainContext(ctx), nil)
 	blockHash := block.Hash()
-	var sysCtx *core.SysContractCallCtx
-	if api.backend.ChainConfig().IsEspresso(block.Number()) {
-		sysVmRunner := api.backend.VmRunnerAtHeader(block.Header(), statedb)
-		sysCtx = core.NewSysContractCallCtx(sysVmRunner)
-	}
 	for th := 0; th < threads; th++ {
 		pend.Add(1)
 		go func() {
 			defer pend.Done()
 			// Fetch and execute the next transaction trace tasks
 			for task := range jobs {
+				var sysCtx *core.SysContractCallCtx
+				vmRunner := api.backend.VmRunnerAtHeader(block.Header(), task.statedb)
+				if api.backend.ChainConfig().IsEspresso(block.Number()) {
+					sysCtx = core.NewSysContractCallCtx(vmRunner)
+				}
 				msg, _ := txs[task.index].AsMessage(signer, nil)
-				vmRunner := api.backend.VmRunnerAtHeader(block.Header(), statedb)
 				txctx := &Context{
 					BlockHash: blockHash,
 					TxIndex:   task.index,
@@ -556,6 +555,11 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac
 			}
 		}()
 	}
+	var sysCtx *core.SysContractCallCtx
+	vmRunner := api.backend.VmRunnerAtHeader(block.Header(), statedb)
+	if api.backend.ChainConfig().IsEspresso(block.Number()) {
+		sysCtx = core.NewSysContractCallCtx(vmRunner)
+	}
 	// Feed the transactions into the tracers and return
 	var failed error
 	for i, tx := range txs {
@@ -566,7 +570,6 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac
 		msg, _ := tx.AsMessage(signer, nil)
 		statedb.Prepare(tx.Hash(), i)
 		vmenv := vm.NewEVM(blockCtx, core.NewEVMTxContext(msg), statedb, api.backend.ChainConfig(), vm.Config{})
-		vmRunner := api.backend.VmRunnerAtHeader(block.Header(), statedb)
 		if _, err := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.Gas()), vmRunner, sysCtx); err != nil {
 			failed = err
 			break

test/node.go

@@ -24,6 +24,7 @@ import (
 	"github.com/celo-org/celo-blockchain/crypto"
 	"github.com/celo-org/celo-blockchain/eth"
 	"github.com/celo-org/celo-blockchain/eth/downloader"
+	"github.com/celo-org/celo-blockchain/eth/tracers"
 	"github.com/celo-org/celo-blockchain/ethclient"
 	"github.com/celo-org/celo-blockchain/mycelo/env"
 	"github.com/celo-org/celo-blockchain/mycelo/genesis"
@@ -35,6 +36,7 @@ import (
 )
 
 var (
+	allModules                  = []string{"admin", "debug", "web3", "eth", "txpool", "personal", "istanbul", "miner", "net"}
 	baseNodeConfig *node.Config = &node.Config{
 		Name:    "celo",
 		Version: params.Version,
@@ -52,6 +54,8 @@ var (
 		HTTPHost:             "0.0.0.0",
 		WSHost:               "0.0.0.0",
 		UsePlaintextKeystore: true,
+		WSModules:            allModules,
+		HTTPModules:          allModules,
 	}
 
 	baseEthConfig = &eth.Config{
@@ -179,6 +183,9 @@ func (n *Node) Start() error {
 	if err != nil {
 		return err
 	}
+	// This manual step is required to enable tracing, it's messy but this is the
+	// approach taken by geth in cmd/utils.RegisterEthService.
+	n.Node.RegisterAPIs(tracers.APIs(n.Eth.APIBackend))
 
 	err = n.Node.Start()
 	if err != nil {