From 3e24d4e555af81c507c7a46cc4b478c827843eef Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Mon, 3 Aug 2026 22:21:06 +0200 Subject: [PATCH 1/9] rpc: resolve eth_feeHistory on the block overlay view eth_feeHistory resolved its head on the committed view while eth_blockNumber publishes the overlay head, so during the forkchoice flush+commit window the whole fee window lagged the published head by one block or more (oldestBlock 25673288 vs 25673290 on a reference node, run 30798564490). Pin one overlay read view in NewGasPriceOracleBackend so the head the oracle resolves and the per-block data it samples come from the same view, and build the backend returned by Fork() through the same constructor: the per-block sampling of FeeHistory and SuggestTipCap runs on the forked tx, which used to be a raw tx with no overlay (the gap noted in the #22006 review). Senders and receipts of the in-flight block are served by the overlay because the senders stage and execution run on the overlay tx before publication (forkchoice.go). Both tests verified red without the fix (window ends on the committed head), green with it. The test harness now also writes the forkchoice head marker to the overlay, which is what rpchelper.GetLatestBlockNumber resolves from in production. --- rpc/jsonrpc/eth_system.go | 11 +++++++-- rpc/jsonrpc/overlay_race_test.go | 38 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/rpc/jsonrpc/eth_system.go b/rpc/jsonrpc/eth_system.go index 9292a6f23de..b26f6237bae 100644 --- a/rpc/jsonrpc/eth_system.go +++ b/rpc/jsonrpc/eth_system.go @@ -497,8 +497,13 @@ type GasPriceOracleBackend struct { baseApi *BaseAPI } +// NewGasPriceOracleBackend pins one block-overlay read view for every read the backend +// does, so the head it resolves and the per-block data it samples come from the same view +// (see rpchelper.GetBlockNumber). Without it the head would be resolved on the overlay by +// the callers that wrap their own tx while the block data came from the committed view, +// leaving the oracle a block or more behind the head the node publishes. func NewGasPriceOracleBackend(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI) *GasPriceOracleBackend { - return &GasPriceOracleBackend{db: db, tx: tx, baseApi: baseApi} + return &GasPriceOracleBackend{db: db, tx: baseApi.filters.WithTemporalOverlay(tx), baseApi: baseApi} } func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBackend, func(), error) { @@ -509,7 +514,9 @@ func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBacken if err != nil { return nil, nil, err } - return &GasPriceOracleBackend{db: b.db, tx: tx, baseApi: b.baseApi}, + // Overlay-wrap the forked tx too: the per-block sampling of SuggestTipCap and + // FeeHistory runs on this one, not on b.tx. + return NewGasPriceOracleBackend(b.db, tx, b.baseApi), func() { tx.Rollback() }, nil } diff --git a/rpc/jsonrpc/overlay_race_test.go b/rpc/jsonrpc/overlay_race_test.go index 1ec6fd9de58..12054017b67 100644 --- a/rpc/jsonrpc/overlay_race_test.go +++ b/rpc/jsonrpc/overlay_race_test.go @@ -102,6 +102,9 @@ func newOverlayAheadTestAPI(t *testing.T) (base *BaseAPI, m *execmoduletester.Ex rawdb.WriteForkchoiceHead(overlay, hash) require.NoError(t, rawdb.WriteCanonicalHash(overlay, hash, overlayNumber)) require.NoError(t, rawdb.WriteBody(overlay, hash, overlayNumber, &types.Body{})) + // The forkchoice marker is what rpchelper.GetLatestBlockNumber resolves the head from, + // so readers that go through it (the gas oracle, "latest" tag resolution) see this block. + rawdb.WriteForkchoiceHead(overlay, hash) events := shards.NewEvents() events.PublishOverlay(doms) @@ -249,3 +252,38 @@ func TestTxPoolContentFrom_UsesOverlayHead(t *testing.T) { require.Equal(t, overlayHeader.BaseFee.ToBig(), got.GasPrice.ToInt(), "pending tx gas price must be derived from the overlay head's base fee, not the stale MDBX head") } + +// TestFeeHistory_SeesOverlayHead pins that eth_feeHistory resolves "latest" through the +// block overlay: the gas oracle's head must be the in-flight block, so the window ends +// there and oldestBlock is that block. Resolving on the committed view instead leaves the +// whole window one block (or more, while a commit backlog drains) behind the head the node +// publishes via eth_blockNumber. +func TestFeeHistory_SeesOverlayHead(t *testing.T) { + t.Parallel() + base, m, overlayHeader := newOverlayAheadTestAPI(t) + api := newEthApiForTest(base, m.DB, nil, nil) + + got, err := api.FeeHistory(m.Ctx, 1, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, overlayHeader.Number.ToBig(), got.OldestBlock.ToInt(), + "the fee history window must end on the overlay head, not the stale MDBX-committed head") + require.NotEmpty(t, got.BaseFee) + require.Equal(t, overlayHeader.BaseFee.ToBig(), got.BaseFee[0].ToInt(), + "the first base fee must come from the overlay head's header") +} + +// TestFeeHistory_OverlayHeadWithRewards is TestFeeHistory_SeesOverlayHead with reward +// percentiles requested, which makes the per-block fetch read the in-flight block and its +// receipts instead of just the header — the path that runs on the tx opened by Fork. +func TestFeeHistory_OverlayHeadWithRewards(t *testing.T) { + t.Parallel() + base, m, overlayHeader := newOverlayAheadTestAPI(t) + api := newEthApiForTest(base, m.DB, nil, nil) + + got, err := api.FeeHistory(m.Ctx, 1, rpc.LatestBlockNumber, []float64{50}) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, overlayHeader.Number.ToBig(), got.OldestBlock.ToInt(), + "the fee history window must end on the overlay head, not the stale MDBX-committed head") +} From 74ab0b6dc12bd272e50beff3771b40c671e1782f Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Wed, 5 Aug 2026 21:23:40 +0200 Subject: [PATCH 2/9] rpc: pin the block overlay in the gas oracle backend across Fork Fork used to re-resolve LatestSD() on its fresh tx: if the commit window closed (PublishOverlay(nil)) between the request start and the fork, the forked backend had neither the overlay nor the committed head block, so eth_feeHistory failed with block-not-found on the header path or silently dropped the head slot on the rewards path. The backend now captures the overlay instance once at construction and Fork wraps its tx with that same instance; plain-table reads on a closed overlay are safe (memStore close is a no-op on the data). Also extend TestFeeHistory_OverlayHeadWithRewards with two overlay txs and their receipt-domain entries: a low percentile lands on the cheap tx only when the receipts' gas is actually read, so the assert pins the receipt values instead of just the window bounds. --- rpc/jsonrpc/eth_system.go | 31 ++++++--- rpc/jsonrpc/overlay_race_test.go | 115 ++++++++++++++++++++++++++++--- rpc/rpchelper/filters.go | 16 +++++ 3 files changed, 142 insertions(+), 20 deletions(-) diff --git a/rpc/jsonrpc/eth_system.go b/rpc/jsonrpc/eth_system.go index b26f6237bae..43367a4390c 100644 --- a/rpc/jsonrpc/eth_system.go +++ b/rpc/jsonrpc/eth_system.go @@ -28,6 +28,7 @@ import ( "github.com/erigontech/erigon/common/hexutil" "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/db/kv/kvcfg" + "github.com/erigontech/erigon/db/kv/membatchwithdb" "github.com/erigontech/erigon/db/kv/prune" "github.com/erigontech/erigon/db/rawdb" "github.com/erigontech/erigon/execution/chain" @@ -495,15 +496,25 @@ type GasPriceOracleBackend struct { db kv.TemporalRoDB // nil if Fork is not supported tx kv.TemporalTx baseApi *BaseAPI + overlay *membatchwithdb.MemoryMutation // pinned at construction; nil when no overlay was published } -// NewGasPriceOracleBackend pins one block-overlay read view for every read the backend -// does, so the head it resolves and the per-block data it samples come from the same view -// (see rpchelper.GetBlockNumber). Without it the head would be resolved on the overlay by -// the callers that wrap their own tx while the block data came from the committed view, -// leaving the oracle a block or more behind the head the node publishes. +// NewGasPriceOracleBackend pins the block overlay once for every read the backend +// does, so the head it resolves and the per-block data it samples come from the same +// view (see rpchelper.GetBlockNumber). Without it the head would be resolved on the +// overlay by the callers that wrap their own tx while the block data came from the +// committed view, leaving the oracle a block or more behind the head the node publishes. func NewGasPriceOracleBackend(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI) *GasPriceOracleBackend { - return &GasPriceOracleBackend{db: db, tx: baseApi.filters.WithTemporalOverlay(tx), baseApi: baseApi} + b := &GasPriceOracleBackend{db: db, baseApi: baseApi, overlay: baseApi.filters.LatestOverlay()} + b.tx = b.withOverlay(tx) + return b +} + +func (b *GasPriceOracleBackend) withOverlay(tx kv.TemporalTx) kv.TemporalTx { + if b.overlay == nil { + return tx + } + return b.overlay.NewReadView(tx) } func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBackend, func(), error) { @@ -514,9 +525,11 @@ func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBacken if err != nil { return nil, nil, err } - // Overlay-wrap the forked tx too: the per-block sampling of SuggestTipCap and - // FeeHistory runs on this one, not on b.tx. - return NewGasPriceOracleBackend(b.db, tx, b.baseApi), + // Wrap the forked tx (the one SuggestTipCap and FeeHistory sample on) with the + // pinned overlay rather than re-resolving it: the overlay may have been + // unpublished since the request started, and re-resolving would leave the fork + // without the head block the parent already resolved. + return &GasPriceOracleBackend{db: b.db, tx: b.withOverlay(tx), baseApi: b.baseApi, overlay: b.overlay}, func() { tx.Rollback() }, nil } diff --git a/rpc/jsonrpc/overlay_race_test.go b/rpc/jsonrpc/overlay_race_test.go index 12054017b67..c2e121f4745 100644 --- a/rpc/jsonrpc/overlay_race_test.go +++ b/rpc/jsonrpc/overlay_race_test.go @@ -19,6 +19,7 @@ package jsonrpc import ( "bytes" "context" + "math/big" "strconv" "testing" @@ -31,6 +32,7 @@ import ( "github.com/erigontech/erigon/common/hexutil" "github.com/erigontech/erigon/db/kv/kvcache" "github.com/erigontech/erigon/db/rawdb" + "github.com/erigontech/erigon/db/rawdb/rawtemporaldb" "github.com/erigontech/erigon/db/state/execctx" "github.com/erigontech/erigon/execution/chain" "github.com/erigontech/erigon/execution/execmodule/execmoduletester" @@ -48,8 +50,18 @@ import ( const ( overlayRaceChainSize = 5 overlayRaceBaseFee = 424242 + overlayRaceLowTip = 1_000_000 + overlayRaceHighTip = 2_000_000 ) +type overlayAheadHarness struct { + base *BaseAPI + m *execmoduletester.ExecModuleTester + overlayHeader *types.Header + overlayTxs types.Transactions + events *shards.Events +} + // newOverlayAheadTestAPI builds overlayRaceChainSize committed MDBX blocks, // then publishes a fabricated block one past them (overlayRaceChainSize+1) // into the block overlay only, never committed to MDBX. This reproduces the @@ -61,11 +73,21 @@ const ( // reliable, deterministic fingerprint for "the code read the overlay head". func newOverlayAheadTestAPI(t *testing.T) (base *BaseAPI, m *execmoduletester.ExecModuleTester, overlayHeader *types.Header) { t.Helper() + h := newOverlayAheadHarness(t, false) + return h.base, h.m, h.overlayHeader +} + +// With withOverlayTxs the overlay block carries two transactions with distinct +// tips plus their receipt-domain entries, and GasUsed equals the transactions' +// real total (not the EIP-1559 target) so reward percentile thresholds relate +// to the receipts' gas. +func newOverlayAheadHarness(t *testing.T, withOverlayTxs bool) *overlayAheadHarness { + t.Helper() var cfg chain.Config require.NoError(t, copier.CopyWithOption(&cfg, chain.TestChainBerlinConfig, copier.Option{DeepCopy: true})) cfg.LondonBlock = common.NewUint64(0) - m = execmoduletester.New(t, execmoduletester.WithChainConfig(&cfg)) + m := execmoduletester.New(t, execmoduletester.WithChainConfig(&cfg)) c, err := m.GenerateChain(overlayRaceChainSize, func(i int, gen *blockgen.BlockGen) { gen.SetCoinbase(common.Address{1}) @@ -84,13 +106,24 @@ func newOverlayAheadTestAPI(t *testing.T) (base *BaseAPI, m *execmoduletester.Ex const overlayGasLimit = 30_000_000 overlayNumber := uint64(overlayRaceChainSize) + 1 - overlayHeader = &types.Header{ + + var overlayTxs types.Transactions + overlayGasUsed := uint64(overlayGasLimit / params.ElasticityMultiplier) // == target: CalcBaseFee leaves BaseFee unchanged + if withOverlayTxs { + overlayTxs = types.Transactions{ + signOverlayRaceTestTxWithTip(t, m, 0, overlayRaceLowTip), + signOverlayRaceTestTxWithTip(t, m, 1, overlayRaceHighTip), + } + overlayGasUsed = uint64(len(overlayTxs)) * params.TxGas + } + + overlayHeader := &types.Header{ ParentHash: c.TopBlock.Hash(), Number: *uint256.NewInt(overlayNumber), Difficulty: *uint256.NewInt(0), Time: c.TopBlock.Time() + 10, GasLimit: overlayGasLimit, - GasUsed: overlayGasLimit / params.ElasticityMultiplier, // == target: CalcBaseFee leaves BaseFee unchanged + GasUsed: overlayGasUsed, BaseFee: uint256.NewInt(overlayRaceBaseFee), } hash := overlayHeader.Hash() @@ -101,18 +134,36 @@ func newOverlayAheadTestAPI(t *testing.T) (base *BaseAPI, m *execmoduletester.Ex require.NoError(t, rawdb.WriteHeadHeaderHash(overlay, hash)) rawdb.WriteForkchoiceHead(overlay, hash) require.NoError(t, rawdb.WriteCanonicalHash(overlay, hash, overlayNumber)) - require.NoError(t, rawdb.WriteBody(overlay, hash, overlayNumber, &types.Body{})) + require.NoError(t, rawdb.WriteBody(overlay, hash, overlayNumber, &types.Body{Transactions: overlayTxs})) // The forkchoice marker is what rpchelper.GetLatestBlockNumber resolves the head from, // so readers that go through it (the gas oracle, "latest" tag resolution) see this block. rawdb.WriteForkchoiceHead(overlay, hash) + if withOverlayTxs { + senders := make([]common.Address, len(overlayTxs)) + for i := range senders { + senders[i] = m.Address + } + require.NoError(t, rawdb.WriteSenders(overlay, hash, overlayNumber, senders)) + // Receipt-domain entries go through the SharedDomains (like execution writes + // them), so readers reach them via the view's DomainReader, not the overlay tables. + minTxNum, err := m.BlockReader.TxnumReader().Min(ctx, overlayRoTx, overlayNumber) + require.NoError(t, err) + putDel := doms.AsPutDel(overlayRoTx) + var cumGas uint64 + for i := range overlayTxs { + cumGas += params.TxGas + require.NoError(t, rawtemporaldb.AppendReceiptMetadata(putDel, 0, cumGas, 0, minTxNum+1+uint64(i))) + } + } + events := shards.NewEvents() events.PublishOverlay(doms) filters := rpchelper.New(ctx, rpchelper.DefaultFiltersConfig, nil, nil, nil, func() {}, m.Log, events) stateCache := kvcache.New(kvcache.DefaultCoherentConfig) - base = newBaseApiWithFiltersForTest(filters, stateCache, m) + base := newBaseApiWithFiltersForTest(filters, stateCache, m) - return base, m, overlayHeader + return &overlayAheadHarness{base: base, m: m, overlayHeader: overlayHeader, overlayTxs: overlayTxs, events: events} } // overlayRaceTxPoolClient extends stubTxPoolClient with canned replies for @@ -132,10 +183,14 @@ func (c *overlayRaceTxPoolClient) All(context.Context, *txpoolproto.AllRequest, } func signOverlayRaceTestTx(t *testing.T, m *execmoduletester.ExecModuleTester, nonce uint64) types.Transaction { + return signOverlayRaceTestTxWithTip(t, m, nonce, 0) +} + +func signOverlayRaceTestTxWithTip(t *testing.T, m *execmoduletester.ExecModuleTester, nonce uint64, tip uint64) types.Transaction { t.Helper() signer := types.LatestSigner(m.ChainConfig) txn, err := types.SignTx( - types.NewEIP1559Transaction(*m.ChainConfig.ChainID, nonce, common.HexToAddress("deadbeef"), uint256.NewInt(1), 21000, nil, uint256.NewInt(0), uint256.NewInt(1_000_000_000_000), nil), + types.NewEIP1559Transaction(*m.ChainConfig.ChainID, nonce, common.HexToAddress("deadbeef"), uint256.NewInt(1), 21000, nil, uint256.NewInt(tip), uint256.NewInt(1_000_000_000_000), nil), *signer, m.Key, ) require.NoError(t, err) @@ -276,14 +331,52 @@ func TestFeeHistory_SeesOverlayHead(t *testing.T) { // TestFeeHistory_OverlayHeadWithRewards is TestFeeHistory_SeesOverlayHead with reward // percentiles requested, which makes the per-block fetch read the in-flight block and its // receipts instead of just the header — the path that runs on the tx opened by Fork. +// The overlay block carries two txs with distinct tips: a low percentile lands on the +// cheap tx only when the receipts' gas is actually read (zero-filled gas walks the +// percentile cursor to the most expensive tx instead), so the assert pins the receipt +// values, not just the window bounds. func TestFeeHistory_OverlayHeadWithRewards(t *testing.T) { t.Parallel() - base, m, overlayHeader := newOverlayAheadTestAPI(t) - api := newEthApiForTest(base, m.DB, nil, nil) + h := newOverlayAheadHarness(t, true) + api := newEthApiForTest(h.base, h.m.DB, nil, nil) - got, err := api.FeeHistory(m.Ctx, 1, rpc.LatestBlockNumber, []float64{50}) + got, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, []float64{10}) require.NoError(t, err) require.NotNil(t, got) - require.Equal(t, overlayHeader.Number.ToBig(), got.OldestBlock.ToInt(), + require.Equal(t, h.overlayHeader.Number.ToBig(), got.OldestBlock.ToInt(), "the fee history window must end on the overlay head, not the stale MDBX-committed head") + require.Len(t, got.Reward, 1) + require.Len(t, got.Reward[0], 1) + require.Equal(t, big.NewInt(overlayRaceLowTip), got.Reward[0][0].ToInt(), + "the 10th percentile must be the cheap tx's tip, weighted by the receipts' real gas") +} + +// TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish pins that Fork reuses the overlay +// resolved when the backend was built: when the overlay is unpublished between the +// request start and the fork (the commit window closing), the forked backend must +// still serve the head the parent resolved instead of failing with block-not-found. +func TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + tx, err := h.m.DB.BeginTemporalRo(h.m.Ctx) + require.NoError(t, err) + defer tx.Rollback() + backend := NewGasPriceOracleBackend(h.m.DB, tx, h.base) + + h.events.PublishOverlay(nil) + + forked, cleanup, err := backend.Fork(h.m.Ctx) + require.NoError(t, err) + require.NotNil(t, forked) + defer cleanup() + + latest, err := forked.GetLatestBlockNumber() + require.NoError(t, err) + require.Equal(t, h.overlayHeader.Number.Uint64(), latest, + "the forked backend must keep resolving the overlay head the parent pinned") + + got, err := forked.HeaderByNumber(h.m.Ctx, rpc.BlockNumber(h.overlayHeader.Number.Uint64())) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, h.overlayHeader.Hash(), got.Hash()) } diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index 4645e6661f9..e1fe462f181 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -36,6 +36,7 @@ import ( "github.com/erigontech/erigon/common/concurrent" "github.com/erigontech/erigon/common/log/v3" "github.com/erigontech/erigon/db/kv" + "github.com/erigontech/erigon/db/kv/membatchwithdb" "github.com/erigontech/erigon/db/state/execctx" "github.com/erigontech/erigon/execution/rlp" "github.com/erigontech/erigon/execution/types" @@ -1188,6 +1189,21 @@ func (ff *Filters) WithOverlay(tx kv.Tx) kv.Tx { return tx } +// LatestOverlay returns the block overlay behind the latest published SD, or nil. +// Callers that must keep serving one consistent head across several txs (e.g. a +// forked backend) pin this instance instead of re-resolving, which could observe +// the overlay being unpublished mid-request. +func (ff *Filters) LatestOverlay() *membatchwithdb.MemoryMutation { + if ff == nil { + return nil + } + sd := ff.LatestSD() + if sd == nil { + return nil + } + return sd.BlockOverlay() +} + // WithTemporalOverlay is like WithOverlay but returns kv.TemporalTx directly, // avoiding repeated type assertions at callsites that need temporal access. func (ff *Filters) WithTemporalOverlay(tx kv.TemporalTx) kv.TemporalTx { From 08df9d7ef1369833b6830816ed0e8bd98ce333f2 Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Wed, 5 Aug 2026 22:41:43 +0200 Subject: [PATCH 3/9] rpc: dedup overlay resolution and trim gas oracle comments WithOverlay and WithTemporalOverlay now resolve through LatestOverlay instead of carrying three copies of the same nil-check chain. Test harness: drop the never-read overlayTxs field, move the docstring onto the constructor that implements the behavior, restore t.Helper in the signing wrapper. No behavior change. --- rpc/jsonrpc/eth_system.go | 13 ++++--------- rpc/jsonrpc/overlay_race_test.go | 23 +++++++++++------------ rpc/rpchelper/filters.go | 18 ++---------------- 3 files changed, 17 insertions(+), 37 deletions(-) diff --git a/rpc/jsonrpc/eth_system.go b/rpc/jsonrpc/eth_system.go index 43367a4390c..f0266379bad 100644 --- a/rpc/jsonrpc/eth_system.go +++ b/rpc/jsonrpc/eth_system.go @@ -499,11 +499,8 @@ type GasPriceOracleBackend struct { overlay *membatchwithdb.MemoryMutation // pinned at construction; nil when no overlay was published } -// NewGasPriceOracleBackend pins the block overlay once for every read the backend -// does, so the head it resolves and the per-block data it samples come from the same -// view (see rpchelper.GetBlockNumber). Without it the head would be resolved on the -// overlay by the callers that wrap their own tx while the block data came from the -// committed view, leaving the oracle a block or more behind the head the node publishes. +// NewGasPriceOracleBackend pins the block overlay once so the head the oracle +// resolves stays readable for the whole request, including on the txs Fork opens. func NewGasPriceOracleBackend(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI) *GasPriceOracleBackend { b := &GasPriceOracleBackend{db: db, baseApi: baseApi, overlay: baseApi.filters.LatestOverlay()} b.tx = b.withOverlay(tx) @@ -525,10 +522,8 @@ func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBacken if err != nil { return nil, nil, err } - // Wrap the forked tx (the one SuggestTipCap and FeeHistory sample on) with the - // pinned overlay rather than re-resolving it: the overlay may have been - // unpublished since the request started, and re-resolving would leave the fork - // without the head block the parent already resolved. + // Reuse the pinned overlay instead of re-resolving: it may have been + // unpublished since the request started. return &GasPriceOracleBackend{db: b.db, tx: b.withOverlay(tx), baseApi: b.baseApi, overlay: b.overlay}, func() { tx.Rollback() }, nil diff --git a/rpc/jsonrpc/overlay_race_test.go b/rpc/jsonrpc/overlay_race_test.go index c2e121f4745..7791ba019b8 100644 --- a/rpc/jsonrpc/overlay_race_test.go +++ b/rpc/jsonrpc/overlay_race_test.go @@ -58,11 +58,16 @@ type overlayAheadHarness struct { base *BaseAPI m *execmoduletester.ExecModuleTester overlayHeader *types.Header - overlayTxs types.Transactions events *shards.Events } -// newOverlayAheadTestAPI builds overlayRaceChainSize committed MDBX blocks, +func newOverlayAheadTestAPI(t *testing.T) (base *BaseAPI, m *execmoduletester.ExecModuleTester, overlayHeader *types.Header) { + t.Helper() + h := newOverlayAheadHarness(t, false) + return h.base, h.m, h.overlayHeader +} + +// newOverlayAheadHarness builds overlayRaceChainSize committed MDBX blocks, // then publishes a fabricated block one past them (overlayRaceChainSize+1) // into the block overlay only, never committed to MDBX. This reproduces the // window where forkchoice publishes the overlay before the MDBX commit @@ -71,16 +76,9 @@ type overlayAheadHarness struct { // The overlay block's GasUsed is set to exactly its EIP-1559 target so // misc.CalcBaseFee leaves BaseFee unchanged, making overlayRaceBaseFee a // reliable, deterministic fingerprint for "the code read the overlay head". -func newOverlayAheadTestAPI(t *testing.T) (base *BaseAPI, m *execmoduletester.ExecModuleTester, overlayHeader *types.Header) { - t.Helper() - h := newOverlayAheadHarness(t, false) - return h.base, h.m, h.overlayHeader -} - -// With withOverlayTxs the overlay block carries two transactions with distinct +// With withOverlayTxs the block instead carries two transactions with distinct // tips plus their receipt-domain entries, and GasUsed equals the transactions' -// real total (not the EIP-1559 target) so reward percentile thresholds relate -// to the receipts' gas. +// real total so reward percentile thresholds relate to the receipts' gas. func newOverlayAheadHarness(t *testing.T, withOverlayTxs bool) *overlayAheadHarness { t.Helper() @@ -163,7 +161,7 @@ func newOverlayAheadHarness(t *testing.T, withOverlayTxs bool) *overlayAheadHarn stateCache := kvcache.New(kvcache.DefaultCoherentConfig) base := newBaseApiWithFiltersForTest(filters, stateCache, m) - return &overlayAheadHarness{base: base, m: m, overlayHeader: overlayHeader, overlayTxs: overlayTxs, events: events} + return &overlayAheadHarness{base: base, m: m, overlayHeader: overlayHeader, events: events} } // overlayRaceTxPoolClient extends stubTxPoolClient with canned replies for @@ -183,6 +181,7 @@ func (c *overlayRaceTxPoolClient) All(context.Context, *txpoolproto.AllRequest, } func signOverlayRaceTestTx(t *testing.T, m *execmoduletester.ExecModuleTester, nonce uint64) types.Transaction { + t.Helper() return signOverlayRaceTestTxWithTip(t, m, nonce, 0) } diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index e1fe462f181..03e10b26f37 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -1176,14 +1176,7 @@ func (ff *Filters) LatestSD() *execctx.SharedDomains { // for data not in the overlay. // Safe to call on a nil receiver. func (ff *Filters) WithOverlay(tx kv.Tx) kv.Tx { - if ff == nil { - return tx - } - sd := ff.LatestSD() - if sd == nil { - return tx - } - if overlay := sd.BlockOverlay(); overlay != nil { + if overlay := ff.LatestOverlay(); overlay != nil { return overlay.NewReadView(tx) } return tx @@ -1207,14 +1200,7 @@ func (ff *Filters) LatestOverlay() *membatchwithdb.MemoryMutation { // WithTemporalOverlay is like WithOverlay but returns kv.TemporalTx directly, // avoiding repeated type assertions at callsites that need temporal access. func (ff *Filters) WithTemporalOverlay(tx kv.TemporalTx) kv.TemporalTx { - if ff == nil { - return tx - } - sd := ff.LatestSD() - if sd == nil { - return tx - } - if overlay := sd.BlockOverlay(); overlay != nil { + if overlay := ff.LatestOverlay(); overlay != nil { return overlay.NewReadView(tx) } return tx From b88c0486d1b19063a0fcf17cf2fbc5b39b87b011 Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Mon, 10 Aug 2026 14:23:55 +0200 Subject: [PATCH 4/9] db, rpc: don't re-wrap txs that already carry a block overlay view A request that pinned an overlay view could have it re-wrapped by downstream helpers with the overlay published at call time, mixing two heads (and, after a same-height reorg, two forks) within one request. Read views now carry a flag exposed via membatchwithdb.CarriesOverlayView, and the overlay wrap points (Filters.WithOverlay, WithTemporalOverlay) return such txs unchanged: the first wrap pins the overlay a request reads from. MemoryMutation behavior is unchanged; the flag is set only by the read-view constructor, so writable batches keep being wrapped. --- .../carries_overlay_view_test.go | 42 +++++++++++++++++++ db/kv/membatchwithdb/memory_mutation.go | 16 +++++++ rpc/rpchelper/filters.go | 9 +++- 3 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 db/kv/membatchwithdb/carries_overlay_view_test.go diff --git a/db/kv/membatchwithdb/carries_overlay_view_test.go b/db/kv/membatchwithdb/carries_overlay_view_test.go new file mode 100644 index 00000000000..aeee94be0c6 --- /dev/null +++ b/db/kv/membatchwithdb/carries_overlay_view_test.go @@ -0,0 +1,42 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package membatchwithdb_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/db/kv/membatchwithdb" +) + +// TestCarriesOverlayView pins the predicate overlay wrap points rely on to +// avoid re-wrapping: read views over an overlay are recognized, while raw txs +// and writable batches are not. +func TestCarriesOverlayView(t *testing.T) { + _, rwTx := newTestTx(t) + + overlay, err := membatchwithdb.NewMemoryBatch(rwTx, "", log.Root()) + require.NoError(t, err) + defer overlay.Close() + + require.False(t, membatchwithdb.CarriesOverlayView(rwTx), "a raw tx is not an overlay view") + require.False(t, membatchwithdb.CarriesOverlayView(overlay), "a writable batch is not an overlay view") + require.True(t, membatchwithdb.CarriesOverlayView(overlay.NewReadView(rwTx))) + require.True(t, membatchwithdb.CarriesOverlayView(overlay.NewTemporalReadView(rwTx))) +} diff --git a/db/kv/membatchwithdb/memory_mutation.go b/db/kv/membatchwithdb/memory_mutation.go index 086a242411e..da60fc5c0d1 100644 --- a/db/kv/membatchwithdb/memory_mutation.go +++ b/db/kv/membatchwithdb/memory_mutation.go @@ -58,6 +58,7 @@ type MemoryMutation struct { db kv.TemporalTx statelessCursors map[string]kv.RwCursor DomainReader DomainReader + readView bool } // NewMemoryBatch creates a pure Go in-memory batch with no OS-thread affinity. @@ -1083,6 +1084,20 @@ func (m *MemoryMutation) NewReadView(tx kv.Tx) kv.TemporalTx { return m.newReadViewMut(tx) } +// CarriesOverlayView reports whether tx is already a read view over a block +// overlay. Overlay wrap points skip such txs so the first wrap pins the +// overlay a request reads from: re-wrapping would layer a possibly newer +// overlay on top, mixing two heads within one request. +func CarriesOverlayView(tx kv.Tx) bool { + switch v := tx.(type) { + case *OverlayTemporalReadView: + return true + case *MemoryMutation: + return v.readView + } + return false +} + // newReadViewMut is the internal constructor that returns the full // *MemoryMutation. Used by NewTemporalReadView which needs to embed it. func (m *MemoryMutation) newReadViewMut(tx kv.Tx) *MemoryMutation { @@ -1099,6 +1114,7 @@ func (m *MemoryMutation) newReadViewMut(tx kv.Tx) *MemoryMutation { clearedTables: m.clearedTables, db: dbTx, DomainReader: m.DomainReader, + readView: true, } } diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index 03e10b26f37..90ed4281d14 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -1173,9 +1173,13 @@ func (ff *Filters) LatestSD() *execctx.SharedDomains { // WithOverlay returns a read view backed by the latest block overlay if one // is available, otherwise returns the given tx unchanged. The read view uses // the overlay's in-memory data for table lookups, falling back to the caller's tx -// for data not in the overlay. +// for data not in the overlay. A tx that is already an overlay view is +// returned unchanged (see membatchwithdb.CarriesOverlayView). // Safe to call on a nil receiver. func (ff *Filters) WithOverlay(tx kv.Tx) kv.Tx { + if membatchwithdb.CarriesOverlayView(tx) { + return tx + } if overlay := ff.LatestOverlay(); overlay != nil { return overlay.NewReadView(tx) } @@ -1200,6 +1204,9 @@ func (ff *Filters) LatestOverlay() *membatchwithdb.MemoryMutation { // WithTemporalOverlay is like WithOverlay but returns kv.TemporalTx directly, // avoiding repeated type assertions at callsites that need temporal access. func (ff *Filters) WithTemporalOverlay(tx kv.TemporalTx) kv.TemporalTx { + if membatchwithdb.CarriesOverlayView(tx) { + return tx + } if overlay := ff.LatestOverlay(); overlay != nil { return overlay.NewReadView(tx) } From d9b1a587e720e129ab78b8a0073e6eaca880454f Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Mon, 10 Aug 2026 14:24:07 +0200 Subject: [PATCH 5/9] rpc: resolve gas oracle requests on one pinned overlay Follow-ups to the fee-history overlay resolution, addressing the rest of the review on this branch: - eth_gasPrice, eth_maxPriorityFeePerGas and eth_feeHistory acquire their tx and the block overlay as one consistent pair (beginTxWithOverlay): a commit or (un)publish landing between the two steps could leave the head visible in neither layer, so the tx is reopened on a mid-acquisition overlay change. - Fee history results for blocks above the committed head are no longer cached (OracleBackend.CacheableBlockLimit): the cache key is block number only, so a not-yet-committed block replaced by a same-height sibling kept serving the dead block's fees. Both lookup and store are skipped, since an entry may pre-exist from a block unwound later. - eth_gasPrice reads the baseFee addend through the pinned backend tx instead of re-resolving the live overlay, so tip and baseFee come from the same head. - The gas oracle's own wrap point skips already-wrapped txs, removing the double wrap on the fillFeeDefaults path. - TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish now pairs PublishOverlay(nil) with doms.Close() like the production teardown. --- rpc/gasprice/feehistory.go | 14 +- rpc/gasprice/gasprice.go | 6 + rpc/gasprice/gasprice_test.go | 2 + rpc/jsonrpc/eth_api.go | 2 +- rpc/jsonrpc/eth_fill_transaction.go | 6 +- rpc/jsonrpc/eth_system.go | 61 +++++++-- rpc/jsonrpc/overlay_race_test.go | 201 ++++++++++++++++++++++++++-- 7 files changed, 265 insertions(+), 27 deletions(-) diff --git a/rpc/gasprice/feehistory.go b/rpc/gasprice/feehistory.go index 0354bce8083..c6f0114a7a6 100644 --- a/rpc/gasprice/feehistory.go +++ b/rpc/gasprice/feehistory.go @@ -345,6 +345,10 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast // Pre-fetch chain config once using the main backend (safe: single goroutine). chainconfig := oracle.backend.ChainConfig() + var cacheableUpTo uint64 + if oracle.historyCache != nil { + cacheableUpTo = oracle.backend.CacheableBlockLimit() + } // Launch up to maxBlockFetchers goroutines. Each goroutine opens its own // TemporalTx via Fork so MDBX transactions are never shared across goroutines. @@ -380,9 +384,13 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast } idx := int(blockNumber - oldestBlock) - // Try the LRU cache first (skip for pending blocks — they are ephemeral). + // Try the LRU cache first. Pending and not-yet-committed blocks are + // excluded from both lookup and store: their number can be re-used by + // a same-height sibling, so an entry keyed by number alone would keep + // serving the dead block's fees. isPending := pendingBlock != nil && blockNumber >= pendingBlock.NumberU64() - if !isPending && oracle.historyCache != nil { + cacheable := !isPending && oracle.historyCache != nil && blockNumber <= cacheableUpTo + if cacheable { if cached, ok := oracle.historyCache.get(cacheKey{blockNumber, percentileKey}); ok { blockResults[idx] = blockResult{processed: cached, hasResult: true} continue @@ -420,7 +428,7 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast } blockResults[idx] = blockResult{processed: fees.results, hasResult: true} - if !isPending && oracle.historyCache != nil { + if cacheable { oracle.historyCache.add(cacheKey{blockNumber, percentileKey}, fees.results) } } diff --git a/rpc/gasprice/gasprice.go b/rpc/gasprice/gasprice.go index 41cf722e7d7..5fd649d8431 100644 --- a/rpc/gasprice/gasprice.go +++ b/rpc/gasprice/gasprice.go @@ -47,6 +47,12 @@ type OracleBackend interface { GetReceiptsGasUsed(ctx context.Context, block *types.Block) (types.Receipts, error) PendingBlockAndReceipts() (*types.Block, types.Receipts) + // CacheableBlockLimit returns the highest block number whose per-block fee + // data may be memoized across requests. Blocks above it are not durably + // committed yet and can still be replaced by a same-height sibling, so + // their results must not outlive the request. + CacheableBlockLimit() uint64 + // Fork opens a new TemporalTx and returns a goroutine-local backend together // with a cleanup function (call via defer cleanup()). // If the backend does not support forking, it returns (nil, nil, nil) and diff --git a/rpc/gasprice/gasprice_test.go b/rpc/gasprice/gasprice_test.go index 941a73df844..dfad61b76b4 100644 --- a/rpc/gasprice/gasprice_test.go +++ b/rpc/gasprice/gasprice_test.go @@ -300,6 +300,8 @@ func (m *mockOracleBackend) PendingBlockAndReceipts() (*types.Block, types.Recei return nil, nil } +func (m *mockOracleBackend) CacheableBlockLimit() uint64 { return math.MaxUint64 } + func (m *mockOracleBackend) Fork(_ context.Context) (gasprice.OracleBackend, func(), error) { return nil, nil, nil // sequential mode } diff --git a/rpc/jsonrpc/eth_api.go b/rpc/jsonrpc/eth_api.go index fcdf6ccf729..71c6e9cc4f8 100644 --- a/rpc/jsonrpc/eth_api.go +++ b/rpc/jsonrpc/eth_api.go @@ -522,7 +522,7 @@ type APIImpl struct { ethBackend rpchelper.ApiBackend txPool txpoolproto.TxpoolClient mining txpoolproto.MiningClient - gasCache *GasPriceCache + gasCache gasprice.Cache feeHistoryCache *gasprice.FeeHistoryCache db kv.TemporalRoDB GasCap uint64 diff --git a/rpc/jsonrpc/eth_fill_transaction.go b/rpc/jsonrpc/eth_fill_transaction.go index ff62814e011..b5012341aa0 100644 --- a/rpc/jsonrpc/eth_fill_transaction.go +++ b/rpc/jsonrpc/eth_fill_transaction.go @@ -165,7 +165,11 @@ func (api *APIImpl) FillTransaction(ctx context.Context, args ethapi.CallArgs) ( } func (api *APIImpl) newGasOracle(dbTx kv.TemporalTx) *gasprice.Oracle { - return gasprice.NewOracle(NewGasPriceOracleBackend(api.db, dbTx, api.BaseAPI), ethconfig.Defaults.GPO, api.gasCache, api.feeHistoryCache, api.logger.New("app", "gasPriceOracle")) + return api.newGasOracleFromBackend(NewGasPriceOracleBackend(api.db, dbTx, api.BaseAPI)) +} + +func (api *APIImpl) newGasOracleFromBackend(backend *GasPriceOracleBackend) *gasprice.Oracle { + return gasprice.NewOracle(backend, ethconfig.Defaults.GPO, api.gasCache, api.feeHistoryCache, api.logger.New("app", "gasPriceOracle")) } func (api *APIImpl) fillFeeDefaults(ctx context.Context, args *ethapi.CallArgs, head *types.Header, dbTx kv.TemporalTx) error { diff --git a/rpc/jsonrpc/eth_system.go b/rpc/jsonrpc/eth_system.go index f0266379bad..107a2b80d7b 100644 --- a/rpc/jsonrpc/eth_system.go +++ b/rpc/jsonrpc/eth_system.go @@ -258,20 +258,20 @@ func (api *APIImpl) ProtocolVersion(ctx context.Context) (hexutil.Uint, error) { // GasPrice implements eth_gasPrice. Returns the current price per gas in wei. func (api *APIImpl) GasPrice(ctx context.Context) (*hexutil.Big, error) { - tx, err := api.db.BeginTemporalRo(ctx) + tx, overlay, err := api.beginTxWithOverlay(ctx) if err != nil { return nil, err } defer tx.Rollback() - oracle := api.newGasOracle(tx) + backend := newGasPriceOracleBackendPinned(api.db, tx, api.BaseAPI, overlay) + oracle := api.newGasOracleFromBackend(backend) tipcap, err := oracle.SuggestTipCap(ctx) if err != nil { return nil, err } gasResult := uint256.NewInt(0) gasResult.Set(tipcap) - overlayTx := api.filters.WithTemporalOverlay(tx) - if head := rawdb.ReadCurrentHeader(overlayTx); head != nil && head.BaseFee != nil { + if head := rawdb.ReadCurrentHeader(backend.tx); head != nil && head.BaseFee != nil { gasResult.Add(tipcap, head.BaseFee) } @@ -280,12 +280,12 @@ func (api *APIImpl) GasPrice(ctx context.Context) (*hexutil.Big, error) { // MaxPriorityFeePerGas returns a suggestion for a gas tip cap for dynamic fee transactions. func (api *APIImpl) MaxPriorityFeePerGas(ctx context.Context) (*hexutil.Big, error) { - tx, err := api.db.BeginTemporalRo(ctx) + tx, overlay, err := api.beginTxWithOverlay(ctx) if err != nil { return nil, err } defer tx.Rollback() - oracle := api.newGasOracle(tx) + oracle := api.newGasOracleFromBackend(newGasPriceOracleBackendPinned(api.db, tx, api.BaseAPI, overlay)) tipcap, err := oracle.SuggestTipCap(ctx) if err != nil { return nil, err @@ -303,12 +303,12 @@ type feeHistoryResult struct { } func (api *APIImpl) FeeHistory(ctx context.Context, blockCount rpc.DecimalOrHex, lastBlock rpc.BlockNumber, rewardPercentiles []float64) (*feeHistoryResult, error) { - tx, err := api.db.BeginTemporalRo(ctx) + tx, overlay, err := api.beginTxWithOverlay(ctx) if err != nil { return nil, err } defer tx.Rollback() - oracle := api.newGasOracle(tx) + oracle := api.newGasOracleFromBackend(newGasPriceOracleBackendPinned(api.db, tx, api.BaseAPI, overlay)) oldest, reward, baseFee, gasUsed, blobBaseFee, blobGasUsedRatio, err := oracle.FeeHistory(ctx, int(blockCount), lastBlock, rewardPercentiles) if err != nil { @@ -494,6 +494,7 @@ func fillForkConfig(chainConfig *chain.Config, forkId [4]byte, activationTime ui type GasPriceOracleBackend struct { db kv.TemporalRoDB // nil if Fork is not supported + rawTx kv.TemporalTx // the caller's tx before the overlay wrap tx kv.TemporalTx baseApi *BaseAPI overlay *membatchwithdb.MemoryMutation // pinned at construction; nil when no overlay was published @@ -502,13 +503,51 @@ type GasPriceOracleBackend struct { // NewGasPriceOracleBackend pins the block overlay once so the head the oracle // resolves stays readable for the whole request, including on the txs Fork opens. func NewGasPriceOracleBackend(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI) *GasPriceOracleBackend { - b := &GasPriceOracleBackend{db: db, baseApi: baseApi, overlay: baseApi.filters.LatestOverlay()} + return newGasPriceOracleBackendPinned(db, tx, baseApi, baseApi.filters.LatestOverlay()) +} + +func newGasPriceOracleBackendPinned(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI, overlay *membatchwithdb.MemoryMutation) *GasPriceOracleBackend { + b := &GasPriceOracleBackend{db: db, rawTx: tx, baseApi: baseApi, overlay: overlay} b.tx = b.withOverlay(tx) return b } -func (b *GasPriceOracleBackend) withOverlay(tx kv.TemporalTx) kv.TemporalTx { +// beginTxWithOverlay opens a read tx and captures the published block overlay +// as one consistent pair. A commit or (un)publish landing between the two +// steps can leave a head block visible in neither the overlay nor the tx +// snapshot, so on a mid-acquisition overlay change the tx is reopened. +func (api *APIImpl) beginTxWithOverlay(ctx context.Context) (kv.TemporalTx, *membatchwithdb.MemoryMutation, error) { + const maxAttempts = 3 + for attempt := 1; ; attempt++ { + overlay := api.filters.LatestOverlay() + tx, err := api.db.BeginTemporalRo(ctx) //nolint:gocritic + if err != nil { + return nil, nil, err + } + current := api.filters.LatestOverlay() + if current == overlay || attempt == maxAttempts { + return tx, current, nil + } + tx.Rollback() + } +} + +// CacheableBlockLimit resolves the committed head from the unwrapped tx: +// blocks past it live only in the overlay and stay uncacheable until their +// commit lands. Only the fee-history path pays for the resolution. +func (b *GasPriceOracleBackend) CacheableBlockLimit() uint64 { if b.overlay == nil { + return math.MaxUint64 + } + committedHead, err := rpchelper.GetLatestBlockNumber(b.rawTx) + if err != nil { + return 0 + } + return committedHead +} + +func (b *GasPriceOracleBackend) withOverlay(tx kv.TemporalTx) kv.TemporalTx { + if b.overlay == nil || membatchwithdb.CarriesOverlayView(tx) { return tx } return b.overlay.NewReadView(tx) @@ -524,7 +563,7 @@ func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBacken } // Reuse the pinned overlay instead of re-resolving: it may have been // unpublished since the request started. - return &GasPriceOracleBackend{db: b.db, tx: b.withOverlay(tx), baseApi: b.baseApi, overlay: b.overlay}, + return &GasPriceOracleBackend{db: b.db, rawTx: tx, tx: b.withOverlay(tx), baseApi: b.baseApi, overlay: b.overlay}, func() { tx.Rollback() }, nil } diff --git a/rpc/jsonrpc/overlay_race_test.go b/rpc/jsonrpc/overlay_race_test.go index 7791ba019b8..44d750c657f 100644 --- a/rpc/jsonrpc/overlay_race_test.go +++ b/rpc/jsonrpc/overlay_race_test.go @@ -21,6 +21,7 @@ import ( "context" "math/big" "strconv" + "sync" "testing" "github.com/holiman/uint256" @@ -30,6 +31,7 @@ import ( "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/hexutil" + "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/db/kv/kvcache" "github.com/erigontech/erigon/db/rawdb" "github.com/erigontech/erigon/db/rawdb/rawtemporaldb" @@ -43,6 +45,7 @@ import ( "github.com/erigontech/erigon/node/gointerfaces/txpoolproto" "github.com/erigontech/erigon/node/shards" "github.com/erigontech/erigon/rpc" + "github.com/erigontech/erigon/rpc/gasprice" "github.com/erigontech/erigon/rpc/rpccfg" "github.com/erigontech/erigon/rpc/rpchelper" ) @@ -54,11 +57,28 @@ const ( overlayRaceHighTip = 2_000_000 ) +// writeHeadBlockMarkers writes the minimal subset of what InsertBlocks and +// updateForkChoice persist for a new head — header, canonical marker, body, +// head-header and forkchoice markers. The forkchoice marker is what +// rpchelper.GetLatestBlockNumber resolves the head from, so reader paths under +// test (the gas oracle, "latest" tag resolution) see this block as current. +func writeHeadBlockMarkers(t *testing.T, tx kv.RwTx, header *types.Header, body *types.Body) { + t.Helper() + hash := header.Hash() + num := header.Number.Uint64() + require.NoError(t, rawdb.WriteHeader(tx, header)) + require.NoError(t, rawdb.WriteHeadHeaderHash(tx, hash)) + require.NoError(t, rawdb.WriteCanonicalHash(tx, hash, num)) + require.NoError(t, rawdb.WriteBody(tx, hash, num, body)) + rawdb.WriteForkchoiceHead(tx, hash) +} + type overlayAheadHarness struct { base *BaseAPI m *execmoduletester.ExecModuleTester overlayHeader *types.Header events *shards.Events + doms *execctx.SharedDomains } func newOverlayAheadTestAPI(t *testing.T) (base *BaseAPI, m *execmoduletester.ExecModuleTester, overlayHeader *types.Header) { @@ -126,16 +146,7 @@ func newOverlayAheadHarness(t *testing.T, withOverlayTxs bool) *overlayAheadHarn } hash := overlayHeader.Hash() overlay := doms.BlockOverlay() - // Minimal subset of what InsertBlocks/updateForkChoice write in production, - // enough for the reader paths under test to resolve this header as current. - require.NoError(t, rawdb.WriteHeader(overlay, overlayHeader)) - require.NoError(t, rawdb.WriteHeadHeaderHash(overlay, hash)) - rawdb.WriteForkchoiceHead(overlay, hash) - require.NoError(t, rawdb.WriteCanonicalHash(overlay, hash, overlayNumber)) - require.NoError(t, rawdb.WriteBody(overlay, hash, overlayNumber, &types.Body{Transactions: overlayTxs})) - // The forkchoice marker is what rpchelper.GetLatestBlockNumber resolves the head from, - // so readers that go through it (the gas oracle, "latest" tag resolution) see this block. - rawdb.WriteForkchoiceHead(overlay, hash) + writeHeadBlockMarkers(t, overlay, overlayHeader, &types.Body{Transactions: overlayTxs}) if withOverlayTxs { senders := make([]common.Address, len(overlayTxs)) @@ -161,7 +172,7 @@ func newOverlayAheadHarness(t *testing.T, withOverlayTxs bool) *overlayAheadHarn stateCache := kvcache.New(kvcache.DefaultCoherentConfig) base := newBaseApiWithFiltersForTest(filters, stateCache, m) - return &overlayAheadHarness{base: base, m: m, overlayHeader: overlayHeader, events: events} + return &overlayAheadHarness{base: base, m: m, overlayHeader: overlayHeader, events: events, doms: doms} } // overlayRaceTxPoolClient extends stubTxPoolClient with canned replies for @@ -354,6 +365,8 @@ func TestFeeHistory_OverlayHeadWithRewards(t *testing.T) { // resolved when the backend was built: when the overlay is unpublished between the // request start and the fork (the commit window closing), the forked backend must // still serve the head the parent resolved instead of failing with block-not-found. +// The SharedDomains is also closed, as production teardown does right after the +// unpublish, so the test covers reads on a closed-but-pinned overlay. func TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish(t *testing.T) { t.Parallel() h := newOverlayAheadHarness(t, false) @@ -363,6 +376,7 @@ func TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish(t *testing.T) { backend := NewGasPriceOracleBackend(h.m.DB, tx, h.base) h.events.PublishOverlay(nil) + h.doms.Close() forked, cleanup, err := backend.Fork(h.m.Ctx) require.NoError(t, err) @@ -379,3 +393,168 @@ func TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish(t *testing.T) { require.NotNil(t, got) require.Equal(t, h.overlayHeader.Hash(), got.Hash()) } + +// publishSiblingOverlay publishes a second overlay holding a same-height +// sibling of the harness's overlay head, with a different base fee so its +// hash and header are distinguishable from the original. +func publishSiblingOverlay(t *testing.T, h *overlayAheadHarness) *types.Header { + t.Helper() + ctx := h.m.Ctx + roTx, err := h.m.DB.BeginTemporalRo(ctx) + require.NoError(t, err) + t.Cleanup(roTx.Rollback) + doms, err := execctx.NewSharedDomains(ctx, roTx, h.m.Log) + require.NoError(t, err) + t.Cleanup(doms.Close) + require.NoError(t, doms.InitBlockOverlay(roTx, h.m.Dirs.Tmp)) + + sibling := types.CopyHeader(h.overlayHeader) + sibling.BaseFee = uint256.NewInt(overlayRaceBaseFee + 1111) + require.NotEqual(t, h.overlayHeader.Hash(), sibling.Hash()) + + writeHeadBlockMarkers(t, doms.BlockOverlay(), sibling, &types.Body{}) + + h.events.PublishOverlay(doms) + return sibling +} + +// TestGasPriceOracle_PinnedViewIgnoresLaterOverlayPublish pins that a backend +// which resolved overlay A at construction keeps serving A's head even after a +// different overlay B (a same-height sibling, as after an in-RAM reorg) is +// published mid-request: downstream helpers must not layer the live overlay +// over the already-pinned view. +func TestGasPriceOracle_PinnedViewIgnoresLaterOverlayPublish(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + tx, err := h.m.DB.BeginTemporalRo(h.m.Ctx) + require.NoError(t, err) + defer tx.Rollback() + backend := NewGasPriceOracleBackend(h.m.DB, tx, h.base) + + sibling := publishSiblingOverlay(t, h) + + got, err := backend.HeaderByNumber(h.m.Ctx, rpc.LatestBlockNumber) + require.NoError(t, err) + require.NotNil(t, got) + require.NotEqual(t, sibling.Hash(), got.Hash(), + "the pinned request must not pick up the sibling head published after it started") + require.Equal(t, h.overlayHeader.Hash(), got.Hash(), + "the pinned request must keep serving the head it resolved at construction") +} + +// TestFeeHistory_DeadOverlayBlockNotServedFromCache pins that fee data computed +// for a not-yet-committed overlay block does not outlive that block: when a +// same-height sibling replaces it (in-RAM reorg, or the commit failing), a new +// request must serve the sibling's fees, not a memoized result keyed only by +// block number. +func TestFeeHistory_DeadOverlayBlockNotServedFromCache(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + api := newEthApiForTest(h.base, h.m.DB, nil, nil) + + first, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.Equal(t, h.overlayHeader.BaseFee.ToBig(), first.BaseFee[0].ToInt()) + + sibling := publishSiblingOverlay(t, h) + + second, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.Equal(t, sibling.Number.ToBig(), second.OldestBlock.ToInt()) + require.Equal(t, sibling.BaseFee.ToBig(), second.BaseFee[0].ToInt(), + "fees cached for the dead overlay block must not be served for its same-height sibling") +} + +// commitOverlayBlock writes the harness's overlay head into MDBX the way the +// background commit would (header, canonical marker, forkchoice head), so txs +// opened afterwards resolve it as the committed head. +func commitOverlayBlock(t *testing.T, h *overlayAheadHarness) { + t.Helper() + rwTx, err := h.m.DB.BeginRw(h.m.Ctx) + require.NoError(t, err) + defer rwTx.Rollback() + writeHeadBlockMarkers(t, rwTx, h.overlayHeader, &types.Body{}) + require.NoError(t, rwTx.Commit()) +} + +// unpublishOnBeginDB simulates the commit window closing while a request +// acquires its tx: the first BeginTemporalRo returns a tx whose snapshot +// predates the commit, with the commit landing and the overlay being +// unpublished right after the open. Later opens behave normally. +type unpublishOnBeginDB struct { + kv.TemporalRoDB + t *testing.T + h *overlayAheadHarness + once sync.Once +} + +func (db *unpublishOnBeginDB) BeginTemporalRo(ctx context.Context) (kv.TemporalTx, error) { + tx, err := db.TemporalRoDB.BeginTemporalRo(ctx) //nolint:gocritic + if err != nil { + return nil, err + } + db.once.Do(func() { + commitOverlayBlock(db.t, db.h) + db.h.events.PublishOverlay(nil) + }) + return tx, nil +} + +// publishSiblingOnGetCache publishes the sibling overlay the first time the +// gas price oracle consults its cache — after the request has pinned its +// overlay, before the baseFee addend is read. +type publishSiblingOnGetCache struct { + inner gasprice.Cache + t *testing.T + h *overlayAheadHarness + once sync.Once +} + +func (c *publishSiblingOnGetCache) GetLatest() (common.Hash, *uint256.Int) { + c.once.Do(func() { publishSiblingOverlay(c.t, c.h) }) + return c.inner.GetLatest() +} + +func (c *publishSiblingOnGetCache) SetLatest(hash common.Hash, price *uint256.Int) { + c.inner.SetLatest(hash, price) +} + +// TestGasPrice_BaseFeeFromPinnedOverlay pins that eth_gasPrice derives its +// baseFee addend from the same overlay the tip was sampled on: an overlay +// published mid-request (a same-height sibling with a different base fee) +// must not leak into the sum. +func TestGasPrice_BaseFeeFromPinnedOverlay(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + api := newEthApiForTest(h.base, h.m.DB, nil, nil) + + tip, err := api.MaxPriorityFeePerGas(h.m.Ctx) + require.NoError(t, err) + + api.gasCache = &publishSiblingOnGetCache{inner: api.gasCache, t: t, h: h} + + got, err := api.GasPrice(h.m.Ctx) + require.NoError(t, err) + want := new(big.Int).Add(tip.ToInt(), h.overlayHeader.BaseFee.ToBig()) + require.Equal(t, want, got.ToInt(), + "the baseFee addend must come from the pinned overlay head, not one published mid-request") +} + +// TestFeeHistory_HeadCommittedDuringTxAcquisition pins that the overlay must +// be captured atomically with the tx: when the commit lands and the overlay is +// unpublished between the tx open and the overlay capture, the request would +// otherwise see neither layer and serve a head one block behind the one the +// node already published via eth_blockNumber. +func TestFeeHistory_HeadCommittedDuringTxAcquisition(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + db := &unpublishOnBeginDB{TemporalRoDB: h.m.DB, t: t, h: h} + api := newEthApiForTest(h.base, db, nil, nil) + + got, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, h.overlayHeader.Number.ToBig(), got.OldestBlock.ToInt(), + "a commit and unpublish landing during tx acquisition must not hide the head block") + require.Equal(t, h.overlayHeader.BaseFee.ToBig(), got.BaseFee[0].ToInt()) +} From 5ed15a188965e92a8f89c1ee4696d181c1fa8d4d Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Thu, 13 Aug 2026 09:18:19 +0200 Subject: [PATCH 6/9] db, node, rpc: pin gas-oracle requests to one overlay view at acquisition Address the third review round on this PR: - Filters.BeginTemporalRoWithOverlay opens the tx and captures the published overlay as one consistent pair: publishes carry a monotonic sequence number, paired atomically with the SD in Events.OverlaySnapshot, the open retries while the sequence moves, and after three unstable attempts the request fails instead of proceeding on a view where the head block is visible in neither layer. - The pin travels inside the tx: read views expose the overlay they were created from (membatchwithdb.OverlayViewCarrier), a "no overlay" resolution is pinned explicitly (noOverlayView), and PinToOverlay is the single wrap point. NewGasPriceOracleBackend adopts the caller's pin and Fork reuses it, so head resolution, per-block sampling and the parallel fetchers all read one view. - The fee-history cache is keyed by block hash, resolved from the canonical-hash index on the pinned view (OracleBackend.CanonicalHash), so a same-height sibling after a reorg misses by construction; CacheableBlockLimit and the rawTx field are gone. Every behavioral fix is pinned by a regression test that was red before it (overlay_race_test.go); the readView flag removal and the wrap-point deduplication are pure refactors covered by the existing suite. The GetModifiedAccountsByNumber tests keep their coverage through the shared harness after the tuple-shim removal. --- db/kv/membatchwithdb/memory_mutation.go | 70 +++++-- node/shards/events.go | 35 +++- rpc/gasprice/feehistory.go | 37 ++-- rpc/gasprice/gasprice.go | 8 +- rpc/gasprice/gasprice_test.go | 4 +- rpc/jsonrpc/eth_system.go | 77 ++----- rpc/jsonrpc/overlay_race_test.go | 258 ++++++++++++++++++------ rpc/rpchelper/filters.go | 51 ++++- 8 files changed, 376 insertions(+), 164 deletions(-) diff --git a/db/kv/membatchwithdb/memory_mutation.go b/db/kv/membatchwithdb/memory_mutation.go index da60fc5c0d1..0e97cbaa135 100644 --- a/db/kv/membatchwithdb/memory_mutation.go +++ b/db/kv/membatchwithdb/memory_mutation.go @@ -58,7 +58,7 @@ type MemoryMutation struct { db kv.TemporalTx statelessCursors map[string]kv.RwCursor DomainReader DomainReader - readView bool + overlay *MemoryMutation // non-nil marks a read view, pointing at the overlay it was created from } // NewMemoryBatch creates a pure Go in-memory batch with no OS-thread affinity. @@ -1084,18 +1084,60 @@ func (m *MemoryMutation) NewReadView(tx kv.Tx) kv.TemporalTx { return m.newReadViewMut(tx) } -// CarriesOverlayView reports whether tx is already a read view over a block -// overlay. Overlay wrap points skip such txs so the first wrap pins the -// overlay a request reads from: re-wrapping would layer a possibly newer -// overlay on top, mixing two heads within one request. +// OverlayViewCarrier is implemented by txs that are pinned overlay views. +// A tx wrapper that embeds such a tx keeps the marker through method +// promotion, where a concrete-type switch would silently lose it. +type OverlayViewCarrier interface { + // OverlayView returns the overlay the tx was pinned to and whether the + // tx is a pinned view at all. A pinned view with a nil overlay resolved + // "no overlay published" and must keep reading committed data only. + OverlayView() (overlay *MemoryMutation, pinned bool) +} + +// CarriesOverlayView reports whether tx is already a pinned overlay view. +// Overlay wrap points skip such txs so the first wrap pins the overlay a +// request reads from: re-wrapping would layer a possibly newer overlay on +// top, mixing two heads within one request. func CarriesOverlayView(tx kv.Tx) bool { - switch v := tx.(type) { - case *OverlayTemporalReadView: - return true - case *MemoryMutation: - return v.readView + _, ok := ViewOverlay(tx) + return ok +} + +// ViewOverlay returns the overlay tx was pinned to, and whether tx is a +// pinned view at all. +func ViewOverlay(tx kv.Tx) (*MemoryMutation, bool) { + if c, ok := tx.(OverlayViewCarrier); ok { + return c.OverlayView() } - return false + return nil, false +} + +// OverlayView implements OverlayViewCarrier for read views; a MemoryMutation +// that owns its overlay data is not a view and carries no pin. +func (m *MemoryMutation) OverlayView() (*MemoryMutation, bool) { + return m.overlay, m.overlay != nil +} + +// noOverlayView pins a tx to "no overlay": it reads committed data only and +// overlay wrap points leave it alone, so an overlay published mid-request +// cannot leak into the view. +type noOverlayView struct { + kv.TemporalTx +} + +func (v *noOverlayView) OverlayView() (*MemoryMutation, bool) { return nil, true } + +// PinToOverlay pins tx to the given overlay: a read view when overlay is +// non-nil, a no-overlay pin otherwise. Txs already carrying a pinned view +// are returned unchanged. +func PinToOverlay(tx kv.TemporalTx, overlay *MemoryMutation) kv.TemporalTx { + if CarriesOverlayView(tx) { + return tx + } + if overlay == nil { + return &noOverlayView{tx} + } + return overlay.NewReadView(tx) } // newReadViewMut is the internal constructor that returns the full @@ -1105,6 +1147,10 @@ func (m *MemoryMutation) newReadViewMut(tx kv.Tx) *MemoryMutation { if t, ok := tx.(kv.TemporalTx); ok { dbTx = t } + overlay := m + if m.overlay != nil { + overlay = m.overlay + } return &MemoryMutation{ mu: m.mu, // share parent's mutex for synchronization memTx: m.memTx, @@ -1114,7 +1160,7 @@ func (m *MemoryMutation) newReadViewMut(tx kv.Tx) *MemoryMutation { clearedTables: m.clearedTables, db: dbTx, DomainReader: m.DomainReader, - readView: true, + overlay: overlay, } } diff --git a/node/shards/events.go b/node/shards/events.go index c2f3329bb40..be92c8ae887 100644 --- a/node/shards/events.go +++ b/node/shards/events.go @@ -58,9 +58,19 @@ type Events struct { hasReceiptSubscriptions bool lock sync.RWMutex - // latestSD holds the most recently published SharedDomains from FCU. + // latestPub holds the most recently published SharedDomains from FCU + // together with its publish sequence number, as one atomic snapshot so + // readers can never observe the SD and the sequence out of step. // Accessible lock-free for the builder and RPC layer. - latestSD atomic.Pointer[execctx.SharedDomains] + latestPub atomic.Pointer[overlayPub] +} + +// overlayPub pairs a published SharedDomains with a monotonic publish +// sequence number. The first publish gets seq 1, so seq 0 uniquely means +// "nothing was ever published". +type overlayPub struct { + sd *execctx.SharedDomains + seq uint64 } func NewEvents() *Events { @@ -266,9 +276,13 @@ func (e *Events) AddOverlaySubscription() (chan *execctx.SharedDomains, func()) // PublishOverlay sends the SharedDomains to all in-process subscribers. // The SD is shared read-only; the background commit goroutine owns its lifecycle. func (e *Events) PublishOverlay(sd *execctx.SharedDomains) { - e.latestSD.Store(sd) e.lock.Lock() defer e.lock.Unlock() + seq := uint64(1) + if prev := e.latestPub.Load(); prev != nil { + seq = prev.seq + 1 + } + e.latestPub.Store(&overlayPub{sd: sd, seq: seq}) for _, ch := range e.overlaySubscriptions { common.PrioritizedSend(ch, sd) } @@ -276,7 +290,20 @@ func (e *Events) PublishOverlay(sd *execctx.SharedDomains) { // LatestSD returns the most recently published SharedDomains, or nil. func (e *Events) LatestSD() *execctx.SharedDomains { - return e.latestSD.Load() + sd, _ := e.OverlaySnapshot() + return sd +} + +// OverlaySnapshot returns the published SharedDomains together with its +// publish sequence number as one coherent pair. Comparing the sequence around +// a tx open detects any publish landing in between — including a +// publish/unpublish cycle that leaves the SD pointer unchanged. +func (e *Events) OverlaySnapshot() (*execctx.SharedDomains, uint64) { + p := e.latestPub.Load() + if p == nil { + return nil, 0 + } + return p.sd, p.seq } func (e *Events) OnNewPendingLogs(logs types.Logs) { diff --git a/rpc/gasprice/feehistory.go b/rpc/gasprice/feehistory.go index c6f0114a7a6..f6247d48b7b 100644 --- a/rpc/gasprice/feehistory.go +++ b/rpc/gasprice/feehistory.go @@ -57,11 +57,14 @@ const ( maxBlockFetchers = 4 ) -// cacheKey identifies a processed block in the fee history cache. +// cacheKey identifies a processed block in the fee history cache. Keying by +// header hash instead of number makes an entry live exactly as long as the +// block itself: a same-height sibling (reorg, or an in-flight block replaced +// before its commit lands) has a different hash and misses. // The percentiles string is a binary encoding of the requested percentile slice, // so identical percentile arrays produce the same key. type cacheKey struct { - number uint64 + hash common.Hash percentiles string } @@ -345,10 +348,6 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast // Pre-fetch chain config once using the main backend (safe: single goroutine). chainconfig := oracle.backend.ChainConfig() - var cacheableUpTo uint64 - if oracle.historyCache != nil { - cacheableUpTo = oracle.backend.CacheableBlockLimit() - } // Launch up to maxBlockFetchers goroutines. Each goroutine opens its own // TemporalTx via Fork so MDBX transactions are never shared across goroutines. @@ -384,16 +383,24 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast } idx := int(blockNumber - oldestBlock) - // Try the LRU cache first. Pending and not-yet-committed blocks are - // excluded from both lookup and store: their number can be re-used by - // a same-height sibling, so an entry keyed by number alone would keep - // serving the dead block's fees. + // The pending block comes from the mining cache and is rebuilt + // continuously, so its results are never memoized. The cache key + // comes from the canonical-hash index — one cheap lookup, no + // header fetch and no keccak on the hit path. isPending := pendingBlock != nil && blockNumber >= pendingBlock.NumberU64() - cacheable := !isPending && oracle.historyCache != nil && blockNumber <= cacheableUpTo + cacheable := !isPending && oracle.historyCache != nil + var blockHash common.Hash if cacheable { - if cached, ok := oracle.historyCache.get(cacheKey{blockNumber, percentileKey}); ok { - blockResults[idx] = blockResult{processed: cached, hasResult: true} - continue + hash, ok, err := localBackend.CanonicalHash(fetchCtx, blockNumber) + if err != nil { + return err + } + blockHash, cacheable = hash, ok + if ok { + if cached, ok := oracle.historyCache.get(cacheKey{blockHash, percentileKey}); ok { + blockResults[idx] = blockResult{processed: cached, hasResult: true} + continue + } } } @@ -429,7 +436,7 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast blockResults[idx] = blockResult{processed: fees.results, hasResult: true} if cacheable { - oracle.historyCache.add(cacheKey{blockNumber, percentileKey}, fees.results) + oracle.historyCache.add(cacheKey{blockHash, percentileKey}, fees.results) } } }) diff --git a/rpc/gasprice/gasprice.go b/rpc/gasprice/gasprice.go index 5fd649d8431..a0a05882ab0 100644 --- a/rpc/gasprice/gasprice.go +++ b/rpc/gasprice/gasprice.go @@ -47,11 +47,9 @@ type OracleBackend interface { GetReceiptsGasUsed(ctx context.Context, block *types.Block) (types.Receipts, error) PendingBlockAndReceipts() (*types.Block, types.Receipts) - // CacheableBlockLimit returns the highest block number whose per-block fee - // data may be memoized across requests. Blocks above it are not durably - // committed yet and can still be replaced by a same-height sibling, so - // their results must not outlive the request. - CacheableBlockLimit() uint64 + // CanonicalHash returns the canonical block hash at the given height on + // the backend's view, or ok=false when the height is beyond the head. + CanonicalHash(ctx context.Context, number uint64) (common.Hash, bool, error) // Fork opens a new TemporalTx and returns a goroutine-local backend together // with a cleanup function (call via defer cleanup()). diff --git a/rpc/gasprice/gasprice_test.go b/rpc/gasprice/gasprice_test.go index dfad61b76b4..ff2b4b9f626 100644 --- a/rpc/gasprice/gasprice_test.go +++ b/rpc/gasprice/gasprice_test.go @@ -300,7 +300,9 @@ func (m *mockOracleBackend) PendingBlockAndReceipts() (*types.Block, types.Recei return nil, nil } -func (m *mockOracleBackend) CacheableBlockLimit() uint64 { return math.MaxUint64 } +func (m *mockOracleBackend) CanonicalHash(_ context.Context, _ uint64) (common.Hash, bool, error) { + return m.head.Hash(), true, nil +} func (m *mockOracleBackend) Fork(_ context.Context) (gasprice.OracleBackend, func(), error) { return nil, nil, nil // sequential mode diff --git a/rpc/jsonrpc/eth_system.go b/rpc/jsonrpc/eth_system.go index 107a2b80d7b..c6241b4594c 100644 --- a/rpc/jsonrpc/eth_system.go +++ b/rpc/jsonrpc/eth_system.go @@ -258,20 +258,19 @@ func (api *APIImpl) ProtocolVersion(ctx context.Context) (hexutil.Uint, error) { // GasPrice implements eth_gasPrice. Returns the current price per gas in wei. func (api *APIImpl) GasPrice(ctx context.Context) (*hexutil.Big, error) { - tx, overlay, err := api.beginTxWithOverlay(ctx) + pinnedTx, tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return nil, err } defer tx.Rollback() - backend := newGasPriceOracleBackendPinned(api.db, tx, api.BaseAPI, overlay) - oracle := api.newGasOracleFromBackend(backend) + oracle := api.newGasOracleFromBackend(NewGasPriceOracleBackend(api.db, pinnedTx, api.BaseAPI)) tipcap, err := oracle.SuggestTipCap(ctx) if err != nil { return nil, err } gasResult := uint256.NewInt(0) gasResult.Set(tipcap) - if head := rawdb.ReadCurrentHeader(backend.tx); head != nil && head.BaseFee != nil { + if head := rawdb.ReadCurrentHeader(pinnedTx); head != nil && head.BaseFee != nil { gasResult.Add(tipcap, head.BaseFee) } @@ -280,12 +279,12 @@ func (api *APIImpl) GasPrice(ctx context.Context) (*hexutil.Big, error) { // MaxPriorityFeePerGas returns a suggestion for a gas tip cap for dynamic fee transactions. func (api *APIImpl) MaxPriorityFeePerGas(ctx context.Context) (*hexutil.Big, error) { - tx, overlay, err := api.beginTxWithOverlay(ctx) + pinnedTx, tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return nil, err } defer tx.Rollback() - oracle := api.newGasOracleFromBackend(newGasPriceOracleBackendPinned(api.db, tx, api.BaseAPI, overlay)) + oracle := api.newGasOracleFromBackend(NewGasPriceOracleBackend(api.db, pinnedTx, api.BaseAPI)) tipcap, err := oracle.SuggestTipCap(ctx) if err != nil { return nil, err @@ -303,12 +302,12 @@ type feeHistoryResult struct { } func (api *APIImpl) FeeHistory(ctx context.Context, blockCount rpc.DecimalOrHex, lastBlock rpc.BlockNumber, rewardPercentiles []float64) (*feeHistoryResult, error) { - tx, overlay, err := api.beginTxWithOverlay(ctx) + pinnedTx, tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return nil, err } defer tx.Rollback() - oracle := api.newGasOracleFromBackend(newGasPriceOracleBackendPinned(api.db, tx, api.BaseAPI, overlay)) + oracle := api.newGasOracleFromBackend(NewGasPriceOracleBackend(api.db, pinnedTx, api.BaseAPI)) oldest, reward, baseFee, gasUsed, blobBaseFee, blobGasUsedRatio, err := oracle.FeeHistory(ctx, int(blockCount), lastBlock, rewardPercentiles) if err != nil { @@ -494,63 +493,21 @@ func fillForkConfig(chainConfig *chain.Config, forkId [4]byte, activationTime ui type GasPriceOracleBackend struct { db kv.TemporalRoDB // nil if Fork is not supported - rawTx kv.TemporalTx // the caller's tx before the overlay wrap tx kv.TemporalTx baseApi *BaseAPI overlay *membatchwithdb.MemoryMutation // pinned at construction; nil when no overlay was published } // NewGasPriceOracleBackend pins the block overlay once so the head the oracle -// resolves stays readable for the whole request, including on the txs Fork opens. +// resolves stays readable for the whole request, including on the txs Fork +// opens. A tx that already carries a pinned view keeps its own overlay. func NewGasPriceOracleBackend(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI) *GasPriceOracleBackend { - return newGasPriceOracleBackendPinned(db, tx, baseApi, baseApi.filters.LatestOverlay()) -} - -func newGasPriceOracleBackendPinned(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI, overlay *membatchwithdb.MemoryMutation) *GasPriceOracleBackend { - b := &GasPriceOracleBackend{db: db, rawTx: tx, baseApi: baseApi, overlay: overlay} - b.tx = b.withOverlay(tx) - return b -} - -// beginTxWithOverlay opens a read tx and captures the published block overlay -// as one consistent pair. A commit or (un)publish landing between the two -// steps can leave a head block visible in neither the overlay nor the tx -// snapshot, so on a mid-acquisition overlay change the tx is reopened. -func (api *APIImpl) beginTxWithOverlay(ctx context.Context) (kv.TemporalTx, *membatchwithdb.MemoryMutation, error) { - const maxAttempts = 3 - for attempt := 1; ; attempt++ { - overlay := api.filters.LatestOverlay() - tx, err := api.db.BeginTemporalRo(ctx) //nolint:gocritic - if err != nil { - return nil, nil, err - } - current := api.filters.LatestOverlay() - if current == overlay || attempt == maxAttempts { - return tx, current, nil - } - tx.Rollback() + overlay, pinned := membatchwithdb.ViewOverlay(tx) + if !pinned { + overlay = baseApi.filters.LatestOverlay() + tx = membatchwithdb.PinToOverlay(tx, overlay) } -} - -// CacheableBlockLimit resolves the committed head from the unwrapped tx: -// blocks past it live only in the overlay and stay uncacheable until their -// commit lands. Only the fee-history path pays for the resolution. -func (b *GasPriceOracleBackend) CacheableBlockLimit() uint64 { - if b.overlay == nil { - return math.MaxUint64 - } - committedHead, err := rpchelper.GetLatestBlockNumber(b.rawTx) - if err != nil { - return 0 - } - return committedHead -} - -func (b *GasPriceOracleBackend) withOverlay(tx kv.TemporalTx) kv.TemporalTx { - if b.overlay == nil || membatchwithdb.CarriesOverlayView(tx) { - return tx - } - return b.overlay.NewReadView(tx) + return &GasPriceOracleBackend{db: db, tx: tx, baseApi: baseApi, overlay: overlay} } func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBackend, func(), error) { @@ -563,11 +520,15 @@ func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBacken } // Reuse the pinned overlay instead of re-resolving: it may have been // unpublished since the request started. - return &GasPriceOracleBackend{db: b.db, rawTx: tx, tx: b.withOverlay(tx), baseApi: b.baseApi, overlay: b.overlay}, + return &GasPriceOracleBackend{db: b.db, tx: membatchwithdb.PinToOverlay(tx, b.overlay), baseApi: b.baseApi, overlay: b.overlay}, func() { tx.Rollback() }, nil } +func (b *GasPriceOracleBackend) CanonicalHash(ctx context.Context, number uint64) (common.Hash, bool, error) { + return b.baseApi._blockReader.CanonicalHash(ctx, b.tx, number) +} + func (b *GasPriceOracleBackend) HeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Header, error) { header, err := b.baseApi.headerByNumber(ctx, number, b.tx) if err != nil { diff --git a/rpc/jsonrpc/overlay_race_test.go b/rpc/jsonrpc/overlay_race_test.go index 44d750c657f..572b1433e81 100644 --- a/rpc/jsonrpc/overlay_race_test.go +++ b/rpc/jsonrpc/overlay_race_test.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "math/big" + "slices" "strconv" "sync" "testing" @@ -81,12 +82,6 @@ type overlayAheadHarness struct { doms *execctx.SharedDomains } -func newOverlayAheadTestAPI(t *testing.T) (base *BaseAPI, m *execmoduletester.ExecModuleTester, overlayHeader *types.Header) { - t.Helper() - h := newOverlayAheadHarness(t, false) - return h.base, h.m, h.overlayHeader -} - // newOverlayAheadHarness builds overlayRaceChainSize committed MDBX blocks, // then publishes a fabricated block one past them (overlayRaceChainSize+1) // into the block overlay only, never committed to MDBX. This reproduces the @@ -149,10 +144,7 @@ func newOverlayAheadHarness(t *testing.T, withOverlayTxs bool) *overlayAheadHarn writeHeadBlockMarkers(t, overlay, overlayHeader, &types.Body{Transactions: overlayTxs}) if withOverlayTxs { - senders := make([]common.Address, len(overlayTxs)) - for i := range senders { - senders[i] = m.Address - } + senders := slices.Repeat([]common.Address{m.Address}, len(overlayTxs)) require.NoError(t, rawdb.WriteSenders(overlay, hash, overlayNumber, senders)) // Receipt-domain entries go through the SharedDomains (like execution writes // them), so readers reach them via the view's DomainReader, not the overlay tables. @@ -219,33 +211,33 @@ func marshalOverlayRaceTestTx(t *testing.T, txn types.Transaction) []byte { // overlay head must return that in-flight block, not the last MDBX-committed one. func TestGetBlockByTimestamp_SeesOverlayHead(t *testing.T) { t.Parallel() - base, m, overlayHeader := newOverlayAheadTestAPI(t) - api := NewErigonAPI(base, m.DB, nil) + h := newOverlayAheadHarness(t, false) + api := NewErigonAPI(h.base, h.m.DB, nil) - resp, err := api.GetBlockByTimestamp(m.Ctx, rpc.Timestamp(overlayHeader.Time), false) + resp, err := api.GetBlockByTimestamp(h.m.Ctx, rpc.Timestamp(h.overlayHeader.Time), false) require.NoError(t, err) require.NotNil(t, resp) - require.Equal(t, overlayHeader.Number.ToBig(), resp["number"].(*hexutil.Big).ToInt(), + require.Equal(t, h.overlayHeader.Number.ToBig(), resp["number"].(*hexutil.Big).ToInt(), "must resolve to the overlay head block, not the stale MDBX-committed head") } func TestGetModifiedAccountsByNumber_UsesCommittedStartTag(t *testing.T) { t.Parallel() - base, m, _ := newOverlayAheadTestAPI(t) - api := NewPrivateDebugAPI(base, m.DB, nil, &rpccfg.DebugApiConfig{}) + h := newOverlayAheadHarness(t, false) + api := NewPrivateDebugAPI(h.base, h.m.DB, nil, &rpccfg.DebugApiConfig{}) - result, err := api.GetModifiedAccountsByNumber(m.Ctx, rpc.LatestBlockNumber, nil) + result, err := api.GetModifiedAccountsByNumber(h.m.Ctx, rpc.LatestBlockNumber, nil) require.NoError(t, err) require.NotEmpty(t, result) } func TestGetModifiedAccountsByNumber_UsesCommittedEndTag(t *testing.T) { t.Parallel() - base, m, _ := newOverlayAheadTestAPI(t) - api := NewPrivateDebugAPI(base, m.DB, nil, &rpccfg.DebugApiConfig{}) + h := newOverlayAheadHarness(t, false) + api := NewPrivateDebugAPI(h.base, h.m.DB, nil, &rpccfg.DebugApiConfig{}) latest := rpc.LatestBlockNumber - result, err := api.GetModifiedAccountsByNumber(m.Ctx, rpc.EarliestBlockNumber, &latest) + result, err := api.GetModifiedAccountsByNumber(h.m.Ctx, rpc.EarliestBlockNumber, &latest) require.NoError(t, err) require.NotEmpty(t, result) } @@ -256,18 +248,18 @@ func TestGetModifiedAccountsByNumber_UsesCommittedEndTag(t *testing.T) { // must reflect the overlay head, not the stale MDBX-committed head. func TestGetTransactionByHash_PendingTx_UsesOverlayHead(t *testing.T) { t.Parallel() - base, m, overlayHeader := newOverlayAheadTestAPI(t) + h := newOverlayAheadHarness(t, false) - pendingTxn := signOverlayRaceTestTx(t, m, 1) + pendingTxn := signOverlayRaceTestTx(t, h.m, 1) pool := &overlayRaceTxPoolClient{ transactionsReply: &txpoolproto.TransactionsReply{RlpTxs: [][]byte{marshalOverlayRaceTestTx(t, pendingTxn)}}, } - api := newEthApiForTest(base, m.DB, pool, nil) + api := newEthApiForTest(h.base, h.m.DB, pool, nil) - got, err := api.GetTransactionByHash(m.Ctx, pendingTxn.Hash()) + got, err := api.GetTransactionByHash(h.m.Ctx, pendingTxn.Hash()) require.NoError(t, err) require.NotNil(t, got) - require.Equal(t, overlayHeader.BaseFee.ToBig(), got.GasPrice.ToInt(), + require.Equal(t, h.overlayHeader.BaseFee.ToBig(), got.GasPrice.ToInt(), "pending tx gas price must be derived from the overlay head's base fee, not the stale MDBX head") } @@ -290,15 +282,15 @@ func newOverlayRacePendingPool(t *testing.T, m *execmoduletester.ExecModuleTeste // header through the block overlay, matching TestGetTransactionByHash_PendingTx_UsesOverlayHead. func TestTxPoolContent_UsesOverlayHead(t *testing.T) { t.Parallel() - base, m, overlayHeader := newOverlayAheadTestAPI(t) - pool, txn := newOverlayRacePendingPool(t, m) - api := NewTxPoolAPI(base, m.DB, pool) + h := newOverlayAheadHarness(t, false) + pool, txn := newOverlayRacePendingPool(t, h.m) + api := NewTxPoolAPI(h.base, h.m.DB, pool) - content, err := api.Content(m.Ctx) + content, err := api.Content(h.m.Ctx) require.NoError(t, err) - got := content["pending"][m.Address.Hex()][strconv.FormatUint(txn.GetNonce(), 10)] + got := content["pending"][h.m.Address.Hex()][strconv.FormatUint(txn.GetNonce(), 10)] require.NotNil(t, got) - require.Equal(t, overlayHeader.BaseFee.ToBig(), got.GasPrice.ToInt(), + require.Equal(t, h.overlayHeader.BaseFee.ToBig(), got.GasPrice.ToInt(), "pending tx gas price must be derived from the overlay head's base fee, not the stale MDBX head") } @@ -306,15 +298,15 @@ func TestTxPoolContent_UsesOverlayHead(t *testing.T) { // current header through the block overlay, matching TestTxPoolContent_UsesOverlayHead. func TestTxPoolContentFrom_UsesOverlayHead(t *testing.T) { t.Parallel() - base, m, overlayHeader := newOverlayAheadTestAPI(t) - pool, txn := newOverlayRacePendingPool(t, m) - api := NewTxPoolAPI(base, m.DB, pool) + h := newOverlayAheadHarness(t, false) + pool, txn := newOverlayRacePendingPool(t, h.m) + api := NewTxPoolAPI(h.base, h.m.DB, pool) - content, err := api.ContentFrom(m.Ctx, m.Address) + content, err := api.ContentFrom(h.m.Ctx, h.m.Address) require.NoError(t, err) got := content["pending"][strconv.FormatUint(txn.GetNonce(), 10)] require.NotNil(t, got) - require.Equal(t, overlayHeader.BaseFee.ToBig(), got.GasPrice.ToInt(), + require.Equal(t, h.overlayHeader.BaseFee.ToBig(), got.GasPrice.ToInt(), "pending tx gas price must be derived from the overlay head's base fee, not the stale MDBX head") } @@ -325,16 +317,16 @@ func TestTxPoolContentFrom_UsesOverlayHead(t *testing.T) { // publishes via eth_blockNumber. func TestFeeHistory_SeesOverlayHead(t *testing.T) { t.Parallel() - base, m, overlayHeader := newOverlayAheadTestAPI(t) - api := newEthApiForTest(base, m.DB, nil, nil) + h := newOverlayAheadHarness(t, false) + api := newEthApiForTest(h.base, h.m.DB, nil, nil) - got, err := api.FeeHistory(m.Ctx, 1, rpc.LatestBlockNumber, nil) + got, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) require.NoError(t, err) require.NotNil(t, got) - require.Equal(t, overlayHeader.Number.ToBig(), got.OldestBlock.ToInt(), + require.Equal(t, h.overlayHeader.Number.ToBig(), got.OldestBlock.ToInt(), "the fee history window must end on the overlay head, not the stale MDBX-committed head") require.NotEmpty(t, got.BaseFee) - require.Equal(t, overlayHeader.BaseFee.ToBig(), got.BaseFee[0].ToInt(), + require.Equal(t, h.overlayHeader.BaseFee.ToBig(), got.BaseFee[0].ToInt(), "the first base fee must come from the overlay head's header") } @@ -394,10 +386,9 @@ func TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish(t *testing.T) { require.Equal(t, h.overlayHeader.Hash(), got.Hash()) } -// publishSiblingOverlay publishes a second overlay holding a same-height -// sibling of the harness's overlay head, with a different base fee so its -// hash and header are distinguishable from the original. -func publishSiblingOverlay(t *testing.T, h *overlayAheadHarness) *types.Header { +// publishOverlayHead publishes a fresh overlay whose forkchoice head is the +// given header, with an empty body. +func publishOverlayHead(t *testing.T, h *overlayAheadHarness, head *types.Header) { t.Helper() ctx := h.m.Ctx roTx, err := h.m.DB.BeginTemporalRo(ctx) @@ -408,13 +399,38 @@ func publishSiblingOverlay(t *testing.T, h *overlayAheadHarness) *types.Header { t.Cleanup(doms.Close) require.NoError(t, doms.InitBlockOverlay(roTx, h.m.Dirs.Tmp)) + writeHeadBlockMarkers(t, doms.BlockOverlay(), head, &types.Body{}) + + h.events.PublishOverlay(doms) +} + +// publishSiblingOverlay publishes a second overlay holding a same-height +// sibling of the harness's overlay head, with a different base fee so its +// hash and header are distinguishable from the original. +func publishSiblingOverlay(t *testing.T, h *overlayAheadHarness) *types.Header { + t.Helper() sibling := types.CopyHeader(h.overlayHeader) sibling.BaseFee = uint256.NewInt(overlayRaceBaseFee + 1111) require.NotEqual(t, h.overlayHeader.Hash(), sibling.Hash()) + publishOverlayHead(t, h, sibling) + return sibling +} - writeHeadBlockMarkers(t, doms.BlockOverlay(), sibling, &types.Body{}) - - h.events.PublishOverlay(doms) +// publishCommittedSiblingOverlay publishes an overlay whose forkchoice head is +// a same-height sibling of an already-committed block — an in-RAM reorg below +// the committed head. +func publishCommittedSiblingOverlay(t *testing.T, h *overlayAheadHarness, number uint64) *types.Header { + t.Helper() + roTx, err := h.m.DB.BeginTemporalRo(h.m.Ctx) + require.NoError(t, err) + defer roTx.Rollback() + committed := rawdb.ReadHeaderByNumber(roTx, number) + require.NotNil(t, committed) + + sibling := types.CopyHeader(committed) + sibling.BaseFee = uint256.NewInt(overlayRaceBaseFee + 2222) + require.NotEqual(t, committed.Hash(), sibling.Hash()) + publishOverlayHead(t, h, sibling) return sibling } @@ -465,6 +481,87 @@ func TestFeeHistory_DeadOverlayBlockNotServedFromCache(t *testing.T) { "fees cached for the dead overlay block must not be served for its same-height sibling") } +// TestGasPriceOracle_ForkSharesCallerPinnedOverlay pins that when the caller +// hands the backend a tx already pinned to an overlay (as FillTransaction +// does), Fork wraps its fresh txs with that same overlay: re-capturing the +// live one would let parent and fork resolve two different heads within one +// oracle operation. +func TestGasPriceOracle_ForkSharesCallerPinnedOverlay(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + tx, err := h.m.DB.BeginTemporalRo(h.m.Ctx) + require.NoError(t, err) + defer tx.Rollback() + pinnedTx := h.base.filters.WithTemporalOverlay(tx) + + sibling := publishSiblingOverlay(t, h) + + backend := NewGasPriceOracleBackend(h.m.DB, pinnedTx, h.base) + parentHead, err := backend.HeaderByNumber(h.m.Ctx, rpc.LatestBlockNumber) + require.NoError(t, err) + require.Equal(t, h.overlayHeader.Hash(), parentHead.Hash()) + + forked, cleanup, err := backend.Fork(h.m.Ctx) + require.NoError(t, err) + require.NotNil(t, forked) + defer cleanup() + + forkHead, err := forked.HeaderByNumber(h.m.Ctx, rpc.LatestBlockNumber) + require.NoError(t, err) + require.NotEqual(t, sibling.Hash(), forkHead.Hash(), + "the fork must not re-capture the overlay published after the caller pinned its tx") + require.Equal(t, parentHead.Hash(), forkHead.Hash(), + "parent and fork must resolve the same pinned head") +} + +// TestGasPriceOracle_NilOverlayPinIgnoresLaterPublish pins that a backend built +// while no overlay was published keeps reading only committed data: an overlay +// published mid-request (a same-height sibling of the committed head) must not +// leak in through downstream helpers re-resolving the live overlay. +func TestGasPriceOracle_NilOverlayPinIgnoresLaterPublish(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + commitOverlayBlock(t, h) + h.events.PublishOverlay(nil) + h.doms.Close() + + tx, err := h.m.DB.BeginTemporalRo(h.m.Ctx) + require.NoError(t, err) + defer tx.Rollback() + backend := NewGasPriceOracleBackend(h.m.DB, tx, h.base) + + sibling := publishSiblingOverlay(t, h) + + got, err := backend.HeaderByNumber(h.m.Ctx, rpc.LatestBlockNumber) + require.NoError(t, err) + require.NotNil(t, got) + require.NotEqual(t, sibling.Hash(), got.Hash(), + "the request pinned to no overlay must not pick up one published mid-request") + require.Equal(t, h.overlayHeader.Hash(), got.Hash(), + "the request must keep serving the committed head it resolved at construction") +} + +// TestFeeHistory_ReorgedCommittedBlockNotServedFromCache pins that fee data +// memoized for a committed block stops being served once an overlay reorg +// replaces that height: a block number alone does not identify a block. +func TestFeeHistory_ReorgedCommittedBlockNotServedFromCache(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + api := newEthApiForTest(h.base, h.m.DB, nil, nil) + + first, err := api.FeeHistory(h.m.Ctx, 2, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.Len(t, first.BaseFee, 3) + + sibling := publishCommittedSiblingOverlay(t, h, overlayRaceChainSize) + + second, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.Equal(t, sibling.Number.ToBig(), second.OldestBlock.ToInt()) + require.Equal(t, sibling.BaseFee.ToBig(), second.BaseFee[0].ToInt(), + "fees cached for the reorged-out committed block must not be served for its same-height sibling") +} + // commitOverlayBlock writes the harness's overlay head into MDBX the way the // background commit would (header, canonical marker, forkchoice head), so txs // opened afterwards resolve it as the committed head. @@ -477,29 +574,63 @@ func commitOverlayBlock(t *testing.T, h *overlayAheadHarness) { require.NoError(t, rwTx.Commit()) } -// unpublishOnBeginDB simulates the commit window closing while a request -// acquires its tx: the first BeginTemporalRo returns a tx whose snapshot -// predates the commit, with the commit landing and the overlay being -// unpublished right after the open. Later opens behave normally. -type unpublishOnBeginDB struct { +// beginHookDB runs a hook right after each BeginTemporalRo returns, simulating +// commit/publish activity landing while a request acquires its tx. One-shot +// scenarios pass a sync.OnceFunc-wrapped hook. +type beginHookDB struct { kv.TemporalRoDB - t *testing.T - h *overlayAheadHarness - once sync.Once + hook func() } -func (db *unpublishOnBeginDB) BeginTemporalRo(ctx context.Context) (kv.TemporalTx, error) { +func (db *beginHookDB) BeginTemporalRo(ctx context.Context) (kv.TemporalTx, error) { tx, err := db.TemporalRoDB.BeginTemporalRo(ctx) //nolint:gocritic if err != nil { return nil, err } - db.once.Do(func() { - commitOverlayBlock(db.t, db.h) - db.h.events.PublishOverlay(nil) - }) + db.hook() return tx, nil } +// TestFeeHistory_PublishCycleDuringTxAcquisition pins that overlay-capture +// stability is tracked with a publish sequence number: a full publish/commit/ +// unpublish cycle landing during tx acquisition leaves the overlay nil on both +// sides of the open, so pointer identity alone would accept a tx snapshot that +// predates the commit and hides the head block. +func TestFeeHistory_PublishCycleDuringTxAcquisition(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + h.events.PublishOverlay(nil) + h.doms.Close() + + db := &beginHookDB{TemporalRoDB: h.m.DB, hook: sync.OnceFunc(func() { + publishOverlayHead(t, h, h.overlayHeader) + commitOverlayBlock(t, h) + h.events.PublishOverlay(nil) + })} + api := newEthApiForTest(h.base, db, nil, nil) + + got, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.Equal(t, h.overlayHeader.Number.ToBig(), got.OldestBlock.ToInt(), + "a publish/commit/unpublish cycle during tx acquisition must not hide the head block") + require.Equal(t, h.overlayHeader.BaseFee.ToBig(), got.BaseFee[0].ToInt()) +} + +// TestFeeHistory_OverlayUnstableDuringTxAcquisition pins that when the overlay +// keeps changing across every acquisition attempt (a fresh sibling published +// on every open), the request fails instead of proceeding on a (tx, overlay) +// pair it has just proven inconsistent. +func TestFeeHistory_OverlayUnstableDuringTxAcquisition(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + db := &beginHookDB{TemporalRoDB: h.m.DB, hook: func() { publishSiblingOverlay(t, h) }} + api := newEthApiForTest(h.base, db, nil, nil) + + _, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) + require.Error(t, err, + "a request whose overlay capture never stabilizes must fail, not serve a gapped view") +} + // publishSiblingOnGetCache publishes the sibling overlay the first time the // gas price oracle consults its cache — after the request has pinned its // overlay, before the baseFee addend is read. @@ -548,7 +679,10 @@ func TestGasPrice_BaseFeeFromPinnedOverlay(t *testing.T) { func TestFeeHistory_HeadCommittedDuringTxAcquisition(t *testing.T) { t.Parallel() h := newOverlayAheadHarness(t, false) - db := &unpublishOnBeginDB{TemporalRoDB: h.m.DB, t: t, h: h} + db := &beginHookDB{TemporalRoDB: h.m.DB, hook: sync.OnceFunc(func() { + commitOverlayBlock(t, h) + h.events.PublishOverlay(nil) + })} api := newEthApiForTest(h.base, db, nil, nil) got, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index 90ed4281d14..50c744ca94e 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -1186,6 +1186,49 @@ func (ff *Filters) WithOverlay(tx kv.Tx) kv.Tx { return tx } +// OverlaySnapshot returns the published block overlay together with its +// publish sequence number as one coherent pair, or (nil, 0) in remote mode +// where no overlay is ever published. +func (ff *Filters) OverlaySnapshot() (*membatchwithdb.MemoryMutation, uint64) { + if ff == nil || ff.events == nil { + return nil, 0 + } + sd, seq := ff.events.OverlaySnapshot() + if sd == nil { + return nil, seq + } + return sd.BlockOverlay(), seq +} + +// errOverlayUnstable is returned when the overlay keeps changing across every +// tx acquisition attempt; the client can simply retry the request. +var errOverlayUnstable = errors.New("block overlay changed during every tx acquisition attempt; retry the request") + +// BeginTemporalRoWithOverlay opens a read tx and pins it to the block overlay +// published at that moment, as one consistent pair: a commit or (un)publish +// landing between the overlay capture and the tx open can leave a head block +// visible in neither layer, so the tx is reopened whenever the publish +// sequence number moves around the open. The pinned view is returned for +// reads; the raw tx is returned for Rollback (rolling back a view does not +// release the underlying tx). +func (ff *Filters) BeginTemporalRoWithOverlay(ctx context.Context, db kv.TemporalRoDB) (pinned kv.TemporalTx, raw kv.TemporalTx, err error) { + const maxAttempts = 3 + for attempt := 1; ; attempt++ { + overlay, seq := ff.OverlaySnapshot() + tx, err := db.BeginTemporalRo(ctx) //nolint:gocritic + if err != nil { + return nil, nil, err + } + if _, current := ff.OverlaySnapshot(); current == seq { + return membatchwithdb.PinToOverlay(tx, overlay), tx, nil + } + tx.Rollback() + if attempt == maxAttempts { + return nil, nil, errOverlayUnstable + } + } +} + // LatestOverlay returns the block overlay behind the latest published SD, or nil. // Callers that must keep serving one consistent head across several txs (e.g. a // forked backend) pin this instance instead of re-resolving, which could observe @@ -1204,13 +1247,7 @@ func (ff *Filters) LatestOverlay() *membatchwithdb.MemoryMutation { // WithTemporalOverlay is like WithOverlay but returns kv.TemporalTx directly, // avoiding repeated type assertions at callsites that need temporal access. func (ff *Filters) WithTemporalOverlay(tx kv.TemporalTx) kv.TemporalTx { - if membatchwithdb.CarriesOverlayView(tx) { - return tx - } - if overlay := ff.LatestOverlay(); overlay != nil { - return overlay.NewReadView(tx) - } - return tx + return ff.WithOverlay(tx).(kv.TemporalTx) } func (ff *Filters) incrementMetrics(ft FilterType, protocol SubProtocol) { From 2d8c1e8e2d2c135d489d003247ed9a9551714d47 Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Thu, 13 Aug 2026 22:27:28 +0200 Subject: [PATCH 7/9] db, rpc: harden the overlay pin plumbing after the fourth review round - CanonicalHash resolves on the pinned tx and falls back to the block reader only for the frozen range: the remote reader ignores the caller's tx, which could cache one sibling's fees under another sibling's hash and paid a remote call per block. The cache store also validates the fetched header's hash against the key. - A single rpchelper.PinnedRoTx serves both the overlay and the no-overlay pin: it forwards BlockFilesRoTx, hands itself to Apply callbacks, and its Rollback releases the raw tx. noOverlayView is gone and the request-pinning policy lives next to BeginTemporalRoWithOverlay, with only the generic marker staying in membatchwithdb. - BeginTemporalRoWithOverlay serves the last capture as one pinned view under sustained publish churn instead of returning a client-visible error. - eth_blockNumber, eth_baseFee, eth_blobBaseFee and eth_fillTransaction acquire through the pinned helper, closing the remaining same-file instances of the non-atomic acquisition. - Cleanups: the marker doc states the interface-embedding caveat, LatestOverlay derives from OverlaySnapshot, the gas-oracle handlers reuse newGasOracle and the backend's overlay field is derived from the pinned tx, the mock honors the CanonicalHash contract, and the unreachable view-of-view branch is dropped. Every behavioral fix is pinned by a regression test that was red before it (overlay_race_test.go). --- db/kv/membatchwithdb/memory_mutation.go | 33 +---- rpc/gasprice/feehistory.go | 4 +- rpc/gasprice/gasprice_test.go | 5 +- rpc/jsonrpc/eth_fill_transaction.go | 10 +- rpc/jsonrpc/eth_system.go | 57 ++++---- rpc/jsonrpc/overlay_race_test.go | 164 +++++++++++++++++++++++- rpc/rpchelper/filters.go | 32 ++--- rpc/rpchelper/pinned_tx.go | 74 +++++++++++ 8 files changed, 290 insertions(+), 89 deletions(-) create mode 100644 rpc/rpchelper/pinned_tx.go diff --git a/db/kv/membatchwithdb/memory_mutation.go b/db/kv/membatchwithdb/memory_mutation.go index 0e97cbaa135..ed03c181b34 100644 --- a/db/kv/membatchwithdb/memory_mutation.go +++ b/db/kv/membatchwithdb/memory_mutation.go @@ -1085,8 +1085,9 @@ func (m *MemoryMutation) NewReadView(tx kv.Tx) kv.TemporalTx { } // OverlayViewCarrier is implemented by txs that are pinned overlay views. -// A tx wrapper that embeds such a tx keeps the marker through method -// promotion, where a concrete-type switch would silently lose it. +// A wrapper that embeds a concrete view type keeps the marker through method +// promotion; one that embeds the bare tx interface must forward OverlayView +// explicitly, or the wrap points will treat it as unpinned. type OverlayViewCarrier interface { // OverlayView returns the overlay the tx was pinned to and whether the // tx is a pinned view at all. A pinned view with a nil overlay resolved @@ -1118,28 +1119,6 @@ func (m *MemoryMutation) OverlayView() (*MemoryMutation, bool) { return m.overlay, m.overlay != nil } -// noOverlayView pins a tx to "no overlay": it reads committed data only and -// overlay wrap points leave it alone, so an overlay published mid-request -// cannot leak into the view. -type noOverlayView struct { - kv.TemporalTx -} - -func (v *noOverlayView) OverlayView() (*MemoryMutation, bool) { return nil, true } - -// PinToOverlay pins tx to the given overlay: a read view when overlay is -// non-nil, a no-overlay pin otherwise. Txs already carrying a pinned view -// are returned unchanged. -func PinToOverlay(tx kv.TemporalTx, overlay *MemoryMutation) kv.TemporalTx { - if CarriesOverlayView(tx) { - return tx - } - if overlay == nil { - return &noOverlayView{tx} - } - return overlay.NewReadView(tx) -} - // newReadViewMut is the internal constructor that returns the full // *MemoryMutation. Used by NewTemporalReadView which needs to embed it. func (m *MemoryMutation) newReadViewMut(tx kv.Tx) *MemoryMutation { @@ -1147,10 +1126,6 @@ func (m *MemoryMutation) newReadViewMut(tx kv.Tx) *MemoryMutation { if t, ok := tx.(kv.TemporalTx); ok { dbTx = t } - overlay := m - if m.overlay != nil { - overlay = m.overlay - } return &MemoryMutation{ mu: m.mu, // share parent's mutex for synchronization memTx: m.memTx, @@ -1160,7 +1135,7 @@ func (m *MemoryMutation) newReadViewMut(tx kv.Tx) *MemoryMutation { clearedTables: m.clearedTables, db: dbTx, DomainReader: m.DomainReader, - overlay: overlay, + overlay: m, } } diff --git a/rpc/gasprice/feehistory.go b/rpc/gasprice/feehistory.go index f6247d48b7b..d6bbcfbf69e 100644 --- a/rpc/gasprice/feehistory.go +++ b/rpc/gasprice/feehistory.go @@ -435,7 +435,9 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast } blockResults[idx] = blockResult{processed: fees.results, hasResult: true} - if cacheable { + // Store only when the fetched block is the one the key names, + // so a resolution/fetch divergence can never poison the cache. + if cacheable && fees.header.Hash() == blockHash { oracle.historyCache.add(cacheKey{blockHash, percentileKey}, fees.results) } } diff --git a/rpc/gasprice/gasprice_test.go b/rpc/gasprice/gasprice_test.go index ff2b4b9f626..d0463e978ca 100644 --- a/rpc/gasprice/gasprice_test.go +++ b/rpc/gasprice/gasprice_test.go @@ -300,7 +300,10 @@ func (m *mockOracleBackend) PendingBlockAndReceipts() (*types.Block, types.Recei return nil, nil } -func (m *mockOracleBackend) CanonicalHash(_ context.Context, _ uint64) (common.Hash, bool, error) { +func (m *mockOracleBackend) CanonicalHash(_ context.Context, number uint64) (common.Hash, bool, error) { + if number > m.head.Number.Uint64() { + return common.Hash{}, false, nil + } return m.head.Hash(), true, nil } diff --git a/rpc/jsonrpc/eth_fill_transaction.go b/rpc/jsonrpc/eth_fill_transaction.go index b5012341aa0..c3d6236a57f 100644 --- a/rpc/jsonrpc/eth_fill_transaction.go +++ b/rpc/jsonrpc/eth_fill_transaction.go @@ -50,15 +50,13 @@ func (api *APIImpl) FillTransaction(ctx context.Context, args ethapi.CallArgs) ( return nil, errors.New("maxFeePerBlobGas, if specified, must be non-zero") } - dbTx, err := api.db.BeginTemporalRo(ctx) + // The pinned view keeps ReadCurrentHeader and the gas oracle on one head + // for the whole request, including the in-flight overlay block. + overlayTx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return nil, err } - defer dbTx.Rollback() - - // Use the overlay so ReadCurrentHeader and the gas oracle see the latest - // in-flight block (which may not yet be committed to MDBX). - overlayTx := api.filters.WithTemporalOverlay(dbTx) + defer overlayTx.Rollback() cc, err := api.chainConfig(ctx, overlayTx) if err != nil { diff --git a/rpc/jsonrpc/eth_system.go b/rpc/jsonrpc/eth_system.go index c6241b4594c..31c492cb6a7 100644 --- a/rpc/jsonrpc/eth_system.go +++ b/rpc/jsonrpc/eth_system.go @@ -193,12 +193,12 @@ func (api *APIImpl) Capabilities(ctx context.Context) (*CapabilitiesResult, erro // BlockNumber implements eth_blockNumber. Returns the block number of most recent block. func (api *APIImpl) BlockNumber(ctx context.Context) (hexutil.Uint64, error) { - tx, err := api.db.BeginTemporalRo(ctx) + tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return 0, err } defer tx.Rollback() - blockNum, err := rpchelper.GetLatestBlockNumber(api.filters.WithOverlay(tx)) + blockNum, err := rpchelper.GetLatestBlockNumber(tx) if err != nil { return 0, err } @@ -258,19 +258,19 @@ func (api *APIImpl) ProtocolVersion(ctx context.Context) (hexutil.Uint, error) { // GasPrice implements eth_gasPrice. Returns the current price per gas in wei. func (api *APIImpl) GasPrice(ctx context.Context) (*hexutil.Big, error) { - pinnedTx, tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) + tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return nil, err } defer tx.Rollback() - oracle := api.newGasOracleFromBackend(NewGasPriceOracleBackend(api.db, pinnedTx, api.BaseAPI)) + oracle := api.newGasOracle(tx) tipcap, err := oracle.SuggestTipCap(ctx) if err != nil { return nil, err } gasResult := uint256.NewInt(0) gasResult.Set(tipcap) - if head := rawdb.ReadCurrentHeader(pinnedTx); head != nil && head.BaseFee != nil { + if head := rawdb.ReadCurrentHeader(tx); head != nil && head.BaseFee != nil { gasResult.Add(tipcap, head.BaseFee) } @@ -279,12 +279,12 @@ func (api *APIImpl) GasPrice(ctx context.Context) (*hexutil.Big, error) { // MaxPriorityFeePerGas returns a suggestion for a gas tip cap for dynamic fee transactions. func (api *APIImpl) MaxPriorityFeePerGas(ctx context.Context) (*hexutil.Big, error) { - pinnedTx, tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) + tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return nil, err } defer tx.Rollback() - oracle := api.newGasOracleFromBackend(NewGasPriceOracleBackend(api.db, pinnedTx, api.BaseAPI)) + oracle := api.newGasOracle(tx) tipcap, err := oracle.SuggestTipCap(ctx) if err != nil { return nil, err @@ -302,12 +302,12 @@ type feeHistoryResult struct { } func (api *APIImpl) FeeHistory(ctx context.Context, blockCount rpc.DecimalOrHex, lastBlock rpc.BlockNumber, rewardPercentiles []float64) (*feeHistoryResult, error) { - pinnedTx, tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) + tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return nil, err } defer tx.Rollback() - oracle := api.newGasOracleFromBackend(NewGasPriceOracleBackend(api.db, pinnedTx, api.BaseAPI)) + oracle := api.newGasOracle(tx) oldest, reward, baseFee, gasUsed, blobBaseFee, blobGasUsedRatio, err := oracle.FeeHistory(ctx, int(blockCount), lastBlock, rewardPercentiles) if err != nil { @@ -346,13 +346,12 @@ func (api *APIImpl) FeeHistory(ctx context.Context, blockCount rpc.DecimalOrHex, // BlobBaseFee returns the base fee for blob gas at the current head. func (api *APIImpl) BlobBaseFee(ctx context.Context) (*hexutil.Big, error) { - tx, err := api.db.BeginTemporalRo(ctx) + tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return nil, err } defer tx.Rollback() - overlayTx := api.filters.WithTemporalOverlay(tx) - header := rawdb.ReadCurrentHeader(overlayTx) + header := rawdb.ReadCurrentHeader(tx) if header == nil || header.ExcessBlobGas == nil { return nil, nil } @@ -373,13 +372,12 @@ func (api *APIImpl) BlobBaseFee(ctx context.Context) (*hexutil.Big, error) { // BaseFee returns the base fee at the current head. func (api *APIImpl) BaseFee(ctx context.Context) (*hexutil.Big, error) { - tx, err := api.db.BeginTemporalRo(ctx) + tx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return nil, err } defer tx.Rollback() - overlayTx := api.filters.WithTemporalOverlay(tx) - header := rawdb.ReadCurrentHeader(overlayTx) + header := rawdb.ReadCurrentHeader(tx) if header == nil { return nil, nil } @@ -493,21 +491,18 @@ func fillForkConfig(chainConfig *chain.Config, forkId [4]byte, activationTime ui type GasPriceOracleBackend struct { db kv.TemporalRoDB // nil if Fork is not supported - tx kv.TemporalTx + tx kv.TemporalTx // always a pinned view; carries the request's overlay resolution baseApi *BaseAPI - overlay *membatchwithdb.MemoryMutation // pinned at construction; nil when no overlay was published } // NewGasPriceOracleBackend pins the block overlay once so the head the oracle // resolves stays readable for the whole request, including on the txs Fork // opens. A tx that already carries a pinned view keeps its own overlay. func NewGasPriceOracleBackend(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI) *GasPriceOracleBackend { - overlay, pinned := membatchwithdb.ViewOverlay(tx) - if !pinned { - overlay = baseApi.filters.LatestOverlay() - tx = membatchwithdb.PinToOverlay(tx, overlay) + if !membatchwithdb.CarriesOverlayView(tx) { + tx = rpchelper.PinToOverlay(tx, baseApi.filters.LatestOverlay()) } - return &GasPriceOracleBackend{db: db, tx: tx, baseApi: baseApi, overlay: overlay} + return &GasPriceOracleBackend{db: db, tx: tx, baseApi: baseApi} } func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBackend, func(), error) { @@ -518,14 +513,26 @@ func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBacken if err != nil { return nil, nil, err } - // Reuse the pinned overlay instead of re-resolving: it may have been - // unpublished since the request started. - return &GasPriceOracleBackend{db: b.db, tx: membatchwithdb.PinToOverlay(tx, b.overlay), baseApi: b.baseApi, overlay: b.overlay}, + // Reuse the parent's pinned overlay instead of re-resolving: it may have + // been unpublished since the request started. + overlay, _ := membatchwithdb.ViewOverlay(b.tx) + return &GasPriceOracleBackend{db: b.db, tx: rpchelper.PinToOverlay(tx, overlay), baseApi: b.baseApi}, func() { tx.Rollback() }, nil } +// CanonicalHash resolves on the pinned tx first: the block reader may resolve +// on a live service (rpcdaemon mode), which would un-pin the fee-history cache +// key. The reader is only the fallback for frozen blocks, whose canonical +// mapping is immutable. func (b *GasPriceOracleBackend) CanonicalHash(ctx context.Context, number uint64) (common.Hash, bool, error) { + hash, err := rawdb.ReadCanonicalHash(b.tx, number) + if err != nil { + return common.Hash{}, false, err + } + if hash != (common.Hash{}) { + return hash, true, nil + } return b.baseApi._blockReader.CanonicalHash(ctx, b.tx, number) } diff --git a/rpc/jsonrpc/overlay_race_test.go b/rpc/jsonrpc/overlay_race_test.go index 572b1433e81..40d5a15015c 100644 --- a/rpc/jsonrpc/overlay_race_test.go +++ b/rpc/jsonrpc/overlay_race_test.go @@ -32,8 +32,10 @@ import ( "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/hexutil" + "github.com/erigontech/erigon/db/dbservices" "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/db/kv/kvcache" + "github.com/erigontech/erigon/db/kv/membatchwithdb" "github.com/erigontech/erigon/db/rawdb" "github.com/erigontech/erigon/db/rawdb/rawtemporaldb" "github.com/erigontech/erigon/db/state/execctx" @@ -46,6 +48,7 @@ import ( "github.com/erigontech/erigon/node/gointerfaces/txpoolproto" "github.com/erigontech/erigon/node/shards" "github.com/erigontech/erigon/rpc" + "github.com/erigontech/erigon/rpc/ethapi" "github.com/erigontech/erigon/rpc/gasprice" "github.com/erigontech/erigon/rpc/rpccfg" "github.com/erigontech/erigon/rpc/rpchelper" @@ -541,6 +544,78 @@ func TestGasPriceOracle_NilOverlayPinIgnoresLaterPublish(t *testing.T) { "the request must keep serving the committed head it resolved at construction") } +// TestBeginTemporalRoWithOverlay_PreservesOptionalInterfaces pins that the +// pinned handle keeps the tx-scoped block-files view and hands itself — not +// the raw tx — to Apply callbacks, in both the overlay and no-overlay cases: +// dropping either silently degrades every read (per-read view acquisition, +// snapshot-merge straddling) or unpins downstream code. +func TestBeginTemporalRoWithOverlay_PreservesOptionalInterfaces(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + + check := func(t *testing.T) { + pinnedTx, err := h.base.filters.BeginTemporalRoWithOverlay(h.m.Ctx, h.m.DB) + require.NoError(t, err) + defer pinnedTx.Rollback() + + bf, ok := pinnedTx.(membatchwithdb.HasBlockFilesRoTx) + require.True(t, ok, "the pinned handle must keep the tx-scoped block-files view") + require.NotNil(t, bf.BlockFilesRoTx()) + + require.NoError(t, pinnedTx.Apply(h.m.Ctx, func(inner kv.Tx) error { + require.True(t, membatchwithdb.CarriesOverlayView(inner), + "Apply must hand the pinned view to the callback, not the raw tx") + return nil + })) + } + + t.Run("overlay published", check) + h.events.PublishOverlay(nil) + h.doms.Close() + t.Run("no overlay", check) +} + +// remoteishBlockReader simulates the rpcdaemon RemoteBlockReader: CanonicalHash +// ignores the caller's tx and resolves on the live view instead. +type remoteishBlockReader struct { + dbservices.FullBlockReader + h *overlayAheadHarness +} + +func (r *remoteishBlockReader) CanonicalHash(ctx context.Context, _ kv.Getter, blockNum uint64) (common.Hash, bool, error) { + tx, err := r.h.m.DB.BeginTemporalRo(ctx) + if err != nil { + return common.Hash{}, false, err + } + defer tx.Rollback() + return r.FullBlockReader.CanonicalHash(ctx, r.h.base.filters.WithTemporalOverlay(tx), blockNum) +} + +// TestGasPriceOracle_CanonicalHashUsesPinnedView pins that the fee-history +// cache key resolves on the pinned view even when the block reader ignores +// the caller's tx (as the rpcdaemon RemoteBlockReader does): a sibling +// published mid-request must not swap the hash under the pinned head. +func TestGasPriceOracle_CanonicalHashUsesPinnedView(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + h.base._blockReader = &remoteishBlockReader{FullBlockReader: h.m.BlockReader, h: h} + + tx, err := h.m.DB.BeginTemporalRo(h.m.Ctx) + require.NoError(t, err) + defer tx.Rollback() + backend := NewGasPriceOracleBackend(h.m.DB, h.base.filters.WithTemporalOverlay(tx), h.base) + + sibling := publishSiblingOverlay(t, h) + + hash, ok, err := backend.CanonicalHash(h.m.Ctx, h.overlayHeader.Number.Uint64()) + require.NoError(t, err) + require.True(t, ok) + require.NotEqual(t, sibling.Hash(), hash, + "the cache key must not come from the live view the reader resolves on") + require.Equal(t, h.overlayHeader.Hash(), hash, + "the cache key must resolve on the pinned view") +} + // TestFeeHistory_ReorgedCommittedBlockNotServedFromCache pins that fee data // memoized for a committed block stops being served once an overlay reorg // replaces that height: a block number alone does not identify a block. @@ -618,17 +693,21 @@ func TestFeeHistory_PublishCycleDuringTxAcquisition(t *testing.T) { // TestFeeHistory_OverlayUnstableDuringTxAcquisition pins that when the overlay // keeps changing across every acquisition attempt (a fresh sibling published -// on every open), the request fails instead of proceeding on a (tx, overlay) -// pair it has just proven inconsistent. +// on every open), the request still serves the last capture as one pinned +// view instead of failing: under FCU churn a slightly stale answer beats a +// client-visible error. func TestFeeHistory_OverlayUnstableDuringTxAcquisition(t *testing.T) { t.Parallel() h := newOverlayAheadHarness(t, false) db := &beginHookDB{TemporalRoDB: h.m.DB, hook: func() { publishSiblingOverlay(t, h) }} api := newEthApiForTest(h.base, db, nil, nil) - _, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) - require.Error(t, err, - "a request whose overlay capture never stabilizes must fail, not serve a gapped view") + got, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) + require.NoError(t, err, + "a request whose overlay capture never stabilizes must serve the last capture, not fail") + require.Equal(t, h.overlayHeader.Number.ToBig(), got.OldestBlock.ToInt()) + require.Equal(t, big.NewInt(overlayRaceBaseFee+1111), got.BaseFee[0].ToInt(), + "the window must come from one pinned sibling view") } // publishSiblingOnGetCache publishes the sibling overlay the first time the @@ -671,6 +750,81 @@ func TestGasPrice_BaseFeeFromPinnedOverlay(t *testing.T) { "the baseFee addend must come from the pinned overlay head, not one published mid-request") } +// TestBlockNumber_PublishCycleDuringTxAcquisition pins that eth_blockNumber +// acquires (tx, overlay) atomically like the fee endpoints: a publish/commit/ +// unpublish cycle landing during the open must not hide the head block. +func TestBlockNumber_PublishCycleDuringTxAcquisition(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + h.events.PublishOverlay(nil) + h.doms.Close() + + db := &beginHookDB{TemporalRoDB: h.m.DB, hook: sync.OnceFunc(func() { + publishOverlayHead(t, h, h.overlayHeader) + commitOverlayBlock(t, h) + h.events.PublishOverlay(nil) + })} + api := newEthApiForTest(h.base, db, nil, nil) + + got, err := api.BlockNumber(h.m.Ctx) + require.NoError(t, err) + require.Equal(t, hexutil.Uint64(h.overlayHeader.Number.Uint64()), got, + "a publish/commit/unpublish cycle during tx acquisition must not hide the head block") +} + +// TestBaseFee_PublishCycleDuringTxAcquisition pins the same atomic acquisition +// for eth_baseFee: the next-block base fee must derive from the real head. +func TestBaseFee_PublishCycleDuringTxAcquisition(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + h.events.PublishOverlay(nil) + h.doms.Close() + + db := &beginHookDB{TemporalRoDB: h.m.DB, hook: sync.OnceFunc(func() { + publishOverlayHead(t, h, h.overlayHeader) + commitOverlayBlock(t, h) + h.events.PublishOverlay(nil) + })} + api := newEthApiForTest(h.base, db, nil, nil) + + got, err := api.BaseFee(h.m.Ctx) + require.NoError(t, err) + // the overlay head's GasUsed sits exactly on target, so the next base fee + // equals its own — the deterministic fingerprint for "derived from head 6" + require.Equal(t, h.overlayHeader.BaseFee.ToBig(), got.ToInt(), + "the next-block base fee must derive from the head the cycle committed") +} + +// TestFillTransaction_PublishCycleDuringTxAcquisition pins that fee defaults +// price on the head resolved by an atomic acquisition: maxFeePerGas embeds +// 2×baseFee of the head, so a publish/commit/unpublish cycle landing during +// the open must not leave it derived from the previous head. +func TestFillTransaction_PublishCycleDuringTxAcquisition(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + h.events.PublishOverlay(nil) + h.doms.Close() + + db := &beginHookDB{TemporalRoDB: h.m.DB, hook: sync.OnceFunc(func() { + publishOverlayHead(t, h, h.overlayHeader) + commitOverlayBlock(t, h) + h.events.PublishOverlay(nil) + })} + api := newEthApiForTest(h.base, db, stubTxPoolClient{}, nil) + + to := common.HexToAddress("0x0d3ab14bbad3d99f4203bd7a11acb94882050e7e") + gas := hexutil.Uint64(21000) + nonce := hexutil.Uint64(0) + result, err := api.FillTransaction(h.m.Ctx, ethapi.CallArgs{From: &h.m.Address, To: &to, Gas: &gas, Nonce: &nonce}) + require.NoError(t, err) + require.NotNil(t, result.Tx.MaxFeePerGas) + require.NotNil(t, result.Tx.MaxPriorityFeePerGas) + + baseFeeComponent := new(big.Int).Sub(result.Tx.MaxFeePerGas.ToInt(), result.Tx.MaxPriorityFeePerGas.ToInt()) + require.Equal(t, new(big.Int).Lsh(h.overlayHeader.BaseFee.ToBig(), 1), baseFeeComponent, + "maxFeePerGas must embed 2×baseFee of the head the cycle committed") +} + // TestFeeHistory_HeadCommittedDuringTxAcquisition pins that the overlay must // be captured atomically with the tx: when the commit lands and the overlay is // unpublished between the tx open and the overlay capture, the request would diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index 50c744ca94e..5e7dbe3d70d 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -1200,32 +1200,26 @@ func (ff *Filters) OverlaySnapshot() (*membatchwithdb.MemoryMutation, uint64) { return sd.BlockOverlay(), seq } -// errOverlayUnstable is returned when the overlay keeps changing across every -// tx acquisition attempt; the client can simply retry the request. -var errOverlayUnstable = errors.New("block overlay changed during every tx acquisition attempt; retry the request") - // BeginTemporalRoWithOverlay opens a read tx and pins it to the block overlay // published at that moment, as one consistent pair: a commit or (un)publish // landing between the overlay capture and the tx open can leave a head block // visible in neither layer, so the tx is reopened whenever the publish -// sequence number moves around the open. The pinned view is returned for -// reads; the raw tx is returned for Rollback (rolling back a view does not -// release the underlying tx). -func (ff *Filters) BeginTemporalRoWithOverlay(ctx context.Context, db kv.TemporalRoDB) (pinned kv.TemporalTx, raw kv.TemporalTx, err error) { +// sequence number moves around the open. Under sustained publish churn the +// last capture is served anyway — a slightly stale pinned view beats a +// client-visible error. The returned handle reads through the pinned view +// and its Rollback releases the underlying tx. +func (ff *Filters) BeginTemporalRoWithOverlay(ctx context.Context, db kv.TemporalRoDB) (kv.TemporalTx, error) { const maxAttempts = 3 for attempt := 1; ; attempt++ { overlay, seq := ff.OverlaySnapshot() tx, err := db.BeginTemporalRo(ctx) //nolint:gocritic if err != nil { - return nil, nil, err + return nil, err } - if _, current := ff.OverlaySnapshot(); current == seq { - return membatchwithdb.PinToOverlay(tx, overlay), tx, nil + if _, current := ff.OverlaySnapshot(); current == seq || attempt == maxAttempts { + return PinToOverlay(tx, overlay), nil } tx.Rollback() - if attempt == maxAttempts { - return nil, nil, errOverlayUnstable - } } } @@ -1234,14 +1228,8 @@ func (ff *Filters) BeginTemporalRoWithOverlay(ctx context.Context, db kv.Tempora // forked backend) pin this instance instead of re-resolving, which could observe // the overlay being unpublished mid-request. func (ff *Filters) LatestOverlay() *membatchwithdb.MemoryMutation { - if ff == nil { - return nil - } - sd := ff.LatestSD() - if sd == nil { - return nil - } - return sd.BlockOverlay() + overlay, _ := ff.OverlaySnapshot() + return overlay } // WithTemporalOverlay is like WithOverlay but returns kv.TemporalTx directly, diff --git a/rpc/rpchelper/pinned_tx.go b/rpc/rpchelper/pinned_tx.go new file mode 100644 index 00000000000..63ee15ea9d0 --- /dev/null +++ b/rpc/rpchelper/pinned_tx.go @@ -0,0 +1,74 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package rpchelper + +import ( + "context" + + "github.com/erigontech/erigon/db/kv" + "github.com/erigontech/erigon/db/kv/membatchwithdb" + "github.com/erigontech/erigon/db/snapshotsync/blocksnapshots" +) + +// PinnedRoTx pins a read tx to the overlay resolution made at acquisition: +// reads go through the pinned view (or the raw tx when no overlay was +// published), and Rollback releases the raw tx — a view does not own the +// underlying resources, so this is the one handle callers need. +type PinnedRoTx struct { + kv.TemporalTx // the pinned read view, or the raw tx when overlay is nil + raw kv.TemporalTx + overlay *membatchwithdb.MemoryMutation +} + +// PinToOverlay pins tx to the given overlay for the rest of the request: a +// read view when overlay is non-nil, an explicit no-overlay pin otherwise, so +// an overlay published mid-request cannot leak in through downstream wrap +// points. A tx already carrying a pinned view is returned unchanged. +func PinToOverlay(tx kv.TemporalTx, overlay *membatchwithdb.MemoryMutation) kv.TemporalTx { + if membatchwithdb.CarriesOverlayView(tx) { + return tx + } + view := tx + if overlay != nil { + view = overlay.NewReadView(tx) + } + return &PinnedRoTx{TemporalTx: view, raw: tx, overlay: overlay} +} + +// Rollback releases the raw tx; the view in between owns no resources. +func (t *PinnedRoTx) Rollback() { + t.raw.Rollback() +} + +// OverlayView implements membatchwithdb.OverlayViewCarrier. +func (t *PinnedRoTx) OverlayView() (*membatchwithdb.MemoryMutation, bool) { + return t.overlay, true +} + +// BlockFilesRoTx keeps the tx-scoped block-files view across the pin, so a +// request cannot straddle a snapshot merge. +func (t *PinnedRoTx) BlockFilesRoTx() *blocksnapshots.View { + if p, ok := t.TemporalTx.(membatchwithdb.HasBlockFilesRoTx); ok { + return p.BlockFilesRoTx() + } + return nil +} + +// Apply hands the pinned handle to the callback, not the raw tx. +func (t *PinnedRoTx) Apply(_ context.Context, f func(tx kv.Tx) error) error { + return f(t) +} From 928a650ca41594a3325ebad308db884aa2cd6902 Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Fri, 14 Aug 2026 18:06:48 +0200 Subject: [PATCH 8/9] db, rpc: address the fifth review round on the overlay pin - The fee-history cache goes dual-regime: at or below the frozen boundary entries stay number-keyed and the hit path is zero-I/O again; above it they are hash-keyed, and a resolution error degrades the block to uncacheable instead of failing the request. The boundary reads the Snapshots stage progress through the pinned tx, which also works in remote mode where the reader/tx frozen APIs panic. Blocks are fetched by the already-resolved (hash, number) pair, so the store guard holds by construction and the per-block keccak is gone. - PinnedRoTx delegates FreezeInfo to the raw tx, routes Apply through the raw tx's guard while still handing the pinned handle to the callback, and forwards UnderlyingTx and Pin. - NewGasPriceOracleBackend requires a pinned tx (panics otherwise) instead of silently re-resolving the overlay, making the pin a constructor invariant; Fork derives the parent's overlay from the pinned tx. - Comment hygiene: the pin-once rationale lives only on PinToOverlay, the fillFeeDefaults comment is scoped to what the pin actually covers, and the duplicated CarriesOverlayView guard and single-caller helper are inlined. - Test harness: the begin hooks return errors and assert on the test goroutine, the five publish/commit/unpublish copies collapse into newCycleHookDB, the harness commit writes the Execution stage progress and canonical TxNums markers, and the mock derives a per-height canonical hash and honors the ok=false-beyond-head contract. Every behavioral fix is pinned by a regression test that was red before it (gasprice_test.go, overlay_race_test.go). --- db/kv/membatchwithdb/memory_mutation.go | 6 +- rpc/gasprice/bench_test.go | 5 +- rpc/gasprice/feehistory.go | 64 ++++++--- rpc/gasprice/feehistory_test.go | 3 +- rpc/gasprice/gasprice.go | 10 ++ rpc/gasprice/gasprice_test.go | 86 ++++++++++- rpc/jsonrpc/eth_fill_transaction.go | 10 +- rpc/jsonrpc/eth_system.go | 28 +++- rpc/jsonrpc/overlay_race_test.go | 184 +++++++++++++++++------- rpc/rpchelper/pinned_tx.go | 30 +++- 10 files changed, 323 insertions(+), 103 deletions(-) diff --git a/db/kv/membatchwithdb/memory_mutation.go b/db/kv/membatchwithdb/memory_mutation.go index b2f08b31489..22b408585b5 100644 --- a/db/kv/membatchwithdb/memory_mutation.go +++ b/db/kv/membatchwithdb/memory_mutation.go @@ -1102,10 +1102,8 @@ type OverlayViewCarrier interface { OverlayView() (overlay *MemoryMutation, pinned bool) } -// CarriesOverlayView reports whether tx is already a pinned overlay view. -// Overlay wrap points skip such txs so the first wrap pins the overlay a -// request reads from: re-wrapping would layer a possibly newer overlay on -// top, mixing two heads within one request. +// CarriesOverlayView reports whether tx is already a pinned overlay view, so +// wrap points leave it alone (rationale on rpchelper.PinToOverlay). func CarriesOverlayView(tx kv.Tx) bool { _, ok := ViewOverlay(tx) return ok diff --git a/rpc/gasprice/bench_test.go b/rpc/gasprice/bench_test.go index 173bc6090a6..aded83628d6 100644 --- a/rpc/gasprice/bench_test.go +++ b/rpc/gasprice/bench_test.go @@ -38,6 +38,7 @@ import ( "github.com/erigontech/erigon/rpc/gasprice/gaspricecfg" "github.com/erigontech/erigon/rpc/jsonrpc" "github.com/erigontech/erigon/rpc/rpccfg" + "github.com/erigontech/erigon/rpc/rpchelper" ) const txsPerBlock = 100 @@ -128,7 +129,7 @@ func BenchmarkSuggestTipCap(b *testing.B) { // Fresh cache every iteration → cold path, no cache hits. cache := jsonrpc.NewGasPriceCache() oracle := gasprice.NewOracle( - jsonrpc.NewGasPriceOracleBackend(dbArg, tx, baseApi), + jsonrpc.NewGasPriceOracleBackend(dbArg, rpchelper.PinToOverlay(tx, nil), baseApi), cfg, cache, nil, @@ -187,7 +188,7 @@ func BenchmarkFeeHistory(b *testing.B) { // starts cold every time (nil historyCache). This ensures we // measure DB round-trips, not cache hits. oracle := gasprice.NewOracle( - jsonrpc.NewGasPriceOracleBackend(m.DB, tx, baseApi), + jsonrpc.NewGasPriceOracleBackend(m.DB, rpchelper.PinToOverlay(tx, nil), baseApi), gaspricecfg.Config{MaxHeaderHistory: 0, MaxBlockHistory: 0}, gasCache, nil, // cold: no history cache diff --git a/rpc/gasprice/feehistory.go b/rpc/gasprice/feehistory.go index d6bbcfbf69e..60ef50e1284 100644 --- a/rpc/gasprice/feehistory.go +++ b/rpc/gasprice/feehistory.go @@ -57,14 +57,18 @@ const ( maxBlockFetchers = 4 ) -// cacheKey identifies a processed block in the fee history cache. Keying by -// header hash instead of number makes an entry live exactly as long as the -// block itself: a same-height sibling (reorg, or an in-flight block replaced -// before its commit lands) has a different hash and misses. +// cacheKey identifies a processed block in the fee history cache. Above the +// frozen boundary the header hash is part of the key, so an entry lives +// exactly as long as the block itself: a same-height sibling (reorg, or an +// in-flight block replaced before its commit lands) has a different hash and +// misses. At or below the boundary the mapping is immutable and the hash +// stays zero: the number alone identifies the block, with no per-block +// resolution on the hit path. // The percentiles string is a binary encoding of the requested percentile slice, // so identical percentile arrays produce the same key. type cacheKey struct { hash common.Hash + number uint64 percentiles string } @@ -348,6 +352,12 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast // Pre-fetch chain config once using the main backend (safe: single goroutine). chainconfig := oracle.backend.ChainConfig() + var frozenBound uint64 + if oracle.historyCache != nil { + if fb, err := oracle.backend.FrozenBlocks(); err == nil { + frozenBound = fb + } + } // Launch up to maxBlockFetchers goroutines. Each goroutine opens its own // TemporalTx via Fork so MDBX transactions are never shared across goroutines. @@ -384,35 +394,45 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast idx := int(blockNumber - oldestBlock) // The pending block comes from the mining cache and is rebuilt - // continuously, so its results are never memoized. The cache key - // comes from the canonical-hash index — one cheap lookup, no - // header fetch and no keccak on the hit path. + // continuously, so its results are never memoized. Above the + // frozen boundary the key includes the canonical hash — one + // cheap lookup; a resolution error degrades the block to + // uncached instead of failing the request. isPending := pendingBlock != nil && blockNumber >= pendingBlock.NumberU64() cacheable := !isPending && oracle.historyCache != nil - var blockHash common.Hash - if cacheable { - hash, ok, err := localBackend.CanonicalHash(fetchCtx, blockNumber) - if err != nil { - return err + byHash := false + key := cacheKey{number: blockNumber, percentiles: percentileKey} + if cacheable && blockNumber > frozenBound { + if hash, ok, err := localBackend.CanonicalHash(fetchCtx, blockNumber); err == nil && ok { + key.hash = hash + byHash = true + } else { + cacheable = false } - blockHash, cacheable = hash, ok - if ok { - if cached, ok := oracle.historyCache.get(cacheKey{blockHash, percentileKey}); ok { - blockResults[idx] = blockResult{processed: cached, hasResult: true} - continue - } + } + if cacheable { + if cached, ok := oracle.historyCache.get(key); ok { + blockResults[idx] = blockResult{processed: cached, hasResult: true} + continue } } + // Fetch by the resolved pair to skip a second canonical resolution. fees := &blockFees{blockNumber: blockNumber} switch { case isPending: fees.block, fees.receipts = pendingBlock, pendingReceipts case len(rewardPercentiles) != 0: - fees.block, fees.err = localBackend.BlockByNumber(fetchCtx, rpc.BlockNumber(blockNumber)) + if byHash { + fees.block, fees.err = localBackend.BlockByHashNumber(fetchCtx, key.hash, blockNumber) + } else { + fees.block, fees.err = localBackend.BlockByNumber(fetchCtx, rpc.BlockNumber(blockNumber)) + } if fees.block != nil && fees.err == nil { fees.receipts, fees.err = localBackend.GetReceiptsGasUsed(fetchCtx, fees.block) } + case byHash: + fees.header, fees.err = localBackend.HeaderByHashNumber(fetchCtx, key.hash, blockNumber) default: fees.header, fees.err = localBackend.HeaderByNumber(fetchCtx, rpc.BlockNumber(blockNumber)) } @@ -435,10 +455,8 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast } blockResults[idx] = blockResult{processed: fees.results, hasResult: true} - // Store only when the fetched block is the one the key names, - // so a resolution/fetch divergence can never poison the cache. - if cacheable && fees.header.Hash() == blockHash { - oracle.historyCache.add(cacheKey{blockHash, percentileKey}, fees.results) + if cacheable { + oracle.historyCache.add(key, fees.results) } } }) diff --git a/rpc/gasprice/feehistory_test.go b/rpc/gasprice/feehistory_test.go index 0cfd4deca60..f160d4f679e 100644 --- a/rpc/gasprice/feehistory_test.go +++ b/rpc/gasprice/feehistory_test.go @@ -33,6 +33,7 @@ import ( "github.com/erigontech/erigon/rpc/gasprice/gaspricecfg" "github.com/erigontech/erigon/rpc/jsonrpc" "github.com/erigontech/erigon/rpc/rpccfg" + "github.com/erigontech/erigon/rpc/rpchelper" ) func TestFeeHistory(t *testing.T) { @@ -86,7 +87,7 @@ func TestFeeHistory(t *testing.T) { defer tx.Rollback() cache := jsonrpc.NewGasPriceCache() - oracle := gasprice.NewOracle(jsonrpc.NewGasPriceOracleBackend(m.DB, tx, baseApi), config, cache, gasprice.NewFeeHistoryCache(), log.New()) + oracle := gasprice.NewOracle(jsonrpc.NewGasPriceOracleBackend(m.DB, rpchelper.PinToOverlay(tx, nil), baseApi), config, cache, gasprice.NewFeeHistoryCache(), log.New()) first, reward, baseFee, ratio, blobBaseFee, blobBaseFeeRatio, err := oracle.FeeHistory(context.Background(), c.count, c.last, c.percent) diff --git a/rpc/gasprice/gasprice.go b/rpc/gasprice/gasprice.go index a0a05882ab0..19925cc995c 100644 --- a/rpc/gasprice/gasprice.go +++ b/rpc/gasprice/gasprice.go @@ -51,6 +51,16 @@ type OracleBackend interface { // the backend's view, or ok=false when the height is beyond the head. CanonicalHash(ctx context.Context, number uint64) (common.Hash, bool, error) + // FrozenBlocks returns the frozen (snapshot) boundary: the canonical + // number-to-hash mapping at or below it is immutable. + FrozenBlocks() (uint64, error) + + // HeaderByHashNumber and BlockByHashNumber fetch by an already-resolved + // canonical (hash, number) pair, so a cached entry can never name a + // different block than the one processed. + HeaderByHashNumber(ctx context.Context, hash common.Hash, number uint64) (*types.Header, error) + BlockByHashNumber(ctx context.Context, hash common.Hash, number uint64) (*types.Block, error) + // Fork opens a new TemporalTx and returns a goroutine-local backend together // with a cleanup function (call via defer cleanup()). // If the backend does not support forking, it returns (nil, nil, nil) and diff --git a/rpc/gasprice/gasprice_test.go b/rpc/gasprice/gasprice_test.go index d0463e978ca..cc5ff4e7ef6 100644 --- a/rpc/gasprice/gasprice_test.go +++ b/rpc/gasprice/gasprice_test.go @@ -22,9 +22,12 @@ package gasprice_test import ( "container/heap" "context" + "encoding/binary" + "errors" "math" "math/big" "math/rand" + "sync/atomic" "testing" "github.com/holiman/uint256" @@ -43,6 +46,7 @@ import ( "github.com/erigontech/erigon/rpc/gasprice/gaspricecfg" "github.com/erigontech/erigon/rpc/jsonrpc" "github.com/erigontech/erigon/rpc/rpccfg" + "github.com/erigontech/erigon/rpc/rpchelper" ) func newTestBackend(t *testing.T) *execmoduletester.ExecModuleTester { @@ -95,7 +99,7 @@ func TestSuggestPrice(t *testing.T) { defer tx.Rollback() cache := jsonrpc.NewGasPriceCache() - oracle := gasprice.NewOracle(jsonrpc.NewGasPriceOracleBackend(nil, tx, baseApi), config, cache, nil, log.New()) + oracle := gasprice.NewOracle(jsonrpc.NewGasPriceOracleBackend(nil, rpchelper.PinToOverlay(tx, nil), baseApi), config, cache, nil, log.New()) // The gas price sampled is: 32G, 31G, 30G, 29G, 28G, 27G got, err := oracle.SuggestTipCap(context.Background()) @@ -272,10 +276,23 @@ func BenchmarkKthPercentile(b *testing.B) { // cancelled, allowing us to verify that cancellation propagates correctly // through fetchBlockPricesParallel. type mockOracleBackend struct { - head *types.Header + head *types.Header + frozen uint64 + canonicalErr error + canonicalCalls atomic.Int32 + headerCalls atomic.Int32 +} + +// mockHeightHash derives a distinct hash per height, honoring the "one hash +// per block" contract the fee-history cache keys rely on. +func mockHeightHash(number uint64) common.Hash { + var h common.Hash + binary.BigEndian.PutUint64(h[:8], number+1) + return h } func (m *mockOracleBackend) HeaderByNumber(_ context.Context, _ rpc.BlockNumber) (*types.Header, error) { + m.headerCalls.Add(1) return m.head, nil } @@ -301,16 +318,75 @@ func (m *mockOracleBackend) PendingBlockAndReceipts() (*types.Block, types.Recei } func (m *mockOracleBackend) CanonicalHash(_ context.Context, number uint64) (common.Hash, bool, error) { + m.canonicalCalls.Add(1) + if m.canonicalErr != nil { + return common.Hash{}, false, m.canonicalErr + } if number > m.head.Number.Uint64() { return common.Hash{}, false, nil } - return m.head.Hash(), true, nil + return mockHeightHash(number), true, nil +} + +func (m *mockOracleBackend) FrozenBlocks() (uint64, error) { + return m.frozen, nil +} + +func (m *mockOracleBackend) HeaderByHashNumber(ctx context.Context, _ common.Hash, _ uint64) (*types.Header, error) { + return m.HeaderByNumber(ctx, 0) +} + +func (m *mockOracleBackend) BlockByHashNumber(ctx context.Context, _ common.Hash, _ uint64) (*types.Block, error) { + return m.BlockByNumber(ctx, 0) } func (m *mockOracleBackend) Fork(_ context.Context) (gasprice.OracleBackend, func(), error) { return nil, nil, nil // sequential mode } +// TestFeeHistory_CanonicalHashErrorDegradesToUncached pins that a transient +// error on the auxiliary cache-key resolution degrades the block to uncached +// instead of failing the whole request — the number-keyed hit path had no +// failure mode at all. +func TestFeeHistory_CanonicalHashErrorDegradesToUncached(t *testing.T) { + head := types.NewEmptyHeaderForAssembling() + head.Number.SetUint64(10) + head.GasLimit = 30_000_000 + head.BaseFee = uint256.NewInt(1_000_000_000) + + backend := &mockOracleBackend{head: head, canonicalErr: errors.New("transient lookup failure")} + oracle := gasprice.NewOracle(backend, gaspricecfg.Config{Blocks: 2, Percentile: 60}, jsonrpc.NewGasPriceCache(), gasprice.NewFeeHistoryCache(), log.New()) + + _, _, baseFee, _, _, _, err := oracle.FeeHistory(context.Background(), 3, rpc.LatestBlockNumber, nil) + require.NoError(t, err, "a cache-key resolution error must degrade to uncached, not fail the request") + require.Len(t, baseFee, 4) +} + +// TestFeeHistory_FrozenRangeCachesByNumberWithoutResolution pins that at or +// below the frozen boundary — where the number-to-hash mapping is immutable — +// no per-block hash resolution happens and repeat requests hit the cache. +func TestFeeHistory_FrozenRangeCachesByNumberWithoutResolution(t *testing.T) { + head := types.NewEmptyHeaderForAssembling() + head.Number.SetUint64(10) + head.GasLimit = 30_000_000 + head.BaseFee = uint256.NewInt(1_000_000_000) + + backend := &mockOracleBackend{head: head, frozen: 10} + oracle := gasprice.NewOracle(backend, gaspricecfg.Config{Blocks: 2, Percentile: 60}, jsonrpc.NewGasPriceCache(), gasprice.NewFeeHistoryCache(), log.New()) + + _, _, _, _, _, _, err := oracle.FeeHistory(context.Background(), 4, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.EqualValues(t, 0, backend.canonicalCalls.Load(), + "the frozen range must not resolve hashes: the mapping is immutable") + fetchesAfterFirst := backend.headerCalls.Load() + + _, _, _, _, _, _, err = oracle.FeeHistory(context.Background(), 4, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.EqualValues(t, 0, backend.canonicalCalls.Load()) + require.Equal(t, fetchesAfterFirst, backend.headerCalls.Load(), + "the second request must be served from number-keyed cache entries") +} + // TestSuggestTipCap_EmptyBlocksFallbackMatchesGeth verifies that on a chain // where all sampled blocks are empty, the oracle uses GWei/1000 as the // fallback price (matching Geth's miner.DefaultConfig.GasPrice) rather than @@ -408,7 +484,7 @@ func TestSuggestTipCap_SparseBlocks(t *testing.T) { defer dbTx.Rollback() cache := jsonrpc.NewGasPriceCache() - oracle := gasprice.NewOracle(jsonrpc.NewGasPriceOracleBackend(nil, dbTx, baseApi), cfg, cache, nil, log.New()) + oracle := gasprice.NewOracle(jsonrpc.NewGasPriceOracleBackend(nil, rpchelper.PinToOverlay(dbTx, nil), baseApi), cfg, cache, nil, log.New()) got, err := oracle.SuggestTipCap(context.Background()) require.NoError(t, err) @@ -445,7 +521,7 @@ func TestSuggestTipCap_AllEmptyBlocks(t *testing.T) { defer dbTx.Rollback() cache := jsonrpc.NewGasPriceCache() - oracle := gasprice.NewOracle(jsonrpc.NewGasPriceOracleBackend(nil, dbTx, baseApi), cfg, cache, nil, log.New()) + oracle := gasprice.NewOracle(jsonrpc.NewGasPriceOracleBackend(nil, rpchelper.PinToOverlay(dbTx, nil), baseApi), cfg, cache, nil, log.New()) // With no transactions anywhere, the oracle returns (nil, nil): no price, no error. _, err = oracle.SuggestTipCap(context.Background()) diff --git a/rpc/jsonrpc/eth_fill_transaction.go b/rpc/jsonrpc/eth_fill_transaction.go index c3d6236a57f..f62fc60fee9 100644 --- a/rpc/jsonrpc/eth_fill_transaction.go +++ b/rpc/jsonrpc/eth_fill_transaction.go @@ -50,8 +50,9 @@ func (api *APIImpl) FillTransaction(ctx context.Context, args ethapi.CallArgs) ( return nil, errors.New("maxFeePerBlobGas, if specified, must be non-zero") } - // The pinned view keeps ReadCurrentHeader and the gas oracle on one head - // for the whole request, including the in-flight overlay block. + // The pinned view keeps ReadCurrentHeader and the gas-oracle fee defaults + // on one head (including the in-flight overlay block); the nonce and + // gas-estimate sub-calls still open their own txs. overlayTx, err := api.filters.BeginTemporalRoWithOverlay(ctx, api.db) if err != nil { return nil, err @@ -163,10 +164,7 @@ func (api *APIImpl) FillTransaction(ctx context.Context, args ethapi.CallArgs) ( } func (api *APIImpl) newGasOracle(dbTx kv.TemporalTx) *gasprice.Oracle { - return api.newGasOracleFromBackend(NewGasPriceOracleBackend(api.db, dbTx, api.BaseAPI)) -} - -func (api *APIImpl) newGasOracleFromBackend(backend *GasPriceOracleBackend) *gasprice.Oracle { + backend := NewGasPriceOracleBackend(api.db, dbTx, api.BaseAPI) return gasprice.NewOracle(backend, ethconfig.Defaults.GPO, api.gasCache, api.feeHistoryCache, api.logger.New("app", "gasPriceOracle")) } diff --git a/rpc/jsonrpc/eth_system.go b/rpc/jsonrpc/eth_system.go index 31c492cb6a7..7767e501747 100644 --- a/rpc/jsonrpc/eth_system.go +++ b/rpc/jsonrpc/eth_system.go @@ -34,6 +34,7 @@ import ( "github.com/erigontech/erigon/execution/chain" "github.com/erigontech/erigon/execution/protocol/misc" "github.com/erigontech/erigon/execution/protocol/params" + "github.com/erigontech/erigon/execution/stagedsync/stages" "github.com/erigontech/erigon/execution/types" "github.com/erigontech/erigon/execution/vm" "github.com/erigontech/erigon/execution/vm/evmtypes" @@ -495,12 +496,13 @@ type GasPriceOracleBackend struct { baseApi *BaseAPI } -// NewGasPriceOracleBackend pins the block overlay once so the head the oracle -// resolves stays readable for the whole request, including on the txs Fork -// opens. A tx that already carries a pinned view keeps its own overlay. +// NewGasPriceOracleBackend requires a tx already pinned at acquisition (see +// rpchelper.BeginTemporalRoWithOverlay): resolving the overlay here, after +// the caller opened the tx, would re-open the torn (tx, overlay) window the +// pinned acquisition exists to close. func NewGasPriceOracleBackend(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI) *GasPriceOracleBackend { if !membatchwithdb.CarriesOverlayView(tx) { - tx = rpchelper.PinToOverlay(tx, baseApi.filters.LatestOverlay()) + panic("NewGasPriceOracleBackend: tx must be pinned via rpchelper.BeginTemporalRoWithOverlay or PinToOverlay") } return &GasPriceOracleBackend{db: db, tx: tx, baseApi: baseApi} } @@ -513,8 +515,7 @@ func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBacken if err != nil { return nil, nil, err } - // Reuse the parent's pinned overlay instead of re-resolving: it may have - // been unpublished since the request started. + // Reuse the parent's pin (rationale on rpchelper.PinToOverlay). overlay, _ := membatchwithdb.ViewOverlay(b.tx) return &GasPriceOracleBackend{db: b.db, tx: rpchelper.PinToOverlay(tx, overlay), baseApi: b.baseApi}, func() { tx.Rollback() }, @@ -536,6 +537,21 @@ func (b *GasPriceOracleBackend) CanonicalHash(ctx context.Context, number uint64 return b.baseApi._blockReader.CanonicalHash(ctx, b.tx, number) } +// FrozenBlocks reads the Snapshots stage progress through the pinned tx — one +// KV read that works in both embedded and remote mode, unlike the block +// reader's FrozenBlocks which panics remotely. +func (b *GasPriceOracleBackend) FrozenBlocks() (uint64, error) { + return stages.GetStageProgress(b.tx, stages.Snapshots) +} + +func (b *GasPriceOracleBackend) HeaderByHashNumber(ctx context.Context, hash common.Hash, number uint64) (*types.Header, error) { + return b.baseApi._blockReader.Header(ctx, b.tx, hash, number) +} + +func (b *GasPriceOracleBackend) BlockByHashNumber(ctx context.Context, hash common.Hash, number uint64) (*types.Block, error) { + return b.baseApi.blockWithSenders(ctx, b.tx, hash, number) +} + func (b *GasPriceOracleBackend) HeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Header, error) { header, err := b.baseApi.headerByNumber(ctx, number, b.tx) if err != nil { diff --git a/rpc/jsonrpc/overlay_race_test.go b/rpc/jsonrpc/overlay_race_test.go index 40d5a15015c..df46f356099 100644 --- a/rpc/jsonrpc/overlay_race_test.go +++ b/rpc/jsonrpc/overlay_race_test.go @@ -42,6 +42,7 @@ import ( "github.com/erigontech/erigon/execution/chain" "github.com/erigontech/erigon/execution/execmodule/execmoduletester" "github.com/erigontech/erigon/execution/protocol/params" + "github.com/erigontech/erigon/execution/stagedsync/stages" "github.com/erigontech/erigon/execution/tests/blockgen" "github.com/erigontech/erigon/execution/types" "github.com/erigontech/erigon/node/gointerfaces" @@ -68,16 +69,32 @@ const ( // test (the gas oracle, "latest" tag resolution) see this block as current. func writeHeadBlockMarkers(t *testing.T, tx kv.RwTx, header *types.Header, body *types.Body) { t.Helper() + require.NoError(t, writeHeadBlockMarkersE(tx, header, body)) +} + +// writeHeadBlockMarkersE is the error-returning core, safe to call from hooks +// running on non-test goroutines (testify's FailNow contract). +func writeHeadBlockMarkersE(tx kv.RwTx, header *types.Header, body *types.Body) error { hash := header.Hash() num := header.Number.Uint64() - require.NoError(t, rawdb.WriteHeader(tx, header)) - require.NoError(t, rawdb.WriteHeadHeaderHash(tx, hash)) - require.NoError(t, rawdb.WriteCanonicalHash(tx, hash, num)) - require.NoError(t, rawdb.WriteBody(tx, hash, num, body)) + if err := rawdb.WriteHeader(tx, header); err != nil { + return err + } + if err := rawdb.WriteHeadHeaderHash(tx, hash); err != nil { + return err + } + if err := rawdb.WriteCanonicalHash(tx, hash, num); err != nil { + return err + } + if err := rawdb.WriteBody(tx, hash, num, body); err != nil { + return err + } rawdb.WriteForkchoiceHead(tx, hash) + return nil } type overlayAheadHarness struct { + t *testing.T base *BaseAPI m *execmoduletester.ExecModuleTester overlayHeader *types.Header @@ -167,7 +184,7 @@ func newOverlayAheadHarness(t *testing.T, withOverlayTxs bool) *overlayAheadHarn stateCache := kvcache.New(kvcache.DefaultCoherentConfig) base := newBaseApiWithFiltersForTest(filters, stateCache, m) - return &overlayAheadHarness{base: base, m: m, overlayHeader: overlayHeader, events: events, doms: doms} + return &overlayAheadHarness{t: t, base: base, m: m, overlayHeader: overlayHeader, events: events, doms: doms} } // overlayRaceTxPoolClient extends stubTxPoolClient with canned replies for @@ -368,7 +385,7 @@ func TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish(t *testing.T) { tx, err := h.m.DB.BeginTemporalRo(h.m.Ctx) require.NoError(t, err) defer tx.Rollback() - backend := NewGasPriceOracleBackend(h.m.DB, tx, h.base) + backend := NewGasPriceOracleBackend(h.m.DB, h.base.filters.WithTemporalOverlay(tx), h.base) h.events.PublishOverlay(nil) h.doms.Close() @@ -393,18 +410,32 @@ func TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish(t *testing.T) { // given header, with an empty body. func publishOverlayHead(t *testing.T, h *overlayAheadHarness, head *types.Header) { t.Helper() + require.NoError(t, publishOverlayHeadE(h, head)) +} + +// publishOverlayHeadE is the hook-safe core: it reports errors instead of +// calling testify's FailNow family, which is test-goroutine-only. t.Cleanup +// is safe from any goroutine. +func publishOverlayHeadE(h *overlayAheadHarness, head *types.Header) error { ctx := h.m.Ctx roTx, err := h.m.DB.BeginTemporalRo(ctx) - require.NoError(t, err) - t.Cleanup(roTx.Rollback) + if err != nil { + return err + } + h.t.Cleanup(roTx.Rollback) doms, err := execctx.NewSharedDomains(ctx, roTx, h.m.Log) - require.NoError(t, err) - t.Cleanup(doms.Close) - require.NoError(t, doms.InitBlockOverlay(roTx, h.m.Dirs.Tmp)) - - writeHeadBlockMarkers(t, doms.BlockOverlay(), head, &types.Body{}) - + if err != nil { + return err + } + h.t.Cleanup(doms.Close) + if err := doms.InitBlockOverlay(roTx, h.m.Dirs.Tmp); err != nil { + return err + } + if err := writeHeadBlockMarkersE(doms.BlockOverlay(), head, &types.Body{}); err != nil { + return err + } h.events.PublishOverlay(doms) + return nil } // publishSiblingOverlay publishes a second overlay holding a same-height @@ -412,13 +443,18 @@ func publishOverlayHead(t *testing.T, h *overlayAheadHarness, head *types.Header // hash and header are distinguishable from the original. func publishSiblingOverlay(t *testing.T, h *overlayAheadHarness) *types.Header { t.Helper() - sibling := types.CopyHeader(h.overlayHeader) - sibling.BaseFee = uint256.NewInt(overlayRaceBaseFee + 1111) + sibling := siblingOfOverlayHead(h) require.NotEqual(t, h.overlayHeader.Hash(), sibling.Hash()) publishOverlayHead(t, h, sibling) return sibling } +func siblingOfOverlayHead(h *overlayAheadHarness) *types.Header { + sibling := types.CopyHeader(h.overlayHeader) + sibling.BaseFee = uint256.NewInt(overlayRaceBaseFee + 1111) + return sibling +} + // publishCommittedSiblingOverlay publishes an overlay whose forkchoice head is // a same-height sibling of an already-committed block — an in-RAM reorg below // the committed head. @@ -448,7 +484,7 @@ func TestGasPriceOracle_PinnedViewIgnoresLaterOverlayPublish(t *testing.T) { tx, err := h.m.DB.BeginTemporalRo(h.m.Ctx) require.NoError(t, err) defer tx.Rollback() - backend := NewGasPriceOracleBackend(h.m.DB, tx, h.base) + backend := NewGasPriceOracleBackend(h.m.DB, h.base.filters.WithTemporalOverlay(tx), h.base) sibling := publishSiblingOverlay(t, h) @@ -531,7 +567,7 @@ func TestGasPriceOracle_NilOverlayPinIgnoresLaterPublish(t *testing.T) { tx, err := h.m.DB.BeginTemporalRo(h.m.Ctx) require.NoError(t, err) defer tx.Rollback() - backend := NewGasPriceOracleBackend(h.m.DB, tx, h.base) + backend := NewGasPriceOracleBackend(h.m.DB, rpchelper.PinToOverlay(tx, nil), h.base) sibling := publishSiblingOverlay(t, h) @@ -567,6 +603,14 @@ func TestBeginTemporalRoWithOverlay_PreservesOptionalInterfaces(t *testing.T) { "Apply must hand the pinned view to the callback, not the raw tx") return nil })) + + require.NotPanics(t, func() { pinnedTx.FreezeInfo() }, + "FreezeInfo must delegate to the raw tx, not promote the view's panic") + ut, ok := pinnedTx.(interface{ UnderlyingTx() kv.TemporalTx }) + require.True(t, ok, "the pinned handle must not hide UnderlyingTx") + require.NotNil(t, ut.UnderlyingTx()) + _, ok = pinnedTx.(interface{ Pin() kv.TemporalFilesPin }) + require.True(t, ok, "the pinned handle must not hide Pin") } t.Run("overlay published", check) @@ -575,6 +619,20 @@ func TestBeginTemporalRoWithOverlay_PreservesOptionalInterfaces(t *testing.T) { t.Run("no overlay", check) } +// TestGasPriceOracleBackend_RequiresPinnedTx pins that the constructor rejects +// an unpinned tx: resolving the overlay after the caller opened the tx would +// re-open the torn (tx, overlay) window the pinned acquisition closes. +func TestGasPriceOracleBackend_RequiresPinnedTx(t *testing.T) { + t.Parallel() + h := newOverlayAheadHarness(t, false) + tx, err := h.m.DB.BeginTemporalRo(h.m.Ctx) + require.NoError(t, err) + defer tx.Rollback() + + require.Panics(t, func() { NewGasPriceOracleBackend(h.m.DB, tx, h.base) }, + "an unpinned tx must be rejected, not silently re-pinned to the live overlay") +} + // remoteishBlockReader simulates the rpcdaemon RemoteBlockReader: CanonicalHash // ignores the caller's tx and resolves on the live view instead. type remoteishBlockReader struct { @@ -638,23 +696,44 @@ func TestFeeHistory_ReorgedCommittedBlockNotServedFromCache(t *testing.T) { } // commitOverlayBlock writes the harness's overlay head into MDBX the way the -// background commit would (header, canonical marker, forkchoice head), so txs -// opened afterwards resolve it as the committed head. +// background commit would, so txs opened afterwards resolve it as the +// committed head. func commitOverlayBlock(t *testing.T, h *overlayAheadHarness) { t.Helper() + require.NoError(t, commitOverlayBlockE(h)) +} + +// commitOverlayBlockE writes head markers plus the Execution stage progress +// and the canonical TxNums entry production advances with them: without those +// two, state readers reject the new head. +func commitOverlayBlockE(h *overlayAheadHarness) error { rwTx, err := h.m.DB.BeginRw(h.m.Ctx) - require.NoError(t, err) + if err != nil { + return err + } defer rwTx.Rollback() - writeHeadBlockMarkers(t, rwTx, h.overlayHeader, &types.Body{}) - require.NoError(t, rwTx.Commit()) + if err := writeHeadBlockMarkersE(rwTx, h.overlayHeader, &types.Body{}); err != nil { + return err + } + num := h.overlayHeader.Number.Uint64() + if err := stages.SaveStageProgress(rwTx, stages.Execution, num); err != nil { + return err + } + if err := rawdb.AppendCanonicalTxNums(rwTx, num); err != nil { + return err + } + return rwTx.Commit() } // beginHookDB runs a hook right after each BeginTemporalRo returns, simulating -// commit/publish activity landing while a request acquires its tx. One-shot -// scenarios pass a sync.OnceFunc-wrapped hook. +// commit/publish activity landing while a request acquires its tx. Hooks also +// run on the oracle's errgroup goroutines (via Fork), so they must not use +// testify's FailNow family; failures are reported with the goroutine-safe +// t.Errorf. type beginHookDB struct { kv.TemporalRoDB - hook func() + t *testing.T + hook func() error } func (db *beginHookDB) BeginTemporalRo(ctx context.Context) (kv.TemporalTx, error) { @@ -662,10 +741,30 @@ func (db *beginHookDB) BeginTemporalRo(ctx context.Context) (kv.TemporalTx, erro if err != nil { return nil, err } - db.hook() + if hookErr := db.hook(); hookErr != nil { + db.t.Errorf("begin hook: %v", hookErr) + } return tx, nil } +// newCycleHookDB returns a DB whose first BeginTemporalRo runs the +// publish/commit/unpublish cycle (optionally skipping the initial publish): +// the window closing while a request acquires its tx. +func newCycleHookDB(h *overlayAheadHarness, publishFirst bool) *beginHookDB { + return &beginHookDB{TemporalRoDB: h.m.DB, t: h.t, hook: sync.OnceValue(func() error { + if publishFirst { + if err := publishOverlayHeadE(h, h.overlayHeader); err != nil { + return err + } + } + if err := commitOverlayBlockE(h); err != nil { + return err + } + h.events.PublishOverlay(nil) + return nil + })} +} + // TestFeeHistory_PublishCycleDuringTxAcquisition pins that overlay-capture // stability is tracked with a publish sequence number: a full publish/commit/ // unpublish cycle landing during tx acquisition leaves the overlay nil on both @@ -677,11 +776,7 @@ func TestFeeHistory_PublishCycleDuringTxAcquisition(t *testing.T) { h.events.PublishOverlay(nil) h.doms.Close() - db := &beginHookDB{TemporalRoDB: h.m.DB, hook: sync.OnceFunc(func() { - publishOverlayHead(t, h, h.overlayHeader) - commitOverlayBlock(t, h) - h.events.PublishOverlay(nil) - })} + db := newCycleHookDB(h, true) api := newEthApiForTest(h.base, db, nil, nil) got, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) @@ -699,7 +794,7 @@ func TestFeeHistory_PublishCycleDuringTxAcquisition(t *testing.T) { func TestFeeHistory_OverlayUnstableDuringTxAcquisition(t *testing.T) { t.Parallel() h := newOverlayAheadHarness(t, false) - db := &beginHookDB{TemporalRoDB: h.m.DB, hook: func() { publishSiblingOverlay(t, h) }} + db := &beginHookDB{TemporalRoDB: h.m.DB, t: h.t, hook: func() error { return publishOverlayHeadE(h, siblingOfOverlayHead(h)) }} api := newEthApiForTest(h.base, db, nil, nil) got, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) @@ -759,11 +854,7 @@ func TestBlockNumber_PublishCycleDuringTxAcquisition(t *testing.T) { h.events.PublishOverlay(nil) h.doms.Close() - db := &beginHookDB{TemporalRoDB: h.m.DB, hook: sync.OnceFunc(func() { - publishOverlayHead(t, h, h.overlayHeader) - commitOverlayBlock(t, h) - h.events.PublishOverlay(nil) - })} + db := newCycleHookDB(h, true) api := newEthApiForTest(h.base, db, nil, nil) got, err := api.BlockNumber(h.m.Ctx) @@ -780,11 +871,7 @@ func TestBaseFee_PublishCycleDuringTxAcquisition(t *testing.T) { h.events.PublishOverlay(nil) h.doms.Close() - db := &beginHookDB{TemporalRoDB: h.m.DB, hook: sync.OnceFunc(func() { - publishOverlayHead(t, h, h.overlayHeader) - commitOverlayBlock(t, h) - h.events.PublishOverlay(nil) - })} + db := newCycleHookDB(h, true) api := newEthApiForTest(h.base, db, nil, nil) got, err := api.BaseFee(h.m.Ctx) @@ -805,11 +892,7 @@ func TestFillTransaction_PublishCycleDuringTxAcquisition(t *testing.T) { h.events.PublishOverlay(nil) h.doms.Close() - db := &beginHookDB{TemporalRoDB: h.m.DB, hook: sync.OnceFunc(func() { - publishOverlayHead(t, h, h.overlayHeader) - commitOverlayBlock(t, h) - h.events.PublishOverlay(nil) - })} + db := newCycleHookDB(h, true) api := newEthApiForTest(h.base, db, stubTxPoolClient{}, nil) to := common.HexToAddress("0x0d3ab14bbad3d99f4203bd7a11acb94882050e7e") @@ -833,10 +916,7 @@ func TestFillTransaction_PublishCycleDuringTxAcquisition(t *testing.T) { func TestFeeHistory_HeadCommittedDuringTxAcquisition(t *testing.T) { t.Parallel() h := newOverlayAheadHarness(t, false) - db := &beginHookDB{TemporalRoDB: h.m.DB, hook: sync.OnceFunc(func() { - commitOverlayBlock(t, h) - h.events.PublishOverlay(nil) - })} + db := newCycleHookDB(h, false) api := newEthApiForTest(h.base, db, nil, nil) got, err := api.FeeHistory(h.m.Ctx, 1, rpc.LatestBlockNumber, nil) diff --git a/rpc/rpchelper/pinned_tx.go b/rpc/rpchelper/pinned_tx.go index 63ee15ea9d0..ca23d3d05fa 100644 --- a/rpc/rpchelper/pinned_tx.go +++ b/rpc/rpchelper/pinned_tx.go @@ -49,7 +49,6 @@ func PinToOverlay(tx kv.TemporalTx, overlay *membatchwithdb.MemoryMutation) kv.T return &PinnedRoTx{TemporalTx: view, raw: tx, overlay: overlay} } -// Rollback releases the raw tx; the view in between owns no resources. func (t *PinnedRoTx) Rollback() { t.raw.Rollback() } @@ -68,7 +67,30 @@ func (t *PinnedRoTx) BlockFilesRoTx() *blocksnapshots.View { return nil } -// Apply hands the pinned handle to the callback, not the raw tx. -func (t *PinnedRoTx) Apply(_ context.Context, f func(tx kv.Tx) error) error { - return f(t) +// Apply goes through the raw tx's guard (closed-tx checks) but hands the +// pinned handle to the callback, not the raw tx. +func (t *PinnedRoTx) Apply(ctx context.Context, f func(tx kv.Tx) error) error { + return t.raw.Apply(ctx, func(kv.Tx) error { return f(t) }) +} + +// FreezeInfo delegates to the raw tx: the overlay read view in between does +// not support it. +func (t *PinnedRoTx) FreezeInfo() kv.FreezeInfo { + return t.raw.FreezeInfo() +} + +// UnderlyingTx exposes the fallback tx like the wrapped view would. +func (t *PinnedRoTx) UnderlyingTx() kv.TemporalTx { + if p, ok := t.TemporalTx.(interface{ UnderlyingTx() kv.TemporalTx }); ok { + return p.UnderlyingTx() + } + return t.raw +} + +// Pin forwards the files-pin capability of the wrapped view (or the raw tx). +func (t *PinnedRoTx) Pin() kv.TemporalFilesPin { + if p, ok := t.TemporalTx.(interface{ Pin() kv.TemporalFilesPin }); ok { + return p.Pin() + } + return nil } From 8badd024534eca82d688b2fccf2744ae296ced1d Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Mon, 17 Aug 2026 15:44:32 +0200 Subject: [PATCH 9/9] rpc/gasprice, rpc/jsonrpc: resolve fee-history cache keys with one range scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hash-keyed cache regime above the frozen boundary resolved one canonical hash per block before consulting the LRU, so a fully memoized eth_feeHistory(1024, latest) cost up to 1024 remote round trips in rpcdaemon mode — user-controlled amplification on what used to be a zero-I/O hit path. OracleBackend.CanonicalHash is replaced by CanonicalHashes(from, to), resolved once per request before the fetchers fan out and over the unfrozen tract only. GasPriceOracleBackend implements it as a single kv.HeaderCanonical range scan on the pinned tx: the overlay head marker stays visible (MemoryMutation.Range merges the memory and db streams) and the block reader, live in rpcdaemon mode, stays out of the cache key. The reader fallback goes away with it, since only unfrozen heights are asked for. A scan error clears the resolved slice, so those blocks degrade to uncached instead of failing the request. Test: TestFeeHistory_HotRangeResolvedInOneScan, red before the fix with eight single-height resolutions. --- rpc/gasprice/feehistory.go | 26 ++++++++++---- rpc/gasprice/gasprice.go | 9 +++-- rpc/gasprice/gasprice_test.go | 58 +++++++++++++++++++++++++++----- rpc/jsonrpc/eth_system.go | 33 ++++++++++++------ rpc/jsonrpc/overlay_race_test.go | 9 ++--- 5 files changed, 103 insertions(+), 32 deletions(-) diff --git a/rpc/gasprice/feehistory.go b/rpc/gasprice/feehistory.go index 27caa2107b6..132f49bd599 100644 --- a/rpc/gasprice/feehistory.go +++ b/rpc/gasprice/feehistory.go @@ -373,6 +373,22 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast } } + // Unfrozen heights are cached by hash, so a reorged-out block can no longer + // be found under its number. The hashes come from one range scan per request + // — resolving them per block costs a remote round trip each in rpcdaemon + // mode, which the cache-hit path used to be free of. Heights left unresolved + // (scan error, beyond the head) are simply not cached. + var hotFrom uint64 + var hotHashes []common.Hash + if oracle.historyCache != nil && lastBlock > frozenBound { + hotFrom = max(oldestBlock, frozenBound+1) + hotHashes, err = oracle.backend.CanonicalHashes(ctx, hotFrom, lastBlock) + if err != nil { + oracle.log.Debug("fee history: canonical range unresolved, serving uncached", "from", hotFrom, "to", lastBlock, "err", err) + hotHashes = nil + } + } + // Launch up to maxBlockFetchers goroutines. Each goroutine opens its own // TemporalTx via Fork so MDBX transactions are never shared across goroutines. // When Fork is not supported (returns nil backend), a single goroutine falls back @@ -408,17 +424,15 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast idx := int(blockNumber - oldestBlock) // The pending block comes from the mining cache and is rebuilt - // continuously, so its results are never memoized. Above the - // frozen boundary the key includes the canonical hash — one - // cheap lookup; a resolution error degrades the block to - // uncached instead of failing the request. + // continuously, so its results are never memoized. isPending := pendingBlock != nil && blockNumber >= pendingBlock.NumberU64() cacheable := !isPending && oracle.historyCache != nil byHash := false key := cacheKey{number: blockNumber, percentiles: percentileKey} if cacheable && blockNumber > frozenBound { - if hash, ok, err := localBackend.CanonicalHash(fetchCtx, blockNumber); err == nil && ok { - key.hash = hash + hotIdx := int(blockNumber - hotFrom) + if hotIdx < len(hotHashes) && hotHashes[hotIdx] != (common.Hash{}) { + key.hash = hotHashes[hotIdx] byHash = true } else { cacheable = false diff --git a/rpc/gasprice/gasprice.go b/rpc/gasprice/gasprice.go index 19925cc995c..dcbb0e5c1d0 100644 --- a/rpc/gasprice/gasprice.go +++ b/rpc/gasprice/gasprice.go @@ -47,9 +47,12 @@ type OracleBackend interface { GetReceiptsGasUsed(ctx context.Context, block *types.Block) (types.Receipts, error) PendingBlockAndReceipts() (*types.Block, types.Receipts) - // CanonicalHash returns the canonical block hash at the given height on - // the backend's view, or ok=false when the height is beyond the head. - CanonicalHash(ctx context.Context, number uint64) (common.Hash, bool, error) + // CanonicalHashes returns the canonical hashes of [from, to] on the + // backend's view, one entry per height. Heights the view has no canonical + // marker for (beyond the head, or pruned) get the zero hash. It resolves + // the whole range at once because a per-height lookup is a remote round + // trip in rpcdaemon mode. + CanonicalHashes(ctx context.Context, from, to uint64) ([]common.Hash, error) // FrozenBlocks returns the frozen (snapshot) boundary: the canonical // number-to-hash mapping at or below it is immutable. diff --git a/rpc/gasprice/gasprice_test.go b/rpc/gasprice/gasprice_test.go index 17de419d52e..b2b9d24c9ef 100644 --- a/rpc/gasprice/gasprice_test.go +++ b/rpc/gasprice/gasprice_test.go @@ -27,6 +27,8 @@ import ( "math" "math/big" "math/rand" + "slices" + "sync" "sync/atomic" "testing" @@ -279,10 +281,12 @@ type mockOracleBackend struct { head *types.Header frozen uint64 canonicalErr error - canonicalCalls atomic.Int32 headerCalls atomic.Int32 safeBlock uint64 finalizedBlock uint64 + + canonicalMu sync.Mutex + canonicalRanges [][2]uint64 } // mockHeightHash derives a distinct hash per height, honoring the "one hash @@ -326,15 +330,24 @@ func (m *mockOracleBackend) PendingBlockAndReceipts() (*types.Block, types.Recei return nil, nil } -func (m *mockOracleBackend) CanonicalHash(_ context.Context, number uint64) (common.Hash, bool, error) { - m.canonicalCalls.Add(1) +func (m *mockOracleBackend) CanonicalHashes(_ context.Context, from, to uint64) ([]common.Hash, error) { + m.canonicalMu.Lock() + m.canonicalRanges = append(m.canonicalRanges, [2]uint64{from, to}) + m.canonicalMu.Unlock() if m.canonicalErr != nil { - return common.Hash{}, false, m.canonicalErr + return nil, m.canonicalErr } - if number > m.head.Number.Uint64() { - return common.Hash{}, false, nil + hashes := make([]common.Hash, to-from+1) + for number := from; number <= min(to, m.head.Number.Uint64()); number++ { + hashes[number-from] = mockHeightHash(number) } - return mockHeightHash(number), true, nil + return hashes, nil +} + +func (m *mockOracleBackend) resolvedRanges() [][2]uint64 { + m.canonicalMu.Lock() + defer m.canonicalMu.Unlock() + return slices.Clone(m.canonicalRanges) } func (m *mockOracleBackend) FrozenBlocks() (uint64, error) { @@ -385,17 +398,44 @@ func TestFeeHistory_FrozenRangeCachesByNumberWithoutResolution(t *testing.T) { _, _, _, _, _, _, err := oracle.FeeHistory(context.Background(), 4, rpc.LatestBlockNumber, nil) require.NoError(t, err) - require.EqualValues(t, 0, backend.canonicalCalls.Load(), + require.Empty(t, backend.resolvedRanges(), "the frozen range must not resolve hashes: the mapping is immutable") fetchesAfterFirst := backend.headerCalls.Load() _, _, _, _, _, _, err = oracle.FeeHistory(context.Background(), 4, rpc.LatestBlockNumber, nil) require.NoError(t, err) - require.EqualValues(t, 0, backend.canonicalCalls.Load()) + require.Empty(t, backend.resolvedRanges()) require.Equal(t, fetchesAfterFirst, backend.headerCalls.Load(), "the second request must be served from number-keyed cache entries") } +// TestFeeHistory_HotRangeResolvedInOneScan pins the cost of the cache-key +// resolution above the frozen boundary: one range resolution per request, +// whatever the window size. Per-block resolution would turn a memoized +// eth_feeHistory into one remote round trip per block in rpcdaemon mode. +func TestFeeHistory_HotRangeResolvedInOneScan(t *testing.T) { + head := types.NewEmptyHeaderForAssembling() + head.Number.SetUint64(20) + head.GasLimit = 30_000_000 + head.BaseFee = uint256.NewInt(1_000_000_000) + + backend := &mockOracleBackend{head: head} + oracle := gasprice.NewOracle(backend, gaspricecfg.Config{Blocks: 2, Percentile: 60}, jsonrpc.NewGasPriceCache(), gasprice.NewFeeHistoryCache(), log.New()) + + _, _, _, _, _, _, err := oracle.FeeHistory(context.Background(), 8, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.Equal(t, [][2]uint64{{13, 20}}, backend.resolvedRanges(), + "the whole hot window must be resolved by a single scan") + fetchesAfterFirst := backend.headerCalls.Load() + + _, _, _, _, _, _, err = oracle.FeeHistory(context.Background(), 8, rpc.LatestBlockNumber, nil) + require.NoError(t, err) + require.Equal(t, [][2]uint64{{13, 20}, {13, 20}}, backend.resolvedRanges(), + "a warm request must not cost more than its one scan") + require.Equal(t, fetchesAfterFirst, backend.headerCalls.Load(), + "the second request must be served from hash-keyed cache entries") +} + func TestFeeHistoryResolvesSafeAndFinalizedBlocks(t *testing.T) { head := types.NewEmptyHeaderForAssembling() head.Number.SetUint64(25) diff --git a/rpc/jsonrpc/eth_system.go b/rpc/jsonrpc/eth_system.go index 7767e501747..e1633f1e18e 100644 --- a/rpc/jsonrpc/eth_system.go +++ b/rpc/jsonrpc/eth_system.go @@ -18,6 +18,7 @@ package jsonrpc import ( "context" + "encoding/binary" "errors" "fmt" "math" @@ -26,9 +27,11 @@ import ( "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/hexutil" + "github.com/erigontech/erigon/common/length" "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/db/kv/kvcfg" "github.com/erigontech/erigon/db/kv/membatchwithdb" + "github.com/erigontech/erigon/db/kv/order" "github.com/erigontech/erigon/db/kv/prune" "github.com/erigontech/erigon/db/rawdb" "github.com/erigontech/erigon/execution/chain" @@ -522,19 +525,29 @@ func (b *GasPriceOracleBackend) Fork(ctx context.Context) (gasprice.OracleBacken nil } -// CanonicalHash resolves on the pinned tx first: the block reader may resolve -// on a live service (rpcdaemon mode), which would un-pin the fee-history cache -// key. The reader is only the fallback for frozen blocks, whose canonical -// mapping is immutable. -func (b *GasPriceOracleBackend) CanonicalHash(ctx context.Context, number uint64) (common.Hash, bool, error) { - hash, err := rawdb.ReadCanonicalHash(b.tx, number) +// CanonicalHashes scans the canonical markers of [from, to] on the pinned tx: +// resolving through the block reader would hit a live service in rpcdaemon +// mode, un-pinning the fee-history cache key, and a per-height read there is a +// round trip each. Callers only ask for unfrozen heights, whose markers are +// always in the db. +func (b *GasPriceOracleBackend) CanonicalHashes(_ context.Context, from, to uint64) ([]common.Hash, error) { + hashes := make([]common.Hash, to-from+1) + it, err := b.tx.Range(kv.HeaderCanonical, hexutil.EncodeTs(from), hexutil.EncodeTs(to+1), order.Asc, kv.Unlim) if err != nil { - return common.Hash{}, false, err + return nil, err } - if hash != (common.Hash{}) { - return hash, true, nil + defer it.Close() + for it.HasNext() { + k, v, err := it.Next() + if err != nil { + return nil, err + } + if len(k) != 8 || len(v) != length.Hash { + continue + } + hashes[binary.BigEndian.Uint64(k)-from] = common.BytesToHash(v) } - return b.baseApi._blockReader.CanonicalHash(ctx, b.tx, number) + return hashes, nil } // FrozenBlocks reads the Snapshots stage progress through the pinned tx — one diff --git a/rpc/jsonrpc/overlay_race_test.go b/rpc/jsonrpc/overlay_race_test.go index fba543db047..41b61798b3d 100644 --- a/rpc/jsonrpc/overlay_race_test.go +++ b/rpc/jsonrpc/overlay_race_test.go @@ -775,12 +775,13 @@ func TestGasPriceOracle_CanonicalHashUsesPinnedView(t *testing.T) { sibling := publishSiblingOverlay(t, h) - hash, ok, err := backend.CanonicalHash(h.m.Ctx, h.overlayHeader.Number.Uint64()) + overlayNumber := h.overlayHeader.Number.Uint64() + hashes, err := backend.CanonicalHashes(h.m.Ctx, overlayNumber, overlayNumber) require.NoError(t, err) - require.True(t, ok) - require.NotEqual(t, sibling.Hash(), hash, + require.Len(t, hashes, 1) + require.NotEqual(t, sibling.Hash(), hashes[0], "the cache key must not come from the live view the reader resolves on") - require.Equal(t, h.overlayHeader.Hash(), hash, + require.Equal(t, h.overlayHeader.Hash(), hashes[0], "the cache key must resolve on the pinned view") }