Problem: concurrent write base fee in fee history

CHANGELOG.md

@@ -62,6 +62,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 - (rpc) [#1685](https://github.com/evmos/ethermint/pull/1685) Fix parse for websocket connID.
 - (rpc) [#1773](https://github.com/evmos/ethermint/pull/1773) Avoid channel get changed when concurrent subscribe happens.
 * (mempool) [#310](https://github.com/crypto-org-chain/ethermint/pull/310) disable vesting messages in check tx mode.
+* (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
 

cmd/ethermintd/root.go

@@ -174,6 +174,7 @@ func queryCommand() *cobra.Command {
 		rpc.BlockCommand(),
 		authcmd.QueryTxsByEventsCmd(),
 		authcmd.QueryTxCmd(),
+		rpc.QueryEventForTxCmd(),
 	)
 
 	app.ModuleBasics.AddQueryCommands(cmd)

rpc/backend/backend.go

@@ -32,7 +32,8 @@
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/rpc"
 	"github.com/ethereum/go-ethereum/signer/core/apitypes"
+	"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"
 	evmtypes "github.com/evmos/ethermint/x/evm/types"
@@ -144,6 +145,14 @@
 
 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
@@ -154,6 +163,7 @@
 	cfg                 config.Config
 	allowUnprotectedTxs bool
 	indexer             ethermint.EVMTxIndexer
+	processBlocker      ProcessBlocker
 }
 
 // NewBackend creates a new Backend instance for cosmos and ethereum namespaces
@@ -174,7 +184,7 @@
 		panic(err)
 	}
 
-	return &Backend{
+	b := &Backend{
 		ctx:                 context.Background(),
 		clientCtx:           clientCtx,
 		queryClient:         rpctypes.NewQueryClient(clientCtx),
@@ -184,4 +194,6 @@
 		allowUnprotectedTxs: allowUnprotectedTxs,
 		indexer:             indexer,
 	}
+	b.processBlocker = b.processBlock
+	return b
 }

rpc/backend/chain_info.go

@@ -252,15 +252,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++ {

rpc/backend/chain_info_test.go

@@ -320,13 +320,14 @@ func (suite *BackendTestSuite) TestGlobalMinGasPrice() {
 
 func (suite *BackendTestSuite) TestFeeHistory() {
 	testCases := []struct {
-		name           string
-		registerMock   func(validator sdk.AccAddress)
-		userBlockCount ethrpc.DecimalOrHex
-		latestBlock    ethrpc.BlockNumber
-		expFeeHistory  *rpc.FeeHistoryResult
-		validator      sdk.AccAddress
-		expPass        bool
+		name              string
+		registerMock      func(validator sdk.AccAddress)
+		userBlockCount    ethrpc.DecimalOrHex
+		latestBlock       ethrpc.BlockNumber
+		expFeeHistory     *rpc.FeeHistoryResult
+		validator         sdk.AccAddress
+		expPass           bool
+		targetNewBaseFees []*big.Int
 	}{
 		{
 			"fail - can't get params ",
@@ -341,6 +342,7 @@ func (suite *BackendTestSuite) TestFeeHistory() {
 			nil,
 			nil,
 			false,
+			nil,
 		},
 		{
 			"fail - user block count higher than max block count ",
@@ -355,6 +357,7 @@ func (suite *BackendTestSuite) TestFeeHistory() {
 			nil,
 			nil,
 			false,
+			nil,
 		},
 		{
 			"fail - Tendermint block fetching error ",
@@ -371,6 +374,7 @@ func (suite *BackendTestSuite) TestFeeHistory() {
 			nil,
 			nil,
 			false,
+			nil,
 		},
 		{
 			"fail - Eth block fetching error",
@@ -388,6 +392,7 @@ func (suite *BackendTestSuite) TestFeeHistory() {
 			nil,
 			nil,
 			true,
+			nil,
 		},
 		{
 			"fail - Invalid base fee",
@@ -409,6 +414,7 @@ func (suite *BackendTestSuite) TestFeeHistory() {
 			nil,
 			sdk.AccAddress(tests.GenerateAddress().Bytes()),
 			false,
+			nil,
 		},
 		{
 			"pass - Valid FeeHistoryResults object",
@@ -438,6 +444,39 @@ func (suite *BackendTestSuite) TestFeeHistory() {
 			},
 			sdk.AccAddress(tests.GenerateAddress().Bytes()),
 			true,
+			nil,
+		},
+		{
+			"pass - Concurrent FeeHistoryResults object",
+			func(validator sdk.AccAddress) {
+				var header metadata.MD
+				baseFee := sdk.NewInt(1)
+				queryClient := suite.backend.queryClient.QueryClient.(*mocks.EVMQueryClient)
+				fQueryClient := suite.backend.queryClient.FeeMarket.(*mocks.FeeMarketQueryClient)
+				client := suite.backend.clientCtx.Client.(*mocks.Client)
+				suite.backend.cfg.JSONRPC.FeeHistoryCap = 2
+				RegisterBlock(client, ethrpc.BlockNumber(1).Int64(), nil)
+				RegisterBlockResults(client, 1)
+				RegisterBaseFee(queryClient, baseFee)
+				RegisterValidatorAccount(queryClient, validator)
+				RegisterConsensusParams(client, 1)
+				RegisterParams(queryClient, &header, 1)
+				RegisterParamsWithoutHeader(queryClient, 1)
+				RegisterFeeMarketParams(fQueryClient, 1)
+			},
+			1,
+			1,
+			&rpc.FeeHistoryResult{
+				OldestBlock:  (*hexutil.Big)(big.NewInt(1)),
+				BaseFee:      []*hexutil.Big{(*hexutil.Big)(big.NewInt(1)), (*hexutil.Big)(big.NewInt(0))},
+				GasUsedRatio: []float64{0},
+				Reward:       [][]*hexutil.Big{{(*hexutil.Big)(big.NewInt(0)), (*hexutil.Big)(big.NewInt(0)), (*hexutil.Big)(big.NewInt(0)), (*hexutil.Big)(big.NewInt(0))}},
+			},
+			sdk.AccAddress(tests.GenerateAddress().Bytes()),
+			true,
+			[]*big.Int{
+				big.NewInt(0), // for overwrite overlap
+			},
 		},
 	}
 
@@ -446,6 +485,22 @@ func (suite *BackendTestSuite) TestFeeHistory() {
 			suite.SetupTest() // reset test and queries
 			tc.registerMock(tc.validator)
 
+			called := 0
+			if len(tc.targetNewBaseFees) > 0 {
+				suite.backend.processBlocker = func(
+					tendermintBlock *tmrpctypes.ResultBlock,
+					ethBlock *map[string]interface{},
+					rewardPercentiles []float64,
+					tendermintBlockResult *tmrpctypes.ResultBlockResults,
+					targetOneFeeHistory *rpc.OneFeeHistory,
+				) error {
+					suite.backend.processBlock(tendermintBlock, ethBlock, rewardPercentiles, tendermintBlockResult, targetOneFeeHistory)
+					targetOneFeeHistory.NextBaseFee = tc.targetNewBaseFees[called]
+					called += 1
+					return nil
+				}
+			}
+
 			feeHistory, err := suite.backend.FeeHistory(tc.userBlockCount, tc.latestBlock, []float64{25, 50, 75, 100})
 			if tc.expPass {
 				suite.Require().NoError(err)

tests/integration_tests/configs/cosmovisor.jsonnet

@@ -12,6 +12,20 @@ config {
             base_fee:: super.base_fee,
           },
         },
+        gov: {
+          voting_params: {
+            voting_period: '10s',
+          },
+          deposit_params: {
+            max_deposit_period: '10s',
+            min_deposit: [
+              {
+                denom: 'aphoton',
+                amount: '1',
+              },
+            ],
+          },
+        },
       },
     },
   },

tests/integration_tests/configs/default.jsonnet

@@ -57,10 +57,8 @@
           },
         },
         gov: {
-          voting_params: {
+          params: {
             voting_period: '10s',
-          },
-          deposit_params: {
             max_deposit_period: '10s',
             min_deposit: [
               {

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:
@@ -700,6 +701,7 @@ def gov_propose(self, proposer, kind, proposal, **kwargs):
 
     def gov_vote(self, voter, proposal_id, option, **kwargs):
         kwargs.setdefault("gas_prices", DEFAULT_GAS_PRICE)
+        kwargs.setdefault("broadcast_mode", "sync")
         return json.loads(
             self.raw(
                 "tx",
@@ -839,3 +841,38 @@ def rollback(self):
 
     def migrate_keystore(self):
         return self.raw("keys", "migrate", home=self.data_dir)
+
+    def get_default_kwargs(self):
+        return {
+            "gas_prices": DEFAULT_GAS_PRICE,
+            "gas": "auto",
+            "gas_adjustment": "1.5",
+        }
+
+    def event_query_tx_for(self, hash):
+        return json.loads(
+            self.raw(
+                "query",
+                "event-query-tx-for",
+                hash,
+                "-y",
+                home=self.data_dir,
+                stderr=subprocess.DEVNULL,
+            )
+        )
+
+    def submit_gov_proposal(self, proposal, **kwargs):
+        default_kwargs = self.get_default_kwargs()
+        kwargs.setdefault("broadcast_mode", "sync")
+        return json.loads(
+            self.raw(
+                "tx",
+                "gov",
+                "submit-proposal",
+                proposal,
+                "-y",
+                home=self.data_dir,
+                stderr=subprocess.DEVNULL,
+                **(default_kwargs | kwargs),
+            )
+        )