From c472a1e9a3123cea4fd672b32bd09e58a59e3417 Mon Sep 17 00:00:00 2001 From: Oleksandr Lystopad Date: Tue, 7 Jul 2026 20:15:22 +0200 Subject: [PATCH 1/5] cl, engineapi: encode client versions in default block graffiti When a validator proposes a block without specifying graffiti, Caplin now fills it with the client-version graffiti standard instead of the literal "Caplin" string, so client-diversity tooling can attribute the block to its execution and consensus clients: e.g. EGa53eCNa53e Caplin uses the reserved consensus client code CN. The execution client's code and commit are obtained via engine_getClientVersionV1, which is added to the ExecutionEngine interface; the result is cached so steady-state block production stays off the engine API. When the execution client does not support the method, the graffiti degrades to the consensus client identifier only. User-specified graffiti is unaffected. Standard: https://github.com/ethereum/execution-apis/blob/main/src/engine/identification.md --- cl/beacon/handler/block_production.go | 64 ++++++++++++-- .../handler/block_production_graffiti_test.go | 86 +++++++++++++++++++ cl/beacon/handler/handler.go | 3 + .../execution_client_direct.go | 5 ++ .../execution_client_engine.go | 4 + .../execution_client/execution_engine_mock.go | 39 +++++++++ cl/phase1/execution_client/interface.go | 2 + cl/phase1/stages/gloas_payload_test.go | 4 + cl/spectest/consensus_tests/fork_choice.go | 4 + execution/engineapi/engine_api_methods.go | 16 +--- execution/engineapi/engine_types/jsonrpc.go | 24 ++++++ 11 files changed, 231 insertions(+), 20 deletions(-) create mode 100644 cl/beacon/handler/block_production_graffiti_test.go diff --git a/cl/beacon/handler/block_production.go b/cl/beacon/handler/block_production.go index eda77114f9a..d2de138437e 100644 --- a/cl/beacon/handler/block_production.go +++ b/cl/beacon/handler/block_production.go @@ -19,7 +19,6 @@ package handler import ( "bytes" "context" - "encoding/hex" "encoding/json" "errors" "fmt" @@ -61,6 +60,7 @@ import ( "github.com/erigontech/erigon/common/length" "github.com/erigontech/erigon/common/log/v3" "github.com/erigontech/erigon/db/kv" + "github.com/erigontech/erigon/db/version" "github.com/erigontech/erigon/execution/engineapi/engine_types" "github.com/erigontech/erigon/execution/protocol/params" "github.com/erigontech/erigon/execution/rlp" @@ -80,7 +80,10 @@ var ( errBuilderNotEnabled = errors.New("builder is not enabled") ) -var defaultGraffitiString = "Caplin" +const ( + caplinClientCode = "CN" + caplinClientName = "caplin" +) const minPayloadPollingWindow = 100 * time.Millisecond @@ -89,6 +92,55 @@ const minPayloadPollingWindow = 100 * time.Millisecond // produced block still reaches attesters in time to earn the proposer boost. const payloadPublicationDivisor = 4 +// defaultGraffiti is used when the validator does not specify a graffiti. It follows the +// client-version graffiti standard, encoding the execution and consensus client codes and +// their commit prefixes so client-diversity tooling can attribute proposed blocks. See +// https://github.com/ethereum/execution-apis/blob/main/src/engine/identification.md +func (a *ApiHandler) defaultGraffiti(ctx context.Context) common.Hash { + graffiti := caplinClientCode + graffitiCommitPrefix(version.GitCommit) + if el := a.executionClientVersion(ctx); el != nil { + graffiti = el.Code + graffitiCommitPrefix(el.Commit) + graffiti + } + return graffitiFromString(graffiti) +} + +// executionClientVersion returns the connected execution client's version, caching it on +// first success so that block production stays off the engine API in steady state (the +// version is static for the lifetime of an execution client connection). +func (a *ApiHandler) executionClientVersion(ctx context.Context) *engine_types.ClientVersionV1 { + if cached := a.elClientVersion.Load(); cached != nil { + return cached + } + if a.engine == nil { + return nil + } + ctx, cancel := context.WithTimeout(ctx, time.Second) + defer cancel() + caplin := engine_types.NewClientVersionV1(caplinClientCode, caplinClientName, a.version, version.GitCommit) + versions, err := a.engine.GetClientVersionV1(ctx, &caplin) + if err != nil || len(versions) == 0 { + return nil + } + el := versions[0] + a.elClientVersion.Store(&el) + return &el +} + +// graffitiCommitPrefix returns the leading 2 bytes (4 hex chars) of a commit hash. +func graffitiCommitPrefix(commit string) string { + commit = strings.TrimPrefix(commit, "0x") + if len(commit) >= 4 { + return commit[:4] + } + return commit + strings.Repeat("0", 4-len(commit)) +} + +func graffitiFromString(s string) common.Hash { + var graffiti common.Hash + copy(graffiti[:], s) + return graffiti +} + type blockBuilderWindow struct { firstGetAt time.Time pollUntil time.Time @@ -310,9 +362,11 @@ func (a *ApiHandler) GetEthV3ValidatorBlock( if r.URL.Query().Has("skip_randao_verification") { randaoReveal = common.Bytes96{0xc0} // infinity bls signature } - graffiti := common.HexToHash(r.URL.Query().Get("graffiti")) - if !r.URL.Query().Has("graffiti") { - graffiti = common.HexToHash(hex.EncodeToString([]byte(defaultGraffitiString))) + var graffiti common.Hash + if r.URL.Query().Has("graffiti") { + graffiti = common.HexToHash(r.URL.Query().Get("graffiti")) + } else { + graffiti = a.defaultGraffiti(ctx) } tx, err := a.indiciesDB.BeginRo(ctx) diff --git a/cl/beacon/handler/block_production_graffiti_test.go b/cl/beacon/handler/block_production_graffiti_test.go new file mode 100644 index 00000000000..fdbb7048200 --- /dev/null +++ b/cl/beacon/handler/block_production_graffiti_test.go @@ -0,0 +1,86 @@ +// 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 handler + +import ( + "bytes" + "context" + "errors" + "testing" + + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/erigontech/erigon/cl/phase1/execution_client" + "github.com/erigontech/erigon/common" + "github.com/erigontech/erigon/db/version" + "github.com/erigontech/erigon/execution/engineapi/engine_types" +) + +func graffitiText(g common.Hash) string { + return string(bytes.TrimRight(g[:], "\x00")) +} + +func TestGraffitiCommitPrefix(t *testing.T) { + require.Equal(t, "a53e", graffitiCommitPrefix("0xa53e9545")) + require.Equal(t, "a53e", graffitiCommitPrefix("a53e9545")) + require.Equal(t, "ab00", graffitiCommitPrefix("ab")) + require.Equal(t, "0000", graffitiCommitPrefix("")) +} + +func TestDefaultGraffiti(t *testing.T) { + clCommit := graffitiCommitPrefix(version.GitCommit) + + t.Run("execution client version available", func(t *testing.T) { + ctrl := gomock.NewController(t) + engine := execution_client.NewMockExecutionEngine(ctrl) + engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). + Return([]engine_types.ClientVersionV1{{Code: "GE", Commit: "0xc3d4e5f6"}}, nil) + + a := &ApiHandler{engine: engine, version: "1.2.3"} + require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) + }) + + t.Run("execution client version unavailable falls back to consensus-only", func(t *testing.T) { + ctrl := gomock.NewController(t) + engine := execution_client.NewMockExecutionEngine(ctrl) + engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). + Return(nil, errors.New("not supported")) + + a := &ApiHandler{engine: engine, version: "1.2.3"} + require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) + }) + + t.Run("no engine falls back to consensus-only", func(t *testing.T) { + a := &ApiHandler{version: "1.2.3"} + require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) + }) + + t.Run("execution client version is cached across calls", func(t *testing.T) { + ctrl := gomock.NewController(t) + engine := execution_client.NewMockExecutionEngine(ctrl) + engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). + Return([]engine_types.ClientVersionV1{{Code: "GE", Commit: "0xc3d4e5f6"}}, nil). + Times(1) + + a := &ApiHandler{engine: engine, version: "1.2.3"} + first := graffitiText(a.defaultGraffiti(context.Background())) + second := graffitiText(a.defaultGraffiti(context.Background())) + require.Equal(t, first, second) + require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, second) + }) +} diff --git a/cl/beacon/handler/handler.go b/cl/beacon/handler/handler.go index 1b5eec39a6c..16d921aa896 100644 --- a/cl/beacon/handler/handler.go +++ b/cl/beacon/handler/handler.go @@ -20,6 +20,7 @@ import ( "errors" "net/http" "sync" + "sync/atomic" "github.com/go-chi/chi/v5" @@ -51,6 +52,7 @@ import ( "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/db/snapshotsync" "github.com/erigontech/erigon/db/snapshotsync/freezeblocks" + "github.com/erigontech/erigon/execution/engineapi/engine_types" "github.com/erigontech/erigon/node/gointerfaces/sentinelproto" ) @@ -108,6 +110,7 @@ type ApiHandler struct { validatorParams *validator_params.ValidatorParams blobBundles *lru.Cache[common.Bytes48, BlobBundle] // Keep recent bundled blobs from the execution layer. engine execution_client.ExecutionEngine + elClientVersion atomic.Pointer[engine_types.ClientVersionV1] // Cached execution client version for default graffiti. syncMessagePool sync_contribution_pool.SyncContributionPool committeeSub *committee_subscription.CommitteeSubscribeMgmt attestationProducer attestation_producer.AttestationDataProducer diff --git a/cl/phase1/execution_client/execution_client_direct.go b/cl/phase1/execution_client/execution_client_direct.go index c4c97349114..cec661ce36f 100644 --- a/cl/phase1/execution_client/execution_client_direct.go +++ b/cl/phase1/execution_client/execution_client_direct.go @@ -231,3 +231,8 @@ func (cc *ExecutionClientDirect) GetBlobs(ctx context.Context, versionedHashes [ } return blobs, proofs, nil } + +// In direct mode the execution layer is the in-process Erigon node, so report it directly. +func (cc *ExecutionClientDirect) GetClientVersionV1(_ context.Context, _ *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error) { + return []engine_types.ClientVersionV1{engine_types.LocalClientVersionV1()}, nil +} diff --git a/cl/phase1/execution_client/execution_client_engine.go b/cl/phase1/execution_client/execution_client_engine.go index a74bade6a73..8321d4f503e 100644 --- a/cl/phase1/execution_client/execution_client_engine.go +++ b/cl/phase1/execution_client/execution_client_engine.go @@ -611,3 +611,7 @@ func (cc *ExecutionClientEngine) GetBlobs(ctx context.Context, versionedHashes [ } return blobs, proofs, nil } + +func (cc *ExecutionClientEngine) GetClientVersionV1(ctx context.Context, callerVersion *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error) { + return cc.engine.GetClientVersionV1(ctx, callerVersion) +} diff --git a/cl/phase1/execution_client/execution_engine_mock.go b/cl/phase1/execution_client/execution_engine_mock.go index 6393a5218b9..3f3f1ba52e8 100644 --- a/cl/phase1/execution_client/execution_engine_mock.go +++ b/cl/phase1/execution_client/execution_engine_mock.go @@ -324,6 +324,45 @@ func (c *MockExecutionEngineGetBodiesByRangeCall) DoAndReturn(f func(context.Con return c } +// GetClientVersionV1 mocks base method. +func (m *MockExecutionEngine) GetClientVersionV1(ctx context.Context, callerVersion *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetClientVersionV1", ctx, callerVersion) + ret0, _ := ret[0].([]engine_types.ClientVersionV1) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetClientVersionV1 indicates an expected call of GetClientVersionV1. +func (mr *MockExecutionEngineMockRecorder) GetClientVersionV1(ctx, callerVersion any) *MockExecutionEngineGetClientVersionV1Call { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetClientVersionV1", reflect.TypeOf((*MockExecutionEngine)(nil).GetClientVersionV1), ctx, callerVersion) + return &MockExecutionEngineGetClientVersionV1Call{Call: call} +} + +// MockExecutionEngineGetClientVersionV1Call wrap *gomock.Call +type MockExecutionEngineGetClientVersionV1Call struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockExecutionEngineGetClientVersionV1Call) Return(arg0 []engine_types.ClientVersionV1, arg1 error) *MockExecutionEngineGetClientVersionV1Call { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockExecutionEngineGetClientVersionV1Call) Do(f func(context.Context, *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error)) *MockExecutionEngineGetClientVersionV1Call { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockExecutionEngineGetClientVersionV1Call) DoAndReturn(f func(context.Context, *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error)) *MockExecutionEngineGetClientVersionV1Call { + c.Call = c.Call.DoAndReturn(f) + return c +} + // HasBlock mocks base method. func (m *MockExecutionEngine) HasBlock(ctx context.Context, hash common.Hash) (bool, error) { m.ctrl.T.Helper() diff --git a/cl/phase1/execution_client/interface.go b/cl/phase1/execution_client/interface.go index ab473a39da4..862f01b56a3 100644 --- a/cl/phase1/execution_client/interface.go +++ b/cl/phase1/execution_client/interface.go @@ -56,4 +56,6 @@ type ExecutionEngine interface { // Blobs GetBlobs(ctx context.Context, versionedHashes []common.Hash, version clparams.StateVersion) (blobs [][]byte, proofs [][][]byte, err error) + // Client identification + GetClientVersionV1(ctx context.Context, callerVersion *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error) } diff --git a/cl/phase1/stages/gloas_payload_test.go b/cl/phase1/stages/gloas_payload_test.go index b09ba8ff907..54337315bd0 100644 --- a/cl/phase1/stages/gloas_payload_test.go +++ b/cl/phase1/stages/gloas_payload_test.go @@ -428,4 +428,8 @@ func (t *testExecutionEngine) GetBlobs(context.Context, []common.Hash, clparams. return nil, nil, nil } +func (t *testExecutionEngine) GetClientVersionV1(context.Context, *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error) { + return nil, nil +} + var _ execution_client.ExecutionEngine = (*testExecutionEngine)(nil) diff --git a/cl/spectest/consensus_tests/fork_choice.go b/cl/spectest/consensus_tests/fork_choice.go index 480042cc1b4..497294ea6be 100644 --- a/cl/spectest/consensus_tests/fork_choice.go +++ b/cl/spectest/consensus_tests/fork_choice.go @@ -111,6 +111,10 @@ func (forkChoiceSpectestEngine) GetBlobs(context.Context, []common.Hash, clparam return nil, nil, nil } +func (forkChoiceSpectestEngine) GetClientVersionV1(context.Context, *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error) { + return nil, nil +} + func (f *ForkChoiceStep) StepType() string { if f.PayloadStatus != nil { return "on_payload_info" diff --git a/execution/engineapi/engine_api_methods.go b/execution/engineapi/engine_api_methods.go index 5f28b66c2d6..9a06ec82099 100644 --- a/execution/engineapi/engine_api_methods.go +++ b/execution/engineapi/engine_api_methods.go @@ -25,7 +25,6 @@ import ( "github.com/erigontech/erigon/cl/clparams" "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/hexutil" - "github.com/erigontech/erigon/db/version" "github.com/erigontech/erigon/execution/engineapi/engine_helpers" "github.com/erigontech/erigon/execution/engineapi/engine_types" "github.com/erigontech/erigon/rpc" @@ -265,20 +264,7 @@ func (e *EngineServer) GetClientVersionV1(ctx context.Context, callerVersion *en if callerVersion != nil { e.logger.Info("[GetClientVersionV1] Received request from" + callerVersion.String()) } - commitString := version.GitCommit - if len(commitString) >= 8 { - commitString = commitString[:8] - } else { - commitString = "00000000" // shouldn't be triggered - } - result := make([]engine_types.ClientVersionV1, 1) - result[0] = engine_types.ClientVersionV1{ - Code: version.ClientCode, - Name: version.ClientName, - Version: version.VersionWithCommit(version.GitCommit), - Commit: "0x" + commitString, - } - return result, nil + return []engine_types.ClientVersionV1{engine_types.LocalClientVersionV1()}, nil } func (e *EngineServer) ExchangeCapabilities(fromCl []string) []string { diff --git a/execution/engineapi/engine_types/jsonrpc.go b/execution/engineapi/engine_types/jsonrpc.go index aa94a1df8b6..25470162c96 100644 --- a/execution/engineapi/engine_types/jsonrpc.go +++ b/execution/engineapi/engine_types/jsonrpc.go @@ -25,6 +25,7 @@ import ( "github.com/erigontech/erigon/cl/clparams" "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/hexutil" + "github.com/erigontech/erigon/db/version" "github.com/erigontech/erigon/execution/types" "github.com/erigontech/erigon/node/gointerfaces" "github.com/erigontech/erigon/node/gointerfaces/executionproto" @@ -180,6 +181,29 @@ func (c ClientVersionV1) String() string { return fmt.Sprintf("ClientCode: %s, %s-%s-%s", c.Code, c.Name, c.Version, c.Commit) } +// NewClientVersionV1 builds a ClientVersionV1 from a git commit hash, truncating +// it to the leading 4 bytes as required by +// https://github.com/ethereum/execution-apis/blob/main/src/engine/identification.md +func NewClientVersionV1(code, name, versionStr, gitCommit string) ClientVersionV1 { + commit := gitCommit + if len(commit) >= 8 { + commit = commit[:8] + } else { + commit = "00000000" + } + return ClientVersionV1{ + Code: code, + Name: name, + Version: versionStr, + Commit: "0x" + commit, + } +} + +// LocalClientVersionV1 returns the ClientVersionV1 describing this node. +func LocalClientVersionV1() ClientVersionV1 { + return NewClientVersionV1(version.ClientCode, version.ClientName, version.VersionWithCommit(version.GitCommit), version.GitCommit) +} + type StringifiedError struct{ err error } func NewStringifiedError(err error) *StringifiedError { From f7d2f8fc32754be76991711201d911feef328b5e Mon Sep 17 00:00:00 2001 From: Oleksandr Lystopad Date: Wed, 8 Jul 2026 07:40:15 +0200 Subject: [PATCH 2/5] cl/beacon/handler: cache execution client version when unavailable executionClientVersion only cached a successful engine_getClientVersionV1 response, so an execution client that does not implement the method (or returns an empty list) was re-queried on every block proposal, incurring the engine API round-trip and its 1s timeout each time. Memoize the unavailable outcome via a sentinel so steady-state block production stays off the engine API in that case too. --- cl/beacon/handler/block_production.go | 14 +++++++++++--- .../handler/block_production_graffiti_test.go | 10 +++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/cl/beacon/handler/block_production.go b/cl/beacon/handler/block_production.go index d2de138437e..5545556d58e 100644 --- a/cl/beacon/handler/block_production.go +++ b/cl/beacon/handler/block_production.go @@ -104,11 +104,18 @@ func (a *ApiHandler) defaultGraffiti(ctx context.Context) common.Hash { return graffitiFromString(graffiti) } -// executionClientVersion returns the connected execution client's version, caching it on -// first success so that block production stays off the engine API in steady state (the -// version is static for the lifetime of an execution client connection). +// elClientVersionUnavailable is a sentinel cached when the execution client does not +// implement engine_getClientVersionV1, so the negative outcome is memoized too. +var elClientVersionUnavailable = &engine_types.ClientVersionV1{} + +// executionClientVersion returns the connected execution client's version, caching the +// outcome (success or unavailable) on first query so that block production stays off the +// engine API in steady state (the version is static for the lifetime of a connection). func (a *ApiHandler) executionClientVersion(ctx context.Context) *engine_types.ClientVersionV1 { if cached := a.elClientVersion.Load(); cached != nil { + if cached == elClientVersionUnavailable { + return nil + } return cached } if a.engine == nil { @@ -119,6 +126,7 @@ func (a *ApiHandler) executionClientVersion(ctx context.Context) *engine_types.C caplin := engine_types.NewClientVersionV1(caplinClientCode, caplinClientName, a.version, version.GitCommit) versions, err := a.engine.GetClientVersionV1(ctx, &caplin) if err != nil || len(versions) == 0 { + a.elClientVersion.Store(elClientVersionUnavailable) return nil } el := versions[0] diff --git a/cl/beacon/handler/block_production_graffiti_test.go b/cl/beacon/handler/block_production_graffiti_test.go index fdbb7048200..7f96bc9f758 100644 --- a/cl/beacon/handler/block_production_graffiti_test.go +++ b/cl/beacon/handler/block_production_graffiti_test.go @@ -55,14 +55,18 @@ func TestDefaultGraffiti(t *testing.T) { require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) }) - t.Run("execution client version unavailable falls back to consensus-only", func(t *testing.T) { + t.Run("execution client version unavailable falls back to consensus-only and is cached", func(t *testing.T) { ctrl := gomock.NewController(t) engine := execution_client.NewMockExecutionEngine(ctrl) engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). - Return(nil, errors.New("not supported")) + Return(nil, errors.New("not supported")). + Times(1) a := &ApiHandler{engine: engine, version: "1.2.3"} - require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) + first := graffitiText(a.defaultGraffiti(context.Background())) + second := graffitiText(a.defaultGraffiti(context.Background())) + require.Equal(t, caplinClientCode+clCommit, first) + require.Equal(t, first, second) }) t.Run("no engine falls back to consensus-only", func(t *testing.T) { From 853c3d4602b59d587fa55748bf835baf8d8b701c Mon Sep 17 00:00:00 2001 From: Oleksandr Lystopad Date: Wed, 8 Jul 2026 08:39:49 +0200 Subject: [PATCH 3/5] cl/beacon/handler, engineapi: only negative-cache unsupported client version Address review follow-ups on the default-graffiti client version lookup: - executionClientVersion memoized the negative result on any engine error, so a transient failure (including the 1s context timeout) would disable EL attribution until restart. Only cache the unavailable outcome when the method is genuinely unsupported: a JSON-RPC method-not-found (-32601) error or an empty version list. Other errors are treated as transient and retried. - NewClientVersionV1 now strips an existing "0x" prefix from the commit before re-prefixing, so a hex-prefixed input no longer yields "0x0x...". --- cl/beacon/handler/block_production.go | 16 +++++++- .../handler/block_production_graffiti_test.go | 38 ++++++++++++++++++- .../engine_types/client_version_test.go | 30 +++++++++++++++ execution/engineapi/engine_types/jsonrpc.go | 3 +- 4 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 execution/engineapi/engine_types/client_version_test.go diff --git a/cl/beacon/handler/block_production.go b/cl/beacon/handler/block_production.go index 5545556d58e..eb1c9fc4293 100644 --- a/cl/beacon/handler/block_production.go +++ b/cl/beacon/handler/block_production.go @@ -125,7 +125,15 @@ func (a *ApiHandler) executionClientVersion(ctx context.Context) *engine_types.C defer cancel() caplin := engine_types.NewClientVersionV1(caplinClientCode, caplinClientName, a.version, version.GitCommit) versions, err := a.engine.GetClientVersionV1(ctx, &caplin) - if err != nil || len(versions) == 0 { + if err != nil { + // Only memoize the negative result when the method is genuinely unsupported; + // a transient error must not disable EL attribution until restart. + if methodNotFound(err) { + a.elClientVersion.Store(elClientVersionUnavailable) + } + return nil + } + if len(versions) == 0 { a.elClientVersion.Store(elClientVersionUnavailable) return nil } @@ -134,6 +142,12 @@ func (a *ApiHandler) executionClientVersion(ctx context.Context) *engine_types.C return &el } +// methodNotFound reports whether err is a JSON-RPC "method not found" (-32601) error. +func methodNotFound(err error) bool { + var coder interface{ ErrorCode() int } + return errors.As(err, &coder) && coder.ErrorCode() == -32601 +} + // graffitiCommitPrefix returns the leading 2 bytes (4 hex chars) of a commit hash. func graffitiCommitPrefix(commit string) string { commit = strings.TrimPrefix(commit, "0x") diff --git a/cl/beacon/handler/block_production_graffiti_test.go b/cl/beacon/handler/block_production_graffiti_test.go index 7f96bc9f758..03b964ad6d2 100644 --- a/cl/beacon/handler/block_production_graffiti_test.go +++ b/cl/beacon/handler/block_production_graffiti_test.go @@ -35,6 +35,11 @@ func graffitiText(g common.Hash) string { return string(bytes.TrimRight(g[:], "\x00")) } +type rpcError struct{ code int } + +func (e rpcError) Error() string { return "rpc error" } +func (e rpcError) ErrorCode() int { return e.code } + func TestGraffitiCommitPrefix(t *testing.T) { require.Equal(t, "a53e", graffitiCommitPrefix("0xa53e9545")) require.Equal(t, "a53e", graffitiCommitPrefix("a53e9545")) @@ -55,11 +60,25 @@ func TestDefaultGraffiti(t *testing.T) { require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) }) - t.Run("execution client version unavailable falls back to consensus-only and is cached", func(t *testing.T) { + t.Run("method-not-found falls back to consensus-only and is cached", func(t *testing.T) { + ctrl := gomock.NewController(t) + engine := execution_client.NewMockExecutionEngine(ctrl) + engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). + Return(nil, rpcError{code: -32601}). + Times(1) + + a := &ApiHandler{engine: engine, version: "1.2.3"} + first := graffitiText(a.defaultGraffiti(context.Background())) + second := graffitiText(a.defaultGraffiti(context.Background())) + require.Equal(t, caplinClientCode+clCommit, first) + require.Equal(t, first, second) + }) + + t.Run("empty version list falls back to consensus-only and is cached", func(t *testing.T) { ctrl := gomock.NewController(t) engine := execution_client.NewMockExecutionEngine(ctrl) engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). - Return(nil, errors.New("not supported")). + Return([]engine_types.ClientVersionV1{}, nil). Times(1) a := &ApiHandler{engine: engine, version: "1.2.3"} @@ -69,6 +88,21 @@ func TestDefaultGraffiti(t *testing.T) { require.Equal(t, first, second) }) + t.Run("transient error is not cached and is retried", func(t *testing.T) { + ctrl := gomock.NewController(t) + engine := execution_client.NewMockExecutionEngine(ctrl) + gomock.InOrder( + engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). + Return(nil, errors.New("timeout")), + engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). + Return([]engine_types.ClientVersionV1{{Code: "GE", Commit: "0xc3d4e5f6"}}, nil), + ) + + a := &ApiHandler{engine: engine, version: "1.2.3"} + require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) + require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) + }) + t.Run("no engine falls back to consensus-only", func(t *testing.T) { a := &ApiHandler{version: "1.2.3"} require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) diff --git a/execution/engineapi/engine_types/client_version_test.go b/execution/engineapi/engine_types/client_version_test.go new file mode 100644 index 00000000000..39f586586a3 --- /dev/null +++ b/execution/engineapi/engine_types/client_version_test.go @@ -0,0 +1,30 @@ +// 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 engine_types + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNewClientVersionV1Commit(t *testing.T) { + require.Equal(t, "0xa53e9545", NewClientVersionV1("EG", "erigon", "1.2.3", "a53e9545abcd").Commit) + require.Equal(t, "0xa53e9545", NewClientVersionV1("EG", "erigon", "1.2.3", "0xa53e9545abcd").Commit) + require.Equal(t, "0x00000000", NewClientVersionV1("EG", "erigon", "1.2.3", "").Commit) + require.Equal(t, "0x00000000", NewClientVersionV1("EG", "erigon", "1.2.3", "0xabc").Commit) +} diff --git a/execution/engineapi/engine_types/jsonrpc.go b/execution/engineapi/engine_types/jsonrpc.go index 25470162c96..7b99680aa84 100644 --- a/execution/engineapi/engine_types/jsonrpc.go +++ b/execution/engineapi/engine_types/jsonrpc.go @@ -21,6 +21,7 @@ import ( "encoding/json" "errors" "fmt" + "strings" "github.com/erigontech/erigon/cl/clparams" "github.com/erigontech/erigon/common" @@ -185,7 +186,7 @@ func (c ClientVersionV1) String() string { // it to the leading 4 bytes as required by // https://github.com/ethereum/execution-apis/blob/main/src/engine/identification.md func NewClientVersionV1(code, name, versionStr, gitCommit string) ClientVersionV1 { - commit := gitCommit + commit := strings.TrimPrefix(gitCommit, "0x") if len(commit) >= 8 { commit = commit[:8] } else { From 775f0e735ee67e6d1c887f9391b1f975b9d5b597 Mon Sep 17 00:00:00 2001 From: Oleksandr Lystopad Date: Wed, 8 Jul 2026 09:40:34 +0200 Subject: [PATCH 4/5] cl/beacon/handler: single-flight the execution client version fetch On a cold cache, multiple concurrent block-production requests could each call engine_getClientVersionV1 before the first result was stored, a small burst of redundant engine calls at startup/reconnect. Collapse concurrent first-time fetches through a singleflight.Group with a double-check inside the flight, so only one engine call is in flight while the atomic-pointer cache and the transient-vs-unsupported handling are unchanged. --- cl/beacon/handler/block_production.go | 32 ++++++++++++++---- .../handler/block_production_graffiti_test.go | 33 +++++++++++++++++++ cl/beacon/handler/handler.go | 2 ++ 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/cl/beacon/handler/block_production.go b/cl/beacon/handler/block_production.go index eb1c9fc4293..b7d7ba375ae 100644 --- a/cl/beacon/handler/block_production.go +++ b/cl/beacon/handler/block_production.go @@ -111,37 +111,55 @@ var elClientVersionUnavailable = &engine_types.ClientVersionV1{} // executionClientVersion returns the connected execution client's version, caching the // outcome (success or unavailable) on first query so that block production stays off the // engine API in steady state (the version is static for the lifetime of a connection). +// Concurrent first-time fetches are collapsed so only one engine call is in flight. func (a *ApiHandler) executionClientVersion(ctx context.Context) *engine_types.ClientVersionV1 { if cached := a.elClientVersion.Load(); cached != nil { - if cached == elClientVersionUnavailable { - return nil - } - return cached + return normalizeELClientVersion(cached) } if a.engine == nil { return nil } + v, _, _ := a.elClientVersionGroup.Do("", func() (any, error) { + return a.fetchExecutionClientVersion(ctx), nil + }) + cached, _ := v.(*engine_types.ClientVersionV1) + return normalizeELClientVersion(cached) +} + +// fetchExecutionClientVersion queries the engine once and caches the outcome. It returns +// nil (without caching) on transient errors so a later call can retry; only a genuinely +// unsupported method (JSON-RPC -32601) or an empty result is memoized as unavailable. +func (a *ApiHandler) fetchExecutionClientVersion(ctx context.Context) *engine_types.ClientVersionV1 { + if cached := a.elClientVersion.Load(); cached != nil { + return cached + } ctx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() caplin := engine_types.NewClientVersionV1(caplinClientCode, caplinClientName, a.version, version.GitCommit) versions, err := a.engine.GetClientVersionV1(ctx, &caplin) if err != nil { - // Only memoize the negative result when the method is genuinely unsupported; - // a transient error must not disable EL attribution until restart. if methodNotFound(err) { a.elClientVersion.Store(elClientVersionUnavailable) + return elClientVersionUnavailable } return nil } if len(versions) == 0 { a.elClientVersion.Store(elClientVersionUnavailable) - return nil + return elClientVersionUnavailable } el := versions[0] a.elClientVersion.Store(&el) return &el } +func normalizeELClientVersion(v *engine_types.ClientVersionV1) *engine_types.ClientVersionV1 { + if v == nil || v == elClientVersionUnavailable { + return nil + } + return v +} + // methodNotFound reports whether err is a JSON-RPC "method not found" (-32601) error. func methodNotFound(err error) bool { var coder interface{ ErrorCode() int } diff --git a/cl/beacon/handler/block_production_graffiti_test.go b/cl/beacon/handler/block_production_graffiti_test.go index 03b964ad6d2..84fe39b12ec 100644 --- a/cl/beacon/handler/block_production_graffiti_test.go +++ b/cl/beacon/handler/block_production_graffiti_test.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "errors" + "sync" "testing" "github.com/stretchr/testify/require" @@ -108,6 +109,38 @@ func TestDefaultGraffiti(t *testing.T) { require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) }) + t.Run("concurrent first fetches are collapsed into one engine call", func(t *testing.T) { + ctrl := gomock.NewController(t) + engine := execution_client.NewMockExecutionEngine(ctrl) + entered := make(chan struct{}) + release := make(chan struct{}) + engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). + DoAndReturn(func(context.Context, *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error) { + close(entered) + <-release + return []engine_types.ClientVersionV1{{Code: "GE", Commit: "0xc3d4e5f6"}}, nil + }). + Times(1) + + a := &ApiHandler{engine: engine, version: "1.2.3"} + const n = 16 + var wg sync.WaitGroup + results := make([]string, n) + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + results[i] = graffitiText(a.defaultGraffiti(context.Background())) + }(i) + } + <-entered + close(release) + wg.Wait() + for _, r := range results { + require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, r) + } + }) + t.Run("execution client version is cached across calls", func(t *testing.T) { ctrl := gomock.NewController(t) engine := execution_client.NewMockExecutionEngine(ctrl) diff --git a/cl/beacon/handler/handler.go b/cl/beacon/handler/handler.go index 16d921aa896..5e4e9e27b1e 100644 --- a/cl/beacon/handler/handler.go +++ b/cl/beacon/handler/handler.go @@ -23,6 +23,7 @@ import ( "sync/atomic" "github.com/go-chi/chi/v5" + "golang.org/x/sync/singleflight" "github.com/erigontech/erigon/cl/aggregation" "github.com/erigontech/erigon/cl/beacon/beacon_router_configuration" @@ -111,6 +112,7 @@ type ApiHandler struct { blobBundles *lru.Cache[common.Bytes48, BlobBundle] // Keep recent bundled blobs from the execution layer. engine execution_client.ExecutionEngine elClientVersion atomic.Pointer[engine_types.ClientVersionV1] // Cached execution client version for default graffiti. + elClientVersionGroup singleflight.Group // Collapses concurrent first-time elClientVersion fetches. syncMessagePool sync_contribution_pool.SyncContributionPool committeeSub *committee_subscription.CommitteeSubscribeMgmt attestationProducer attestation_producer.AttestationDataProducer From 9121d16ef31fee8868b6265728e125419e3b1b71 Mon Sep 17 00:00:00 2001 From: Oleksandr Lystopad Date: Wed, 8 Jul 2026 10:18:14 +0200 Subject: [PATCH 5/5] cl/beacon/handler, engineapi: keep default-graffiti EL lookup off the proposal path Address review follow-ups on the default-graffiti client version lookup: - Move the engine_getClientVersionV1 fetch off the block-production critical path. A cold cache no longer blocks the proposal for up to the 1s engine timeout (which risked missed slots when the EL is slow/unreachable); instead the first proposal falls back to consensus-only graffiti and a single background fetch populates the cache for later proposals. A one-shot atomic guard replaces the singleflight group and still ensures only one in-flight fetch, while transient errors stay uncached and are retried. - Clamp the execution client code to the 2 bytes the graffiti standard reserves so a non-conforming EL code cannot misalign the encoding. - Add the missing space in the GetClientVersionV1 request log message. - Correct the NewClientVersionV1 doc comment to note the all-zero fallback for a missing or too-short commit hash. --- cl/beacon/handler/block_production.go | 70 ++++++---- .../handler/block_production_graffiti_test.go | 120 +++++++++++------- cl/beacon/handler/handler.go | 3 +- execution/engineapi/engine_api_methods.go | 2 +- execution/engineapi/engine_types/jsonrpc.go | 6 +- 5 files changed, 123 insertions(+), 78 deletions(-) diff --git a/cl/beacon/handler/block_production.go b/cl/beacon/handler/block_production.go index b7d7ba375ae..be1b06ea758 100644 --- a/cl/beacon/handler/block_production.go +++ b/cl/beacon/handler/block_production.go @@ -96,10 +96,10 @@ const payloadPublicationDivisor = 4 // client-version graffiti standard, encoding the execution and consensus client codes and // their commit prefixes so client-diversity tooling can attribute proposed blocks. See // https://github.com/ethereum/execution-apis/blob/main/src/engine/identification.md -func (a *ApiHandler) defaultGraffiti(ctx context.Context) common.Hash { +func (a *ApiHandler) defaultGraffiti() common.Hash { graffiti := caplinClientCode + graffitiCommitPrefix(version.GitCommit) - if el := a.executionClientVersion(ctx); el != nil { - graffiti = el.Code + graffitiCommitPrefix(el.Commit) + graffiti + if el := a.executionClientVersion(); el != nil { + graffiti = graffitiClientCode(el.Code) + graffitiCommitPrefix(el.Commit) + graffiti } return graffitiFromString(graffiti) } @@ -108,49 +108,54 @@ func (a *ApiHandler) defaultGraffiti(ctx context.Context) common.Hash { // implement engine_getClientVersionV1, so the negative outcome is memoized too. var elClientVersionUnavailable = &engine_types.ClientVersionV1{} -// executionClientVersion returns the connected execution client's version, caching the -// outcome (success or unavailable) on first query so that block production stays off the -// engine API in steady state (the version is static for the lifetime of a connection). -// Concurrent first-time fetches are collapsed so only one engine call is in flight. -func (a *ApiHandler) executionClientVersion(ctx context.Context) *engine_types.ClientVersionV1 { +// executionClientVersion returns the connected execution client's version if it is already +// cached, otherwise it kicks off a one-shot background fetch and returns nil. Block +// production never blocks on the engine API: the first proposal after startup falls back to +// consensus-only graffiti and later proposals pick up the execution client code once the +// fetch has populated the cache (the version is static for the lifetime of a connection). +func (a *ApiHandler) executionClientVersion() *engine_types.ClientVersionV1 { if cached := a.elClientVersion.Load(); cached != nil { return normalizeELClientVersion(cached) } - if a.engine == nil { - return nil - } - v, _, _ := a.elClientVersionGroup.Do("", func() (any, error) { - return a.fetchExecutionClientVersion(ctx), nil - }) - cached, _ := v.(*engine_types.ClientVersionV1) - return normalizeELClientVersion(cached) + a.triggerELClientVersionFetch() + return nil } -// fetchExecutionClientVersion queries the engine once and caches the outcome. It returns -// nil (without caching) on transient errors so a later call can retry; only a genuinely -// unsupported method (JSON-RPC -32601) or an empty result is memoized as unavailable. -func (a *ApiHandler) fetchExecutionClientVersion(ctx context.Context) *engine_types.ClientVersionV1 { - if cached := a.elClientVersion.Load(); cached != nil { - return cached +// triggerELClientVersionFetch starts a single background fetch of the execution client +// version, unless one is already in flight or the version is already cached. +func (a *ApiHandler) triggerELClientVersionFetch() { + if a.engine == nil || a.elClientVersion.Load() != nil { + return + } + if !a.elClientVersionFetching.CompareAndSwap(false, true) { + return } - ctx, cancel := context.WithTimeout(ctx, time.Second) + go func() { + defer a.elClientVersionFetching.Store(false) + a.fetchExecutionClientVersion() + }() +} + +// fetchExecutionClientVersion queries the engine once and caches the outcome. Transient +// errors are left uncached so a later fetch can retry; only a genuinely unsupported method +// (JSON-RPC -32601) or an empty result is memoized as unavailable. +func (a *ApiHandler) fetchExecutionClientVersion() { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) defer cancel() caplin := engine_types.NewClientVersionV1(caplinClientCode, caplinClientName, a.version, version.GitCommit) versions, err := a.engine.GetClientVersionV1(ctx, &caplin) if err != nil { if methodNotFound(err) { a.elClientVersion.Store(elClientVersionUnavailable) - return elClientVersionUnavailable } - return nil + return } if len(versions) == 0 { a.elClientVersion.Store(elClientVersionUnavailable) - return elClientVersionUnavailable + return } el := versions[0] a.elClientVersion.Store(&el) - return &el } func normalizeELClientVersion(v *engine_types.ClientVersionV1) *engine_types.ClientVersionV1 { @@ -160,6 +165,15 @@ func normalizeELClientVersion(v *engine_types.ClientVersionV1) *engine_types.Cli return v } +// graffitiClientCode clamps a client code to the 2 bytes the graffiti standard reserves for +// it, guarding against a non-conforming execution client returning an over-long code. +func graffitiClientCode(code string) string { + if len(code) > 2 { + return code[:2] + } + return code +} + // methodNotFound reports whether err is a JSON-RPC "method not found" (-32601) error. func methodNotFound(err error) bool { var coder interface{ ErrorCode() int } @@ -406,7 +420,7 @@ func (a *ApiHandler) GetEthV3ValidatorBlock( if r.URL.Query().Has("graffiti") { graffiti = common.HexToHash(r.URL.Query().Get("graffiti")) } else { - graffiti = a.defaultGraffiti(ctx) + graffiti = a.defaultGraffiti() } tx, err := a.indiciesDB.BeginRo(ctx) diff --git a/cl/beacon/handler/block_production_graffiti_test.go b/cl/beacon/handler/block_production_graffiti_test.go index 84fe39b12ec..ebda83609dc 100644 --- a/cl/beacon/handler/block_production_graffiti_test.go +++ b/cl/beacon/handler/block_production_graffiti_test.go @@ -22,6 +22,7 @@ import ( "errors" "sync" "testing" + "time" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" @@ -48,20 +49,28 @@ func TestGraffitiCommitPrefix(t *testing.T) { require.Equal(t, "0000", graffitiCommitPrefix("")) } -func TestDefaultGraffiti(t *testing.T) { - clCommit := graffitiCommitPrefix(version.GitCommit) +func TestGraffitiClientCode(t *testing.T) { + require.Equal(t, "GE", graffitiClientCode("GE")) + require.Equal(t, "GE", graffitiClientCode("GETH")) + require.Equal(t, "N", graffitiClientCode("N")) +} - t.Run("execution client version available", func(t *testing.T) { +func TestFetchExecutionClientVersion(t *testing.T) { + t.Run("available is cached", func(t *testing.T) { ctrl := gomock.NewController(t) engine := execution_client.NewMockExecutionEngine(ctrl) engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). - Return([]engine_types.ClientVersionV1{{Code: "GE", Commit: "0xc3d4e5f6"}}, nil) + Return([]engine_types.ClientVersionV1{{Code: "GE", Commit: "0xc3d4e5f6"}}, nil). + Times(1) a := &ApiHandler{engine: engine, version: "1.2.3"} - require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) + a.fetchExecutionClientVersion() + got := a.elClientVersion.Load() + require.NotNil(t, got) + require.Equal(t, "GE", got.Code) }) - t.Run("method-not-found falls back to consensus-only and is cached", func(t *testing.T) { + t.Run("method-not-found is cached as unavailable", func(t *testing.T) { ctrl := gomock.NewController(t) engine := execution_client.NewMockExecutionEngine(ctrl) engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). @@ -69,13 +78,11 @@ func TestDefaultGraffiti(t *testing.T) { Times(1) a := &ApiHandler{engine: engine, version: "1.2.3"} - first := graffitiText(a.defaultGraffiti(context.Background())) - second := graffitiText(a.defaultGraffiti(context.Background())) - require.Equal(t, caplinClientCode+clCommit, first) - require.Equal(t, first, second) + a.fetchExecutionClientVersion() + require.Same(t, elClientVersionUnavailable, a.elClientVersion.Load()) }) - t.Run("empty version list falls back to consensus-only and is cached", func(t *testing.T) { + t.Run("empty version list is cached as unavailable", func(t *testing.T) { ctrl := gomock.NewController(t) engine := execution_client.NewMockExecutionEngine(ctrl) engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). @@ -83,13 +90,11 @@ func TestDefaultGraffiti(t *testing.T) { Times(1) a := &ApiHandler{engine: engine, version: "1.2.3"} - first := graffitiText(a.defaultGraffiti(context.Background())) - second := graffitiText(a.defaultGraffiti(context.Background())) - require.Equal(t, caplinClientCode+clCommit, first) - require.Equal(t, first, second) + a.fetchExecutionClientVersion() + require.Same(t, elClientVersionUnavailable, a.elClientVersion.Load()) }) - t.Run("transient error is not cached and is retried", func(t *testing.T) { + t.Run("transient error is not cached and a later fetch can succeed", func(t *testing.T) { ctrl := gomock.NewController(t) engine := execution_client.NewMockExecutionEngine(ctrl) gomock.InOrder( @@ -100,58 +105,85 @@ func TestDefaultGraffiti(t *testing.T) { ) a := &ApiHandler{engine: engine, version: "1.2.3"} - require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) - require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) + a.fetchExecutionClientVersion() + require.Nil(t, a.elClientVersion.Load()) + a.fetchExecutionClientVersion() + got := a.elClientVersion.Load() + require.NotNil(t, got) + require.Equal(t, "GE", got.Code) + }) +} + +func TestDefaultGraffiti(t *testing.T) { + clCommit := graffitiCommitPrefix(version.GitCommit) + + t.Run("cached execution client version yields full graffiti", func(t *testing.T) { + a := &ApiHandler{version: "1.2.3"} + a.elClientVersion.Store(&engine_types.ClientVersionV1{Code: "GE", Commit: "0xc3d4e5f6"}) + require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, graffitiText(a.defaultGraffiti())) + }) + + t.Run("over-long execution client code is clamped to two bytes", func(t *testing.T) { + a := &ApiHandler{version: "1.2.3"} + a.elClientVersion.Store(&engine_types.ClientVersionV1{Code: "GETH", Commit: "0xc3d4e5f6"}) + require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, graffitiText(a.defaultGraffiti())) + }) + + t.Run("cached-unavailable yields consensus-only", func(t *testing.T) { + a := &ApiHandler{version: "1.2.3"} + a.elClientVersion.Store(elClientVersionUnavailable) + require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti())) }) - t.Run("no engine falls back to consensus-only", func(t *testing.T) { + t.Run("no engine yields consensus-only", func(t *testing.T) { a := &ApiHandler{version: "1.2.3"} - require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti(context.Background()))) + require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti())) }) - t.Run("concurrent first fetches are collapsed into one engine call", func(t *testing.T) { + t.Run("cold cache does not block and fills asynchronously", func(t *testing.T) { ctrl := gomock.NewController(t) engine := execution_client.NewMockExecutionEngine(ctrl) - entered := make(chan struct{}) release := make(chan struct{}) engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). DoAndReturn(func(context.Context, *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error) { - close(entered) <-release return []engine_types.ClientVersionV1{{Code: "GE", Commit: "0xc3d4e5f6"}}, nil }). Times(1) a := &ApiHandler{engine: engine, version: "1.2.3"} - const n = 16 - var wg sync.WaitGroup - results := make([]string, n) - for i := 0; i < n; i++ { - wg.Add(1) - go func(i int) { - defer wg.Done() - results[i] = graffitiText(a.defaultGraffiti(context.Background())) - }(i) - } - <-entered + // Returns immediately with consensus-only graffiti while the engine call is still blocked. + require.Equal(t, caplinClientCode+clCommit, graffitiText(a.defaultGraffiti())) close(release) - wg.Wait() - for _, r := range results { - require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, r) - } + require.Eventually(t, func() bool { + return graffitiText(a.defaultGraffiti()) == "GEc3d4"+caplinClientCode+clCommit + }, time.Second, time.Millisecond) }) - t.Run("execution client version is cached across calls", func(t *testing.T) { + t.Run("concurrent cold-cache proposals trigger at most one fetch", func(t *testing.T) { ctrl := gomock.NewController(t) engine := execution_client.NewMockExecutionEngine(ctrl) + release := make(chan struct{}) engine.EXPECT().GetClientVersionV1(gomock.Any(), gomock.Any()). - Return([]engine_types.ClientVersionV1{{Code: "GE", Commit: "0xc3d4e5f6"}}, nil). + DoAndReturn(func(context.Context, *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error) { + <-release + return []engine_types.ClientVersionV1{{Code: "GE", Commit: "0xc3d4e5f6"}}, nil + }). Times(1) a := &ApiHandler{engine: engine, version: "1.2.3"} - first := graffitiText(a.defaultGraffiti(context.Background())) - second := graffitiText(a.defaultGraffiti(context.Background())) - require.Equal(t, first, second) - require.Equal(t, "GEc3d4"+caplinClientCode+clCommit, second) + var wg sync.WaitGroup + for i := 0; i < 16; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _ = a.defaultGraffiti() + }() + } + wg.Wait() // returns while the engine call is still blocked, proving proposals never block on it + close(release) + require.Eventually(t, func() bool { + return graffitiText(a.defaultGraffiti()) == "GEc3d4"+caplinClientCode+clCommit + }, time.Second, time.Millisecond) }) } diff --git a/cl/beacon/handler/handler.go b/cl/beacon/handler/handler.go index 5e4e9e27b1e..89e8febb729 100644 --- a/cl/beacon/handler/handler.go +++ b/cl/beacon/handler/handler.go @@ -23,7 +23,6 @@ import ( "sync/atomic" "github.com/go-chi/chi/v5" - "golang.org/x/sync/singleflight" "github.com/erigontech/erigon/cl/aggregation" "github.com/erigontech/erigon/cl/beacon/beacon_router_configuration" @@ -112,7 +111,7 @@ type ApiHandler struct { blobBundles *lru.Cache[common.Bytes48, BlobBundle] // Keep recent bundled blobs from the execution layer. engine execution_client.ExecutionEngine elClientVersion atomic.Pointer[engine_types.ClientVersionV1] // Cached execution client version for default graffiti. - elClientVersionGroup singleflight.Group // Collapses concurrent first-time elClientVersion fetches. + elClientVersionFetching atomic.Bool // Guards a single in-flight background elClientVersion fetch. syncMessagePool sync_contribution_pool.SyncContributionPool committeeSub *committee_subscription.CommitteeSubscribeMgmt attestationProducer attestation_producer.AttestationDataProducer diff --git a/execution/engineapi/engine_api_methods.go b/execution/engineapi/engine_api_methods.go index 9a06ec82099..37262fc4411 100644 --- a/execution/engineapi/engine_api_methods.go +++ b/execution/engineapi/engine_api_methods.go @@ -262,7 +262,7 @@ func (e *EngineServer) GetPayloadBodiesByRangeV2(ctx context.Context, start, cou // See https://github.com/ethereum/execution-apis/blob/main/src/engine/identification.md#engine_getclientversionv1 func (e *EngineServer) GetClientVersionV1(ctx context.Context, callerVersion *engine_types.ClientVersionV1) ([]engine_types.ClientVersionV1, error) { if callerVersion != nil { - e.logger.Info("[GetClientVersionV1] Received request from" + callerVersion.String()) + e.logger.Info("[GetClientVersionV1] Received request from " + callerVersion.String()) } return []engine_types.ClientVersionV1{engine_types.LocalClientVersionV1()}, nil } diff --git a/execution/engineapi/engine_types/jsonrpc.go b/execution/engineapi/engine_types/jsonrpc.go index 7b99680aa84..170d98e6193 100644 --- a/execution/engineapi/engine_types/jsonrpc.go +++ b/execution/engineapi/engine_types/jsonrpc.go @@ -182,9 +182,9 @@ func (c ClientVersionV1) String() string { return fmt.Sprintf("ClientCode: %s, %s-%s-%s", c.Code, c.Name, c.Version, c.Commit) } -// NewClientVersionV1 builds a ClientVersionV1 from a git commit hash, truncating -// it to the leading 4 bytes as required by -// https://github.com/ethereum/execution-apis/blob/main/src/engine/identification.md +// NewClientVersionV1 builds a ClientVersionV1 from a git commit hash, using its leading +// 4 bytes as required by the standard, or all-zero bytes when the hash is missing or too +// short. See https://github.com/ethereum/execution-apis/blob/main/src/engine/identification.md func NewClientVersionV1(code, name, versionStr, gitCommit string) ClientVersionV1 { commit := strings.TrimPrefix(gitCommit, "0x") if len(commit) >= 8 {