-
-
Notifications
You must be signed in to change notification settings - Fork 289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: download blocks as ssz #5923
Changes from all commits
f77128f
4f8b9ff
902e651
008805f
a5f4783
bf70d88
97dfcfc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,46 @@ | ||
import {ChainForkConfig} from "@lodestar/config"; | ||
import {Api, ReqTypes, routesData, getReqSerializers, getReturnTypes} from "../routes/beacon/index.js"; | ||
import {IHttpClient, generateGenericJsonClient} from "../../utils/client/index.js"; | ||
import {Api, ReqTypes, routesData, getReqSerializers, getReturnTypes, BlockId} from "../routes/beacon/index.js"; | ||
import {IHttpClient, generateGenericJsonClient, getFetchOptsSerializers} from "../../utils/client/index.js"; | ||
import {ResponseFormat} from "../../interfaces.js"; | ||
import {BlockResponse, BlockV2Response} from "../routes/beacon/block.js"; | ||
|
||
/** | ||
* REST HTTP client for beacon routes | ||
*/ | ||
export function getClient(config: ChainForkConfig, httpClient: IHttpClient): Api { | ||
const reqSerializers = getReqSerializers(config); | ||
const returnTypes = getReturnTypes(); | ||
// All routes return JSON, use a client auto-generator | ||
return generateGenericJsonClient<Api, ReqTypes>(routesData, reqSerializers, returnTypes, httpClient); | ||
// Some routes return JSON, use a client auto-generator | ||
const client = generateGenericJsonClient<Api, ReqTypes>(routesData, reqSerializers, returnTypes, httpClient); | ||
const fetchOptsSerializer = getFetchOptsSerializers<Api, ReqTypes>(routesData, reqSerializers); | ||
|
||
return { | ||
...client, | ||
async getBlock<T extends ResponseFormat = "json">(blockId: BlockId, format?: T) { | ||
if (format === "ssz") { | ||
const res = await httpClient.arrayBuffer({ | ||
...fetchOptsSerializer.getBlock(blockId, format), | ||
}); | ||
return { | ||
ok: true, | ||
response: new Uint8Array(res.body), | ||
status: res.status, | ||
} as BlockResponse<T>; | ||
} | ||
return client.getBlock(blockId, format); | ||
}, | ||
async getBlockV2<T extends ResponseFormat = "json">(blockId: BlockId, format?: T) { | ||
if (format === "ssz") { | ||
const res = await httpClient.arrayBuffer({ | ||
...fetchOptsSerializer.getBlockV2(blockId, format), | ||
}); | ||
return { | ||
ok: true, | ||
response: new Uint8Array(res.body), | ||
status: res.status, | ||
} as BlockV2Response<T>; | ||
} | ||
return client.getBlockV2(blockId, format); | ||
}, | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -18,7 +18,7 @@ import { | |||
ContainerData, | ||||
} from "../../../utils/index.js"; | ||||
import {HttpStatusCode} from "../../../utils/client/httpStatusCode.js"; | ||||
import {ApiClientResponse} from "../../../interfaces.js"; | ||||
import {ApiClientResponse, ResponseFormat} from "../../../interfaces.js"; | ||||
import { | ||||
SignedBlockContents, | ||||
SignedBlindedBlockContents, | ||||
|
@@ -31,6 +31,7 @@ import { | |||
// See /packages/api/src/routes/index.ts for reasoning and instructions to add new routes | ||||
|
||||
export type BlockId = RootHex | Slot | "head" | "genesis" | "finalized"; | ||||
export const mimeTypeSSZ = "application/octet-stream"; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This already exists for state downloads. Do you think we should move this to a CONST and pull both from the same place?
|
||||
|
||||
/** | ||||
* True if the response references an unverified execution payload. Optimistic information may be invalidated at | ||||
|
@@ -51,6 +52,26 @@ export enum BroadcastValidation { | |||
consensusAndEquivocation = "consensus_and_equivocation", | ||||
} | ||||
|
||||
export type BlockResponse<T extends ResponseFormat = "json"> = T extends "ssz" | ||||
? ApiClientResponse<{[HttpStatusCode.OK]: Uint8Array}, HttpStatusCode.BAD_REQUEST | HttpStatusCode.NOT_FOUND> | ||||
: ApiClientResponse< | ||||
{[HttpStatusCode.OK]: {data: allForks.SignedBeaconBlock}}, | ||||
HttpStatusCode.BAD_REQUEST | HttpStatusCode.NOT_FOUND | ||||
>; | ||||
|
||||
export type BlockV2Response<T extends ResponseFormat = "json"> = T extends "ssz" | ||||
? ApiClientResponse<{[HttpStatusCode.OK]: Uint8Array}, HttpStatusCode.BAD_REQUEST | HttpStatusCode.NOT_FOUND> | ||||
: ApiClientResponse< | ||||
{ | ||||
[HttpStatusCode.OK]: { | ||||
data: allForks.SignedBeaconBlock; | ||||
executionOptimistic: ExecutionOptimistic; | ||||
version: ForkName; | ||||
}; | ||||
}, | ||||
HttpStatusCode.BAD_REQUEST | HttpStatusCode.NOT_FOUND | ||||
>; | ||||
|
||||
Comment on lines
+55
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very nice TS work!!! 🚀 Is very helpful to see how you created these response types |
||||
export type Api = { | ||||
/** | ||||
* Get block | ||||
|
@@ -60,26 +81,15 @@ export type Api = { | |||
* @param blockId Block identifier. | ||||
* Can be one of: "head" (canonical head in node's view), "genesis", "finalized", \<slot\>, \<hex encoded blockRoot with 0x prefix\>. | ||||
*/ | ||||
getBlock(blockId: BlockId): Promise<ApiClientResponse<{[HttpStatusCode.OK]: {data: allForks.SignedBeaconBlock}}>>; | ||||
getBlock<T extends ResponseFormat = "json">(blockId: BlockId, format?: T): Promise<BlockResponse<T>>; | ||||
|
||||
/** | ||||
* Get block | ||||
* Retrieves block details for given block id. | ||||
* @param blockId Block identifier. | ||||
* Can be one of: "head" (canonical head in node's view), "genesis", "finalized", \<slot\>, \<hex encoded blockRoot with 0x prefix\>. | ||||
*/ | ||||
getBlockV2(blockId: BlockId): Promise< | ||||
ApiClientResponse< | ||||
{ | ||||
[HttpStatusCode.OK]: { | ||||
data: allForks.SignedBeaconBlock; | ||||
executionOptimistic: ExecutionOptimistic; | ||||
version: ForkName; | ||||
}; | ||||
}, | ||||
HttpStatusCode.BAD_REQUEST | HttpStatusCode.NOT_FOUND | ||||
> | ||||
>; | ||||
getBlockV2<T extends ResponseFormat = "json">(blockId: BlockId, format?: T): Promise<BlockV2Response<T>>; | ||||
|
||||
/** | ||||
* Get block attestations | ||||
|
@@ -246,11 +256,12 @@ export const routesData: RoutesData<Api> = { | |||
|
||||
/* eslint-disable @typescript-eslint/naming-convention */ | ||||
|
||||
type GetBlockReq = {params: {block_id: string}; headers: {accept?: string}}; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity why do we use the snake case for url params. Seems like a good pattern and I see that its the norm throughout the codebase. Just want to expand my understanding. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its part of the beacon api spec They say: use this specific url path with these specific url query params, should return a response with these codes and these payloads |
||||
type BlockIdOnlyReq = {params: {block_id: string}}; | ||||
|
||||
export type ReqTypes = { | ||||
getBlock: BlockIdOnlyReq; | ||||
getBlockV2: BlockIdOnlyReq; | ||||
getBlock: GetBlockReq; | ||||
getBlockV2: GetBlockReq; | ||||
getBlockAttestations: BlockIdOnlyReq; | ||||
getBlockHeader: BlockIdOnlyReq; | ||||
getBlockHeaders: {query: {slot?: number; parent_root?: string}}; | ||||
|
@@ -263,12 +274,21 @@ export type ReqTypes = { | |||
}; | ||||
|
||||
export function getReqSerializers(config: ChainForkConfig): ReqSerializers<Api, ReqTypes> { | ||||
const blockIdOnlyReq: ReqSerializer<Api["getBlock"], BlockIdOnlyReq> = { | ||||
const blockIdOnlyReq: ReqSerializer<Api["getBlockHeader"], BlockIdOnlyReq> = { | ||||
writeReq: (block_id) => ({params: {block_id: String(block_id)}}), | ||||
parseReq: ({params}) => [params.block_id], | ||||
schema: {params: {block_id: Schema.StringRequired}}, | ||||
}; | ||||
|
||||
const getBlockReq: ReqSerializer<Api["getBlock"], GetBlockReq> = { | ||||
writeReq: (block_id, format) => ({ | ||||
params: {block_id: String(block_id)}, | ||||
headers: {accept: format === "ssz" ? mimeTypeSSZ : "application/json"}, | ||||
}), | ||||
parseReq: ({params, headers}) => [params.block_id, headers.accept === mimeTypeSSZ ? "ssz" : "json"], | ||||
schema: {params: {block_id: Schema.StringRequired}}, | ||||
}; | ||||
|
||||
// Compute block type from JSON payload. See https://github.com/ethereum/eth2.0-APIs/pull/142 | ||||
const getSignedBeaconBlockType = (data: allForks.SignedBeaconBlock): allForks.AllForksSSZTypes["SignedBeaconBlock"] => | ||||
config.getForkTypes(data.message.slot).SignedBeaconBlock; | ||||
|
@@ -304,8 +324,8 @@ export function getReqSerializers(config: ChainForkConfig): ReqSerializers<Api, | |||
}; | ||||
|
||||
return { | ||||
getBlock: blockIdOnlyReq, | ||||
getBlockV2: blockIdOnlyReq, | ||||
getBlock: getBlockReq, | ||||
getBlockV2: getBlockReq, | ||||
getBlockAttestations: blockIdOnlyReq, | ||||
getBlockHeader: blockIdOnlyReq, | ||||
getBlockHeaders: { | ||||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -3,6 +3,7 @@ import {Resolves} from "./utils/types.js"; | |||
|
||||
/* eslint-disable @typescript-eslint/no-explicit-any */ | ||||
|
||||
export type ResponseFormat = "json" | "ssz"; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This type is also used by the debug state routes. Should we put this somewhere that both can pull from the same type?
|
||||
export type APIClientHandler = (...args: any) => PromiseLike<ApiClientResponse>; | ||||
export type APIServerHandler = (...args: any) => PromiseLike<unknown>; | ||||
|
||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -30,11 +30,11 @@ export const testData: GenericServerTestCases<Api> = { | |||||||||||||||||
// block | ||||||||||||||||||
|
||||||||||||||||||
getBlock: { | ||||||||||||||||||
args: ["head"], | ||||||||||||||||||
args: ["head", "json"], | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also check for ssz? Not sure if its needed. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these test data are keyed by api name so we can only test with either "json" or "ssz". This is "json" because by default we use it, and majority of consumers use "json" so I'd go with it to maintain the test case going forward There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't look like we unit test the ssz format for state either. I will be happy to add a unit test case after this gets merged. Will be good practice for me. I haven't worked with the API code much lodestar/packages/api/test/unit/beacon/testData/debug.ts Lines 47 to 54 in 97dfcfc
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is just testing the shape/types of the input and output data, not any behavior. Since we added a param to the getBlock functions, we need to add a param here. |
||||||||||||||||||
res: {data: ssz.phase0.SignedBeaconBlock.defaultValue()}, | ||||||||||||||||||
}, | ||||||||||||||||||
getBlockV2: { | ||||||||||||||||||
args: ["head"], | ||||||||||||||||||
args: ["head", "json"], | ||||||||||||||||||
res: {executionOptimistic: true, data: ssz.bellatrix.SignedBeaconBlock.defaultValue(), version: ForkName.bellatrix}, | ||||||||||||||||||
}, | ||||||||||||||||||
getBlockAttestations: { | ||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is sweet. I didnt know our httpClient did so much under the hood. was looking at how its implemented.