Repository navigation
Merge upstream migalabs/goteth 3.9.0 into Obol fork - #11
Merged
Merged
Conversation
Adds the f_activation_eligibility_epoch column to t_validator_last_status so the pre-activation queue (f_status = 0) can be split into the deposit-lifecycle states aligned with the Beacon API RFC: pending_balance (eligibility = FAR_FUTURE_EPOCH) and pending_lookahead (eligibility set). The coarse f_status (0-3) encoding is left unchanged. Migration 037 uses ADD COLUMN IF NOT EXISTS so it is idempotent on databases where the earlier (reverted) migalabs#268 migration already added the column. docs/tables.md documents the derived vocabulary, including deposit_inqueue, which is materialised downstream from t_deposit_requests because pre-registration deposits have no validator index yet.
Align the derived deposit-lifecycle vocabulary with the agreed naming (pending_minBalance) used in the beacon-APIs proposal.
…statuses feat: persist f_activation_eligibility_epoch on t_validator_last_status
The comment 'reverted in code; this' contained a semicolon. With x-multi-statement=true the ClickHouse golang-migrate driver splits the file on ';', so the leading comment became a standalone statement and the migration failed on a fresh database with 'Empty query' (code 62), leaving the schema dirty at version 37. Replace the ';' with a comma so the file is a single statement again.
Track blocks delivered by the ETHGas relay. Pubkeys taken from the official docs (docs.ethgas.com, Technical Integration); both data APIs verified to serve proposer_payload_delivered bid traces at head.
feat(relay): add ETHGas relay to mainnet and hoodi relay lists
- Group validators without a pool tag under a synthetic 'unknown' pool instead of dropping them, so the sum across pools always matches the network totals in t_epoch_metrics_summary (refs migalabs#277) - Guard the missed-blocks condition against ClickHouse LEFT JOIN default values, which counted validator index 0 as a missed proposal on every epoch it did not propose (refs migalabs#254) - Add RefreshPoolSummaryRange (delete-first recompute in epoch batches) and a cheap order-independent fingerprint of t_eth2_pubkeys to detect pool re-tagging
- New --pool-reroll-epochs flag (POOL_REROLL_EPOCHS, default 0, disabled): on finalized checkpoints, if the t_eth2_pubkeys fingerprint changed, recompute t_pool_summary for the trailing N epochs with delete-first, clamped to the rewards retention window so summaries beyond it are never wiped - New pool-summary-refill command for manual repairs after re-taggings or rewards backfills Refs migalabs#254, migalabs#277
…tency fix: consistent pool summaries (untagged bucket, duty join guard, refresh on re-tagging)
The scripts that refresh t_eth2_pubkeys typically do TRUNCATE + INSERT, leaving the table empty or partial for seconds to minutes. A finalized checkpoint landing inside that window would observe a collapsed fingerprint and reroll the trailing epochs with most validators grouped under 'unknown', until the next checkpoint repaired it. Guard: if the mapping is empty, or its row count dropped by more than half since the last stable observation, skip the reroll (and keep the previous fingerprint) so it fires exactly once when the sync finishes. Refs migalabs#277
fix(val-window): guard pool reroll against mid-sync pubkeys truncation
…(PeerDAS) Backport of the goteth-v4 fix to v3. Closes migalabs#280. Blob arrival timings stopped being recorded at the Fulu fork and nobody noticed: t_blob_sidecars_events took its last row at slot 13162611 (2025-12-03) on every v3 deployment. Under PeerDAS blobs are no longer gossiped whole: they are erasure-coded into columns and each node custodies a subset, so beacon nodes emit data_column_sidecar and never blob_sidecar again. The old topic is still accepted (HTTP 200) but silently never fires, so goteth kept logging 'subscribed to blob_sidecar events' with zero errors while recording nothing. Verified on a synced Lighthouse v8.2.1: 128 data_column_sidecar events vs 0 blob_sidecar events over the same 15s window. Replaces the dead subscription with data_column_sidecar into a new t_data_column_sidecars_events table (one row per column arrival, up to 128 per slot), same shape and column names as v4 so bindeth needs no changes beyond its already-merged #352. The channel is buffered at 1024 because the handler does a blocking send and PeerDAS bursts one event per column instead of <=6 per block. Migration 000038 (renumbered from v4's 000039: v3 migrations end at 000037) renames t_blob_sidecars_events to t_blob_sidecars_events_pre_fulu rather than dropping it, preserving the pre-Fulu history. Deploy notes: - bindeth must already include PR #352 when this migration runs, otherwise its blob endpoints break on the renamed table. - Subscribing to data_column_sidecar on a pre-Fulu beacon node may fail at startup; all current deployments (mainnet, hoodi, sepolia) are post-Fulu.
ClickHouse DDL is not transactional: if the RENAME right after the CREATE fails, golang-migrate marks the migration dirty, and on retry after a manual force the unguarded CREATE dies with 'table already exists'. IF NOT EXISTS makes the retry reach the RENAME instead, consistent with the ADD COLUMN IF NOT EXISTS guard migration 000037 uses for the same reason. Requested in the migalabs#281 review.
…subscribe failure Requested in the migalabs#281 review: pointing goteth at a beacon node where the subscription cannot be established should not take the whole process down, it should just run without blob arrival timings. Note on scope: go-eth2-client v0.27.2 returns errors from Events() synchronously only for client-side conditions (inactive client, topic not in its supported list, missing handler); a node that rejects the topic surfaces asynchronously inside the library's own goroutine, which logs and retries every second and never hit the old Panicf. The change makes the client-side conditions degrade gracefully too. The head, finalized and reorg subscriptions keep their panic: without those topics the analyzer cannot make progress at all.
Each data column arrival was persisted individually and synchronously inside runHead's select loop. Under PeerDAS a full-custody node emits up to 128 of these events per slot, so a single slot triggered ~128 serialized, mutex-guarded ClickHouse INSERTs on the same loop that reads head events and dispatches block downloads; a slow write could stall block following. The pre-Fulu blob_sidecar path produced at most 6-9. runHead no longer touches DataColumnSidecarChan. A dedicated persister goroutine drains the channel and feeds PersistDataColumnSidecarsEvents (which always accepted a slice and batches rows into one INSERT) in batches: immediately at 128 events (one full-custody per-slot burst) and on a 1s ticker otherwise. On shutdown it drains the channel and does a best-effort final flush before DBService.Finish() runs; it registers on wgMainRoutine so the shutdown sequence waits for it. Previously up to 1024 queued events were silently dropped on shutdown. The batching core is a standalone function with an injected tick channel, covered by deterministic unit tests (no beacon node or database needed). Closes migalabs#282
The flag was a plain bool written from Close(), Run() and two download error paths, and read from four routines plus the new data column persister, all without synchronization. The race was benign in practice (stale bool reads) but is exactly what the race detector flags, and the persister added one more reader. atomic.Bool keeps the semantics and removes the race; ChainAnalyzer is only ever used through a pointer, so the non-copyable field is safe.
The subscribe path now logs and skips on failure instead of crashing, which reintroduces a silent-loss risk: the analyzer would run without recording blob arrival timings and only a startup log line would tell. goteth_analyzer_data_column_subscription_active makes that state visible to operators (1 while the subscription is established, 0 otherwise, so it can be alerted on). The flag lives behind a pointer so the Events struct stays copyable.
…-events-280 fix(blobs): record p2p arrival timings again via data_column_sidecar (PeerDAS)
…events-282 perf(analyzer): batch data_column_sidecar persistence off the head loop
…-events-280 Catch-up: land the migalabs#283 batching commits on dev
…placed
f_deposits_num is written as exactly 0 for some epochs while t_deposits holds
the corresponding rows. Never slightly wrong, always exactly zero.
The counters are accumulated on one state and read from another, an epoch
later:
processPendingDeposits() state := p.baseMetrics.NextState
state.DepositsNum += 1
ExportToEpoch() DepositsNum: int(s.CurrentState.DepositsNum)
The value written into the epoch row for E-1 was accumulated while that object
was NextState, during the processing of E-1. It survives only if the same object
is still in the cache when E is processed. Three things replace or reset it in
between:
- AdvanceFinalized deletes and re-downloads a state whose root changed
- ensureDependencyStates re-downloads states evicted by CleanUpTo
- RefreshBlocks resets the counters outright, and post-Electra AddBlocks does
not refill them, since deposits arrive through the pending queue rather than
block bodies
A replaced object carries zeros and nothing recomputes them, so the epoch row
reports no deposits while the detail rows exist. Measured on mainnet: 252 epochs
over 450421-470670, 1,653 deposits and roughly 113,000 ETH absent from the
summaries, against 10,029 epochs written correctly. 97.5% right, and wrong in
exactly the way a lost accumulator is wrong.
The same reading explains why the consolidation counters fail on a different set
of epochs, overlapping the deposit failures in only 18 of 37 cases: RefreshBlocks
resets the deposit counters and not the consolidation ones, so the two have
different wipe conditions.
Fix: record on the state whether its deposit counters were ever accumulated, and
recompute them from the state itself when they were not. What
processPendingDepositsFor derives depends only on the state handed to it, so a
state that came back from the beacon node reaches the same numbers the original
run reached. It must run at most once per object, since the counters accumulate
with += rather than being assigned, and the new flag is what enforces that.
The recomputation runs before the existing map clears, so its contribution to
DepositedAmounts is discarded exactly as the surrounding code expects and the
reward path repopulates that map afterwards; only the two counters survive.
Deposit rows are persisted from NextState.DepositsProcessed, so recomputing for
CurrentState cannot duplicate them. The common case costs a boolean check.
Seven tests. One found a hole in the first version of this fix: RefreshBlocks
zeroed the counters without clearing the flag, so a refreshed state still
claimed it had been accumulated and the recomputation never fired. Others cover
the claim the fix rests on, that a re-downloaded state recomputes to the numbers
the original reached, keep "no deposits this epoch" distinguishable from "never
counted", and pin the doubling that makes the flag necessary.
Pre-existing failures in pkg/analyzer are unchanged, 13 before and 13 after;
they need a live beacon endpoint.
Closes migalabs#287
A blob's versioned hash is sha256 of its KZG commitment and nothing else, so two transactions carrying byte-identical blobs produce the same hash. GetTxHash matched sidecars on that hash and did not stop at the first match, so every copy was credited to whichever transaction was compared last, leaving the others with no blobs at all. Mainnet slot 14414626 has three consecutive transactions each carrying the same near-empty blob, 130,816 of its 131,072 bytes being trailing zeros. All three blobs landed on the last of them. Over 136,175 mainnet slots this affects 252, and every one of them contains a repeated blob hash: no slot mismatches without one, and no slot with one is correct. A block's blob commitments are the concatenation of each type-3 transaction's blob hashes in transaction order, so the sidecar holding index i belongs to the transaction owning the i-th entry. That mapping is exact and unaffected by repeated content. Nothing is assigned until the whole block checks out: the sidecars must account for exactly the blobs the transactions committed to, each index must appear once, and every hash must agree with the one committed at that position. A transaction whose receipt never arrived is dropped while parsing, which breaks the first of those, so the mapping is left alone and the slot logged rather than sliding every blob onto its neighbour. Nine cases cover it, including the slot above, one transaction carrying the same blob twice, sidecars and transactions arriving out of order, and each way the block can fail to line up. Verified against the old algorithm on the same fixture: it reproduces the production rows exactly, so the test fails without this change. Pre-existing failures in pkg/analyzer are unchanged, 13 before and 13 after; they need a live beacon endpoint. Closes migalabs#286
DeleteStateMetrics(E) removed six things while
ProcessStateTransitionMetrics(E) writes back three. The other three belong to
later epochs:
written by processing E deleted by DeleteStateMetrics(E)
epoch_metrics(E-1) epoch_metrics(E-1) rewritten
proposer_duties(E) epoch_metrics(E) orphaned
rewards(E) proposer_duties(E) rewritten
rewards(E) rewritten
rewards(E+1) orphaned
rewards(E+2) orphaned
epoch_metrics(E) is written by processing E+1 (ExportToEpoch uses
CurrentState.Epoch), rewards(E+1) by E+1 and rewards(E+2) by E+2. The loop does
reprocess those when the state at E was replaced, but only if it reaches them:
epochs at or past finalizedEpoch are skipped, and the flags that would have
marked them do not survive the call, so the next invocation no longer knows they
are stale. Their rows stay deleted for good.
Six such holes were live on mainnet when this was written, every one matching
the signature exactly: epoch_metrics absent at E with E-1 and E+1 present, and
t_validator_rewards_summary empty at E+1 and E+2 while E holds ~902k rows. The
oldest had survived 774 epochs, so it is not indexing lag. New ones appear about
every day and a half, and the two production deployments do not agree on which
epochs are affected, both consistent with a boundary race rather than
deterministic logic.
Two changes:
Delete only what this epoch's processing writes back. Rows belonging to E+1 and
E+2 are deleted and rewritten when those epochs are processed. The invariant is
one-directional: everything deleted is rewritten by the same call. Processing
also writes slashings, deposits, consolidations, requests, last status, block
rewards and pool metrics, none of which were deleted before this change either.
Carry the epochs whose derived rows went stale but that this invocation could
not reach, so a later one rewrites them. Without this the narrower delete would
leave them holding pre-reorg values, which is worse than a hole: wrong data that
looks present.
What gets carried is keyed on the state at E having been replaced, not on its
blocks having changed. Both a state root mismatch, which deletes and
re-downloads the state, and a block change, which refreshes the state's derived
fields, rewrite state E, and it is state E that epoch_metrics(E), rewards(E+1)
and rewards(E+2) derive from. Keying on blocks alone would leave the root
mismatch case holding stale values. The block case is recorded where it is
detected rather than after the state root check, since that check can fail and
skip the rest of the iteration.
Epochs reprocessed only because a predecessor changed are not carried: their own
state is untouched, so nothing downstream of them went stale, and propagating
from them would cascade forward without end.
The carried set lives in memory and is guarded by advanceFinalizedMu, which
AdvanceFinalized already holds for its whole body. It does not survive a
restart: an epoch carried but not yet reached when the process stops keeps its
pre-reorg values instead of being rewritten. Making that durable needs somewhere
to persist it and is left out of this change.
Seven tests cover the carry-over rule against the real functions: that a
dependent below the boundary is not carried, that a state replaced without any
block change still is, that a carried epoch is consumed exactly once, and that
consuming one does not cascade. They exercise the rule directly; which set
AdvanceFinalized hands it is a single line that no test reaches, since that
function needs a live beacon endpoint.
The three realigned lines in chain_analyzer.go are gofmt: the new field is
longer than the column the block was aligned to. That file was already
unformatted on dev.
Pre-existing test failures in pkg/analyzer are unchanged, 13 before and 13
after; they need a live beacon endpoint.
Closes migalabs#285
Addresses the review on migalabs#289. The previous version guarded one call site: PreProcessBundle checked PendingDepositsProcessed before recomputing from CurrentState. The other path, processPendingDeposits -> processPendingDepositsFor(NextState), still ran unconditionally on every ProcessStateTransitionMetrics call, so an epoch reprocessed against a state still held in cache accumulated its counters a second time and doubled them. A dependency-triggered pass in AdvanceFinalized, a carried reprocess, or a RefreshStateBlocks failure that left the flag unset all reach that path. The guard now lives in processPendingDepositsFor, where every path meets, and the caller-side condition is gone. Guarding at a call site only ever protects that call site. TestRunningTwiceOnOneStateDoublesTheCounters asserted the doubling as intended behaviour. It pinned the bug. Replaced with tests that assert idempotence for the scalar counters, for the per-validator amounts, and for the NextState path specifically. Found while writing those: RefreshBlocks zeroes DepositsNum and TotalDepositsAmount but left DepositedAmounts populated, so a refreshed state recomputed to correct scalars and doubled per-validator amounts. Its own doc says it "resets the accumulators first so that calling it on a state whose blocks were already set does not double-count values", and DepositedAmounts is an accumulator it missed. Reset it there, with a test that fails without the line. Five mutations, all caught.
…ed transactions Addresses the review on migalabs#290. AssignTxHashes took AgnosticTransactions, which ParseTransactionsFromBlock builds by matching transactions against EL receipts and which only contains the ones that matched. A receipt that had not arrived therefore dropped a blob carrier from the list, the block then appeared to commit fewer blobs than there were sidecars, the consistency check refused the whole slot, and every blob in it was persisted with an empty tx_hash. The code this replaced would still have attributed the blobs of the transactions that were present, so this was a regression. Partial receipt fetches are common enough to have their own recovery path in recoverBlockReceipts, so attribution must not depend on them. Everything the mapping needs is in the block body: each type-3 transaction's hash and the blob hashes it committed. AssignTxHashes now takes block.ExecutionPayload.Transactions and decodes them itself, so receipts are not an input at all. That field is populated from block.<Fork>.Message.Body.ExecutionPayload.Transactions for every fork that can carry blobs, Fulu included. The block's transaction order is authoritative, which also removes the sort the previous version needed to put receipt-parsed transactions back in order. The strict checks stay, and now mean what they say: a mismatch is genuinely a block whose sidecars are not the ones it committed to, not a slow execution client. processBlobSidecars loses its transactions parameter, along with the len(txs) > 0 guard that existed only because AgnosticTransactions could legitimately be empty. Also fixes a panic that predates this branch. Index is a uint64 from the beacon API, and the bounds check narrowed it to int before comparing. Any index at or above 2^63 wraps negative, passes the "past the end" check because a negative is below the length, and then panics indexing the seen array, taking block processing down with it. The comparison now happens before the narrowing. An undecodable transaction aborts attribution for the slot rather than being skipped. Skipping one would leave every later position shifted with no way to tell, since the undecodable transaction may itself have been a blob carrier. Tests are rebuilt on encoded transactions rather than synthetic structs, so each hash is derived from its own transaction instead of chosen. They are unsigned, so the hashes are not ones that would appear on chain; attribution does not depend on that, only on the hashes being derived and distinct. Added the case the review asked for, and three more found while auditing: a block that commits more blobs than arrived, which the index-bound check cannot see and which would otherwise publish a slot quietly short a blob; the wrapping index above; and an assertion that the error counts blob-carrying transactions rather than all transactions. Seven mutations, all caught.
From the review on migalabs#290. Every other check in AssignTxHashes reports the values it compared; the hash comparison reported only the index. That is the one most likely to need reading against the chain, because when several blobs in a block share a versioned hash "does not match" alone does not say which sidecar arrived where it should not have. The test asserts the order as well as the presence of both hashes. Naming them the wrong way round is worse than naming neither: it reads plausibly and sends whoever is debugging after the wrong sidecar. Mutation testing found exactly that gap - swapping the two arguments passed a test that only checked both strings appeared.
Addresses the HandleReorg half of the review on migalabs#288. DeleteStateMetrics is shared by AdvanceFinalized and HandleReorg, and only the first was taught the narrower rules. HandleReorg walks slots from the head downwards, so it reaches a higher epoch before the lower ones it derives from. ProcessStateTransitionMetrics(E) reads the states for E, E-1 and E-2 out of the cache, so rewriting E during the walk computed it from whichever of those the walk had not replaced yet: the stale pre-reorg copies. Under the previous wide delete those rows were removed and left as a visible hole. Under the narrow one they stay, holding values derived from stale state, and AdvanceFinalized will not correct them later because by then the state roots already match the finalized node. That trades a detectable gap for a silent wrong value, which is the worse of the two. The walk now only records which epochs had their state replaced, and the rewrites happen after it, so every rewrite reads states that have all been brought up to date. They run in ascending epoch order, which matters separately: ProcessStateTransitionMetrics already waits for epoch-1 before starting, because ProcessAttestations writes ManualReward onto block objects the following epoch reads back (migalabs#242). Running E before E-1 satisfied that barrier trivially, E-1 not having started, and then let E-1 mutate blocks E had already read. The rewrite loop takes its delete and process as parameters so the order can be asserted without a database or a beacon node. It also deletes each epoch immediately before rewriting it rather than clearing everything up front, which would leave every epoch after the first deleted and not yet written for as long as the rewrites take. ProcessStateTransitionMetrics is synchronous - it is only launched with go from the live path in routines.go - so the sequential calls here genuinely serialise and the ordering does what it claims. Five mutations, all caught.
… check failed Both from the review comments on migalabs#288. DeleteStateMetrics(0) computed epoch-1 on an unsigned type, wrapping to the maximum uint64 and addressing a row that cannot exist. Harmless today only because nothing matches it, which is not a property worth relying on. Reachable from a low initSlot or a fresh testnet, where HandleReorg derives epoch from slot/32 and a reorg near genesis gives epoch 0. The guard is scoped to the epoch row rather than the whole function. Processing epoch 0 writes no epoch row - there is nothing before genesis - but it does write the proposer duties and validator rewards for epoch 0, so those deletes still have to run. Returning early would have swapped an underflow for a hole. Second, AdvanceFinalized gave up on an epoch when it could not fetch the finalized state root, and the continue happened before the epoch was recorded or its carried debt consumed. Nothing else covers that case: carryStaleDependents carries an epoch's dependents, never an epoch whose own check failed, and CleanUpTo can evict the state before another invocation looks again. Rewriting anyway was suggested and is not safe: the rows would derive from a state that may still be the pre-reorg copy, which is the silent wrong value this branch exists to remove. The epoch is carried instead, so a later invocation decides with a root it could actually fetch. That debt re-arms for as long as the fetch keeps failing, which is the point. Three mutations, all caught. The test for the wrapped row is written through a variable on purpose: the compiler rejects the constant form as an overflow, which is exactly why this went unnoticed, since the real code wraps a variable silently.
…an-retry fix(reorg): AdvanceFinalized can retry an epoch it failed to rewrite
Four places created a time.Ticker, waited on one tick, and never stopped it. An
unstopped ticker is never collected, so each one went on waking the runtime once
a second for the life of the process.
All four sat in retry or rate-limit paths, which is what makes it more than
untidy: they accumulate exactly when the node is already under stress. A long
sync with intermittent beacon node timeouts leaked one per retry and never gave
any of them back.
pkg/clientapi/block.go one per failed block request retry
pkg/clientapi/state.go one per timed-out state request retry
pkg/clientapi/transaction.go one per failed receipt request retry
pkg/analyzer/routines.go one per rate-limit hit in runHistorical
A one-shot wait is a sleep. migalabs#295 suggests time.After, but block.go already uses
time.Sleep three lines above for its 404 retry, so this follows the file's own
idiom rather than introducing a second one. Measured: a ticker's first tick and
time.Sleep over the same duration are the same wait.
The escalating backoff in block.go is unchanged - the duration still grows with
the attempt count.
migalabs#295 lists these four as correct but untidy: each is created once per long-running routine and lives as long as the process, so none of them leaks in practice. They are stopped anyway, for a reason the issue does not cover. The invariant worth having is "every ticker is stopped", and that cannot be checked while four known-unstopped tickers exist - the check would have to carry an exception list, which is the kind of thing that grows. Behaviour is unchanged: the deferred Stop runs when the routine returns, which is process shutdown.
This is the fifth bug of a shape this repository keeps producing, so the rule is written down where it can fail rather than left as a convention. Walks the tree with go/ast and, for every variable assigned from time.NewTicker, requires a Stop on it in the same function. It reports how many it checked and fails outright if it finds none, so a move or a rename cannot leave it quietly testing nothing. It also flags a NewTicker whose result is not held at all. That was a real blind spot rather than a hypothetical: `<-time.NewTicker(d).C` compiles, leaks exactly like the assigned form, and the first version of this test passed it while reporting "checked 10" - it had not even seen it. Both shapes are now checked, and the count moves to 11 when one appears. Known limit: a ticker assigned to a struct field is not flagged. Its lifecycle is genuinely different - stopped elsewhere, possibly in another method - so flagging it would be a false positive. No such usage exists in the repository today, verified rather than assumed. Mutation-checked in three ways: the original assigned-leak shape, a dropped defer on a long-lived ticker, and the inline form. The guard's count matches what git grep finds, and CI runs it through `go test -race ./...`.
…e text The check asked whether the rendered error contained the substring "404". The beacon node is addressed by slot, so on a transport failure go-eth2-client joins in the error from net/http, which carries the full request URL: failed to call GET endpoint Get "http://bn:5052/eth/v2/beacon/blocks/404": dial tcp ...: refused 79,447 of the first 15,000,000 slots contain "404" in decimal, about one in two hundred. For those slots an unreachable beacon node satisfied the check. The reverse also held: any status code passed as long as something laundered a "404" into the message, so a 500 naming slot 4040 read as not-found. Five call sites were affected, in two different shapes. RequestBeaconBlock and the three blob.go sites went through response404(err.Error()). RequestBeaconBlock took the missing-block branch and returned CreateMissingBlock(slot) with a nil error: a synthetic empty block was written and the epoch completed clean, indistinguishable downstream from a genuinely empty slot. The blob sites did the same more quietly, returning empty sidecars for a slot never fetched. RequestBlockRoot open-coded the same check inline, which is why it did not turn up alongside the others, and it is the damaging one. It returns a zero root on a match and calls log.Panicf otherwise. A zero root never equals the cached root, so reorg.go deletes that slot's block metrics and redownloads it. A transient beacon node failure during reorg handling, on a 404-shaped slot, therefore deleted correct data, and the redownload hit the first bug and wrote a fabricated empty block in its place. The two compounded. isNotFound asks the error for its status code instead. The slot, the port, the peer address and any hex root echoed back in a body can no longer be mistaken for a status. This changes behaviour, deliberately, in two places. In RequestBeaconBlock a transport failure on a 404-shaped slot now falls through to the retry loop and, if the node stays down, to the existing "unable to retrieve Beacon Block at slot %d" error, so an epoch that used to complete against a flapping node will now stop. In RequestBlockRoot the same failure now reaches the log.Panicf that every other slot already reached. Both make a silent wrong answer into a visible one. The panic in RequestBlockRoot is its own problem and is left alone here rather than folded into this fix. go-eth2-client declares Error() on the value receiver, so api.Error and *api.Error both satisfy the error interface. It returns the pointer form today; both are matched, because a switch to the value form would otherwise stop every genuine 404 being recognised and send every missing slot into a retry loop. The tests cover the statuses accepted and rejected, unwrapping through fmt.Errorf and errors.Join, the value form, and the six shapes that fooled the old check. One test makes a request that really fails against a closed loopback port and asserts the hand-built transport error used by the others still matches it, so those cannot drift into asserting against a fiction. Seven mutations were tried, including restoring the old substring implementation, and all seven were caught; an equivalent mutation was confirmed to survive. isNotFound is at 100%. Fixes migalabs#298
fix(clientapi): decide "block not found" from the status code, not the text
fix: stop leaking a runtime timer per retry
…e epoch Each relay is asked once per processed epoch for its last 32 delivered payloads, and nothing asks again later, so one failed request lost the epoch's bids from that relay: the blocks it delivered were stored with no relay and no bid commission. The failures seen are timeouts and DNS lookups that time out, at times for two relays in the same second. A request is now tried up to three times, waiting 5 s and then 10 s, and counts as one failure for the circuit breaker. A relay whose last request failed, the breaker's half-open probe included, gets a single attempt, so a relay that is down does not hold every epoch up. An empty answer, which the client returns as no bids and no error, is an answer and not a failure. The error message is unchanged.
bloxroute.max-profit.blxrbdn.com no longer resolves and its last delivered payload is slot 15077175. Every epoch's request to it failed.
fix(relay): retry a failed delivered bids request, and remove bloXroute max-profit
…e epoch Each relay is asked once per processed epoch for its last 32 delivered payloads, and nothing asks again later, so one failed request lost the epoch's bids from that relay: the blocks it delivered were stored with no relay and no bid commission. The failures seen are timeouts and DNS lookups that time out, at times for two relays in the same second. A request is now tried up to three times, waiting 5 s and then 10 s, and counts as one failure for the circuit breaker. A relay whose last request failed, the breaker's half-open probe included, gets a single attempt, so a relay that is down does not hold every epoch up. An empty answer, which the client returns as no bids and no error, is an answer and not a failure. The error message is unchanged.
bloxroute.max-profit.blxrbdn.com no longer resolves and its last delivered payload is slot 15077175. Every epoch's request to it failed.
hotfix(relay): retry failed delivered bids requests, remove bloXroute max-profit
Release v3.9.0: PeerDAS blob timings, ETHGas relay, reorg and stability fixes
add citation.cff file
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
anadi2311
changed the title
aly-obol
approved these changes
Nov 11, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
aly-obol
approved these changes
Nov 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
migalabs/gotethrelease3.9.0(65 commits sincev3.8.2) into the Obol fork.pkg/analyzer/process_state.go: take upstream'swrote := false(needed for the new return value) and drop our old "Boolean values …" debug log that printed every epoch.pkg/analyzer/metrics_test.go(test only): keep the ObolNewAPIClientsignature (bnApiKey, CF Access ID/secret) with upstream'stestBNEndpoint()/testELEndpoint()helpers.bn-api-key/ CF Access flags, Go 1.26.10 toolchain,x/cryptov0.52.0, native arm64/amd64 CI.Why
3.9.0 fixes data correctness for tables we use:
t_pool_summaryconsistency;AdvanceFinalized;It also fixes two leaks (processerBook slot, per-retry timer) and adds blob timings after Fulu.
Notes / Risk
000037:ADD COLUMN IF NOT EXISTS activation_eligibility_epochont_validator_last_status. Metadata only.000038: createst_data_column_sidecars_eventsand renamest_blob_sidecars_events→t_blob_sidecars_events_pre_fulu.Rollout
build_push.ymldoesn't trigger onmaster, and the upstream tag has nov. Use the short SHA tag, notnext/latest.goteth_versionon thegoteth-hoodimodule block only. Mainnet stays on499029f(v3.8.2).t_data_column_sidecars_eventsfilling;Test plan
go build ./...,go vet ./...,go test ./...pass locallybuild-and-testand CodeQL passghcr.io/obolnetwork/goteth:<short-sha>🤖 Generated with Claude Code