From aa91788dfbeeb79c8f38a68e3ee610ce11357fde Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Fri, 15 Oct 2021 19:46:46 +0100 Subject: [PATCH 01/46] Yield the `pre` state for this test type before making blocks --- .../eth2spec/test/helpers/fork_transition.py | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py b/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py index 947954b80c..fbed08f090 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py @@ -244,15 +244,6 @@ def run_transition_with_operation(state, signed_exits = prepare_signed_exits(spec, state, [selected_validator_index]) operation_dict = {'voluntary_exits': signed_exits} - blocks = [] - - if is_right_before_fork: - # add a block with operation. - block = build_empty_block_for_next_slot(spec, state) - _set_operations_by_dict(block, operation_dict) - signed_block = state_transition_and_sign_block(spec, state, block) - blocks.append(pre_tag(signed_block)) - def _check_state(): if operation_type == OperationType.PROPOSER_SLASHING: slashed_proposer = state.validators[proposer_slashing.signed_header_1.message.proposer_index] @@ -274,10 +265,18 @@ def _check_state(): validator = state.validators[selected_validator_index] assert validator.exit_epoch < post_spec.FAR_FUTURE_EPOCH + yield "pre", state + + blocks = [] + if is_right_before_fork: - _check_state() + # add a block with operation. + block = build_empty_block_for_next_slot(spec, state) + _set_operations_by_dict(block, operation_dict) + signed_block = state_transition_and_sign_block(spec, state, block) + blocks.append(pre_tag(signed_block)) - yield "pre", state + _check_state() # irregular state transition to handle fork: _operation_at_slot = operation_dict if is_at_fork else None From c5b169bd62bd42a0dd5bcbf6e98e0175c9afa82c Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 15 Oct 2021 16:15:37 -0600 Subject: [PATCH 02/46] fix issue with mutation in test generation --- tests/core/pyspec/eth2spec/test/helpers/fork_transition.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py b/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py index fbed08f090..310c4a8d2a 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py @@ -288,7 +288,7 @@ def _check_state(): # after the fork if operation_type == OperationType.DEPOSIT: - _transition_until_active(post_spec, state, post_tag, blocks, selected_validator_index) + state = _transition_until_active(post_spec, state, post_tag, blocks, selected_validator_index) else: # avoid using the slashed validators as block proposers ignoring_proposers = [selected_validator_index] if is_slashing_operation else None @@ -332,3 +332,5 @@ def _transition_until_active(post_spec, state, post_tag, blocks, validator_index state_transition_across_slots(post_spec, state, to_slot, block_filter=only_at(to_slot)) ]) assert post_spec.is_active_validator(state.validators[validator_index], post_spec.get_current_epoch(state)) + + return state From 7e080c18f7448b970cfe59fd5f36e5624d911336 Mon Sep 17 00:00:00 2001 From: protolambda Date: Sat, 16 Oct 2021 01:15:31 +0200 Subject: [PATCH 03/46] update remerkleable --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 3dcaaa44fb..863761daa3 100644 --- a/setup.py +++ b/setup.py @@ -1021,7 +1021,7 @@ def run(self): "py_ecc==5.2.0", "milagro_bls_binding==1.6.3", "dataclasses==0.6", - "remerkleable==0.1.22", + "remerkleable==0.1.24", RUAMEL_YAML_VERSION, "lru-dict==1.1.6", MARKO_VERSION, From cd5cf60deb83b654e847d00a60be31b5617154c2 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 18 Oct 2021 13:32:29 +0800 Subject: [PATCH 04/46] Clarify `get_pow_block` block-not-found case --- setup.py | 2 +- specs/merge/fork-choice.md | 12 +++++++++++- specs/merge/validator.md | 1 + 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 3dcaaa44fb..0611e55677 100644 --- a/setup.py +++ b/setup.py @@ -510,7 +510,7 @@ def sundry_functions(cls) -> str: ExecutionState = Any -def get_pow_block(hash: Bytes32) -> PowBlock: +def get_pow_block(hash: Bytes32) -> Optional[PowBlock]: return PowBlock(block_hash=hash, parent_hash=Bytes32(), total_difficulty=uint256(0), difficulty=uint256(0)) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index ea4a5588a9..eb99cce64e 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -64,7 +64,8 @@ class PowBlock(Container): ### `get_pow_block` -Let `get_pow_block(block_hash: Hash32) -> PowBlock` be the function that given the hash of the PoW block returns its data. +Let `get_pow_block(block_hash: Hash32) -> Optional[PowBlock]` be the function that given the hash of the PoW block returns its data. +It may result in `None` if the requested block is not found if execution engine is still syncing. *Note*: The `eth_getBlockByHash` JSON-RPC method may be used to pull this information from an execution client. @@ -90,6 +91,12 @@ def is_valid_terminal_pow_block(block: PowBlock, parent: PowBlock) -> bool: ```python def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: + """ + Run ``on_block`` upon receiving a new block. + + An block that is asserted as invalid due to unavailable PoW block may be valid at a later time, + consider scheduling it for later processing in such case. + """ block = signed_block.message # Parent block must be known assert block.parent_root in store.block_states @@ -110,8 +117,11 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # [New in Merge] if is_merge_block(pre_state, block.body): + # Note: the unavailable PoW block(s) may be available later pow_block = get_pow_block(block.body.execution_payload.parent_hash) + assert pow_block is not None pow_parent = get_pow_block(pow_block.parent_hash) + assert pow_parent is not None assert is_valid_terminal_pow_block(pow_block, pow_parent) # Add new block to the store diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 2de4ef6b1a..9fd1a24f73 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -103,6 +103,7 @@ def get_pow_block_at_terminal_total_difficulty(pow_chain: Sequence[PowBlock]) -> # `pow_chain` abstractly represents all blocks in the PoW chain for block in pow_chain: parent = get_pow_block(block.parent_hash) + assert parent is not None block_reached_ttd = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY if block_reached_ttd and not parent_reached_ttd: From ec516a762553376e7a4a0f8c7aee96e089141053 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Mon, 18 Oct 2021 02:34:52 -0700 Subject: [PATCH 05/46] Update `ssz_generic` test format README The existing README has a reference to an alias type `Bytes[N]` that has been removed from the repo so it is not clear what it exactly refers to. This PR updates the type to the equivalent `List[T, N]` using more recent SSZ typing syntax. --- tests/formats/ssz_generic/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/formats/ssz_generic/README.md b/tests/formats/ssz_generic/README.md index 85a507985e..dacd727796 100644 --- a/tests/formats/ssz_generic/README.md +++ b/tests/formats/ssz_generic/README.md @@ -180,7 +180,7 @@ class ComplexTestStruct(Container): A: uint16 B: List[uint16, 128] C: uint8 - D: Bytes[256] + D: List[uint8, 256] E: VarTestStruct F: Vector[FixedTestStruct, 4] G: Vector[VarTestStruct, 2] From afb62eebf0f4c9d10abfc620b87f4f3492f024a3 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 18 Oct 2021 17:42:47 +0800 Subject: [PATCH 06/46] Add pytest CLI option `--fork` so that we can just run with specific phase (fork) --- tests/core/pyspec/eth2spec/test/conftest.py | 20 ++++++++++++++++++++ tests/core/pyspec/eth2spec/test/context.py | 5 ++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/test/conftest.py b/tests/core/pyspec/eth2spec/test/conftest.py index e6d8352e03..55613be0a9 100644 --- a/tests/core/pyspec/eth2spec/test/conftest.py +++ b/tests/core/pyspec/eth2spec/test/conftest.py @@ -1,4 +1,7 @@ from eth2spec.test import context +from eth2spec.test.helpers.constants import ( + ALL_PHASES, +) from eth2spec.utils import bls as bls_utils # We import pytest only when it's present, i.e. when we are running tests. @@ -29,6 +32,13 @@ def pytest_addoption(parser): "--preset", action="store", type=str, default="minimal", help="preset: make the pyspec use the specified preset" ) + parser.addoption( + "--fork", action="append", type=str, + help=( + "fork: make the pyspec only run with the specified phase." + " To run multiple phases, e.g., --fork=phase0 --fork=altair" + ) + ) parser.addoption( "--disable-bls", action="store_true", default=False, help="bls-default: make tests that are not dependent on BLS run without BLS" @@ -44,6 +54,16 @@ def preset(request): context.DEFAULT_TEST_PRESET = request.config.getoption("--preset") +@fixture(autouse=True) +def run_phases(request): + phases = request.config.getoption("--fork") + if phases: + phases = [phase.lower() for phase in phases] + context.DEFAULT_PYTEST_FORKS = set(phases) + else: + context.DEFAULT_PYTEST_FORKS = ALL_PHASES + + @fixture(autouse=True) def bls_default(request): disable_bls = request.config.getoption("--disable-bls") diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index 6062648d60..944351bfde 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -22,6 +22,9 @@ # Without pytest CLI arg or pyspec-test-generator 'preset' argument, this will be the config to apply. DEFAULT_TEST_PRESET = MINIMAL +# Without pytest CLI arg or pyspec-test-generator 'run-phase' argument, this will be the config to apply. +DEFAULT_PYTEST_FORKS = ALL_PHASES + # TODO: currently phases are defined as python modules. # It would be better if they would be more well-defined interfaces for stronger typing. @@ -351,7 +354,7 @@ def with_phases(phases, other_phases=None): """ def decorator(fn): def wrapper(*args, **kw): - run_phases = phases + run_phases = set(phases).intersection(DEFAULT_PYTEST_FORKS) # limit phases if one explicitly specified if 'phase' in kw: From 2f6e817f3d06286565ad2fbae5f089ae31d54185 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 18 Oct 2021 18:30:33 +0800 Subject: [PATCH 07/46] Set CI job for each fork --- .circleci/config.yml | 40 ++++++++++++++++++++++++++++++++++++---- Makefile | 9 ++++++++- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index bb1df99096..bcce74bd91 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -90,7 +90,7 @@ jobs: name: Install pyspec requirements command: make install_test - save_pyspec_cached_venv - test: + test-phase0: docker: - image: circleci/python:3.8 working_directory: ~/specs-repo @@ -100,7 +100,33 @@ jobs: - restore_pyspec_cached_venv - run: name: Run py-tests - command: make citest + command: make citest fork=phase0 + - store_test_results: + path: tests/core/pyspec/test-reports + test-altair: + docker: + - image: circleci/python:3.8 + working_directory: ~/specs-repo + steps: + - restore_cache: + key: v3-specs-repo-{{ .Branch }}-{{ .Revision }} + - restore_pyspec_cached_venv + - run: + name: Run py-tests + command: make citest fork=altair + - store_test_results: + path: tests/core/pyspec/test-reports + test-merge: + docker: + - image: circleci/python:3.8 + working_directory: ~/specs-repo + steps: + - restore_cache: + key: v3-specs-repo-{{ .Branch }}-{{ .Revision }} + - restore_pyspec_cached_venv + - run: + name: Run py-tests + command: make citest fork=merge - store_test_results: path: tests/core/pyspec/test-reports table_of_contents: @@ -208,14 +234,20 @@ workflows: - install_pyspec_test: requires: - checkout_specs - - test: + - test-phase0: + requires: + - install_pyspec_test + - test-altair: + requires: + - install_pyspec_test + - test-merge: requires: - install_pyspec_test - table_of_contents - codespell - lint: requires: - - test + - install_pyspec_test # NOTE: Since phase 0 has been launched, we disabled the deposit contract tests. # - install_deposit_contract_web3_tester: # requires: diff --git a/Makefile b/Makefile index 9771410783..547ed215f3 100644 --- a/Makefile +++ b/Makefile @@ -105,8 +105,15 @@ find_test: pyspec python3 -m pytest -k=$(K) --disable-bls --cov=eth2spec.phase0.minimal --cov=eth2spec.altair.minimal --cov-report="html:$(COV_HTML_OUT)" --cov-branch eth2spec citest: pyspec - mkdir -p tests/core/pyspec/test-reports/eth2spec; . venv/bin/activate; cd $(PY_SPEC_DIR); \ + mkdir -p tests/core/pyspec/test-reports/eth2spec; +ifdef fork + . venv/bin/activate; cd $(PY_SPEC_DIR); \ + python3 -m pytest -n 4 --bls-type=milagro --fork=$(fork) --junitxml=eth2spec/test_results.xml eth2spec +else + . venv/bin/activate; cd $(PY_SPEC_DIR); \ python3 -m pytest -n 4 --bls-type=milagro --junitxml=eth2spec/test_results.xml eth2spec +endif + open_cov: ((open "$(COV_INDEX_FILE)" || xdg-open "$(COV_INDEX_FILE)") &> /dev/null) & From 4f3637851cb91473263120255ca5f50ddd2ab967 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 19 Oct 2021 00:54:29 +0800 Subject: [PATCH 08/46] Disable some too-slow mainnet preset fork choice tests --- .../pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index c00a91b28b..79bf353b17 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -564,6 +564,7 @@ def test_new_justified_is_later_than_store_justified(spec, state): @with_all_phases @spec_state_test +@with_presets([MINIMAL], reason="too slow") def test_new_finalized_slot_is_not_justified_checkpoint_ancestor(spec, state): """ J: Justified @@ -641,6 +642,7 @@ def test_new_finalized_slot_is_not_justified_checkpoint_ancestor(spec, state): @with_all_phases @spec_state_test +@with_presets([MINIMAL], reason="too slow") def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): """ J: Justified From 190ef9fb503743b779ce137527cdf5dc96b6a7d0 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 18 Oct 2021 12:07:33 -0600 Subject: [PATCH 09/46] call notify_forkchoice_updated with finalized_block_hash == 0x00..00 if not yet finalized --- specs/merge/fork-choice.md | 1 + 1 file changed, 1 insertion(+) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index ea4a5588a9..32bb8215e8 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -49,6 +49,7 @@ def notify_forkchoice_updated(self: ExecutionEngine, head_block_hash: Hash32, fi ``` *Note*: The call of the `notify_forkchoice_updated` function maps on the `POS_FORKCHOICE_UPDATED` event defined in the [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#definitions). +As per EIP-3675, before a post-transition block is finalized, `notify_forkchoice_updated` must be called with `finalized_block_hash = Hash32()`. ## Helpers From 3bfdc917e13ec56c50ca8f0ff4b60485ab0b1a72 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 18 Oct 2021 13:38:08 -0600 Subject: [PATCH 10/46] ad TBH_ACTIVATION_EPOCH --- configs/mainnet.yaml | 4 +++- configs/minimal.yaml | 4 +++- specs/merge/beacon-chain.md | 3 ++- specs/merge/client-settings.md | 9 +++++---- specs/merge/fork-choice.md | 6 ++++-- specs/merge/validator.md | 5 +++++ 6 files changed, 22 insertions(+), 9 deletions(-) diff --git a/configs/mainnet.yaml b/configs/mainnet.yaml index 77d7a136fd..74896c90f9 100644 --- a/configs/mainnet.yaml +++ b/configs/mainnet.yaml @@ -7,8 +7,10 @@ PRESET_BASE: 'mainnet' # --------------------------------------------------------------- # TBD, 2**256-2**10 is a placeholder TERMINAL_TOTAL_DIFFICULTY: 115792089237316195423570985008687907853269984665640564039457584007913129638912 -# By default, don't use this param +# By default, don't use these params TERMINAL_BLOCK_HASH: 0x0000000000000000000000000000000000000000000000000000000000000000 +TBH_ACTIVATION_EPOCH: 18446744073709551615 + # Genesis diff --git a/configs/minimal.yaml b/configs/minimal.yaml index a8b30fd546..35e321e84e 100644 --- a/configs/minimal.yaml +++ b/configs/minimal.yaml @@ -7,8 +7,10 @@ PRESET_BASE: 'minimal' # --------------------------------------------------------------- # TBD, 2**256-2**10 is a placeholder TERMINAL_TOTAL_DIFFICULTY: 115792089237316195423570985008687907853269984665640564039457584007913129638912 -# By default, don't use this param +# By default, don't use these params TERMINAL_BLOCK_HASH: 0x0000000000000000000000000000000000000000000000000000000000000000 +TBH_ACTIVATION_EPOCH: 18446744073709551615 + # Genesis diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index f5977db5e3..12919b4746 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -74,7 +74,8 @@ This patch adds transaction execution to the beacon chain as part of the Merge f | Name | Value | | - | - | | `TERMINAL_TOTAL_DIFFICULTY` | **TBD** | -| `TERMINAL_BLOCK_HASH` | `Hash32('0x0000000000000000000000000000000000000000000000000000000000000000')` | +| `TERMINAL_BLOCK_HASH` | `Hash32()` | +| `TBH_ACTIVATION_EPOCH` | `FAR_FUTURE_EPOCH` | ## Containers diff --git a/specs/merge/client-settings.md b/specs/merge/client-settings.md index 4a4b38d3e2..63ddfe5ae4 100644 --- a/specs/merge/client-settings.md +++ b/specs/merge/client-settings.md @@ -18,11 +18,12 @@ This document specifies configurable settings that clients must implement for th To coordinate manual overrides to [`TERMINAL_TOTAL_DIFFICULTY`](./beacon-chain.md#Transition-settings) parameter, clients must provide `--terminal-total-difficulty-override` as a configurable setting. The value provided by this setting must take precedence over pre-configured `TERMINAL_TOTAL_DIFFICULTY` parameter. Clients should accept the setting as a decimal value (i.e., *not* hexadecimal). -Except under exceptional scenarios, this setting is expected to not be used. Sufficient warning to the user about this exceptional configurable setting should be provided. +Except under exceptional scenarios, this setting is not expected to be used. Sufficient warning to the user about this exceptional configurable setting should be provided. ### Override terminal block hash -To allow for transition coordination around a specific PoW block, clients must also provide `--terminal-block-hash-override` as a configurable setting. -The value provided by this setting takes precedence over the pre-configured `TERMINAL_BLOCK_HASH` parameter. +To allow for transition coordination around a specific PoW block, clients must also provide `--terminal-block-hash-override` and `--terminal-block-hash-epoch-override` as configurable settings. +* The value provided by `--terminal-block-hash-override` takes precedence over the pre-configured `TERMINAL_BLOCK_HASH` parameter. +* The value provided by `--terminal-block-hash-epoch-override` takes precedence over the pre-configured `TBH_ACTIVATION_EPOCH` parameter. -Except under exceptional scenarios, this setting is expected to not be used. Sufficient warning to the user about this exceptional configurable setting should be provided. +Except under exceptional scenarios, these settings are not expected to be used. Sufficient warning to the user about this exceptional configurable setting should be provided. diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index ea4a5588a9..f1f5e265c6 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -74,8 +74,8 @@ Used by fork-choice handler, `on_block`. ```python def is_valid_terminal_pow_block(block: PowBlock, parent: PowBlock) -> bool: - if block.block_hash == TERMINAL_BLOCK_HASH: - return True + if TERMINAL_BLOCK_HASH != Hash32(): + return block.block_hash == TERMINAL_BLOCK_HASH is_total_difficulty_reached = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY is_parent_total_difficulty_valid = parent.total_difficulty < TERMINAL_TOTAL_DIFFICULTY @@ -113,6 +113,8 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: pow_block = get_pow_block(block.body.execution_payload.parent_hash) pow_parent = get_pow_block(pow_block.parent_hash) assert is_valid_terminal_pow_block(pow_block, pow_parent) + if TERMINAL_BLOCK_HASH != Hash32(): + assert compute_epoch_at_slot(block.slot) >= TBH_ACTIVATION_EPOCH # Add new block to the store store.blocks[hash_tree_root(block)] = block diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 2de4ef6b1a..f15f62f3df 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -127,6 +127,11 @@ def prepare_execution_payload(state: BeaconState, fee_recipient: ExecutionAddress, execution_engine: ExecutionEngine) -> Optional[PayloadId]: if not is_merge_complete(state): + is_tbh_override_set = TERMINAL_BLOCK_HASH != Hash32() + if is_tbh_override_set and get_current_epoch(state.slot) < TBH_ACTIVATION_EPOCH: + # TBH 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 From d5be6b5d6868321c8928eb82586256601c6d75d2 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 18 Oct 2021 18:10:27 -0600 Subject: [PATCH 11/46] remove prepare_payload in favor of a unification with notify_forkchoice_updated --- setup.py | 9 ++- specs/merge/fork-choice.md | 21 ++++++- specs/merge/validator.md | 118 ++++++++++++++++++++++--------------- 3 files changed, 96 insertions(+), 52 deletions(-) diff --git a/setup.py b/setup.py index 863761daa3..fa396ab506 100644 --- a/setup.py +++ b/setup.py @@ -497,7 +497,7 @@ def imports(cls, preset_name: str): return super().imports(preset_name) + f''' from typing import Protocol from eth2spec.altair import {preset_name} as altair -from eth2spec.utils.ssz.ssz_typing import Bytes20, ByteList, ByteVector, uint256, Union +from eth2spec.utils.ssz.ssz_typing import Bytes8, Bytes20, ByteList, ByteVector, uint256, Union ''' @classmethod @@ -527,7 +527,10 @@ class NoopExecutionEngine(ExecutionEngine): def execute_payload(self: ExecutionEngine, execution_payload: ExecutionPayload) -> bool: return True - def notify_forkchoice_updated(self: ExecutionEngine, head_block_hash: Hash32, finalized_block_hash: Hash32) -> None: + def notify_forkchoice_updated(self: ExecutionEngine, + head_block_hash: Hash32, + finalized_block_hash: Hash32, + payload_attributes: Optional[PayloadAttributes]) -> None: pass def prepare_payload(self: ExecutionEngine, @@ -683,7 +686,7 @@ def combine_dicts(old_dict: Dict[str, T], new_dict: Dict[str, T]) -> Dict[str, T ignored_dependencies = [ 'bit', 'boolean', 'Vector', 'List', 'Container', 'BLSPubkey', 'BLSSignature', - 'Bytes1', 'Bytes4', 'Bytes20', 'Bytes32', 'Bytes48', 'Bytes96', 'Bitlist', 'Bitvector', + 'Bytes1', 'Bytes4', 'Bytes8', 'Bytes20', 'Bytes32', 'Bytes48', 'Bytes96', 'Bitlist', 'Bitvector', 'uint8', 'uint16', 'uint32', 'uint64', 'uint128', 'uint256', 'bytes', 'byte', 'ByteList', 'ByteVector', 'Dict', 'dict', 'field', 'ceillog2', 'floorlog2', 'Set', diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index ea4a5588a9..44e729104a 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -12,6 +12,7 @@ - [`ExecutionEngine`](#executionengine) - [`notify_forkchoice_updated`](#notify_forkchoice_updated) - [Helpers](#helpers) + - [`PayloadAttributes`](#payloadattributes) - [`PowBlock`](#powblock) - [`get_pow_block`](#get_pow_block) - [`is_valid_terminal_pow_block`](#is_valid_terminal_pow_block) @@ -43,8 +44,14 @@ This function performs two actions *atomically*: * Applies finality to the execution state: it irreversibly persists the chain of all execution payloads and corresponding state, up to and including `finalized_block_hash`. +Additionally, if `payload_attributes` is provided, this function sets in motion a payload build process on top of +`head_block_hash` with the result to be gathered by a followup call to `get_payload`. + ```python -def notify_forkchoice_updated(self: ExecutionEngine, head_block_hash: Hash32, finalized_block_hash: Hash32) -> None: +def notify_forkchoice_updated(self: ExecutionEngine, + head_block_hash: Hash32, + finalized_block_hash: Hash32, + payload_attributes: Optional[PayloadAttributes]) -> None: ... ``` @@ -52,6 +59,18 @@ def notify_forkchoice_updated(self: ExecutionEngine, head_block_hash: Hash32, fi ## Helpers +### `PayloadAttributes` + +Used to signal to initiate the payload build process via `notify_forkchoice_updated`. + +```python +@dataclass +class PayloadAttributes(object): + timestamp: uint64 + random: Bytes32 + fee_recipient: ExecutionAddress +``` + ### `PowBlock` ```python diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 2de4ef6b1a..cbca6eedae 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -11,9 +11,12 @@ - [Introduction](#introduction) - [Prerequisites](#prerequisites) - [Custom types](#custom-types) +- [Helpers](#helpers) + - [`get_pow_block_at_terminal_total_difficulty`](#get_pow_block_at_terminal_total_difficulty) + - [`get_terminal_pow_block`](#get_terminal_pow_block) + - [`get_payload_id`](#get_payload_id) - [Protocols](#protocols) - [`ExecutionEngine`](#executionengine) - - [`prepare_payload`](#prepare_payload) - [`get_payload`](#get_payload) - [Beacon chain responsibilities](#beacon-chain-responsibilities) - [Block proposal](#block-proposal) @@ -29,7 +32,7 @@ This document represents the changes to be made in the code of an "honest valida ## Prerequisites -This document is an extension of the [Altair -- Honest Validator](../altair/validator.md) guide. +This documens is an extension of the [Altair -- Honest Validator](../altair/validator.md) guide. All behaviors and definitions defined in this document, and documents it extends, carry over unless explicitly noted or overridden. All terminology, constants, functions, and protocol mechanics defined in the updated Beacon Chain doc of [The Merge](./beacon-chain.md) are requisite for this document and used throughout. @@ -39,38 +42,73 @@ Please see related Beacon Chain doc before continuing and use them as a referenc | Name | SSZ equivalent | Description | | - | - | - | -| `PayloadId` | `uint64` | Identifier of a payload building process | +| `PayloadId` | `Bytes8` | Identifier of a payload building process | -## Protocols +## Helpers -### `ExecutionEngine` +### `get_pow_block_at_terminal_total_difficulty` -*Note*: `prepare_payload` and `get_payload` functions are added to the `ExecutionEngine` protocol for use as a validator. +```python +def get_pow_block_at_terminal_total_difficulty(pow_chain: Sequence[PowBlock]) -> Optional[PowBlock]: + # `pow_chain` abstractly represents all blocks in the PoW chain + for block in pow_chain: + parent = get_pow_block(block.parent_hash) + block_reached_ttd = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY + parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY + if block_reached_ttd and not parent_reached_ttd: + return block -The body of each of these functions is implementation dependent. -The Engine API may be used to implement them with an external execution engine. + return None +``` -#### `prepare_payload` +### `get_terminal_pow_block` -Given the set of execution payload attributes, `prepare_payload` initiates a process of building an execution payload -on top of the execution chain tip identified by `parent_hash`. +```python +def get_terminal_pow_block(pow_chain: Sequence[PowBlock]) -> Optional[PowBlock]: + if TERMINAL_BLOCK_HASH != Hash32(): + # Terminal block hash override takes precedence over terminal total difficulty + pow_block_overrides = [block for block in pow_chain if block.block_hash == TERMINAL_BLOCK_HASH] + if not any(pow_block_overrides): + return None + return pow_block_overrides[0] + + return get_pow_block_at_terminal_total_difficulty(pow_chain) +``` + +### `get_payload_id` + +Given the `head_block_hash` and the `payload_attributes` that were used to +initiate the build process via `notify_forkchoice_updated`, `get_payload_id()` +returns the `payload_id` used to retrieve the payload via `get_payload`. ```python -def prepare_payload(self: ExecutionEngine, - parent_hash: Hash32, - timestamp: uint64, - random: Bytes32, - fee_recipient: ExecutionAddress) -> PayloadId: - """ - Return ``payload_id`` that is used to obtain the execution payload in a subsequent ``get_payload`` call. - """ - ... +def get_payload_id(parent_hash: Hash32, payload_attributes: PayloadAttributes) -> PayloadId: + return PayloadId( + hash( + parent_hash + + uint_to_bytes(payload_attributes.timestamp) + + payload_attributes.random + + payload_attributes.fee_recipient + )[0:8] + ) ``` +*Note*: This function does *not* use simple serialize `hash_tree_root` as to +avoid requiring simple serialize hashing capabilities in the Execution Layer. + +## Protocols + +### `ExecutionEngine` + +*Note*: `prepare_payload` and `get_payload` functions are added to the `ExecutionEngine` protocol for use as a validator. + +The body of each of these functions is implementation dependent. +The Engine API may be used to implement them with an external execution engine. + #### `get_payload` Given the `payload_id`, `get_payload` returns the most recent version of the execution payload that -has been built since the corresponding call to `prepare_payload` method. +has been built since the corresponding call to `notify_forkchoice_updated` method. ```python def get_payload(self: ExecutionEngine, payload_id: PayloadId) -> ExecutionPayload: @@ -92,38 +130,17 @@ All validator responsibilities remain unchanged other than those noted below. Na To obtain an execution payload, a block proposer building a block on top of a `state` must take the following actions: -1. Set `payload_id = prepare_execution_payload(state, pow_chain, fee_recipient, execution_engine)`, where: +1. Set `payload_id = prepare_execution_payload(state, pow_chain, finalized_block_hash, fee_recipient, execution_engine)`, where: * `state` is the state object after applying `process_slots(state, slot)` transition to the resulting state of the parent block processing * `pow_chain` is a list that abstractly represents all blocks in the PoW chain + * `finalized_block_hash` is the hash of the latest finalized execution payload (`Hash32()` if none yet finalized) * `fee_recipient` is the value suggested to be used for the `coinbase` field of the execution payload ```python -def get_pow_block_at_terminal_total_difficulty(pow_chain: Sequence[PowBlock]) -> Optional[PowBlock]: - # `pow_chain` abstractly represents all blocks in the PoW chain - for block in pow_chain: - parent = get_pow_block(block.parent_hash) - block_reached_ttd = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY - parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY - if block_reached_ttd and not parent_reached_ttd: - return block - - return None - - -def get_terminal_pow_block(pow_chain: Sequence[PowBlock]) -> Optional[PowBlock]: - if TERMINAL_BLOCK_HASH != Hash32(): - # Terminal block hash override takes precedence over terminal total difficulty - pow_block_overrides = [block for block in pow_chain if block.block_hash == TERMINAL_BLOCK_HASH] - if not any(pow_block_overrides): - return None - return pow_block_overrides[0] - - return get_pow_block_at_terminal_total_difficulty(pow_chain) - - def prepare_execution_payload(state: BeaconState, pow_chain: Sequence[PowBlock], + finalized_block_hash: Hash32, fee_recipient: ExecutionAddress, execution_engine: ExecutionEngine) -> Optional[PayloadId]: if not is_merge_complete(state): @@ -137,9 +154,14 @@ def prepare_execution_payload(state: BeaconState, # Post-merge, normal payload parent_hash = state.latest_execution_payload_header.block_hash - timestamp = compute_timestamp_at_slot(state, state.slot) - random = get_randao_mix(state, get_current_epoch(state)) - return execution_engine.prepare_payload(parent_hash, timestamp, random, fee_recipient) + # Set the forkchoice head and initiate the payload build process + payload_attributes = PayloadAttributes( + timestamp=compute_timestamp_at_slot(state, state.slot), + random=get_randao_mix(state, get_current_epoch(state)), + fee_recipient=fee_recipient, + ) + execution_engine.notify_forkchoice_updated(parent_hash, finalized_block_hash, payload_attributes) + return get_payload_id(parent_hash, payload_attributes) ``` 2. Set `block.body.execution_payload = get_execution_payload(payload_id, execution_engine)`, where: From ba582b3e3a1848a856281869977dc2cdacf64504 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 19 Oct 2021 11:25:50 +0800 Subject: [PATCH 12/46] Fix setup.py parser and rename `TBH_ACTIVATION_EPOCH` -> `TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH` --- configs/mainnet.yaml | 2 +- configs/minimal.yaml | 2 +- setup.py | 2 +- specs/merge/beacon-chain.md | 2 +- specs/merge/client-settings.md | 2 +- specs/merge/fork-choice.md | 2 +- specs/merge/validator.md | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/configs/mainnet.yaml b/configs/mainnet.yaml index 74896c90f9..6d23073b7f 100644 --- a/configs/mainnet.yaml +++ b/configs/mainnet.yaml @@ -9,7 +9,7 @@ PRESET_BASE: 'mainnet' TERMINAL_TOTAL_DIFFICULTY: 115792089237316195423570985008687907853269984665640564039457584007913129638912 # By default, don't use these params TERMINAL_BLOCK_HASH: 0x0000000000000000000000000000000000000000000000000000000000000000 -TBH_ACTIVATION_EPOCH: 18446744073709551615 +TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH: 18446744073709551615 diff --git a/configs/minimal.yaml b/configs/minimal.yaml index 35e321e84e..4f99fce313 100644 --- a/configs/minimal.yaml +++ b/configs/minimal.yaml @@ -9,7 +9,7 @@ PRESET_BASE: 'minimal' TERMINAL_TOTAL_DIFFICULTY: 115792089237316195423570985008687907853269984665640564039457584007913129638912 # By default, don't use these params TERMINAL_BLOCK_HASH: 0x0000000000000000000000000000000000000000000000000000000000000000 -TBH_ACTIVATION_EPOCH: 18446744073709551615 +TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH: 18446744073709551615 diff --git a/setup.py b/setup.py index 863761daa3..fe990fbeee 100644 --- a/setup.py +++ b/setup.py @@ -603,7 +603,7 @@ def format_protocol(protocol_name: str, protocol_def: ProtocolDefinition) -> str # Access global dict of config vars for runtime configurables for name in spec_object.config_vars.keys(): - functions_spec = functions_spec.replace(name, 'config.' + name) + functions_spec = re.sub(r"\b%s\b" % name, 'config.' + name, functions_spec) def format_config_var(name: str, vardef: VariableDefinition) -> str: if vardef.type_name is None: diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index 12919b4746..2e1fd88fb1 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -75,7 +75,7 @@ This patch adds transaction execution to the beacon chain as part of the Merge f | - | - | | `TERMINAL_TOTAL_DIFFICULTY` | **TBD** | | `TERMINAL_BLOCK_HASH` | `Hash32()` | -| `TBH_ACTIVATION_EPOCH` | `FAR_FUTURE_EPOCH` | +| `TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH` | `FAR_FUTURE_EPOCH` | ## Containers diff --git a/specs/merge/client-settings.md b/specs/merge/client-settings.md index 63ddfe5ae4..64f912372e 100644 --- a/specs/merge/client-settings.md +++ b/specs/merge/client-settings.md @@ -24,6 +24,6 @@ Except under exceptional scenarios, this setting is not expected to be used. Suf To allow for transition coordination around a specific PoW block, clients must also provide `--terminal-block-hash-override` and `--terminal-block-hash-epoch-override` as configurable settings. * The value provided by `--terminal-block-hash-override` takes precedence over the pre-configured `TERMINAL_BLOCK_HASH` parameter. -* The value provided by `--terminal-block-hash-epoch-override` takes precedence over the pre-configured `TBH_ACTIVATION_EPOCH` parameter. +* The value provided by `--terminal-block-hash-epoch-override` takes precedence over the pre-configured `TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH` parameter. Except under exceptional scenarios, these settings are not expected to be used. Sufficient warning to the user about this exceptional configurable setting should be provided. diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index f1f5e265c6..d5fa08ee26 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -114,7 +114,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: pow_parent = get_pow_block(pow_block.parent_hash) assert is_valid_terminal_pow_block(pow_block, pow_parent) if TERMINAL_BLOCK_HASH != Hash32(): - assert compute_epoch_at_slot(block.slot) >= TBH_ACTIVATION_EPOCH + assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH # Add new block to the store store.blocks[hash_tree_root(block)] = block diff --git a/specs/merge/validator.md b/specs/merge/validator.md index f15f62f3df..e0066ddce1 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -128,7 +128,7 @@ def prepare_execution_payload(state: BeaconState, execution_engine: ExecutionEngine) -> Optional[PayloadId]: if not is_merge_complete(state): is_tbh_override_set = TERMINAL_BLOCK_HASH != Hash32() - if is_tbh_override_set and get_current_epoch(state.slot) < TBH_ACTIVATION_EPOCH: + if is_tbh_override_set and get_current_epoch(state.slot) < TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH: # TBH is set but activation epoch is not yet reached, no prepare payload call is needed return None From 34335e0334763bc0d3f7f56a8c1707add053aac7 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 19 Oct 2021 15:30:49 +0800 Subject: [PATCH 13/46] Remove `prepare_payload` leftover --- setup.py | 7 ------- specs/merge/validator.md | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/setup.py b/setup.py index fa396ab506..6ca46c6368 100644 --- a/setup.py +++ b/setup.py @@ -533,13 +533,6 @@ def notify_forkchoice_updated(self: ExecutionEngine, payload_attributes: Optional[PayloadAttributes]) -> None: pass - def prepare_payload(self: ExecutionEngine, - parent_hash: Hash32, - timestamp: uint64, - random: Bytes32, - feeRecipient: ExecutionAddress) -> PayloadId: - raise NotImplementedError("no default block production") - def get_payload(self: ExecutionEngine, payload_id: PayloadId) -> ExecutionPayload: raise NotImplementedError("no default block production") diff --git a/specs/merge/validator.md b/specs/merge/validator.md index cbca6eedae..379687dc41 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -100,7 +100,7 @@ avoid requiring simple serialize hashing capabilities in the Execution Layer. ### `ExecutionEngine` -*Note*: `prepare_payload` and `get_payload` functions are added to the `ExecutionEngine` protocol for use as a validator. +*Note*: `get_payload` function is added to the `ExecutionEngine` protocol for use as a validator. The body of each of these functions is implementation dependent. The Engine API may be used to implement them with an external execution engine. From 294b60a48befc76177ca72caadbe3a77f27ca8eb Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 19 Oct 2021 09:24:07 -0600 Subject: [PATCH 14/46] pr feedback --- specs/merge/fork-choice.md | 2 ++ specs/merge/validator.md | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index d5fa08ee26..83bba3adf3 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -110,9 +110,11 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # [New in Merge] if is_merge_block(pre_state, block.body): + # Check the parent PoW block of execution payload is a valid terminal PoW block. pow_block = get_pow_block(block.body.execution_payload.parent_hash) pow_parent = get_pow_block(pow_block.parent_hash) assert is_valid_terminal_pow_block(pow_block, pow_parent) + # If `TERMINAL_BLOCK_HASH` is used as an override, the activation epoch must be reached. if TERMINAL_BLOCK_HASH != Hash32(): assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH diff --git a/specs/merge/validator.md b/specs/merge/validator.md index e0066ddce1..88bf96643e 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -127,9 +127,10 @@ def prepare_execution_payload(state: BeaconState, fee_recipient: ExecutionAddress, execution_engine: ExecutionEngine) -> Optional[PayloadId]: if not is_merge_complete(state): - is_tbh_override_set = TERMINAL_BLOCK_HASH != Hash32() - if is_tbh_override_set and get_current_epoch(state.slot) < TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH: - # TBH is set but activation epoch is not yet reached, no prepare payload call is needed + is_terminal_block_hash_set = TERMINAL_BLOCK_HASH != Hash32() + is_activation_epoch_reached = get_current_epoch(state.slot) < TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH + if is_terminal_block_hash_set and 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) From 8023edc94bd13c1f8cb396d1c6dc774dedc4d71b Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 19 Oct 2021 09:28:52 -0600 Subject: [PATCH 15/46] cleanup some copy relatedto removal of prepare_payload --- specs/merge/validator.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 379687dc41..1e90b6b387 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -32,7 +32,7 @@ This document represents the changes to be made in the code of an "honest valida ## Prerequisites -This documens is an extension of the [Altair -- Honest Validator](../altair/validator.md) guide. +This document is an extension of the [Altair -- Honest Validator](../altair/validator.md) guide. All behaviors and definitions defined in this document, and documents it extends, carry over unless explicitly noted or overridden. All terminology, constants, functions, and protocol mechanics defined in the updated Beacon Chain doc of [The Merge](./beacon-chain.md) are requisite for this document and used throughout. @@ -102,8 +102,8 @@ avoid requiring simple serialize hashing capabilities in the Execution Layer. *Note*: `get_payload` function is added to the `ExecutionEngine` protocol for use as a validator. -The body of each of these functions is implementation dependent. -The Engine API may be used to implement them with an external execution engine. +The body of this function is implementation dependent. +The Engine API may be used to implement it with an external execution engine. #### `get_payload` From e787da395403f732df46ba60a44cf117abd18e4c Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 19 Oct 2021 14:35:30 -0600 Subject: [PATCH 16/46] Apply suggestions from code review Co-authored-by: Hsiao-Wei Wang --- specs/merge/fork-choice.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index eb99cce64e..18f3d7d7f1 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -65,7 +65,7 @@ class PowBlock(Container): ### `get_pow_block` Let `get_pow_block(block_hash: Hash32) -> Optional[PowBlock]` be the function that given the hash of the PoW block returns its data. -It may result in `None` if the requested block is not found if execution engine is still syncing. +It may result in `None` if the requested block is not yet available. *Note*: The `eth_getBlockByHash` JSON-RPC method may be used to pull this information from an execution client. @@ -117,7 +117,8 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # [New in Merge] if is_merge_block(pre_state, block.body): - # Note: the unavailable PoW block(s) may be available later + # Note: unavailable PoW block(s) may later become available. + # Nodes should queue such beacon blocks for later processing. pow_block = get_pow_block(block.body.execution_payload.parent_hash) assert pow_block is not None pow_parent = get_pow_block(pow_block.parent_hash) From 8a27a7cb5d3c252c7393c3facb1420c211a3bd22 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 19 Oct 2021 15:46:18 -0600 Subject: [PATCH 17/46] remove union type for eecution-payload txs --- setup.py | 4 ++-- specs/merge/beacon-chain.md | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/setup.py b/setup.py index e414d27217..4524df2ca1 100644 --- a/setup.py +++ b/setup.py @@ -497,7 +497,7 @@ def imports(cls, preset_name: str): return super().imports(preset_name) + f''' from typing import Protocol from eth2spec.altair import {preset_name} as altair -from eth2spec.utils.ssz.ssz_typing import Bytes8, Bytes20, ByteList, ByteVector, uint256, Union +from eth2spec.utils.ssz.ssz_typing import Bytes8, Bytes20, ByteList, ByteVector, uint256 ''' @classmethod @@ -543,7 +543,7 @@ def get_payload(self: ExecutionEngine, payload_id: PayloadId) -> ExecutionPayloa @classmethod def hardcoded_custom_type_dep_constants(cls) -> str: constants = { - 'MAX_BYTES_PER_OPAQUE_TRANSACTION': 'uint64(2**20)', + 'MAX_BYTES_PER_TRANSACTION': 'uint64(2**20)', } return {**super().hardcoded_custom_type_dep_constants(), **constants} diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index 2e1fd88fb1..8fafb07206 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -50,8 +50,7 @@ This patch adds transaction execution to the beacon chain as part of the Merge f | Name | SSZ equivalent | Description | | - | - | - | -| `OpaqueTransaction` | `ByteList[MAX_BYTES_PER_OPAQUE_TRANSACTION]` | a [typed transaction envelope](https://eips.ethereum.org/EIPS/eip-2718#opaque-byte-array-rather-than-an-rlp-array) structured as `TransactionType \|\| TransactionPayload` | -| `Transaction` | `Union[OpaqueTransaction]` | a transaction | +| `Transaction` | `ByteList[MAX_BYTES_PER_TRANSACTION]` | either a [typed transaction envelope](https://eips.ethereum.org/EIPS/eip-2718#opaque-byte-array-rather-than-an-rlp-array) or a legacy transaction| | `ExecutionAddress` | `Bytes20` | Address of account on the execution layer | ## Constants @@ -60,7 +59,7 @@ This patch adds transaction execution to the beacon chain as part of the Merge f | Name | Value | | - | - | -| `MAX_BYTES_PER_OPAQUE_TRANSACTION` | `uint64(2**20)` (= 1,048,576) | +| `MAX_BYTES_PER_TRANSACTION` | `uint64(2**20)` (= 1,048,576) | | `MAX_TRANSACTIONS_PER_PAYLOAD` | `uint64(2**14)` (= 16,384) | | `BYTES_PER_LOGS_BLOOM` | `uint64(2**8)` (= 256) | | `GAS_LIMIT_DENOMINATOR` | `uint64(2**10)` (= 1,024) | From c6af7b3228001b283fcc66d030d0babc5c7ad758 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 19 Oct 2021 15:46:41 -0600 Subject: [PATCH 18/46] add merge execution values to preset yaml files --- presets/mainnet/merge.yaml | 15 ++++++++++++++- presets/minimal/merge.yaml | 15 ++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/presets/mainnet/merge.yaml b/presets/mainnet/merge.yaml index 97f07c7f03..66f7abe282 100644 --- a/presets/mainnet/merge.yaml +++ b/presets/mainnet/merge.yaml @@ -1,3 +1,16 @@ # Mainnet preset - The Merge -# No presets here. +# Execution +# --------------------------------------------------------------- +# 2**20 (= 1,048,576) +MAX_BYTES_PER_TRANSACTION: 1048576 +# 2**14 (= 16,384) +MAX_TRANSACTIONS_PER_PAYLOAD: 16384 +# 2**8 (= 256) +BYTES_PER_LOGS_BLOOM: 256 +# 2**10 (= 1,024) +GAS_LIMIT_DENOMINATOR: 1024 +# 5,000 +MIN_GAS_LIMIT: 5000 +# 2**5 (= 32) +MAX_EXTRA_DATA_BYTES: 32 diff --git a/presets/minimal/merge.yaml b/presets/minimal/merge.yaml index 88aa86c09b..d9b5640ddd 100644 --- a/presets/minimal/merge.yaml +++ b/presets/minimal/merge.yaml @@ -1,3 +1,16 @@ # Minimal preset - The Merge -# No presets here. +# Execution +# --------------------------------------------------------------- +# 2**20 (= 1,048,576) +MAX_BYTES_PER_TRANSACTION: 1048576 +# 2**14 (= 16,384) +MAX_TRANSACTIONS_PER_PAYLOAD: 16384 +# 2**8 (= 256) +BYTES_PER_LOGS_BLOOM: 256 +# 2**10 (= 1,024) +GAS_LIMIT_DENOMINATOR: 1024 +# 5,000 +MIN_GAS_LIMIT: 5000 +# 2**5 (= 32) +MAX_EXTRA_DATA_BYTES: 32 From edb5a023c85fde7756fb711a2cf1d601dfe2b417 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 20 Oct 2021 12:24:00 -0600 Subject: [PATCH 19/46] remove extraneous p2p condition --- specs/merge/p2p-interface.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/specs/merge/p2p-interface.md b/specs/merge/p2p-interface.md index 954c1b008c..3cbba2e234 100644 --- a/specs/merge/p2p-interface.md +++ b/specs/merge/p2p-interface.md @@ -74,8 +74,6 @@ Alias `block = signed_beacon_block.message`, `execution_payload = block.body.exe -- i.e. `execution_payload.timestamp == compute_timestamp_at_slot(state, block.slot)`. - _[REJECT]_ Gas used is less than the gas limit -- i.e. `execution_payload.gas_used <= execution_payload.gas_limit`. - - _[REJECT]_ The execution payload block hash is not equal to the parent hash -- - i.e. `execution_payload.block_hash != execution_payload.parent_hash`. - _[REJECT]_ The execution payload transaction list data is within expected size limits, the data MUST NOT be larger than the SSZ list-limit, and a client MAY be more strict. From cb513aa82d19feb81a85915df33b290df1be595d Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 21 Oct 2021 11:37:02 +0800 Subject: [PATCH 20/46] --fork cli option: fix the case of unset directory path + validate fork name --- tests/core/pyspec/eth2spec/test/conftest.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/conftest.py b/tests/core/pyspec/eth2spec/test/conftest.py index 55613be0a9..751d4d3f17 100644 --- a/tests/core/pyspec/eth2spec/test/conftest.py +++ b/tests/core/pyspec/eth2spec/test/conftest.py @@ -49,6 +49,15 @@ def pytest_addoption(parser): ) +def _validate_fork_name(forks): + for fork in forks: + if fork not in ALL_PHASES: + raise ValueError( + f'The given --fork argument "{fork}" is not an available fork.' + f' The available forks: {ALL_PHASES}' + ) + + @fixture(autouse=True) def preset(request): context.DEFAULT_TEST_PRESET = request.config.getoption("--preset") @@ -56,10 +65,11 @@ def preset(request): @fixture(autouse=True) def run_phases(request): - phases = request.config.getoption("--fork") - if phases: - phases = [phase.lower() for phase in phases] - context.DEFAULT_PYTEST_FORKS = set(phases) + forks = request.config.getoption("--fork", default=None) + if forks: + forks = [phase.lower() for phase in forks] + _validate_fork_name(forks) + context.DEFAULT_PYTEST_FORKS = set(forks) else: context.DEFAULT_PYTEST_FORKS = ALL_PHASES From 405f2e2c2f84f1f435105a6b86244585262706a3 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 21 Oct 2021 23:21:56 +0800 Subject: [PATCH 21/46] Update tests/core/pyspec/eth2spec/test/conftest.py Co-authored-by: Danny Ryan --- tests/core/pyspec/eth2spec/test/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/test/conftest.py b/tests/core/pyspec/eth2spec/test/conftest.py index 751d4d3f17..b3c250c11f 100644 --- a/tests/core/pyspec/eth2spec/test/conftest.py +++ b/tests/core/pyspec/eth2spec/test/conftest.py @@ -67,7 +67,7 @@ def preset(request): def run_phases(request): forks = request.config.getoption("--fork", default=None) if forks: - forks = [phase.lower() for phase in forks] + forks = [fork.lower() for fork in forks] _validate_fork_name(forks) context.DEFAULT_PYTEST_FORKS = set(forks) else: From 314840117d6304854c1574fb6f7065c8f76bfb77 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 26 Oct 2021 15:54:29 +0800 Subject: [PATCH 22/46] Add `test_invalid_previous_source_root` and fix `test_invalid_current_source_root` --- .../test_process_attestation.py | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py b/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py index c303200667..8d9e679982 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py +++ b/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py @@ -275,10 +275,38 @@ def test_invalid_current_source_root(spec, state): state.previous_justified_checkpoint = spec.Checkpoint(epoch=3, root=b'\x01' * 32) state.current_justified_checkpoint = spec.Checkpoint(epoch=4, root=b'\x32' * 32) - attestation = get_valid_attestation(spec, state, slot=(spec.SLOTS_PER_EPOCH * 3) + 1) + next_slots(spec, state, spec.MIN_ATTESTATION_INCLUSION_DELAY) + + attestation = get_valid_attestation(spec, state, slot=spec.SLOTS_PER_EPOCH * 5) + + # Test logic sanity checks: + assert attestation.data.target.epoch == spec.get_current_epoch(state) + assert state.current_justified_checkpoint.root != state.previous_justified_checkpoint.root + assert attestation.data.source.root == state.current_justified_checkpoint.root + + # Make attestation source root invalid: should be current justified, not previous one + attestation.data.source.root = state.previous_justified_checkpoint.root + + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation, False) + + +@with_all_phases +@spec_state_test +def test_invalid_previous_source_root(spec, state): + next_slots(spec, state, spec.SLOTS_PER_EPOCH * 5) + + state.finalized_checkpoint.epoch = 2 + + state.previous_justified_checkpoint = spec.Checkpoint(epoch=3, root=b'\x01' * 32) + state.current_justified_checkpoint = spec.Checkpoint(epoch=4, root=b'\x32' * 32) + + attestation = get_valid_attestation(spec, state, slot=(spec.SLOTS_PER_EPOCH * 4) + 1) next_slots(spec, state, spec.MIN_ATTESTATION_INCLUSION_DELAY) # Test logic sanity checks: + assert attestation.data.target.epoch == spec.get_previous_epoch(state) assert state.current_justified_checkpoint.root != state.previous_justified_checkpoint.root assert attestation.data.source.root == state.previous_justified_checkpoint.root From 71d315950fa12b1cf27616854b37eab10db19e28 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 26 Oct 2021 21:38:22 +0800 Subject: [PATCH 23/46] Fix typo Co-authored-by: Mikhail Kalinin --- specs/merge/fork-choice.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 7b066c8018..42c61b83d4 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -114,7 +114,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: """ Run ``on_block`` upon receiving a new block. - An block that is asserted as invalid due to unavailable PoW block may be valid at a later time, + A block that is asserted as invalid due to unavailable PoW block may be valid at a later time, consider scheduling it for later processing in such case. """ block = signed_block.message From 62504d9efd334edf00793a0f60ab28e2e8eb878b Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 26 Oct 2021 21:59:33 +0800 Subject: [PATCH 24/46] Update `pow_chain` to dict and reuse it instead of calling `get_pow_block` --- specs/merge/validator.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index b32fed194b..678f7fe006 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -49,10 +49,10 @@ Please see related Beacon Chain doc before continuing and use them as a referenc ### `get_pow_block_at_terminal_total_difficulty` ```python -def get_pow_block_at_terminal_total_difficulty(pow_chain: Sequence[PowBlock]) -> Optional[PowBlock]: +def get_pow_block_at_terminal_total_difficulty(pow_chain: Dict[Hash32, PowBlock]) -> Optional[PowBlock]: # `pow_chain` abstractly represents all blocks in the PoW chain for block in pow_chain: - parent = get_pow_block(block.parent_hash) + parent = pow_chain[block.parent_hash] block_reached_ttd = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY if block_reached_ttd and not parent_reached_ttd: @@ -64,13 +64,13 @@ def get_pow_block_at_terminal_total_difficulty(pow_chain: Sequence[PowBlock]) -> ### `get_terminal_pow_block` ```python -def get_terminal_pow_block(pow_chain: Sequence[PowBlock]) -> Optional[PowBlock]: +def get_terminal_pow_block(pow_chain: Dict[Hash32, PowBlock]) -> Optional[PowBlock]: if TERMINAL_BLOCK_HASH != Hash32(): # Terminal block hash override takes precedence over terminal total difficulty - pow_block_overrides = [block for block in pow_chain if block.block_hash == TERMINAL_BLOCK_HASH] - if not any(pow_block_overrides): + if TERMINAL_BLOCK_HASH in pow_chain: + return pow_chain[TERMINAL_BLOCK_HASH] + else: return None - return pow_block_overrides[0] return get_pow_block_at_terminal_total_difficulty(pow_chain) ``` @@ -132,14 +132,14 @@ To obtain an execution payload, a block proposer building a block on top of a `s 1. Set `payload_id = prepare_execution_payload(state, pow_chain, finalized_block_hash, fee_recipient, execution_engine)`, where: * `state` is the state object after applying `process_slots(state, slot)` transition to the resulting state of the parent block processing - * `pow_chain` is a list that abstractly represents all blocks in the PoW chain + * `pow_chain` is a `Dict[Hash32, PowBlock]` dictionary that abstractly represents all blocks in the PoW chain with block hash as the dictionary key * `finalized_block_hash` is the hash of the latest finalized execution payload (`Hash32()` if none yet finalized) * `fee_recipient` is the value suggested to be used for the `coinbase` field of the execution payload ```python def prepare_execution_payload(state: BeaconState, - pow_chain: Sequence[PowBlock], + pow_chain: Dict[Hash32, PowBlock], finalized_block_hash: Hash32, fee_recipient: ExecutionAddress, execution_engine: ExecutionEngine) -> Optional[PayloadId]: From 3e320dacff74d9fe876f351d4488eae59f59831d Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 20 Oct 2021 12:06:58 -0600 Subject: [PATCH 25/46] fix gossip and tx size limits for the merge --- presets/mainnet/merge.yaml | 4 +-- presets/minimal/merge.yaml | 4 +-- specs/merge/beacon-chain.md | 2 +- specs/merge/p2p-interface.md | 49 +++++++++++++++++++++++++++++++++--- 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/presets/mainnet/merge.yaml b/presets/mainnet/merge.yaml index 66f7abe282..10834dcc37 100644 --- a/presets/mainnet/merge.yaml +++ b/presets/mainnet/merge.yaml @@ -2,8 +2,8 @@ # Execution # --------------------------------------------------------------- -# 2**20 (= 1,048,576) -MAX_BYTES_PER_TRANSACTION: 1048576 +# 2**24 (= 16,777,216) +MAX_BYTES_PER_TRANSACTION: 16777216 # 2**14 (= 16,384) MAX_TRANSACTIONS_PER_PAYLOAD: 16384 # 2**8 (= 256) diff --git a/presets/minimal/merge.yaml b/presets/minimal/merge.yaml index d9b5640ddd..ec2dd384d8 100644 --- a/presets/minimal/merge.yaml +++ b/presets/minimal/merge.yaml @@ -2,8 +2,8 @@ # Execution # --------------------------------------------------------------- -# 2**20 (= 1,048,576) -MAX_BYTES_PER_TRANSACTION: 1048576 +# 2**24 (= 16,777,216) +MAX_BYTES_PER_TRANSACTION: 16777216 # 2**14 (= 16,384) MAX_TRANSACTIONS_PER_PAYLOAD: 16384 # 2**8 (= 256) diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index 8fafb07206..9bb7db6c91 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -59,7 +59,7 @@ This patch adds transaction execution to the beacon chain as part of the Merge f | Name | Value | | - | - | -| `MAX_BYTES_PER_TRANSACTION` | `uint64(2**20)` (= 1,048,576) | +| `MAX_BYTES_PER_TRANSACTION` | `uint64(2**24)` (= 16,777,216) | | `MAX_TRANSACTIONS_PER_PAYLOAD` | `uint64(2**14)` (= 16,384) | | `BYTES_PER_LOGS_BLOOM` | `uint64(2**8)` (= 256) | | `GAS_LIMIT_DENOMINATOR` | `uint64(2**10)` (= 1,024) | diff --git a/specs/merge/p2p-interface.md b/specs/merge/p2p-interface.md index 3cbba2e234..e3eb3b152c 100644 --- a/specs/merge/p2p-interface.md +++ b/specs/merge/p2p-interface.md @@ -14,6 +14,7 @@ Readers should understand the Phase 0 and Altair documents and use them as a bas - [Warning](#warning) - [Modifications in the Merge](#modifications-in-the-merge) + - [Configuration](#configuration) - [The gossip domain: gossipsub](#the-gossip-domain-gossipsub) - [Topics and messages](#topics-and-messages) - [Global topics](#global-topics) @@ -23,6 +24,9 @@ Readers should understand the Phase 0 and Altair documents and use them as a bas - [Messages](#messages) - [BeaconBlocksByRange v2](#beaconblocksbyrange-v2) - [BeaconBlocksByRoot v2](#beaconblocksbyroot-v2) +- [Design decision rationale](#design-decision-rationale) + - [Gossipsub](#gossipsub) + - [Why was the max gossip message size increased at the Merge?](#why-was-the-max-gossip-message-size-increased-at-the-merge) @@ -34,6 +38,14 @@ Refer to the note in the [validator guide](./validator.md) for further details. # Modifications in the Merge +## Configuration + +This section outlines modifications constants that are used in this spec. + +| Name | Value | Description | +|---|---|---| +| `GOSSIP_MAX_SIZE_MERGE` | `10 * 2**20` (= 10485760, 10 MiB) | The maximum allowed size of uncompressed gossip messages starting at the Merge upgrade | + ## The gossip domain: gossipsub Some gossip meshes are upgraded in the Merge to support upgraded types. @@ -43,7 +55,12 @@ Some gossip meshes are upgraded in the Merge to support upgraded types. Topics follow the same specification as in prior upgrades. All topics remain stable except the beacon block topic which is updated with the modified type. -The specification around the creation, validation, and dissemination of messages has not changed from the Phase 0 and Altair documents. +The specification around the creation, validation, and dissemination of messages has not changed from the Phase 0 and Altair documents unless explicitly noted here. + +Starting at the Merge upgrade, each gossipsub [message](https://github.com/libp2p/go-libp2p-pubsub/blob/master/pb/rpc.proto#L17-L24) +has a maximum size of `GOSSIP_MAX_SIZE_MERGE`. +Clients MUST reject (fail validation) messages that are over this size limit. +Likewise, clients MUST NOT emit or propagate messages larger than this limit. The derivation of the `message-id` remains stable. @@ -75,8 +92,7 @@ Alias `block = signed_beacon_block.message`, `execution_payload = block.body.exe - _[REJECT]_ Gas used is less than the gas limit -- i.e. `execution_payload.gas_used <= execution_payload.gas_limit`. - _[REJECT]_ The execution payload transaction list data is within expected size limits, - the data MUST NOT be larger than the SSZ list-limit, - and a client MAY be more strict. + the data MUST NOT be larger than the SSZ list-limit. *Note*: Additional [gossip validations](https://github.com/ethereum/devp2p/blob/master/caps/eth.md#block-encoding-and-validity) (see block "data validity" conditions) that rely more heavily on execution-layer state and logic are currently under consideration. @@ -123,3 +139,30 @@ Per `context = compute_fork_digest(fork_version, genesis_validators_root)`: | `GENESIS_FORK_VERSION` | `phase0.SignedBeaconBlock` | | `ALTAIR_FORK_VERSION` | `altair.SignedBeaconBlock` | | `MERGE_FORK_VERSION` | `merge.SignedBeaconBlock` | + +# Design decision rationale + +## Gossipsub + +### Why was the max gossip message size increased at the Merge? + +With the addition of `ExecutionPayload` to `BeaconBlock`s, there is a dynamic +field -- `transactions` -- which can validly exceed the `GOSSIP_MAX_SIZE` limit (1 MiB) put in place in +place at Phase 0. At the `GAS_LIMIT` (~30M) currently seen on mainnet in 2021, a single transaction +filled entirely with data at a cost of 16 gas per byte can create a valid +`ExecutionPayload` of ~2 MiB. Thus we need a size limit to at least account for +current mainnet conditions. + +Geth currently has a [max gossip message size](https://github.com/ethereum/go-ethereum/blob/3ce9f6d96f38712f5d6756e97b59ccc20cc403b3/eth/protocols/eth/protocol.go#L49) of 10 MiB. +To support backward compatibility with this previously defined network limit, +we adopt `GOSSIP_MAX_SIZE_MERGE` of 10 MiB for maximum gossip sizes at the +point of the Merge and beyond. Note, that clients SHOULD still reject objects +that exceed their maximum theoretical bounds which in most cases is less than `GOSSIP_MAX_SIZE_MERGE`. + +Note, that due to additional size induced by the `BeaconBlock` contents (e.g. +proposer signature, operations lists, etc) this does reduce the +theoretical max valid `ExecutionPayload` (and `transactions` list) size as +slightly lower than 10 MiB. Considering that `BeaconBlock` max size is on the +order of 128 KiB in the worst case and the current gas limit (~30M) bounds max blocksize to less +than 2 MiB today, this marginal difference in theoretical bounds will have zero +impact on network functionality and security. From dae5b87c2ba61cab8608fc906c641c6ef92f33d6 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Thu, 21 Oct 2021 15:56:58 -0600 Subject: [PATCH 26/46] increase TX sizes to account for very high gas limits --- presets/mainnet/merge.yaml | 8 ++++---- presets/minimal/merge.yaml | 8 ++++---- specs/merge/beacon-chain.md | 4 ++-- specs/merge/p2p-interface.md | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/presets/mainnet/merge.yaml b/presets/mainnet/merge.yaml index 10834dcc37..3deac4574c 100644 --- a/presets/mainnet/merge.yaml +++ b/presets/mainnet/merge.yaml @@ -2,10 +2,10 @@ # Execution # --------------------------------------------------------------- -# 2**24 (= 16,777,216) -MAX_BYTES_PER_TRANSACTION: 16777216 -# 2**14 (= 16,384) -MAX_TRANSACTIONS_PER_PAYLOAD: 16384 +# 2**30 (= 1,073,741,824) +MAX_BYTES_PER_TRANSACTION: 1073741824 +# 2**20 (= 1,048,576) +MAX_TRANSACTIONS_PER_PAYLOAD: 1048576 # 2**8 (= 256) BYTES_PER_LOGS_BLOOM: 256 # 2**10 (= 1,024) diff --git a/presets/minimal/merge.yaml b/presets/minimal/merge.yaml index ec2dd384d8..5df32ff579 100644 --- a/presets/minimal/merge.yaml +++ b/presets/minimal/merge.yaml @@ -2,10 +2,10 @@ # Execution # --------------------------------------------------------------- -# 2**24 (= 16,777,216) -MAX_BYTES_PER_TRANSACTION: 16777216 -# 2**14 (= 16,384) -MAX_TRANSACTIONS_PER_PAYLOAD: 16384 +# 2**30 (= 1,073,741,824) +MAX_BYTES_PER_TRANSACTION: 1073741824 +# 2**20 (= 1,048,576) +MAX_TRANSACTIONS_PER_PAYLOAD: 1048576 # 2**8 (= 256) BYTES_PER_LOGS_BLOOM: 256 # 2**10 (= 1,024) diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index 9bb7db6c91..aa2a771c5f 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -59,8 +59,8 @@ This patch adds transaction execution to the beacon chain as part of the Merge f | Name | Value | | - | - | -| `MAX_BYTES_PER_TRANSACTION` | `uint64(2**24)` (= 16,777,216) | -| `MAX_TRANSACTIONS_PER_PAYLOAD` | `uint64(2**14)` (= 16,384) | +| `MAX_BYTES_PER_TRANSACTION` | `uint64(2**30)` (= 1,073,741,824) | +| `MAX_TRANSACTIONS_PER_PAYLOAD` | `uint64(2**20)` (= 1,048,576) | | `BYTES_PER_LOGS_BLOOM` | `uint64(2**8)` (= 256) | | `GAS_LIMIT_DENOMINATOR` | `uint64(2**10)` (= 1,024) | | `MIN_GAS_LIMIT` | `uint64(5000)` (= 5,000) | diff --git a/specs/merge/p2p-interface.md b/specs/merge/p2p-interface.md index e3eb3b152c..4283a05176 100644 --- a/specs/merge/p2p-interface.md +++ b/specs/merge/p2p-interface.md @@ -44,7 +44,7 @@ This section outlines modifications constants that are used in this spec. | Name | Value | Description | |---|---|---| -| `GOSSIP_MAX_SIZE_MERGE` | `10 * 2**20` (= 10485760, 10 MiB) | The maximum allowed size of uncompressed gossip messages starting at the Merge upgrade | +| `GOSSIP_MAX_SIZE_MERGE` | `10 * 2**20` (= 10,485,760, 10 MiB) | The maximum allowed size of uncompressed gossip messages starting at the Merge upgrade | ## The gossip domain: gossipsub From bda07e15ad4518acaae7745aa3baca23ebaaf6e9 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 26 Oct 2021 13:42:31 -0600 Subject: [PATCH 27/46] remove extraneous gossip condition --- specs/merge/p2p-interface.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/specs/merge/p2p-interface.md b/specs/merge/p2p-interface.md index 4283a05176..ff37bd6e3e 100644 --- a/specs/merge/p2p-interface.md +++ b/specs/merge/p2p-interface.md @@ -91,8 +91,6 @@ Alias `block = signed_beacon_block.message`, `execution_payload = block.body.exe -- i.e. `execution_payload.timestamp == compute_timestamp_at_slot(state, block.slot)`. - _[REJECT]_ Gas used is less than the gas limit -- i.e. `execution_payload.gas_used <= execution_payload.gas_limit`. - - _[REJECT]_ The execution payload transaction list data is within expected size limits, - the data MUST NOT be larger than the SSZ list-limit. *Note*: Additional [gossip validations](https://github.com/ethereum/devp2p/blob/master/caps/eth.md#block-encoding-and-validity) (see block "data validity" conditions) that rely more heavily on execution-layer state and logic are currently under consideration. From 43a659a51bb958e80438ef9aad5fb2704242ef16 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Wed, 27 Oct 2021 05:55:53 -0600 Subject: [PATCH 28/46] Update tests/formats/ssz_generic/README.md --- tests/formats/ssz_generic/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/formats/ssz_generic/README.md b/tests/formats/ssz_generic/README.md index dacd727796..c46025847a 100644 --- a/tests/formats/ssz_generic/README.md +++ b/tests/formats/ssz_generic/README.md @@ -180,7 +180,7 @@ class ComplexTestStruct(Container): A: uint16 B: List[uint16, 128] C: uint8 - D: List[uint8, 256] + D: ByteList[256] E: VarTestStruct F: Vector[FixedTestStruct, 4] G: Vector[VarTestStruct, 2] From d58b1c57efd862009411cd2686e902ce745a443f Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 27 Oct 2021 10:44:55 -0600 Subject: [PATCH 29/46] bump up MAX_CHUNK_SIZE at the merge as well --- specs/merge/p2p-interface.md | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/specs/merge/p2p-interface.md b/specs/merge/p2p-interface.md index ff37bd6e3e..ea945a1b34 100644 --- a/specs/merge/p2p-interface.md +++ b/specs/merge/p2p-interface.md @@ -27,6 +27,8 @@ Readers should understand the Phase 0 and Altair documents and use them as a bas - [Design decision rationale](#design-decision-rationale) - [Gossipsub](#gossipsub) - [Why was the max gossip message size increased at the Merge?](#why-was-the-max-gossip-message-size-increased-at-the-merge) + - [Req/Resp](#reqresp) + - [Why was the max chunk response size increased at the Merge?](#why-was-the-max-chunk-response-size-increased-at-the-merge) @@ -44,7 +46,8 @@ This section outlines modifications constants that are used in this spec. | Name | Value | Description | |---|---|---| -| `GOSSIP_MAX_SIZE_MERGE` | `10 * 2**20` (= 10,485,760, 10 MiB) | The maximum allowed size of uncompressed gossip messages starting at the Merge upgrade | +| `GOSSIP_MAX_SIZE_MERGE` | `10 * 2**20` (= 10,485,760, 10 MiB) | The maximum allowed size of uncompressed gossip messages starting at the Merge upgrade. | +| `MAX_CHUNK_SIZE_MERGE` | `10 * 2**20` (= 10,485,760, 10 MiB) | The maximum allowed size of uncompressed req/resp chunked responses starting at the Merge upgrade. | ## The gossip domain: gossipsub @@ -108,7 +111,12 @@ details on how to handle transitioning gossip topics for the Merge. **Protocol ID:** `/eth2/beacon_chain/req/beacon_blocks_by_range/2/` -Request and Response remain unchanged. +Request and Response remain unchanged unless explicitly noted here. + +Starting at the Merge upgrade, +a global maximum uncompressed byte size of `MAX_CHUNK_SIZE_MERGE` MUST be applied to all method response chunks +regardless of type specific bounds that *MUST* also be respected. + The Merge fork-digest is introduced to the `context` enum to specify the Merge block type. Per `context = compute_fork_digest(fork_version, genesis_validators_root)`: @@ -164,3 +172,17 @@ slightly lower than 10 MiB. Considering that `BeaconBlock` max size is on the order of 128 KiB in the worst case and the current gas limit (~30M) bounds max blocksize to less than 2 MiB today, this marginal difference in theoretical bounds will have zero impact on network functionality and security. + +## Req/Resp + +### Why was the max chunk response size increased at the Merge? + +Similar to the discussion about the maximum gossip size increase, the +`ExecutionPayload` type can cause `BeaconBlock`s to exceed the 1 MiB bounds put +in place during Phase 0. + +As with the gossip limit, 10 MiB is selected because this is firmly below any +valid block sizes in the range of gas limits expected in the medium term. + +As with both gossip and req/rsp maximum values, type-specific limits should +always by simultaneously respected. From 60163475d719dcfa48cb6a669bc60299cd93dccd Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 28 Oct 2021 16:00:01 +0800 Subject: [PATCH 30/46] PR feedback from @mkalinin --- specs/merge/fork-choice.md | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 42c61b83d4..eec084f62f 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -16,6 +16,7 @@ - [`PowBlock`](#powblock) - [`get_pow_block`](#get_pow_block) - [`is_valid_terminal_pow_block`](#is_valid_terminal_pow_block) + - [`validate_merge_block`](#validate_merge_block) - [Updated fork-choice handlers](#updated-fork-choice-handlers) - [`on_block`](#on_block) @@ -103,6 +104,30 @@ def is_valid_terminal_pow_block(block: PowBlock, parent: PowBlock) -> bool: return is_total_difficulty_reached and is_parent_total_difficulty_valid ``` +### `validate_merge_block` + +```python +def validate_merge_block(block: BeaconBlock) -> None: + """ + Check the parent PoW block of execution payload is a valid terminal PoW block. + + Note: Unavailable PoW block(s) may later become available and a client software MAY delay + a call to ``validate_merge_block`` until the PoW block(s) become available. + """ + pow_block = get_pow_block(block.body.execution_payload.parent_hash) + # Check if `pow_block` is unavailable + assert pow_block is not None + pow_parent = get_pow_block(pow_block.parent_hash) + # Check if `pow_parent` is unavailable + assert pow_parent is not None + # Check if `pow_block` is a valid terminal PoW block + assert is_valid_terminal_pow_block(pow_block, pow_parent) + + # If `TERMINAL_BLOCK_HASH` is used as an override, the activation epoch must be reached. + if TERMINAL_BLOCK_HASH != Hash32(): + assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH +``` + ## Updated fork-choice handlers ### `on_block` @@ -137,17 +162,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # [New in Merge] if is_merge_block(pre_state, block.body): - # Check the parent PoW block of execution payload is a valid terminal PoW block. - # Note: unavailable PoW block(s) may later become available. - # Nodes should queue such beacon blocks for later processing. - pow_block = get_pow_block(block.body.execution_payload.parent_hash) - assert pow_block is not None - pow_parent = get_pow_block(pow_block.parent_hash) - assert pow_parent is not None - assert is_valid_terminal_pow_block(pow_block, pow_parent) - # If `TERMINAL_BLOCK_HASH` is used as an override, the activation epoch must be reached. - if TERMINAL_BLOCK_HASH != Hash32(): - assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH + validate_merge_block(block) # Add new block to the store store.blocks[hash_tree_root(block)] = block From 8ecbffa821818ecd89fa5029dd003662407c6b65 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 28 Oct 2021 16:06:58 +0800 Subject: [PATCH 31/46] Minor comment fix --- specs/merge/fork-choice.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index eec084f62f..e1e12ad7ac 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -111,14 +111,15 @@ def validate_merge_block(block: BeaconBlock) -> None: """ Check the parent PoW block of execution payload is a valid terminal PoW block. - Note: Unavailable PoW block(s) may later become available and a client software MAY delay - a call to ``validate_merge_block`` until the PoW block(s) become available. + Note: Unavailable PoW block(s) may later become available, + and a client software MAY delay a call to ``validate_merge_block`` + until the PoW block(s) become available. """ pow_block = get_pow_block(block.body.execution_payload.parent_hash) - # Check if `pow_block` is unavailable + # Check if `pow_block` is available assert pow_block is not None pow_parent = get_pow_block(pow_block.parent_hash) - # Check if `pow_parent` is unavailable + # Check if `pow_parent` is available assert pow_parent is not None # Check if `pow_block` is a valid terminal PoW block assert is_valid_terminal_pow_block(pow_block, pow_parent) From d0bcf294a8a02c5c9b5e9d2ce7a363c57a5527cf Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 28 Oct 2021 18:42:16 +0800 Subject: [PATCH 32/46] Fix typo --- specs/phase0/validator.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/phase0/validator.md b/specs/phase0/validator.md index 814859c12a..8de16e8056 100644 --- a/specs/phase0/validator.md +++ b/specs/phase0/validator.md @@ -334,7 +334,7 @@ Each deposit in `block.body.deposits` must verify against `state.eth1_data.eth1_ ###### `get_eth1_data` -Let `Eth1Block` be an abstract object representing Eth1 blocks with the `timestamp` and depost contract data available. +Let `Eth1Block` be an abstract object representing Eth1 blocks with the `timestamp` and deposit contract data available. Let `get_eth1_data(block: Eth1Block) -> Eth1Data` be the function that returns the Eth1 data for a given Eth1 block. From 78040ac3ae8572a856880320f849dc62bc4bacff Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 29 Oct 2021 12:39:29 -0600 Subject: [PATCH 33/46] update penalty params for Merge --- presets/mainnet/merge.yaml | 9 ++ presets/minimal/merge.yaml | 9 ++ specs/merge/beacon-chain.md | 109 +++++++++++++++++- .../test/helpers/proposer_slashings.py | 6 +- .../pyspec/eth2spec/test/helpers/rewards.py | 15 ++- .../test_process_slashings.py | 6 +- .../unittests/test_config_invariants.py | 6 +- 7 files changed, 145 insertions(+), 15 deletions(-) diff --git a/presets/mainnet/merge.yaml b/presets/mainnet/merge.yaml index 3deac4574c..a09822f2c7 100644 --- a/presets/mainnet/merge.yaml +++ b/presets/mainnet/merge.yaml @@ -1,5 +1,14 @@ # Mainnet preset - The Merge +# Updated penalty values +# --------------------------------------------------------------- +# 2**24 (= 16,777,216) +INACTIVITY_PENALTY_QUOTIENT_MERGE: 16777216 +# 2**5 (= 32) +MIN_SLASHING_PENALTY_QUOTIENT_MERGE: 32 +# 2 +PROPORTIONAL_SLASHING_MULTIPLIER_MERGE: 3 + # Execution # --------------------------------------------------------------- # 2**30 (= 1,073,741,824) diff --git a/presets/minimal/merge.yaml b/presets/minimal/merge.yaml index 5df32ff579..01a8e56b34 100644 --- a/presets/minimal/merge.yaml +++ b/presets/minimal/merge.yaml @@ -1,5 +1,14 @@ # Minimal preset - The Merge +# Updated penalty values +# --------------------------------------------------------------- +# 2**24 (= 16,777,216) +INACTIVITY_PENALTY_QUOTIENT_MERGE: 16777216 +# 2**5 (= 32) +MIN_SLASHING_PENALTY_QUOTIENT_MERGE: 32 +# 2 +PROPORTIONAL_SLASHING_MULTIPLIER_MERGE: 3 + # Execution # --------------------------------------------------------------- # 2**30 (= 1,073,741,824) diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index aa2a771c5f..50c72d4e1e 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -12,6 +12,8 @@ - [Custom types](#custom-types) - [Constants](#constants) - [Execution](#execution) +- [Preset](#preset) + - [Updated penalty values](#updated-penalty-values) - [Configuration](#configuration) - [Transition settings](#transition-settings) - [Containers](#containers) @@ -28,13 +30,19 @@ - [`is_execution_enabled`](#is_execution_enabled) - [Misc](#misc) - [`compute_timestamp_at_slot`](#compute_timestamp_at_slot) + - [Beacon state accessors](#beacon-state-accessors) + - [Modified `get_inactivity_penalty_deltas`](#modified-get_inactivity_penalty_deltas) + - [Beacon state mutators](#beacon-state-mutators) + - [Modified `slash_validator`](#modified-slash_validator) - [Beacon chain state transition function](#beacon-chain-state-transition-function) - [Execution engine](#execution-engine) - [`execute_payload`](#execute_payload) - [Block processing](#block-processing) - - [Execution payload processing](#execution-payload-processing) - - [`is_valid_gas_limit`](#is_valid_gas_limit) - - [`process_execution_payload`](#process_execution_payload) + - [Execution payload](#execution-payload) + - [`is_valid_gas_limit`](#is_valid_gas_limit) + - [`process_execution_payload`](#process_execution_payload) + - [Epoch processing](#epoch-processing) + - [Slashings](#slashings) - [Testing](#testing) @@ -66,6 +74,20 @@ This patch adds transaction execution to the beacon chain as part of the Merge f | `MIN_GAS_LIMIT` | `uint64(5000)` (= 5,000) | | `MAX_EXTRA_DATA_BYTES` | `2**5` (= 32) | +## Preset + +### Updated penalty values + +The Merge updates a few configuration values to move penalty parameters to their final, maximum security values. + +*Note*: The spec does *not* override previous configuration values but instead creates new values and replaces usage throughout. + +| Name | Value | +| - | - | +| `INACTIVITY_PENALTY_QUOTIENT_MERGE` | `uint64(2**24)` (= 16,777,216) | +| `MIN_SLASHING_PENALTY_QUOTIENT_MERGE` | `uint64(2**5)` (= 32) | +| `PROPORTIONAL_SLASHING_MULTIPLIER_MERGE` | `uint64(3)` | + ## Configuration ### Transition settings @@ -223,6 +245,62 @@ def compute_timestamp_at_slot(state: BeaconState, slot: Slot) -> uint64: return uint64(state.genesis_time + slots_since_genesis * SECONDS_PER_SLOT) ``` +### Beacon state accessors + +#### Modified `get_inactivity_penalty_deltas` + +*Note*: The function `get_inactivity_penalty_deltas` is modified to use `INACTIVITY_PENALTY_QUOTIENT_MERGE`. + +```python +def get_inactivity_penalty_deltas(state: BeaconState) -> Tuple[Sequence[Gwei], Sequence[Gwei]]: + """ + Return the inactivity penalty deltas by considering timely target participation flags and inactivity scores. + """ + rewards = [Gwei(0) for _ in range(len(state.validators))] + penalties = [Gwei(0) for _ in range(len(state.validators))] + previous_epoch = get_previous_epoch(state) + matching_target_indices = get_unslashed_participating_indices(state, TIMELY_TARGET_FLAG_INDEX, previous_epoch) + for index in get_eligible_validator_indices(state): + if index not in matching_target_indices: + penalty_numerator = state.validators[index].effective_balance * state.inactivity_scores[index] + penalty_denominator = INACTIVITY_SCORE_BIAS * INACTIVITY_PENALTY_QUOTIENT_MERGE + penalties[index] += Gwei(penalty_numerator // penalty_denominator) + return rewards, penalties +``` + +### Beacon state mutators + +#### Modified `slash_validator` + +*Note*: The function `slash_validator` is modified to use `MIN_SLASHING_PENALTY_QUOTIENT_MERGE`. + +```python +def slash_validator(state: BeaconState, + slashed_index: ValidatorIndex, + whistleblower_index: ValidatorIndex=None) -> None: + """ + Slash the validator with index ``slashed_index``. + """ + epoch = get_current_epoch(state) + initiate_validator_exit(state, slashed_index) + validator = state.validators[slashed_index] + validator.slashed = True + validator.withdrawable_epoch = max(validator.withdrawable_epoch, Epoch(epoch + EPOCHS_PER_SLASHINGS_VECTOR)) + state.slashings[epoch % EPOCHS_PER_SLASHINGS_VECTOR] += validator.effective_balance + decrease_balance(state, slashed_index, validator.effective_balance // MIN_SLASHING_PENALTY_QUOTIENT_MERGE) + + # Apply proposer and whistleblower rewards + proposer_index = get_beacon_proposer_index(state) + if whistleblower_index is None: + whistleblower_index = proposer_index + whistleblower_reward = Gwei(validator.effective_balance // WHISTLEBLOWER_REWARD_QUOTIENT) + proposer_reward = Gwei(whistleblower_reward * PROPOSER_WEIGHT // WEIGHT_DENOMINATOR) + increase_balance(state, proposer_index, proposer_reward) + increase_balance(state, whistleblower_index, Gwei(whistleblower_reward - proposer_reward)) +``` + + + ## Beacon chain state transition function ### Execution engine @@ -262,9 +340,9 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: process_sync_aggregate(state, block.body.sync_aggregate) ``` -### Execution payload processing +#### Execution payload -#### `is_valid_gas_limit` +##### `is_valid_gas_limit` ```python def is_valid_gas_limit(payload: ExecutionPayload, parent: ExecutionPayloadHeader) -> bool: @@ -287,7 +365,7 @@ def is_valid_gas_limit(payload: ExecutionPayload, parent: ExecutionPayloadHeader return True ``` -#### `process_execution_payload` +##### `process_execution_payload` ```python def process_execution_payload(state: BeaconState, payload: ExecutionPayload, execution_engine: ExecutionEngine) -> None: @@ -322,6 +400,25 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe ) ``` +### Epoch processing + +#### Slashings + +*Note*: The function `process_slashings` is modified to use `PROPORTIONAL_SLASHING_MULTIPLIER_MERGE`. + +```python +def process_slashings(state: BeaconState) -> None: + epoch = get_current_epoch(state) + total_balance = get_total_active_balance(state) + adjusted_total_slashing_balance = min(sum(state.slashings) * PROPORTIONAL_SLASHING_MULTIPLIER_MERGE, total_balance) + for index, validator in enumerate(state.validators): + if validator.slashed and epoch + EPOCHS_PER_SLASHINGS_VECTOR // 2 == validator.withdrawable_epoch: + increment = EFFECTIVE_BALANCE_INCREMENT # Factored out from penalty numerator to avoid uint64 overflow + penalty_numerator = validator.effective_balance // increment * adjusted_total_slashing_balance + penalty = penalty_numerator // total_balance * increment + decrease_balance(state, ValidatorIndex(index), penalty) +``` + ## Testing *Note*: The function `initialize_beacon_state_from_eth1` is modified for pure Merge testing only. diff --git a/tests/core/pyspec/eth2spec/test/helpers/proposer_slashings.py b/tests/core/pyspec/eth2spec/test/helpers/proposer_slashings.py index ac0a1cce2d..6a8cd2dcb6 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/proposer_slashings.py +++ b/tests/core/pyspec/eth2spec/test/helpers/proposer_slashings.py @@ -1,4 +1,4 @@ -from eth2spec.test.context import is_post_altair +from eth2spec.test.context import is_post_altair, is_post_merge from eth2spec.test.helpers.block_header import sign_block_header from eth2spec.test.helpers.keys import pubkey_to_privkey from eth2spec.test.helpers.state import get_balance @@ -9,7 +9,9 @@ def get_min_slashing_penalty_quotient(spec): - if is_post_altair(spec): + if is_post_merge(spec): + return spec.MIN_SLASHING_PENALTY_QUOTIENT_MERGE + elif is_post_altair(spec): return spec.MIN_SLASHING_PENALTY_QUOTIENT_ALTAIR else: return spec.MIN_SLASHING_PENALTY_QUOTIENT diff --git a/tests/core/pyspec/eth2spec/test/helpers/rewards.py b/tests/core/pyspec/eth2spec/test/helpers/rewards.py index ec617bda9b..49a7b0cd10 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/rewards.py +++ b/tests/core/pyspec/eth2spec/test/helpers/rewards.py @@ -2,7 +2,7 @@ from lru import LRU from eth2spec.phase0.mainnet import VALIDATOR_REGISTRY_LIMIT # equal everywhere, fine to import -from eth2spec.test.context import is_post_altair +from eth2spec.test.context import is_post_altair, is_post_merge from eth2spec.test.helpers.state import ( next_epoch, ) @@ -21,6 +21,15 @@ class Deltas(Container): penalties: List[uint64, VALIDATOR_REGISTRY_LIMIT] +def get_inactivity_penalty_quotient(spec): + if is_post_merge(spec): + return spec.INACTIVITY_PENALTY_QUOTIENT_MERGE + elif is_post_altair(spec): + return spec.INACTIVITY_PENALTY_QUOTIENT_ALTAIR + else: + return spec.INACTIVITY_PENALTY_QUOTIENT + + def has_enough_for_reward(spec, state, index): """ Check if base_reward will be non-zero. @@ -45,7 +54,7 @@ def has_enough_for_leak_penalty(spec, state, index): if is_post_altair(spec): return ( state.validators[index].effective_balance * state.inactivity_scores[index] - > spec.config.INACTIVITY_SCORE_BIAS * spec.INACTIVITY_PENALTY_QUOTIENT_ALTAIR + > spec.config.INACTIVITY_SCORE_BIAS * get_inactivity_penalty_quotient(spec) ) else: return ( @@ -266,7 +275,7 @@ def run_get_inactivity_penalty_deltas(spec, state): else: # copied from spec: penalty_numerator = state.validators[index].effective_balance * state.inactivity_scores[index] - penalty_denominator = spec.config.INACTIVITY_SCORE_BIAS * spec.INACTIVITY_PENALTY_QUOTIENT_ALTAIR + penalty_denominator = spec.config.INACTIVITY_SCORE_BIAS * get_inactivity_penalty_quotient(spec) assert penalties[index] == penalty_numerator // penalty_denominator diff --git a/tests/core/pyspec/eth2spec/test/phase0/epoch_processing/test_process_slashings.py b/tests/core/pyspec/eth2spec/test/phase0/epoch_processing/test_process_slashings.py index 1b977640d5..d3593d9ac6 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/epoch_processing/test_process_slashings.py +++ b/tests/core/pyspec/eth2spec/test/phase0/epoch_processing/test_process_slashings.py @@ -1,5 +1,5 @@ from random import Random -from eth2spec.test.context import spec_state_test, with_all_phases, is_post_altair +from eth2spec.test.context import spec_state_test, with_all_phases, is_post_altair, is_post_merge from eth2spec.test.helpers.epoch_processing import ( run_epoch_processing_with, run_epoch_processing_to ) @@ -31,7 +31,9 @@ def slash_validators(spec, state, indices, out_epochs): def get_slashing_multiplier(spec): - if is_post_altair(spec): + if is_post_merge(spec): + return spec.PROPORTIONAL_SLASHING_MULTIPLIER_MERGE + elif is_post_altair(spec): return spec.PROPORTIONAL_SLASHING_MULTIPLIER_ALTAIR else: return spec.PROPORTIONAL_SLASHING_MULTIPLIER 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 b39b011b4f..3183904c8b 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 @@ -1,7 +1,7 @@ from eth2spec.test.context import ( spec_state_test, with_all_phases, - is_post_altair, + is_post_altair, is_post_merge, ) from eth2spec.test.helpers.constants import MAX_UINT_64 @@ -52,7 +52,9 @@ def test_hysteresis_quotient(spec, state): @spec_state_test def test_incentives(spec, state): # Ensure no ETH is minted in slash_validator - if is_post_altair(spec): + if is_post_merge(spec): + assert spec.MIN_SLASHING_PENALTY_QUOTIENT_MERGE <= spec.WHISTLEBLOWER_REWARD_QUOTIENT + elif is_post_altair(spec): assert spec.MIN_SLASHING_PENALTY_QUOTIENT_ALTAIR <= spec.WHISTLEBLOWER_REWARD_QUOTIENT else: assert spec.MIN_SLASHING_PENALTY_QUOTIENT <= spec.WHISTLEBLOWER_REWARD_QUOTIENT From 4da53e62d9c75e921892cc7e95ba2dba64551436 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 29 Oct 2021 14:04:25 -0600 Subject: [PATCH 34/46] Apply suggestions from code review Co-authored-by: Diederik Loerakker --- presets/minimal/merge.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/presets/minimal/merge.yaml b/presets/minimal/merge.yaml index 01a8e56b34..f92674a183 100644 --- a/presets/minimal/merge.yaml +++ b/presets/minimal/merge.yaml @@ -6,7 +6,7 @@ INACTIVITY_PENALTY_QUOTIENT_MERGE: 16777216 # 2**5 (= 32) MIN_SLASHING_PENALTY_QUOTIENT_MERGE: 32 -# 2 +# 3 PROPORTIONAL_SLASHING_MULTIPLIER_MERGE: 3 # Execution From 43da796b29d39813337f6063b386ce5a00b3ea09 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Sun, 31 Oct 2021 06:05:02 -0700 Subject: [PATCH 35/46] Update setup.py Reflect changes to max transaction size in #2686. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index d9a1220705..4bb26bccd1 100644 --- a/setup.py +++ b/setup.py @@ -543,7 +543,7 @@ def get_payload(self: ExecutionEngine, payload_id: PayloadId) -> ExecutionPayloa @classmethod def hardcoded_custom_type_dep_constants(cls) -> str: constants = { - 'MAX_BYTES_PER_TRANSACTION': 'uint64(2**20)', + 'MAX_BYTES_PER_TRANSACTION': 'uint64(2**30)', } return {**super().hardcoded_custom_type_dep_constants(), **constants} From fe8d6edfe3c60bc45d966c32d00d57a0f9141016 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Sun, 31 Oct 2021 09:08:55 -0600 Subject: [PATCH 36/46] Apply suggestions from code review Co-authored-by: Diederik Loerakker --- presets/mainnet/merge.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/presets/mainnet/merge.yaml b/presets/mainnet/merge.yaml index a09822f2c7..4ee3cf8377 100644 --- a/presets/mainnet/merge.yaml +++ b/presets/mainnet/merge.yaml @@ -6,7 +6,7 @@ INACTIVITY_PENALTY_QUOTIENT_MERGE: 16777216 # 2**5 (= 32) MIN_SLASHING_PENALTY_QUOTIENT_MERGE: 32 -# 2 +# 3 PROPORTIONAL_SLASHING_MULTIPLIER_MERGE: 3 # Execution From d5f5f8bf6155a252c2f32f4a59da67751463565f Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Sun, 31 Oct 2021 09:47:58 -0600 Subject: [PATCH 37/46] add modified in merge --- specs/merge/beacon-chain.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index 50c72d4e1e..d164b5564e 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -50,7 +50,10 @@ ## Introduction -This patch adds transaction execution to the beacon chain as part of the Merge fork. +This upgrade adds transaction execution to the beacon chain as part of the Merge fork. + +Additionally, this upgrade introduces the following minor changes: +* Penalty parameter updates to their planned maximally punitive values ## Custom types @@ -263,6 +266,7 @@ def get_inactivity_penalty_deltas(state: BeaconState) -> Tuple[Sequence[Gwei], S for index in get_eligible_validator_indices(state): if index not in matching_target_indices: penalty_numerator = state.validators[index].effective_balance * state.inactivity_scores[index] + # [Modified in Merge] penalty_denominator = INACTIVITY_SCORE_BIAS * INACTIVITY_PENALTY_QUOTIENT_MERGE penalties[index] += Gwei(penalty_numerator // penalty_denominator) return rewards, penalties @@ -287,7 +291,8 @@ def slash_validator(state: BeaconState, validator.slashed = True validator.withdrawable_epoch = max(validator.withdrawable_epoch, Epoch(epoch + EPOCHS_PER_SLASHINGS_VECTOR)) state.slashings[epoch % EPOCHS_PER_SLASHINGS_VECTOR] += validator.effective_balance - decrease_balance(state, slashed_index, validator.effective_balance // MIN_SLASHING_PENALTY_QUOTIENT_MERGE) + slashing_penalty = validator.effective_balance // MIN_SLASHING_PENALTY_QUOTIENT_MERGE # [Modified in Merge] + decrease_balance(state, slashed_index, slashing_penalty) # Apply proposer and whistleblower rewards proposer_index = get_beacon_proposer_index(state) @@ -410,7 +415,10 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe def process_slashings(state: BeaconState) -> None: epoch = get_current_epoch(state) total_balance = get_total_active_balance(state) - adjusted_total_slashing_balance = min(sum(state.slashings) * PROPORTIONAL_SLASHING_MULTIPLIER_MERGE, total_balance) + adjusted_total_slashing_balance = min( + sum(state.slashings) * PROPORTIONAL_SLASHING_MULTIPLIER_MERGE, # [Modified in Merge] + total_balance + ) for index, validator in enumerate(state.validators): if validator.slashed and epoch + EPOCHS_PER_SLASHINGS_VECTOR // 2 == validator.withdrawable_epoch: increment = EFFECTIVE_BALANCE_INCREMENT # Factored out from penalty numerator to avoid uint64 overflow From 3ba3915396d93be3e6d683b6f0c28959d044b1f7 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 1 Nov 2021 12:11:15 +0800 Subject: [PATCH 38/46] Set execution params to presets and add builder checks. --- setup.py | 5 +++++ specs/merge/beacon-chain.md | 7 ++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/setup.py b/setup.py index 4bb26bccd1..61950021e6 100644 --- a/setup.py +++ b/setup.py @@ -634,6 +634,10 @@ def format_constant(name: str, vardef: VariableDefinition) -> str: ssz_dep_constants = '\n'.join(map(lambda x: '%s = %s' % (x, builder.hardcoded_ssz_dep_constants()[x]), builder.hardcoded_ssz_dep_constants())) ssz_dep_constants_verification = '\n'.join(map(lambda x: 'assert %s == %s' % (x, spec_object.ssz_dep_constants[x]), builder.hardcoded_ssz_dep_constants())) custom_type_dep_constants = '\n'.join(map(lambda x: '%s = %s' % (x, builder.hardcoded_custom_type_dep_constants()[x]), builder.hardcoded_custom_type_dep_constants())) + custom_type_dep_constants_verification = '\n'.join(map( + lambda x: 'assert %s == %s' % (x, f'{spec_object.preset_vars[x].type_name}({spec_object.preset_vars[x].value})'), + builder.hardcoded_custom_type_dep_constants(), + )) spec = ( builder.imports(preset_name) + builder.preparations() @@ -654,6 +658,7 @@ def format_constant(name: str, vardef: VariableDefinition) -> str: # 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 '') + + ('\n\n\n' + custom_type_dep_constants_verification if custom_type_dep_constants_verification != '' else '') + '\n' ) return spec diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index d164b5564e..992526591a 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -10,9 +10,8 @@ - [Introduction](#introduction) - [Custom types](#custom-types) -- [Constants](#constants) - - [Execution](#execution) - [Preset](#preset) + - [Execution](#execution) - [Updated penalty values](#updated-penalty-values) - [Configuration](#configuration) - [Transition settings](#transition-settings) @@ -64,7 +63,7 @@ Additionally, this upgrade introduces the following minor changes: | `Transaction` | `ByteList[MAX_BYTES_PER_TRANSACTION]` | either a [typed transaction envelope](https://eips.ethereum.org/EIPS/eip-2718#opaque-byte-array-rather-than-an-rlp-array) or a legacy transaction| | `ExecutionAddress` | `Bytes20` | Address of account on the execution layer | -## Constants +## Preset ### Execution @@ -77,8 +76,6 @@ Additionally, this upgrade introduces the following minor changes: | `MIN_GAS_LIMIT` | `uint64(5000)` (= 5,000) | | `MAX_EXTRA_DATA_BYTES` | `2**5` (= 32) | -## Preset - ### Updated penalty values The Merge updates a few configuration values to move penalty parameters to their final, maximum security values. From dce6b09f519de5089606b7159d735c13418b8157 Mon Sep 17 00:00:00 2001 From: Mikhail Kalinin Date: Mon, 1 Nov 2021 19:31:51 +0600 Subject: [PATCH 39/46] State that validator must consider only fully validated blocks --- specs/merge/validator.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 678f7fe006..90b715b2de 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -122,6 +122,8 @@ def get_payload(self: ExecutionEngine, payload_id: PayloadId) -> ExecutionPayloa All validator responsibilities remain unchanged other than those noted below. Namely, the transition block handling and the addition of `ExecutionPayload`. +*Note*: A validator must not propose on or attest to a block that isn't deemed valid, i.e. hasn't yet passed the beacon chain state transition and execution validations. + ### Block proposal #### Constructing the `BeaconBlockBody` From 52a97ab494faa20a4a637633ddde26814904cd2f Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 29 Oct 2021 12:44:17 -0600 Subject: [PATCH 40/46] remove gas validations from CL --- specs/merge/beacon-chain.md | 27 +-- .../test_process_execution_payload.py | 155 ------------------ 2 files changed, 1 insertion(+), 181 deletions(-) diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index d164b5564e..9cdbda3361 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -39,7 +39,6 @@ - [`execute_payload`](#execute_payload) - [Block processing](#block-processing) - [Execution payload](#execution-payload) - - [`is_valid_gas_limit`](#is_valid_gas_limit) - [`process_execution_payload`](#process_execution_payload) - [Epoch processing](#epoch-processing) - [Slashings](#slashings) @@ -347,39 +346,15 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: #### Execution payload -##### `is_valid_gas_limit` - -```python -def is_valid_gas_limit(payload: ExecutionPayload, parent: ExecutionPayloadHeader) -> bool: - parent_gas_limit = parent.gas_limit - - # Check if the payload used too much gas - if payload.gas_used > payload.gas_limit: - return False - - # Check if the payload changed the gas limit too much - if payload.gas_limit >= parent_gas_limit + parent_gas_limit // GAS_LIMIT_DENOMINATOR: - return False - if payload.gas_limit <= parent_gas_limit - parent_gas_limit // GAS_LIMIT_DENOMINATOR: - return False - - # Check if the gas limit is at least the minimum gas limit - if payload.gas_limit < MIN_GAS_LIMIT: - return False - - return True -``` - ##### `process_execution_payload` ```python def process_execution_payload(state: BeaconState, payload: ExecutionPayload, execution_engine: ExecutionEngine) -> None: - # Verify consistency of the parent hash, block number, base fee per gas and gas limit + # Verify consistency of the parent hash and block number # with respect to the previous execution payload header if is_merge_complete(state): assert payload.parent_hash == state.latest_execution_payload_header.block_hash assert payload.block_number == state.latest_execution_payload_header.block_number + uint64(1) - assert is_valid_gas_limit(payload, state.latest_execution_payload_header) # Verify random assert payload.random == get_randao_mix(state, get_current_epoch(state)) # Verify timestamp diff --git a/tests/core/pyspec/eth2spec/test/merge/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/merge/block_processing/test_process_execution_payload.py index 26a56d150b..d44bad58c5 100644 --- a/tests/core/pyspec/eth2spec/test/merge/block_processing/test_process_execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/merge/block_processing/test_process_execution_payload.py @@ -1,4 +1,3 @@ -from eth2spec.utils.ssz.ssz_typing import uint64 from eth2spec.test.helpers.execution_payload import ( build_empty_execution_payload, get_execution_payload_header, @@ -228,157 +227,3 @@ def test_bad_timestamp_regular_payload(spec, state): execution_payload.timestamp = execution_payload.timestamp + 1 yield from run_execution_payload_processing(spec, state, execution_payload, valid=False) - - -@with_merge_and_later -@spec_state_test -def test_gaslimit_zero_first_payload(spec, state): - # pre-state - state = build_state_with_incomplete_transition(spec, state) - next_slot(spec, state) - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.gas_limit = uint64(0) - - yield from run_execution_payload_processing(spec, state, execution_payload) - - -@with_merge_and_later -@spec_state_test -def test_gaslimit_max_first_payload(spec, state): - # pre-state - state = build_state_with_incomplete_transition(spec, state) - next_slot(spec, state) - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.gas_limit = uint64(2**64 - 1) - - yield from run_execution_payload_processing(spec, state, execution_payload) - - -@with_merge_and_later -@spec_state_test -def test_gaslimit_upper_plus_regular_payload(spec, state): - # pre-state - state = build_state_with_complete_transition(spec, state) - next_slot(spec, state) - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.gas_limit = ( - execution_payload.gas_limit + - execution_payload.gas_limit // spec.GAS_LIMIT_DENOMINATOR - ) - - yield from run_execution_payload_processing(spec, state, execution_payload, valid=False) - - -@with_merge_and_later -@spec_state_test -def test_gaslimit_upper_regular_payload(spec, state): - # pre-state - state = build_state_with_complete_transition(spec, state) - next_slot(spec, state) - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.gas_limit = ( - execution_payload.gas_limit + - execution_payload.gas_limit // spec.GAS_LIMIT_DENOMINATOR - uint64(1) - ) - - yield from run_execution_payload_processing(spec, state, execution_payload) - - -@with_merge_and_later -@spec_state_test -def test_gaslimit_lower_minus_regular_payload(spec, state): - # pre-state - state = build_state_with_complete_transition(spec, state) - next_slot(spec, state) - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.gas_limit = ( - execution_payload.gas_limit - - execution_payload.gas_limit // spec.GAS_LIMIT_DENOMINATOR - ) - - yield from run_execution_payload_processing(spec, state, execution_payload, valid=False) - - -@with_merge_and_later -@spec_state_test -def test_gaslimit_lower_regular_payload(spec, state): - # pre-state - state = build_state_with_complete_transition(spec, state) - next_slot(spec, state) - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.gas_limit = ( - execution_payload.gas_limit - - execution_payload.gas_limit // spec.GAS_LIMIT_DENOMINATOR + uint64(1) - ) - - yield from run_execution_payload_processing(spec, state, execution_payload) - - -@with_merge_and_later -@spec_state_test -def test_gaslimit_minimum_regular_payload(spec, state): - # pre-state - state = build_state_with_complete_transition(spec, state) - next_slot(spec, state) - state.latest_execution_payload_header.gas_limit = spec.MIN_GAS_LIMIT - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.gas_limit = execution_payload.gas_limit - - yield from run_execution_payload_processing(spec, state, execution_payload) - - -@with_merge_and_later -@spec_state_test -def test_gaslimit_minimum_minus_regular_payload(spec, state): - # pre-state - state = build_state_with_complete_transition(spec, state) - next_slot(spec, state) - state.latest_execution_payload_header.gas_limit = spec.MIN_GAS_LIMIT - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.gas_limit = execution_payload.gas_limit - uint64(1) - - yield from run_execution_payload_processing(spec, state, execution_payload, valid=False) - - -@with_merge_and_later -@spec_state_test -def test_gasused_gaslimit_regular_payload(spec, state): - # pre-state - state = build_state_with_complete_transition(spec, state) - next_slot(spec, state) - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.gas_used = execution_payload.gas_limit - - yield from run_execution_payload_processing(spec, state, execution_payload) - - -@with_merge_and_later -@spec_state_test -def test_gasused_gaslimit_plus_regular_payload(spec, state): - # pre-state - state = build_state_with_complete_transition(spec, state) - next_slot(spec, state) - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.gas_used = execution_payload.gas_limit + uint64(1) - - yield from run_execution_payload_processing(spec, state, execution_payload, valid=False) From 879bd2f3e91fe6e25cd98b50caa14b9d998d9974 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Sun, 31 Oct 2021 08:38:54 -0600 Subject: [PATCH 41/46] remove etra gas constants and p2p validations --- presets/mainnet/merge.yaml | 4 ---- presets/minimal/merge.yaml | 4 ---- specs/merge/beacon-chain.md | 2 -- specs/merge/p2p-interface.md | 5 ----- 4 files changed, 15 deletions(-) diff --git a/presets/mainnet/merge.yaml b/presets/mainnet/merge.yaml index 4ee3cf8377..5f2e27bfc2 100644 --- a/presets/mainnet/merge.yaml +++ b/presets/mainnet/merge.yaml @@ -17,9 +17,5 @@ MAX_BYTES_PER_TRANSACTION: 1073741824 MAX_TRANSACTIONS_PER_PAYLOAD: 1048576 # 2**8 (= 256) BYTES_PER_LOGS_BLOOM: 256 -# 2**10 (= 1,024) -GAS_LIMIT_DENOMINATOR: 1024 -# 5,000 -MIN_GAS_LIMIT: 5000 # 2**5 (= 32) MAX_EXTRA_DATA_BYTES: 32 diff --git a/presets/minimal/merge.yaml b/presets/minimal/merge.yaml index f92674a183..21f2f59290 100644 --- a/presets/minimal/merge.yaml +++ b/presets/minimal/merge.yaml @@ -17,9 +17,5 @@ MAX_BYTES_PER_TRANSACTION: 1073741824 MAX_TRANSACTIONS_PER_PAYLOAD: 1048576 # 2**8 (= 256) BYTES_PER_LOGS_BLOOM: 256 -# 2**10 (= 1,024) -GAS_LIMIT_DENOMINATOR: 1024 -# 5,000 -MIN_GAS_LIMIT: 5000 # 2**5 (= 32) MAX_EXTRA_DATA_BYTES: 32 diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index 9cdbda3361..fefe0eefac 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -72,8 +72,6 @@ Additionally, this upgrade introduces the following minor changes: | `MAX_BYTES_PER_TRANSACTION` | `uint64(2**30)` (= 1,073,741,824) | | `MAX_TRANSACTIONS_PER_PAYLOAD` | `uint64(2**20)` (= 1,048,576) | | `BYTES_PER_LOGS_BLOOM` | `uint64(2**8)` (= 256) | -| `GAS_LIMIT_DENOMINATOR` | `uint64(2**10)` (= 1,024) | -| `MIN_GAS_LIMIT` | `uint64(5000)` (= 5,000) | | `MAX_EXTRA_DATA_BYTES` | `2**5` (= 32) | ## Preset diff --git a/specs/merge/p2p-interface.md b/specs/merge/p2p-interface.md index ea945a1b34..0ab3d08258 100644 --- a/specs/merge/p2p-interface.md +++ b/specs/merge/p2p-interface.md @@ -92,11 +92,6 @@ Alias `block = signed_beacon_block.message`, `execution_payload = block.body.exe then validate the following: - _[REJECT]_ The block's execution payload timestamp is correct with respect to the slot -- i.e. `execution_payload.timestamp == compute_timestamp_at_slot(state, block.slot)`. - - _[REJECT]_ Gas used is less than the gas limit -- - i.e. `execution_payload.gas_used <= execution_payload.gas_limit`. - -*Note*: Additional [gossip validations](https://github.com/ethereum/devp2p/blob/master/caps/eth.md#block-encoding-and-validity) -(see block "data validity" conditions) that rely more heavily on execution-layer state and logic are currently under consideration. ### Transitioning the gossip From 7e3ccb706d74a1447963167efae1a91a37b23eee Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 1 Nov 2021 07:57:49 -0600 Subject: [PATCH 42/46] remove block_number validation from CL --- specs/merge/beacon-chain.md | 4 +--- .../test_process_execution_payload.py | 17 ++--------------- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index fefe0eefac..f8b9a8378e 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -348,11 +348,9 @@ 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 and block number - # with respect to the previous execution payload header + # Verify consistency of the parent hash with respect to the previous execution payload header if is_merge_complete(state): assert payload.parent_hash == state.latest_execution_payload_header.block_hash - assert payload.block_number == state.latest_execution_payload_header.block_number + uint64(1) # Verify random assert payload.random == get_randao_mix(state, get_current_epoch(state)) # Verify timestamp diff --git a/tests/core/pyspec/eth2spec/test/merge/block_processing/test_process_execution_payload.py b/tests/core/pyspec/eth2spec/test/merge/block_processing/test_process_execution_payload.py index d44bad58c5..d12a68bf53 100644 --- a/tests/core/pyspec/eth2spec/test/merge/block_processing/test_process_execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/merge/block_processing/test_process_execution_payload.py @@ -172,20 +172,6 @@ def test_bad_random_regular_payload(spec, state): yield from run_execution_payload_processing(spec, state, execution_payload, valid=False) -@with_merge_and_later -@spec_state_test -def test_bad_number_regular_payload(spec, state): - # pre-state - state = build_state_with_complete_transition(spec, state) - next_slot(spec, state) - - # execution payload - execution_payload = build_empty_execution_payload(spec, state) - execution_payload.block_number = execution_payload.block_number + 1 - - yield from run_execution_payload_processing(spec, state, execution_payload, valid=False) - - @with_merge_and_later @spec_state_test def test_bad_everything_regular_payload(spec, state): @@ -196,7 +182,8 @@ def test_bad_everything_regular_payload(spec, state): # execution payload execution_payload = build_empty_execution_payload(spec, state) execution_payload.parent_hash = spec.Hash32() - execution_payload.block_number = execution_payload.block_number + 1 + execution_payload.random = spec.Bytes32() + execution_payload.timestamp = 0 yield from run_execution_payload_processing(spec, state, execution_payload, valid=False) From 16cabb2881e28c8922c63163f8fd38e568c65156 Mon Sep 17 00:00:00 2001 From: Mikhail Kalinin Date: Mon, 1 Nov 2021 21:00:49 +0600 Subject: [PATCH 43/46] Note the Proof-of-Custody for execution in validator.md Co-authored-by: Danny Ryan --- specs/merge/validator.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 90b715b2de..f47ede4f79 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -122,7 +122,7 @@ def get_payload(self: ExecutionEngine, payload_id: PayloadId) -> ExecutionPayloa All validator responsibilities remain unchanged other than those noted below. Namely, the transition block handling and the addition of `ExecutionPayload`. -*Note*: A validator must not propose on or attest to a block that isn't deemed valid, i.e. hasn't yet passed the beacon chain state transition and execution validations. +*Note*: A validator must not propose on or attest to a block that isn't deemed valid, i.e. hasn't yet passed the beacon chain state transition and execution validations. In future upgrades, an "execution Proof-of-Custody" will be integrated to prevent outsourcing of execution payload validations. ### Block proposal From e09a749345dcc6d5055de73350546d5a9096d2b6 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 2 Nov 2021 13:15:28 +0800 Subject: [PATCH 44/46] Revert `custom_type_dep_constants_verification` change --- setup.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/setup.py b/setup.py index 61950021e6..4bb26bccd1 100644 --- a/setup.py +++ b/setup.py @@ -634,10 +634,6 @@ def format_constant(name: str, vardef: VariableDefinition) -> str: ssz_dep_constants = '\n'.join(map(lambda x: '%s = %s' % (x, builder.hardcoded_ssz_dep_constants()[x]), builder.hardcoded_ssz_dep_constants())) ssz_dep_constants_verification = '\n'.join(map(lambda x: 'assert %s == %s' % (x, spec_object.ssz_dep_constants[x]), builder.hardcoded_ssz_dep_constants())) custom_type_dep_constants = '\n'.join(map(lambda x: '%s = %s' % (x, builder.hardcoded_custom_type_dep_constants()[x]), builder.hardcoded_custom_type_dep_constants())) - custom_type_dep_constants_verification = '\n'.join(map( - lambda x: 'assert %s == %s' % (x, f'{spec_object.preset_vars[x].type_name}({spec_object.preset_vars[x].value})'), - builder.hardcoded_custom_type_dep_constants(), - )) spec = ( builder.imports(preset_name) + builder.preparations() @@ -658,7 +654,6 @@ def format_constant(name: str, vardef: VariableDefinition) -> str: # 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 '') - + ('\n\n\n' + custom_type_dep_constants_verification if custom_type_dep_constants_verification != '' else '') + '\n' ) return spec From 948d476fd168b015f55a14f9c90015b94111e168 Mon Sep 17 00:00:00 2001 From: Mikhail Kalinin Date: Tue, 2 Nov 2021 20:20:40 +0600 Subject: [PATCH 45/46] Polish TBH handling functionality --- specs/merge/fork-choice.md | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index e1e12ad7ac..bf1634de4f 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -96,9 +96,6 @@ Used by fork-choice handler, `on_block`. ```python def is_valid_terminal_pow_block(block: PowBlock, parent: PowBlock) -> bool: - if TERMINAL_BLOCK_HASH != Hash32(): - return block.block_hash == TERMINAL_BLOCK_HASH - is_total_difficulty_reached = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY is_parent_total_difficulty_valid = parent.total_difficulty < TERMINAL_TOTAL_DIFFICULTY return is_total_difficulty_reached and is_parent_total_difficulty_valid @@ -115,6 +112,11 @@ def validate_merge_block(block: BeaconBlock) -> None: and a client software MAY delay a call to ``validate_merge_block`` until the PoW block(s) become available. """ + if TERMINAL_BLOCK_HASH != Hash32(): + # If `TERMINAL_BLOCK_HASH` is used as an override, the activation epoch must be reached. + assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH + return block.block_hash == TERMINAL_BLOCK_HASH + pow_block = get_pow_block(block.body.execution_payload.parent_hash) # Check if `pow_block` is available assert pow_block is not None @@ -123,10 +125,6 @@ def validate_merge_block(block: BeaconBlock) -> None: assert pow_parent is not None # Check if `pow_block` is a valid terminal PoW block assert is_valid_terminal_pow_block(pow_block, pow_parent) - - # If `TERMINAL_BLOCK_HASH` is used as an override, the activation epoch must be reached. - if TERMINAL_BLOCK_HASH != Hash32(): - assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH ``` ## Updated fork-choice handlers From 1361078a75de84f9918dff0dca1b073ccfda82f1 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 2 Nov 2021 09:38:49 -0600 Subject: [PATCH 46/46] bump VERSION.txt to 1.1.4 --- 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 9c1218c201..1b87bcd0b0 100644 --- a/tests/core/pyspec/eth2spec/VERSION.txt +++ b/tests/core/pyspec/eth2spec/VERSION.txt @@ -1 +1 @@ -1.1.3 \ No newline at end of file +1.1.4 \ No newline at end of file