Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 27 additions & 17 deletions packages/beacon-node/src/api/impl/beacon/blocks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -739,16 +739,23 @@ export function getBeaconBlockApi({
peerIdStr: undefined,
});

if (dataColumnSidecars.length > 0) {
for (const columnSidecar of dataColumnSidecars) {
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,
Expand Down Expand Up @@ -787,15 +794,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 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,
Expand Down
30 changes: 30 additions & 0 deletions packages/beacon-node/src/api/impl/beacon/blocks/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,36 @@ 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
ColumnWithSource,
CreateFromBidProps,
CreateFromBlockProps,
PayloadEnvelopeInputSource,
SourceMeta,
} from "./types.js";

Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
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);
});
});