From 52a97ab494faa20a4a637633ddde26814904cd2f Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 29 Oct 2021 12:44:17 -0600 Subject: [PATCH 1/3] 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 d164b5564..9cdbda336 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 26a56d150..d44bad58c 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 2/3] 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 4ee3cf837..5f2e27bfc 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 f92674a18..21f2f5929 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 9cdbda336..fefe0eefa 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 ea945a1b3..0ab3d0825 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 3/3] 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 fefe0eefa..f8b9a8378 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 d44bad58c..d12a68bf5 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)