fix: skip already-seen data columns in zero-peer publish warning#9580
fix: skip already-seen data columns in zero-peer publish warning#9580lodekeeper wants to merge 4 commits into
Conversation
Post-gloas self-builds publish the block before the execution payload envelope. Peers fetch the blobs from their EL and gossip the data columns back to us in between, so by the time we publish the envelope's columns they are often already in our seen cache. Publishing them then resolves to zero recipients (a benign gossipsub duplicate), not a propagation failure - yet we counted them towards the "Published data columns to 0 peers, increased risk of reorg" warning, which fired on every published slot on builder nodes despite the columns being fully available on the network. Track which columns were already present in the payload input (addColumn returns false) and exclude them when counting zero-peer publishes, so the warning only fires for columns we genuinely failed to disseminate. Ref ChainSafe#9527 🤖 Generated with AI assistance Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to track whether data columns were already present in the seen cache before self-publishing. It adds a helper function countColumnsPublishedWithZeroPeers to avoid triggering false-positive warnings about zero-peer propagation for columns that were already gossiped to the node. Unit tests have also been added to verify this behavior. There are no review comments, so I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a2f6043dd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // 0-peers publish is an expected duplicate rather than a propagation failure (see #9527). | ||
| const columnAlreadyPresent = dataColumnSidecars.map( | ||
| (columnSidecar) => | ||
| !payloadInput.addColumn({ |
There was a problem hiding this comment.
Distinguish cached columns from gossiped duplicates
This uses addColumn() returning false as proof that a peer already gossiped the column, but PayloadEnvelopeInput can also be pre-populated from non-gossip paths such as execution-engine getBlobsV2, req/resp, or recovery. In those cases the later publishDataColumnSidecar() can still return 0 because there are no topic peers, yet the column is excluded from the reorg-risk warning, and the earlier non-gossip population path does not report this zero-peer condition.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — you're right, and it's reachable. I was keying the suppression off cache presence (addColumn() returning false), but the payload-envelope cache is also populated by non-gossip paths — verified: engine/getBlobsV2 (util/execution.ts), recovery (util/dataColumns.ts), and req/resp by-range/by-root — all into the same payloadInput. Those columns aren't necessarily on the network, so a 0-peer publish of them is a real propagation failure that my check was silently suppressing.
Fixed in aff57d1: added PayloadEnvelopeInput.getColumnSource() and now suppress the warning only when the already-present column was gossip-sourced (a peer actively pushed it to us → it's propagating). Columns cached from engine/recovery/req_resp still count toward the zero-peer warning. Added a regression test for the non-gossip case.
…olumns Address Codex review on ChainSafe#9580: the suppression keyed off "column already in the payload-envelope cache", but that cache is also populated by non-gossip paths (engine/getBlobsV2, req/resp, recovery). Those columns are not necessarily on the network, so a 0-peer publish of them is a real propagation failure that was being silently suppressed. Add PayloadEnvelopeInput.getColumnSource() and suppress the warning only when the already-present column was gossip-sourced (a peer actively pushed it, so it is propagating). Columns cached from non-gossip sources still count toward the zero-peer warning. Rename the helper param/docs to columnAlreadyGossiped and add a regression test. 🤖 Generated with AI assistance Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nflaig
left a comment
There was a problem hiding this comment.
@lodekeeper I don't like this solution, can you please open a libp2p pr to fix the issue properly or come up with a cleaner solution for this on the lodestar side
|
Dug into the root cause — it doesn't need a libp2p PR, gossipsub already gives us the signal we're currently throwing away. Root cause: In the self-build flow we publish the block first; peers then fetch the blobs from their EL and gossip the columns back to us, so by the time we publish the envelope's columns they're already in our gossipsub seen cache. Decision — cleaner Lodestar-side fix, no libp2p PR: gossipsub already distinguishes the two cases ( Implementing that now and will update the PR. |
… of source heuristic
Replaces the `getColumnSource`/`columnAlreadyGossiped` heuristic with the
built-in gossipsub error distinction: `PublishError.Duplicate` (column
already seen, propagating) vs `PublishError.NoPeersSubscribedToTopic`
(genuine zero-peers failure). This is cleaner because gossipsub already
differentiates these cases at the protocol level.
`publishDataColumnSidecar` now returns `{sentPeers, alreadyPublished}`:
- `alreadyPublished: true` means gossipsub rejected as duplicate — the
column is already propagating on the network (expected in self-build
flows where peers gossip columns back before the envelope is published)
- `alreadyPublished: false` + `sentPeers === 0` is a real propagation
failure that still triggers the warning
Removes `countColumnsPublishedWithZeroPeers` helper and its test.
Closes ChainSafe#9527.
🤖 Generated with AI assistance
Co-Authored-By: lodekeeper <lodekeeper@users.noreply.github.com>
The gossip-source heuristic that used it was replaced by gossipsub's PublishError.Duplicate signal (cb30d1c); getColumnSource has no callers left. 🤖 Generated with AI assistance Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Problem
Builder nodes on glamsterdam-devnet-6 log this warning on every published slot:
buildoor-lodestar-besu-1produced 4888 of these since its Jun-26 restart, despite being synced with 31 peers and the columns being fully available on the network. It's a false alarm (see #9527).Root cause
When a post-gloas node self-builds, it publishes the block first, then the execution payload envelope (with its data columns) a moment later. In between, peers fetch the blobs from their EL via
getBlobsand gossip the columns back to us, so those columns land in ourpayloadInput(and gossipsub's seen cache) before we publish them.gossipsub's
publish()returnsrecipients: []in two different situations that we currently conflate:ignoreDuplicatePublishError: true) → the column is already on the networkallowPublishToZeroTopicPeers: true) → the real problemThe warning counted both as zero-peer publishes, so on a builder the benign duplicate case dominated and fired every slot.
Fix
payloadInput.addColumn()already returnsfalsewhen the column index is already cached — i.e. a peer gossiped it to us first. Track that per column and exclude already-present columns when counting zero-peer publishes, so the warning only fires for columns we genuinely failed to disseminate.This is the in-repo fix (Opt 2 from the Discord discussion), matching the issue note:
The
sentPeersPerSubnethistogram is intentionally left unchanged — 0 recipients is factually correct for a duplicate publish; only the reorg-risk warning is de-noised.The cleaner long-term fix (Opt 1) is to give gossipsub's
PublishResulta discriminator so the network layer can tell "empty because duplicate" from "empty because no peers" at the source. That needs a@libp2p/gossipsubrelease + bump, so it's left as follow-up and #9527 stays open to track it.Testing
countColumnsPublishedWithZeroPeerscovering all-new, all-duplicate, mixed, has-peers, and empty batches.biome checkclean; existing blocks-api unit tests pass.unstableare unchanged by this diff).Ref #9527
🤖 Generated with Claude Code