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..11caee2886f5 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -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++; } } @@ -774,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, @@ -790,9 +797,11 @@ export function getBeaconBlockApi({ 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 {sentPeers, alreadyPublished} = sentPeersArr[i + 1] as {sentPeers: number; alreadyPublished: boolean}; metrics?.dataColumns.sentPeersPerSubnet.observe(sentPeers); - if (sentPeers === 0) { + // 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++; } } 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