From 19a9a5090fa9b538c68b6035c72a089b54846b7c Mon Sep 17 00:00:00 2001 From: Julien Eluard Date: Mon, 22 Jan 2024 09:11:54 +0100 Subject: [PATCH 1/8] feat: blob sidecars can be filtered by indices --- .../api/src/beacon/routes/beacon/block.ts | 19 ++++++++++++++++--- .../api/test/unit/beacon/oapiSpec.test.ts | 6 ------ .../api/test/unit/beacon/testData/beacon.ts | 2 +- .../src/api/impl/beacon/blocks/index.ts | 6 ++++-- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/packages/api/src/beacon/routes/beacon/block.ts b/packages/api/src/beacon/routes/beacon/block.ts index 95a32097028d..f142be1a55db 100644 --- a/packages/api/src/beacon/routes/beacon/block.ts +++ b/packages/api/src/beacon/routes/beacon/block.ts @@ -229,8 +229,12 @@ export type Api = { * Retrieves BlobSidecar included in requested block. * @param blockId Block identifier. * Can be one of: "head" (canonical head in node's view), "genesis", "finalized", \, \. + * @param indices Array of indices for blob sidecars to request for in the specified block. Returns all blob sidecars in the block if not specified. */ - getBlobSidecars(blockId: BlockId): Promise< + getBlobSidecars( + blockId: BlockId, + indices?: string[] + ): Promise< ApiClientResponse<{ [HttpStatusCode.OK]: {executionOptimistic: ExecutionOptimistic; data: deneb.BlobSidecars}; }> @@ -270,7 +274,7 @@ export type ReqTypes = { publishBlockV2: {body: unknown; query: {broadcast_validation?: string}}; publishBlindedBlock: {body: unknown}; publishBlindedBlockV2: {body: unknown; query: {broadcast_validation?: string}}; - getBlobSidecars: BlockIdOnlyReq; + getBlobSidecars: {params: {block_id: string}; query: {indices?: string[]}}; }; export function getReqSerializers(config: ChainForkConfig): ReqSerializers { @@ -356,7 +360,16 @@ export function getReqSerializers(config: ChainForkConfig): ReqSerializers ({ + params: {block_id: String(block_id)}, + query: {indices}, + }), + parseReq: ({params, query}) => [params.block_id, query.indices], + schema: { + query: {indices: Schema.StringArray}, + }, + }, }; } diff --git a/packages/api/test/unit/beacon/oapiSpec.test.ts b/packages/api/test/unit/beacon/oapiSpec.test.ts index 15a10bfeb6f7..f7d6cb9a077c 100644 --- a/packages/api/test/unit/beacon/oapiSpec.test.ts +++ b/packages/api/test/unit/beacon/oapiSpec.test.ts @@ -130,12 +130,6 @@ const ignoredProperties: Record = { */ getHealth: {request: ["query.syncing_status"]}, - /** - * https://github.com/ChainSafe/lodestar/issues/6185 - * - must have required property 'query' - */ - getBlobSidecars: {request: ["query"]}, - /* https://github.com/ChainSafe/lodestar/issues/4638 /query - must have required property 'skip_randao_verification' diff --git a/packages/api/test/unit/beacon/testData/beacon.ts b/packages/api/test/unit/beacon/testData/beacon.ts index 7fa8368c590b..6a3eb502f77f 100644 --- a/packages/api/test/unit/beacon/testData/beacon.ts +++ b/packages/api/test/unit/beacon/testData/beacon.ts @@ -71,7 +71,7 @@ export const testData: GenericServerTestCases = { res: undefined, }, getBlobSidecars: { - args: ["head"], + args: ["head", ["0"]], res: {executionOptimistic: true, data: ssz.deneb.BlobSidecars.defaultValue()}, }, 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 f2e29f00fe57..0230161a912b 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -406,7 +406,7 @@ export function getBeaconBlockApi({ await publishBlock(signedBlockOrContents, opts); }, - async getBlobSidecars(blockId) { + async getBlobSidecars(blockId, indices = []) { const {block, executionOptimistic} = await resolveBlockId(chain, blockId); const blockRoot = config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message); @@ -418,9 +418,11 @@ export function getBeaconBlockApi({ if (!blobSidecars) { throw Error(`blobSidecars not found in db for slot=${block.message.slot} root=${toHexString(blockRoot)}`); } + + const indicesSet = new Set(indices?.map((index) => parseInt(index))); return { executionOptimistic, - data: blobSidecars, + data: blobSidecars.filter(({index}) => indicesSet.has(index)), }; }, }; From b588603572b446547d7ddf1336999fbf37662baf Mon Sep 17 00:00:00 2001 From: Julien Eluard Date: Wed, 24 Jan 2024 09:41:08 +0100 Subject: [PATCH 2/8] fix: properly filter blobs --- packages/api/src/beacon/routes/beacon/block.ts | 2 +- packages/beacon-node/src/api/impl/beacon/blocks/index.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/api/src/beacon/routes/beacon/block.ts b/packages/api/src/beacon/routes/beacon/block.ts index f142be1a55db..d377c26e0d36 100644 --- a/packages/api/src/beacon/routes/beacon/block.ts +++ b/packages/api/src/beacon/routes/beacon/block.ts @@ -229,7 +229,7 @@ export type Api = { * Retrieves BlobSidecar included in requested block. * @param blockId Block identifier. * Can be one of: "head" (canonical head in node's view), "genesis", "finalized", \, \. - * @param indices Array of indices for blob sidecars to request for in the specified block. Returns all blob sidecars in the block if not specified. + * @param indices Array of indices for blob sidecars to request for in the specified block. Returns all blob sidecars in the block if not specified. */ getBlobSidecars( blockId: BlockId, 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 0230161a912b..fe0706fbeb90 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -422,7 +422,8 @@ export function getBeaconBlockApi({ const indicesSet = new Set(indices?.map((index) => parseInt(index))); return { executionOptimistic, - data: blobSidecars.filter(({index}) => indicesSet.has(index)), + // Returnonly all sidecars if no indices are specified, only the requested ones (identified by `indices`) otherwise + data: indicesSet.size == 0 ? blobSidecars : blobSidecars.filter(({index}) => indicesSet.has(index)), }; }, }; From 9ae3a9cd3081897f2f15335aeb9f9da7c89e6f13 Mon Sep 17 00:00:00 2001 From: Julien Eluard Date: Wed, 24 Jan 2024 11:12:25 +0100 Subject: [PATCH 3/8] fix: type indices as number --- packages/api/src/beacon/routes/beacon/block.ts | 8 ++++---- packages/api/test/unit/beacon/testData/beacon.ts | 2 +- packages/api/test/utils/checkAgainstSpec.ts | 2 ++ packages/beacon-node/src/api/impl/beacon/blocks/index.ts | 6 +++--- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/api/src/beacon/routes/beacon/block.ts b/packages/api/src/beacon/routes/beacon/block.ts index d377c26e0d36..c1f1e723e358 100644 --- a/packages/api/src/beacon/routes/beacon/block.ts +++ b/packages/api/src/beacon/routes/beacon/block.ts @@ -233,7 +233,7 @@ export type Api = { */ getBlobSidecars( blockId: BlockId, - indices?: string[] + indices?: number[] ): Promise< ApiClientResponse<{ [HttpStatusCode.OK]: {executionOptimistic: ExecutionOptimistic; data: deneb.BlobSidecars}; @@ -274,7 +274,7 @@ export type ReqTypes = { publishBlockV2: {body: unknown; query: {broadcast_validation?: string}}; publishBlindedBlock: {body: unknown}; publishBlindedBlockV2: {body: unknown; query: {broadcast_validation?: string}}; - getBlobSidecars: {params: {block_id: string}; query: {indices?: string[]}}; + getBlobSidecars: {params: {block_id: string}; query: {indices?: number[]}}; }; export function getReqSerializers(config: ChainForkConfig): ReqSerializers { @@ -361,13 +361,13 @@ export function getReqSerializers(config: ChainForkConfig): ReqSerializers ({ + writeReq: (block_id, indices) => ({ params: {block_id: String(block_id)}, query: {indices}, }), parseReq: ({params, query}) => [params.block_id, query.indices], schema: { - query: {indices: Schema.StringArray}, + query: {indices: Schema.UintArray}, }, }, }; diff --git a/packages/api/test/unit/beacon/testData/beacon.ts b/packages/api/test/unit/beacon/testData/beacon.ts index 6a3eb502f77f..6d6bc6576f56 100644 --- a/packages/api/test/unit/beacon/testData/beacon.ts +++ b/packages/api/test/unit/beacon/testData/beacon.ts @@ -71,7 +71,7 @@ export const testData: GenericServerTestCases = { res: undefined, }, getBlobSidecars: { - args: ["head", ["0"]], + args: ["head", [0]], res: {executionOptimistic: true, data: ssz.deneb.BlobSidecars.defaultValue()}, }, diff --git a/packages/api/test/utils/checkAgainstSpec.ts b/packages/api/test/utils/checkAgainstSpec.ts index ed65279bca22..461e59f70015 100644 --- a/packages/api/test/utils/checkAgainstSpec.ts +++ b/packages/api/test/utils/checkAgainstSpec.ts @@ -204,6 +204,8 @@ function stringifyProperties(obj: Record): Record value.toString(10)); } } 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 fe0706fbeb90..f161220b4a2c 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -406,7 +406,7 @@ export function getBeaconBlockApi({ await publishBlock(signedBlockOrContents, opts); }, - async getBlobSidecars(blockId, indices = []) { + async getBlobSidecars(blockId, indices) { const {block, executionOptimistic} = await resolveBlockId(chain, blockId); const blockRoot = config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message); @@ -419,10 +419,10 @@ export function getBeaconBlockApi({ throw Error(`blobSidecars not found in db for slot=${block.message.slot} root=${toHexString(blockRoot)}`); } - const indicesSet = new Set(indices?.map((index) => parseInt(index))); + const indicesSet = new Set(indices); return { executionOptimistic, - // Returnonly all sidecars if no indices are specified, only the requested ones (identified by `indices`) otherwise + // Return all sidecars if no indices are specified, only the requested ones (identified by `indices`) otherwise data: indicesSet.size == 0 ? blobSidecars : blobSidecars.filter(({index}) => indicesSet.has(index)), }; }, From 1b428051330e4035065c55b443e32cf1b3602c76 Mon Sep 17 00:00:00 2001 From: Julien Eluard Date: Thu, 25 Jan 2024 09:10:29 +0100 Subject: [PATCH 4/8] chore: remove use of set --- packages/beacon-node/src/api/impl/beacon/blocks/index.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 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 f161220b4a2c..11a9901cb5ea 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -406,7 +406,7 @@ export function getBeaconBlockApi({ await publishBlock(signedBlockOrContents, opts); }, - async getBlobSidecars(blockId, indices) { + async getBlobSidecars(blockId, indices = []) { const {block, executionOptimistic} = await resolveBlockId(chain, blockId); const blockRoot = config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message); @@ -419,11 +419,10 @@ export function getBeaconBlockApi({ throw Error(`blobSidecars not found in db for slot=${block.message.slot} root=${toHexString(blockRoot)}`); } - const indicesSet = new Set(indices); return { executionOptimistic, // Return all sidecars if no indices are specified, only the requested ones (identified by `indices`) otherwise - data: indicesSet.size == 0 ? blobSidecars : blobSidecars.filter(({index}) => indicesSet.has(index)), + data: indices.length == 0 ? blobSidecars : blobSidecars.filter(({index}) => indices.includes(index)), }; }, }; From 9347c92566e06e45ef1d17479600c8dc1aed08f0 Mon Sep 17 00:00:00 2001 From: Julien Eluard Date: Thu, 25 Jan 2024 10:32:27 +0100 Subject: [PATCH 5/8] fix: make stringify more robust --- packages/api/test/utils/checkAgainstSpec.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/api/test/utils/checkAgainstSpec.ts b/packages/api/test/utils/checkAgainstSpec.ts index 461e59f70015..fd93bd611b36 100644 --- a/packages/api/test/utils/checkAgainstSpec.ts +++ b/packages/api/test/utils/checkAgainstSpec.ts @@ -199,14 +199,24 @@ function prettyAjvErrors(errors: ErrorObject[] | null | undefined): string { return errors.map((e) => `${e.instancePath ?? "."} - ${e.message}`).join("\n"); } +type StringifiedProperty = string | StringifiedProperty[] | null | undefined; + +function stringifyProperty(value: unknown): StringifiedProperty { + if (value === undefined || value === null || typeof value === "string") { + // Handle specifically null as `typeof null === "object"` + return value; + } else if (typeof value === "number") { + return value.toString(10); + } else if (Array.isArray(value)) { + return value.map(stringifyProperty); + } + return String(value); +} + function stringifyProperties(obj: Record): Record { for (const key of Object.keys(obj)) { const value = obj[key]; - if (typeof value === "number") { - obj[key] = value.toString(10); - } else if (Array.isArray(value)) { - obj[key] = value.map((value) => value.toString(10)); - } + obj[key] = stringifyProperty(value); } return obj; From e933acbe3a3e40b37daff12eb089580848549b18 Mon Sep 17 00:00:00 2001 From: Julien Date: Thu, 25 Jan 2024 07:02:06 -0800 Subject: [PATCH 6/8] Update packages/beacon-node/src/api/impl/beacon/blocks/index.ts Co-authored-by: Nico Flaig --- packages/beacon-node/src/api/impl/beacon/blocks/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 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 11a9901cb5ea..2a911b4e6cfe 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -421,8 +421,7 @@ export function getBeaconBlockApi({ return { executionOptimistic, - // Return all sidecars if no indices are specified, only the requested ones (identified by `indices`) otherwise - data: indices.length == 0 ? blobSidecars : blobSidecars.filter(({index}) => indices.includes(index)), + data: indices ? blobSidecars.filter(({index}) => indices.includes(index)) : blobSidecars, }; }, }; From e2365f5efff958d7f9b6f9d6673f5da8cc819fd5 Mon Sep 17 00:00:00 2001 From: Julien Eluard Date: Thu, 25 Jan 2024 16:41:47 +0100 Subject: [PATCH 7/8] fix: cleanup --- packages/api/test/utils/checkAgainstSpec.ts | 5 +---- packages/beacon-node/src/api/impl/beacon/blocks/index.ts | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/api/test/utils/checkAgainstSpec.ts b/packages/api/test/utils/checkAgainstSpec.ts index fd93bd611b36..db167d852480 100644 --- a/packages/api/test/utils/checkAgainstSpec.ts +++ b/packages/api/test/utils/checkAgainstSpec.ts @@ -202,10 +202,7 @@ function prettyAjvErrors(errors: ErrorObject[] | null | undefined): string { type StringifiedProperty = string | StringifiedProperty[] | null | undefined; function stringifyProperty(value: unknown): StringifiedProperty { - if (value === undefined || value === null || typeof value === "string") { - // Handle specifically null as `typeof null === "object"` - return value; - } else if (typeof value === "number") { + if (typeof value === "number") { return value.toString(10); } else if (Array.isArray(value)) { return value.map(stringifyProperty); 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 2a911b4e6cfe..6fde04bc737c 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -406,7 +406,7 @@ export function getBeaconBlockApi({ await publishBlock(signedBlockOrContents, opts); }, - async getBlobSidecars(blockId, indices = []) { + async getBlobSidecars(blockId, indices) { const {block, executionOptimistic} = await resolveBlockId(chain, blockId); const blockRoot = config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message); From 65348ec799853b8dae3264ca1ebfddd0a92c68f9 Mon Sep 17 00:00:00 2001 From: Julien Eluard Date: Fri, 26 Jan 2024 16:57:04 +0100 Subject: [PATCH 8/8] fix: remove useless types --- packages/api/test/utils/checkAgainstSpec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/test/utils/checkAgainstSpec.ts b/packages/api/test/utils/checkAgainstSpec.ts index db167d852480..c887f66e95e6 100644 --- a/packages/api/test/utils/checkAgainstSpec.ts +++ b/packages/api/test/utils/checkAgainstSpec.ts @@ -199,7 +199,7 @@ function prettyAjvErrors(errors: ErrorObject[] | null | undefined): string { return errors.map((e) => `${e.instancePath ?? "."} - ${e.message}`).join("\n"); } -type StringifiedProperty = string | StringifiedProperty[] | null | undefined; +type StringifiedProperty = string | StringifiedProperty[]; function stringifyProperty(value: unknown): StringifiedProperty { if (typeof value === "number") {