From 808f9c7f7f1f634d794c8aef048ff664aac61f94 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 15 Jul 2022 23:54:53 +0800 Subject: [PATCH] Use `validate_*` for the valdiation functions that return `None` --- specs/eip4844/p2p-interface.md | 4 +-- specs/eip4844/validator.md | 27 ++++++++++--------- .../unittests/validator/test_validator.py | 16 +++++------ 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/specs/eip4844/p2p-interface.md b/specs/eip4844/p2p-interface.md index 147017eb7..279594903 100644 --- a/specs/eip4844/p2p-interface.md +++ b/specs/eip4844/p2p-interface.md @@ -119,7 +119,7 @@ where `proposer_pubkey` is the pubkey of the beacon block proposer of `blobs_sid Note that a sidecar may be propagated before or after the corresponding beacon block. -Once both sidecar and beacon block are received, `verify_blobs_sidecar` can unlock the data-availability fork-choice dependency. +Once both sidecar and beacon block are received, `validate_blobs_sidecar` can unlock the data-availability fork-choice dependency. ### Transitioning the gossip @@ -190,7 +190,7 @@ The response is unsigned, i.e. `BlobsSidecarsByRange`, as the signature of the b may not be available beyond the initial distribution via gossip. Before consuming the next response chunk, the response reader SHOULD verify the blobs sidecar is well-formatted and -correct w.r.t. the expected KZG commitments through `verify_blobs_sidecar`. +correct w.r.t. the expected KZG commitments through `validate_blobs_sidecar`. `BlobsSidecarsByRange` is primarily used to sync blobs that may have been missed on gossip. diff --git a/specs/eip4844/validator.md b/specs/eip4844/validator.md index 89389380f..1a45342d0 100644 --- a/specs/eip4844/validator.md +++ b/specs/eip4844/validator.md @@ -19,7 +19,7 @@ - [`hash_to_bls_field`](#hash_to_bls_field) - [`compute_powers`](#compute_powers) - [`compute_aggregated_poly_and_commitment`](#compute_aggregated_poly_and_commitment) - - [`verify_blobs_sidecar`](#verify_blobs_sidecar) + - [`validate_blobs_sidecar`](#validate_blobs_sidecar) - [`compute_proof_from_blobs`](#compute_proof_from_blobs) - [`get_blobs_and_kzg_commitments`](#get_blobs_and_kzg_commitments) - [Beacon chain responsibilities](#beacon-chain-responsibilities) @@ -74,7 +74,7 @@ class PolynomialAndCommitment(Container): The implementation of `is_data_available` is meant to change with later sharding upgrades. Initially, it requires every verifying actor to retrieve the matching `BlobsSidecar`, -and verify the sidecar with `verify_blobs_sidecar`. +and validate the sidecar with `validate_blobs_sidecar`. Without the sidecar the block may be processed further optimistically, but MUST NOT be considered valid until a valid `BlobsSidecar` has been downloaded. @@ -83,7 +83,9 @@ but MUST NOT be considered valid until a valid `BlobsSidecar` has been downloade def is_data_available(slot: Slot, beacon_block_root: Root, blob_kzg_commitments: Sequence[KZGCommitment]) -> bool: # `retrieve_blobs_sidecar` is implementation dependent, raises an exception if not available. sidecar = retrieve_blobs_sidecar(slot, beacon_block_root) - return verify_blobs_sidecar(slot, beacon_block_root, blob_kzg_commitments, sidecar) + validate_blobs_sidecar(slot, beacon_block_root, blob_kzg_commitments, sidecar) + + return True ``` ### `hash_to_bls_field` @@ -132,11 +134,13 @@ def compute_aggregated_poly_and_commitment( return aggregated_poly, aggregated_poly_commitment ``` -### `verify_blobs_sidecar` +### `validate_blobs_sidecar` ```python -def verify_blobs_sidecar(slot: Slot, beacon_block_root: Root, - expected_kzg_commitments: Sequence[KZGCommitment], blobs_sidecar: BlobsSidecar) -> bool: +def validate_blobs_sidecar(slot: Slot, + beacon_block_root: Root, + expected_kzg_commitments: Sequence[KZGCommitment], + blobs_sidecar: BlobsSidecar) -> None: assert slot == blobs_sidecar.beacon_block_slot assert beacon_block_root == blobs_sidecar.beacon_block_root blobs = blobs_sidecar.blobs @@ -156,7 +160,7 @@ def verify_blobs_sidecar(slot: Slot, beacon_block_root: Root, y = evaluate_polynomial_in_evaluation_form(aggregated_poly, x) # Verify aggregated proof - return verify_kzg_proof(aggregated_poly_commitment, x, y, kzg_aggregated_proof) + assert verify_kzg_proof(aggregated_poly_commitment, x, y, kzg_aggregated_proof) ``` ### `compute_proof_from_blobs` @@ -197,19 +201,18 @@ Namely, the blob handling and the addition of `BlobsSidecar`. 1. After retrieving the execution payload from the execution engine as specified in Bellatrix, use the `payload_id` to retrieve `blobs` and `blob_kzg_commitments` via `get_blobs_and_kzg_commitments(payload_id)`. -2. Verify `blobs` and `blob_kzg_commitments`: +2. Validate `blobs` and `blob_kzg_commitments`: ```python -def verify_blobs_and_kzg_commitments(execution_payload: ExecutionPayload, - blobs: Sequence[BLSFieldElement], - blob_kzg_commitments: Sequence[KZGCommitment]) -> bool: +def validate_blobs_and_kzg_commitments(execution_payload: ExecutionPayload, + blobs: Sequence[BLSFieldElement], + blob_kzg_commitments: Sequence[KZGCommitment]) -> None: # Optionally sanity-check that the KZG commitments match the versioned hashes in the transactions assert verify_kzg_commitments_against_transactions(execution_payload.transactions, blob_kzg_commitments) # Optionally sanity-check that the KZG commitments match the blobs (as produced by the execution engine) assert len(blob_kzg_commitments) == len(blobs) assert [blob_to_kzg_commitment(blob) == commitment for blob, commitment in zip(blobs, blob_kzg_commitments)] - return True ``` 3. If valid, set `block.body.blob_kzg_commitments = blob_kzg_commitments`. diff --git a/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py index bd6374f3f..d0250f3df 100644 --- a/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py @@ -30,7 +30,7 @@ def test_verify_kzg_proof(spec, state): assert spec.verify_kzg_proof(commitment, x, y, proof) -def _run_verify_blobs_sidecar_test(spec, state, blob_count): +def _run_validate_blobs_sidecar_test(spec, state, blob_count): block = build_empty_block_for_next_slot(spec, state) opaque_tx, blobs, blob_kzg_commitments = get_sample_opaque_tx(spec, blob_count=blob_count) block.body.blob_kzg_commitments = blob_kzg_commitments @@ -41,22 +41,22 @@ def _run_verify_blobs_sidecar_test(spec, state, blob_count): privkey = privkeys[1] spec.get_signed_blobs_sidecar(state, blobs_sidecar, privkey) expected_commitments = [spec.blob_to_kzg_commitment(blobs[i]) for i in range(blob_count)] - assert spec.verify_blobs_sidecar(block.slot, block.hash_tree_root(), expected_commitments, blobs_sidecar) + spec.validate_blobs_sidecar(block.slot, block.hash_tree_root(), expected_commitments, blobs_sidecar) @with_eip4844_and_later @spec_state_test -def test_verify_blobs_sidecar_one_blob(spec, state): - _run_verify_blobs_sidecar_test(spec, state, blob_count=1) +def test_validate_blobs_sidecar_one_blob(spec, state): + _run_validate_blobs_sidecar_test(spec, state, blob_count=1) @with_eip4844_and_later @spec_state_test -def test_verify_blobs_sidecar_two_blobs(spec, state): - _run_verify_blobs_sidecar_test(spec, state, blob_count=2) +def test_validate_blobs_sidecar_two_blobs(spec, state): + _run_validate_blobs_sidecar_test(spec, state, blob_count=2) @with_eip4844_and_later @spec_state_test -def test_verify_blobs_sidecar_ten_blobs(spec, state): - _run_verify_blobs_sidecar_test(spec, state, blob_count=10) +def test_validate_blobs_sidecar_ten_blobs(spec, state): + _run_validate_blobs_sidecar_test(spec, state, blob_count=10)