From 3c0110b0f8c9660d2b21f6ea05a21614342505cf Mon Sep 17 00:00:00 2001 From: harkamal Date: Sat, 17 Jun 2023 12:54:02 +0530 Subject: [PATCH 1/4] feat: publish blobsidecars instead of blobssidecar --- .../src/api/impl/beacon/blocks/index.ts | 21 +++++++--------- .../src/network/gossip/interface.ts | 4 +++ .../beacon-node/src/network/gossip/topic.ts | 5 ++++ packages/beacon-node/src/network/interface.ts | 4 +-- packages/beacon-node/src/network/network.ts | 25 +++++-------------- .../src/network/processor/gossipHandlers.ts | 4 +++ .../src/network/processor/gossipQueues.ts | 6 +++++ .../src/network/processor/index.ts | 1 + 8 files changed, 36 insertions(+), 34 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 9e7ee48579d9..02f5136d89b0 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -208,20 +208,16 @@ export function getBeaconBlockApi({ let blockForImport: BlockInput, signedBlock: allForks.SignedBeaconBlock, signedBlobs: deneb.SignedBlobSidecars; if (isSignedBlockContents(signedBlockOrContents)) { - // Build a blockInput for post deneb, signedBlobs will be be used in followup PRs ({signedBlock, signedBlobSidecars: signedBlobs} = signedBlockOrContents); - const blobsSidecar = blobSidecarsToBlobsSidecar( - config, - signedBlock, - signedBlobs.map(({message}) => message) - ); - blockForImport = getBlockInput.postDeneb( config, signedBlock, BlockSource.api, - // The blobsSidecar will be replaced in the followup PRs with just blobs - blobsSidecar + blobSidecarsToBlobsSidecar( + config, + signedBlock, + signedBlobs.map((sblob) => sblob.message) + ) ); } else { signedBlock = signedBlockOrContents; @@ -231,7 +227,8 @@ export function getBeaconBlockApi({ // Simple implementation of a pending block queue. Keeping the block here recycles the API logic, and keeps the // REST request promise without any extra infrastructure. - const msToBlockSlot = computeTimeAtSlot(config, signedBlock.message.slot, chain.genesisTime) * 1000 - Date.now(); + const msToBlockSlot = + computeTimeAtSlot(config, blockForImport.block.message.slot, chain.genesisTime) * 1000 - Date.now(); if (msToBlockSlot <= MAX_API_CLOCK_DISPARITY_MS && msToBlockSlot > 0) { // If block is a bit early, hold it in a promise. Equivalent to a pending queue. await sleep(msToBlockSlot); @@ -242,7 +239,7 @@ export function getBeaconBlockApi({ const publishPromises = [ // Send the block, regardless of whether or not it is valid. The API // specification is very clear that this is the desired behaviour. - () => network.publishBeaconBlockMaybeBlobs(blockForImport) as Promise, + () => network.publishBeaconBlock(signedBlock) as Promise, () => // there is no rush to persist block since we published it to gossip anyway chain.processBlock(blockForImport, {...opts, eagerPersistBlock: false}).catch((e) => { @@ -254,7 +251,7 @@ export function getBeaconBlockApi({ } throw e; }), - // TODO deneb: publish signed blobs as well + ...signedBlobs.map((signedBlob) => () => network.publishBlobSidecar(signedBlob)), ]; await promiseAllMaybeAsync(publishPromises); }, diff --git a/packages/beacon-node/src/network/gossip/interface.ts b/packages/beacon-node/src/network/gossip/interface.ts index e77205de01d9..dc178e8ee246 100644 --- a/packages/beacon-node/src/network/gossip/interface.ts +++ b/packages/beacon-node/src/network/gossip/interface.ts @@ -10,6 +10,7 @@ import {JobItemQueue} from "../../util/queue/index.js"; export enum GossipType { beacon_block = "beacon_block", + blob_sidecar = "blob_sidecar", beacon_block_and_blobs_sidecar = "beacon_block_and_blobs_sidecar", beacon_aggregate_and_proof = "beacon_aggregate_and_proof", beacon_attestation = "beacon_attestation", @@ -38,6 +39,7 @@ export interface IGossipTopic { export type GossipTopicTypeMap = { [GossipType.beacon_block]: {type: GossipType.beacon_block}; + [GossipType.blob_sidecar]: {type: GossipType.blob_sidecar; index: number}; [GossipType.beacon_block_and_blobs_sidecar]: {type: GossipType.beacon_block_and_blobs_sidecar}; [GossipType.beacon_aggregate_and_proof]: {type: GossipType.beacon_aggregate_and_proof}; [GossipType.beacon_attestation]: {type: GossipType.beacon_attestation; subnet: number}; @@ -68,6 +70,7 @@ export type SSZTypeOfGossipTopic = T extends {type: infer export type GossipTypeMap = { [GossipType.beacon_block]: allForks.SignedBeaconBlock; + [GossipType.blob_sidecar]: deneb.SignedBlobSidecar; [GossipType.beacon_block_and_blobs_sidecar]: deneb.SignedBeaconBlockAndBlobsSidecar; [GossipType.beacon_aggregate_and_proof]: phase0.SignedAggregateAndProof; [GossipType.beacon_attestation]: phase0.Attestation; @@ -83,6 +86,7 @@ export type GossipTypeMap = { export type GossipFnByType = { [GossipType.beacon_block]: (signedBlock: allForks.SignedBeaconBlock) => Promise | void; + [GossipType.blob_sidecar]: (signedBlobSidecar: deneb.SignedBlobSidecar) => Promise | void; [GossipType.beacon_block_and_blobs_sidecar]: ( signedBeaconBlockAndBlobsSidecar: deneb.SignedBeaconBlockAndBlobsSidecar ) => Promise | void; diff --git a/packages/beacon-node/src/network/gossip/topic.ts b/packages/beacon-node/src/network/gossip/topic.ts index 6cbf43bf66c9..3044b3190a6d 100644 --- a/packages/beacon-node/src/network/gossip/topic.ts +++ b/packages/beacon-node/src/network/gossip/topic.ts @@ -73,6 +73,8 @@ function stringifyGossipTopicType(topic: GossipTopic): string { case GossipType.beacon_attestation: case GossipType.sync_committee: return `${topic.type}_${topic.subnet}`; + case GossipType.blob_sidecar: + return `${topic.type}_${topic.index}`; } } @@ -82,6 +84,8 @@ export function getGossipSSZType(topic: GossipTopic) { case GossipType.beacon_block: // beacon_block is updated in altair to support the updated SignedBeaconBlock type return ssz[topic.fork].SignedBeaconBlock; + case GossipType.blob_sidecar: + return ssz.deneb.SignedBlobSidecar; case GossipType.beacon_block_and_blobs_sidecar: return ssz.deneb.SignedBeaconBlockAndBlobsSidecar; case GossipType.beacon_aggregate_and_proof: @@ -253,6 +257,7 @@ function parseEncodingStr(encodingStr: string): GossipEncoding { // TODO: Review which yes, and which not export const gossipTopicIgnoreDuplicatePublishError: Record = { [GossipType.beacon_block]: true, + [GossipType.blob_sidecar]: true, [GossipType.beacon_block_and_blobs_sidecar]: true, [GossipType.beacon_aggregate_and_proof]: true, [GossipType.beacon_attestation]: true, diff --git a/packages/beacon-node/src/network/interface.ts b/packages/beacon-node/src/network/interface.ts index 705e71ab6453..12c327f1108b 100644 --- a/packages/beacon-node/src/network/interface.ts +++ b/packages/beacon-node/src/network/interface.ts @@ -3,7 +3,6 @@ import {Connection} from "@libp2p/interface-connection"; import {Registrar} from "@libp2p/interface-registrar"; import {ConnectionManager} from "@libp2p/interface-connection-manager"; import {Slot, SlotRootHex, allForks, altair, capella, deneb, phase0} from "@lodestar/types"; -import {BlockInput} from "../chain/blocks/types.js"; import {PeerIdStr} from "../util/peerId.js"; import {INetworkEventBus} from "./events.js"; import {INetworkCorePublic} from "./core/types.js"; @@ -44,9 +43,8 @@ export interface INetwork extends INetworkCorePublic { sendBlobSidecarsByRoot(peerId: PeerIdStr, request: deneb.BlobSidecarsByRootRequest): Promise; // Gossip - publishBeaconBlockMaybeBlobs(blockInput: BlockInput): Promise; publishBeaconBlock(signedBlock: allForks.SignedBeaconBlock): Promise; - publishSignedBeaconBlockAndBlobsSidecar(item: deneb.SignedBeaconBlockAndBlobsSidecar): Promise; + publishBlobSidecar(signedBlobSidecar: deneb.SignedBlobSidecar): Promise; publishBeaconAggregateAndProof(aggregateAndProof: phase0.SignedAggregateAndProof): Promise; publishBeaconAttestation(attestation: phase0.Attestation, subnet: number): Promise; publishVoluntaryExit(voluntaryExit: phase0.SignedVoluntaryExit): Promise; diff --git a/packages/beacon-node/src/network/network.ts b/packages/beacon-node/src/network/network.ts index e4cfbee6f149..b07d01c3e083 100644 --- a/packages/beacon-node/src/network/network.ts +++ b/packages/beacon-node/src/network/network.ts @@ -14,7 +14,6 @@ import {IBeaconChain} from "../chain/index.js"; import {IBeaconDb} from "../db/interface.js"; import {PeerIdStr, peerIdToString} from "../util/peerId.js"; import {IClock} from "../util/clock.js"; -import {BlockInput, BlockInputType} from "../chain/blocks/types.js"; import {NetworkOptions} from "./options.js"; import {INetwork} from "./interface.js"; import {ReqRespMethod} from "./reqresp/index.js"; @@ -276,19 +275,6 @@ export class Network implements INetwork { // Gossip - async publishBeaconBlockMaybeBlobs(blockInput: BlockInput): Promise { - switch (blockInput.type) { - case BlockInputType.preDeneb: - return this.publishBeaconBlock(blockInput.block); - - case BlockInputType.postDeneb: - return this.publishSignedBeaconBlockAndBlobsSidecar({ - beaconBlock: blockInput.block as deneb.SignedBeaconBlock, - blobsSidecar: blockInput.blobs, - }); - } - } - async publishBeaconBlock(signedBlock: allForks.SignedBeaconBlock): Promise { const fork = this.config.getForkName(signedBlock.message.slot); return this.publishGossip({type: GossipType.beacon_block, fork}, signedBlock, { @@ -296,11 +282,12 @@ export class Network implements INetwork { }); } - async publishSignedBeaconBlockAndBlobsSidecar(item: deneb.SignedBeaconBlockAndBlobsSidecar): Promise { - const fork = this.config.getForkName(item.beaconBlock.message.slot); - return this.publishGossip( - {type: GossipType.beacon_block_and_blobs_sidecar, fork}, - item, + async publishBlobSidecar(signedBlobSidecar: deneb.SignedBlobSidecar): Promise { + const fork = this.config.getForkName(signedBlobSidecar.message.slot); + const index = signedBlobSidecar.message.index; + return this.publishGossip( + {type: GossipType.blob_sidecar, fork, index}, + signedBlobSidecar, {ignoreDuplicatePublishError: true} ); } diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index 5804eceb6144..98fb43c5ff60 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -190,6 +190,10 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH handleValidBeaconBlock({...blockInput, serializedData}, peerIdStr, seenTimestampSec); }, + [GossipType.blob_sidecar]: async (_data, _topic, _peerIdStr, _seenTimestampSec) => { + // TODO DENEB: impl to be added on migration of blockinput + }, + [GossipType.beacon_block_and_blobs_sidecar]: async ({serializedData}, topic, peerIdStr, seenTimestampSec) => { const blockAndBlocks = sszDeserialize(topic, serializedData); const {beaconBlock, blobsSidecar} = blockAndBlocks; diff --git a/packages/beacon-node/src/network/processor/gossipQueues.ts b/packages/beacon-node/src/network/processor/gossipQueues.ts index ace41af43b47..63abd16ad3a8 100644 --- a/packages/beacon-node/src/network/processor/gossipQueues.ts +++ b/packages/beacon-node/src/network/processor/gossipQueues.ts @@ -35,6 +35,12 @@ const gossipQueueOpts: { } = { // validation gossip block asap [GossipType.beacon_block]: {maxLength: 1024, type: QueueType.FIFO, dropOpts: {type: DropType.count, count: 1}}, + // gossip length for blob is beacon block length * max blobs per block = 4096 + [GossipType.blob_sidecar]: { + maxLength: 4096, + type: QueueType.FIFO, + dropOpts: {type: DropType.count, count: 1}, + }, // TODO DENEB: What's a good queue max given that now blocks are much bigger? [GossipType.beacon_block_and_blobs_sidecar]: { maxLength: 32, diff --git a/packages/beacon-node/src/network/processor/index.ts b/packages/beacon-node/src/network/processor/index.ts index 04f2dc480224..9d5d05e7a672 100644 --- a/packages/beacon-node/src/network/processor/index.ts +++ b/packages/beacon-node/src/network/processor/index.ts @@ -56,6 +56,7 @@ type WorkOpts = { */ const executeGossipWorkOrderObj: Record = { [GossipType.beacon_block]: {bypassQueue: true}, + [GossipType.blob_sidecar]: {bypassQueue: true}, [GossipType.beacon_block_and_blobs_sidecar]: {bypassQueue: true}, [GossipType.beacon_aggregate_and_proof]: {}, [GossipType.voluntary_exit]: {}, From 3c474bac75e36fa8509e2387a49134bdd7f23565 Mon Sep 17 00:00:00 2001 From: harkamal Date: Sat, 17 Jun 2023 12:56:40 +0530 Subject: [PATCH 2/4] add comment --- packages/beacon-node/src/api/impl/beacon/blocks/index.ts | 1 + 1 file changed, 1 insertion(+) 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 02f5136d89b0..aee1ddfc2ce2 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -213,6 +213,7 @@ export function getBeaconBlockApi({ config, signedBlock, BlockSource.api, + // The blobsSidecar will be replaced in the followup PRs with just blobs blobSidecarsToBlobsSidecar( config, signedBlock, From 0213641b43bced2f14fcd3b5f3577b1b8432db3e Mon Sep 17 00:00:00 2001 From: harkamal Date: Sat, 17 Jun 2023 13:01:32 +0530 Subject: [PATCH 3/4] fix test --- packages/beacon-node/test/unit/network/gossip/topic.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/beacon-node/test/unit/network/gossip/topic.test.ts b/packages/beacon-node/test/unit/network/gossip/topic.test.ts index 7eb037a8fe43..c2e6bb5c8015 100644 --- a/packages/beacon-node/test/unit/network/gossip/topic.test.ts +++ b/packages/beacon-node/test/unit/network/gossip/topic.test.ts @@ -15,6 +15,12 @@ describe("network / gossip / topic", function () { topicStr: "/eth2/18ae4ccb/beacon_block/ssz_snappy", }, ], + [GossipType.blob_sidecar]: [ + { + topic: {type: GossipType.blob_sidecar, index: 1, fork: ForkName.deneb, encoding}, + topicStr: "/eth2/46acb19a/blob_sidecar_1/ssz_snappy", + }, + ], [GossipType.beacon_block_and_blobs_sidecar]: [ { topic: {type: GossipType.beacon_block_and_blobs_sidecar, fork: ForkName.deneb, encoding}, From 2531003efea9d1a2e53a814100dc1288903eedca Mon Sep 17 00:00:00 2001 From: harkamal Date: Sat, 17 Jun 2023 21:40:40 +0530 Subject: [PATCH 4/4] fix test --- packages/beacon-node/src/network/gossip/topic.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/beacon-node/src/network/gossip/topic.ts b/packages/beacon-node/src/network/gossip/topic.ts index 3044b3190a6d..8c5a35e74cfd 100644 --- a/packages/beacon-node/src/network/gossip/topic.ts +++ b/packages/beacon-node/src/network/gossip/topic.ts @@ -185,6 +185,13 @@ export function parseGossipTopic(forkDigestContext: ForkDigestContext, topicStr: } } + if (gossipTypeStr.startsWith(GossipType.blob_sidecar)) { + const indexStr = gossipTypeStr.slice(GossipType.blob_sidecar.length + 1); // +1 for '_' concatenating the topic name and the index + const index = parseInt(indexStr, 10); + if (Number.isNaN(index)) throw Error(`index ${indexStr} is not a number`); + return {type: GossipType.blob_sidecar, index, fork, encoding}; + } + throw Error(`Unknown gossip type ${gossipTypeStr}`); } catch (e) { (e as Error).message = `Invalid gossip topic ${topicStr}: ${(e as Error).message}`;