From ea4cdad801904773a050b4e238ba3683859aa35a Mon Sep 17 00:00:00 2001 From: Julien Date: Thu, 18 Jan 2024 14:29:16 -0800 Subject: [PATCH] fix: make light client events spec compliant (#6309) * fix: make eventstream spec compliant * fix: lints * fix: rely on version for light-client event serialization * fix: align lightClientUpdate event to existing pattern * fix: use top level fromJSON * fix: remove useless config * chore: cleanup unused event * Update test data to match spec examples * Determine fork based on attested header slot * Remove unrelated ssz types update --------- Co-authored-by: Nico Flaig --- packages/api/src/beacon/client/events.ts | 5 +- packages/api/src/beacon/client/index.ts | 2 +- packages/api/src/beacon/routes/events.ts | 61 +++++------------ packages/api/src/beacon/server/events.ts | 5 +- packages/api/src/beacon/server/index.ts | 2 +- .../beacon/genericServerTest/events.test.ts | 5 +- .../api/test/unit/beacon/oapiSpec.test.ts | 2 +- .../api/test/unit/beacon/testData/events.ts | 65 ++++++++++++++++--- .../src/chain/lightClient/index.ts | 17 +++-- packages/beacon-node/src/network/network.ts | 8 ++- packages/light-client/src/transport/rest.ts | 4 +- .../light-client/test/unit/sync.node.test.ts | 5 +- 12 files changed, 103 insertions(+), 78 deletions(-) diff --git a/packages/api/src/beacon/client/events.ts b/packages/api/src/beacon/client/events.ts index 4be5078735e4..1517d4bf3a56 100644 --- a/packages/api/src/beacon/client/events.ts +++ b/packages/api/src/beacon/client/events.ts @@ -1,4 +1,3 @@ -import {ChainForkConfig} from "@lodestar/config"; import {Api, BeaconEvent, routesData, getEventSerdes} from "../routes/events.js"; import {stringifyQuery, urlJoin} from "../../utils/client/format.js"; import {getEventSource} from "../../utils/client/eventSource.js"; @@ -7,8 +6,8 @@ import {HttpStatusCode} from "../../utils/client/httpStatusCode.js"; /** * REST HTTP client for events routes */ -export function getClient(config: ChainForkConfig, baseUrl: string): Api { - const eventSerdes = getEventSerdes(config); +export function getClient(baseUrl: string): Api { + const eventSerdes = getEventSerdes(); return { eventstream: async (topics, signal, onEvent) => { diff --git a/packages/api/src/beacon/client/index.ts b/packages/api/src/beacon/client/index.ts index 2dc0200dbda0..9fbe17bf337a 100644 --- a/packages/api/src/beacon/client/index.ts +++ b/packages/api/src/beacon/client/index.ts @@ -28,7 +28,7 @@ export function getClient(opts: HttpClientOptions, modules: ClientModules): Api beacon: beacon.getClient(config, httpClient), config: configApi.getClient(config, httpClient), debug: debug.getClient(config, httpClient), - events: events.getClient(config, httpClient.baseUrl), + events: events.getClient(httpClient.baseUrl), lightclient: lightclient.getClient(config, httpClient), lodestar: lodestar.getClient(config, httpClient), node: node.getClient(config, httpClient), diff --git a/packages/api/src/beacon/routes/events.ts b/packages/api/src/beacon/routes/events.ts index 105da89fced5..41a8e2e9cad9 100644 --- a/packages/api/src/beacon/routes/events.ts +++ b/packages/api/src/beacon/routes/events.ts @@ -1,7 +1,6 @@ import {ContainerType, ValueOf} from "@chainsafe/ssz"; import {Epoch, phase0, capella, Slot, ssz, StringType, RootHex, altair, UintNum64, allForks} from "@lodestar/types"; -import {ChainForkConfig} from "@lodestar/config"; -import {isForkExecution, ForkName} from "@lodestar/params"; +import {isForkExecution, ForkName, isForkLightClient} from "@lodestar/params"; import {RouteDef, TypeJson, WithVersion} from "../../utils/index.js"; import {HttpStatusCode} from "../../utils/client/httpStatusCode.js"; @@ -48,8 +47,6 @@ export enum EventType { lightClientOptimisticUpdate = "light_client_optimistic_update", /** New or better finality update available */ lightClientFinalityUpdate = "light_client_finality_update", - /** New or better light client update available */ - lightClientUpdate = "light_client_update", /** Payload attributes for block proposal */ payloadAttributes = "payload_attributes", /** The node has received a valid blobSidecar (from P2P or API) */ @@ -67,7 +64,6 @@ export const eventTypes: {[K in EventType]: K} = { [EventType.contributionAndProof]: EventType.contributionAndProof, [EventType.lightClientOptimisticUpdate]: EventType.lightClientOptimisticUpdate, [EventType.lightClientFinalityUpdate]: EventType.lightClientFinalityUpdate, - [EventType.lightClientUpdate]: EventType.lightClientUpdate, [EventType.payloadAttributes]: EventType.payloadAttributes, [EventType.blobSidecar]: EventType.blobSidecar, }; @@ -107,9 +103,8 @@ export type EventData = { executionOptimistic: boolean; }; [EventType.contributionAndProof]: altair.SignedContributionAndProof; - [EventType.lightClientOptimisticUpdate]: allForks.LightClientOptimisticUpdate; - [EventType.lightClientFinalityUpdate]: allForks.LightClientFinalityUpdate; - [EventType.lightClientUpdate]: allForks.LightClientUpdate; + [EventType.lightClientOptimisticUpdate]: {version: ForkName; data: allForks.LightClientOptimisticUpdate}; + [EventType.lightClientFinalityUpdate]: {version: ForkName; data: allForks.LightClientFinalityUpdate}; [EventType.payloadAttributes]: {version: ForkName; data: allForks.SSEPayloadAttributes}; [EventType.blobSidecar]: BlobSidecarSSE; }; @@ -146,9 +141,12 @@ export type ReqTypes = { // It doesn't make sense to define a getReqSerializers() here given the exotic argument of eventstream() // The request is very simple: (topics) => {query: {topics}}, and the test will ensure compatibility server - client -export function getTypeByEvent(config: ChainForkConfig): {[K in EventType]: TypeJson} { - const getLightClientTypeFromHeader = (data: allForks.LightClientHeader): allForks.AllForksLightClientSSZTypes => { - return config.getLightClientForkTypes(data.beacon.slot); +export function getTypeByEvent(): {[K in EventType]: TypeJson} { + const getLightClientType = (fork: ForkName): allForks.AllForksLightClientSSZTypes => { + if (!isForkLightClient(fork)) { + throw Error(`Invalid fork=${fork} for lightclient fork types`); + } + return ssz.allForksLightClient[fork]; }; return { @@ -208,45 +206,16 @@ export function getTypeByEvent(config: ChainForkConfig): {[K in EventType]: Type ), [EventType.blobSidecar]: blobSidecarSSE, - [EventType.lightClientOptimisticUpdate]: { - toJson: (data) => - getLightClientTypeFromHeader((data as unknown as allForks.LightClientOptimisticUpdate).attestedHeader)[ - "LightClientOptimisticUpdate" - ].toJson(data), - fromJson: (data) => - getLightClientTypeFromHeader( - // eslint-disable-next-line @typescript-eslint/naming-convention - (data as {attested_header: allForks.LightClientHeader}).attested_header - )["LightClientOptimisticUpdate"].fromJson(data), - }, - [EventType.lightClientFinalityUpdate]: { - toJson: (data) => - getLightClientTypeFromHeader((data as unknown as allForks.LightClientFinalityUpdate).attestedHeader)[ - "LightClientFinalityUpdate" - ].toJson(data), - fromJson: (data) => - getLightClientTypeFromHeader( - // eslint-disable-next-line @typescript-eslint/naming-convention - (data as {attested_header: allForks.LightClientHeader}).attested_header - )["LightClientFinalityUpdate"].fromJson(data), - }, - [EventType.lightClientUpdate]: { - toJson: (data) => - getLightClientTypeFromHeader((data as unknown as allForks.LightClientUpdate).attestedHeader)[ - "LightClientUpdate" - ].toJson(data), - fromJson: (data) => - getLightClientTypeFromHeader( - // eslint-disable-next-line @typescript-eslint/naming-convention - (data as {attested_header: allForks.LightClientHeader}).attested_header - )["LightClientUpdate"].fromJson(data), - }, + [EventType.lightClientOptimisticUpdate]: WithVersion( + (fork) => getLightClientType(fork).LightClientOptimisticUpdate + ), + [EventType.lightClientFinalityUpdate]: WithVersion((fork) => getLightClientType(fork).LightClientFinalityUpdate), }; } // eslint-disable-next-line @typescript-eslint/explicit-function-return-type -export function getEventSerdes(config: ChainForkConfig) { - const typeByEvent = getTypeByEvent(config); +export function getEventSerdes() { + const typeByEvent = getTypeByEvent(); return { toJson: (event: BeaconEvent): unknown => { diff --git a/packages/api/src/beacon/server/events.ts b/packages/api/src/beacon/server/events.ts index 834b6b67fca4..6d780ed1bf70 100644 --- a/packages/api/src/beacon/server/events.ts +++ b/packages/api/src/beacon/server/events.ts @@ -1,10 +1,9 @@ -import {ChainForkConfig} from "@lodestar/config"; import {Api, ReqTypes, routesData, getEventSerdes, eventTypes} from "../routes/events.js"; import {ApiError, ServerRoutes} from "../../utils/server/index.js"; import {ServerApi} from "../../interfaces.js"; -export function getRoutes(config: ChainForkConfig, api: ServerApi): ServerRoutes { - const eventSerdes = getEventSerdes(config); +export function getRoutes(api: ServerApi): ServerRoutes { + const eventSerdes = getEventSerdes(); return { // Non-JSON route. Server Sent Events (SSE) diff --git a/packages/api/src/beacon/server/index.ts b/packages/api/src/beacon/server/index.ts index b935a5432a3c..da77dfad32af 100644 --- a/packages/api/src/beacon/server/index.ts +++ b/packages/api/src/beacon/server/index.ts @@ -37,7 +37,7 @@ export function registerRoutes( beacon: () => beacon.getRoutes(config, api.beacon), config: () => configApi.getRoutes(config, api.config), debug: () => debug.getRoutes(config, api.debug), - events: () => events.getRoutes(config, api.events), + events: () => events.getRoutes(api.events), lightclient: () => lightclient.getRoutes(config, api.lightclient), lodestar: () => lodestar.getRoutes(config, api.lodestar), node: () => node.getRoutes(config, api.node), diff --git a/packages/api/test/unit/beacon/genericServerTest/events.test.ts b/packages/api/test/unit/beacon/genericServerTest/events.test.ts index 48ff8ad3d157..3e1c02396710 100644 --- a/packages/api/test/unit/beacon/genericServerTest/events.test.ts +++ b/packages/api/test/unit/beacon/genericServerTest/events.test.ts @@ -1,6 +1,5 @@ import {describe, it, expect, beforeEach, afterEach} from "vitest"; import {sleep} from "@lodestar/utils"; -import {config} from "@lodestar/config/default"; import {Api, routesData, EventType, BeaconEvent} from "../../../../src/beacon/routes/events.js"; import {getClient} from "../../../../src/beacon/client/events.js"; import {getRoutes} from "../../../../src/beacon/server/events.js"; @@ -11,7 +10,7 @@ import {eventTestData} from "../testData/events.js"; describe("beacon / events", () => { const {baseUrl, server} = getTestServer(); const mockApi = getMockApi(routesData); - for (const route of Object.values(getRoutes(config, mockApi))) { + for (const route of Object.values(getRoutes(mockApi))) { registerRoute(server, route); } @@ -53,7 +52,7 @@ describe("beacon / events", () => { }); // Capture them on the client - const client = getClient(config, baseUrl); + const client = getClient(baseUrl); void client.eventstream(topicsToRequest, controller.signal, (event) => { eventsReceived.push(event); if (eventsReceived.length >= eventsToSend.length) resolve(); diff --git a/packages/api/test/unit/beacon/oapiSpec.test.ts b/packages/api/test/unit/beacon/oapiSpec.test.ts index 1a300eba6f36..15a10bfeb6f7 100644 --- a/packages/api/test/unit/beacon/oapiSpec.test.ts +++ b/packages/api/test/unit/beacon/oapiSpec.test.ts @@ -204,7 +204,7 @@ describe("eventstream event data", () => { } }); - const eventSerdes = routes.events.getEventSerdes(config); + const eventSerdes = routes.events.getEventSerdes(); const knownTopics = new Set(Object.values(routes.events.eventTypes)); for (const [topic, {value}] of Object.entries(eventstreamExamples ?? {}).filter( diff --git a/packages/api/test/unit/beacon/testData/events.ts b/packages/api/test/unit/beacon/testData/events.ts index af33f4a2b011..7bfac9a59a88 100644 --- a/packages/api/test/unit/beacon/testData/events.ts +++ b/packages/api/test/unit/beacon/testData/events.ts @@ -4,7 +4,6 @@ import {Api, EventData, EventType, blobSidecarSSE} from "../../../../src/beacon/ import {GenericServerTestCases} from "../../../utils/genericServerTest.js"; const abortController = new AbortController(); -const root = new Uint8Array(32); /* eslint-disable @typescript-eslint/no-empty-function, @typescript-eslint/naming-convention */ @@ -93,18 +92,64 @@ export const eventTestData: EventData = { "0xac118511474a94f857300b315c50585c32a713e4452e26a6bb98cdb619936370f126ed3b6bb64469259ee92e69791d9e12d324ce6fd90081680ce72f39d85d50b0ff977260a8667465e613362c6d6e6e745e1f9323ec1d6f16041c4e358839ac", }), [EventType.lightClientOptimisticUpdate]: { - syncAggregate: ssz.altair.SyncAggregate.defaultValue(), - attestedHeader: ssz.altair.LightClientHeader.defaultValue(), - signatureSlot: ssz.Slot.defaultValue(), + version: ForkName.altair, + data: ssz.altair.LightClientOptimisticUpdate.fromJson({ + attested_header: { + beacon: { + slot: "1", + proposer_index: "1", + parent_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + state_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + body_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + }, + }, + sync_aggregate: { + sync_committee_bits: + "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffbfffffffffffffffffffffffbffffffffffffffffffffbffffffffffffff", + sync_committee_signature: + "0x1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505cc411d61252fb6cb3fa0017b679f8bb2305b26a285fa2737f175668d0dff91cc1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505", + }, + signature_slot: "1", + }), }, [EventType.lightClientFinalityUpdate]: { - attestedHeader: ssz.altair.LightClientHeader.defaultValue(), - finalizedHeader: ssz.altair.LightClientHeader.defaultValue(), - finalityBranch: [root], - syncAggregate: ssz.altair.SyncAggregate.defaultValue(), - signatureSlot: ssz.Slot.defaultValue(), + version: ForkName.altair, + data: ssz.altair.LightClientFinalityUpdate.fromJson({ + attested_header: { + beacon: { + slot: "1", + proposer_index: "1", + parent_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + state_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + body_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + }, + }, + finalized_header: { + beacon: { + slot: "1", + proposer_index: "1", + parent_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + state_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + body_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + }, + }, + finality_branch: [ + "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + ], + sync_aggregate: { + sync_committee_bits: + "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffbfffffffffffffffffffffffbffffffffffffffffffffbffffffffffffff", + sync_committee_signature: + "0x1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505cc411d61252fb6cb3fa0017b679f8bb2305b26a285fa2737f175668d0dff91cc1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505", + }, + signature_slot: "1", + }), }, - [EventType.lightClientUpdate]: ssz.altair.LightClientUpdate.defaultValue(), [EventType.payloadAttributes]: { version: ForkName.bellatrix, data: ssz.bellatrix.SSEPayloadAttributes.defaultValue(), diff --git a/packages/beacon-node/src/chain/lightClient/index.ts b/packages/beacon-node/src/chain/lightClient/index.ts index 1a09652dc233..75d094036f2d 100644 --- a/packages/beacon-node/src/chain/lightClient/index.ts +++ b/packages/beacon-node/src/chain/lightClient/index.ts @@ -494,9 +494,15 @@ export class LightClientServer { return; } + // Fork of LightClientOptimisticUpdate and LightClientFinalityUpdate is based off on attested header's fork + const attestedFork = this.config.getForkName(attestedHeader.beacon.slot); + // Emit update // Note: Always emit optimistic update even if we have emitted one with higher or equal attested_header.slot - this.emitter.emit(routes.events.EventType.lightClientOptimisticUpdate, headerUpdate); + this.emitter.emit(routes.events.EventType.lightClientOptimisticUpdate, { + version: attestedFork, + data: headerUpdate, + }); // Persist latest best update for getLatestHeadUpdate() // TODO: Once SyncAggregate are constructed from P2P too, count bits to decide "best" @@ -515,8 +521,6 @@ export class LightClientServer { finalizedHeader.beacon.slot > this.finalized.finalizedHeader.beacon.slot || syncAggregateParticipation > sumBits(this.finalized.syncAggregate.syncCommitteeBits)) ) { - // Fork of LightClientFinalityUpdate is based off on attested header's fork - const attestedFork = this.config.getForkName(attestedHeader.beacon.slot); if (this.config.getForkName(finalizedHeader.beacon.slot) !== attestedFork) { finalizedHeader = upgradeLightClientHeader(this.config, attestedFork, finalizedHeader); } @@ -529,8 +533,11 @@ export class LightClientServer { }; this.metrics?.lightclientServer.onSyncAggregate.inc({event: "update_latest_finalized_update"}); - // Note: Ignores gossip rule to always emit finaly_update with higher finalized_header.slot, for simplicity - this.emitter.emit(routes.events.EventType.lightClientFinalityUpdate, this.finalized); + // Note: Ignores gossip rule to always emit finality_update with higher finalized_header.slot, for simplicity + this.emitter.emit(routes.events.EventType.lightClientFinalityUpdate, { + version: attestedFork, + data: this.finalized, + }); } } diff --git a/packages/beacon-node/src/network/network.ts b/packages/beacon-node/src/network/network.ts index 609ee2d44ba1..f4b57fe0c658 100644 --- a/packages/beacon-node/src/network/network.ts +++ b/packages/beacon-node/src/network/network.ts @@ -105,8 +105,12 @@ export class Network implements INetwork { this.events.on(NetworkEvent.peerConnected, this.onPeerConnected); this.events.on(NetworkEvent.peerDisconnected, this.onPeerDisconnected); this.chain.emitter.on(routes.events.EventType.head, this.onHead); - this.chain.emitter.on(routes.events.EventType.lightClientFinalityUpdate, this.onLightClientFinalityUpdate); - this.chain.emitter.on(routes.events.EventType.lightClientOptimisticUpdate, this.onLightClientOptimisticUpdate); + this.chain.emitter.on(routes.events.EventType.lightClientFinalityUpdate, ({data}) => + this.onLightClientFinalityUpdate(data) + ); + this.chain.emitter.on(routes.events.EventType.lightClientOptimisticUpdate, ({data}) => + this.onLightClientOptimisticUpdate(data) + ); } static async init({ diff --git a/packages/light-client/src/transport/rest.ts b/packages/light-client/src/transport/rest.ts index 765e55d7f5c5..ebbd8e52f691 100644 --- a/packages/light-client/src/transport/rest.ts +++ b/packages/light-client/src/transport/rest.ts @@ -80,11 +80,11 @@ export class LightClientRestTransport extends (EventEmitter as {new (): RestEven (event) => { switch (event.type) { case routes.events.EventType.lightClientOptimisticUpdate: - this.eventEmitter.emit(routes.events.EventType.lightClientOptimisticUpdate, event.message); + this.eventEmitter.emit(routes.events.EventType.lightClientOptimisticUpdate, event.message.data); break; case routes.events.EventType.lightClientFinalityUpdate: - this.eventEmitter.emit(routes.events.EventType.lightClientFinalityUpdate, event.message); + this.eventEmitter.emit(routes.events.EventType.lightClientFinalityUpdate, event.message.data); break; } } diff --git a/packages/light-client/test/unit/sync.node.test.ts b/packages/light-client/test/unit/sync.node.test.ts index 168bfeceb5f9..75073c80070b 100644 --- a/packages/light-client/test/unit/sync.node.test.ts +++ b/packages/light-client/test/unit/sync.node.test.ts @@ -155,7 +155,10 @@ describe("sync", () => { }; lightclientServerApi.latestHeadUpdate = headUpdate; - eventsServerApi.emit({type: routes.events.EventType.lightClientOptimisticUpdate, message: headUpdate}); + eventsServerApi.emit({ + type: routes.events.EventType.lightClientOptimisticUpdate, + message: {version: config.getForkName(headUpdate.attestedHeader.beacon.slot), data: headUpdate}, + }); testLogger.debug("Emitted EventType.lightClientOptimisticUpdate", {slot}); } });