Repository navigation
Conversation
go-eth2-client now uses github.com/OffchainLabs/go-bitfield, and requires go 1.25.
Refetched blocks and attestations do not carry canonical information, so keep the existing values rather than overwriting them with NULL. Also run the bulk attestation copy in a savepoint, so that a failure leaves the transaction usable for the fallback of setting attestations individually.
Adds t_block_execution_payload_bids to hold the execution payload bid carried by blocks from Gloas onwards.
Stores the execution payload bid with the block, and the execution payload from the envelope once a child block confirms that the payload was delivered.
The previous version of goid returns 0 for all goroutines on Go 1.25+, causing go-deadlock to report false recursive locking in the scheduler tests.
Builds golangci-lint from source, as the v1 release binaries are built with Go 1.24 and refuse to lint newer Go versions.
| Block: parent.Root.String(), | ||
| }) | ||
| if err != nil { | ||
| return errors.Wrap(err, "failed to obtain parent execution payload envelope") |
There was a problem hiding this comment.
Any failure to fetch the envelope permanently wedges the block service on this slot.
onBlockGloas is called from OnBlock inside the per-slot transaction opened by UpdateSlot. Returning an error here makes UpdateSlot cancel() the transaction and bail before md.LatestSlot is advanced, and catchup then logs and returns. The next head event restarts catchup from the same slot, so chaind stops indexing entirely and retries forever.
Concrete scenarios that produce an error rather than a transient hiccup:
- The consensus node returns 404 for
/eth/.../execution_payload_envelopebecause the envelope has been pruned, or because the node simply never saw it (the comment above says the payload "may never arrive"). parentPayloadIsFullsays the parent was full, but the node disagrees/hasn't yet imported the envelope — a plain 404.- Line 596:
response.Data.Gloas == nilon a non-Gloas-shaped response also hard-errors.
Every other remote fetch in this file tolerates a missing resource: updateBlockForSlot treats 404 as a missed slot, and updateBlobSidecarsForBlock logs a warning and returns nil on 404. This one should do the same — e.g. check errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound, warn, and return nil — otherwise one unavailable envelope halts the whole indexer.
| return errors.Wrap(err, "failed to update sync aggregate") | ||
| } | ||
| if len(dbBlock.BlobKZGCommitments) > 0 { | ||
| if err := s.updateBlobSidecarsForBlock(ctx, dbBlock.Root); err != nil { |
There was a problem hiding this comment.
Blob sidecars are fetched before the payload they belong to has been revealed, and are never retried.
dbBlock.BlobKZGCommitments for a Gloas block comes from the bid (dbBlockGloas), i.e. it is a commitment to blobs that the builder publishes together with the execution payload envelope — which, per the design of the rest of this PR, arrives after the block and is only picked up when a child block confirms it.
So at the moment onBlockGloas runs, BlobSidecars(blockRoot) is very likely to 404 (or return nothing). updateBlobSidecarsForBlock swallows a 404 with a warning, and nothing ever re-attempts the fetch — updateParentExecutionPayload stores the payload but does not re-fetch sidecars, and updateBlockForSlot skips slots whose block is already in the database. Net effect with blocks.blobs.enable on: blobs are systematically missing for Gloas blocks.
The sidecar fetch probably belongs in updateParentExecutionPayload, keyed on the parent root, alongside the payload.
| res[key] = bid | ||
| } | ||
|
|
||
| return res, nil |
There was a problem hiding this comment.
rows.Err() is never checked, so a truncated result set is silently reported as "no bid".
If the query fails part-way through iteration (connection reset, server-side error after the first row, context deadline), rows.Next() returns false and this returns a partial map with nil error. Callers cannot distinguish "this block has no bid" from "we failed to read it":
Blocks()/BlockByRoot()hand back blocks withExecutionPayloadBid == nil.parentPayloadIsFullthen returnsfalse, soupdateParentExecutionPayloadquietly skips the parent — and becauseupdateBlockForSlotwon't re-fetch a slot whose block already exists, that parent's execution payload (and its withdrawals) is lost permanently with nothing logged.
Adding if err := rows.Err(); err != nil { return nil, err } before the return turns this into a retryable slot failure instead of silent data loss. (executionPayloads has the same gap, but this new code gates a write path, which makes it worse here.)
| return err | ||
| if err != nil { | ||
| if rollbackErr := nestedTx.Rollback(ctx); rollbackErr != nil { | ||
| return errors.Wrap(rollbackErr, "failed to roll back nested transaction") |
There was a problem hiding this comment.
Returning the rollback error discards the real failure and sends the caller down the fallback path with an aborted transaction.
The copy error in err is the thing the caller wants: updateAttestationsForBlock/updateElectraAttestationsForBlock log it at debug ("Failed to set attestations en masse, setting individually") before falling back. When Rollback also fails, that diagnostic is replaced by "failed to roll back nested transaction" and the original constraint violation is gone.
Worse, a failed ROLLBACK TO SAVEPOINT means the outer transaction is still in an aborted state — which is precisely the situation this savepoint was added to avoid. The caller nonetheless proceeds to issue SetAttestation for every attestation, each of which fails with 25P02 current transaction is aborted, so the slot fails after N pointless round trips.
Suggest joining the two (errors.Wrapf(err, "...; rollback also failed: %v", rollbackErr)) so the copy error always reaches the log.
| if err != nil { | ||
| return nil, err | ||
| } | ||
| block.ExecutionPayloadBid, err = s.executionPayloadBid(ctx, tx, block.Root) |
There was a problem hiding this comment.
N+1 query: the batch helper already exists and isn't used here.
executionPayloadBid is a thin wrapper that calls executionPayloadBids with a one-element slice, so every block in the range costs an extra SELECT ... WHERE f_block_root = ANY($1) round trip. BlocksForSlotRange is called over whole epochs (and wider) by the summarizer and finalizer, so a range of 32 blocks goes from 2 payload-related queries to 33, and this cost is paid by pre-Gloas deployments that will never have a row in the new table.
Blocks() (line ~272) already does the right thing: collect the roots, call executionPayloadBids once, then assign. The same treatment applies to BlocksBySlot (386), BlocksForSlotRange (494), BlocksByParentRoot (739) and LatestBlocks (935) — note those four also already issue a per-block executionPayload call, so a single batched pass would be a strict improvement.
Glaos Support
t_block_execution_payload_bidstable (schema v17).Other minor improvements:
COALESCEon upsert).