Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 13 additions & 1 deletion rpc/backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -179,7 +189,7 @@ func NewBackend(
clientCtx = clientCtx.WithKeyring(kr)
}

return &Backend{
b := &Backend{
ctx: context.Background(),
clientCtx: clientCtx,
queryClient: rpctypes.NewQueryClient(clientCtx),
Expand All @@ -189,4 +199,6 @@ func NewBackend(
allowUnprotectedTxs: allowUnprotectedTxs,
indexer: indexer,
}
b.processBlocker = b.processBlock
yihuang marked this conversation as resolved.
Show resolved Hide resolved
return b
}
7 changes: 5 additions & 2 deletions rpc/backend/chain_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++ {
Expand Down
5 changes: 3 additions & 2 deletions tests/integration_tests/cosmoscli.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import subprocess
import tempfile

import requests
Expand All @@ -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:
Expand Down
25 changes: 25 additions & 0 deletions tests/integration_tests/test_fee_history.py
Original file line number Diff line number Diff line change
Expand Up @@ -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