Skip to content

Resolve builder index from chain state instead of only lifecycle registration - #159

Closed
damilolaedwards wants to merge 1 commit into
ethpandaops:mainfrom
damilolaedwards:fix-builder-index-lifecycle-fallback
Closed

Resolve builder index from chain state instead of only lifecycle registration#159
damilolaedwards wants to merge 1 commit into
ethpandaops:mainfrom
damilolaedwards:fix-builder-index-lifecycle-fallback

Conversation

@damilolaedwards

Copy link
Copy Markdown

Problem

RevealService and the epbs Builder API handler only learn the on-chain
builder index through a lifecycle registration callback wired in
cmd/run.go, gated behind lifecycleMgr != nil && epbsSvc != nil. That
callback only fires when the process runs with --el-rpc and
--wallet-privkey.

Run --epbs without those flags and a builder that is already registered
on-chain will sign every bid and reveal envelope with BuilderIndex=0
instead of its real index. This happens silently: the envelope still gets
signed and published, the payload builder still marks it a success, and
nothing logs an error. The signature just won't verify against the right
builder, so the bid or reveal is rejected downstream.

Fix

Resolve the builder index directly from chain state
(chainSvc.GetBuilderByPubkey(pubkey).Index) at startup, in both
RevealService.Start() and epbs.NewHandler(), instead of relying solely on
the lifecycle callback. This mirrors the pattern p2p_bidder already uses
correctly for the same value. The lifecycle callback still updates the index
live if registration happens while the process is running.

Testing

Added a test per package proving the index is picked up from chain state
without the lifecycle callback ever firing:

  • TestRevealService_ResolvesBuilderIndexFromChainStateOnStart
  • TestNewHandler_ResolvesBuilderIndexFromChainState

go test ./pkg/... passes (aside from a pre-existing, unrelated failure in
pkg/webui caused by the frontend not being built in this checkout).

…stration

RevealService and the epbs Builder API handler only learned the on-chain
builder index through a lifecycle registration callback, which never fires
when the process runs without --el-rpc/--wallet-privkey. A builder already
registered on-chain in that setup would sign every bid and reveal envelope
with index 0, producing consensus-invalid messages with no error surfaced
anywhere.

Both now resolve the index from chain state directly (GetBuilderByPubkey) at
startup, matching the pattern the p2p bidder already uses. The lifecycle
callback still updates the index live when registration happens during the
run.
@damilolaedwards
damilolaedwards force-pushed the fix-builder-index-lifecycle-fallback branch from 09f150a to cb0724e Compare August 10, 2026 20:27
pk910 added a commit that referenced this pull request Aug 12, 2026
PR #159 fixes RevealService and the epbs Builder API handler signing with
BuilderIndex=0 when the process runs without --lifecycle: both learned the
index only from the lifecycle registration callback, which never fires
without --el-rpc / --wallet-privkey.

The managed key set removed both single-index caches. Every key's index now
comes from chainSvc.GetBuilders() in Registry.Refresh(), so the fleet resolves
its indexes with no callback involved, and the p2p bid and reveal paths refuse
a key that reads unregistered instead of signing with the zero value.

The Builder API bid path was the one site still discarding that flag. Key
selection only offers active keys, which by definition carry an on-chain
index, but signing with the zero value there would be silent: the bid is
well-formed and served, its signature just belongs to whoever holds builder
index 0. It now takes the same 204 path as "no key ready".

Cover the property the whole fleet's ability to bid rests on: the registry
resolving indexes from beacon state alone.
@pk910

pk910 commented Aug 12, 2026

Copy link
Copy Markdown
Member

Heya @damilolaedwards
Thx for digging into this and proposing a fix. It looks technically fine :)
However, we've changed the whole builder registry management in #153, and the issue was fixed by that rework.
Closing this PR

@pk910 pk910 closed this Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants