From 5f4fa9ae69877fcbb154a29b23e8cb70acdf3c7c Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 3 Apr 2024 02:05:29 +0200 Subject: [PATCH] avoid code repetition across forks for signed block contents (#6150) Use forks sugar to make `RestPublishedSignedBlockContents` more concise. --- beacon_chain/rpc/rest_beacon_api.nim | 131 ++++------------- .../eth2_apis/eth2_rest_serialization.nim | 137 +++++------------- beacon_chain/spec/eth2_apis/rest_types.nim | 26 ++++ 3 files changed, 92 insertions(+), 202 deletions(-) diff --git a/beacon_chain/rpc/rest_beacon_api.nim b/beacon_chain/rpc/rest_beacon_api.nim index 5774bdf41..0a24dad32 100644 --- a/beacon_chain/rpc/rest_beacon_api.nim +++ b/beacon_chain/rpc/rest_beacon_api.nim @@ -129,21 +129,6 @@ proc toString*(kind: ValidatorFilterKind): string = of ValidatorFilterKind.WithdrawalDone: "withdrawal_done" -func checkRestBlockBlobsValid( - forkyBlck: deneb.SignedBeaconBlock | electra.SignedBeaconBlock, - kzg_proofs: KzgProofs, - blobs: Blobs): Result[void, string] = - if kzg_proofs.len != blobs.len: - return err("Invalid block publish: " & $kzg_proofs.len & " KZG proofs and " & - $blobs.len & " blobs") - - if kzg_proofs.len != forkyBlck.message.body.blob_kzg_commitments.len: - return err("Invalid block publish: " & $kzg_proofs.len & - " KZG proofs and " & $forkyBlck.message.body.blob_kzg_commitments.len & - " KZG commitments") - - ok() - proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = # https://github.com/ethereum/EIPs/blob/master/EIPS/eip-4881.md router.api2(MethodGet, "/eth/v1/beacon/deposit_snapshot") do ( @@ -900,50 +885,23 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = let body = contentBody.get() version = request.headers.getString("eth-consensus-version") - var - restBlock = decodeBody(RestPublishedSignedBlockContents, body, - version).valueOr: - return RestApiResponse.jsonError(error) - forked = ForkedSignedBeaconBlock.init(restBlock) + restBlock = decodeBody( + RestPublishedSignedBlockContents, body, version).valueOr: + return RestApiResponse.jsonError(error) - if restBlock.kind != node.dag.cfg.consensusForkAtEpoch( - getForkedBlockField(forked, slot).epoch): - doAssert strictVerification notin node.dag.updateFlags - return RestApiResponse.jsonError(Http400, InvalidBlockObjectError) + withForkyBlck(restBlock): + if restBlock.kind != node.dag.cfg.consensusForkAtEpoch( + forkyBlck.message.slot.epoch): + doAssert strictVerification notin node.dag.updateFlags + return RestApiResponse.jsonError(Http400, InvalidBlockObjectError) - case restBlock.kind - of ConsensusFork.Phase0: - var blck = restBlock.phase0Data - blck.root = hash_tree_root(blck.message) - await node.router.routeSignedBeaconBlock( - blck, Opt.none(seq[BlobSidecar])) - of ConsensusFork.Altair: - var blck = restBlock.altairData - blck.root = hash_tree_root(blck.message) - await node.router.routeSignedBeaconBlock( - blck, Opt.none(seq[BlobSidecar])) - of ConsensusFork.Bellatrix: - var blck = restBlock.bellatrixData - blck.root = hash_tree_root(blck.message) - await node.router.routeSignedBeaconBlock( - blck, Opt.none(seq[BlobSidecar])) - of ConsensusFork.Capella: - var blck = restBlock.capellaData - blck.root = hash_tree_root(blck.message) - await node.router.routeSignedBeaconBlock( - blck, Opt.none(seq[BlobSidecar])) - of ConsensusFork.Deneb: - var blck = restBlock.denebData.signed_block - blck.root = hash_tree_root(blck.message) - - let validity = checkRestBlockBlobsValid( - blck, restBlock.denebData.kzg_proofs, restBlock.denebData.blobs) - if validity.isErr: - return RestApiResponse.jsonError(Http400, validity.error) - - await node.router.routeSignedBeaconBlock( - blck, Opt.some(blck.create_blob_sidecars( - restBlock.denebData.kzg_proofs, restBlock.denebData.blobs))) + when consensusFork >= ConsensusFork.Deneb: + await node.router.routeSignedBeaconBlock( + forkyBlck, Opt.some( + forkyBlck.create_blob_sidecars(kzg_proofs, blobs))) + else: + await node.router.routeSignedBeaconBlock( + forkyBlck, Opt.none(seq[BlobSidecar])) if res.isErr(): return RestApiResponse.jsonError( @@ -981,51 +939,24 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = if contentBody.isNone(): return RestApiResponse.jsonError(Http400, EmptyRequestBodyError) contentBody.get() - var - restBlock = decodeBodyJsonOrSsz(RestPublishedSignedBlockContents, - body, version).valueOr: - return RestApiResponse.jsonError(error) - forked = ForkedSignedBeaconBlock.init(restBlock) + restBlock = decodeBodyJsonOrSsz( + RestPublishedSignedBlockContents, body, version).valueOr: + return RestApiResponse.jsonError(error) - # TODO (henridf): handle broadcast_validation flag - if restBlock.kind != node.dag.cfg.consensusForkAtEpoch( - getForkedBlockField(forked, slot).epoch): - doAssert strictVerification notin node.dag.updateFlags - return RestApiResponse.jsonError(Http400, InvalidBlockObjectError) + withForkyBlck(restBlock): + # TODO (henridf): handle broadcast_validation flag + if restBlock.kind != node.dag.cfg.consensusForkAtEpoch( + forkyBlck.message.slot.epoch): + doAssert strictVerification notin node.dag.updateFlags + return RestApiResponse.jsonError(Http400, InvalidBlockObjectError) - case restBlock.kind - of ConsensusFork.Phase0: - var blck = restBlock.phase0Data - blck.root = hash_tree_root(blck.message) - await node.router.routeSignedBeaconBlock( - blck, Opt.none(seq[BlobSidecar])) - of ConsensusFork.Altair: - var blck = restBlock.altairData - blck.root = hash_tree_root(blck.message) - await node.router.routeSignedBeaconBlock( - blck, Opt.none(seq[BlobSidecar])) - of ConsensusFork.Bellatrix: - var blck = restBlock.bellatrixData - blck.root = hash_tree_root(blck.message) - await node.router.routeSignedBeaconBlock( - blck, Opt.none(seq[BlobSidecar])) - of ConsensusFork.Capella: - var blck = restBlock.capellaData - blck.root = hash_tree_root(blck.message) - await node.router.routeSignedBeaconBlock( - blck, Opt.none(seq[BlobSidecar])) - of ConsensusFork.Deneb: - var blck = restBlock.denebData.signed_block - blck.root = hash_tree_root(blck.message) - - let validity = checkRestBlockBlobsValid( - blck, restBlock.denebData.kzg_proofs, restBlock.denebData.blobs) - if validity.isErr: - return RestApiResponse.jsonError(Http400, validity.error) - - await node.router.routeSignedBeaconBlock( - blck, Opt.some(blck.create_blob_sidecars( - restBlock.denebData.kzg_proofs, restBlock.denebData.blobs))) + when consensusFork >= ConsensusFork.Deneb: + await node.router.routeSignedBeaconBlock( + forkyBlck, Opt.some( + forkyBlck.create_blob_sidecars(kzg_proofs, blobs))) + else: + await node.router.routeSignedBeaconBlock( + forkyBlck, Opt.none(seq[BlobSidecar])) if res.isErr(): return RestApiResponse.jsonError( diff --git a/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim b/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim index ebd87f5cc..2962bdb07 100644 --- a/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim +++ b/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim @@ -2020,44 +2020,8 @@ proc readValue*(reader: var JsonReader[RestJson], reader.raiseUnexpectedValue("Field `message` is missing") let blck = ForkedBeaconBlock(message.get()) - value = RestPublishedSignedBeaconBlock( - case blck.kind - of ConsensusFork.Phase0: - ForkedSignedBeaconBlock.init( - phase0.SignedBeaconBlock( - message: blck.phase0Data, - signature: signature.get() - ) - ) - of ConsensusFork.Altair: - ForkedSignedBeaconBlock.init( - altair.SignedBeaconBlock( - message: blck.altairData, - signature: signature.get() - ) - ) - of ConsensusFork.Bellatrix: - ForkedSignedBeaconBlock.init( - bellatrix.SignedBeaconBlock( - message: blck.bellatrixData, - signature: signature.get() - ) - ) - of ConsensusFork.Capella: - ForkedSignedBeaconBlock.init( - capella.SignedBeaconBlock( - message: blck.capellaData, - signature: signature.get() - ) - ) - of ConsensusFork.Deneb: - ForkedSignedBeaconBlock.init( - deneb.SignedBeaconBlock( - message: blck.denebData, - signature: signature.get() - ) - ) - ) + value = RestPublishedSignedBeaconBlock ForkedSignedBeaconBlock.init( + blck, blck.hash_tree_root(), signature.get()) proc readValue*(reader: var JsonReader[RestJson], value: var RestPublishedSignedBlockContents) {. @@ -2104,17 +2068,6 @@ proc readValue*(reader: var JsonReader[RestJson], Opt.none(RestPublishedSignedBeaconBlock) if signed_message.isNone(): reader.raiseUnexpectedValue("Incorrect signed_block format") - - # Only needed to signal fork to the blck.kind case selection - let blck = ForkedSignedBeaconBlock(signed_message.get()) - message = Opt.some(RestPublishedBeaconBlock( - case blck.kind - of ConsensusFork.Phase0, ConsensusFork.Altair, ConsensusFork.Bellatrix, - ConsensusFork.Capella: - reader.raiseUnexpectedValue("Incorrect signed_block format") - of ConsensusFork.Deneb: - ForkedBeaconBlock.init(blck.denebData.message) - )) of "kzg_proofs": if kzg_proofs.isSome(): reader.raiseUnexpectedField( @@ -2138,69 +2091,49 @@ proc readValue*(reader: var JsonReader[RestJson], else: unrecognizedFieldWarning() - if signed_message.isNone(): - # Pre-Deneb; conditions for when signed_message.isSome checked in case body - if signature.isNone(): - reader.raiseUnexpectedValue("Field `signature` is missing") - if message.isNone(): - reader.raiseUnexpectedValue("Field `message` is missing") - - let blck = ForkedBeaconBlock(message.get()) - - if blck.kind >= ConsensusFork.Deneb: + if signed_message.isSome(): + if message.isSome(): + reader.raiseUnexpectedValue("Field `message` found but unsupported") + if signature.isSome(): + reader.raiseUnexpectedValue("Field `signature` found but unsupported") if kzg_proofs.isNone(): reader.raiseUnexpectedValue("Field `kzg_proofs` is missing") if blobs.isNone(): reader.raiseUnexpectedValue("Field `blobs` is missing") + if kzg_proofs.get.len != blobs.get.len: + reader.raiseUnexpectedValue("Length mismatch of `kzg_proofs` and `blobs`") + + withBlck(distinctBase(signed_message.get)): + when consensusFork >= ConsensusFork.Deneb: + template kzg_commitments: untyped = + forkyBlck.message.body.blob_kzg_commitments + if kzg_proofs.get().len != kzg_commitments.len: + reader.raiseUnexpectedValue( + "Length mismatch of `kzg_proofs` and `blob_kzg_commitments`") + value = RestPublishedSignedBlockContents.init( + consensusFork.BlockContents( + `block`: forkyBlck.message, + kzg_proofs: kzg_proofs.get(), + blobs: blobs.get()), + forkyBlck.root, forkyBlck.signature) + else: + reader.raiseUnexpectedValue("`signed_message` supported post-Deneb") else: + if signature.isNone(): + reader.raiseUnexpectedValue("Field `signature` is missing") + if message.isNone(): + reader.raiseUnexpectedValue("Field `message` is missing") if kzg_proofs.isSome(): reader.raiseUnexpectedValue("Field `kzg_proofs` found but unsupported") if blobs.isSome(): reader.raiseUnexpectedValue("Field `blobs` found but unsupported") - case blck.kind - of ConsensusFork.Phase0: - value = RestPublishedSignedBlockContents( - kind: ConsensusFork.Phase0, - phase0Data: phase0.SignedBeaconBlock( - message: blck.phase0Data, - signature: signature.get() - ) - ) - of ConsensusFork.Altair: - value = RestPublishedSignedBlockContents( - kind: ConsensusFork.Altair, - altairData: altair.SignedBeaconBlock( - message: blck.altairData, - signature: signature.get() - ) - ) - of ConsensusFork.Bellatrix: - value = RestPublishedSignedBlockContents( - kind: ConsensusFork.Bellatrix, - bellatrixData: bellatrix.SignedBeaconBlock( - message: blck.bellatrixData, - signature: signature.get() - ) - ) - of ConsensusFork.Capella: - value = RestPublishedSignedBlockContents( - kind: ConsensusFork.Capella, - capellaData: capella.SignedBeaconBlock( - message: blck.capellaData, - signature: signature.get() - ) - ) - of ConsensusFork.Deneb: - value = RestPublishedSignedBlockContents( - kind: ConsensusFork.Deneb, - denebData: DenebSignedBlockContents( - # Constructed to be internally consistent - signed_block: signed_message.get().distinctBase.denebData, - kzg_proofs: kzg_proofs.get(), - blobs: blobs.get() - ) - ) + withBlck(distinctBase(message.get)): + when consensusFork < ConsensusFork.Deneb: + value = RestPublishedSignedBlockContents.init( + forkyBlck, forkyBlck.hash_tree_root(), signature.get) + else: + reader.raiseUnexpectedValue("`message` support stopped at Deneb") ## ForkedSignedBeaconBlock proc readValue*(reader: var JsonReader[RestJson], diff --git a/beacon_chain/spec/eth2_apis/rest_types.nim b/beacon_chain/spec/eth2_apis/rest_types.nim index 7f7115842..7951d1351 100644 --- a/beacon_chain/spec/eth2_apis/rest_types.nim +++ b/beacon_chain/spec/eth2_apis/rest_types.nim @@ -612,6 +612,32 @@ type func `==`*(a, b: RestValidatorIndex): bool = uint64(a) == uint64(b) +template withForkyBlck*( + x: RestPublishedSignedBlockContents, body: untyped): untyped = + case x.kind + of ConsensusFork.Deneb: + const consensusFork {.inject, used.} = ConsensusFork.Deneb + template forkyBlck: untyped {.inject, used.} = x.denebData.signed_block + template kzg_proofs: untyped {.inject, used.} = x.denebData.kzg_proofs + template blobs: untyped {.inject, used.} = x.denebData.blobs + body + of ConsensusFork.Capella: + const consensusFork {.inject, used.} = ConsensusFork.Capella + template forkyBlck: untyped {.inject, used.} = x.capellaData + body + of ConsensusFork.Bellatrix: + const consensusFork {.inject, used.} = ConsensusFork.Bellatrix + template forkyBlck: untyped {.inject, used.} = x.bellatrixData + body + of ConsensusFork.Altair: + const consensusFork {.inject, used.} = ConsensusFork.Altair + template forkyBlck: untyped {.inject, used.} = x.altairData + body + of ConsensusFork.Phase0: + const consensusFork {.inject, used.} = ConsensusFork.Phase0 + template forkyBlck: untyped {.inject, used.} = x.phase0Data + body + func init*(T: type ForkedSignedBeaconBlock, contents: RestPublishedSignedBlockContents): T = return