From 8a2f6043ddc2ae5d21d864f298279395aad99b37 Mon Sep 17 00:00:00 2001 From: lodekeeper Date: Thu, 2 Jul 2026 20:53:36 +0000 Subject: [PATCH 1/4] fix: skip already-seen data columns in zero-peer publish warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #9527 🤖 Generated with AI assistance Co-Authored-By: Claude Opus 4.8 --- .../src/api/impl/beacon/blocks/index.ts | 33 +++++++++++-------- .../src/api/impl/beacon/blocks/utils.ts | 27 +++++++++++++++ .../unit/api/impl/beacon/blocks/utils.test.ts | 30 +++++++++++++++++ 3 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts diff --git a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts index 8a2c82192618..fe0699083c60 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -68,7 +68,7 @@ import {kzg} from "../../../../util/kzg.js"; import {promiseAllMaybeAsync} from "../../../../util/promises.js"; import {ApiModules} from "../../types.js"; import {assertUniqueItems} from "../../utils.js"; -import {getBlockResponse, toBeaconHeaderResponse} from "./utils.js"; +import {countColumnsPublishedWithZeroPeers, getBlockResponse, toBeaconHeaderResponse} from "./utils.js"; type PublishBlockOpts = ImportBlockOpts; @@ -739,16 +739,20 @@ export function getBeaconBlockApi({ peerIdStr: undefined, }); - if (dataColumnSidecars.length > 0) { - for (const columnSidecar of dataColumnSidecars) { - payloadInput.addColumn({ + // Track which columns were already present in the shared payload input before we self-publish. + // addColumn() returns false when the column index is already cached, i.e. a peer gossiped the + // column to us first (block published first -> peers getBlobs from EL -> disseminate columns) + // before we published the envelope. Such columns are already on the network, so a subsequent + // 0-peers publish is an expected duplicate rather than a propagation failure (see #9527). + const columnAlreadyPresent = dataColumnSidecars.map( + (columnSidecar) => + !payloadInput.addColumn({ columnSidecar, source: PayloadEnvelopeInputSource.api, seenTimestampSec, peerIdStr: undefined, - }); - } - } + }) + ); const valLogMeta = { slot, @@ -787,15 +791,18 @@ export function getBeaconBlockApi({ // Track metrics for data column publishing if (dataColumnSidecars.length > 0) { - let columnsPublishedWithZeroPeers = 0; // Skip first entry (envelope); the final entry is processExecutionPayload(), which returns void. - for (let i = 0; i < dataColumnSidecars.length; i++) { - const sentPeers = sentPeersArr[i + 1] as number; + const sentPeersPerColumn = dataColumnSidecars.map((_, i) => sentPeersArr[i + 1] as number); + for (const sentPeers of sentPeersPerColumn) { metrics?.dataColumns.sentPeersPerSubnet.observe(sentPeers); - if (sentPeers === 0) { - columnsPublishedWithZeroPeers++; - } } + // Columns already in our seen cache were gossiped to us first, so publishing them is a no-op + // duplicate with 0 recipients — expected, not a propagation failure. Only warn for columns we + // actually introduced to the network. See https://github.com/ChainSafe/lodestar/issues/9527. + const columnsPublishedWithZeroPeers = countColumnsPublishedWithZeroPeers( + sentPeersPerColumn, + columnAlreadyPresent + ); if (columnsPublishedWithZeroPeers > 0) { chain.logger.warn("Published data columns to 0 peers, increased risk of reorg", { slot, diff --git a/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts b/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts index b27979dc4149..c2f0840da59f 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts @@ -56,6 +56,33 @@ export function resolveBlockId(forkChoice: IForkChoice, blockId: routes.beacon.B return blockSlot; } +/** + * Count data columns that were published to zero peers AND were newly introduced to the network by us. + * + * When self-building post-gloas, the block is published first, then peers fetch the blobs from their EL + * and disseminate the columns via gossip before we publish the execution payload envelope. Those columns + * can therefore already be in our seen cache by the time we publish them, in which case our publish is a + * no-op duplicate that resolves to zero recipients — expected, not a propagation failure. Only columns we + * actually introduced to the network (not already present) should count towards the zero-peers warning. + * + * See https://github.com/ChainSafe/lodestar/issues/9527. + * + * @param sentPeersPerColumn number of peers each column was published to, aligned with `columnAlreadyPresent` + * @param columnAlreadyPresent whether each column was already in our seen cache before we published it + */ +export function countColumnsPublishedWithZeroPeers( + sentPeersPerColumn: number[], + columnAlreadyPresent: boolean[] +): number { + let count = 0; + for (let i = 0; i < sentPeersPerColumn.length; i++) { + if (sentPeersPerColumn[i] === 0 && !columnAlreadyPresent[i]) { + count++; + } + } + return count; +} + export async function getBlockResponse( chain: IBeaconChain, blockId: routes.beacon.BlockId diff --git a/packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts b/packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts new file mode 100644 index 000000000000..e155628394a0 --- /dev/null +++ b/packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts @@ -0,0 +1,30 @@ +import {describe, expect, it} from "vitest"; +import {countColumnsPublishedWithZeroPeers} from "../../../../../../src/api/impl/beacon/blocks/utils.js"; + +describe("api - beacon - blocks - countColumnsPublishedWithZeroPeers", () => { + it("counts newly introduced columns published to zero peers", () => { + // none of the columns were already in the seen cache, all reached zero peers + expect(countColumnsPublishedWithZeroPeers([0, 0, 0], [false, false, false])).toBe(3); + }); + + it("ignores zero-peer columns that were already in the seen cache (benign duplicates)", () => { + // the buildoor case: peers gossiped every column to us first, so all publishes are duplicates + expect(countColumnsPublishedWithZeroPeers([0, 0, 0], [true, true, true])).toBe(0); + }); + + it("only counts newly introduced zero-peer columns in a mixed batch", () => { + // index 0: newly introduced, zero peers -> counted + // index 1: already present, zero peers -> benign duplicate, not counted + // index 2: newly introduced, reached peers -> not counted + // index 3: already present, reached peers -> not counted + expect(countColumnsPublishedWithZeroPeers([0, 0, 5, 5], [false, true, false, true])).toBe(1); + }); + + it("never counts columns that reached at least one peer", () => { + expect(countColumnsPublishedWithZeroPeers([1, 2, 3], [false, false, false])).toBe(0); + }); + + it("returns 0 for an empty batch", () => { + expect(countColumnsPublishedWithZeroPeers([], [])).toBe(0); + }); +}); From aff57d130813ed8e4475439de08f009d8c0b950d Mon Sep 17 00:00:00 2001 From: lodekeeper Date: Thu, 2 Jul 2026 21:09:42 +0000 Subject: [PATCH 2/4] fix(api): only suppress zero-peer column warning for gossip-sourced columns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Codex review on #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 --- .../src/api/impl/beacon/blocks/index.ts | 35 ++++++++++--------- .../src/api/impl/beacon/blocks/utils.ts | 17 +++++---- .../payloadEnvelopeInput.ts | 6 ++++ .../unit/api/impl/beacon/blocks/utils.test.ts | 16 ++++++--- 4 files changed, 46 insertions(+), 28 deletions(-) diff --git a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts index fe0699083c60..96eb7be92624 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -739,20 +739,23 @@ export function getBeaconBlockApi({ peerIdStr: undefined, }); - // Track which columns were already present in the shared payload input before we self-publish. - // addColumn() returns false when the column index is already cached, i.e. a peer gossiped the - // column to us first (block published first -> peers getBlobs from EL -> disseminate columns) - // before we published the envelope. Such columns are already on the network, so a subsequent - // 0-peers publish is an expected duplicate rather than a propagation failure (see #9527). - const columnAlreadyPresent = dataColumnSidecars.map( - (columnSidecar) => - !payloadInput.addColumn({ - columnSidecar, - source: PayloadEnvelopeInputSource.api, - seenTimestampSec, - peerIdStr: undefined, - }) - ); + // Track which columns a peer had already gossiped to us before we self-publish. A gossip-sourced + // column is actively propagating on the network (block published first -> peers getBlobs from EL + // -> disseminate columns) before we publish the envelope, so a subsequent 0-peers publish is an + // expected duplicate rather than a propagation failure (see #9527). We check the column's *source* + // rather than mere presence: columns cached from non-gossip paths (engine/getBlobs, req/resp, + // recovery) are not necessarily on the network, so a 0-peers publish of those is a real + // propagation failure and must still warn. + const columnAlreadyGossiped = dataColumnSidecars.map((columnSidecar) => { + const alreadyGossiped = payloadInput.getColumnSource(columnSidecar.index) === PayloadEnvelopeInputSource.gossip; + payloadInput.addColumn({ + columnSidecar, + source: PayloadEnvelopeInputSource.api, + seenTimestampSec, + peerIdStr: undefined, + }); + return alreadyGossiped; + }); const valLogMeta = { slot, @@ -796,12 +799,12 @@ export function getBeaconBlockApi({ for (const sentPeers of sentPeersPerColumn) { metrics?.dataColumns.sentPeersPerSubnet.observe(sentPeers); } - // Columns already in our seen cache were gossiped to us first, so publishing them is a no-op + // Columns a peer already gossiped to us are actively propagating, so publishing them is a no-op // duplicate with 0 recipients — expected, not a propagation failure. Only warn for columns we // actually introduced to the network. See https://github.com/ChainSafe/lodestar/issues/9527. const columnsPublishedWithZeroPeers = countColumnsPublishedWithZeroPeers( sentPeersPerColumn, - columnAlreadyPresent + columnAlreadyGossiped ); if (columnsPublishedWithZeroPeers > 0) { chain.logger.warn("Published data columns to 0 peers, increased risk of reorg", { diff --git a/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts b/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts index c2f0840da59f..644c0baf2067 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts @@ -61,22 +61,25 @@ export function resolveBlockId(forkChoice: IForkChoice, blockId: routes.beacon.B * * When self-building post-gloas, the block is published first, then peers fetch the blobs from their EL * and disseminate the columns via gossip before we publish the execution payload envelope. Those columns - * can therefore already be in our seen cache by the time we publish them, in which case our publish is a - * no-op duplicate that resolves to zero recipients — expected, not a propagation failure. Only columns we - * actually introduced to the network (not already present) should count towards the zero-peers warning. + * can therefore already be in our seen cache (gossip-sourced) by the time we publish them, in which case + * our publish is a no-op duplicate that resolves to zero recipients — expected, not a propagation failure. + * Only columns we actually introduced to the network (not already gossiped to us) should count towards + * the zero-peers warning. Columns cached from non-gossip paths (engine/getBlobs, req/resp, recovery) are + * NOT necessarily on the network, so the caller must pass `columnAlreadyGossiped` (gossip source only), + * not mere cache presence. * * See https://github.com/ChainSafe/lodestar/issues/9527. * - * @param sentPeersPerColumn number of peers each column was published to, aligned with `columnAlreadyPresent` - * @param columnAlreadyPresent whether each column was already in our seen cache before we published it + * @param sentPeersPerColumn number of peers each column was published to, aligned with `columnAlreadyGossiped` + * @param columnAlreadyGossiped whether each column was gossiped to us by a peer before we published it */ export function countColumnsPublishedWithZeroPeers( sentPeersPerColumn: number[], - columnAlreadyPresent: boolean[] + columnAlreadyGossiped: boolean[] ): number { let count = 0; for (let i = 0; i < sentPeersPerColumn.length; i++) { - if (sentPeersPerColumn[i] === 0 && !columnAlreadyPresent[i]) { + if (sentPeersPerColumn[i] === 0 && !columnAlreadyGossiped[i]) { count++; } } diff --git a/packages/beacon-node/src/chain/blocks/payloadEnvelopeInput/payloadEnvelopeInput.ts b/packages/beacon-node/src/chain/blocks/payloadEnvelopeInput/payloadEnvelopeInput.ts index 7eeee4f33240..119759709008 100644 --- a/packages/beacon-node/src/chain/blocks/payloadEnvelopeInput/payloadEnvelopeInput.ts +++ b/packages/beacon-node/src/chain/blocks/payloadEnvelopeInput/payloadEnvelopeInput.ts @@ -9,6 +9,7 @@ import { ColumnWithSource, CreateFromBidProps, CreateFromBlockProps, + PayloadEnvelopeInputSource, SourceMeta, } from "./types.js"; @@ -278,6 +279,11 @@ export class PayloadEnvelopeInput { return this.columnsCache.get(index)?.columnSidecar; } + /** Source that first populated the cached column (gossip, engine/getBlobs, req/resp, recovery, ...), if present. */ + getColumnSource(index: ColumnIndex): PayloadEnvelopeInputSource | undefined { + return this.columnsCache.get(index)?.source; + } + getAllColumns(): gloas.DataColumnSidecar[] { return [...this.columnsCache.values()].map(({columnSidecar}) => columnSidecar); } diff --git a/packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts b/packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts index e155628394a0..47137a7ee5e3 100644 --- a/packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts +++ b/packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts @@ -7,16 +7,22 @@ describe("api - beacon - blocks - countColumnsPublishedWithZeroPeers", () => { expect(countColumnsPublishedWithZeroPeers([0, 0, 0], [false, false, false])).toBe(3); }); - it("ignores zero-peer columns that were already in the seen cache (benign duplicates)", () => { + it("ignores zero-peer columns that a peer already gossiped to us (benign duplicates)", () => { // the buildoor case: peers gossiped every column to us first, so all publishes are duplicates expect(countColumnsPublishedWithZeroPeers([0, 0, 0], [true, true, true])).toBe(0); }); + it("still counts a zero-peer column cached via a non-gossip path (engine/getBlobs, req/resp, recovery)", () => { + // Such columns are not necessarily on the network, so the caller passes them as `false` (not + // gossiped) and their 0-peer publish is a real propagation failure that must warn (see #9580 review). + expect(countColumnsPublishedWithZeroPeers([0, 0], [false, false])).toBe(2); + }); + it("only counts newly introduced zero-peer columns in a mixed batch", () => { - // index 0: newly introduced, zero peers -> counted - // index 1: already present, zero peers -> benign duplicate, not counted - // index 2: newly introduced, reached peers -> not counted - // index 3: already present, reached peers -> not counted + // index 0: not gossiped to us, zero peers -> counted + // index 1: already gossiped to us, zero peers -> benign duplicate, not counted + // index 2: not gossiped to us, reached peers -> not counted + // index 3: already gossiped to us, reached peers -> not counted expect(countColumnsPublishedWithZeroPeers([0, 0, 5, 5], [false, true, false, true])).toBe(1); }); From cb30d1c79df832da26ec0d1751ff800a6272ee02 Mon Sep 17 00:00:00 2001 From: lodekeeper Date: Sun, 5 Jul 2026 16:27:08 +0000 Subject: [PATCH 3/4] fix: catch PublishError.Duplicate in publishDataColumnSidecar instead of source heuristic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 https://github.com/ChainSafe/lodestar/issues/9527. 🤖 Generated with AI assistance Co-Authored-By: lodekeeper --- .../src/api/impl/beacon/blocks/index.ts | 63 +++++++++---------- .../src/api/impl/beacon/blocks/utils.ts | 30 --------- packages/beacon-node/src/network/interface.ts | 4 +- packages/beacon-node/src/network/network.ts | 39 +++++++----- packages/beacon-node/src/network/util.ts | 5 ++ packages/beacon-node/src/util/execution.ts | 2 +- .../unit/api/impl/beacon/blocks/utils.test.ts | 36 ----------- 7 files changed, 64 insertions(+), 115 deletions(-) delete mode 100644 packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts diff --git a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts index 96eb7be92624..11caee2886f5 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -68,7 +68,7 @@ import {kzg} from "../../../../util/kzg.js"; import {promiseAllMaybeAsync} from "../../../../util/promises.js"; import {ApiModules} from "../../types.js"; import {assertUniqueItems} from "../../utils.js"; -import {countColumnsPublishedWithZeroPeers, getBlockResponse, toBeaconHeaderResponse} from "./utils.js"; +import {getBlockResponse, toBeaconHeaderResponse} from "./utils.js"; type PublishBlockOpts = ImportBlockOpts; @@ -345,7 +345,9 @@ export function getBeaconBlockApi({ throw e; }), ]; - const sentPeersArr = await promiseAllMaybeAsync(publishPromises); + const sentPeersArr = await promiseAllMaybeAsync<{sentPeers: number; alreadyPublished: boolean} | number | void>( + publishPromises + ); if (isForkPostGloas(fork)) { // After gloas, data columns are not published with the block but when publishing the execution payload envelope @@ -356,10 +358,13 @@ export function getBeaconBlockApi({ // + 1 because we publish to beacon_block first for (let i = 0; i < dataColumnSidecars.length; i++) { // + 1 because we publish to beacon_block first - const sentPeers = sentPeersArr[i + 1] as number; - // sent peers could be 0 as we set `allowPublishToZeroTopicPeers=true` in network.publishDataColumnSidecar() api + const {sentPeers, alreadyPublished} = sentPeersArr[i + 1] as {sentPeers: number; alreadyPublished: boolean}; + // sent peers could be 0 as we set `allowPublishToZeroTopicPeers=true` in network.publishDataColumnSidecar() metrics?.dataColumns.sentPeersPerSubnet.observe(sentPeers); - if (sentPeers === 0) { + // A duplicate publish (alreadyPublished=true) means the column is already propagating on the network — + // expected in self-build flows where peers gossip columns back to us before we publish the envelope. + // Only warn when we genuinely failed to reach any peer. See https://github.com/ChainSafe/lodestar/issues/9527. + if (sentPeers === 0 && !alreadyPublished) { columnsPublishedWithZeroPeers++; } } @@ -739,23 +744,16 @@ export function getBeaconBlockApi({ peerIdStr: undefined, }); - // Track which columns a peer had already gossiped to us before we self-publish. A gossip-sourced - // column is actively propagating on the network (block published first -> peers getBlobs from EL - // -> disseminate columns) before we publish the envelope, so a subsequent 0-peers publish is an - // expected duplicate rather than a propagation failure (see #9527). We check the column's *source* - // rather than mere presence: columns cached from non-gossip paths (engine/getBlobs, req/resp, - // recovery) are not necessarily on the network, so a 0-peers publish of those is a real - // propagation failure and must still warn. - const columnAlreadyGossiped = dataColumnSidecars.map((columnSidecar) => { - const alreadyGossiped = payloadInput.getColumnSource(columnSidecar.index) === PayloadEnvelopeInputSource.gossip; - payloadInput.addColumn({ - columnSidecar, - source: PayloadEnvelopeInputSource.api, - seenTimestampSec, - peerIdStr: undefined, - }); - return alreadyGossiped; - }); + if (dataColumnSidecars.length > 0) { + for (const columnSidecar of dataColumnSidecars) { + payloadInput.addColumn({ + columnSidecar, + source: PayloadEnvelopeInputSource.api, + seenTimestampSec, + peerIdStr: undefined, + }); + } + } const valLogMeta = { slot, @@ -781,7 +779,9 @@ export function getBeaconBlockApi({ () => chain.processExecutionPayload(payloadInput, {validSignature: true}), ]; - const publishPromise = promiseAllMaybeAsync(publishPromises); + const publishPromise = promiseAllMaybeAsync<{sentPeers: number; alreadyPublished: boolean} | number | void>( + publishPromises + ); chain.emitter.emit(routes.events.EventType.executionPayloadGossip, { slot, @@ -794,18 +794,17 @@ export function getBeaconBlockApi({ // Track metrics for data column publishing if (dataColumnSidecars.length > 0) { + let columnsPublishedWithZeroPeers = 0; // Skip first entry (envelope); the final entry is processExecutionPayload(), which returns void. - const sentPeersPerColumn = dataColumnSidecars.map((_, i) => sentPeersArr[i + 1] as number); - for (const sentPeers of sentPeersPerColumn) { + for (let i = 0; i < dataColumnSidecars.length; i++) { + const {sentPeers, alreadyPublished} = sentPeersArr[i + 1] as {sentPeers: number; alreadyPublished: boolean}; metrics?.dataColumns.sentPeersPerSubnet.observe(sentPeers); + // A duplicate publish means the column is already propagating — expected in self-build flows. + // Only warn when we genuinely failed to reach any peer. See https://github.com/ChainSafe/lodestar/issues/9527. + if (sentPeers === 0 && !alreadyPublished) { + columnsPublishedWithZeroPeers++; + } } - // Columns a peer already gossiped to us are actively propagating, so publishing them is a no-op - // duplicate with 0 recipients — expected, not a propagation failure. Only warn for columns we - // actually introduced to the network. See https://github.com/ChainSafe/lodestar/issues/9527. - const columnsPublishedWithZeroPeers = countColumnsPublishedWithZeroPeers( - sentPeersPerColumn, - columnAlreadyGossiped - ); if (columnsPublishedWithZeroPeers > 0) { chain.logger.warn("Published data columns to 0 peers, increased risk of reorg", { slot, diff --git a/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts b/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts index 644c0baf2067..b27979dc4149 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/utils.ts @@ -56,36 +56,6 @@ export function resolveBlockId(forkChoice: IForkChoice, blockId: routes.beacon.B return blockSlot; } -/** - * Count data columns that were published to zero peers AND were newly introduced to the network by us. - * - * When self-building post-gloas, the block is published first, then peers fetch the blobs from their EL - * and disseminate the columns via gossip before we publish the execution payload envelope. Those columns - * can therefore already be in our seen cache (gossip-sourced) by the time we publish them, in which case - * our publish is a no-op duplicate that resolves to zero recipients — expected, not a propagation failure. - * Only columns we actually introduced to the network (not already gossiped to us) should count towards - * the zero-peers warning. Columns cached from non-gossip paths (engine/getBlobs, req/resp, recovery) are - * NOT necessarily on the network, so the caller must pass `columnAlreadyGossiped` (gossip source only), - * not mere cache presence. - * - * See https://github.com/ChainSafe/lodestar/issues/9527. - * - * @param sentPeersPerColumn number of peers each column was published to, aligned with `columnAlreadyGossiped` - * @param columnAlreadyGossiped whether each column was gossiped to us by a peer before we published it - */ -export function countColumnsPublishedWithZeroPeers( - sentPeersPerColumn: number[], - columnAlreadyGossiped: boolean[] -): number { - let count = 0; - for (let i = 0; i < sentPeersPerColumn.length; i++) { - if (sentPeersPerColumn[i] === 0 && !columnAlreadyGossiped[i]) { - count++; - } - } - return count; -} - export async function getBlockResponse( chain: IBeaconChain, blockId: routes.beacon.BlockId diff --git a/packages/beacon-node/src/network/interface.ts b/packages/beacon-node/src/network/interface.ts index a6c9b364cf34..e475eca079d7 100644 --- a/packages/beacon-node/src/network/interface.ts +++ b/packages/beacon-node/src/network/interface.ts @@ -103,7 +103,9 @@ export interface INetwork extends INetworkCorePublic { publishBlobSidecar(blobSidecar: deneb.BlobSidecar): Promise; publishBeaconAggregateAndProof(aggregateAndProof: SignedAggregateAndProof): Promise; publishBeaconAttestation(attestation: SingleAttestation, subnet: SubnetID): Promise; - publishDataColumnSidecar(dataColumnSideCar: DataColumnSidecar): Promise; + publishDataColumnSidecar( + dataColumnSideCar: DataColumnSidecar + ): Promise<{sentPeers: number; alreadyPublished: boolean}>; publishVoluntaryExit(voluntaryExit: phase0.SignedVoluntaryExit): Promise; publishBlsToExecutionChange(blsToExecutionChange: capella.SignedBLSToExecutionChange): Promise; publishProposerSlashing(proposerSlashing: phase0.ProposerSlashing): Promise; diff --git a/packages/beacon-node/src/network/network.ts b/packages/beacon-node/src/network/network.ts index 8b2f9bab4fcc..d10b879ab498 100644 --- a/packages/beacon-node/src/network/network.ts +++ b/packages/beacon-node/src/network/network.ts @@ -65,7 +65,7 @@ import { } from "./reqresp/utils/collect.js"; import {collectSequentialBlocksInRange} from "./reqresp/utils/collectSequentialBlocksInRange.js"; import {CommitteeSubscription} from "./subnets/index.js"; -import {isPublishToZeroPeersError, prettyPrintPeerIdStr} from "./util.js"; +import {isPublishDuplicateError, isPublishToZeroPeersError, prettyPrintPeerIdStr} from "./util.js"; type NetworkModules = { opts: NetworkOptions; @@ -366,7 +366,9 @@ export class Network implements INetwork { }); } - async publishDataColumnSidecar(dataColumnSidecar: DataColumnSidecar): Promise { + async publishDataColumnSidecar( + dataColumnSidecar: DataColumnSidecar + ): Promise<{sentPeers: number; alreadyPublished: boolean}> { const slot = isGloasDataColumnSidecar(dataColumnSidecar) ? dataColumnSidecar.slot : dataColumnSidecar.signedBlockHeader.message.slot; @@ -374,18 +376,25 @@ export class Network implements INetwork { const boundary = this.config.getForkBoundaryAtEpoch(epoch); const subnet = computeSubnetForDataColumnSidecar(this.config, dataColumnSidecar); - return this.publishGossip( - {type: GossipType.data_column_sidecar, boundary, subnet}, - dataColumnSidecar, - { - ignoreDuplicatePublishError: true, - // we ensure having all topic peers via prioritizePeers() function - // in the worse case, if there is 0 peer on the topic, the overall publish operation could be still a success - // because supernode will rebuild and publish missing data column sidecars for us - // hence we want to track sent peers as 0 instead of an error - allowPublishToZeroTopicPeers: true, + try { + const sentPeers = await this.publishGossip( + {type: GossipType.data_column_sidecar, boundary, subnet}, + dataColumnSidecar, + { + // we ensure having all topic peers via prioritizePeers() function + // in the worse case, if there is 0 peer on the topic, the overall publish operation could be still a success + // because supernode will rebuild and publish missing data column sidecars for us + // hence we want to track sent peers as 0 instead of an error + allowPublishToZeroTopicPeers: true, + } + ); + return {sentPeers, alreadyPublished: false}; + } catch (e) { + if (isPublishDuplicateError(e as Error)) { + return {sentPeers: 0, alreadyPublished: true}; } - ); + throw e; + } } async publishBeaconAggregateAndProof(aggregateAndProof: SignedAggregateAndProof): Promise { @@ -860,8 +869,8 @@ export class Network implements INetwork { this.core.setTargetGroupCount(count); }; - private onPublishDataColumns = (sidecars: DataColumnSidecar[]): Promise => { - return promiseAllMaybeAsync(sidecars.map((sidecar) => () => this.publishDataColumnSidecar(sidecar))); + private onPublishDataColumns = async (sidecars: DataColumnSidecar[]): Promise => { + await promiseAllMaybeAsync(sidecars.map((sidecar) => () => this.publishDataColumnSidecar(sidecar))); }; private onPublishBlobSidecars = (sidecars: deneb.BlobSidecar[]): Promise => { diff --git a/packages/beacon-node/src/network/util.ts b/packages/beacon-node/src/network/util.ts index 0a905f2c60c8..38a3b6cff398 100644 --- a/packages/beacon-node/src/network/util.ts +++ b/packages/beacon-node/src/network/util.ts @@ -27,3 +27,8 @@ export function getConnection(libp2p: Libp2p, peerIdStr: string): Connection | u export function isPublishToZeroPeersError(e: Error): boolean { return e.message.includes("PublishError.NoPeersSubscribedToTopic"); } + +// https://github.com/libp2p/js-libp2p/blob/f87cba928991736d9646b3e054c367f55cab315c/packages/gossipsub/src/gossipsub.ts#L2076 +export function isPublishDuplicateError(e: Error): boolean { + return e.message.includes("PublishError.Duplicate"); +} diff --git a/packages/beacon-node/src/util/execution.ts b/packages/beacon-node/src/util/execution.ts index b3a3e8cfe3b1..c1d367bfcda6 100644 --- a/packages/beacon-node/src/util/execution.ts +++ b/packages/beacon-node/src/util/execution.ts @@ -199,7 +199,7 @@ export async function getDataColumnSidecarsFromExecution( const previouslyMissingColumns = input.getMissingSampledColumnMeta().missing; const sampledColumns = previouslyMissingColumns.map((columnIndex) => dataColumnSidecars[columnIndex]); - // for columns that we already seen, it will be ignored through `ignoreDuplicatePublishError` gossip option + // for columns we have already seen, publishDataColumnSidecar() catches PublishError.Duplicate and marks alreadyPublished=true emitter.emit(ChainEvent.publishDataColumns, sampledColumns); // TODO: Can we record dataColumns.sentPeersPerSubnet metric here somehow diff --git a/packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts b/packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts deleted file mode 100644 index 47137a7ee5e3..000000000000 --- a/packages/beacon-node/test/unit/api/impl/beacon/blocks/utils.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -import {describe, expect, it} from "vitest"; -import {countColumnsPublishedWithZeroPeers} from "../../../../../../src/api/impl/beacon/blocks/utils.js"; - -describe("api - beacon - blocks - countColumnsPublishedWithZeroPeers", () => { - it("counts newly introduced columns published to zero peers", () => { - // none of the columns were already in the seen cache, all reached zero peers - expect(countColumnsPublishedWithZeroPeers([0, 0, 0], [false, false, false])).toBe(3); - }); - - it("ignores zero-peer columns that a peer already gossiped to us (benign duplicates)", () => { - // the buildoor case: peers gossiped every column to us first, so all publishes are duplicates - expect(countColumnsPublishedWithZeroPeers([0, 0, 0], [true, true, true])).toBe(0); - }); - - it("still counts a zero-peer column cached via a non-gossip path (engine/getBlobs, req/resp, recovery)", () => { - // Such columns are not necessarily on the network, so the caller passes them as `false` (not - // gossiped) and their 0-peer publish is a real propagation failure that must warn (see #9580 review). - expect(countColumnsPublishedWithZeroPeers([0, 0], [false, false])).toBe(2); - }); - - it("only counts newly introduced zero-peer columns in a mixed batch", () => { - // index 0: not gossiped to us, zero peers -> counted - // index 1: already gossiped to us, zero peers -> benign duplicate, not counted - // index 2: not gossiped to us, reached peers -> not counted - // index 3: already gossiped to us, reached peers -> not counted - expect(countColumnsPublishedWithZeroPeers([0, 0, 5, 5], [false, true, false, true])).toBe(1); - }); - - it("never counts columns that reached at least one peer", () => { - expect(countColumnsPublishedWithZeroPeers([1, 2, 3], [false, false, false])).toBe(0); - }); - - it("returns 0 for an empty batch", () => { - expect(countColumnsPublishedWithZeroPeers([], [])).toBe(0); - }); -}); From 3fe6662f495fe1b79769885811c77f76af4362e9 Mon Sep 17 00:00:00 2001 From: lodekeeper Date: Mon, 6 Jul 2026 03:10:10 +0000 Subject: [PATCH 4/4] refactor(chain): drop now-unused getColumnSource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gossip-source heuristic that used it was replaced by gossipsub's PublishError.Duplicate signal (cb30d1c7); getColumnSource has no callers left. 🤖 Generated with AI assistance Co-Authored-By: Claude Opus 4.8 --- .../blocks/payloadEnvelopeInput/payloadEnvelopeInput.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/payloadEnvelopeInput/payloadEnvelopeInput.ts b/packages/beacon-node/src/chain/blocks/payloadEnvelopeInput/payloadEnvelopeInput.ts index 119759709008..7eeee4f33240 100644 --- a/packages/beacon-node/src/chain/blocks/payloadEnvelopeInput/payloadEnvelopeInput.ts +++ b/packages/beacon-node/src/chain/blocks/payloadEnvelopeInput/payloadEnvelopeInput.ts @@ -9,7 +9,6 @@ import { ColumnWithSource, CreateFromBidProps, CreateFromBlockProps, - PayloadEnvelopeInputSource, SourceMeta, } from "./types.js"; @@ -279,11 +278,6 @@ export class PayloadEnvelopeInput { return this.columnsCache.get(index)?.columnSidecar; } - /** Source that first populated the cached column (gossip, engine/getBlobs, req/resp, recovery, ...), if present. */ - getColumnSource(index: ColumnIndex): PayloadEnvelopeInputSource | undefined { - return this.columnsCache.get(index)?.source; - } - getAllColumns(): gloas.DataColumnSidecar[] { return [...this.columnsCache.values()].map(({columnSidecar}) => columnSidecar); }