From d5ae30715abb3a760d8130decaa0f626aed05cd8 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Sat, 4 Nov 2023 11:14:55 +0100 Subject: [PATCH 1/2] wrap `kzgs`/`proofs`/`blobs` fields as `BlobsBundle` Less type conversion / copying by keeping the `BlobsBundle` together. --- beacon_chain/el/el_manager.nim | 11 ++++--- beacon_chain/rpc/rest_validator_api.nim | 33 ++++++------------- beacon_chain/spec/datatypes/deneb.nim | 17 +++++----- beacon_chain/spec/state_transition.nim | 2 +- beacon_chain/validators/beacon_validators.nim | 22 ++++--------- 5 files changed, 32 insertions(+), 53 deletions(-) diff --git a/beacon_chain/el/el_manager.nim b/beacon_chain/el/el_manager.nim index e9334594f7..271afc47b0 100644 --- a/beacon_chain/el/el_manager.nim +++ b/beacon_chain/el/el_manager.nim @@ -551,10 +551,13 @@ func asConsensusType*(payload: engine_api.GetPayloadV3Response): # The `mapIt` calls below are necessary only because we use different distinct # types for KZG commitments and Blobs in the `web3` and the `deneb` spec types. # Both are defined as `array[N, byte]` under the hood. - kzgs: KzgCommitments payload.blobsBundle.commitments.mapIt(it.bytes), - proofs: payload.blobsBundle.proofs.mapIt(it.bytes), - blobs: Blobs payload.blobsBundle.blobs.mapIt(it.bytes) - ) + blobsBundle: BlobsBundle( + commitments: KzgCommitments.init( + payload.blobsBundle.commitments.mapIt(it.bytes)), + proofs: KzgProofs.init( + payload.blobsBundle.proofs.mapIt(it.bytes)), + blobs: Blobs.init( + payload.blobsBundle.blobs.mapIt(it.bytes)))) func asEngineExecutionPayload*(executionPayload: bellatrix.ExecutionPayload): ExecutionPayloadV1 = diff --git a/beacon_chain/rpc/rest_validator_api.nim b/beacon_chain/rpc/rest_validator_api.nim index 4312ff0410..c016f899f7 100644 --- a/beacon_chain/rpc/rest_validator_api.nim +++ b/beacon_chain/rpc/rest_validator_api.nim @@ -392,21 +392,13 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = qslot, proposer, qrandao, qskip_randao_verification): return RestApiResponse.jsonError(Http400, InvalidRandaoRevealValue) - let res = - case node.dag.cfg.consensusForkAtEpoch(qslot.epoch) - of ConsensusFork.Deneb: - await makeBeaconBlockForHeadAndSlot( - deneb.ExecutionPayloadForSigning, - node, qrandao, proposer, qgraffiti, qhead, qslot) - of ConsensusFork.Capella: - await makeBeaconBlockForHeadAndSlot( - capella.ExecutionPayloadForSigning, - node, qrandao, proposer, qgraffiti, qhead, qslot) - of ConsensusFork.Bellatrix: + let res = withConsensusFork( + node.dag.cfg.consensusForkAtEpoch(qslot.epoch)): + when consensusFork >= ConsensusFork.Bellatrix: await makeBeaconBlockForHeadAndSlot( - bellatrix.ExecutionPayloadForSigning, + consensusFork.ExecutionPayloadForSigning, node, qrandao, proposer, qgraffiti, qhead, qslot) - of ConsensusFork.Altair, ConsensusFork.Phase0: + else: return RestApiResponse.jsonError(Http400, InvalidSlotValueError) if res.isErr(): return RestApiResponse.jsonError(Http400, res.error()) @@ -414,7 +406,7 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = return withBlck(message.blck): let data = - when forkyBlck is deneb.BeaconBlock: + when consensusFork >= ConsensusFork.Deneb: let bundle = message.blobsBundleOpt.get() let blockRoot = hash_tree_root(forkyBlck) var sidecars = newSeqOfCap[BlobSidecar](bundle.blobs.len) @@ -426,7 +418,7 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = block_parent_root: forkyBlck.parent_root, proposer_index: forkyBlck.proposer_index, blob: bundle.blobs[i], - kzg_commitment: bundle.kzgs[i], + kzg_commitment: bundle.commitments[i], kzg_proof: bundle.proofs[i] ) sidecars.add(sidecar) @@ -435,18 +427,13 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = `block`: forkyBlck, blob_sidecars: List[BlobSidecar, Limit MAX_BLOBS_PER_BLOCK].init(sidecars)) - elif forkyBlck is phase0.BeaconBlock or - forkyBlck is altair.BeaconBlock or - forkyBlck is bellatrix.BeaconBlock or - forkyBlck is capella.BeaconBlock: - forkyBlck else: - static: raiseAssert "produceBlockV2 received unexpected version" + forkyBlck if contentType == sszMediaType: - let headers = [("eth-consensus-version", message.blck.kind.toString())] + let headers = [("eth-consensus-version", consensusFork.toString())] RestApiResponse.sszResponse(data, headers) elif contentType == jsonMediaType: - RestApiResponse.jsonResponseWVersion(data, message.blck.kind) + RestApiResponse.jsonResponseWVersion(data, consensusFork) else: raiseAssert "preferredContentType() returns invalid content type" diff --git a/beacon_chain/spec/datatypes/deneb.nim b/beacon_chain/spec/datatypes/deneb.nim index f58a8accec..e5e2cafc3f 100644 --- a/beacon_chain/spec/datatypes/deneb.nim +++ b/beacon_chain/spec/datatypes/deneb.nim @@ -38,6 +38,7 @@ const type KzgCommitments* = List[KzgCommitment, Limit MAX_BLOB_COMMITMENTS_PER_BLOCK] + KzgProofs* = List[KzgProof, Limit MAX_BLOB_COMMITMENTS_PER_BLOCK] Blobs* = List[Blob, Limit MAX_BLOB_COMMITMENTS_PER_BLOCK] # TODO this apparently is suppposed to be SSZ-equivalent to Bytes32, but @@ -50,6 +51,12 @@ type # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.2/specs/deneb/polynomial-commitments.md#custom-types Blob* = array[BYTES_PER_FIELD_ELEMENT * FIELD_ELEMENTS_PER_BLOB, byte] + # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/deneb/validator.md#blobsbundle + BlobsBundle* = object + commitments*: KzgCommitments + proofs*: KzgProofs + blobs*: Blobs + # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.3/specs/deneb/p2p-interface.md#blobsidecar BlobSidecar* = object block_root*: Eth2Digest @@ -103,9 +110,7 @@ type ExecutionPayloadForSigning* = object executionPayload*: ExecutionPayload blockValue*: Wei - kzgs*: KzgCommitments - proofs*: seq[KZGProof] - blobs*: Blobs + blobsBundle*: BlobsBundle # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.3/specs/deneb/beacon-chain.md#executionpayloadheader ExecutionPayloadHeader* = object @@ -131,12 +136,6 @@ type blob_gas_used*: uint64 # [New in Deneb:EIP4844] excess_blob_gas*: uint64 # [New in Deneb:EIP4844] - # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.3/specs/deneb/validator.md#blobsbundle - BlobsBundle* = object - commitments*: seq[KZGCommitment] - proofs*: seq[KZGProof] - blobs*: seq[Blob] - ExecutePayload* = proc( execution_payload: ExecutionPayload): bool {.gcsafe, raises: [].} diff --git a/beacon_chain/spec/state_transition.nim b/beacon_chain/spec/state_transition.nim index 6b8cb9ded6..3e77163632 100644 --- a/beacon_chain/spec/state_transition.nim +++ b/beacon_chain/spec/state_transition.nim @@ -380,7 +380,7 @@ func partialBeaconBlock*( # https://github.com/ethereum/consensus-specs/blob/v1.3.0/specs/deneb/validator.md#constructing-the-beaconblockbody when consensusFork >= ConsensusFork.Deneb: - res.body.blob_kzg_commitments = execution_payload.kzgs + res.body.blob_kzg_commitments = execution_payload.blobsBundle.commitments res diff --git a/beacon_chain/validators/beacon_validators.nim b/beacon_chain/validators/beacon_validators.nim index 7face4f0d2..ee27a0027c 100644 --- a/beacon_chain/validators/beacon_validators.nim +++ b/beacon_chain/validators/beacon_validators.nim @@ -88,9 +88,6 @@ declarePublicGauge(attached_validator_balance_total, logScope: topics = "beacval" type - BlobsBundle = tuple[blobs: deneb.Blobs, - kzgs: KzgCommitments, - proofs: seq[kzg_abi.KZGProof]] ForkedBlockResult = Result[tuple[blck: ForkedBeaconBlock, blockValue: Wei, @@ -541,10 +538,7 @@ proc makeBeaconBlockForHeadAndSlot*( var blobsBundleOpt = Opt.none(BlobsBundle) when payload is deneb.ExecutionPayloadForSigning: - let bb: BlobsBundle = (blobs: payload.blobs, - kzgs: payload.kzgs, - proofs: payload.proofs) - blobsBundleOpt = Opt.some(bb) + blobsBundleOpt = Opt.some(payload.blobsBundle) return if blck.isOk: ok((blck.get, payload.blockValue, blobsBundleOpt)) else: @@ -1176,10 +1170,11 @@ proc proposeBlockAux( .registerBlock(validator_index, validator.pubkey, slot, signingRoot) let blobSidecarsOpt = - when forkyBlck is deneb.BeaconBlock: + when consensusFork >= ConsensusFork.Deneb: var sidecars: seq[BlobSidecar] let bundle = collectedBids.engineBlockFut.read.get().blobsBundleOpt.get - let (blobs, kzgs, proofs) = (bundle.blobs, bundle.kzgs, bundle.proofs) + let (blobs, commitments, proofs) = ( + bundle.blobs, bundle.commitments, bundle.proofs) for i in 0.. Date: Sat, 4 Nov 2023 11:42:48 +0100 Subject: [PATCH 2/2] consistent naming --- beacon_chain/sync/request_manager.nim | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/beacon_chain/sync/request_manager.nim b/beacon_chain/sync/request_manager.nim index 8fe3b3ff57..d37a13d504 100644 --- a/beacon_chain/sync/request_manager.nim +++ b/beacon_chain/sync/request_manager.nim @@ -292,7 +292,7 @@ proc getMissingBlobs(rman: RequestManager): seq[BlobIdentifier] = warn "quarantine missing blobs, but missing indices is empty", blk=blobless.root, indices=rman.blobQuarantine[].blobIndices(blobless.root), - kzgs=len(blobless.message.body.blob_kzg_commitments) + commitments=len(blobless.message.body.blob_kzg_commitments) for idx in missing.indices: let id = BlobIdentifier(block_root: blobless.root, index: idx) if id notin fetches: @@ -302,7 +302,7 @@ proc getMissingBlobs(rman: RequestManager): seq[BlobIdentifier] = warn "missing blob handler found blobless block with all blobs", blk=blobless.root, indices=rman.blobQuarantine[].blobIndices(blobless.root), - kzgs=len(blobless.message.body.blob_kzg_commitments) + commitments=len(blobless.message.body.blob_kzg_commitments) discard rman.blockVerifier(ForkedSignedBeaconBlock.init(blobless), false) rman.quarantine[].removeBlobless(blobless) @@ -356,4 +356,3 @@ proc stop*(rman: RequestManager) = rman.blockLoopFuture.cancelSoon() if not(isNil(rman.blobLoopFuture)): rman.blobLoopFuture.cancelSoon() -