From 73df1935b1dbc09036f793e2759e3c4f794fc04b Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 23 May 2023 22:25:43 +0800 Subject: [PATCH] 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