From ce987c4a64d392af1b6c33d4ec010cda4e10a42c Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 27 Jan 2023 09:12:39 +0100 Subject: [PATCH 01/39] Delete `is_merge_transition_block` check from Capella --- specs/capella/fork-choice.md | 55 ++++++++++++++++++++++++++++++++++++ specs/eip4844/fork-choice.md | 4 --- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/specs/capella/fork-choice.md b/specs/capella/fork-choice.md index 0e0a393c3..c08be1c3f 100644 --- a/specs/capella/fork-choice.md +++ b/specs/capella/fork-choice.md @@ -14,6 +14,8 @@ - [`notify_forkchoice_updated`](#notify_forkchoice_updated) - [Helpers](#helpers) - [Extended `PayloadAttributes`](#extended-payloadattributes) +- [Updated fork-choice handlers](#updated-fork-choice-handlers) + - [`on_block`](#on_block) @@ -60,3 +62,56 @@ class PayloadAttributes(object): suggested_fee_recipient: ExecutionAddress withdrawals: Sequence[Withdrawal] # [New in Capella] ``` + +## Updated fork-choice handlers + +### `on_block` + +*Note*: The only modification is the deletion of the verification of merge transition block conditions. + +```python +def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: + """ + Run ``on_block`` upon receiving a new block. + """ + block = signed_block.message + # Parent block must be known + assert block.parent_root in store.block_states + # Make a copy of the state to avoid mutability issues + pre_state = copy(store.block_states[block.parent_root]) + # Blocks cannot be in the future. If they are, their consideration must be delayed until they are in the past. + assert get_current_slot(store) >= block.slot + + # Check that block is later than the finalized epoch slot (optimization to reduce calls to get_ancestor) + finalized_slot = compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) + assert block.slot > finalized_slot + # Check block is a descendant of the finalized block at the checkpoint finalized slot + assert get_ancestor(store, block.parent_root, finalized_slot) == store.finalized_checkpoint.root + + # Check the block is valid and compute the post-state + state = pre_state.copy() + state_transition(state, signed_block, True) + + # Add new block to the store + store.blocks[hash_tree_root(block)] = block + # Add new state for this block to the store + store.block_states[hash_tree_root(block)] = state + + # Add proposer score boost if the block is timely + time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT + is_before_attesting_interval = time_into_slot < SECONDS_PER_SLOT // INTERVALS_PER_SLOT + if get_current_slot(store) == block.slot and is_before_attesting_interval: + store.proposer_boost_root = hash_tree_root(block) + + # Update justified checkpoint + if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch: + if state.current_justified_checkpoint.epoch > store.best_justified_checkpoint.epoch: + store.best_justified_checkpoint = state.current_justified_checkpoint + if should_update_justified_checkpoint(store, state.current_justified_checkpoint): + store.justified_checkpoint = state.current_justified_checkpoint + + # Update finalized checkpoint + if state.finalized_checkpoint.epoch > store.finalized_checkpoint.epoch: + store.finalized_checkpoint = state.finalized_checkpoint + store.justified_checkpoint = state.current_justified_checkpoint +``` diff --git a/specs/eip4844/fork-choice.md b/specs/eip4844/fork-choice.md index 8dea28ded..865443e04 100644 --- a/specs/eip4844/fork-choice.md +++ b/specs/eip4844/fork-choice.md @@ -108,10 +108,6 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: state = pre_state.copy() state_transition(state, signed_block, True) - # Check the merge transition - if is_merge_transition_block(pre_state, block.body): - validate_merge_block(block) - # Add new block to the store store.blocks[hash_tree_root(block)] = block # Add new state for this block to the store From 0f5ac1186e29ec883abd11842530f91c74d5bc52 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 10 May 2023 02:29:00 +0800 Subject: [PATCH 02/39] Remove `is_execution_enabled` condition since Capella --- specs/_features/eip6110/beacon-chain.md | 8 +++---- specs/_features/sharding/beacon-chain.md | 3 +-- specs/capella/beacon-chain.md | 6 ++--- specs/deneb/beacon-chain.md | 5 ++-- .../test/bellatrix/sanity/test_blocks.py | 7 ++++-- .../test/capella/sanity/test_blocks.py | 23 +++++++++++++++++++ 6 files changed, 37 insertions(+), 15 deletions(-) diff --git a/specs/_features/eip6110/beacon-chain.md b/specs/_features/eip6110/beacon-chain.md index 708418e1c..4d69fb4e0 100644 --- a/specs/_features/eip6110/beacon-chain.md +++ b/specs/_features/eip6110/beacon-chain.md @@ -176,9 +176,8 @@ class BeaconState(Container): ```python def process_block(state: BeaconState, block: BeaconBlock) -> None: process_block_header(state, block) - if is_execution_enabled(state, block.body): - process_withdrawals(state, block.body.execution_payload) - process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in EIP6110] + process_withdrawals(state, block.body.execution_payload) + process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in EIP6110] process_randao(state, block.body) process_eth1_data(state, block.body) process_operations(state, block.body) # [Modified in EIP6110] @@ -212,8 +211,7 @@ def process_operations(state: BeaconState, body: BeaconBlockBody) -> None: for_ops(body.bls_to_execution_changes, process_bls_to_execution_change) # [New in EIP6110] - if is_execution_enabled(state, body): - for_ops(body.execution_payload.deposit_receipts, process_deposit_receipt) + for_ops(body.execution_payload.deposit_receipts, process_deposit_receipt) ``` #### New `process_deposit_receipt` diff --git a/specs/_features/sharding/beacon-chain.md b/specs/_features/sharding/beacon-chain.md index 7d6df51aa..f7de7af65 100644 --- a/specs/_features/sharding/beacon-chain.md +++ b/specs/_features/sharding/beacon-chain.md @@ -236,8 +236,7 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: process_block_header(state, block) verify_builder_block_bid(state, block) process_sharded_data(state, block) - if is_execution_enabled(state, block.body): - process_execution_payload(state, block, EXECUTION_ENGINE) + process_execution_payload(state, block, EXECUTION_ENGINE) if not is_builder_block_slot(block.slot): process_randao(state, block.body) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 1df617daa..72260108f 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -331,9 +331,9 @@ def process_historical_summaries_update(state: BeaconState) -> None: ```python def process_block(state: BeaconState, block: BeaconBlock) -> None: process_block_header(state, block) - if is_execution_enabled(state, block.body): - process_withdrawals(state, block.body.execution_payload) # [New in Capella] - process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in Capella] + # Removed `is_execution_enabled` check + process_withdrawals(state, block.body.execution_payload) # [New in Capella] + process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in Capella] process_randao(state, block.body) process_eth1_data(state, block.body) process_operations(state, block.body) # [Modified in Capella] diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 359c7fc95..6aaa2567f 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -200,9 +200,8 @@ def verify_kzg_commitments_against_transactions(transactions: Sequence[Transacti ```python def process_block(state: BeaconState, block: BeaconBlock) -> None: process_block_header(state, block) - if is_execution_enabled(state, block.body): - process_withdrawals(state, block.body.execution_payload) - process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in Deneb] + process_withdrawals(state, block.body.execution_payload) + process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in Deneb] process_randao(state, block.body) process_eth1_data(state, block.body) process_operations(state, block.body) diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/bellatrix/sanity/test_blocks.py index ef6bb75a9..75dfa0c9c 100644 --- a/tests/core/pyspec/eth2spec/test/bellatrix/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/bellatrix/sanity/test_blocks.py @@ -10,7 +10,10 @@ from eth2spec.test.helpers.execution_payload import ( build_randomized_execution_payload ) from eth2spec.test.context import ( - with_bellatrix_and_later, spec_state_test + BELLATRIX, + with_bellatrix_and_later, + with_phases, + spec_state_test, ) @@ -44,7 +47,7 @@ def test_empty_block_transition_randomized_payload(spec, state): yield 'post', state -@with_bellatrix_and_later +@with_phases([BELLATRIX]) @spec_state_test def test_is_execution_enabled_false(spec, state): # Set `latest_execution_payload_header` to empty diff --git a/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py index d62e458be..11d6f4e91 100644 --- a/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py @@ -31,6 +31,29 @@ from eth2spec.test.helpers.deposits import ( from eth2spec.test.helpers.voluntary_exits import prepare_signed_exits +# +# `is_execution_enabled` has been removed from Capella +# + +@with_capella_and_later +@spec_state_test +def test_invalid_is_execution_enabled_false(spec, state): + # Set `latest_execution_payload_header` to empty + state.latest_execution_payload_header = spec.ExecutionPayloadHeader() + yield 'pre', state + + block = build_empty_block_for_next_slot(spec, state) + + # Set `execution_payload` to empty + block.body.execution_payload = spec.ExecutionPayload() + assert len(block.body.execution_payload.transactions) == 0 + + signed_block = state_transition_and_sign_block(spec, state, block, expect_fail=True) + + yield 'blocks', [signed_block] + yield 'post', None + + # # BLSToExecutionChange # From 5b983f40972e44a07a9022390898a48fab37e599 Mon Sep 17 00:00:00 2001 From: gajinder Date: Fri, 12 May 2023 21:44:43 +0530 Subject: [PATCH 03/39] Update the endianess of the polynomial commitments to be big endian --- specs/deneb/polynomial-commitments.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/specs/deneb/polynomial-commitments.md b/specs/deneb/polynomial-commitments.md index e23c31fab..fb2d70237 100644 --- a/specs/deneb/polynomial-commitments.md +++ b/specs/deneb/polynomial-commitments.md @@ -77,6 +77,7 @@ Public functions MUST accept raw bytes as input and perform the required cryptog | `BYTES_PER_FIELD_ELEMENT` | `uint64(32)` | Bytes used to encode a BLS scalar field element | | `BYTES_PER_BLOB` | `uint64(BYTES_PER_FIELD_ELEMENT * FIELD_ELEMENTS_PER_BLOB)` | The number of bytes in a blob | | `G1_POINT_AT_INFINITY` | `Bytes48(b'\xc0' + b'\x00' * 47)` | Serialized form of the point at infinity on the G1 group | +| `KZG_ENDIANNESS` | `'big'` | The endianess of the field elements including blobs | ## Preset @@ -161,7 +162,7 @@ def hash_to_bls_field(data: bytes) -> BLSFieldElement: The output is not uniform over the BLS field. """ hashed_data = hash(data) - return BLSFieldElement(int.from_bytes(hashed_data, ENDIANNESS) % BLS_MODULUS) + return BLSFieldElement(int.from_bytes(hashed_data, KZG_ENDIANNESS) % BLS_MODULUS) ``` #### `bytes_to_bls_field` @@ -172,7 +173,7 @@ def bytes_to_bls_field(b: Bytes32) -> BLSFieldElement: Convert untrusted bytes to a trusted and validated BLS scalar field element. This function does not accept inputs greater than the BLS modulus. """ - field_element = int.from_bytes(b, ENDIANNESS) + field_element = int.from_bytes(b, KZG_ENDIANNESS) assert field_element < BLS_MODULUS return BLSFieldElement(field_element) ``` @@ -237,7 +238,7 @@ def compute_challenge(blob: Blob, """ # Append the degree of the polynomial as a domain separator - degree_poly = int.to_bytes(FIELD_ELEMENTS_PER_BLOB, 16, ENDIANNESS) + degree_poly = int.to_bytes(FIELD_ELEMENTS_PER_BLOB, 16, KZG_ENDIANNESS) data = FIAT_SHAMIR_PROTOCOL_DOMAIN + degree_poly data += blob @@ -406,15 +407,15 @@ def verify_kzg_proof_batch(commitments: Sequence[KZGCommitment], # Compute a random challenge. Note that it does not have to be computed from a hash, # r just has to be random. - degree_poly = int.to_bytes(FIELD_ELEMENTS_PER_BLOB, 8, ENDIANNESS) - num_commitments = int.to_bytes(len(commitments), 8, ENDIANNESS) + degree_poly = int.to_bytes(FIELD_ELEMENTS_PER_BLOB, 8, KZG_ENDIANNESS) + num_commitments = int.to_bytes(len(commitments), 8, KZG_ENDIANNESS) data = RANDOM_CHALLENGE_KZG_BATCH_DOMAIN + degree_poly + num_commitments # Append all inputs to the transcript before we hash for commitment, z, y, proof in zip(commitments, zs, ys, proofs): data += commitment \ - + int.to_bytes(z, BYTES_PER_FIELD_ELEMENT, ENDIANNESS) \ - + int.to_bytes(y, BYTES_PER_FIELD_ELEMENT, ENDIANNESS) \ + + int.to_bytes(z, BYTES_PER_FIELD_ELEMENT, KZG_ENDIANNESS) \ + + int.to_bytes(y, BYTES_PER_FIELD_ELEMENT, KZG_ENDIANNESS) \ + proof r = hash_to_bls_field(data) @@ -451,7 +452,7 @@ def compute_kzg_proof(blob: Blob, z_bytes: Bytes32) -> Tuple[KZGProof, Bytes32]: assert len(z_bytes) == BYTES_PER_FIELD_ELEMENT polynomial = blob_to_polynomial(blob) proof, y = compute_kzg_proof_impl(polynomial, bytes_to_bls_field(z_bytes)) - return proof, y.to_bytes(BYTES_PER_FIELD_ELEMENT, ENDIANNESS) + return proof, y.to_bytes(BYTES_PER_FIELD_ELEMENT, KZG_ENDIANNESS) ``` #### `compute_quotient_eval_within_domain` From eea04704d48e8ddb7626f9835d9d81c548f15498 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 15 May 2023 17:16:41 +0800 Subject: [PATCH 04/39] Fix tests --- setup.py | 2 +- .../polynomial_commitments/test_polynomial_commitments.py | 6 +++--- tests/core/pyspec/eth2spec/test/helpers/sharding.py | 2 +- tests/generators/kzg_4844/main.py | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/setup.py b/setup.py index b2316ed95..c6a9ebaa9 100644 --- a/setup.py +++ b/setup.py @@ -279,7 +279,7 @@ def get_spec(file_name: Path, preset: Dict[str, str], config: Dict[str, str], pr elif name in config: config_vars[name] = VariableDefinition(value_def.type_name, config[name], value_def.comment, None) else: - if name == 'ENDIANNESS': + if name in ('ENDIANNESS', 'KZG_ENDIANNESS'): # Deal with mypy Literal typing check value_def = _parse_value(name, value, type_hint='Final') constant_vars[name] = value_def diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/polynomial_commitments/test_polynomial_commitments.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/polynomial_commitments/test_polynomial_commitments.py index 6d3f377a3..5ee8097c8 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/polynomial_commitments/test_polynomial_commitments.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/polynomial_commitments/test_polynomial_commitments.py @@ -33,7 +33,7 @@ def bls_add_one(x): def field_element_bytes(x): - return int.to_bytes(x % BLS_MODULUS, 32, "little") + return int.to_bytes(x % BLS_MODULUS, 32, "big") @with_deneb_and_later @@ -304,7 +304,7 @@ def test_bytes_to_bls_field_modulus_minus_one(spec): Verify that `bytes_to_bls_field` handles modulus minus one """ - spec.bytes_to_bls_field((BLS_MODULUS - 1).to_bytes(spec.BYTES_PER_FIELD_ELEMENT, spec.ENDIANNESS)) + spec.bytes_to_bls_field((BLS_MODULUS - 1).to_bytes(spec.BYTES_PER_FIELD_ELEMENT, spec.KZG_ENDIANNESS)) @with_deneb_and_later @@ -316,7 +316,7 @@ def test_bytes_to_bls_field_modulus(spec): """ expect_assertion_error(lambda: spec.bytes_to_bls_field( - BLS_MODULUS.to_bytes(spec.BYTES_PER_FIELD_ELEMENT, spec.ENDIANNESS) + BLS_MODULUS.to_bytes(spec.BYTES_PER_FIELD_ELEMENT, spec.KZG_ENDIANNESS) )) diff --git a/tests/core/pyspec/eth2spec/test/helpers/sharding.py b/tests/core/pyspec/eth2spec/test/helpers/sharding.py index 6b913b90e..cd88ae3ea 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/sharding.py +++ b/tests/core/pyspec/eth2spec/test/helpers/sharding.py @@ -61,7 +61,7 @@ def get_sample_blob(spec, rng=None): b = bytes() for v in values: - b += v.to_bytes(32, spec.ENDIANNESS) + b += v.to_bytes(32, spec.KZG_ENDIANNESS) return spec.Blob(b) diff --git a/tests/generators/kzg_4844/main.py b/tests/generators/kzg_4844/main.py index 2f3efeb21..9297e524a 100644 --- a/tests/generators/kzg_4844/main.py +++ b/tests/generators/kzg_4844/main.py @@ -27,11 +27,11 @@ def expect_exception(func, *args): def field_element_bytes(x): - return int.to_bytes(x % spec.BLS_MODULUS, 32, spec.ENDIANNESS) + return int.to_bytes(x % spec.BLS_MODULUS, 32, spec.KZG_ENDIANNESS) def field_element_bytes_unchecked(x): - return int.to_bytes(x, 32, spec.ENDIANNESS) + return int.to_bytes(x, 32, spec.KZG_ENDIANNESS) def encode_hex_list(a): @@ -54,7 +54,7 @@ def evaluate_blob_at(blob, z): ) -BLS_MODULUS_BYTES = spec.BLS_MODULUS.to_bytes(32, spec.ENDIANNESS) +BLS_MODULUS_BYTES = spec.BLS_MODULUS.to_bytes(32, spec.KZG_ENDIANNESS) G1 = bls.G1_to_bytes48(bls.G1()) G1_INVALID_TOO_FEW_BYTES = G1[:-1] From 340f3cc1a3c54bac1892050d7555b53d305ecd56 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 15 May 2023 17:25:48 +0800 Subject: [PATCH 05/39] Rebase the fc changes --- specs/capella/fork-choice.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/specs/capella/fork-choice.md b/specs/capella/fork-choice.md index c08be1c3f..87fec02f8 100644 --- a/specs/capella/fork-choice.md +++ b/specs/capella/fork-choice.md @@ -86,16 +86,22 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: finalized_slot = compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) assert block.slot > finalized_slot # Check block is a descendant of the finalized block at the checkpoint finalized slot - assert get_ancestor(store, block.parent_root, finalized_slot) == store.finalized_checkpoint.root + finalized_checkpoint_block = get_checkpoint_block( + store, + block.parent_root, + store.finalized_checkpoint.epoch, + ) + assert store.finalized_checkpoint.root == finalized_checkpoint_block # Check the block is valid and compute the post-state state = pre_state.copy() + block_root = hash_tree_root(block) state_transition(state, signed_block, True) # Add new block to the store - store.blocks[hash_tree_root(block)] = block + store.blocks[block_root] = block # Add new state for this block to the store - store.block_states[hash_tree_root(block)] = state + store.block_states[block_root] = state # Add proposer score boost if the block is timely time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT @@ -103,15 +109,9 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: if get_current_slot(store) == block.slot and is_before_attesting_interval: store.proposer_boost_root = hash_tree_root(block) - # Update justified checkpoint - if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch: - if state.current_justified_checkpoint.epoch > store.best_justified_checkpoint.epoch: - store.best_justified_checkpoint = state.current_justified_checkpoint - if should_update_justified_checkpoint(store, state.current_justified_checkpoint): - store.justified_checkpoint = state.current_justified_checkpoint + # Update checkpoints in store if necessary + update_checkpoints(store, state.current_justified_checkpoint, state.finalized_checkpoint) - # Update finalized checkpoint - if state.finalized_checkpoint.epoch > store.finalized_checkpoint.epoch: - store.finalized_checkpoint = state.finalized_checkpoint - store.justified_checkpoint = state.current_justified_checkpoint + # Eagerly compute unrealized justification and finality. + compute_pulled_up_tip(store, block_root) ``` From 928fd42517b6ffc41ffd96e2e06066f47762ad4d Mon Sep 17 00:00:00 2001 From: g11tech Date: Mon, 15 May 2023 15:53:18 +0530 Subject: [PATCH 06/39] fix typo Co-authored-by: Hsiao-Wei Wang --- specs/deneb/polynomial-commitments.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/deneb/polynomial-commitments.md b/specs/deneb/polynomial-commitments.md index fb2d70237..fe5ae845a 100644 --- a/specs/deneb/polynomial-commitments.md +++ b/specs/deneb/polynomial-commitments.md @@ -77,7 +77,7 @@ Public functions MUST accept raw bytes as input and perform the required cryptog | `BYTES_PER_FIELD_ELEMENT` | `uint64(32)` | Bytes used to encode a BLS scalar field element | | `BYTES_PER_BLOB` | `uint64(BYTES_PER_FIELD_ELEMENT * FIELD_ELEMENTS_PER_BLOB)` | The number of bytes in a blob | | `G1_POINT_AT_INFINITY` | `Bytes48(b'\xc0' + b'\x00' * 47)` | Serialized form of the point at infinity on the G1 group | -| `KZG_ENDIANNESS` | `'big'` | The endianess of the field elements including blobs | +| `KZG_ENDIANNESS` | `'big'` | The endianness of the field elements including blobs | ## Preset From 058137327a4cb9163c4202e1ed3bd4d886d745cf Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 16 May 2023 00:27:00 +0800 Subject: [PATCH 07/39] Use new `engine_newPayloadV3` --- setup.py | 2 +- specs/_features/eip6110/beacon-chain.md | 10 ++- specs/bellatrix/beacon-chain.md | 25 +++++- specs/capella/beacon-chain.md | 8 +- specs/deneb/beacon-chain.md | 85 ++++++++----------- specs/deneb/validator.md | 4 +- .../test_process_execution_payload.py | 4 +- .../test/phase0/sanity/test_blocks.py | 2 +- 8 files changed, 74 insertions(+), 66 deletions(-) diff --git a/setup.py b/setup.py index b2316ed95..8ef66df23 100644 --- a/setup.py +++ b/setup.py @@ -578,7 +578,7 @@ def get_pow_chain_head() -> PowBlock: class NoopExecutionEngine(ExecutionEngine): - def notify_new_payload(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: + def notify_new_payload(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: return True def notify_forkchoice_updated(self: ExecutionEngine, diff --git a/specs/_features/eip6110/beacon-chain.md b/specs/_features/eip6110/beacon-chain.md index 708418e1c..c9dccbdeb 100644 --- a/specs/_features/eip6110/beacon-chain.md +++ b/specs/_features/eip6110/beacon-chain.md @@ -178,12 +178,11 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: process_block_header(state, block) if is_execution_enabled(state, block.body): process_withdrawals(state, block.body.execution_payload) - process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in EIP6110] + process_execution_payload(state, block.body, EXECUTION_ENGINE) # [Modified in EIP6110] process_randao(state, block.body) process_eth1_data(state, block.body) process_operations(state, block.body) # [Modified in EIP6110] process_sync_aggregate(state, block.body.sync_aggregate) - process_blob_kzg_commitments(block.body) ``` #### Modified `process_operations` @@ -238,7 +237,9 @@ def process_deposit_receipt(state: BeaconState, deposit_receipt: DepositReceipt) *Note*: The function `process_execution_payload` is modified to use the new `ExecutionPayloadHeader` type. ```python -def process_execution_payload(state: BeaconState, payload: ExecutionPayload, execution_engine: ExecutionEngine) -> None: +def process_execution_payload(state: BeaconState, body: BeaconBlockBody, execution_engine: ExecutionEngine) -> None: + payload = body.execution_payload + # Verify consistency of the parent hash with respect to the previous execution payload header if is_merge_transition_complete(state): assert payload.parent_hash == state.latest_execution_payload_header.block_hash @@ -247,7 +248,8 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe # Verify timestamp assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid - assert execution_engine.notify_new_payload(payload) + versioned_hashes = [kzg_commitment_to_versioned_hash(commitment) for commitment in body.blob_kzg_commitments] + assert execution_engine.notify_new_payload(NewPayloadRequest(execution_payload=payload)) # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( parent_hash=payload.parent_hash, diff --git a/specs/bellatrix/beacon-chain.md b/specs/bellatrix/beacon-chain.md index 1133cba06..a4c7be16e 100644 --- a/specs/bellatrix/beacon-chain.md +++ b/specs/bellatrix/beacon-chain.md @@ -33,6 +33,9 @@ - [Modified `slash_validator`](#modified-slash_validator) - [Beacon chain state transition function](#beacon-chain-state-transition-function) - [Execution engine](#execution-engine) + - [Request data](#request-data) + - [`NewPayloadRequest`](#newpayloadrequest) + - [Engine APIs](#engine-apis) - [`notify_new_payload`](#notify_new_payload) - [Block processing](#block-processing) - [Execution payload](#execution-payload) @@ -300,6 +303,18 @@ def slash_validator(state: BeaconState, ### Execution engine +#### Request data + +##### `NewPayloadRequest` + +```python +@dataclass +class NewPayloadRequest(object): + execution_payload: ExecutionPayload +``` + +#### Engine APIs + The implementation-dependent `ExecutionEngine` protocol encapsulates the execution sub-system logic via: * a state object `self.execution_state` of type `ExecutionState` @@ -313,7 +328,7 @@ The Engine API may be used to implement this and similarly defined functions via #### `notify_new_payload` ```python -def notify_new_payload(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: +def notify_new_payload(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: """ Return ``True`` if and only if ``execution_payload`` is valid with respect to ``self.execution_state``. """ @@ -328,7 +343,7 @@ def notify_new_payload(self: ExecutionEngine, execution_payload: ExecutionPayloa def process_block(state: BeaconState, block: BeaconBlock) -> None: process_block_header(state, block) if is_execution_enabled(state, block.body): - process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [New in Bellatrix] + process_execution_payload(state, block.body, EXECUTION_ENGINE) # [New in Bellatrix] process_randao(state, block.body) process_eth1_data(state, block.body) process_operations(state, block.body) @@ -340,7 +355,9 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: ##### `process_execution_payload` ```python -def process_execution_payload(state: BeaconState, payload: ExecutionPayload, execution_engine: ExecutionEngine) -> None: +def process_execution_payload(state: BeaconState, body: BeaconBlockBody, execution_engine: ExecutionEngine) -> None: + payload = body.execution_payload + # Verify consistency of the parent hash with respect to the previous execution payload header if is_merge_transition_complete(state): assert payload.parent_hash == state.latest_execution_payload_header.block_hash @@ -349,7 +366,7 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe # Verify timestamp assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid - assert execution_engine.notify_new_payload(payload) + assert execution_engine.notify_new_payload(NewPayloadRequest(execution_payload=payload)) # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( parent_hash=payload.parent_hash, diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 1df617daa..1799f4e6e 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -333,7 +333,7 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: process_block_header(state, block) if is_execution_enabled(state, block.body): process_withdrawals(state, block.body.execution_payload) # [New in Capella] - process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in Capella] + process_execution_payload(state, block.body, EXECUTION_ENGINE) # [Modified in Capella] process_randao(state, block.body) process_eth1_data(state, block.body) process_operations(state, block.body) # [Modified in Capella] @@ -407,7 +407,9 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: *Note*: The function `process_execution_payload` is modified to use the new `ExecutionPayloadHeader` type. ```python -def process_execution_payload(state: BeaconState, payload: ExecutionPayload, execution_engine: ExecutionEngine) -> None: +def process_execution_payload(state: BeaconState, body: BeaconBlockBody, execution_engine: ExecutionEngine) -> None: + payload = body.execution_payload + # Verify consistency of the parent hash with respect to the previous execution payload header if is_merge_transition_complete(state): assert payload.parent_hash == state.latest_execution_payload_header.block_hash @@ -416,7 +418,7 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe # Verify timestamp assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid - assert execution_engine.notify_new_payload(payload) + assert execution_engine.notify_new_payload(NewPayloadRequest(execution_payload=payload)) # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( parent_hash=payload.parent_hash, diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 359c7fc95..52719dcf6 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -24,13 +24,15 @@ - [Helper functions](#helper-functions) - [Misc](#misc) - [`kzg_commitment_to_versioned_hash`](#kzg_commitment_to_versioned_hash) - - [`tx_peek_blob_versioned_hashes`](#tx_peek_blob_versioned_hashes) - - [`verify_kzg_commitments_against_transactions`](#verify_kzg_commitments_against_transactions) - [Beacon chain state transition function](#beacon-chain-state-transition-function) + - [Execution engine](#execution-engine) + - [Request data](#request-data) + - [Modified `NewPayloadRequest`](#modified-newpayloadrequest) + - [Engine APIs](#engine-apis) + - [Modified `notify_new_payload`](#modified-notify_new_payload) - [Block processing](#block-processing) - [Execution payload](#execution-payload) - [`process_execution_payload`](#process_execution_payload) - - [Blob KZG commitments](#blob-kzg-commitments) - [Testing](#testing) @@ -158,43 +160,33 @@ def kzg_commitment_to_versioned_hash(kzg_commitment: KZGCommitment) -> Versioned return VERSIONED_HASH_VERSION_KZG + hash(kzg_commitment)[1:] ``` -#### `tx_peek_blob_versioned_hashes` - -This function retrieves the hashes from the `SignedBlobTransaction` as defined in Deneb, using SSZ offsets. -Offsets are little-endian `uint32` values, as defined in the [SSZ specification](../../ssz/simple-serialize.md). -See [the full details of `blob_versioned_hashes` offset calculation](https://gist.github.com/protolambda/23bd106b66f6d4bb854ce46044aa3ca3). - -```python -def tx_peek_blob_versioned_hashes(opaque_tx: Transaction) -> Sequence[VersionedHash]: - assert opaque_tx[0] == BLOB_TX_TYPE - message_offset = 1 + uint32.decode_bytes(opaque_tx[1:5]) - # field offset: 32 + 8 + 32 + 32 + 8 + 4 + 32 + 4 + 4 + 32 = 188 - blob_versioned_hashes_offset = ( - message_offset - + uint32.decode_bytes(opaque_tx[(message_offset + 188):(message_offset + 192)]) - ) - # `VersionedHash` is a 32-byte object - assert (len(opaque_tx) - blob_versioned_hashes_offset) % 32 == 0 - return [ - VersionedHash(opaque_tx[x:(x + 32)]) - for x in range(blob_versioned_hashes_offset, len(opaque_tx), 32) - ] -``` - -#### `verify_kzg_commitments_against_transactions` - -```python -def verify_kzg_commitments_against_transactions(transactions: Sequence[Transaction], - kzg_commitments: Sequence[KZGCommitment]) -> bool: - all_versioned_hashes: List[VersionedHash] = [] - for tx in transactions: - if tx[0] == BLOB_TX_TYPE: - all_versioned_hashes += tx_peek_blob_versioned_hashes(tx) - return all_versioned_hashes == [kzg_commitment_to_versioned_hash(commitment) for commitment in kzg_commitments] -``` - ## Beacon chain state transition function +### Execution engine + +#### Request data + +##### Modified `NewPayloadRequest` + +```python +@dataclass +class NewPayloadRequest(object): + execution_payload: ExecutionPayload + versioned_hashes: Sequence[VersionedHash] +``` + +#### Engine APIs + +#### Modified `notify_new_payload` + +```python +def notify_new_payload(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: + """ + Return ``True`` if and only if ``execution_payload`` is valid with respect to ``self.execution_state``. + """ + ... +``` + ### Block processing ```python @@ -202,12 +194,11 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: process_block_header(state, block) if is_execution_enabled(state, block.body): process_withdrawals(state, block.body.execution_payload) - process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in Deneb] + process_execution_payload(state, block.body, EXECUTION_ENGINE) # [Modified in Deneb] process_randao(state, block.body) process_eth1_data(state, block.body) process_operations(state, block.body) process_sync_aggregate(state, block.body.sync_aggregate) - process_blob_kzg_commitments(block.body) # [New in Deneb] ``` #### Execution payload @@ -215,7 +206,9 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: ##### `process_execution_payload` ```python -def process_execution_payload(state: BeaconState, payload: ExecutionPayload, execution_engine: ExecutionEngine) -> None: +def process_execution_payload(state: BeaconState, body: BeaconBlockBody, execution_engine: ExecutionEngine) -> None: + payload = body.execution_payload + # Verify consistency of the parent hash with respect to the previous execution payload header if is_merge_transition_complete(state): assert payload.parent_hash == state.latest_execution_payload_header.block_hash @@ -224,7 +217,8 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe # Verify timestamp assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid - assert execution_engine.notify_new_payload(payload) + versioned_hashes = [kzg_commitment_to_versioned_hash(commitment) for commitment in body.blob_kzg_commitments] + assert execution_engine.notify_new_payload(NewPayloadRequest(execution_payload=payload, versioned_hashes=versioned_hashes)) # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( @@ -247,13 +241,6 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe ) ``` -#### Blob KZG commitments - -```python -def process_blob_kzg_commitments(body: BeaconBlockBody) -> None: - assert verify_kzg_commitments_against_transactions(body.execution_payload.transactions, body.blob_kzg_commitments) -``` - ## Testing *Note*: The function `initialize_beacon_state_from_eth1` is modified for pure Deneb testing only. diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index 6562c91dd..e52ce9eaa 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -98,8 +98,8 @@ def validate_blobs_and_kzg_commitments(execution_payload: ExecutionPayload, blobs: Sequence[Blob], blob_kzg_commitments: Sequence[KZGCommitment], blob_kzg_proofs: Sequence[KZGProof]) -> 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) + # TODO: can we just remove it? + # 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) == len(blob_kzg_proofs) diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py index 3ec58b31e..05e16f1d1 100644 --- a/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py @@ -29,10 +29,10 @@ def run_execution_payload_processing(spec, state, execution_payload, valid=True, called_new_block = False class TestEngine(spec.NoopExecutionEngine): - def notify_new_payload(self, payload) -> bool: + def notify_new_payload(self, new_payload_request) -> bool: nonlocal called_new_block, execution_valid called_new_block = True - assert payload == execution_payload + assert new_payload_request.execution_payload == execution_payload return execution_valid if not valid: diff --git a/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py index 2e1a2a369..d31955e82 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py @@ -143,7 +143,7 @@ def process_and_sign_block_without_header_validations(spec, state, block): ) if is_post_bellatrix(spec): if spec.is_execution_enabled(state, block.body): - spec.process_execution_payload(state, block.body.execution_payload, spec.EXECUTION_ENGINE) + spec.process_execution_payload(state, block.body, spec.EXECUTION_ENGINE) # Perform rest of process_block transitions spec.process_randao(state, block.body) From 0b2f604f86a037d5b92e82aa79ab8bfc3a55d812 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 16 May 2023 23:58:01 +0800 Subject: [PATCH 08/39] Fix tests --- specs/_features/eip6110/beacon-chain.md | 4 +- specs/deneb/beacon-chain.md | 5 ++- specs/deneb/validator.md | 6 +-- .../test_process_execution_payload.py | 12 ++--- .../test_process_withdrawals.py | 2 +- .../eth2spec/test/deneb/sanity/test_blocks.py | 45 ++++++++++++------- .../test/deneb/unittests/test_offset.py | 23 ---------- .../unittests/validator/test_validator.py | 6 +-- tests/formats/operations/README.md | 2 +- 9 files changed, 48 insertions(+), 57 deletions(-) delete mode 100644 tests/core/pyspec/eth2spec/test/deneb/unittests/test_offset.py diff --git a/specs/_features/eip6110/beacon-chain.md b/specs/_features/eip6110/beacon-chain.md index c9dccbdeb..591d8dc86 100644 --- a/specs/_features/eip6110/beacon-chain.md +++ b/specs/_features/eip6110/beacon-chain.md @@ -249,7 +249,9 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid versioned_hashes = [kzg_commitment_to_versioned_hash(commitment) for commitment in body.blob_kzg_commitments] - assert execution_engine.notify_new_payload(NewPayloadRequest(execution_payload=payload)) + assert execution_engine.notify_new_payload( + NewPayloadRequest(execution_payload=payload, versioned_hashes=versioned_hashes) + ) # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( parent_hash=payload.parent_hash, diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 52719dcf6..2c4bb6133 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -217,8 +217,11 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi # Verify timestamp assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid + # [Modified in Deneb] versioned_hashes = [kzg_commitment_to_versioned_hash(commitment) for commitment in body.blob_kzg_commitments] - assert execution_engine.notify_new_payload(NewPayloadRequest(execution_payload=payload, versioned_hashes=versioned_hashes)) + assert execution_engine.notify_new_payload( + NewPayloadRequest(execution_payload=payload, versioned_hashes=versioned_hashes) + ) # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index e52ce9eaa..03297c59e 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -94,13 +94,9 @@ via `get_payload(payload_id).blobs_bundle`. 2. Validate `blobs` and `blob_kzg_commitments`: ```python -def validate_blobs_and_kzg_commitments(execution_payload: ExecutionPayload, - blobs: Sequence[Blob], +def validate_blobs_and_kzg_commitments(blobs: Sequence[Blob], blob_kzg_commitments: Sequence[KZGCommitment], blob_kzg_proofs: Sequence[KZGProof]) -> None: - # TODO: can we just remove it? - # 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) == len(blob_kzg_proofs) assert verify_blob_kzg_proof_batch(blobs, blob_kzg_commitments, blob_kzg_proofs) diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py index 05e16f1d1..b83563235 100644 --- a/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py @@ -21,10 +21,12 @@ def run_execution_payload_processing(spec, state, execution_payload, valid=True, - post-state ('post'). If ``valid == False``, run expecting ``AssertionError`` """ + # Before Deneb, only `body.execution_payload` matters. `BeaconBlockBody` is just a wrapper. + body = spec.BeaconBlockBody(execution_payload=execution_payload) yield 'pre', state yield 'execution', {'execution_valid': execution_valid} - yield 'execution_payload', execution_payload + yield 'body', body called_new_block = False @@ -32,22 +34,22 @@ def run_execution_payload_processing(spec, state, execution_payload, valid=True, def notify_new_payload(self, new_payload_request) -> bool: nonlocal called_new_block, execution_valid called_new_block = True - assert new_payload_request.execution_payload == execution_payload + assert new_payload_request.execution_payload == body.execution_payload return execution_valid if not valid: - expect_assertion_error(lambda: spec.process_execution_payload(state, execution_payload, TestEngine())) + expect_assertion_error(lambda: spec.process_execution_payload(state, body, TestEngine())) yield 'post', None return - spec.process_execution_payload(state, execution_payload, TestEngine()) + spec.process_execution_payload(state, body, TestEngine()) # Make sure we called the engine assert called_new_block yield 'post', state - assert state.latest_execution_payload_header == get_execution_payload_header(spec, execution_payload) + assert state.latest_execution_payload_header == get_execution_payload_header(spec, body.execution_payload) def run_success_test(spec, state): diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index d7813fb1f..ad71e2c73 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -56,7 +56,7 @@ def verify_post_state(state, spec, expected_withdrawals, def run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=None, fully_withdrawable_indices=None, partial_withdrawals_indices=None, valid=True): """ - Run ``process_execution_payload``, yielding: + Run ``process_withdrawals``, yielding: - pre-state ('pre') - execution payload ('execution_payload') - post-state ('post'). diff --git a/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py index 111565cce..f27a774ab 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py @@ -53,7 +53,10 @@ def test_max_blobs(spec, state): @with_deneb_and_later @spec_state_test -def test_invalid_incorrect_blob_tx_type(spec, state): +def test_incorrect_blob_tx_type(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ yield 'pre', state block = build_empty_block_for_next_slot(spec, state) @@ -62,15 +65,18 @@ def test_invalid_incorrect_blob_tx_type(spec, state): opaque_tx = b'\x04' + opaque_tx[1:] # incorrect tx type block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block, expect_fail=True) + signed_block = state_transition_and_sign_block(spec, state, block) yield 'blocks', [signed_block] - yield 'post', None + yield 'post', state @with_deneb_and_later @spec_state_test -def test_invalid_incorrect_transaction_length_1_byte(spec, state): +def test_incorrect_transaction_length_1_byte(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ yield 'pre', state block = build_empty_block_for_next_slot(spec, state) @@ -79,15 +85,18 @@ def test_invalid_incorrect_transaction_length_1_byte(spec, state): opaque_tx = opaque_tx + b'\x12' # incorrect tx length block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block, expect_fail=True) + signed_block = state_transition_and_sign_block(spec, state, block) yield 'blocks', [signed_block] - yield 'post', None + yield 'post', state @with_deneb_and_later @spec_state_test -def test_invalid_incorrect_transaction_length_32_bytes(spec, state): +def test_incorrect_transaction_length_32_bytes(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ yield 'pre', state block = build_empty_block_for_next_slot(spec, state) @@ -96,15 +105,18 @@ def test_invalid_incorrect_transaction_length_32_bytes(spec, state): opaque_tx = opaque_tx + b'\x12' * 32 # incorrect tx length block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block, expect_fail=True) + signed_block = state_transition_and_sign_block(spec, state, block) yield 'blocks', [signed_block] - yield 'post', None + yield 'post', state @with_deneb_and_later @spec_state_test -def test_invalid_incorrect_commitment(spec, state): +def test_incorrect_commitment(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ yield 'pre', state block = build_empty_block_for_next_slot(spec, state) @@ -113,15 +125,18 @@ def test_invalid_incorrect_commitment(spec, state): block.body.blob_kzg_commitments = blob_kzg_commitments block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block, expect_fail=True) + signed_block = state_transition_and_sign_block(spec, state, block) yield 'blocks', [signed_block] - yield 'post', None + yield 'post', state @with_deneb_and_later @spec_state_test -def test_invalid_incorrect_commitments_order(spec, state): +def test_incorrect_commitments_order(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ yield 'pre', state block = build_empty_block_for_next_slot(spec, state) @@ -129,10 +144,10 @@ def test_invalid_incorrect_commitments_order(spec, state): block.body.blob_kzg_commitments = [blob_kzg_commitments[1], blob_kzg_commitments[0]] # incorrect order block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block, expect_fail=True) + signed_block = state_transition_and_sign_block(spec, state, block) yield 'blocks', [signed_block] - yield 'post', None + yield 'post', state @with_deneb_and_later diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/test_offset.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/test_offset.py deleted file mode 100644 index 3c3b51ff1..000000000 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/test_offset.py +++ /dev/null @@ -1,23 +0,0 @@ - -from eth2spec.test.helpers.constants import ( - DENEB, - MINIMAL, -) -from eth2spec.test.helpers.sharding import ( - get_sample_opaque_tx, -) -from eth2spec.test.context import ( - with_phases, - spec_state_test, - with_presets, -) - - -@with_phases([DENEB]) -@spec_state_test -@with_presets([MINIMAL]) -def test_tx_peek_blob_versioned_hashes(spec, state): - otx, _, commitments, _ = get_sample_opaque_tx(spec) - data_hashes = spec.tx_peek_blob_versioned_hashes(otx) - expected = [spec.kzg_commitment_to_versioned_hash(blob_commitment) for blob_commitment in commitments] - assert expected == data_hashes diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py index 07039ccfe..62b8e9dc1 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py @@ -31,8 +31,7 @@ def test_validate_blobs_and_kzg_commitments(spec, state): block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - spec.validate_blobs_and_kzg_commitments(block.body.execution_payload, - blobs, + spec.validate_blobs_and_kzg_commitments(blobs, blob_kzg_commitments, proofs) @@ -52,7 +51,6 @@ def test_validate_blobs_and_kzg_commitments_missing_blob(spec, state): expect_assertion_error( lambda: spec.validate_blobs_and_kzg_commitments( - block.body.execution_payload, blobs[:-1], blob_kzg_commitments, proofs @@ -75,7 +73,6 @@ def test_validate_blobs_and_kzg_commitments_missing_proof(spec, state): expect_assertion_error( lambda: spec.validate_blobs_and_kzg_commitments( - block.body.execution_payload, blobs, blob_kzg_commitments, proofs[:-1] @@ -100,7 +97,6 @@ def test_validate_blobs_and_kzg_commitments_incorrect_blob(spec, state): expect_assertion_error( lambda: spec.validate_blobs_and_kzg_commitments( - block.body.execution_payload, blobs, blob_kzg_commitments, proofs diff --git a/tests/formats/operations/README.md b/tests/formats/operations/README.md index 245ce8565..ba764879b 100644 --- a/tests/formats/operations/README.md +++ b/tests/formats/operations/README.md @@ -42,7 +42,7 @@ Operations: | `proposer_slashing` | `ProposerSlashing` | `proposer_slashing` | `process_proposer_slashing(state, proposer_slashing)` | | `voluntary_exit` | `SignedVoluntaryExit` | `voluntary_exit` | `process_voluntary_exit(state, voluntary_exit)` | | `sync_aggregate` | `SyncAggregate` | `sync_aggregate` | `process_sync_aggregate(state, sync_aggregate)` (new in Altair) | -| `execution_payload` | `ExecutionPayload` | `execution_payload` | `process_execution_payload(state, execution_payload)` (new in Bellatrix) | +| `execution_payload` | `BeaconBlockBody` | **`body`** | `process_execution_payload(state, body)` (new in Bellatrix) | | `withdrawals` | `ExecutionPayload` | `execution_payload` | `process_withdrawals(state, execution_payload)` (new in Capella) | | `bls_to_execution_change` | `SignedBLSToExecutionChange` | `address_change` | `process_bls_to_execution_change(state, address_change)` (new in Capella) | | `deposit_receipt` | `DepositReceipt` | `deposit_receipt` | `process_deposit_receipt(state, deposit_receipt)` (new in EIP6110) | From a7f8659725ad4372ae7f5be970c235b2e4f6dfbc Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Fri, 19 May 2023 17:17:24 +0300 Subject: [PATCH 09/39] KZG test vector format also uses big endian now --- tests/formats/kzg/compute_kzg_proof.md | 4 ++-- tests/formats/kzg/verify_kzg_proof.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/formats/kzg/compute_kzg_proof.md b/tests/formats/kzg/compute_kzg_proof.md index 0713d50d8..b10105129 100644 --- a/tests/formats/kzg/compute_kzg_proof.md +++ b/tests/formats/kzg/compute_kzg_proof.md @@ -14,8 +14,8 @@ output: Tuple[KZGProof, Bytes32] -- The KZG proof and the value y = f(z) ``` - `blob` here is encoded as a string: hexadecimal encoding of `4096 * 32 = 131072` bytes, prefixed with `0x`. -- `z` here is encoded as a string: hexadecimal encoding of `32` bytes representing a little endian encoded field element, prefixed with `0x`. -- `y` here is encoded as a string: hexadecimal encoding of `32` bytes representing a little endian encoded field element, prefixed with `0x`. +- `z` here is encoded as a string: hexadecimal encoding of `32` bytes representing a big endian encoded field element, prefixed with `0x`. +- `y` here is encoded as a string: hexadecimal encoding of `32` bytes representing a big endian encoded field element, prefixed with `0x`. All byte(s) fields are encoded as strings, hexadecimal encoding, prefixed with `0x`. diff --git a/tests/formats/kzg/verify_kzg_proof.md b/tests/formats/kzg/verify_kzg_proof.md index 143466b66..18e02710c 100644 --- a/tests/formats/kzg/verify_kzg_proof.md +++ b/tests/formats/kzg/verify_kzg_proof.md @@ -15,8 +15,8 @@ input: output: bool -- true (valid proof) or false (incorrect proof) ``` -- `z` here is encoded as a string: hexadecimal encoding of `32` bytes representing a little endian encoded field element, prefixed with `0x`. -- `y` here is encoded as a string: hexadecimal encoding of `32` bytes representing a little endian encoded field element, prefixed with `0x`. +- `z` here is encoded as a string: hexadecimal encoding of `32` bytes representing a big endian encoded field element, prefixed with `0x`. +- `y` here is encoded as a string: hexadecimal encoding of `32` bytes representing a big endian encoded field element, prefixed with `0x`. All byte(s) fields are encoded as strings, hexadecimal encoding, prefixed with `0x`. From f87e1436bfbc1f51a18aedc337c6013cd3b3305e Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 16 May 2023 23:36:52 +0800 Subject: [PATCH 10/39] Add networking configs to config files --- configs/mainnet.yaml | 28 ++++++++ configs/minimal.yaml | 28 ++++++++ presets/mainnet/phase0.yaml | 5 ++ presets/minimal/phase0.yaml | 5 ++ setup.py | 1 + specs/phase0/p2p-interface.md | 72 +++++++++++++++++-- specs/phase0/validator.md | 45 ------------ .../unittests/test_config_invariants.py | 8 ++- .../validator/test_validator_unittest.py | 4 +- 9 files changed, 141 insertions(+), 55 deletions(-) diff --git a/configs/mainnet.yaml b/configs/mainnet.yaml index 5ad394c08..1a1537b26 100644 --- a/configs/mainnet.yaml +++ b/configs/mainnet.yaml @@ -94,3 +94,31 @@ PROPOSER_SCORE_BOOST: 40 DEPOSIT_CHAIN_ID: 1 DEPOSIT_NETWORK_ID: 1 DEPOSIT_CONTRACT_ADDRESS: 0x00000000219ab540356cBB839Cbe05303d7705Fa + + +# Networking +# --------------------------------------------------------------- +# `2**20` (= 1048576, 1 MiB) +GOSSIP_MAX_SIZE: 1048576 +# `2**10` (= 1024) +MAX_REQUEST_BLOCKS: 1024 +# `MIN_VALIDATOR_WITHDRAWABILITY_DELAY + CHURN_LIMIT_QUOTIENT // 2` (= 33024, ~5 months) +MIN_EPOCHS_FOR_BLOCK_REQUESTS: 33024 +# `2**20` (=1048576, 1 MiB) +MAX_CHUNK_SIZE: 1048576 +# 5s +TTFB_TIMEOUT: 5 +# 10s +RESP_TIMEOUT: 10 +ATTESTATION_PROPAGATION_SLOT_RANGE: 32 +# 500ms +MAXIMUM_GOSSIP_CLOCK_DISPARITY: 500 +MESSAGE_DOMAIN_INVALID_SNAPPY: 0x00000000 +MESSAGE_DOMAIN_VALID_SNAPPY: 0x01000000 +# 2 subnets per node +SUBNETS_PER_NODE: 2 +# 2**8 (= 64) +ATTESTATION_SUBNET_COUNT: 64 +ATTESTATION_SUBNET_EXTRA_BITS: 0 +# ceillog2(ATTESTATION_SUBNET_COUNT) + ATTESTATION_SUBNET_EXTRA_BITS +ATTESTATION_SUBNET_PREFIX_BITS: 6 diff --git a/configs/minimal.yaml b/configs/minimal.yaml index 5895cfc70..32d5682a8 100644 --- a/configs/minimal.yaml +++ b/configs/minimal.yaml @@ -95,3 +95,31 @@ DEPOSIT_CHAIN_ID: 5 DEPOSIT_NETWORK_ID: 5 # Configured on a per testnet basis DEPOSIT_CONTRACT_ADDRESS: 0x1234567890123456789012345678901234567890 + + +# Networking +# --------------------------------------------------------------- +# `2**20` (= 1048576, 1 MiB) +GOSSIP_MAX_SIZE: 1048576 +# `2**10` (= 1024) +MAX_REQUEST_BLOCKS: 1024 +# [customized] `MIN_VALIDATOR_WITHDRAWABILITY_DELAY + CHURN_LIMIT_QUOTIENT // 2` (= 272) +MIN_EPOCHS_FOR_BLOCK_REQUESTS: 272 +# `2**20` (=1048576, 1 MiB) +MAX_CHUNK_SIZE: 1048576 +# 5s +TTFB_TIMEOUT: 5 +# 10s +RESP_TIMEOUT: 10 +ATTESTATION_PROPAGATION_SLOT_RANGE: 32 +# 500ms +MAXIMUM_GOSSIP_CLOCK_DISPARITY: 500 +MESSAGE_DOMAIN_INVALID_SNAPPY: 0x00000000 +MESSAGE_DOMAIN_VALID_SNAPPY: 0x01000000 +# 2 subnets per node +SUBNETS_PER_NODE: 2 +# 2**8 (= 64) +ATTESTATION_SUBNET_COUNT: 64 +ATTESTATION_SUBNET_EXTRA_BITS: 0 +# ceillog2(ATTESTATION_SUBNET_COUNT) + ATTESTATION_SUBNET_EXTRA_BITS +ATTESTATION_SUBNET_PREFIX_BITS: 6 diff --git a/presets/mainnet/phase0.yaml b/presets/mainnet/phase0.yaml index 02bc96c8c..c84a93595 100644 --- a/presets/mainnet/phase0.yaml +++ b/presets/mainnet/phase0.yaml @@ -86,3 +86,8 @@ MAX_ATTESTATIONS: 128 MAX_DEPOSITS: 16 # 2**4 (= 16) MAX_VOLUNTARY_EXITS: 16 + +# Networking +# --------------------------------------------------------------- +# 2**8 (= 256) +EPOCHS_PER_SUBNET_SUBSCRIPTION: 256 diff --git a/presets/minimal/phase0.yaml b/presets/minimal/phase0.yaml index e7028f5a4..c55f7de7b 100644 --- a/presets/minimal/phase0.yaml +++ b/presets/minimal/phase0.yaml @@ -86,3 +86,8 @@ MAX_ATTESTATIONS: 128 MAX_DEPOSITS: 16 # 2**4 (= 16) MAX_VOLUNTARY_EXITS: 16 + +# Networking +# --------------------------------------------------------------- +# 2**8 (= 256) +EPOCHS_PER_SUBNET_SUBSCRIPTION: 256 diff --git a/setup.py b/setup.py index f053412b5..4f338f27f 100644 --- a/setup.py +++ b/setup.py @@ -988,6 +988,7 @@ class PySpecCommand(Command): specs/phase0/fork-choice.md specs/phase0/validator.md specs/phase0/weak-subjectivity.md + specs/phase0/p2p-interface.md """ if self.spec_fork in (ALTAIR, BELLATRIX, CAPELLA, DENEB, EIP6110): self.md_doc_paths += """ diff --git a/specs/phase0/p2p-interface.md b/specs/phase0/p2p-interface.md index 2503d906c..3b8a91cc9 100644 --- a/specs/phase0/p2p-interface.md +++ b/specs/phase0/p2p-interface.md @@ -20,6 +20,9 @@ It consists of four main sections: - [Protocol Negotiation](#protocol-negotiation) - [Multiplexing](#multiplexing) - [Consensus-layer network interaction domains](#consensus-layer-network-interaction-domains) + - [Custom types](#custom-types) + - [Constants](#constants) + - [Preset](#preset) - [Configuration](#configuration) - [MetaData](#metadata) - [The gossip domain: gossipsub](#the-gossip-domain-gossipsub) @@ -53,6 +56,7 @@ It consists of four main sections: - [ENR structure](#enr-structure) - [Attestation subnet bitfield](#attestation-subnet-bitfield) - [`eth2` field](#eth2-field) + - [Attestation subnet subcription](#attestation-subnet-subcription) - [Design decision rationale](#design-decision-rationale) - [Transport](#transport-1) - [Why are we defining specific transports?](#why-are-we-defining-specific-transports) @@ -165,22 +169,46 @@ See the [Rationale](#design-decision-rationale) section below for tradeoffs. ## Consensus-layer network interaction domains +### Custom types + +We define the following Python custom types for type hinting and readability: + +| Name | SSZ equivalent | Description | +| - | - | - | +| `NodeID` | `uint256` | node identifier | +| `SubnetID` | `uint64` | subnet identifier | + +### Constants + +| Name | Value | Unit | Duration | +| - | - | :-: | :-: | +| `NODE_ID_BITS` | `256` | The bit length of uint256 is 256 | + +### Preset +| Name | Value | Unit | Duration | +| - | - | :-: | :-: | +| `EPOCHS_PER_SUBNET_SUBSCRIPTION` | `2**8` (= 256) | epochs | ~27 hours | + ### Configuration -This section outlines constants that are used in this spec. +This section outlines configurations that are used in this spec. | Name | Value | Description | |---|---|---| | `GOSSIP_MAX_SIZE` | `2**20` (= 1048576, 1 MiB) | The maximum allowed size of uncompressed gossip messages. | | `MAX_REQUEST_BLOCKS` | `2**10` (= 1024) | Maximum number of blocks in a single request | | `MIN_EPOCHS_FOR_BLOCK_REQUESTS` | `MIN_VALIDATOR_WITHDRAWABILITY_DELAY + CHURN_LIMIT_QUOTIENT // 2` (= 33024, ~5 months) | The minimum epoch range over which a node must serve blocks | -| `MAX_CHUNK_SIZE` | `2**20` (1048576, 1 MiB) | The maximum allowed size of uncompressed req/resp chunked responses. | -| `TTFB_TIMEOUT` | `5s` | The maximum time to wait for first byte of request response (time-to-first-byte). | -| `RESP_TIMEOUT` | `10s` | The maximum time for complete response transfer. | +| `MAX_CHUNK_SIZE` | `2**20` (=1048576, 1 MiB) | The maximum allowed size of uncompressed req/resp chunked responses. | +| `TTFB_TIMEOUT` | `5` | The maximum duration in **seconds** to wait for first byte of request response (time-to-first-byte). | +| `RESP_TIMEOUT` | `10` | The maximum duration in **seconds** for complete response transfer. | | `ATTESTATION_PROPAGATION_SLOT_RANGE` | `32` | The maximum number of slots during which an attestation can be propagated. | -| `MAXIMUM_GOSSIP_CLOCK_DISPARITY` | `500ms` | The maximum milliseconds of clock disparity assumed between honest nodes. | -| `MESSAGE_DOMAIN_INVALID_SNAPPY` | `0x00000000` | 4-byte domain for gossip message-id isolation of *invalid* snappy messages | -| `MESSAGE_DOMAIN_VALID_SNAPPY` | `0x01000000` | 4-byte domain for gossip message-id isolation of *valid* snappy messages | +| `MAXIMUM_GOSSIP_CLOCK_DISPARITY` | `500` | The maximum **milliseconds** of clock disparity assumed between honest nodes. | +| `MESSAGE_DOMAIN_INVALID_SNAPPY` | `DomainType('0x00000000')` | 4-byte domain for gossip message-id isolation of *invalid* snappy messages | +| `MESSAGE_DOMAIN_VALID_SNAPPY` | `DomainType('0x01000000')` | 4-byte domain for gossip message-id isolation of *valid* snappy messages | +| `SUBNETS_PER_NODE` | `2` | The number of long-lived subnets a beacon node should be subscribed to. | +| `ATTESTATION_SUBNET_COUNT` | `2**6` (= 64) | The number of attestation subnets used in the gossipsub protocol. | +| `ATTESTATION_SUBNET_EXTRA_BITS` | `0` | The number of extra bits of a NodeId to use when mapping to a subscribed subnet | +| `ATTESTATION_SUBNET_PREFIX_BITS` | `int(ceillog2(ATTESTATION_SUBNET_COUNT) + ATTESTATION_SUBNET_EXTRA_BITS)` | | ### MetaData @@ -979,6 +1007,34 @@ Clients MAY connect to peers with the same `fork_digest` but a different `next_f Unless `ENRForkID` is manually updated to matching prior to the earlier `next_fork_epoch` of the two clients, these connecting clients will be unable to successfully interact starting at the earlier `next_fork_epoch`. +### Attestation subnet subcription + +Because Phase 0 does not have shards and thus does not have Shard Committees, there is no stable backbone to the attestation subnets (`beacon_attestation_{subnet_id}`). To provide this stability, each beacon node should: + +* Remain subscribed to `SUBNETS_PER_NODE` for `EPOCHS_PER_SUBNET_SUBSCRIPTION` epochs. +* Maintain advertisement of the selected subnets in their node's ENR `attnets` entry by setting the selected `subnet_id` bits to `True` (e.g. `ENR["attnets"][subnet_id] = True`) for all persistent attestation subnets. +* Select these subnets based on their node-id as specified by the following `compute_subscribed_subnets(node_id, epoch)` function. + +```python +def compute_subscribed_subnet(node_id: NodeID, epoch: Epoch, index: int) -> SubnetID: + node_id_prefix = node_id >> (NODE_ID_BITS - ATTESTATION_SUBNET_PREFIX_BITS) + node_offset = node_id % EPOCHS_PER_SUBNET_SUBSCRIPTION + permutation_seed = hash(uint_to_bytes(uint64((epoch + node_offset) // EPOCHS_PER_SUBNET_SUBSCRIPTION))) + permutated_prefix = compute_shuffled_index( + node_id_prefix, + 1 << ATTESTATION_SUBNET_PREFIX_BITS, + permutation_seed, + ) + return SubnetID((permutated_prefix + index) % ATTESTATION_SUBNET_COUNT) +``` + +```python +def compute_subscribed_subnets(node_id: NodeID, epoch: Epoch) -> Sequence[SubnetID]: + return [compute_subscribed_subnet(node_id, epoch, index) for index in range(SUBNETS_PER_NODE)] +``` + +*Note*: When preparing for a hard fork, a node must select and subscribe to subnets of the future fork versioning at least `EPOCHS_PER_SUBNET_SUBSCRIPTION` epochs in advance of the fork. These new subnets for the fork are maintained in addition to those for the current fork until the fork occurs. After the fork occurs, let the subnets from the previous fork reach the end of life with no replacements. + ## Design decision rationale ### Transport @@ -1438,6 +1494,8 @@ the epoch range that a new node syncing from a checkpoint must backfill. [weak subjectivity guide](./weak-subjectivity.md). Specifically to find this max epoch range, we use the worst case event of a very large validator size (`>= MIN_PER_EPOCH_CHURN_LIMIT * CHURN_LIMIT_QUOTIENT`). +[0]: # (eth2spec: skip) + ```python MIN_EPOCHS_FOR_BLOCK_REQUESTS = ( MIN_VALIDATOR_WITHDRAWABILITY_DELAY diff --git a/specs/phase0/validator.md b/specs/phase0/validator.md index b0a9ac507..602df0973 100644 --- a/specs/phase0/validator.md +++ b/specs/phase0/validator.md @@ -10,7 +10,6 @@ This is an accompanying document to [Phase 0 -- The Beacon Chain](./beacon-chain - [Introduction](#introduction) - [Prerequisites](#prerequisites) -- [Custom types](#custom-types) - [Constants](#constants) - [Misc](#misc) - [Containers](#containers) @@ -64,7 +63,6 @@ This is an accompanying document to [Phase 0 -- The Beacon Chain](./beacon-chain - [Aggregation bits](#aggregation-bits-1) - [Aggregate signature](#aggregate-signature-1) - [Broadcast aggregate](#broadcast-aggregate) -- [Phase 0 attestation subnet stability](#phase-0-attestation-subnet-stability) - [How to avoid slashing](#how-to-avoid-slashing) - [Proposer slashing](#proposer-slashing) - [Attester slashing](#attester-slashing) @@ -83,15 +81,6 @@ A validator is an entity that participates in the consensus of the Ethereum proo All terminology, constants, functions, and protocol mechanics defined in the [Phase 0 -- The Beacon Chain](./beacon-chain.md) and [Phase 0 -- Deposit Contract](./deposit-contract.md) doc are requisite for this document and used throughout. Please see the Phase 0 doc before continuing and use as a reference throughout. -## Custom types - -We define the following Python custom types for type hinting and readability: - -| Name | SSZ equivalent | Description | -| - | - | - | -| `NodeID` | `uint256` | node identifier | -| `SubnetID` | `uint64` | subnet identifier | - ## Constants ### Misc @@ -99,12 +88,6 @@ We define the following Python custom types for type hinting and readability: | Name | Value | Unit | Duration | | - | - | :-: | :-: | | `TARGET_AGGREGATORS_PER_COMMITTEE` | `2**4` (= 16) | validators | -| `EPOCHS_PER_SUBNET_SUBSCRIPTION` | `2**8` (= 256) | epochs | ~27 hours | -| `ATTESTATION_SUBNET_COUNT` | `64` | The number of attestation subnets used in the gossipsub protocol. | -| `ATTESTATION_SUBNET_EXTRA_BITS` | `0` | The number of extra bits of a NodeId to use when mapping to a subscribed subnet | -| `SUBNETS_PER_NODE` | `2` | The number of long-lived subnets a beacon node should be subscribed to. | -| `ATTESTATION_SUBNET_PREFIX_BITS` | `(ceillog2(ATTESTATION_SUBNET_COUNT) + ATTESTATION_SUBNET_EXTRA_BITS)` | | -| `NODE_ID_BITS` | `256` | The bit length of uint256 is 256 | ## Containers @@ -619,34 +602,6 @@ def get_aggregate_and_proof_signature(state: BeaconState, return bls.Sign(privkey, signing_root) ``` -## Phase 0 attestation subnet stability - -Because Phase 0 does not have shards and thus does not have Shard Committees, there is no stable backbone to the attestation subnets (`beacon_attestation_{subnet_id}`). To provide this stability, each beacon node should: - -* Remain subscribed to `SUBNETS_PER_NODE` for `EPOCHS_PER_SUBNET_SUBSCRIPTION` epochs. -* Maintain advertisement of the selected subnets in their node's ENR `attnets` entry by setting the selected `subnet_id` bits to `True` (e.g. `ENR["attnets"][subnet_id] = True`) for all persistent attestation subnets. -* Select these subnets based on their node-id as specified by the following `compute_subscribed_subnets(node_id, epoch)` function. - -```python -def compute_subscribed_subnet(node_id: NodeID, epoch: Epoch, index: int) -> SubnetID: - node_id_prefix = node_id >> (NODE_ID_BITS - int(ATTESTATION_SUBNET_PREFIX_BITS)) - node_offset = node_id % EPOCHS_PER_SUBNET_SUBSCRIPTION - permutation_seed = hash(uint_to_bytes(uint64((epoch + node_offset) // EPOCHS_PER_SUBNET_SUBSCRIPTION))) - permutated_prefix = compute_shuffled_index( - node_id_prefix, - 1 << int(ATTESTATION_SUBNET_PREFIX_BITS), - permutation_seed, - ) - return SubnetID((permutated_prefix + index) % ATTESTATION_SUBNET_COUNT) -``` - -```python -def compute_subscribed_subnets(node_id: NodeID, epoch: Epoch) -> Sequence[SubnetID]: - return [compute_subscribed_subnet(node_id, epoch, index) for index in range(SUBNETS_PER_NODE)] -``` - -*Note*: When preparing for a hard fork, a validator must select and subscribe to subnets of the future fork versioning at least `EPOCHS_PER_SUBNET_SUBSCRIPTION` epochs in advance of the fork. These new subnets for the fork are maintained in addition to those for the current fork until the fork occurs. After the fork occurs, let the subnets from the previous fork reach the end of life with no replacements. - ## How to avoid slashing "Slashing" is the burning of some amount of validator funds and immediate ejection from the active validator set. In Phase 0, there are two ways in which funds can be slashed: [proposer slashing](#proposer-slashing) and [attester slashing](#attester-slashing). Although being slashed has serious repercussions, it is simple enough to avoid being slashed all together by remaining _consistent_ with respect to the messages a validator has previously signed. diff --git a/tests/core/pyspec/eth2spec/test/phase0/unittests/test_config_invariants.py b/tests/core/pyspec/eth2spec/test/phase0/unittests/test_config_invariants.py index b0fd06374..6216ea2f0 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/unittests/test_config_invariants.py +++ b/tests/core/pyspec/eth2spec/test/phase0/unittests/test_config_invariants.py @@ -75,7 +75,13 @@ def test_time(spec, state): @with_all_phases @spec_state_test def test_networking(spec, state): - assert spec.SUBNETS_PER_NODE <= spec.ATTESTATION_SUBNET_COUNT + assert spec.config.MIN_EPOCHS_FOR_BLOCK_REQUESTS == ( + spec.config.MIN_VALIDATOR_WITHDRAWABILITY_DELAY + spec.config.CHURN_LIMIT_QUOTIENT // 2 + ) + assert spec.config.ATTESTATION_SUBNET_PREFIX_BITS == ( + spec.ceillog2(spec.config.ATTESTATION_SUBNET_COUNT) + spec.config.ATTESTATION_SUBNET_EXTRA_BITS + ) + assert spec.config.SUBNETS_PER_NODE <= spec.config.ATTESTATION_SUBNET_COUNT node_id_length = spec.NodeID(1).type_byte_length() # in bytes assert node_id_length * 8 == spec.NODE_ID_BITS # in bits diff --git a/tests/core/pyspec/eth2spec/test/phase0/unittests/validator/test_validator_unittest.py b/tests/core/pyspec/eth2spec/test/phase0/unittests/validator/test_validator_unittest.py index 177748eac..918ab96e2 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/unittests/validator/test_validator_unittest.py +++ b/tests/core/pyspec/eth2spec/test/phase0/unittests/validator/test_validator_unittest.py @@ -371,7 +371,7 @@ def test_compute_subnet_for_attestation(spec, state): slots_since_epoch_start = slot % spec.SLOTS_PER_EPOCH committees_since_epoch_start = committees_per_slot * slots_since_epoch_start - expected_subnet_id = (committees_since_epoch_start + committee_idx) % spec.ATTESTATION_SUBNET_COUNT + expected_subnet_id = (committees_since_epoch_start + committee_idx) % spec.config.ATTESTATION_SUBNET_COUNT assert actual_subnet_id == expected_subnet_id @@ -488,7 +488,7 @@ def run_compute_subscribed_subnets_arguments(spec, rng=random.Random(1111)): node_id = rng.randint(0, 2**40 - 1) # try VALIDATOR_REGISTRY_LIMIT epoch = rng.randint(0, 2**64 - 1) subnets = spec.compute_subscribed_subnets(node_id, epoch) - assert len(subnets) == spec.SUBNETS_PER_NODE + assert len(subnets) == spec.config.SUBNETS_PER_NODE @with_all_phases From 92324ca845dee97b3c817d075352b85fa7a1e9c7 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sat, 20 May 2023 01:14:10 +0800 Subject: [PATCH 11/39] Remove `is_merge_transition_complete` check from Capella --- specs/_features/eip4788/validator.md | 19 ++----------------- specs/_features/eip6110/beacon-chain.md | 3 +-- specs/_features/sharding/beacon-chain.md | 3 +-- specs/bellatrix/validator.md | 5 +++-- specs/capella/beacon-chain.md | 6 +++--- specs/capella/validator.md | 19 ++----------------- specs/deneb/beacon-chain.md | 3 +-- .../unittests/validator/test_validator.py | 11 +++++++---- 8 files changed, 20 insertions(+), 49 deletions(-) diff --git a/specs/_features/eip4788/validator.md b/specs/_features/eip4788/validator.md index 421e297ce..f24c97353 100644 --- a/specs/_features/eip4788/validator.md +++ b/specs/_features/eip4788/validator.md @@ -64,27 +64,12 @@ parameter to the `PayloadAttributes`. ```python def prepare_execution_payload(state: BeaconState, - pow_chain: Dict[Hash32, PowBlock], safe_block_hash: Hash32, finalized_block_hash: Hash32, suggested_fee_recipient: ExecutionAddress, execution_engine: ExecutionEngine) -> Optional[PayloadId]: - if not is_merge_transition_complete(state): - is_terminal_block_hash_set = TERMINAL_BLOCK_HASH != Hash32() - is_activation_epoch_reached = get_current_epoch(state) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH - if is_terminal_block_hash_set and not is_activation_epoch_reached: - # Terminal block hash is set but activation epoch is not yet reached, no prepare payload call is needed - return None - - terminal_pow_block = get_terminal_pow_block(pow_chain) - if terminal_pow_block is None: - # Pre-merge, no prepare payload call is needed - return None - # Signify merge via producing on top of the terminal PoW block - parent_hash = terminal_pow_block.block_hash - else: - # Post-merge, normal payload - parent_hash = state.latest_execution_payload_header.block_hash + # Verify consistency of the parent hash with respect to the previous execution payload header + parent_hash = state.latest_execution_payload_header.block_hash # Set the forkchoice head and initiate the payload build process payload_attributes = PayloadAttributes( diff --git a/specs/_features/eip6110/beacon-chain.md b/specs/_features/eip6110/beacon-chain.md index 4d69fb4e0..04ba5d3a7 100644 --- a/specs/_features/eip6110/beacon-chain.md +++ b/specs/_features/eip6110/beacon-chain.md @@ -238,8 +238,7 @@ def process_deposit_receipt(state: BeaconState, deposit_receipt: DepositReceipt) ```python def process_execution_payload(state: BeaconState, payload: ExecutionPayload, execution_engine: ExecutionEngine) -> None: # Verify consistency of the parent hash with respect to the previous execution payload header - if is_merge_transition_complete(state): - assert payload.parent_hash == state.latest_execution_payload_header.block_hash + assert payload.parent_hash == state.latest_execution_payload_header.block_hash # Verify prev_randao assert payload.prev_randao == get_randao_mix(state, get_current_epoch(state)) # Verify timestamp diff --git a/specs/_features/sharding/beacon-chain.md b/specs/_features/sharding/beacon-chain.md index f7de7af65..e67bc09ed 100644 --- a/specs/_features/sharding/beacon-chain.md +++ b/specs/_features/sharding/beacon-chain.md @@ -370,8 +370,7 @@ def process_execution_payload(state: BeaconState, block: BeaconBlock, execution_ payload = block.body.payload_data.value.execution_payload # Verify consistency of the parent hash with respect to the previous execution payload header - if is_merge_transition_complete(state): - assert payload.parent_hash == state.latest_execution_payload_header.block_hash + assert payload.parent_hash == state.latest_execution_payload_header.block_hash # Verify random assert payload.random == get_randao_mix(state, get_current_epoch(state)) # Verify timestamp diff --git a/specs/bellatrix/validator.md b/specs/bellatrix/validator.md index a176d7534..e6fd5443a 100644 --- a/specs/bellatrix/validator.md +++ b/specs/bellatrix/validator.md @@ -118,12 +118,13 @@ To obtain an execution payload, a block proposer building a block on top of a `s ```python def prepare_execution_payload(state: BeaconState, - pow_chain: Dict[Hash32, PowBlock], safe_block_hash: Hash32, finalized_block_hash: Hash32, suggested_fee_recipient: ExecutionAddress, - execution_engine: ExecutionEngine) -> Optional[PayloadId]: + execution_engine: ExecutionEngine, + pow_chain: Optional[Dict[Hash32, PowBlock]]=None) -> Optional[PayloadId]: if not is_merge_transition_complete(state): + assert pow_chain is not None is_terminal_block_hash_set = TERMINAL_BLOCK_HASH != Hash32() is_activation_epoch_reached = get_current_epoch(state) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH if is_terminal_block_hash_set and not is_activation_epoch_reached: diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 72260108f..3ea54ab12 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -331,7 +331,7 @@ def process_historical_summaries_update(state: BeaconState) -> None: ```python def process_block(state: BeaconState, block: BeaconBlock) -> None: process_block_header(state, block) - # Removed `is_execution_enabled` check + # [Modified in Capella] Removed `is_execution_enabled` check in Capella process_withdrawals(state, block.body.execution_payload) # [New in Capella] process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in Capella] process_randao(state, block.body) @@ -408,9 +408,9 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: ```python def process_execution_payload(state: BeaconState, payload: ExecutionPayload, execution_engine: ExecutionEngine) -> None: + # [Modified in Capella] Removed `is_merge_transition_complete` check in Capella # Verify consistency of the parent hash with respect to the previous execution payload header - if is_merge_transition_complete(state): - assert payload.parent_hash == state.latest_execution_payload_header.block_hash + assert payload.parent_hash == state.latest_execution_payload_header.block_hash # Verify prev_randao assert payload.prev_randao == get_randao_mix(state, get_current_epoch(state)) # Verify timestamp diff --git a/specs/capella/validator.md b/specs/capella/validator.md index 644ee476f..12c93b6b0 100644 --- a/specs/capella/validator.md +++ b/specs/capella/validator.md @@ -69,27 +69,12 @@ That is, `state` is the `previous_state` processed through any empty slots up to ```python def prepare_execution_payload(state: BeaconState, - pow_chain: Dict[Hash32, PowBlock], safe_block_hash: Hash32, finalized_block_hash: Hash32, suggested_fee_recipient: ExecutionAddress, execution_engine: ExecutionEngine) -> Optional[PayloadId]: - if not is_merge_transition_complete(state): - is_terminal_block_hash_set = TERMINAL_BLOCK_HASH != Hash32() - is_activation_epoch_reached = get_current_epoch(state) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH - if is_terminal_block_hash_set and not is_activation_epoch_reached: - # Terminal block hash is set but activation epoch is not yet reached, no prepare payload call is needed - return None - - terminal_pow_block = get_terminal_pow_block(pow_chain) - if terminal_pow_block is None: - # Pre-merge, no prepare payload call is needed - return None - # Signify merge via producing on top of the terminal PoW block - parent_hash = terminal_pow_block.block_hash - else: - # Post-merge, normal payload - parent_hash = state.latest_execution_payload_header.block_hash + # [Modified in Capella] Removed `is_merge_transition_complete` check in Capella + parent_hash = state.latest_execution_payload_header.block_hash # Set the forkchoice head and initiate the payload build process payload_attributes = PayloadAttributes( diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 6aaa2567f..5834d5a37 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -216,8 +216,7 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: ```python def process_execution_payload(state: BeaconState, payload: ExecutionPayload, execution_engine: ExecutionEngine) -> None: # Verify consistency of the parent hash with respect to the previous execution payload header - if is_merge_transition_complete(state): - assert payload.parent_hash == state.latest_execution_payload_header.block_hash + assert payload.parent_hash == state.latest_execution_payload_header.block_hash # Verify prev_randao assert payload.prev_randao == get_randao_mix(state, get_current_epoch(state)) # Verify timestamp diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/bellatrix/unittests/validator/test_validator.py index 6398aedd8..c55b6e3c3 100644 --- a/tests/core/pyspec/eth2spec/test/bellatrix/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/bellatrix/unittests/validator/test_validator.py @@ -4,9 +4,12 @@ from typing import Optional from eth2spec.test.helpers.pow_block import ( prepare_random_pow_chain, ) +from eth2spec.test.helpers.constants import ( + BELLATRIX, +) from eth2spec.test.context import ( spec_state_test, - with_bellatrix_and_later, + with_phases, ) @@ -30,7 +33,7 @@ expected_results = [ # it would return the first block (IS_HEAD_PARENT_BLOCK). -@with_bellatrix_and_later +@with_phases([BELLATRIX]) @spec_state_test def test_get_pow_block_at_terminal_total_difficulty(spec, state): for result in expected_results: @@ -90,7 +93,7 @@ prepare_execution_payload_expected_results = [ ] -@with_bellatrix_and_later +@with_phases([BELLATRIX]) @spec_state_test def test_prepare_execution_payload(spec, state): for result in prepare_execution_payload_expected_results: @@ -157,11 +160,11 @@ def test_prepare_execution_payload(spec, state): payload_id = spec.prepare_execution_payload( state=state, - pow_chain=pow_chain.to_dict(), safe_block_hash=safe_block_hash, finalized_block_hash=finalized_block_hash, suggested_fee_recipient=suggested_fee_recipient, execution_engine=TestEngine(), + pow_chain=pow_chain.to_dict(), ) assert payload_id == result_payload_id From 17dac8cab92d2982d7940966d0de40c5e3283828 Mon Sep 17 00:00:00 2001 From: gajinder Date: Tue, 25 Apr 2023 15:36:48 +0530 Subject: [PATCH 12/39] Update MAX_BLOBS_PER_BLOCK to a higher bound --- presets/mainnet/deneb.yaml | 4 ++-- specs/deneb/beacon-chain.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/presets/mainnet/deneb.yaml b/presets/mainnet/deneb.yaml index ebe33f2d1..7cf41bbf4 100644 --- a/presets/mainnet/deneb.yaml +++ b/presets/mainnet/deneb.yaml @@ -4,5 +4,5 @@ # --------------------------------------------------------------- # `uint64(4096)` FIELD_ELEMENTS_PER_BLOB: 4096 -# `uint64(2**2)` (= 4) -MAX_BLOBS_PER_BLOCK: 4 +# `uint64(2**10)` (= 1024) +MAX_BLOBS_PER_BLOCK: 1024 diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 359c7fc95..a123f921b 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -68,7 +68,7 @@ This upgrade adds blobs to the beacon chain as part of Deneb. This is an extensi | Name | Value | | - | - | -| `MAX_BLOBS_PER_BLOCK` | `uint64(2**2)` (= 4) | +| `MAX_BLOBS_PER_BLOCK` | `uint64(2**10)` (= 1024) | ## Configuration From 5e43f43df135a576cbdff51fa3aec6d85a08cf24 Mon Sep 17 00:00:00 2001 From: gajinder Date: Fri, 28 Apr 2023 10:55:35 +0530 Subject: [PATCH 13/39] update limit to 4844 friendly 16 blobs --- presets/mainnet/deneb.yaml | 4 ++-- specs/deneb/beacon-chain.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/presets/mainnet/deneb.yaml b/presets/mainnet/deneb.yaml index 7cf41bbf4..d5dafe0d0 100644 --- a/presets/mainnet/deneb.yaml +++ b/presets/mainnet/deneb.yaml @@ -4,5 +4,5 @@ # --------------------------------------------------------------- # `uint64(4096)` FIELD_ELEMENTS_PER_BLOB: 4096 -# `uint64(2**10)` (= 1024) -MAX_BLOBS_PER_BLOCK: 1024 +# `uint64(2**4)` (= 16) +MAX_BLOBS_PER_BLOCK: 16 diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index a123f921b..fc59e3be8 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -68,7 +68,7 @@ This upgrade adds blobs to the beacon chain as part of Deneb. This is an extensi | Name | Value | | - | - | -| `MAX_BLOBS_PER_BLOCK` | `uint64(2**10)` (= 1024) | +| `MAX_BLOBS_PER_BLOCK` | `uint64(2**4)` (= 16) | ## Configuration From 1aad9b5fa0dce3a0043710bd6595646260459fb2 Mon Sep 17 00:00:00 2001 From: gajinder Date: Sat, 6 May 2023 17:19:28 +0530 Subject: [PATCH 14/39] adding a fixed theoretical limit for commitments in a block --- specs/deneb/beacon-chain.md | 4 ++-- specs/deneb/p2p-interface.md | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index fc59e3be8..934a84013 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -68,7 +68,7 @@ This upgrade adds blobs to the beacon chain as part of Deneb. This is an extensi | Name | Value | | - | - | -| `MAX_BLOBS_PER_BLOCK` | `uint64(2**4)` (= 16) | +| `MAX_BLOB_COMMITMENTS_PER_BLOCK` | `uint64(2**10)` (= 1024) | hardfork independent fixed theoretical limit | ## Configuration @@ -96,7 +96,7 @@ class BeaconBlockBody(Container): # Execution execution_payload: ExecutionPayload # [Modified in Deneb] bls_to_execution_changes: List[SignedBLSToExecutionChange, MAX_BLS_TO_EXECUTION_CHANGES] - blob_kzg_commitments: List[KZGCommitment, MAX_BLOBS_PER_BLOCK] # [New in Deneb] + blob_kzg_commitments: List[KZGCommitment, MAX_BLOB_COMMITMENTS_PER_BLOCK] # [New in Deneb] ``` #### `ExecutionPayload` diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index d67c144b2..514991bb7 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -43,6 +43,7 @@ The specification of these changes continues in the same format as the network s | Name | Value | Description | |------------------------------------------|-----------------------------------|---------------------------------------------------------------------| | `MAX_REQUEST_BLOCKS_DENEB` | `2**7` (= 128) | Maximum number of blocks in a single request | +| `MAX_BLOBS_PER_BLOCK` | `2**4` (= 16) | Maximum number of blobs in a single request, to be always <= `MAX_BLOB_COMMITMENTS_PER_BLOCK` | | `MAX_REQUEST_BLOB_SIDECARS` | `MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK` | Maximum number of blob sidecars in a single request | | `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` | `2**12` (= 4096 epochs, ~18 days) | The minimum epoch range over which a node must serve blob sidecars | @@ -119,7 +120,7 @@ The *type* of the payload of this topic changes to the (modified) `SignedBeaconB ###### `blob_sidecar_{subnet_id}` -This topic is used to propagate signed blob sidecars, where each blob index maps to some `subnet_id`. +This topic is used to propagate signed blob sidecars, where each blob index maps to some `subnet_id`. However the actual blobs that will be generated by the EL will be limited by `MAX_DATA_GAS_PER_BLOCK // DATA_GAS_PER_BLOB` and will require an EL hardfork to upgrade the limit. The following validations MUST pass before forwarding the `signed_blob_sidecar` on the network, assuming the alias `sidecar = signed_blob_sidecar.message`: From 9f530a7741e220b2c2611452a98e8ee195e6f02c Mon Sep 17 00:00:00 2001 From: gajinder Date: Sun, 7 May 2023 19:52:59 +0530 Subject: [PATCH 15/39] update max commitments per block limit to blobs per tx limit from eip4844 --- specs/deneb/beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 934a84013..971b67f0e 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -68,7 +68,7 @@ This upgrade adds blobs to the beacon chain as part of Deneb. This is an extensi | Name | Value | | - | - | -| `MAX_BLOB_COMMITMENTS_PER_BLOCK` | `uint64(2**10)` (= 1024) | hardfork independent fixed theoretical limit | +| `MAX_BLOB_COMMITMENTS_PER_BLOCK` | `uint64(2**12)` (= 4096) | hardfork independent fixed theoretical limit same as `LIMIT_BLOBS_PER_TX` (see EIP 4844) | ## Configuration From a75292beebdea362b70d65e7ceb3b78ae03d5be4 Mon Sep 17 00:00:00 2001 From: gajinder Date: Thu, 11 May 2023 19:03:43 +0530 Subject: [PATCH 16/39] restore the 4844 max limit to 4 --- presets/mainnet/deneb.yaml | 4 ++-- specs/deneb/p2p-interface.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/presets/mainnet/deneb.yaml b/presets/mainnet/deneb.yaml index d5dafe0d0..ebe33f2d1 100644 --- a/presets/mainnet/deneb.yaml +++ b/presets/mainnet/deneb.yaml @@ -4,5 +4,5 @@ # --------------------------------------------------------------- # `uint64(4096)` FIELD_ELEMENTS_PER_BLOB: 4096 -# `uint64(2**4)` (= 16) -MAX_BLOBS_PER_BLOCK: 16 +# `uint64(2**2)` (= 4) +MAX_BLOBS_PER_BLOCK: 4 diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 514991bb7..4de7f4a83 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -43,7 +43,7 @@ The specification of these changes continues in the same format as the network s | Name | Value | Description | |------------------------------------------|-----------------------------------|---------------------------------------------------------------------| | `MAX_REQUEST_BLOCKS_DENEB` | `2**7` (= 128) | Maximum number of blocks in a single request | -| `MAX_BLOBS_PER_BLOCK` | `2**4` (= 16) | Maximum number of blobs in a single request, to be always <= `MAX_BLOB_COMMITMENTS_PER_BLOCK` | +| `MAX_BLOBS_PER_BLOCK` | `2**2` (= 4) | Maximum number of blobs in a single request, can never exceed `MAX_BLOB_COMMITMENTS_PER_BLOCK` limit | | `MAX_REQUEST_BLOB_SIDECARS` | `MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK` | Maximum number of blob sidecars in a single request | | `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` | `2**12` (= 4096 epochs, ~18 days) | The minimum epoch range over which a node must serve blob sidecars | From 8ccc2570d15774b474b2151337304be727750cf1 Mon Sep 17 00:00:00 2001 From: gajinder Date: Sat, 20 May 2023 19:35:22 +0530 Subject: [PATCH 17/39] apply feedback --- specs/deneb/beacon-chain.md | 3 +++ specs/deneb/p2p-interface.md | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 971b67f0e..1926cc8ce 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -40,6 +40,8 @@ This upgrade adds blobs to the beacon chain as part of Deneb. This is an extension of the Capella upgrade. +The blob transactions are packed into the execution payload by the EL/builder with their corresponding blobs being independently transmitted and are limited by `MAX_DATA_GAS_PER_BLOCK // DATA_GAS_PER_BLOB`. However the CL limit is independently defined by `MAX_BLOBS_PER_BLOCK`. + ## Custom types | Name | SSZ equivalent | Description | @@ -69,6 +71,7 @@ This upgrade adds blobs to the beacon chain as part of Deneb. This is an extensi | Name | Value | | - | - | | `MAX_BLOB_COMMITMENTS_PER_BLOCK` | `uint64(2**12)` (= 4096) | hardfork independent fixed theoretical limit same as `LIMIT_BLOBS_PER_TX` (see EIP 4844) | +| `MAX_BLOBS_PER_BLOCK` | `uint64(2**2)` (= 4) | Maximum number of blobs in a single block limited by `MAX_BLOB_COMMITMENTS_PER_BLOCK` | ## Configuration diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 4de7f4a83..d67c144b2 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -43,7 +43,6 @@ The specification of these changes continues in the same format as the network s | Name | Value | Description | |------------------------------------------|-----------------------------------|---------------------------------------------------------------------| | `MAX_REQUEST_BLOCKS_DENEB` | `2**7` (= 128) | Maximum number of blocks in a single request | -| `MAX_BLOBS_PER_BLOCK` | `2**2` (= 4) | Maximum number of blobs in a single request, can never exceed `MAX_BLOB_COMMITMENTS_PER_BLOCK` limit | | `MAX_REQUEST_BLOB_SIDECARS` | `MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK` | Maximum number of blob sidecars in a single request | | `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` | `2**12` (= 4096 epochs, ~18 days) | The minimum epoch range over which a node must serve blob sidecars | @@ -120,7 +119,7 @@ The *type* of the payload of this topic changes to the (modified) `SignedBeaconB ###### `blob_sidecar_{subnet_id}` -This topic is used to propagate signed blob sidecars, where each blob index maps to some `subnet_id`. However the actual blobs that will be generated by the EL will be limited by `MAX_DATA_GAS_PER_BLOCK // DATA_GAS_PER_BLOB` and will require an EL hardfork to upgrade the limit. +This topic is used to propagate signed blob sidecars, where each blob index maps to some `subnet_id`. The following validations MUST pass before forwarding the `signed_blob_sidecar` on the network, assuming the alias `sidecar = signed_blob_sidecar.message`: From 4458645f0cffc65133618690ff97ace401f83f27 Mon Sep 17 00:00:00 2001 From: gajinder Date: Mon, 22 May 2023 18:41:40 +0530 Subject: [PATCH 18/39] add check --- specs/deneb/beacon-chain.md | 1 + 1 file changed, 1 insertion(+) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 1926cc8ce..b2f6b1763 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -254,6 +254,7 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe ```python def process_blob_kzg_commitments(body: BeaconBlockBody) -> None: + assert len(body.blob_kzg_commitments) <= MAX_BLOBS_PER_BLOCK assert verify_kzg_commitments_against_transactions(body.execution_payload.transactions, body.blob_kzg_commitments) ``` From feb1968e433daa1a4c540d9226a98c6c4f9b238b Mon Sep 17 00:00:00 2001 From: gajinder Date: Mon, 22 May 2023 18:44:12 +0530 Subject: [PATCH 19/39] add comment --- specs/deneb/beacon-chain.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index b2f6b1763..f492a948c 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -254,7 +254,9 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe ```python def process_blob_kzg_commitments(body: BeaconBlockBody) -> None: + # Verify commitments are under limit assert len(body.blob_kzg_commitments) <= MAX_BLOBS_PER_BLOCK + # Verify commitments match with corresponding blob transactions assert verify_kzg_commitments_against_transactions(body.execution_payload.transactions, body.blob_kzg_commitments) ``` From 3247bcf8f721cef4b88deac6b3e22c18fbf815eb Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 23 May 2023 15:16:12 +0800 Subject: [PATCH 20/39] PR feedback from @ppopth --- specs/bellatrix/beacon-chain.md | 2 +- specs/deneb/beacon-chain.md | 17 +---------------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/specs/bellatrix/beacon-chain.md b/specs/bellatrix/beacon-chain.md index a4c7be16e..9df07d69f 100644 --- a/specs/bellatrix/beacon-chain.md +++ b/specs/bellatrix/beacon-chain.md @@ -330,7 +330,7 @@ The Engine API may be used to implement this and similarly defined functions via ```python def notify_new_payload(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: """ - Return ``True`` if and only if ``execution_payload`` is valid with respect to ``self.execution_state``. + Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``. """ ... ``` diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 2c4bb6133..20455ab0d 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -30,7 +30,6 @@ - [Modified `NewPayloadRequest`](#modified-newpayloadrequest) - [Engine APIs](#engine-apis) - [Modified `notify_new_payload`](#modified-notify_new_payload) - - [Block processing](#block-processing) - [Execution payload](#execution-payload) - [`process_execution_payload`](#process_execution_payload) - [Testing](#testing) @@ -182,25 +181,11 @@ class NewPayloadRequest(object): ```python def notify_new_payload(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: """ - Return ``True`` if and only if ``execution_payload`` is valid with respect to ``self.execution_state``. + Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``. """ ... ``` -### Block processing - -```python -def process_block(state: BeaconState, block: BeaconBlock) -> None: - process_block_header(state, block) - if is_execution_enabled(state, block.body): - process_withdrawals(state, block.body.execution_payload) - process_execution_payload(state, block.body, EXECUTION_ENGINE) # [Modified in Deneb] - process_randao(state, block.body) - process_eth1_data(state, block.body) - process_operations(state, block.body) - process_sync_aggregate(state, block.body.sync_aggregate) -``` - #### Execution payload ##### `process_execution_payload` From fc45220a7d1ee5bc75be4c5a0a834512f812f8f1 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 23 May 2023 15:38:54 +0800 Subject: [PATCH 21/39] Move old Deneb sanity tests to block_processing (operations) tests --- .../test/deneb/block_processing/__init__.py | 0 .../test_process_execution_payload.py | 175 ++++++++++++++++++ .../eth2spec/test/deneb/sanity/test_blocks.py | 138 -------------- tests/generators/operations/main.py | 5 +- 4 files changed, 179 insertions(+), 139 deletions(-) create mode 100644 tests/core/pyspec/eth2spec/test/deneb/block_processing/__init__.py create mode 100644 tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py diff --git a/tests/core/pyspec/eth2spec/test/deneb/block_processing/__init__.py b/tests/core/pyspec/eth2spec/test/deneb/block_processing/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py new file mode 100644 index 000000000..68da478b7 --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py @@ -0,0 +1,175 @@ +from random import Random + +from eth2spec.test.helpers.execution_payload import ( + build_empty_execution_payload, + compute_el_block_hash, + get_execution_payload_header, +) +from eth2spec.test.context import ( + spec_state_test, + expect_assertion_error, + with_deneb_and_later +) +from eth2spec.test.helpers.sharding import ( + get_sample_opaque_tx, +) + + +def run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments, + valid=True, execution_valid=True): + """ + Run ``process_execution_payload``, yielding: + - pre-state ('pre') + - execution payload ('execution_payload') + - execution details, to mock EVM execution ('execution.yml', a dict with 'execution_valid' key and boolean value) + - post-state ('post'). + If ``valid == False``, run expecting ``AssertionError`` + """ + # Before Deneb, only `body.execution_payload` matters. `BeaconBlockBody` is just a wrapper. + body = spec.BeaconBlockBody( + blob_kzg_commitments=blob_kzg_commitments, + execution_payload=execution_payload + ) + + yield 'pre', state + yield 'execution', {'execution_valid': execution_valid} + yield 'body', body + + called_new_block = False + + class TestEngine(spec.NoopExecutionEngine): + def notify_new_payload(self, new_payload_request) -> bool: + nonlocal called_new_block, execution_valid + called_new_block = True + assert new_payload_request.execution_payload == body.execution_payload + return execution_valid + + if not valid: + expect_assertion_error(lambda: spec.process_execution_payload(state, body, TestEngine())) + yield 'post', None + return + + spec.process_execution_payload(state, body, TestEngine()) + + # Make sure we called the engine + assert called_new_block + + yield 'post', state + + assert state.latest_execution_payload_header == get_execution_payload_header(spec, body.execution_payload) + + +@with_deneb_and_later +@spec_state_test +def test_incorrect_blob_tx_type(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ + execution_payload = build_empty_execution_payload(spec, state) + + opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) + opaque_tx = b'\x04' + opaque_tx[1:] # incorrect tx type + + execution_payload.transactions = [opaque_tx] + execution_payload.block_hash = compute_el_block_hash(spec, execution_payload) + + yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments) + + +@with_deneb_and_later +@spec_state_test +def test_incorrect_transaction_length_1_byte(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ + execution_payload = build_empty_execution_payload(spec, state) + + opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) + opaque_tx = opaque_tx + b'\x12' # incorrect tx length + + execution_payload.transactions = [opaque_tx] + execution_payload.block_hash = compute_el_block_hash(spec, execution_payload) + + yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments) + + +@with_deneb_and_later +@spec_state_test +def test_incorrect_transaction_length_32_bytes(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ + execution_payload = build_empty_execution_payload(spec, state) + + opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) + opaque_tx = opaque_tx + b'\x12' * 32 # incorrect tx length + + execution_payload.transactions = [opaque_tx] + execution_payload.block_hash = compute_el_block_hash(spec, execution_payload) + + yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments) + + +@with_deneb_and_later +@spec_state_test +def test_incorrect_commitment(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ + execution_payload = build_empty_execution_payload(spec, state) + + opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) + blob_kzg_commitments[0] = b'\x12' * 48 # incorrect commitment + + execution_payload.transactions = [opaque_tx] + execution_payload.block_hash = compute_el_block_hash(spec, execution_payload) + + yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments) + + +@with_deneb_and_later +@spec_state_test +def test_incorrect_commitments_order(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ + execution_payload = build_empty_execution_payload(spec, state) + + opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec, blob_count=2, rng=Random(1111)) + blob_kzg_commitments = [blob_kzg_commitments[1], blob_kzg_commitments[0]] # incorrect order + + execution_payload.transactions = [opaque_tx] + execution_payload.block_hash = compute_el_block_hash(spec, execution_payload) + + yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments) + + +@with_deneb_and_later +@spec_state_test +def test_incorrect_block_hash(spec, state): + execution_payload = build_empty_execution_payload(spec, state) + + opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) + + execution_payload.transactions = [opaque_tx] + execution_payload.block_hash = b'\x12' * 32 # incorrect block hash + + # CL itself doesn't verify EL block hash + yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments) + + +@with_deneb_and_later +@spec_state_test +def test_zeroed_commitment(spec, state): + """ + The blob is invalid, but the commitment is in correct form. + """ + execution_payload = build_empty_execution_payload(spec, state) + + opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec, blob_count=1, is_valid_blob=False) + assert all(commitment == b'\x00' * 48 for commitment in blob_kzg_commitments) + + execution_payload.transactions = [opaque_tx] + execution_payload.block_hash = compute_el_block_hash(spec, execution_payload) + + yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments) diff --git a/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py index f65514f23..36caacd3f 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py @@ -1,5 +1,3 @@ -import random - from eth2spec.test.helpers.state import ( state_transition_and_sign_block ) @@ -49,139 +47,3 @@ def test_one_blob(spec, state): @spec_state_test def test_max_blobs(spec, state): yield from run_block_with_blobs(spec, state, blob_count=spec.MAX_BLOBS_PER_BLOCK) - - -@with_deneb_and_later -@spec_state_test -def test_incorrect_blob_tx_type(spec, state): - """ - The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. - """ - yield 'pre', state - - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) - block.body.blob_kzg_commitments = blob_kzg_commitments - opaque_tx = b'\x04' + opaque_tx[1:] # incorrect tx type - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block) - - yield 'blocks', [signed_block] - yield 'post', state - - -@with_deneb_and_later -@spec_state_test -def test_incorrect_transaction_length_1_byte(spec, state): - """ - The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. - """ - yield 'pre', state - - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) - block.body.blob_kzg_commitments = blob_kzg_commitments - opaque_tx = opaque_tx + b'\x12' # incorrect tx length - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block) - - yield 'blocks', [signed_block] - yield 'post', state - - -@with_deneb_and_later -@spec_state_test -def test_incorrect_transaction_length_32_bytes(spec, state): - """ - The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. - """ - yield 'pre', state - - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) - block.body.blob_kzg_commitments = blob_kzg_commitments - opaque_tx = opaque_tx + b'\x12' * 32 # incorrect tx length - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block) - - yield 'blocks', [signed_block] - yield 'post', state - - -@with_deneb_and_later -@spec_state_test -def test_incorrect_commitment(spec, state): - """ - The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. - """ - yield 'pre', state - - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) - blob_kzg_commitments[0] = b'\x12' * 48 # incorrect commitment - block.body.blob_kzg_commitments = blob_kzg_commitments - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block) - - yield 'blocks', [signed_block] - yield 'post', state - - -@with_deneb_and_later -@spec_state_test -def test_incorrect_commitments_order(spec, state): - """ - The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. - """ - yield 'pre', state - - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec, blob_count=2, rng=random.Random(1111)) - block.body.blob_kzg_commitments = [blob_kzg_commitments[1], blob_kzg_commitments[0]] # incorrect order - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block) - - yield 'blocks', [signed_block] - yield 'post', state - - -@with_deneb_and_later -@spec_state_test -def test_incorrect_block_hash(spec, state): - yield 'pre', state - - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) - block.body.blob_kzg_commitments = blob_kzg_commitments - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = b'\x12' * 32 # incorrect block hash - # CL itself doesn't verify EL block hash - signed_block = state_transition_and_sign_block(spec, state, block) - - yield 'blocks', [signed_block] - yield 'post', state - - -@with_deneb_and_later -@spec_state_test -def test_zeroed_commitment(spec, state): - """ - The blob is invalid, but the commitment is in correct form. - """ - yield 'pre', state - - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec, blob_count=1, is_valid_blob=False) - assert all(commitment == b'\x00' * 48 for commitment in blob_kzg_commitments) - block.body.blob_kzg_commitments = blob_kzg_commitments - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block) - - yield 'blocks', [signed_block] - yield 'post', state diff --git a/tests/generators/operations/main.py b/tests/generators/operations/main.py index fc2217917..293bcdea7 100644 --- a/tests/generators/operations/main.py +++ b/tests/generators/operations/main.py @@ -36,7 +36,10 @@ if __name__ == "__main__": ]} capella_mods = combine_mods(_new_capella_mods, bellatrix_mods) - deneb_mods = capella_mods + _new_deneb_mods = {key: 'eth2spec.test.deneb.block_processing.test_process_' + key for key in [ + 'execution_payload', + ]} + deneb_mods = combine_mods(_new_deneb_mods, capella_mods) _new_eip6110_mods = {key: 'eth2spec.test.eip6110.block_processing.test_process_' + key for key in [ 'deposit_receipt', From bee8fa16e567cec88f68372a3768e4fec476a6b2 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 23 May 2023 18:34:00 +0800 Subject: [PATCH 22/39] Add test_invalid_correct_input__execution_invalid --- .../test_process_execution_payload.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py index 68da478b7..d3a04ddfe 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py @@ -173,3 +173,20 @@ def test_zeroed_commitment(spec, state): execution_payload.block_hash = compute_el_block_hash(spec, execution_payload) yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments) + + +@with_deneb_and_later +@spec_state_test +def test_invalid_correct_input__execution_invalid(spec, state): + """ + The versioned hashes are wrong, but the testing ExecutionEngine returns VALID by default. + """ + execution_payload = build_empty_execution_payload(spec, state) + + opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec) + + execution_payload.transactions = [opaque_tx] + execution_payload.block_hash = compute_el_block_hash(spec, execution_payload) + + yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments, + valid=False, execution_valid=False) From 73df1935b1dbc09036f793e2759e3c4f794fc04b Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 23 May 2023 22:25:43 +0800 Subject: [PATCH 23/39] Use `verify_and_notify_new_payload` approach --- setup.py | 58 ++++++++++++++++++- specs/_features/eip6110/beacon-chain.md | 2 +- specs/bellatrix/beacon-chain.md | 6 +- specs/capella/beacon-chain.md | 2 +- specs/deneb/beacon-chain.md | 40 +++++++++++-- .../test_process_execution_payload.py | 8 ++- .../test_process_execution_payload.py | 2 +- 7 files changed, 104 insertions(+), 14 deletions(-) diff --git a/setup.py b/setup.py index e8bbeefbb..609d2acf3 100644 --- a/setup.py +++ b/setup.py @@ -336,6 +336,10 @@ class SpecBuilder(ABC): """ raise NotImplementedError() + @classmethod + def execution_engine_cls(cls) -> str: + raise NotImplementedError() + @classmethod @abstractmethod def hardcoded_ssz_dep_constants(cls) -> Dict[str, str]: @@ -469,6 +473,12 @@ get_attesting_indices = cache_this( ), _get_attesting_indices, lru_size=SLOTS_PER_EPOCH * MAX_COMMITTEES_PER_SLOT * 3)''' + + @classmethod + def execution_engine_cls(cls) -> str: + return "" + + @classmethod def hardcoded_ssz_dep_constants(cls) -> Dict[str, str]: return {} @@ -573,12 +583,14 @@ def get_execution_state(_execution_state_root: Bytes32) -> ExecutionState: def get_pow_chain_head() -> PowBlock: - pass - + pass""" + @classmethod + def execution_engine_cls(cls) -> str: + return "\n\n" + """ class NoopExecutionEngine(ExecutionEngine): - def notify_new_payload(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: + def notify_new_payload(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: return True def notify_forkchoice_updated(self: ExecutionEngine, @@ -658,6 +670,39 @@ def retrieve_blobs_and_proofs(beacon_block_root: Root) -> PyUnion[Tuple[Blob, KZ # pylint: disable=unused-argument return ("TEST", "TEST")''' + @classmethod + def execution_engine_cls(cls) -> str: + return "\n\n" + """ +class NoopExecutionEngine(ExecutionEngine): + + def notify_new_payload(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: + return True + + def notify_forkchoice_updated(self: ExecutionEngine, + head_block_hash: Hash32, + safe_block_hash: Hash32, + finalized_block_hash: Hash32, + payload_attributes: Optional[PayloadAttributes]) -> Optional[PayloadId]: + pass + + def get_payload(self: ExecutionEngine, payload_id: PayloadId) -> GetPayloadResponse: + # pylint: disable=unused-argument + raise NotImplementedError("no default block production") + + def is_valid_block_hash(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: + return True + + def is_valid_versioned_hashes(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: + return True + + def verify_and_notify_new_payload(self: ExecutionEngine, + new_payload_request: NewPayloadRequest) -> bool: + return True + + +EXECUTION_ENGINE = NoopExecutionEngine()""" + + @classmethod def hardcoded_custom_type_dep_constants(cls, spec_object) -> str: constants = { @@ -708,6 +753,12 @@ def objects_to_spec(preset_name: str, ) def format_protocol(protocol_name: str, protocol_def: ProtocolDefinition) -> str: + if "verify_and_notify_new_payload" in protocol_def.functions: + # del protocol_def.functions['verify_and_notify_new_payload'] + protocol_def.functions['verify_and_notify_new_payload'] = """def verify_and_notify_new_payload(self: ExecutionEngine, + new_payload_request: NewPayloadRequest) -> bool: + ...""" + protocol = f"class {protocol_name}(Protocol):" for fn_source in protocol_def.functions.values(): fn_source = fn_source.replace("self: "+protocol_name, "self") @@ -783,6 +834,7 @@ def objects_to_spec(preset_name: str, + ('\n\n\n' + protocols_spec if protocols_spec != '' else '') + '\n\n\n' + functions_spec + '\n\n' + builder.sundry_functions() + + builder.execution_engine_cls() # Since some constants are hardcoded in setup.py, the following assertions verify that the hardcoded constants are # as same as the spec definition. + ('\n\n\n' + ssz_dep_constants_verification if ssz_dep_constants_verification != '' else '') diff --git a/specs/_features/eip6110/beacon-chain.md b/specs/_features/eip6110/beacon-chain.md index 591d8dc86..084f88f14 100644 --- a/specs/_features/eip6110/beacon-chain.md +++ b/specs/_features/eip6110/beacon-chain.md @@ -249,7 +249,7 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid versioned_hashes = [kzg_commitment_to_versioned_hash(commitment) for commitment in body.blob_kzg_commitments] - assert execution_engine.notify_new_payload( + assert execution_engine.verify_and_notify_new_payload( NewPayloadRequest(execution_payload=payload, versioned_hashes=versioned_hashes) ) # Cache execution payload header diff --git a/specs/bellatrix/beacon-chain.md b/specs/bellatrix/beacon-chain.md index 9df07d69f..9ae1e5a71 100644 --- a/specs/bellatrix/beacon-chain.md +++ b/specs/bellatrix/beacon-chain.md @@ -328,9 +328,9 @@ The Engine API may be used to implement this and similarly defined functions via #### `notify_new_payload` ```python -def notify_new_payload(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: +def notify_new_payload(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: """ - Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``. + Return ``True`` if and only if ``execution_payload`` is valid with respect to ``self.execution_state``. """ ... ``` @@ -366,7 +366,7 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi # Verify timestamp assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid - assert execution_engine.notify_new_payload(NewPayloadRequest(execution_payload=payload)) + assert execution_engine.notify_new_payload(payload) # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( parent_hash=payload.parent_hash, diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 1799f4e6e..c8e969d25 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -418,7 +418,7 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi # Verify timestamp assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid - assert execution_engine.notify_new_payload(NewPayloadRequest(execution_payload=payload)) + assert execution_engine.notify_new_payload(payload) # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( parent_hash=payload.parent_hash, diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 20455ab0d..bed64db51 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -29,7 +29,9 @@ - [Request data](#request-data) - [Modified `NewPayloadRequest`](#modified-newpayloadrequest) - [Engine APIs](#engine-apis) - - [Modified `notify_new_payload`](#modified-notify_new_payload) + - [`is_valid_block_hash`](#is_valid_block_hash) + - [`is_valid_versioned_hashes`](#is_valid_versioned_hashes) + - [`verify_and_notify_new_payload`](#verify_and_notify_new_payload) - [Execution payload](#execution-payload) - [`process_execution_payload`](#process_execution_payload) - [Testing](#testing) @@ -176,14 +178,44 @@ class NewPayloadRequest(object): #### Engine APIs -#### Modified `notify_new_payload` +#### `is_valid_block_hash` + +[0]: # (eth2spec: skip) ```python -def notify_new_payload(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: +def is_valid_block_hash(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: + """ + Return ``True`` if and only if ``execution_payload.block_hash`` is computed correctly. + """ + ... +``` + +#### `is_valid_versioned_hashes` + +[0]: # (eth2spec: skip) + +```python +def is_valid_versioned_hashes(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: + """ + Return ``True`` if and only if the version hashes computed by the blob transactions of + ``new_payload_request.execution_payload`` matches ``new_payload_request.version_hashes``. + """ + ... +``` + +#### `verify_and_notify_new_payload` + +```python +def verify_and_notify_new_payload(self: ExecutionEngine, + new_payload_request: NewPayloadRequest) -> bool: """ Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``. """ + assert self.is_valid_block_hash(new_payload_request.execution_payload) + assert self.is_valid_versioned_hashes(new_payload_request) + assert self.notify_new_payload(new_payload_request.execution_payload) ... + return True ``` #### Execution payload @@ -204,7 +236,7 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi # Verify the execution payload is valid # [Modified in Deneb] versioned_hashes = [kzg_commitment_to_versioned_hash(commitment) for commitment in body.blob_kzg_commitments] - assert execution_engine.notify_new_payload( + assert execution_engine.verify_and_notify_new_payload( NewPayloadRequest(execution_payload=payload, versioned_hashes=versioned_hashes) ) diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py index b83563235..b54afafc3 100644 --- a/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py @@ -31,7 +31,13 @@ def run_execution_payload_processing(spec, state, execution_payload, valid=True, called_new_block = False class TestEngine(spec.NoopExecutionEngine): - def notify_new_payload(self, new_payload_request) -> bool: + def notify_new_payload(self, payload) -> bool: + nonlocal called_new_block, execution_valid + called_new_block = True + assert payload == execution_payload + return execution_valid + + def verify_and_notify_new_payload(self, new_payload_request) -> bool: nonlocal called_new_block, execution_valid called_new_block = True assert new_payload_request.execution_payload == body.execution_payload diff --git a/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py index d3a04ddfe..df59385ab 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py @@ -38,7 +38,7 @@ def run_execution_payload_processing(spec, state, execution_payload, blob_kzg_co called_new_block = False class TestEngine(spec.NoopExecutionEngine): - def notify_new_payload(self, new_payload_request) -> bool: + def verify_and_notify_new_payload(self, new_payload_request) -> bool: nonlocal called_new_block, execution_valid called_new_block = True assert new_payload_request.execution_payload == body.execution_payload From 7ec5efb10666af3ccff9a3a84cbe46ba6578ac1a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 23 May 2023 23:55:35 +0800 Subject: [PATCH 24/39] Add `### Block processing` back --- specs/deneb/beacon-chain.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 20455ab0d..548d493cc 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -30,6 +30,7 @@ - [Modified `NewPayloadRequest`](#modified-newpayloadrequest) - [Engine APIs](#engine-apis) - [Modified `notify_new_payload`](#modified-notify_new_payload) + - [Block processing](#block-processing) - [Execution payload](#execution-payload) - [`process_execution_payload`](#process_execution_payload) - [Testing](#testing) @@ -186,6 +187,8 @@ def notify_new_payload(self: ExecutionEngine, new_payload_request: NewPayloadReq ... ``` +### Block processing + #### Execution payload ##### `process_execution_payload` From 48618fe866383c07f4fc814b819af72bdff8b4b7 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 00:08:45 +0800 Subject: [PATCH 25/39] Fix tests --- .../test_process_execution_payload.py | 10 ++++++-- .../test_process_execution_payload.py | 24 +++++++++++++++++++ tests/generators/operations/main.py | 3 ++- 3 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_execution_payload.py diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py index 3ec58b31e..d960ceb0a 100644 --- a/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py @@ -8,7 +8,13 @@ from eth2spec.test.helpers.execution_payload import ( build_state_with_incomplete_transition, build_state_with_complete_transition, ) -from eth2spec.test.context import spec_state_test, expect_assertion_error, with_bellatrix_and_later +from eth2spec.test.context import ( + BELLATRIX, + expect_assertion_error, + spec_state_test, + with_bellatrix_and_later, + with_phases, +) from eth2spec.test.helpers.state import next_slot @@ -117,7 +123,7 @@ def test_invalid_bad_execution_regular_payload(spec, state): yield from run_bad_execution_test(spec, state) -@with_bellatrix_and_later +@with_phases([BELLATRIX]) @spec_state_test def test_bad_parent_hash_first_payload(spec, state): state = build_state_with_incomplete_transition(spec, state) diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_execution_payload.py new file mode 100644 index 000000000..1a2c1771a --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_execution_payload.py @@ -0,0 +1,24 @@ +from eth2spec.test.helpers.execution_payload import ( + build_empty_execution_payload, + compute_el_block_hash, + build_state_with_incomplete_transition, +) +from eth2spec.test.context import ( + spec_state_test, + with_capella_and_later, +) +from eth2spec.test.helpers.state import next_slot +from eth2spec.test.bellatrix.block_processing.test_process_execution_payload import run_execution_payload_processing + + +@with_capella_and_later +@spec_state_test +def test_invalid_bad_parent_hash_first_payload(spec, state): + state = build_state_with_incomplete_transition(spec, state) + next_slot(spec, state) + + execution_payload = build_empty_execution_payload(spec, state) + execution_payload.parent_hash = b'\x55' * 32 + execution_payload.block_hash = compute_el_block_hash(spec, execution_payload) + + yield from run_execution_payload_processing(spec, state, execution_payload, valid=False) diff --git a/tests/generators/operations/main.py b/tests/generators/operations/main.py index fc2217917..3983931b3 100644 --- a/tests/generators/operations/main.py +++ b/tests/generators/operations/main.py @@ -30,8 +30,9 @@ if __name__ == "__main__": bellatrix_mods = combine_mods(_new_bellatrix_mods, altair_mods) _new_capella_mods = {key: 'eth2spec.test.capella.block_processing.test_process_' + key for key in [ - 'deposit', 'bls_to_execution_change', + 'deposit', + 'execution_payload', 'withdrawals', ]} capella_mods = combine_mods(_new_capella_mods, bellatrix_mods) From dd5e6f813f82cd20636dfb36950ec281a428d48f Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 01:58:49 +0800 Subject: [PATCH 26/39] Add `make_function_abstract` to make it more general --- setup.py | 14 +++++++++----- specs/deneb/beacon-chain.md | 4 ---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/setup.py b/setup.py index 609d2acf3..22a05ee32 100644 --- a/setup.py +++ b/setup.py @@ -736,6 +736,11 @@ def is_byte_vector(value: str) -> bool: return value.startswith(('ByteVector')) +def make_function_abstract(protocol_def: ProtocolDefinition, key: str): + function = protocol_def.functions[key].split('"""') + protocol_def.functions[key] = function[0] + "..." + + def objects_to_spec(preset_name: str, spec_object: SpecObject, builder: SpecBuilder, @@ -753,11 +758,10 @@ def objects_to_spec(preset_name: str, ) def format_protocol(protocol_name: str, protocol_def: ProtocolDefinition) -> str: - if "verify_and_notify_new_payload" in protocol_def.functions: - # del protocol_def.functions['verify_and_notify_new_payload'] - protocol_def.functions['verify_and_notify_new_payload'] = """def verify_and_notify_new_payload(self: ExecutionEngine, - new_payload_request: NewPayloadRequest) -> bool: - ...""" + abstract_functions = ["verify_and_notify_new_payload"] + for key in protocol_def.functions.keys(): + if key in abstract_functions: + make_function_abstract(protocol_def, key) protocol = f"class {protocol_name}(Protocol):" for fn_source in protocol_def.functions.values(): diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index bed64db51..615af60d4 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -180,8 +180,6 @@ class NewPayloadRequest(object): #### `is_valid_block_hash` -[0]: # (eth2spec: skip) - ```python def is_valid_block_hash(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: """ @@ -192,8 +190,6 @@ def is_valid_block_hash(self: ExecutionEngine, execution_payload: ExecutionPaylo #### `is_valid_versioned_hashes` -[0]: # (eth2spec: skip) - ```python def is_valid_versioned_hashes(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: """ From 68ce45b3058186ff9e8bdecb19d51327851a4259 Mon Sep 17 00:00:00 2001 From: djrtwo Date: Tue, 23 May 2023 13:52:36 -0600 Subject: [PATCH 27/39] move epoch subscription length to config --- configs/mainnet.yaml | 2 ++ configs/minimal.yaml | 2 ++ presets/mainnet/phase0.yaml | 5 ----- presets/minimal/phase0.yaml | 5 ----- specs/phase0/p2p-interface.md | 7 +------ 5 files changed, 5 insertions(+), 16 deletions(-) diff --git a/configs/mainnet.yaml b/configs/mainnet.yaml index 1a1537b26..baf489739 100644 --- a/configs/mainnet.yaml +++ b/configs/mainnet.yaml @@ -102,6 +102,8 @@ DEPOSIT_CONTRACT_ADDRESS: 0x00000000219ab540356cBB839Cbe05303d7705Fa GOSSIP_MAX_SIZE: 1048576 # `2**10` (= 1024) MAX_REQUEST_BLOCKS: 1024 +# `2**8` (= 256) +EPOCHS_PER_SUBNET_SUBSCRIPTION: 256 # `MIN_VALIDATOR_WITHDRAWABILITY_DELAY + CHURN_LIMIT_QUOTIENT // 2` (= 33024, ~5 months) MIN_EPOCHS_FOR_BLOCK_REQUESTS: 33024 # `2**20` (=1048576, 1 MiB) diff --git a/configs/minimal.yaml b/configs/minimal.yaml index 32d5682a8..43f1fc83f 100644 --- a/configs/minimal.yaml +++ b/configs/minimal.yaml @@ -103,6 +103,8 @@ DEPOSIT_CONTRACT_ADDRESS: 0x1234567890123456789012345678901234567890 GOSSIP_MAX_SIZE: 1048576 # `2**10` (= 1024) MAX_REQUEST_BLOCKS: 1024 +# `2**8` (= 256) +EPOCHS_PER_SUBNET_SUBSCRIPTION: 256 # [customized] `MIN_VALIDATOR_WITHDRAWABILITY_DELAY + CHURN_LIMIT_QUOTIENT // 2` (= 272) MIN_EPOCHS_FOR_BLOCK_REQUESTS: 272 # `2**20` (=1048576, 1 MiB) diff --git a/presets/mainnet/phase0.yaml b/presets/mainnet/phase0.yaml index c84a93595..02bc96c8c 100644 --- a/presets/mainnet/phase0.yaml +++ b/presets/mainnet/phase0.yaml @@ -86,8 +86,3 @@ MAX_ATTESTATIONS: 128 MAX_DEPOSITS: 16 # 2**4 (= 16) MAX_VOLUNTARY_EXITS: 16 - -# Networking -# --------------------------------------------------------------- -# 2**8 (= 256) -EPOCHS_PER_SUBNET_SUBSCRIPTION: 256 diff --git a/presets/minimal/phase0.yaml b/presets/minimal/phase0.yaml index c55f7de7b..e7028f5a4 100644 --- a/presets/minimal/phase0.yaml +++ b/presets/minimal/phase0.yaml @@ -86,8 +86,3 @@ MAX_ATTESTATIONS: 128 MAX_DEPOSITS: 16 # 2**4 (= 16) MAX_VOLUNTARY_EXITS: 16 - -# Networking -# --------------------------------------------------------------- -# 2**8 (= 256) -EPOCHS_PER_SUBNET_SUBSCRIPTION: 256 diff --git a/specs/phase0/p2p-interface.md b/specs/phase0/p2p-interface.md index 3b8a91cc9..de3e0e529 100644 --- a/specs/phase0/p2p-interface.md +++ b/specs/phase0/p2p-interface.md @@ -22,7 +22,6 @@ It consists of four main sections: - [Consensus-layer network interaction domains](#consensus-layer-network-interaction-domains) - [Custom types](#custom-types) - [Constants](#constants) - - [Preset](#preset) - [Configuration](#configuration) - [MetaData](#metadata) - [The gossip domain: gossipsub](#the-gossip-domain-gossipsub) @@ -184,11 +183,6 @@ We define the following Python custom types for type hinting and readability: | - | - | :-: | :-: | | `NODE_ID_BITS` | `256` | The bit length of uint256 is 256 | -### Preset -| Name | Value | Unit | Duration | -| - | - | :-: | :-: | -| `EPOCHS_PER_SUBNET_SUBSCRIPTION` | `2**8` (= 256) | epochs | ~27 hours | - ### Configuration This section outlines configurations that are used in this spec. @@ -197,6 +191,7 @@ This section outlines configurations that are used in this spec. |---|---|---| | `GOSSIP_MAX_SIZE` | `2**20` (= 1048576, 1 MiB) | The maximum allowed size of uncompressed gossip messages. | | `MAX_REQUEST_BLOCKS` | `2**10` (= 1024) | Maximum number of blocks in a single request | +| `EPOCHS_PER_SUBNET_SUBSCRIPTION` | `2**8` (= 256) | Number of epochs on a subnet subscription (~27 hours) | | `MIN_EPOCHS_FOR_BLOCK_REQUESTS` | `MIN_VALIDATOR_WITHDRAWABILITY_DELAY + CHURN_LIMIT_QUOTIENT // 2` (= 33024, ~5 months) | The minimum epoch range over which a node must serve blocks | | `MAX_CHUNK_SIZE` | `2**20` (=1048576, 1 MiB) | The maximum allowed size of uncompressed req/resp chunked responses. | | `TTFB_TIMEOUT` | `5` | The maximum duration in **seconds** to wait for first byte of request response (time-to-first-byte). | From 1f75acadd979734988b07c018ce6d454ccf8d0e9 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 23 May 2023 14:10:35 -0600 Subject: [PATCH 28/39] Update VERSION.txt to 1.4.0-alpha.0 --- tests/core/pyspec/eth2spec/VERSION.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/VERSION.txt b/tests/core/pyspec/eth2spec/VERSION.txt index f0bb29e76..779260991 100644 --- a/tests/core/pyspec/eth2spec/VERSION.txt +++ b/tests/core/pyspec/eth2spec/VERSION.txt @@ -1 +1 @@ -1.3.0 +1.4.0-alpha.0 From 289d8147be38438fac7b196622e2a133b9f2adf9 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 11:12:03 +0800 Subject: [PATCH 29/39] Move `verify_and_notify_new_payload` to Bellatrix --- setup.py | 7 ++++ specs/bellatrix/beacon-chain.md | 32 ++++++++++++++++--- specs/capella/beacon-chain.md | 2 +- specs/deneb/beacon-chain.md | 14 ++------ sync/optimistic.md | 4 +-- .../test_process_execution_payload.py | 6 ---- .../eth2spec/test/helpers/optimistic_sync.py | 2 +- 7 files changed, 41 insertions(+), 26 deletions(-) diff --git a/setup.py b/setup.py index 22a05ee32..12a561514 100644 --- a/setup.py +++ b/setup.py @@ -604,6 +604,13 @@ class NoopExecutionEngine(ExecutionEngine): # pylint: disable=unused-argument raise NotImplementedError("no default block production") + def is_valid_block_hash(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: + return True + + def verify_and_notify_new_payload(self: ExecutionEngine, + new_payload_request: NewPayloadRequest) -> bool: + return True + EXECUTION_ENGINE = NoopExecutionEngine()""" diff --git a/specs/bellatrix/beacon-chain.md b/specs/bellatrix/beacon-chain.md index 9ae1e5a71..e8eef346d 100644 --- a/specs/bellatrix/beacon-chain.md +++ b/specs/bellatrix/beacon-chain.md @@ -320,13 +320,13 @@ The implementation-dependent `ExecutionEngine` protocol encapsulates the executi * a state object `self.execution_state` of type `ExecutionState` * a notification function `self.notify_new_payload` which may apply changes to the `self.execution_state` -*Note*: `notify_new_payload` is a function accessed through the `EXECUTION_ENGINE` module which instantiates the `ExecutionEngine` protocol. - -The body of this function is implementation dependent. +The body of these functions are implementation dependent. The Engine API may be used to implement this and similarly defined functions via an external execution engine. #### `notify_new_payload` +`notify_new_payload` is a function accessed through the `EXECUTION_ENGINE` module which instantiates the `ExecutionEngine` protocol. + ```python def notify_new_payload(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: """ @@ -335,6 +335,30 @@ def notify_new_payload(self: ExecutionEngine, execution_payload: ExecutionPayloa ... ``` +#### `is_valid_block_hash` + +```python +def is_valid_block_hash(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: + """ + Return ``True`` if and only if ``execution_payload.block_hash`` is computed correctly. + """ + ... +``` + +#### `verify_and_notify_new_payload` + +```python +def verify_and_notify_new_payload(self: ExecutionEngine, + new_payload_request: NewPayloadRequest) -> bool: + """ + Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``. + """ + assert self.is_valid_block_hash(new_payload_request.execution_payload) + assert self.notify_new_payload(new_payload_request.execution_payload) + ... + return True +``` + ### Block processing *Note*: The call to the `process_execution_payload` must happen before the call to the `process_randao` as the former depends on the `randao_mix` computed with the reveal of the previous block. @@ -366,7 +390,7 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi # Verify timestamp assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid - assert execution_engine.notify_new_payload(payload) + assert execution_engine.verify_and_notify_new_payload(NewPayloadRequest(execution_payload=payload)) # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( parent_hash=payload.parent_hash, diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index c8e969d25..af4083017 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -418,7 +418,7 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi # Verify timestamp assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) # Verify the execution payload is valid - assert execution_engine.notify_new_payload(payload) + assert execution_engine.verify_and_notify_new_payload(NewPayloadRequest(execution_payload=payload)) # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( parent_hash=payload.parent_hash, diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 517913a36..bffa6a456 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -179,16 +179,6 @@ class NewPayloadRequest(object): #### Engine APIs -#### `is_valid_block_hash` - -```python -def is_valid_block_hash(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: - """ - Return ``True`` if and only if ``execution_payload.block_hash`` is computed correctly. - """ - ... -``` - #### `is_valid_versioned_hashes` ```python @@ -200,7 +190,7 @@ def is_valid_versioned_hashes(self: ExecutionEngine, new_payload_request: NewPay ... ``` -#### `verify_and_notify_new_payload` +#### Modified `verify_and_notify_new_payload` ```python def verify_and_notify_new_payload(self: ExecutionEngine, @@ -209,7 +199,7 @@ def verify_and_notify_new_payload(self: ExecutionEngine, Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``. """ assert self.is_valid_block_hash(new_payload_request.execution_payload) - assert self.is_valid_versioned_hashes(new_payload_request) + assert self.is_valid_versioned_hashes(new_payload_request) # [Modified in Deneb] assert self.notify_new_payload(new_payload_request.execution_payload) ... return True diff --git a/sync/optimistic.md b/sync/optimistic.md index 14eb99fb1..1fd1ba46e 100644 --- a/sync/optimistic.md +++ b/sync/optimistic.md @@ -159,7 +159,7 @@ these conditions.* To optimistically import a block: -- The [`notify_new_payload`](../specs/bellatrix/beacon-chain.md#notify_new_payload) function MUST return `True` if the execution +- The [`verify_and_notify_new_payload`](../specs/bellatrix/beacon-chain.md#verify_and_notify_new_payload) function MUST return `True` if the execution engine returns `NOT_VALIDATED` or `VALID`. An `INVALIDATED` response MUST return `False`. - The [`validate_merge_block`](../specs/bellatrix/fork-choice.md#validate_merge_block) function MUST NOT raise an assertion if both the @@ -172,7 +172,7 @@ In addition to this change in validation, the consensus engine MUST track which blocks returned `NOT_VALIDATED` and which returned `VALID` for subsequent processing. Optimistically imported blocks MUST pass all verifications included in -`process_block` (withstanding the modifications to `notify_new_payload`). +`process_block` (withstanding the modifications to `verify_and_notify_new_payload`). A consensus engine MUST be able to retrospectively (i.e., after import) modify the status of `NOT_VALIDATED` blocks to be either `VALID` or `INVALIDATED` based upon responses diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py index b54afafc3..1aa4a2c37 100644 --- a/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py @@ -31,12 +31,6 @@ def run_execution_payload_processing(spec, state, execution_payload, valid=True, called_new_block = False class TestEngine(spec.NoopExecutionEngine): - def notify_new_payload(self, payload) -> bool: - nonlocal called_new_block, execution_valid - called_new_block = True - assert payload == execution_payload - return execution_valid - def verify_and_notify_new_payload(self, new_payload_request) -> bool: nonlocal called_new_block, execution_valid called_new_block = True diff --git a/tests/core/pyspec/eth2spec/test/helpers/optimistic_sync.py b/tests/core/pyspec/eth2spec/test/helpers/optimistic_sync.py index 816c7a10b..ad23dbebc 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/optimistic_sync.py +++ b/tests/core/pyspec/eth2spec/test/helpers/optimistic_sync.py @@ -91,7 +91,7 @@ def add_optimistic_block(spec, mega_store, signed_block, test_steps, Add a block with optimistic sync logic ``valid`` indicates if the given ``signed_block.message.body.execution_payload`` is valid/invalid - from ``notify_new_payload`` method response. + from ``verify_and_notify_new_payload`` method response. """ block = signed_block.message block_root = block.hash_tree_root() From 212a314287568a1414f389569a0bda0a8343354f Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 11:27:26 +0800 Subject: [PATCH 30/39] Fix ToC and remove `validate_blobs_and_kzg_commitments` --- specs/bellatrix/beacon-chain.md | 2 + specs/deneb/beacon-chain.md | 3 +- specs/deneb/validator.md | 13 +-- .../unittests/validator/test_validator.py | 87 ------------------- 4 files changed, 4 insertions(+), 101 deletions(-) diff --git a/specs/bellatrix/beacon-chain.md b/specs/bellatrix/beacon-chain.md index e8eef346d..6cc69d747 100644 --- a/specs/bellatrix/beacon-chain.md +++ b/specs/bellatrix/beacon-chain.md @@ -37,6 +37,8 @@ - [`NewPayloadRequest`](#newpayloadrequest) - [Engine APIs](#engine-apis) - [`notify_new_payload`](#notify_new_payload) + - [`is_valid_block_hash`](#is_valid_block_hash) + - [`verify_and_notify_new_payload`](#verify_and_notify_new_payload) - [Block processing](#block-processing) - [Execution payload](#execution-payload) - [`process_execution_payload`](#process_execution_payload) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 5c31f1fc0..85c1fbdce 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -29,9 +29,8 @@ - [Request data](#request-data) - [Modified `NewPayloadRequest`](#modified-newpayloadrequest) - [Engine APIs](#engine-apis) - - [`is_valid_block_hash`](#is_valid_block_hash) - [`is_valid_versioned_hashes`](#is_valid_versioned_hashes) - - [`verify_and_notify_new_payload`](#verify_and_notify_new_payload) + - [Modified `verify_and_notify_new_payload`](#modified-verify_and_notify_new_payload) - [Block processing](#block-processing) - [Execution payload](#execution-payload) - [`process_execution_payload`](#process_execution_payload) diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index fa6f1651e..ae5f26673 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -101,18 +101,7 @@ All validator responsibilities remain unchanged other than those noted below. 1. After retrieving the execution payload from the execution engine as specified in Capella, use the `payload_id` to retrieve `blobs`, `blob_kzg_commitments`, and `blob_kzg_proofs` via `get_payload(payload_id).blobs_bundle`. -2. Validate `blobs` and `blob_kzg_commitments`: - -```python -def validate_blobs_and_kzg_commitments(blobs: Sequence[Blob], - blob_kzg_commitments: Sequence[KZGCommitment], - blob_kzg_proofs: Sequence[KZGProof]) -> None: - # Optionally sanity-check that the KZG commitments match the blobs (as produced by the execution engine) - assert len(blob_kzg_commitments) == len(blobs) == len(blob_kzg_proofs) - assert verify_blob_kzg_proof_batch(blobs, blob_kzg_commitments, blob_kzg_proofs) -``` - -3. If valid, set `block.body.blob_kzg_commitments = blob_kzg_commitments`. +2. Set `block.body.blob_kzg_commitments = blob_kzg_commitments`. #### Constructing the `SignedBlobSidecar`s diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py index 62b8e9dc1..876824107 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py @@ -2,7 +2,6 @@ from eth2spec.test.context import ( always_bls, spec_state_test, with_deneb_and_later, - expect_assertion_error ) from eth2spec.test.helpers.execution_payload import ( compute_el_block_hash, @@ -18,92 +17,6 @@ from eth2spec.test.helpers.keys import ( ) -@with_deneb_and_later -@spec_state_test -def test_validate_blobs_and_kzg_commitments(spec, state): - """ - Test `validate_blobs_and_kzg_commitments` - """ - blob_count = 4 - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, blobs, blob_kzg_commitments, proofs = get_sample_opaque_tx(spec, blob_count=blob_count) - block.body.blob_kzg_commitments = blob_kzg_commitments - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - - spec.validate_blobs_and_kzg_commitments(blobs, - blob_kzg_commitments, - proofs) - - -@with_deneb_and_later -@spec_state_test -def test_validate_blobs_and_kzg_commitments_missing_blob(spec, state): - """ - Test `validate_blobs_and_kzg_commitments` - """ - blob_count = 4 - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, blobs, blob_kzg_commitments, proofs = get_sample_opaque_tx(spec, blob_count=blob_count) - block.body.blob_kzg_commitments = blob_kzg_commitments - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - - expect_assertion_error( - lambda: spec.validate_blobs_and_kzg_commitments( - blobs[:-1], - blob_kzg_commitments, - proofs - ) - ) - - -@with_deneb_and_later -@spec_state_test -def test_validate_blobs_and_kzg_commitments_missing_proof(spec, state): - """ - Test `validate_blobs_and_kzg_commitments` - """ - blob_count = 4 - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, blobs, blob_kzg_commitments, proofs = get_sample_opaque_tx(spec, blob_count=blob_count) - block.body.blob_kzg_commitments = blob_kzg_commitments - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - - expect_assertion_error( - lambda: spec.validate_blobs_and_kzg_commitments( - blobs, - blob_kzg_commitments, - proofs[:-1] - ) - ) - - -@with_deneb_and_later -@spec_state_test -def test_validate_blobs_and_kzg_commitments_incorrect_blob(spec, state): - """ - Test `validate_blobs_and_kzg_commitments` - """ - blob_count = 4 - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, blobs, blob_kzg_commitments, proofs = get_sample_opaque_tx(spec, blob_count=blob_count) - block.body.blob_kzg_commitments = blob_kzg_commitments - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - - blobs[1] = spec.Blob(blobs[1][:13] + bytes([(blobs[1][13] + 1) % 256]) + blobs[1][14:]) - - expect_assertion_error( - lambda: spec.validate_blobs_and_kzg_commitments( - blobs, - blob_kzg_commitments, - proofs - ) - ) - - @with_deneb_and_later @spec_state_test def test_blob_sidecar_signature(spec, state): From 53a9221d1a67799549f55438a2b1ce5e533d3b5b Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 11:55:22 +0800 Subject: [PATCH 31/39] Fix ToC --- specs/deneb/beacon-chain.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 85c1fbdce..1ab1df611 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -29,8 +29,8 @@ - [Request data](#request-data) - [Modified `NewPayloadRequest`](#modified-newpayloadrequest) - [Engine APIs](#engine-apis) - - [`is_valid_versioned_hashes`](#is_valid_versioned_hashes) - - [Modified `verify_and_notify_new_payload`](#modified-verify_and_notify_new_payload) + - [`is_valid_versioned_hashes`](#is_valid_versioned_hashes) + - [Modified `verify_and_notify_new_payload`](#modified-verify_and_notify_new_payload) - [Block processing](#block-processing) - [Execution payload](#execution-payload) - [`process_execution_payload`](#process_execution_payload) @@ -178,7 +178,7 @@ class NewPayloadRequest(object): #### Engine APIs -#### `is_valid_versioned_hashes` +##### `is_valid_versioned_hashes` ```python def is_valid_versioned_hashes(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool: @@ -189,7 +189,7 @@ def is_valid_versioned_hashes(self: ExecutionEngine, new_payload_request: NewPay ... ``` -#### Modified `verify_and_notify_new_payload` +##### Modified `verify_and_notify_new_payload` ```python def verify_and_notify_new_payload(self: ExecutionEngine, From ec1ee74edb97112fdd0361a6d7c46d8a509684a6 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 12:17:07 +0800 Subject: [PATCH 32/39] Fix typo Co-authored-by: Danny Ryan --- specs/deneb/beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 1ab1df611..735dcf628 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -198,7 +198,7 @@ def verify_and_notify_new_payload(self: ExecutionEngine, Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``. """ assert self.is_valid_block_hash(new_payload_request.execution_payload) - assert self.is_valid_versioned_hashes(new_payload_request) # [Modified in Deneb] + assert self.is_valid_versioned_hashes(new_payload_request) # [New in Deneb] assert self.notify_new_payload(new_payload_request.execution_payload) ... return True From 7a827638e66c982a341e2b10b303f772ea9d1f70 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 12:55:49 +0800 Subject: [PATCH 33/39] Ensure `verify_and_notify_new_payload` returns bool --- specs/bellatrix/beacon-chain.md | 7 ++++--- specs/deneb/beacon-chain.md | 14 ++++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/specs/bellatrix/beacon-chain.md b/specs/bellatrix/beacon-chain.md index 6cc69d747..6f67be96a 100644 --- a/specs/bellatrix/beacon-chain.md +++ b/specs/bellatrix/beacon-chain.md @@ -355,9 +355,10 @@ def verify_and_notify_new_payload(self: ExecutionEngine, """ Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``. """ - assert self.is_valid_block_hash(new_payload_request.execution_payload) - assert self.notify_new_payload(new_payload_request.execution_payload) - ... + if not self.is_valid_block_hash(new_payload_request.execution_payload): + return False + if not self.notify_new_payload(new_payload_request.execution_payload): + return False return True ``` diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 735dcf628..bcbf4d1d0 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -197,10 +197,16 @@ def verify_and_notify_new_payload(self: ExecutionEngine, """ Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``. """ - assert self.is_valid_block_hash(new_payload_request.execution_payload) - assert self.is_valid_versioned_hashes(new_payload_request) # [New in Deneb] - assert self.notify_new_payload(new_payload_request.execution_payload) - ... + if not self.is_valid_block_hash(new_payload_request.execution_payload): + return False + + # [New in Deneb] + if not self.is_valid_versioned_hashes(new_payload_request): + return False + + if not self.notify_new_payload(new_payload_request.execution_payload): + return False + return True ``` From f0a4281afdeeea8cc41c85405fb35203cf60463a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 16:11:32 +0800 Subject: [PATCH 34/39] Add tests. Add validation in the p2p beacon block gossiping --- presets/mainnet/deneb.yaml | 2 ++ presets/minimal/deneb.yaml | 2 ++ specs/_features/eip6110/beacon-chain.md | 2 ++ specs/deneb/p2p-interface.md | 5 +++++ .../test_process_execution_payload.py | 13 ++++++++++++ .../eth2spec/test/deneb/sanity/test_blocks.py | 20 +++++++++++++++---- 6 files changed, 40 insertions(+), 4 deletions(-) diff --git a/presets/mainnet/deneb.yaml b/presets/mainnet/deneb.yaml index ebe33f2d1..10c5025ed 100644 --- a/presets/mainnet/deneb.yaml +++ b/presets/mainnet/deneb.yaml @@ -4,5 +4,7 @@ # --------------------------------------------------------------- # `uint64(4096)` FIELD_ELEMENTS_PER_BLOB: 4096 +# `uint64(2**12)` (= 4096) +MAX_BLOB_COMMITMENTS_PER_BLOCK: 4096 # `uint64(2**2)` (= 4) MAX_BLOBS_PER_BLOCK: 4 diff --git a/presets/minimal/deneb.yaml b/presets/minimal/deneb.yaml index e51b5587d..91120f9da 100644 --- a/presets/minimal/deneb.yaml +++ b/presets/minimal/deneb.yaml @@ -4,5 +4,7 @@ # --------------------------------------------------------------- # [customized] FIELD_ELEMENTS_PER_BLOB: 4 +# [customized] +MAX_BLOB_COMMITMENTS_PER_BLOCK: 16 # `uint64(2**2)` (= 4) MAX_BLOBS_PER_BLOCK: 4 diff --git a/specs/_features/eip6110/beacon-chain.md b/specs/_features/eip6110/beacon-chain.md index 5f6fd99cf..e1384fdae 100644 --- a/specs/_features/eip6110/beacon-chain.md +++ b/specs/_features/eip6110/beacon-chain.md @@ -244,6 +244,8 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi assert payload.prev_randao == get_randao_mix(state, get_current_epoch(state)) # Verify timestamp assert payload.timestamp == compute_timestamp_at_slot(state, state.slot) + # Verify commitments are under limit + assert len(body.blob_kzg_commitments) <= MAX_BLOBS_PER_BLOCK # Verify the execution payload is valid versioned_hashes = [kzg_commitment_to_versioned_hash(commitment) for commitment in body.blob_kzg_commitments] assert execution_engine.verify_and_notify_new_payload( diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index d67c144b2..2d6454737 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -117,6 +117,11 @@ Deneb introduces new global topics for blob sidecars. The *type* of the payload of this topic changes to the (modified) `SignedBeaconBlock` found in deneb. +New validation: + +- _[REJECT]_ The length of KZG commitments is less than or equal to the limitation defined in Consensus Layer -- + i.e. validate that `len(body.signed_beacon_block.message.blob_kzg_commitments) <= MAX_BLOBS_PER_BLOCK` + ###### `blob_sidecar_{subnet_id}` This topic is used to propagate signed blob sidecars, where each blob index maps to some `subnet_id`. diff --git a/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py index df59385ab..988070278 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/deneb/block_processing/test_process_execution_payload.py @@ -190,3 +190,16 @@ def test_invalid_correct_input__execution_invalid(spec, state): yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments, valid=False, execution_valid=False) + + +@with_deneb_and_later +@spec_state_test +def test_invalid_exceed_max_blobs_per_block(spec, state): + execution_payload = build_empty_execution_payload(spec, state) + + opaque_tx, _, blob_kzg_commitments, _ = get_sample_opaque_tx(spec, blob_count=spec.MAX_BLOBS_PER_BLOCK + 1) + + execution_payload.transactions = [opaque_tx] + execution_payload.block_hash = compute_el_block_hash(spec, execution_payload) + + yield from run_execution_payload_processing(spec, state, execution_payload, blob_kzg_commitments, valid=False) diff --git a/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py index 36caacd3f..13a042b9f 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py @@ -16,7 +16,7 @@ from eth2spec.test.helpers.sharding import ( ) -def run_block_with_blobs(spec, state, blob_count, excess_data_gas=1): +def run_block_with_blobs(spec, state, blob_count, excess_data_gas=1, valid=True): yield 'pre', state block = build_empty_block_for_next_slot(spec, state) @@ -25,10 +25,16 @@ def run_block_with_blobs(spec, state, blob_count, excess_data_gas=1): block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.excess_data_gas = excess_data_gas block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - signed_block = state_transition_and_sign_block(spec, state, block) + + print(len(block.body.blob_kzg_commitments)) + + if valid: + signed_block = state_transition_and_sign_block(spec, state, block) + else: + signed_block = state_transition_and_sign_block(spec, state, block, expect_fail=True) yield 'blocks', [signed_block] - yield 'post', state + yield 'post', state if valid else None @with_deneb_and_later @@ -45,5 +51,11 @@ def test_one_blob(spec, state): @with_deneb_and_later @spec_state_test -def test_max_blobs(spec, state): +def test_max_blobs_per_block(spec, state): yield from run_block_with_blobs(spec, state, blob_count=spec.MAX_BLOBS_PER_BLOCK) + + +@with_deneb_and_later +@spec_state_test +def test_invalid_exceed_max_blobs_per_block(spec, state): + yield from run_block_with_blobs(spec, state, blob_count=spec.MAX_BLOBS_PER_BLOCK + 1, valid=False) From e9cc8dcc051ccff90916547027e6088b55083da5 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 16:32:39 +0800 Subject: [PATCH 35/39] PR feedback of Danny + verify `MAX_BLOBS_PER_BLOCK` size in unittest --- specs/deneb/beacon-chain.md | 2 +- .../test/deneb/unittests/test_config_invariants.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 tests/core/pyspec/eth2spec/test/deneb/unittests/test_config_invariants.py diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index c8b79f919..cd02d2494 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -73,7 +73,7 @@ The blob transactions are packed into the execution payload by the EL/builder wi | Name | Value | | - | - | -| `MAX_BLOB_COMMITMENTS_PER_BLOCK` | `uint64(2**12)` (= 4096) | hardfork independent fixed theoretical limit same as `LIMIT_BLOBS_PER_TX` (see EIP 4844) | +| `MAX_BLOB_COMMITMENTS_PER_BLOCK` | `uint64(2**12)` (= 4096) | hardfork independent fixed theoretical limit same as `LIMIT_BLOBS_PER_TX` (see EIP 4844) | | `MAX_BLOBS_PER_BLOCK` | `uint64(2**2)` (= 4) | Maximum number of blobs in a single block limited by `MAX_BLOB_COMMITMENTS_PER_BLOCK` | ## Configuration diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/test_config_invariants.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/test_config_invariants.py new file mode 100644 index 000000000..13a54225e --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/test_config_invariants.py @@ -0,0 +1,12 @@ +from eth2spec.test.context import ( + single_phase, + spec_test, + with_deneb_and_later, +) + + +@with_deneb_and_later +@spec_test +@single_phase +def test_length(spec): + assert spec.MAX_BLOBS_PER_BLOCK < spec.MAX_BLOB_COMMITMENTS_PER_BLOCK From 9bc27bad306c5b5be7c6e3381c8a01e10232bd91 Mon Sep 17 00:00:00 2001 From: djrtwo Date: Wed, 24 May 2023 05:41:42 -0600 Subject: [PATCH 36/39] a couple of minor cleanups --- specs/deneb/beacon-chain.md | 2 +- tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index cd02d2494..e1690c062 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -234,7 +234,7 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi assert len(body.blob_kzg_commitments) <= MAX_BLOBS_PER_BLOCK # Verify the execution payload is valid - # [Modified in Deneb] Pass `versioned_hashes` to Engine API + # [Modified in Deneb] Pass `versioned_hashes` to Execution Engine versioned_hashes = [kzg_commitment_to_versioned_hash(commitment) for commitment in body.blob_kzg_commitments] assert execution_engine.verify_and_notify_new_payload( NewPayloadRequest(execution_payload=payload, versioned_hashes=versioned_hashes) diff --git a/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py index 13a042b9f..ba0797a04 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/deneb/sanity/test_blocks.py @@ -26,8 +26,6 @@ def run_block_with_blobs(spec, state, blob_count, excess_data_gas=1, valid=True) block.body.execution_payload.excess_data_gas = excess_data_gas block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - print(len(block.body.blob_kzg_commitments)) - if valid: signed_block = state_transition_and_sign_block(spec, state, block) else: From 8d32e79b8d40993eeb6fa2cb67183682518bd3aa Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 20:54:22 +0800 Subject: [PATCH 37/39] Fix multiprocessing --- tests/generators/shuffling/main.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tests/generators/shuffling/main.py b/tests/generators/shuffling/main.py index b85fd42a2..88c586b6c 100644 --- a/tests/generators/shuffling/main.py +++ b/tests/generators/shuffling/main.py @@ -1,5 +1,5 @@ -from eth_utils import to_tuple from typing import Iterable +import random from eth2spec.gen_helpers.gen_base import gen_runner, gen_typing from eth2spec.test.helpers.typing import PresetBaseName @@ -8,6 +8,16 @@ from eth2spec.phase0 import mainnet as spec_mainnet, minimal as spec_minimal from eth2spec.test.helpers.constants import PHASE0, MINIMAL, MAINNET +def generate_random_bytes(rng=random.Random(5566)): + random_bytes = bytes(rng.randint(0, 255) for _ in range(32)) + return random_bytes + + +# NOTE: somehow the random.Random generated seeds do not have pickle issue. +rng = random.Random(1234) +seeds = [generate_random_bytes(rng) for i in range(30)] + + def shuffling_case_fn(spec, seed, count): yield 'mapping', 'data', { 'seed': '0x' + seed.hex(), @@ -20,9 +30,8 @@ def shuffling_case(spec, seed, count): return f'shuffle_0x{seed.hex()}_{count}', lambda: shuffling_case_fn(spec, seed, count) -@to_tuple def shuffling_test_cases(spec): - for seed in [spec.hash(seed_init_value.to_bytes(length=4, byteorder='little')) for seed_init_value in range(30)]: + for seed in seeds: for count in [0, 1, 2, 3, 5, 10, 33, 100, 1000, 9999]: yield shuffling_case(spec, seed, count) @@ -47,11 +56,13 @@ def create_provider(preset_name: PresetBaseName) -> gen_typing.TestProvider: handler_name='core', suite_name='shuffle', case_name=case_name, - case_fn=case_fn + case_fn=case_fn, ) return gen_typing.TestProvider(prepare=prepare_fn, make_cases=cases_fn) if __name__ == "__main__": - gen_runner.run_generator("shuffling", [create_provider(MINIMAL), create_provider(MAINNET)]) + gen_runner.run_generator("shuffling", [ + create_provider(MINIMAL), create_provider(MAINNET)] + ) From a10affa24580602670b319ffc5e349664b53b92e Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 May 2023 21:15:01 +0800 Subject: [PATCH 38/39] Fix preset table --- specs/deneb/beacon-chain.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index e1690c062..175a80edf 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -71,8 +71,8 @@ The blob transactions are packed into the execution payload by the EL/builder wi ### Execution -| Name | Value | -| - | - | +| Name | Value | Description | +| - | - | - | | `MAX_BLOB_COMMITMENTS_PER_BLOCK` | `uint64(2**12)` (= 4096) | hardfork independent fixed theoretical limit same as `LIMIT_BLOBS_PER_TX` (see EIP 4844) | | `MAX_BLOBS_PER_BLOCK` | `uint64(2**2)` (= 4) | Maximum number of blobs in a single block limited by `MAX_BLOB_COMMITMENTS_PER_BLOCK` | From b2771c67c31f5635b4d87eefe7f3a03aeeddeb7c Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 25 May 2023 00:16:19 +0800 Subject: [PATCH 39/39] Fallback to MODE_SINGLE_PROCESS --- tests/core/pyspec/eth2spec/gen_helpers/gen_base/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/gen_helpers/gen_base/settings.py b/tests/core/pyspec/eth2spec/gen_helpers/gen_base/settings.py index b8c3e591f..2f462dddb 100644 --- a/tests/core/pyspec/eth2spec/gen_helpers/gen_base/settings.py +++ b/tests/core/pyspec/eth2spec/gen_helpers/gen_base/settings.py @@ -5,7 +5,7 @@ import multiprocessing MODE_SINGLE_PROCESS = 'MODE_SINGLE_PROCESS' MODE_MULTIPROCESSING = 'MODE_MULTIPROCESSING' # Test generator mode -GENERATOR_MODE = MODE_MULTIPROCESSING +GENERATOR_MODE = MODE_SINGLE_PROCESS # Number of subprocesses when using MODE_MULTIPROCESSING NUM_PROCESS = multiprocessing.cpu_count() // 2 - 1