From ed372949699fc15a766ac35d86a5c4197393ea3e Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 00:29:19 +0100 Subject: [PATCH 01/13] bolster `BlobSidecar` syncing on incomplete responses Avoid marking blocks invalid when corresponding `blobSidecarsByRange` returns an incomplete / incorrect response while syncing. The block itself may still be valid in that scenario. --- .../gossip_processing/gossip_validation.nim | 13 ++--- beacon_chain/spec/datatypes/deneb.nim | 14 ++++++ beacon_chain/sync/sync_manager.nim | 48 +++++++++++-------- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/beacon_chain/gossip_processing/gossip_validation.nim b/beacon_chain/gossip_processing/gossip_validation.nim index 9faad0fc59..86710c2196 100644 --- a/beacon_chain/gossip_processing/gossip_validation.nim +++ b/beacon_chain/gossip_processing/gossip_validation.nim @@ -179,16 +179,9 @@ func check_attestation_subnet( ok() -# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/deneb/p2p-interface.md#verify_blob_sidecar_inclusion_proof -func verify_blob_sidecar_inclusion_proof( +func check_blob_sidecar_inclusion_proof( blob_sidecar: deneb.BlobSidecar): Result[void, ValidationError] = - let gindex = kzg_commitment_inclusion_proof_gindex(blob_sidecar.index) - if not is_valid_merkle_branch( - hash_tree_root(blob_sidecar.kzg_commitment), - blob_sidecar.kzg_commitment_inclusion_proof, - KZG_COMMITMENT_INCLUSION_PROOF_DEPTH, - get_subtree_index(gindex), - blob_sidecar.signed_block_header.message.body_root): + if blob_sidecar.verify_blob_sidecar_inclusion_proof().isErr: return errReject("BlobSidecar: inclusion proof not valid") ok() @@ -361,7 +354,7 @@ proc validateBlobSidecar*( # [REJECT] The sidecar's inclusion proof is valid as verified by # `verify_blob_sidecar_inclusion_proof(blob_sidecar)`. block: - let v = verify_blob_sidecar_inclusion_proof(blob_sidecar) + let v = check_blob_sidecar_inclusion_proof(blob_sidecar) if v.isErr: return dag.checkedReject(v.error) diff --git a/beacon_chain/spec/datatypes/deneb.nim b/beacon_chain/spec/datatypes/deneb.nim index dd624c335f..c3412b478b 100644 --- a/beacon_chain/spec/datatypes/deneb.nim +++ b/beacon_chain/spec/datatypes/deneb.nim @@ -616,6 +616,20 @@ func kzg_commitment_inclusion_proof_gindex*( BLOB_KZG_COMMITMENTS_FIRST_GINDEX + index +# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/deneb/p2p-interface.md#check_blob_sidecar_inclusion_proof +func verify_blob_sidecar_inclusion_proof*( + blob_sidecar: BlobSidecar): Opt[void] = + let gindex = kzg_commitment_inclusion_proof_gindex(blob_sidecar.index) + if not is_valid_merkle_branch( + hash_tree_root(blob_sidecar.kzg_commitment), + blob_sidecar.kzg_commitment_inclusion_proof, + KZG_COMMITMENT_INCLUSION_PROOF_DEPTH, + get_subtree_index(gindex), + blob_sidecar.signed_block_header.message.body_root): + return err() + + ok() + # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.5/specs/deneb/light-client/sync-protocol.md#modified-get_lc_execution_root func get_lc_execution_root*( header: LightClientHeader, cfg: RuntimeConfig): Eth2Digest = diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index 39f656bf5b..df447c2e7f 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -193,8 +193,9 @@ proc shouldGetBlobs[A, B](man: SyncManager[A, B], e: Epoch): bool = (wallEpoch < man.MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS or e >= wallEpoch - man.MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS) -proc getBlobSidecars*[A, B](man: SyncManager[A, B], peer: A, - req: SyncRequest): Future[BlobSidecarsRes] {.async.} = +proc getBlobSidecars[A, B](man: SyncManager[A, B], peer: A, + req: SyncRequest + ): Future[BlobSidecarsRes] {.async.} = mixin getScore, `==` logScope: @@ -241,23 +242,32 @@ func groupBlobs*[T](req: SyncRequest[T], blocks: seq[ref ForkedSignedBeaconBlock], blobs: seq[ref BlobSidecar]): Result[seq[BlobSidecars], string] = - var grouped = newSeq[BlobSidecars](len(blocks)) - var blobCursor = 0 - var i = 0 - for blck in blocks: - let slot = blck[].slot - if blobCursor == len(blobs): - # reached end of blobs, have more blobless blocks - break - for blob in blobs[blobCursor..len(blobs)-1]: - if blob.signed_block_header.message.slot < slot: - return Result[seq[BlobSidecars], string].err "invalid blob sequence" - if blob.signed_block_header.message.slot == slot: - grouped[i].add(blob) - blobCursor = blobCursor + 1 - i = i + 1 - - if blobCursor != len(blobs): + var + grouped = newSeq[BlobSidecars](len(blocks)) + blob_cursor = 0 + for block_idx, blck in blocks: + withBlck(blck): + when consensusFork >= ConsensusFork.Deneb: + template commits: untyped = forkyBlck.message.body.blob_kzg_commitments + if commits.len == 0: + continue + let header = forkyBlck.toBeaconBlockHeader() + for blob_idx, kzg_commitment in commits: + if blob_cursor >= blobs.len: + return err("BlobSidecar: response too short") + let blob_sidecar = blobs[blob_cursor] + if blob_sidecar.index != BlobIndex blob_idx: + return err("BlobSidecar: unexpected index") + if blob_sidecar.kzg_commitment != kzg_commitment: + return err("BlobSidecar: unexpected kzg_commitment") + if blob_sidecar.signed_block_header != header: + return err("BlobSidecar: unexpected signed_block_header") + if blob_sidecar.verify_blob_sidecar_inclusion_proof().isErr: + return err("BlobSidecar: inclusion proof not valid") + grouped[block_idx].add(blob_sidecar) + inc blob_cursor + + if blob_cursor != len(blobs): # we reached end of blocks without consuming all blobs so either # the peer we got too few blocks in the paired request, or the # peer is sending us spurious blobs. From f43ddd26c747dbbac2522db2b7365ff150c3f3a7 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 00:35:47 +0100 Subject: [PATCH 02/13] move verify helper to `helpers` --- beacon_chain/spec/datatypes/deneb.nim | 14 -------------- beacon_chain/spec/helpers.nim | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/beacon_chain/spec/datatypes/deneb.nim b/beacon_chain/spec/datatypes/deneb.nim index c3412b478b..dd624c335f 100644 --- a/beacon_chain/spec/datatypes/deneb.nim +++ b/beacon_chain/spec/datatypes/deneb.nim @@ -616,20 +616,6 @@ func kzg_commitment_inclusion_proof_gindex*( BLOB_KZG_COMMITMENTS_FIRST_GINDEX + index -# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/deneb/p2p-interface.md#check_blob_sidecar_inclusion_proof -func verify_blob_sidecar_inclusion_proof*( - blob_sidecar: BlobSidecar): Opt[void] = - let gindex = kzg_commitment_inclusion_proof_gindex(blob_sidecar.index) - if not is_valid_merkle_branch( - hash_tree_root(blob_sidecar.kzg_commitment), - blob_sidecar.kzg_commitment_inclusion_proof, - KZG_COMMITMENT_INCLUSION_PROOF_DEPTH, - get_subtree_index(gindex), - blob_sidecar.signed_block_header.message.body_root): - return err() - - ok() - # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.5/specs/deneb/light-client/sync-protocol.md#modified-get_lc_execution_root func get_lc_execution_root*( header: LightClientHeader, cfg: RuntimeConfig): Eth2Digest = diff --git a/beacon_chain/spec/helpers.nim b/beacon_chain/spec/helpers.nim index e22ca25872..128535ed91 100644 --- a/beacon_chain/spec/helpers.nim +++ b/beacon_chain/spec/helpers.nim @@ -211,6 +211,20 @@ func has_flag*(flags: ParticipationFlags, flag_index: TimelyFlag): bool = let flag = ParticipationFlags(1'u8 shl ord(flag_index)) (flags and flag) == flag +# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/deneb/p2p-interface.md#check_blob_sidecar_inclusion_proof +func verify_blob_sidecar_inclusion_proof*( + blob_sidecar: BlobSidecar): Opt[void] = + let gindex = kzg_commitment_inclusion_proof_gindex(blob_sidecar.index) + if not is_valid_merkle_branch( + hash_tree_root(blob_sidecar.kzg_commitment), + blob_sidecar.kzg_commitment_inclusion_proof, + KZG_COMMITMENT_INCLUSION_PROOF_DEPTH, + get_subtree_index(gindex), + blob_sidecar.signed_block_header.message.body_root): + return err() + + ok() + func create_blob_sidecars*( forkyBlck: deneb.SignedBeaconBlock, kzg_proofs: KzgProofs, From dd06a85b1e33feced21af3e18a12f054436a04c9 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 00:44:53 +0100 Subject: [PATCH 03/13] deref --- beacon_chain/sync/sync_manager.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index df447c2e7f..50fd367e07 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -246,7 +246,7 @@ func groupBlobs*[T](req: SyncRequest[T], grouped = newSeq[BlobSidecars](len(blocks)) blob_cursor = 0 for block_idx, blck in blocks: - withBlck(blck): + withBlck(blck[]): when consensusFork >= ConsensusFork.Deneb: template commits: untyped = forkyBlck.message.body.blob_kzg_commitments if commits.len == 0: From 3adee835e323e35f7bc260435c03ea542ab68bb9 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 00:48:56 +0100 Subject: [PATCH 04/13] use lower descore on inconsistent blobs sequence --- beacon_chain/sync/sync_manager.nim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index 50fd367e07..092d943f64 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -464,9 +464,9 @@ proc syncStep[A, B](man: SyncManager[A, B], index: int, peer: A) {.async.} = return let groupedBlobs = groupBlobs(req, blockData, blobData) if groupedBlobs.isErr(): - peer.updateScore(PeerScoreBadResponse) + peer.updateScore(PeerScoreNoValues) man.queue.push(req) - warn "Received blobs sequence is invalid", + info "Received blobs sequence is inconsistent", blobs_map = getShortMap(req, blobData), request = req, msg=groupedBlobs.error() return Opt.some(groupedBlobs.get()) From 30cace09d98cac49cac6067ce115a2b0feb8fd46 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 00:49:18 +0100 Subject: [PATCH 05/13] remove unnecessary checks that are already guaranteed to hold --- beacon_chain/sync/sync_manager.nim | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index 092d943f64..55d370524c 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -473,21 +473,6 @@ proc syncStep[A, B](man: SyncManager[A, B], index: int, peer: A) {.async.} = else: Opt.none(seq[BlobSidecars]) - if blobData.isSome: - let blobs = blobData.get() - if len(blobs) != len(blockData): - peer.updateScore(PeerScoreNoValues) - man.queue.push(req) - info "block and blobs have different lengths", blobs=len(blobs), blocks=len(blockData) - return - for i, blk in blockData: - if len(blobs[i]) > 0 and blk[].slot != - blobs[i][0].signed_block_header.message.slot: - peer.updateScore(PeerScoreNoValues) - man.queue.push(req) - debug "block and blobs data have inconsistent slots" - return - if len(blockData) == 0 and man.direction == SyncQueueKind.Backward and req.contains(man.getSafeSlot()): # The sync protocol does not distinguish between: From d804a9bec36164af2ba10f749ec4f4adf4d4b342 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 00:58:46 +0100 Subject: [PATCH 06/13] also compare proposer sig --- beacon_chain/sync/sync_manager.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index 55d370524c..41797f4626 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -251,7 +251,7 @@ func groupBlobs*[T](req: SyncRequest[T], template commits: untyped = forkyBlck.message.body.blob_kzg_commitments if commits.len == 0: continue - let header = forkyBlck.toBeaconBlockHeader() + let header = forkyBlck.toSignedBeaconBlockHeader() for blob_idx, kzg_commitment in commits: if blob_cursor >= blobs.len: return err("BlobSidecar: response too short") From 7f227c0ec9749ce0f1b93b9bd4c2b954114b98db Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 01:01:38 +0100 Subject: [PATCH 07/13] deref --- beacon_chain/sync/sync_manager.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index 41797f4626..65dcfda003 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -262,7 +262,7 @@ func groupBlobs*[T](req: SyncRequest[T], return err("BlobSidecar: unexpected kzg_commitment") if blob_sidecar.signed_block_header != header: return err("BlobSidecar: unexpected signed_block_header") - if blob_sidecar.verify_blob_sidecar_inclusion_proof().isErr: + if blob_sidecar[].verify_blob_sidecar_inclusion_proof().isErr: return err("BlobSidecar: inclusion proof not valid") grouped[block_idx].add(blob_sidecar) inc blob_cursor From 756e6afc6e26588d8d0d66e671293f8adee9ec2f Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 13:06:50 +0100 Subject: [PATCH 08/13] split blobs check from grouping --- beacon_chain/sync/sync_manager.nim | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index 65dcfda003..48323586da 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -262,8 +262,6 @@ func groupBlobs*[T](req: SyncRequest[T], return err("BlobSidecar: unexpected kzg_commitment") if blob_sidecar.signed_block_header != header: return err("BlobSidecar: unexpected signed_block_header") - if blob_sidecar[].verify_blob_sidecar_inclusion_proof().isErr: - return err("BlobSidecar: inclusion proof not valid") grouped[block_idx].add(blob_sidecar) inc blob_cursor @@ -275,6 +273,12 @@ func groupBlobs*[T](req: SyncRequest[T], else: Result[seq[BlobSidecars], string].ok grouped +func checkBlobs(blob_sidecars: seq[BlobSidecars]): Result[void, string] = + for blob_sidecar in blob_sidecars: + if blob_sidecar[].verify_blob_sidecar_inclusion_proof().isErr: + return err("BlobSidecar: inclusion proof not valid") + ok() + proc syncStep[A, B](man: SyncManager[A, B], index: int, peer: A) {.async.} = logScope: peer_score = peer.getScore() @@ -469,6 +473,15 @@ proc syncStep[A, B](man: SyncManager[A, B], index: int, peer: A) {.async.} = info "Received blobs sequence is inconsistent", blobs_map = getShortMap(req, blobData), request = req, msg=groupedBlobs.error() return + if (let checkedBlobs = groupedBlobs.checkBlobs(); checkedBlobs.isErr): + peer.updateScore(PeerScoreBadResponse) + man.queue.push(req) + warn "Received blobs sequence is invalid", + blobs_count = len(blobData), + blobs_map = getShortMap(req, blobData), + request = req, + msg = checkedblobs.error + return Opt.some(groupedBlobs.get()) else: Opt.none(seq[BlobSidecars]) From 20fa70b5091eb2fd0d4bff7e7c1cd5885c3accc9 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 13:12:52 +0100 Subject: [PATCH 09/13] double list --- beacon_chain/sync/sync_manager.nim | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index 48323586da..0e40f9c11d 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -273,10 +273,11 @@ func groupBlobs*[T](req: SyncRequest[T], else: Result[seq[BlobSidecars], string].ok grouped -func checkBlobs(blob_sidecars: seq[BlobSidecars]): Result[void, string] = - for blob_sidecar in blob_sidecars: - if blob_sidecar[].verify_blob_sidecar_inclusion_proof().isErr: - return err("BlobSidecar: inclusion proof not valid") +func checkBlobs(blobs: seq[BlobSidecars]): Result[void, string] = + for blob_sidecars in blobs: + for blob_sidecar in blob_sidecars: + if blob_sidecar[].verify_blob_sidecar_inclusion_proof().isErr: + return err("BlobSidecar: inclusion proof not valid") ok() proc syncStep[A, B](man: SyncManager[A, B], index: int, peer: A) {.async.} = From 9c82eddb9c5041c61e4692f5f544f189ba6c160e Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 13:26:01 +0100 Subject: [PATCH 10/13] add spec reference, confirming that correct blob order can be expected --- beacon_chain/sync/sync_manager.nim | 3 +++ 1 file changed, 3 insertions(+) diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index 0e40f9c11d..ee67ceb11d 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -251,6 +251,9 @@ func groupBlobs*[T](req: SyncRequest[T], template commits: untyped = forkyBlck.message.body.blob_kzg_commitments if commits.len == 0: continue + # Clients MUST include all blob sidecars of each block from which they include blob sidecars. + # The following blob sidecars, where they exist, MUST be sent in consecutive (slot, index) order. + # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.5/specs/deneb/p2p-interface.md#blobsidecarsbyrange-v1 let header = forkyBlck.toSignedBeaconBlockHeader() for blob_idx, kzg_commitment in commits: if blob_cursor >= blobs.len: From b0965d91a2b226048a43a99fa5bde847c404bc44 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 13:48:44 +0100 Subject: [PATCH 11/13] unwrap --- beacon_chain/sync/sync_manager.nim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index ee67ceb11d..2304886e43 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -477,14 +477,14 @@ proc syncStep[A, B](man: SyncManager[A, B], index: int, peer: A) {.async.} = info "Received blobs sequence is inconsistent", blobs_map = getShortMap(req, blobData), request = req, msg=groupedBlobs.error() return - if (let checkedBlobs = groupedBlobs.checkBlobs(); checkedBlobs.isErr): + if (let checkRes = groupedBlobs.get.checkBlobs(); checkRes.isErr): peer.updateScore(PeerScoreBadResponse) man.queue.push(req) warn "Received blobs sequence is invalid", blobs_count = len(blobData), blobs_map = getShortMap(req, blobData), request = req, - msg = checkedblobs.error + msg = checkRes.error return Opt.some(groupedBlobs.get()) else: From 443a50d9da719610a05b3d11c25653296122183c Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 16:21:09 +0100 Subject: [PATCH 12/13] test --- beacon_chain/sync/sync_manager.nim | 6 ++--- tests/test_sync_manager.nim | 39 ++++++++++++++++++++++-------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index 2304886e43..c606e5d5d8 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -248,14 +248,14 @@ func groupBlobs*[T](req: SyncRequest[T], for block_idx, blck in blocks: withBlck(blck[]): when consensusFork >= ConsensusFork.Deneb: - template commits: untyped = forkyBlck.message.body.blob_kzg_commitments - if commits.len == 0: + template kzgs: untyped = forkyBlck.message.body.blob_kzg_commitments + if kzgs.len == 0: continue # Clients MUST include all blob sidecars of each block from which they include blob sidecars. # The following blob sidecars, where they exist, MUST be sent in consecutive (slot, index) order. # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.5/specs/deneb/p2p-interface.md#blobsidecarsbyrange-v1 let header = forkyBlck.toSignedBeaconBlockHeader() - for blob_idx, kzg_commitment in commits: + for blob_idx, kzg_commitment in kzgs: if blob_cursor >= blobs.len: return err("BlobSidecar: response too short") let blob_sidecar = blobs[blob_cursor] diff --git a/tests/test_sync_manager.nim b/tests/test_sync_manager.nim index b5c61c9839..bdbde9f2b9 100644 --- a/tests/test_sync_manager.nim +++ b/tests/test_sync_manager.nim @@ -12,7 +12,6 @@ import unittest2 import chronos import ../beacon_chain/gossip_processing/block_processor, ../beacon_chain/sync/sync_manager, - ../beacon_chain/spec/datatypes/phase0, ../beacon_chain/spec/forks type @@ -66,16 +65,36 @@ suite "SyncManager test suite": var res = newSeq[ref ForkedSignedBeaconBlock](count) var curslot = start for item in res.mitems(): - item = new ForkedSignedBeaconBlock - item[].phase0Data.message.slot = curslot + item = newClone ForkedSignedBeaconBlock(kind: ConsensusFork.Deneb) + item[].denebData.message.slot = curslot curslot = curslot + 1'u64 res - func createBlobs(slots: seq[Slot]): seq[ref BlobSidecar] = + func createBlobs( + blocks: var seq[ref ForkedSignedBeaconBlock], slots: seq[Slot] + ): seq[ref BlobSidecar] = var res = newSeq[ref BlobSidecar](len(slots)) - for (i, item) in res.mpairs(): - item = new BlobSidecar - item[].signed_block_header.message.slot = slots[i] + for blck in blocks: + withBlck(blck[]): + when consensusFork >= ConsensusFork.Deneb: + template kzgs: untyped = forkyBlck.message.body.blob_kzg_commitments + for i, slot in slots: + if slot == forkyBlck.message.slot: + doAssert kzgs.add default(KzgCommitment) + if kzgs.len > 0: + forkyBlck.root = hash_tree_root(forkyBlck.message) + var + kzg_proofs: KzgProofs + blobs: Blobs + for _ in kzgs: + doAssert kzg_proofs.add default(KzgProof) + doAssert blobs.add default(Blob) + let sidecars = forkyBlck.create_blob_sidecars(kzg_proofs, blobs) + var sidecarIdx = 0 + for i, slot in slots: + if slot == forkyBlck.message.slot: + res[i] = newClone sidecars[sidecarIdx] + inc sidecarIdx res proc getSlice(chain: openArray[ref ForkedSignedBeaconBlock], startSlot: Slot, @@ -1064,8 +1083,8 @@ suite "SyncManager test suite": checkResponse(r21, @[slots[3]]) == false test "[SyncManager] groupBlobs() test": - var blobs = createBlobs(@[Slot(11), Slot(11), Slot(12), Slot(14)]) var blocks = createChain(Slot(10), Slot(15)) + var blobs = createBlobs(blocks, @[Slot(11), Slot(11), Slot(12), Slot(14)]) let req = SyncRequest[SomeTPeer](slot: Slot(10)) let groupedRes = groupBlobs(req, blocks, blobs) @@ -1095,8 +1114,8 @@ suite "SyncManager test suite": len(grouped[5]) == 0 # Add block with a gap from previous block. - let block17 = new (ref ForkedSignedBeaconBlock) - block17[].phase0Data.message.slot = Slot(17) + let block17 = newClone ForkedSignedBeaconBlock(kind: ConsensusFork.Deneb) + block17[].denebData.message.slot = Slot(17) blocks.add(block17) let groupedRes2 = groupBlobs(req, blocks, blobs) From b0786847076d5bb98596061a78a8ce5ae52b4c23 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 17 Jan 2024 16:41:32 +0100 Subject: [PATCH 13/13] `Result[void, string]` --- beacon_chain/gossip_processing/gossip_validation.nim | 5 +++-- beacon_chain/spec/helpers.nim | 5 ++--- beacon_chain/sync/sync_manager.nim | 3 +-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/beacon_chain/gossip_processing/gossip_validation.nim b/beacon_chain/gossip_processing/gossip_validation.nim index 86710c2196..5cd4c17dca 100644 --- a/beacon_chain/gossip_processing/gossip_validation.nim +++ b/beacon_chain/gossip_processing/gossip_validation.nim @@ -181,8 +181,9 @@ func check_attestation_subnet( func check_blob_sidecar_inclusion_proof( blob_sidecar: deneb.BlobSidecar): Result[void, ValidationError] = - if blob_sidecar.verify_blob_sidecar_inclusion_proof().isErr: - return errReject("BlobSidecar: inclusion proof not valid") + let res = blob_sidecar.verify_blob_sidecar_inclusion_proof() + if res.isErr: + return errReject(res.error) ok() diff --git a/beacon_chain/spec/helpers.nim b/beacon_chain/spec/helpers.nim index 128535ed91..e56058a106 100644 --- a/beacon_chain/spec/helpers.nim +++ b/beacon_chain/spec/helpers.nim @@ -213,7 +213,7 @@ func has_flag*(flags: ParticipationFlags, flag_index: TimelyFlag): bool = # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/deneb/p2p-interface.md#check_blob_sidecar_inclusion_proof func verify_blob_sidecar_inclusion_proof*( - blob_sidecar: BlobSidecar): Opt[void] = + blob_sidecar: BlobSidecar): Result[void, string] = let gindex = kzg_commitment_inclusion_proof_gindex(blob_sidecar.index) if not is_valid_merkle_branch( hash_tree_root(blob_sidecar.kzg_commitment), @@ -221,8 +221,7 @@ func verify_blob_sidecar_inclusion_proof*( KZG_COMMITMENT_INCLUSION_PROOF_DEPTH, get_subtree_index(gindex), blob_sidecar.signed_block_header.message.body_root): - return err() - + return err("BlobSidecar: inclusion proof not valid") ok() func create_blob_sidecars*( diff --git a/beacon_chain/sync/sync_manager.nim b/beacon_chain/sync/sync_manager.nim index c606e5d5d8..4e231eb138 100644 --- a/beacon_chain/sync/sync_manager.nim +++ b/beacon_chain/sync/sync_manager.nim @@ -279,8 +279,7 @@ func groupBlobs*[T](req: SyncRequest[T], func checkBlobs(blobs: seq[BlobSidecars]): Result[void, string] = for blob_sidecars in blobs: for blob_sidecar in blob_sidecars: - if blob_sidecar[].verify_blob_sidecar_inclusion_proof().isErr: - return err("BlobSidecar: inclusion proof not valid") + ? blob_sidecar[].verify_blob_sidecar_inclusion_proof() ok() proc syncStep[A, B](man: SyncManager[A, B], index: int, peer: A) {.async.} =