Problem: concurrent write base fee in fee history (backport: #364) (#366

)

* Problem: concurrent write base fee in fee history (#364)

* Problem: fee history with baseFeePerGas is not tested

* only use NextBaseFee as last item to avoid concurrent write

* concurrent unit test

* update pystarport

* reproduce with param change

* fix lint

CHANGELOG.md

@@ -59,6 +59,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (rpc) [#1638](https://github.com/evmos/ethermint/pull/1638) Align results when querying `eth_getTransactionCount` for future blocks for accounts with zero and non-zero transaction counts.
 * (rpc) [#1688](https://github.com/evmos/ethermint/pull/1688) Align filter rule for `debug_traceBlockByNumber`
 * (rpc) [#1639](https://github.com/evmos/ethermint/pull/1639) Align block number input behaviour for `eth_getProof` as Ethereum.
+* (rpc) [#364](https://github.com/crypto-org-chain/ethermint/pull/364) Only use NextBaseFee as last item to avoid concurrent write in `eth_feeHistory`.
 
 ### Improvements
 

rpc/backend/backend.go

@@ -17,6 +17,7 @@ import (
 	"github.com/ethereum/go-ethereum/rpc"
 	"github.com/ethereum/go-ethereum/signer/core/apitypes"
 	"github.com/evmos/ethermint/crypto/hd"
+	"github.com/evmos/ethermint/rpc/types"
 	rpctypes "github.com/evmos/ethermint/rpc/types"
 	"github.com/evmos/ethermint/server/config"
 	ethermint "github.com/evmos/ethermint/types"
@@ -132,6 +133,14 @@ var _ BackendI = (*Backend)(nil)
 
 var bAttributeKeyEthereumBloom = []byte(evmtypes.AttributeKeyEthereumBloom)
 
+type ProcessBlocker func(
+	tendermintBlock *tmrpctypes.ResultBlock,
+	ethBlock *map[string]interface{},
+	rewardPercentiles []float64,
+	tendermintBlockResult *tmrpctypes.ResultBlockResults,
+	targetOneFeeHistory *types.OneFeeHistory,
+) error
+
 // Backend implements the BackendI interface
 type Backend struct {
 	ctx                 context.Context
@@ -142,6 +151,7 @@ type Backend struct {
 	cfg                 config.Config
 	allowUnprotectedTxs bool
 	indexer             ethermint.EVMTxIndexer
+	processBlocker      ProcessBlocker
 }
 
 // NewBackend creates a new Backend instance for cosmos and ethereum namespaces
@@ -179,7 +189,7 @@ func NewBackend(
 		clientCtx = clientCtx.WithKeyring(kr)
 	}
 
-	return &Backend{
+	b := &Backend{
 		ctx:                 context.Background(),
 		clientCtx:           clientCtx,
 		queryClient:         rpctypes.NewQueryClient(clientCtx),
@@ -189,4 +199,6 @@ func NewBackend(
 		allowUnprotectedTxs: allowUnprotectedTxs,
 		indexer:             indexer,
 	}
+	b.processBlocker = b.processBlock
+	return b
 }

rpc/backend/chain_info.go

@@ -231,15 +231,18 @@ func (b *Backend) FeeHistory(
 				}
 
 				oneFeeHistory := rpctypes.OneFeeHistory{}
-				err = b.processBlock(tendermintblock, &ethBlock, rewardPercentiles, tendermintBlockResult, &oneFeeHistory)
+				err = b.processBlocker(tendermintblock, &ethBlock, rewardPercentiles, tendermintBlockResult, &oneFeeHistory)
 				if err != nil {
 					chanErr <- err
 					return
 				}
 
 				// copy
 				thisBaseFee[index] = (*hexutil.Big)(oneFeeHistory.BaseFee)
-				thisBaseFee[index+1] = (*hexutil.Big)(oneFeeHistory.NextBaseFee)
+				// only use NextBaseFee as last item to avoid concurrent write
+				if int(index) == len(thisBaseFee)-2 {
+					thisBaseFee[index+1] = (*hexutil.Big)(oneFeeHistory.NextBaseFee)
+				}
 				thisGasUsedRatio[index] = oneFeeHistory.GasUsedRatio
 				if calculateRewards {
 					for j := 0; j < rewardCount; j++ {

tests/integration_tests/cosmoscli.py

@@ -1,4 +1,5 @@
 import json
+import subprocess
 import tempfile
 
 import requests
@@ -13,10 +14,10 @@ class ChainCommand:
     def __init__(self, cmd):
         self.cmd = cmd
 
-    def __call__(self, cmd, *args, stdin=None, **kwargs):
+    def __call__(self, cmd, *args, stdin=None, stderr=subprocess.STDOUT, **kwargs):
         "execute chain-maind"
         args = " ".join(build_cli_args_safe(cmd, *args, **kwargs))
-        return interact(f"{self.cmd} {args}", input=stdin)
+        return interact(f"{self.cmd} {args}", input=stdin, stderr=stderr)
 
 
 class CosmosCLI:

tests/integration_tests/test_fee_history.py

@@ -160,3 +160,28 @@ def test_percentiles(cluster):
         ]
         result = [future.result() for future in as_completed(tasks)]
         assert all(msg in res["error"]["message"] for res in result)
+
+
+def test_concurrent(custom_ethermint):
+    w3: Web3 = custom_ethermint.w3
+    tx = {"to": ADDRS["community"], "value": 10, "gasPrice": w3.eth.gas_price}
+    # send multi txs, overlap happens with query with 2nd tx's block number
+    send_transaction(w3, tx)
+    receipt1 = send_transaction(w3, tx)
+    b1 = receipt1.blockNumber
+    send_transaction(w3, tx)
+
+    call = w3.provider.make_request
+    field = "baseFeePerGas"
+
+    percentiles = []
+    method = "eth_feeHistory"
+    # big enough concurrent requests to trigger overwrite bug
+    total = 10
+    size = 2
+    params = [size, hex(b1), percentiles]
+    res = []
+    with ThreadPoolExecutor(total) as exec:
+        t = [exec.submit(call, method, params) for i in range(total)]
+        res = [future.result()["result"][field] for future in as_completed(t)]
+    assert all(sublist == res[0] for sublist in res), res