Skip to content
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.

Sign up for GitHub

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

wrap kzgs/proofs/blobs fields as BlobsBundle #5562

Merged
merged 2 commits into from
Nov 4, 2023
Merged

wrap kzgs/proofs/blobs fields as BlobsBundle #5562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5ae307
wrap `kzgs`/`proofs`/`blobs` fields as `BlobsBundle`
etan-status Nov 4, 2023
9c950a2
consistent naming
etan-status Nov 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions beacon_chain/el/el_manager.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
33 changes: 10 additions & 23 deletions beacon_chain/rpc/rest_validator_api.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,29 +392,21 @@ 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())
res.get
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)
Expand All @@ -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)
Expand All @@ -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"

Expand Down
17 changes: 8 additions & 9 deletions beacon_chain/spec/datatypes/deneb.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const

type
KzgCommitments* = List[KzgCommitment, Limit MAX_BLOB_COMMITMENTS_PER_BLOCK]
KzgProofs* = List[KzgProof, Limit MAX_BLOB_COMMITMENTS_PER_BLOCK]
tersec marked this conversation as resolved.
Show resolved Hide resolved
Blobs* = List[Blob, Limit MAX_BLOB_COMMITMENTS_PER_BLOCK]

# TODO this apparently is suppposed to be SSZ-equivalent to Bytes32, but
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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: [].}

Expand Down
2 changes: 1 addition & 1 deletion beacon_chain/spec/state_transition.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 2 additions & 3 deletions beacon_chain/sync/request_manager.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand Down Expand Up @@ -356,4 +356,3 @@ proc stop*(rman: RequestManager) =
rman.blockLoopFuture.cancelSoon()
if not(isNil(rman.blobLoopFuture)):
rman.blobLoopFuture.cancelSoon()

22 changes: 6 additions & 16 deletions beacon_chain/validators/beacon_validators.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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..<blobs.len:
var sidecar = BlobSidecar(
block_root: blockRoot,
Expand All @@ -1188,18 +1183,13 @@ proc proposeBlockAux(
block_parent_root: forkyBlck.parent_root,
proposer_index: forkyBlck.proposer_index,
blob: blobs[i],
kzg_commitment: kzgs[i],
kzg_commitment: commitments[i],
kzg_proof: proofs[i]
)
sidecars.add(sidecar)
Opt.some(sidecars)
elif forkyBlck is phase0.BeaconBlock or
forkyBlck is altair.BeaconBlock or
forkyBlck is bellatrix.BeaconBlock or
forkyBlck is capella.BeaconBlock:
Opt.none(seq[BlobSidecar])
else:
static: doAssert "Unknown BeaconBlock type"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching static: doAssert to Opt.none means that it won't trigger automatically anymore when the E-fork is added, but will incorrectly and silently use the pre-Deneb/sidecar version.

Better to find out about missing functionality at compile-time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will take the Deneb version, not the none version (consensusFork >= ConsensusFork.Deneb), and will compile-time fail if no longer compatible

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other one will compile-time break on Electra because DenebBlockContents requires a Deneb block, while the generated code will try to put an Electra block into it.

This instance here will be fine with Electra as well (using the blob logic from deneb)

Opt.none(seq[BlobSidecar])

if notSlashable.isErr:
warn "Slashing protection activated for block proposal",
Expand Down
Loading