mirror of
https://github.com/ethereum/go-ethereum.git
synced 2026-05-24 08:49:29 +00:00
internal/ethapi, eth, eth/catalyst: gate eth_syncing on CL handshake
eth_syncing currently returns false as soon as the local downloader
believes the chain to be done. On a freshly started node this happens
before the consensus client has talked to it: the persisted head loads
into memory, no CL handshake has occurred, the downloader sees nothing
to do, Progress.Done() is true, eth_syncing reports synced.
That is wrong from an operator perspective. Load balancers (HAProxy,
NGINX), L2 supervisors and multi-node setups commonly gate routing on
eth_syncing. They start sending live traffic to a node that has not
actually learned about any new head yet, which surfaces as missing
state, stale reads, and unhealthy upstreams.
Maintainer-endorsed direction in the issue thread: "default geth to
'syncing' on startup and only switch to 'synced' once we learn about
a new block".
Implement that with a sticky atomic.Bool on *Ethereum, set the first
time the consensus layer drives the node via the Engine API
(ForkchoiceUpdated or NewPayload), and consulted from eth_syncing.
- eth/backend.go: add Ethereum.clContacted with
MarkConsensusContacted/ConsensusContacted helpers
- eth/catalyst/api.go: call MarkConsensusContacted at the same point
where lastForkchoiceUpdate / lastNewPayloadUpdate are stamped, so
the gate flips on every CL message regardless of the response
status (handshake recorded even when we reply STATUS_SYNCING)
- internal/ethapi/backend.go: add ConsensusContacted() to the Backend
interface and to the two test mocks (api_test.go testBackend,
transaction_args_test.go backendMock; both default to true so
existing tests keep their original semantics)
- eth/api_backend.go: implement ConsensusContacted on EthAPIBackend
- internal/ethapi/api.go: in EthereumAPI.Syncing, only short-circuit
to "false" when both progress.Done() AND ConsensusContacted() are
true; otherwise return the progress map as during an active sync
Adds dedicated tests in internal/ethapi/syncing_test.go covering:
- the new gate (Done but no CL contact -> truthy progress)
- normal post-handshake behavior (Done + CL contact -> false)
- active-sync behavior is unchanged regardless of the gate
Refs #33687.
This commit is contained in:
parent
21c5a287f9
commit
238b160a67
8 changed files with 146 additions and 2 deletions
|
|
@ -422,6 +422,12 @@ func (b *EthAPIBackend) SyncProgress(ctx context.Context) ethereum.SyncProgress
|
|||
return prog
|
||||
}
|
||||
|
||||
// ConsensusContacted reports whether the consensus layer has driven this node
|
||||
// via the Engine API at least once since process start.
|
||||
func (b *EthAPIBackend) ConsensusContacted() bool {
|
||||
return b.eth.ConsensusContacted()
|
||||
}
|
||||
|
||||
func (b *EthAPIBackend) SuggestGasTipCap(ctx context.Context) (*big.Int, error) {
|
||||
return b.gpo.SuggestTipCap(ctx)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import (
|
|||
"math/big"
|
||||
"runtime"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/ethereum/go-ethereum/accounts"
|
||||
|
|
@ -123,6 +124,26 @@ type Ethereum struct {
|
|||
lock sync.RWMutex // Protects the variadic fields (e.g. gas price and etherbase)
|
||||
|
||||
shutdownTracker *shutdowncheck.ShutdownTracker // Tracks if and when the node has shutdown ungracefully
|
||||
|
||||
// clContacted records whether the consensus client has ever spoken to us
|
||||
// via the Engine API. Until that happens, the node has not learned about
|
||||
// any new head from the network and cannot truthfully report itself as
|
||||
// "synced" — eth_syncing falls back to reporting an in-progress sync.
|
||||
clContacted atomic.Bool
|
||||
}
|
||||
|
||||
// MarkConsensusContacted records that the consensus layer has driven this node
|
||||
// at least once via the Engine API. The flag is sticky: once set, it stays set
|
||||
// for the lifetime of the process. eth_syncing uses it to avoid reporting a
|
||||
// freshly started node as "synced" before any CL handshake has occurred.
|
||||
func (s *Ethereum) MarkConsensusContacted() {
|
||||
s.clContacted.Store(true)
|
||||
}
|
||||
|
||||
// ConsensusContacted reports whether the consensus layer has ever driven this
|
||||
// node via the Engine API since process start.
|
||||
func (s *Ethereum) ConsensusContacted() bool {
|
||||
return s.clContacted.Load()
|
||||
}
|
||||
|
||||
// New creates a new Ethereum object (including the initialisation of the common Ethereum object),
|
||||
|
|
|
|||
|
|
@ -251,6 +251,9 @@ func (api *ConsensusAPI) forkchoiceUpdated(ctx context.Context, update engine.Fo
|
|||
}
|
||||
// Stash away the last update to warn the user if the beacon client goes offline
|
||||
api.lastForkchoiceUpdate.Store(time.Now().Unix())
|
||||
// Record that the consensus layer has driven us at least once. eth_syncing
|
||||
// uses this to avoid claiming "synced" before any CL handshake has occurred.
|
||||
api.eth.MarkConsensusContacted()
|
||||
|
||||
// Check whether we have the block yet in our database or not. If not, we'll
|
||||
// need to either trigger a sync, or to reject this forkchoice update for a
|
||||
|
|
@ -846,6 +849,9 @@ func (api *ConsensusAPI) newPayload(ctx context.Context, params engine.Executabl
|
|||
}
|
||||
// Stash away the last update to warn the user if the beacon client goes offline
|
||||
api.lastNewPayloadUpdate.Store(time.Now().Unix())
|
||||
// Record that the consensus layer has driven us at least once. eth_syncing
|
||||
// uses this to avoid claiming "synced" before any CL handshake has occurred.
|
||||
api.eth.MarkConsensusContacted()
|
||||
|
||||
// If we already have the block locally, ignore the entire execution and just
|
||||
// return a fake success.
|
||||
|
|
|
|||
|
|
@ -153,11 +153,20 @@ func (api *EthereumAPI) BlobBaseFee(ctx context.Context) *hexutil.Big {
|
|||
// - highestBlock: block number of the highest block header this node has received from peers
|
||||
// - pulledStates: number of state entries processed until now
|
||||
// - knownStates: number of known state entries that still need to be pulled
|
||||
//
|
||||
// Until the consensus layer has driven the node at least once via the Engine
|
||||
// API, the node has not actually learned about any new chain head and cannot
|
||||
// truthfully report itself as "synced". In that case Syncing returns the
|
||||
// progress object regardless of whether progress.Done() would be true.
|
||||
func (api *EthereumAPI) Syncing(ctx context.Context) (interface{}, error) {
|
||||
progress := api.b.SyncProgress(ctx)
|
||||
|
||||
// Return not syncing if the synchronisation already completed
|
||||
if progress.Done() {
|
||||
// Return not syncing if the synchronisation already completed AND we have
|
||||
// observed at least one Engine API call from the consensus layer. The CL
|
||||
// gate prevents a freshly started node from being advertised as synced
|
||||
// before any CL handshake has happened (a common operational footgun for
|
||||
// load balancers and L2 stacks gating on eth_syncing).
|
||||
if progress.Done() && api.b.ConsensusContacted() {
|
||||
return false, nil
|
||||
}
|
||||
// Otherwise gather the block sync stats
|
||||
|
|
|
|||
|
|
@ -493,6 +493,7 @@ func newTestBackend(t *testing.T, n int, gspec *core.Genesis, engine consensus.E
|
|||
func (b testBackend) SyncProgress(ctx context.Context) ethereum.SyncProgress {
|
||||
return ethereum.SyncProgress{}
|
||||
}
|
||||
func (b testBackend) ConsensusContacted() bool { return true }
|
||||
func (b testBackend) SuggestGasTipCap(ctx context.Context) (*big.Int, error) {
|
||||
return big.NewInt(0), nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -42,6 +42,10 @@ import (
|
|||
type Backend interface {
|
||||
// General Ethereum API
|
||||
SyncProgress(ctx context.Context) ethereum.SyncProgress
|
||||
// ConsensusContacted reports whether the consensus layer has driven this
|
||||
// node via the Engine API at least once since process start. eth_syncing
|
||||
// uses it to avoid reporting "synced" before any CL handshake has happened.
|
||||
ConsensusContacted() bool
|
||||
|
||||
SuggestGasTipCap(ctx context.Context) (*big.Int, error)
|
||||
FeeHistory(ctx context.Context, blockCount uint64, lastBlock rpc.BlockNumber, rewardPercentiles []float64) (*big.Int, [][]*big.Int, []*big.Int, []float64, []*big.Int, []float64, error)
|
||||
|
|
|
|||
96
internal/ethapi/syncing_test.go
Normal file
96
internal/ethapi/syncing_test.go
Normal file
|
|
@ -0,0 +1,96 @@
|
|||
// Copyright 2026 The go-ethereum Authors
|
||||
// This file is part of the go-ethereum library.
|
||||
//
|
||||
// The go-ethereum library 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.
|
||||
//
|
||||
// The go-ethereum library 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 the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
|
||||
|
||||
package ethapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/ethereum/go-ethereum"
|
||||
)
|
||||
|
||||
// syncingBackend is a minimal Backend embedding that only implements the two
|
||||
// methods Syncing calls. Embedding the interface avoids pulling in the full
|
||||
// testBackend setup just to flip a single bool.
|
||||
type syncingBackend struct {
|
||||
Backend
|
||||
progress ethereum.SyncProgress
|
||||
contacted bool
|
||||
}
|
||||
|
||||
func (b *syncingBackend) SyncProgress(_ context.Context) ethereum.SyncProgress { return b.progress }
|
||||
func (b *syncingBackend) ConsensusContacted() bool { return b.contacted }
|
||||
|
||||
// TestSyncingReportsBeforeConsensusContact verifies that eth_syncing returns a
|
||||
// truthy progress object until the consensus layer has driven the node via the
|
||||
// Engine API at least once, even when the local downloader believes itself to
|
||||
// be done.
|
||||
func TestSyncingReportsBeforeConsensusContact(t *testing.T) {
|
||||
api := NewEthereumAPI(&syncingBackend{
|
||||
// progress.Done() returns true on a zero-valued struct because all
|
||||
// remaining counters are zero and CurrentBlock >= HighestBlock.
|
||||
progress: ethereum.SyncProgress{},
|
||||
contacted: false,
|
||||
})
|
||||
res, err := api.Syncing(context.Background())
|
||||
if err != nil {
|
||||
t.Fatalf("Syncing returned error: %v", err)
|
||||
}
|
||||
if v, ok := res.(bool); ok && !v {
|
||||
t.Fatal("expected truthy syncing payload before CL handshake, got false")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSyncingReportsFalseAfterConsensusContact verifies that once the
|
||||
// consensus layer has handshaken at least once and progress.Done() is true,
|
||||
// eth_syncing reports false.
|
||||
func TestSyncingReportsFalseAfterConsensusContact(t *testing.T) {
|
||||
api := NewEthereumAPI(&syncingBackend{
|
||||
progress: ethereum.SyncProgress{},
|
||||
contacted: true,
|
||||
})
|
||||
res, err := api.Syncing(context.Background())
|
||||
if err != nil {
|
||||
t.Fatalf("Syncing returned error: %v", err)
|
||||
}
|
||||
v, ok := res.(bool)
|
||||
if !ok || v {
|
||||
t.Fatalf("expected false after CL handshake when sync is done, got %v", res)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSyncingReportsActiveSyncEvenWithoutConsensusContact verifies that when
|
||||
// the downloader is actively syncing, eth_syncing returns the progress map
|
||||
// regardless of the CL gate. This preserves the legacy semantics for the case
|
||||
// the issue thread did not affect.
|
||||
func TestSyncingReportsActiveSyncEvenWithoutConsensusContact(t *testing.T) {
|
||||
api := NewEthereumAPI(&syncingBackend{
|
||||
progress: ethereum.SyncProgress{
|
||||
StartingBlock: 100,
|
||||
CurrentBlock: 150,
|
||||
HighestBlock: 200, // CurrentBlock < HighestBlock => Done()=false
|
||||
},
|
||||
contacted: false,
|
||||
})
|
||||
res, err := api.Syncing(context.Background())
|
||||
if err != nil {
|
||||
t.Fatalf("Syncing returned error: %v", err)
|
||||
}
|
||||
if _, ok := res.(map[string]interface{}); !ok {
|
||||
t.Fatalf("expected progress map during active sync, got %T", res)
|
||||
}
|
||||
}
|
||||
|
|
@ -326,6 +326,7 @@ func (b *backendMock) ChainConfig() *params.ChainConfig { return b.config }
|
|||
func (b *backendMock) SyncProgress(ctx context.Context) ethereum.SyncProgress {
|
||||
return ethereum.SyncProgress{}
|
||||
}
|
||||
func (b *backendMock) ConsensusContacted() bool { return true }
|
||||
func (b *backendMock) FeeHistory(ctx context.Context, blockCount uint64, lastBlock rpc.BlockNumber, rewardPercentiles []float64) (*big.Int, [][]*big.Int, []*big.Int, []float64, []*big.Int, []float64, error) {
|
||||
return nil, nil, nil, nil, nil, nil, nil
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue