mirror of
https://github.com/ethereum/go-ethereum.git
synced 2026-05-24 08:49:29 +00:00
internal/ethapi, eth: only gate eth_syncing when Engine API is registered
The previous version of this change unconditionally returned the progress
map until the consensus client had driven the node at least once. That
broke ethclient.TestEthClient/StatusFunctions and any other backend that
runs without a consensus client (in-process tests, --dev mode without
catalyst, light/legacy backends), where reporting "syncing" forever is
clearly wrong.
Split the gate into two flags:
- clExpected: set in eth/catalyst.Register, the only entry point that
attaches the Engine API to a node. If a backend never calls Register,
it is not paired with a consensus client.
- clContacted: set on every Engine API call (forkchoiceUpdated and
newPayload), unchanged from before.
Replace ConsensusContacted on the Backend interface with ConsensusReady,
which folds the two flags into the question eth_syncing actually wants
answered: "is the synced claim meaningful right now?" Backends that
never expect a CL answer yes immediately, preserving legacy behavior.
Backends that do expect one answer yes only after the first FCU/NewPayload.
- eth/backend.go: clExpected, clContacted, MarkConsensusExpected,
MarkConsensusContacted, ConsensusReady on (*Ethereum)
- eth/catalyst/api.go: backend.MarkConsensusExpected() in Register
- eth/api_backend.go: ConsensusReady delegates to (*Ethereum)
- internal/ethapi/backend.go: rename interface method to ConsensusReady
- internal/ethapi/api.go: Syncing checks ConsensusReady
- internal/ethapi/{api_test,transaction_args_test}.go: rename the test
mock methods (default to true so existing tests are unaffected)
- internal/ethapi/syncing_test.go: rename the helper field; tests now
cover (a) CL-paired node before handshake -> truthy, (b) ready node
-> false, (c) active sync -> progress map regardless of gate
Refs #33687.
This commit is contained in:
parent
238b160a67
commit
20d6757391
8 changed files with 68 additions and 43 deletions
|
|
@ -422,10 +422,10 @@ 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()
|
||||
// ConsensusReady reports whether the node's "synced" claim is currently
|
||||
// meaningful. See (*Ethereum).ConsensusReady for the gating rules.
|
||||
func (b *EthAPIBackend) ConsensusReady() bool {
|
||||
return b.eth.ConsensusReady()
|
||||
}
|
||||
|
||||
func (b *EthAPIBackend) SuggestGasTipCap(ctx context.Context) (*big.Int, error) {
|
||||
|
|
|
|||
|
|
@ -125,24 +125,40 @@ type Ethereum struct {
|
|||
|
||||
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.
|
||||
// clExpected is set when the Engine API is registered on this node
|
||||
// (catalyst.Register). When unset (no Engine API attached, e.g. in
|
||||
// tests, in --dev mode without catalyst, in light/legacy backends),
|
||||
// the node cannot be paired with a consensus client and should not be
|
||||
// gated on a CL handshake. clContacted records whether the consensus
|
||||
// client has spoken to us via the Engine API at least once.
|
||||
clExpected atomic.Bool
|
||||
clContacted atomic.Bool
|
||||
}
|
||||
|
||||
// MarkConsensusExpected records that this node expects to be driven by a
|
||||
// consensus layer via the Engine API. It should be invoked once during node
|
||||
// setup, when the Engine API is registered.
|
||||
func (s *Ethereum) MarkConsensusExpected() {
|
||||
s.clExpected.Store(true)
|
||||
}
|
||||
|
||||
// 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.
|
||||
// for the lifetime of the process.
|
||||
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 {
|
||||
// ConsensusReady reports whether the node's "synced" claim is meaningful right
|
||||
// now. If the node does not expect a consensus client (no Engine API), the
|
||||
// answer is always yes. If it does, the answer is yes only after the consensus
|
||||
// client has driven the node at least once. eth_syncing uses this to avoid
|
||||
// claiming "synced" before any CL handshake has occurred on freshly started
|
||||
// post-merge nodes.
|
||||
func (s *Ethereum) ConsensusReady() bool {
|
||||
if !s.clExpected.Load() {
|
||||
return true
|
||||
}
|
||||
return s.clContacted.Load()
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -49,6 +49,11 @@ import (
|
|||
|
||||
// Register adds the engine API and related APIs to the full node.
|
||||
func Register(stack *node.Node, backend *eth.Ethereum) error {
|
||||
// Mark the backend as expecting a consensus client. eth_syncing uses
|
||||
// this to gate "synced" responses on a CL handshake: a node configured
|
||||
// to take direction from a CL should not advertise itself as synced
|
||||
// until that CL has actually driven it at least once.
|
||||
backend.MarkConsensusExpected()
|
||||
stack.RegisterAPIs([]rpc.API{
|
||||
newTestingAPI(backend),
|
||||
{
|
||||
|
|
|
|||
|
|
@ -154,19 +154,19 @@ func (api *EthereumAPI) BlobBaseFee(ctx context.Context) *hexutil.Big {
|
|||
// - 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.
|
||||
// On post-merge nodes paired with a consensus client, Syncing also waits for
|
||||
// the consensus client to drive the node at least once before reporting
|
||||
// "false". This avoids advertising a freshly started node as synced before any
|
||||
// CL handshake has happened — see (*Ethereum).ConsensusReady. Backends without
|
||||
// an Engine API (tests, --dev mode without catalyst, light clients) report
|
||||
// ready immediately, preserving legacy behavior.
|
||||
func (api *EthereumAPI) Syncing(ctx context.Context) (interface{}, error) {
|
||||
progress := api.b.SyncProgress(ctx)
|
||||
|
||||
// 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 not syncing if the synchronisation already completed AND, for
|
||||
// backends that expect one, the consensus client has driven the node at
|
||||
// least once.
|
||||
if progress.Done() && api.b.ConsensusReady() {
|
||||
return false, nil
|
||||
}
|
||||
// Otherwise gather the block sync stats
|
||||
|
|
|
|||
|
|
@ -493,7 +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) ConsensusReady() bool { return true }
|
||||
func (b testBackend) SuggestGasTipCap(ctx context.Context) (*big.Int, error) {
|
||||
return big.NewInt(0), nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -42,10 +42,13 @@ 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
|
||||
// ConsensusReady reports whether the node's "synced" claim is currently
|
||||
// meaningful. For backends that do not expect a consensus client (no
|
||||
// Engine API attached) the answer is always true. For backends that do,
|
||||
// the answer flips to true only after the consensus client has driven
|
||||
// the node at least once. eth_syncing uses this to avoid reporting
|
||||
// "synced" before any CL handshake has happened.
|
||||
ConsensusReady() 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)
|
||||
|
|
|
|||
|
|
@ -28,23 +28,24 @@ import (
|
|||
// testBackend setup just to flip a single bool.
|
||||
type syncingBackend struct {
|
||||
Backend
|
||||
progress ethereum.SyncProgress
|
||||
contacted bool
|
||||
progress ethereum.SyncProgress
|
||||
ready bool
|
||||
}
|
||||
|
||||
func (b *syncingBackend) SyncProgress(_ context.Context) ethereum.SyncProgress { return b.progress }
|
||||
func (b *syncingBackend) ConsensusContacted() bool { return b.contacted }
|
||||
func (b *syncingBackend) ConsensusReady() bool { return b.ready }
|
||||
|
||||
// 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.
|
||||
// TestSyncingReportsBeforeConsensusContact verifies that on a CL-paired node
|
||||
// (ConsensusReady false), eth_syncing returns a truthy progress object even
|
||||
// when the local downloader believes itself to be done. This is the bug fix
|
||||
// for issue #33687: a freshly started node must not advertise itself as
|
||||
// "synced" before the consensus client has actually driven it.
|
||||
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,
|
||||
progress: ethereum.SyncProgress{},
|
||||
ready: false,
|
||||
})
|
||||
res, err := api.Syncing(context.Background())
|
||||
if err != nil {
|
||||
|
|
@ -56,12 +57,12 @@ func TestSyncingReportsBeforeConsensusContact(t *testing.T) {
|
|||
}
|
||||
|
||||
// TestSyncingReportsFalseAfterConsensusContact verifies that once the
|
||||
// consensus layer has handshaken at least once and progress.Done() is true,
|
||||
// eth_syncing reports false.
|
||||
// consensus layer has handshaken at least once (or the backend does not
|
||||
// expect one) and progress.Done() is true, eth_syncing reports false.
|
||||
func TestSyncingReportsFalseAfterConsensusContact(t *testing.T) {
|
||||
api := NewEthereumAPI(&syncingBackend{
|
||||
progress: ethereum.SyncProgress{},
|
||||
contacted: true,
|
||||
progress: ethereum.SyncProgress{},
|
||||
ready: true,
|
||||
})
|
||||
res, err := api.Syncing(context.Background())
|
||||
if err != nil {
|
||||
|
|
@ -84,7 +85,7 @@ func TestSyncingReportsActiveSyncEvenWithoutConsensusContact(t *testing.T) {
|
|||
CurrentBlock: 150,
|
||||
HighestBlock: 200, // CurrentBlock < HighestBlock => Done()=false
|
||||
},
|
||||
contacted: false,
|
||||
ready: false,
|
||||
})
|
||||
res, err := api.Syncing(context.Background())
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -326,7 +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) ConsensusReady() 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