From d958ed70b49e6480f43b1defdbd4f57b3e0f7558 Mon Sep 17 00:00:00 2001 From: Potuz Date: Fri, 28 Oct 2022 11:38:32 -0300 Subject: [PATCH 01/36] Implement withdrawals without queues --- specs/capella/beacon-chain.md | 147 ++++++---------------------------- specs/capella/fork.md | 4 +- 2 files changed, 25 insertions(+), 126 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 3b6dc4453..880db0889 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -7,12 +7,10 @@ - [Introduction](#introduction) -- [Custom types](#custom-types) - [Constants](#constants) - [Domain types](#domain-types) - [Preset](#preset) - [Misc](#misc) - - [State list lengths](#state-list-lengths) - [Max operations per block](#max-operations-per-block) - [Execution](#execution) - [Configuration](#configuration) @@ -27,16 +25,11 @@ - [`BeaconBlockBody`](#beaconblockbody) - [`BeaconState`](#beaconstate) - [Helpers](#helpers) - - [Beacon state mutators](#beacon-state-mutators) - - [`withdraw_balance`](#withdraw_balance) - [Predicates](#predicates) - [`has_eth1_withdrawal_credential`](#has_eth1_withdrawal_credential) - [`is_fully_withdrawable_validator`](#is_fully_withdrawable_validator) - [`is_partially_withdrawable_validator`](#is_partially_withdrawable_validator) - [Beacon chain state transition function](#beacon-chain-state-transition-function) - - [Epoch processing](#epoch-processing) - - [Full withdrawals](#full-withdrawals) - - [Partial withdrawals](#partial-withdrawals) - [Block processing](#block-processing) - [New `process_withdrawals`](#new-process_withdrawals) - [Modified `process_execution_payload`](#modified-process_execution_payload) @@ -57,14 +50,6 @@ to validator withdrawals. Including: * Operation to change from `BLS_WITHDRAWAL_PREFIX` to `ETH1_ADDRESS_WITHDRAWAL_PREFIX` versioned withdrawal credentials to enable withdrawals for a validator. -## Custom types - -We define the following Python custom types for type hinting and readability: - -| Name | SSZ equivalent | Description | -| - | - | - | -| `WithdrawalIndex` | `uint64` | an index of a `Withdrawal` | - ## Constants ### Domain types @@ -73,20 +58,6 @@ We define the following Python custom types for type hinting and readability: | - | - | | `DOMAIN_BLS_TO_EXECUTION_CHANGE` | `DomainType('0x0A000000')` | -## Preset - -### Misc - -| Name | Value | -| - | - | -| `MAX_PARTIAL_WITHDRAWALS_PER_EPOCH` | `uint64(2**8)` (= 256) | - -### State list lengths - -| Name | Value | Unit | -| - | - | :-: | -| `WITHDRAWAL_QUEUE_LIMIT` | `uint64(2**40)` (= 1,099,511,627,776) | withdrawals enqueued in state | - ### Max operations per block | Name | Value | @@ -99,8 +70,6 @@ We define the following Python custom types for type hinting and readability: | - | - | - | | `MAX_WITHDRAWALS_PER_PAYLOAD` | `uint64(2**4)` (= 16) | Maximum amount of withdrawals allowed in each payload | -## Configuration - ## Containers ### New containers @@ -109,7 +78,6 @@ We define the following Python custom types for type hinting and readability: ```python class Withdrawal(Container): - index: WithdrawalIndex validator_index: ValidatorIndex address: ExecutionAddress amount: Gwei @@ -241,32 +209,11 @@ class BeaconState(Container): # Execution latest_execution_payload_header: ExecutionPayloadHeader # Withdrawals - withdrawal_queue: List[Withdrawal, WITHDRAWAL_QUEUE_LIMIT] # [New in Capella] - next_withdrawal_index: WithdrawalIndex # [New in Capella] - next_partial_withdrawal_validator_index: ValidatorIndex # [New in Capella] + last_withdrawal_validator_index: ValidatorIndex # [New in Capella] ``` ## Helpers -### Beacon state mutators - -#### `withdraw_balance` - -```python -def withdraw_balance(state: BeaconState, validator_index: ValidatorIndex, amount: Gwei) -> None: - # Decrease the validator's balance - decrease_balance(state, validator_index, amount) - # Create a corresponding withdrawal receipt - withdrawal = Withdrawal( - index=state.next_withdrawal_index, - validator_index=validator_index, - address=ExecutionAddress(state.validators[validator_index].withdrawal_credentials[12:]), - amount=amount, - ) - state.next_withdrawal_index = WithdrawalIndex(state.next_withdrawal_index + 1) - state.withdrawal_queue.append(withdrawal) -``` - ### Predicates #### `has_eth1_withdrawal_credential` @@ -307,66 +254,6 @@ def is_partially_withdrawable_validator(validator: Validator, balance: Gwei) -> ## Beacon chain state transition function -### Epoch processing - -```python -def process_epoch(state: BeaconState) -> None: - process_justification_and_finalization(state) - process_inactivity_updates(state) - process_rewards_and_penalties(state) - process_registry_updates(state) - process_slashings(state) - process_eth1_data_reset(state) - process_effective_balance_updates(state) - process_slashings_reset(state) - process_randao_mixes_reset(state) - process_historical_roots_update(state) - process_participation_flag_updates(state) - process_sync_committee_updates(state) - process_full_withdrawals(state) # [New in Capella] - process_partial_withdrawals(state) # [New in Capella] - -``` - -#### Full withdrawals - -*Note*: The function `process_full_withdrawals` is new. - -```python -def process_full_withdrawals(state: BeaconState) -> None: - current_epoch = get_current_epoch(state) - for index in range(len(state.validators)): - balance = state.balances[index] - validator = state.validators[index] - if is_fully_withdrawable_validator(validator, balance, current_epoch): - withdraw_balance(state, ValidatorIndex(index), balance) -``` - -#### Partial withdrawals - -*Note*: The function `process_partial_withdrawals` is new. - -```python -def process_partial_withdrawals(state: BeaconState) -> None: - partial_withdrawals_count = 0 - # Begin where we left off last time - validator_index = state.next_partial_withdrawal_validator_index - for _ in range(len(state.validators)): - balance = state.balances[validator_index] - validator = state.validators[validator_index] - if is_partially_withdrawable_validator(validator, balance): - withdraw_balance(state, validator_index, balance - MAX_EFFECTIVE_BALANCE) - partial_withdrawals_count += 1 - - # Iterate to next validator to check for partial withdrawal - validator_index = ValidatorIndex((validator_index + 1) % len(state.validators)) - # Exit if performed maximum allowable withdrawals - if partial_withdrawals_count == MAX_PARTIAL_WITHDRAWALS_PER_EPOCH: - break - - state.next_partial_withdrawal_validator_index = validator_index -``` - ### Block processing ```python @@ -385,15 +272,29 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: ```python def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: - num_withdrawals = min(MAX_WITHDRAWALS_PER_PAYLOAD, len(state.withdrawal_queue)) - dequeued_withdrawals = state.withdrawal_queue[:num_withdrawals] - - assert len(dequeued_withdrawals) == len(payload.withdrawals) - for dequeued_withdrawal, withdrawal in zip(dequeued_withdrawals, payload.withdrawals): - assert dequeued_withdrawal == withdrawal - - # Remove dequeued withdrawals from state - state.withdrawal_queue = state.withdrawal_queue[num_withdrawals:] + epoch = get_current_epoch(state) + index = state.last_withdrawal_validator_index + for withdrawal in payload.withdrawals: + index = ValidatorIndex((index + 1) % len(state.validators)) + while index != withdrawal.validator_index: + val state.validators[index] + balance = state.balances[index] + assert !is_fully_withdrawable_validator(val, balance, epoch): + assert !is_partially_withdrawable_validator(val, balance): + index += ValidatorIndex((index + 1) % len(state.validators)) + assert withdrawal.validator_index == index + val = state.validators[index] + balance = state.balances[index] + fully_withdrawable = is_fully_withdrawable_validator(val, balance, epoch) + partial_withdrawable = is_partially_withdrawable_validator(val, balance) + assert fully_withdrawable || partial_withdrawable + if fully_withdrawable: + assert withdrawal.amount == balance + decrease_balance(state, index, balance) + else: + assert withdrawal.amount == balance - MAX_EFFECTIVE_BALANCE + decrease_balance(state, index, balance - MAX_EFFECTIVE_BALANCE) + state.last_withdrawal_validator_index = index ``` #### Modified `process_execution_payload` diff --git a/specs/capella/fork.md b/specs/capella/fork.md index bc3c95aed..27e2afcac 100644 --- a/specs/capella/fork.md +++ b/specs/capella/fork.md @@ -129,9 +129,7 @@ def upgrade_to_capella(pre: bellatrix.BeaconState) -> BeaconState: # Execution-layer latest_execution_payload_header=latest_execution_payload_header, # Withdrawals - withdrawal_queue=[], - next_withdrawal_index=WithdrawalIndex(0), - next_partial_withdrawal_validator_index=ValidatorIndex(0), + last_withdrawal_validator_index=ValidatorIndex(0), ) return post From 88f49382e0f1716a7941cccf3f885b4a9eda92ec Mon Sep 17 00:00:00 2001 From: Potuz Date: Fri, 28 Oct 2022 11:41:40 -0300 Subject: [PATCH 02/36] remove unnecessary constants --- presets/mainnet/capella.yaml | 12 ------------ presets/minimal/capella.yaml | 12 ------------ 2 files changed, 24 deletions(-) diff --git a/presets/mainnet/capella.yaml b/presets/mainnet/capella.yaml index f04bdd06f..62306c8dc 100644 --- a/presets/mainnet/capella.yaml +++ b/presets/mainnet/capella.yaml @@ -1,23 +1,11 @@ # Mainnet preset - Capella # Misc -# --------------------------------------------------------------- -# 2**8 (= 256) withdrawals -MAX_PARTIAL_WITHDRAWALS_PER_EPOCH: 256 - - -# State list lengths -# --------------------------------------------------------------- -# 2**40 (= 1,099,511,627,776) withdrawals -WITHDRAWAL_QUEUE_LIMIT: 1099511627776 - - # Max operations per block # --------------------------------------------------------------- # 2**4 (= 16) MAX_BLS_TO_EXECUTION_CHANGES: 16 - # Execution # --------------------------------------------------------------- # 2**4 (= 16) withdrawals diff --git a/presets/minimal/capella.yaml b/presets/minimal/capella.yaml index 0476172a1..7bddd1e23 100644 --- a/presets/minimal/capella.yaml +++ b/presets/minimal/capella.yaml @@ -1,17 +1,5 @@ # Minimal preset - Capella -# Misc -# --------------------------------------------------------------- -# [customized] 16 for more interesting tests at low validator count -MAX_PARTIAL_WITHDRAWALS_PER_EPOCH: 16 - - -# State list lengths -# --------------------------------------------------------------- -# 2**40 (= 1,099,511,627,776) withdrawals -WITHDRAWAL_QUEUE_LIMIT: 1099511627776 - - # Max operations per block # --------------------------------------------------------------- # 2**4 (= 16) From f506087af5af483e31c4570bb55d8098828e5379 Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 31 Oct 2022 14:46:01 -0300 Subject: [PATCH 03/36] rebase on top of develop --- specs/eip4844/p2p-interface.md | 8 ++++++++ .../eip4844/unittests/validator/test_validator.py | 15 +++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/specs/eip4844/p2p-interface.md b/specs/eip4844/p2p-interface.md index 1576fe96f..6dc6e662e 100644 --- a/specs/eip4844/p2p-interface.md +++ b/specs/eip4844/p2p-interface.md @@ -57,6 +57,14 @@ class SignedBeaconBlockAndBlobsSidecar(Container): blobs_sidecar: BlobsSidecar ``` +### `SignedBeaconBlockAndBlobsSidecar` + +```python +class SignedBeaconBlockAndBlobsSidecar(Container): + beacon_block: SignedBeaconBlock + blobs_sidecar: BlobsSidecar +``` + ## The gossip domain: gossipsub Some gossip meshes are upgraded in the fork of EIP4844 to support upgraded types. diff --git a/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py index 634daca2d..8b9d6928f 100644 --- a/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py @@ -13,6 +13,21 @@ from eth2spec.test.helpers.sharding import ( ) +@with_eip4844_and_later +@spec_state_test +def test_verify_kzg_proof(spec, state): + x = 3 + polynomial = get_sample_blob(spec) + polynomial = [int(i) for i in polynomial] + commitment = spec.blob_to_kzg_commitment(polynomial) + + # Get the proof + proof = spec.compute_kzg_proof(polynomial, x) + + y = spec.evaluate_polynomial_in_evaluation_form(polynomial, x) + assert spec.verify_kzg_proof(commitment, x, y, proof) + + def _run_validate_blobs_sidecar_test(spec, state, blob_count): block = build_empty_block_for_next_slot(spec, state) opaque_tx, blobs, blob_kzg_commitments = get_sample_opaque_tx(spec, blob_count=blob_count) From 7dbd50e958ce5fc226d6e8839db37309d78b10e3 Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 31 Oct 2022 15:32:16 -0300 Subject: [PATCH 04/36] Reviewers' comments - Implemented many of Alex's comments including reinsertion of the withdrawal index in the BeaconState - Implemented Sean's suggestion of separating the logic for block production so that one matches the list in the payload with what `get_expected_withdrawals` returns - Changed `get_expected_wihdrawals` to match the current behavior and moved it to `beacon-chain.md` --- specs/capella/beacon-chain.md | 79 +++++++++++++++++++++++++---------- specs/capella/validator.md | 7 ---- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 880db0889..cbf288a21 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -7,6 +7,7 @@ - [Introduction](#introduction) +- [Custom types](#custom-types) - [Constants](#constants) - [Domain types](#domain-types) - [Preset](#preset) @@ -31,6 +32,7 @@ - [`is_partially_withdrawable_validator`](#is_partially_withdrawable_validator) - [Beacon chain state transition function](#beacon-chain-state-transition-function) - [Block processing](#block-processing) + - [New `get_expected_withdrawals`](#new-get_expected_withdrawals) - [New `process_withdrawals`](#new-process_withdrawals) - [Modified `process_execution_payload`](#modified-process_execution_payload) - [Modified `process_operations`](#modified-process_operations) @@ -50,6 +52,14 @@ to validator withdrawals. Including: * Operation to change from `BLS_WITHDRAWAL_PREFIX` to `ETH1_ADDRESS_WITHDRAWAL_PREFIX` versioned withdrawal credentials to enable withdrawals for a validator. +## Custom types + +We define the following Python custom types for type hinting and readability: + +| Name | SSZ equivalent | Description | +| - | - | - | +| `WithdrawalIndex` | `uint64` | an index of a `Withdrawal` | + ## Constants ### Domain types @@ -78,6 +88,7 @@ to validator withdrawals. Including: ```python class Withdrawal(Container): + index: WithdrawalIndex validator_index: ValidatorIndex address: ExecutionAddress amount: Gwei @@ -209,6 +220,7 @@ class BeaconState(Container): # Execution latest_execution_payload_header: ExecutionPayloadHeader # Withdrawals + next_withdrawal_index: WithdrawalIndex # [New in Capella] last_withdrawal_validator_index: ValidatorIndex # [New in Capella] ``` @@ -268,33 +280,54 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: process_sync_aggregate(state, block.body.sync_aggregate) ``` +#### New `get_expected_withdrawals` + +```python +def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: + epoch = get_current_epoch(state) + withdrawal_index = state.next_withdrawal_index + index = ValidatorIndex((state.last_withdrawal_validator_index + 1) % len(state.validators)) + ret = [] + probed = 0 + while (len(ret) < MAX_WITHDRAWALS_PER_PAYLOAD) and (probed < len(state.validators)): + val = state.validators[index] + balance = state.balances[index] + if is_fully_withdrawable(val, balance, epoch): + withdrawal = Withdrawal( + index=withdrawal_index, + validator_index=index, + address=ExecutionAddress(val.withdrawal_credentials[12:]), + amount=balance, + ) + ret.append(withdrawal) + withdrawal_index = WithdrawalIndex(withdrawal_index + 1) + else if is_partially_withdrawable(val, balance): + withdrawal = Withdrawal( + index=withdrawal_index, + validator_index=index, + address=ExecutionAddress(val.withdrawal_credentials[12:]), + amount=balance-MAX_EFFECTIVE_BALANCE, + ) + ret.append(withdrawal) + withdrawal_index = WithdrawalIndex(withdrawal_index + 1) + probed += 1 + index = ValidatorIndex((index + probed) % len(state.validators)) + return ret +``` + #### New `process_withdrawals` ```python def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: - epoch = get_current_epoch(state) - index = state.last_withdrawal_validator_index - for withdrawal in payload.withdrawals: - index = ValidatorIndex((index + 1) % len(state.validators)) - while index != withdrawal.validator_index: - val state.validators[index] - balance = state.balances[index] - assert !is_fully_withdrawable_validator(val, balance, epoch): - assert !is_partially_withdrawable_validator(val, balance): - index += ValidatorIndex((index + 1) % len(state.validators)) - assert withdrawal.validator_index == index - val = state.validators[index] - balance = state.balances[index] - fully_withdrawable = is_fully_withdrawable_validator(val, balance, epoch) - partial_withdrawable = is_partially_withdrawable_validator(val, balance) - assert fully_withdrawable || partial_withdrawable - if fully_withdrawable: - assert withdrawal.amount == balance - decrease_balance(state, index, balance) - else: - assert withdrawal.amount == balance - MAX_EFFECTIVE_BALANCE - decrease_balance(state, index, balance - MAX_EFFECTIVE_BALANCE) - state.last_withdrawal_validator_index = index + expected_withdrawals = get_expected_withdrawals(state) + assert len(payload.withdrawals) == len(expected_withdrawals) + + for expected_withdrawal, withdrawal in zip(get_expected_withdrawals(state), payload.withdrawals): + assert withdrawal == expected_withdrawal + decrease_balance(state, withdrawal.validator_index, withdrawal.amount) + + state.next_withdrawal_index = WithdrawalIndex(withdrawal.index + 1) + state.last_withdrawal_validator_index = withdrawal.validator_index ``` #### Modified `process_execution_payload` diff --git a/specs/capella/validator.md b/specs/capella/validator.md index 85dbd7e00..4f9d538e0 100644 --- a/specs/capella/validator.md +++ b/specs/capella/validator.md @@ -58,13 +58,6 @@ All validator responsibilities remain unchanged other than those noted below. expected withdrawals for the slot must be gathered from the `state` (utilizing the helper `get_expected_withdrawals`) and passed into the `ExecutionEngine` within `prepare_execution_payload`. - -```python -def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: - num_withdrawals = min(MAX_WITHDRAWALS_PER_PAYLOAD, len(state.withdrawal_queue)) - return state.withdrawal_queue[:num_withdrawals] -``` - *Note*: The only change made to `prepare_execution_payload` is to call `get_expected_withdrawals()` to set the new `withdrawals` field of `PayloadAttributes`. From 49a2519da27e35554627816faa18b6163da289b9 Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 31 Oct 2022 15:55:53 -0300 Subject: [PATCH 05/36] lint --- specs/capella/beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index cbf288a21..349ca02af 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -301,7 +301,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: ) ret.append(withdrawal) withdrawal_index = WithdrawalIndex(withdrawal_index + 1) - else if is_partially_withdrawable(val, balance): + elif is_partially_withdrawable(val, balance): withdrawal = Withdrawal( index=withdrawal_index, validator_index=index, From c156ea6adcc798a0377ef81435480e5429f2c298 Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 31 Oct 2022 16:52:07 -0300 Subject: [PATCH 06/36] linting and typo --- specs/capella/beacon-chain.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 349ca02af..cadebcd18 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -292,7 +292,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: while (len(ret) < MAX_WITHDRAWALS_PER_PAYLOAD) and (probed < len(state.validators)): val = state.validators[index] balance = state.balances[index] - if is_fully_withdrawable(val, balance, epoch): + if is_fully_withdrawable_validator(val, balance, epoch): withdrawal = Withdrawal( index=withdrawal_index, validator_index=index, @@ -301,12 +301,12 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: ) ret.append(withdrawal) withdrawal_index = WithdrawalIndex(withdrawal_index + 1) - elif is_partially_withdrawable(val, balance): + elif is_partially_withdrawable_validator(val, balance): withdrawal = Withdrawal( index=withdrawal_index, validator_index=index, address=ExecutionAddress(val.withdrawal_credentials[12:]), - amount=balance-MAX_EFFECTIVE_BALANCE, + amount=balance - MAX_EFFECTIVE_BALANCE, ) ret.append(withdrawal) withdrawal_index = WithdrawalIndex(withdrawal_index + 1) From 0f74ab5686e4a13fa153ddfcf48f250fb8891bdc Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 31 Oct 2022 17:00:20 -0300 Subject: [PATCH 07/36] type annotation --- specs/capella/beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index cadebcd18..e08813c22 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -287,7 +287,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: epoch = get_current_epoch(state) withdrawal_index = state.next_withdrawal_index index = ValidatorIndex((state.last_withdrawal_validator_index + 1) % len(state.validators)) - ret = [] + ret: Sequence[Withdrawal] = [] probed = 0 while (len(ret) < MAX_WITHDRAWALS_PER_PAYLOAD) and (probed < len(state.validators)): val = state.validators[index] From 5b92eae08f2c24afbe9c51e34303476356ecb20b Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 31 Oct 2022 17:18:06 -0300 Subject: [PATCH 08/36] type annotation --- specs/capella/beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index e08813c22..db5b49dfe 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -287,7 +287,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: epoch = get_current_epoch(state) withdrawal_index = state.next_withdrawal_index index = ValidatorIndex((state.last_withdrawal_validator_index + 1) % len(state.validators)) - ret: Sequence[Withdrawal] = [] + ret: List[Withdrawal] = [] probed = 0 while (len(ret) < MAX_WITHDRAWALS_PER_PAYLOAD) and (probed < len(state.validators)): val = state.validators[index] From ff1dd90012caa15dea3c507436efcf20b3dbc63d Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 31 Oct 2022 17:35:32 -0300 Subject: [PATCH 09/36] fix toc --- specs/capella/beacon-chain.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index db5b49dfe..71dbbfc21 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -11,10 +11,8 @@ - [Constants](#constants) - [Domain types](#domain-types) - [Preset](#preset) - - [Misc](#misc) - [Max operations per block](#max-operations-per-block) - [Execution](#execution) -- [Configuration](#configuration) - [Containers](#containers) - [New containers](#new-containers) - [`Withdrawal`](#withdrawal) @@ -68,6 +66,8 @@ We define the following Python custom types for type hinting and readability: | - | - | | `DOMAIN_BLS_TO_EXECUTION_CHANGE` | `DomainType('0x0A000000')` | +## Preset + ### Max operations per block | Name | Value | From 7e4d1696da22f4716c7a859504232ab4c79fdb6f Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 31 Oct 2022 17:48:56 -0300 Subject: [PATCH 10/36] update minimal preset --- presets/minimal/capella.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/presets/minimal/capella.yaml b/presets/minimal/capella.yaml index 7bddd1e23..476738f3f 100644 --- a/presets/minimal/capella.yaml +++ b/presets/minimal/capella.yaml @@ -8,5 +8,5 @@ MAX_BLS_TO_EXECUTION_CHANGES: 16 # Execution # --------------------------------------------------------------- -# [customized] Lower than MAX_PARTIAL_WITHDRAWALS_PER_EPOCH so not all processed in one block -MAX_WITHDRAWALS_PER_PAYLOAD: 8 +# 2**2 (= 4) +MAX_WITHDRAWALS_PER_PAYLOAD: 4 From a14479a70f7f9909bc6c8d1ab58727d0c25f7f98 Mon Sep 17 00:00:00 2001 From: Potuz Date: Tue, 1 Nov 2022 17:06:35 -0300 Subject: [PATCH 11/36] g11tech review --- specs/capella/beacon-chain.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 71dbbfc21..318346347 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -288,8 +288,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: withdrawal_index = state.next_withdrawal_index index = ValidatorIndex((state.last_withdrawal_validator_index + 1) % len(state.validators)) ret: List[Withdrawal] = [] - probed = 0 - while (len(ret) < MAX_WITHDRAWALS_PER_PAYLOAD) and (probed < len(state.validators)): + for probed in range(len(state.validators))): val = state.validators[index] balance = state.balances[index] if is_fully_withdrawable_validator(val, balance, epoch): @@ -310,6 +309,8 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: ) ret.append(withdrawal) withdrawal_index = WithdrawalIndex(withdrawal_index + 1) + if len(ret) == MAX_WITHDRAWALS_PER_PAYLOAD: + break probed += 1 index = ValidatorIndex((index + probed) % len(state.validators)) return ret From 39e6ec5d14d98127003c84cc14e9acfa715b0de5 Mon Sep 17 00:00:00 2001 From: Potuz Date: Tue, 1 Nov 2022 17:20:20 -0300 Subject: [PATCH 12/36] lint --- specs/capella/beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 318346347..063dc8e82 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -288,7 +288,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: withdrawal_index = state.next_withdrawal_index index = ValidatorIndex((state.last_withdrawal_validator_index + 1) % len(state.validators)) ret: List[Withdrawal] = [] - for probed in range(len(state.validators))): + for probed in range(len(state.validators)): val = state.validators[index] balance = state.balances[index] if is_fully_withdrawable_validator(val, balance, epoch): From ad3654848083bd31b231930c9cd617674a785247 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 3 Nov 2022 11:46:07 -0500 Subject: [PATCH 13/36] Fix auto-rebase errors --- specs/eip4844/p2p-interface.md | 8 -------- .../eip4844/unittests/validator/test_validator.py | 15 --------------- 2 files changed, 23 deletions(-) diff --git a/specs/eip4844/p2p-interface.md b/specs/eip4844/p2p-interface.md index 6dc6e662e..1576fe96f 100644 --- a/specs/eip4844/p2p-interface.md +++ b/specs/eip4844/p2p-interface.md @@ -57,14 +57,6 @@ class SignedBeaconBlockAndBlobsSidecar(Container): blobs_sidecar: BlobsSidecar ``` -### `SignedBeaconBlockAndBlobsSidecar` - -```python -class SignedBeaconBlockAndBlobsSidecar(Container): - beacon_block: SignedBeaconBlock - blobs_sidecar: BlobsSidecar -``` - ## The gossip domain: gossipsub Some gossip meshes are upgraded in the fork of EIP4844 to support upgraded types. diff --git a/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py index 8b9d6928f..634daca2d 100644 --- a/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/eip4844/unittests/validator/test_validator.py @@ -13,21 +13,6 @@ from eth2spec.test.helpers.sharding import ( ) -@with_eip4844_and_later -@spec_state_test -def test_verify_kzg_proof(spec, state): - x = 3 - polynomial = get_sample_blob(spec) - polynomial = [int(i) for i in polynomial] - commitment = spec.blob_to_kzg_commitment(polynomial) - - # Get the proof - proof = spec.compute_kzg_proof(polynomial, x) - - y = spec.evaluate_polynomial_in_evaluation_form(polynomial, x) - assert spec.verify_kzg_proof(commitment, x, y, proof) - - def _run_validate_blobs_sidecar_test(spec, state, blob_count): block = build_empty_block_for_next_slot(spec, state) opaque_tx, blobs, blob_kzg_commitments = get_sample_opaque_tx(spec, blob_count=blob_count) From 12404d0250f16fbb57436e80ea37566273d34fd9 Mon Sep 17 00:00:00 2001 From: Potuz Date: Thu, 3 Nov 2022 14:56:38 -0300 Subject: [PATCH 14/36] fix for loop --- specs/capella/beacon-chain.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 063dc8e82..7409c9726 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -286,9 +286,10 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: epoch = get_current_epoch(state) withdrawal_index = state.next_withdrawal_index - index = ValidatorIndex((state.last_withdrawal_validator_index + 1) % len(state.validators)) + index = state.last_withdrawal_validator_index ret: List[Withdrawal] = [] - for probed in range(len(state.validators)): + for i in range(len(state.validators)): + index = ValidatorIndex((index + 1) % len(state.validators)) val = state.validators[index] balance = state.balances[index] if is_fully_withdrawable_validator(val, balance, epoch): @@ -311,8 +312,6 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: withdrawal_index = WithdrawalIndex(withdrawal_index + 1) if len(ret) == MAX_WITHDRAWALS_PER_PAYLOAD: break - probed += 1 - index = ValidatorIndex((index + probed) % len(state.validators)) return ret ``` From 329bafa6e20db6bace8b181fe356e11ab0f18a30 Mon Sep 17 00:00:00 2001 From: Potuz Date: Thu, 3 Nov 2022 14:58:55 -0300 Subject: [PATCH 15/36] dapplion's suggestions --- specs/capella/beacon-chain.md | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 7409c9726..42fbf85bd 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -287,28 +287,26 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: epoch = get_current_epoch(state) withdrawal_index = state.next_withdrawal_index index = state.last_withdrawal_validator_index - ret: List[Withdrawal] = [] - for i in range(len(state.validators)): + withdrawals: List[Withdrawal] = [] + for _ in range(len(state.validators)): index = ValidatorIndex((index + 1) % len(state.validators)) val = state.validators[index] balance = state.balances[index] if is_fully_withdrawable_validator(val, balance, epoch): - withdrawal = Withdrawal( + withdrawals.append(Withdrawal( index=withdrawal_index, validator_index=index, address=ExecutionAddress(val.withdrawal_credentials[12:]), amount=balance, - ) - ret.append(withdrawal) + )) withdrawal_index = WithdrawalIndex(withdrawal_index + 1) elif is_partially_withdrawable_validator(val, balance): - withdrawal = Withdrawal( + withdrawals.append(Withdrawal( index=withdrawal_index, validator_index=index, address=ExecutionAddress(val.withdrawal_credentials[12:]), amount=balance - MAX_EFFECTIVE_BALANCE, - ) - ret.append(withdrawal) + )) withdrawal_index = WithdrawalIndex(withdrawal_index + 1) if len(ret) == MAX_WITHDRAWALS_PER_PAYLOAD: break From a09d617737500f09998cb05c22bcc38e5bfdc649 Mon Sep 17 00:00:00 2001 From: Potuz Date: Thu, 3 Nov 2022 15:07:03 -0300 Subject: [PATCH 16/36] right ret --- specs/capella/beacon-chain.md | 2 +- tests/core/pyspec/eth2spec/test/helpers/execution_payload.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 42fbf85bd..3e89bc257 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -310,7 +310,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: withdrawal_index = WithdrawalIndex(withdrawal_index + 1) if len(ret) == MAX_WITHDRAWALS_PER_PAYLOAD: break - return ret + return withdrawals ``` #### New `process_withdrawals` diff --git a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py index 83162e1c2..93befaf5f 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py @@ -30,8 +30,7 @@ def build_empty_execution_payload(spec, state, randao_mix=None): transactions=empty_txs, ) if is_post_capella(spec): - num_withdrawals = min(spec.MAX_WITHDRAWALS_PER_PAYLOAD, len(state.withdrawal_queue)) - payload.withdrawals = state.withdrawal_queue[:num_withdrawals] + payload.withdrawals = spec.get_expected_withdrawals(state) # TODO: real RLP + block hash logic would be nice, requires RLP and keccak256 dependency however. payload.block_hash = spec.Hash32(spec.hash(payload.hash_tree_root() + b"FAKE RLP HASH")) From 0b1f32ec25f3064db6ac417b72725b7f06003281 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 3 Nov 2022 11:21:29 -0500 Subject: [PATCH 17/36] Fix capella/block_processing tests --- specs/capella/beacon-chain.md | 10 +- .../test_process_withdrawals.py | 267 +++++++++++++++--- 2 files changed, 234 insertions(+), 43 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 3e89bc257..c93eaf150 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -308,7 +308,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: amount=balance - MAX_EFFECTIVE_BALANCE, )) withdrawal_index = WithdrawalIndex(withdrawal_index + 1) - if len(ret) == MAX_WITHDRAWALS_PER_PAYLOAD: + if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: break return withdrawals ``` @@ -320,12 +320,12 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: expected_withdrawals = get_expected_withdrawals(state) assert len(payload.withdrawals) == len(expected_withdrawals) - for expected_withdrawal, withdrawal in zip(get_expected_withdrawals(state), payload.withdrawals): + for expected_withdrawal, withdrawal in zip(expected_withdrawals, payload.withdrawals): assert withdrawal == expected_withdrawal decrease_balance(state, withdrawal.validator_index, withdrawal.amount) - - state.next_withdrawal_index = WithdrawalIndex(withdrawal.index + 1) - state.last_withdrawal_validator_index = withdrawal.validator_index + if len(expected_withdrawals) > 0: + state.next_withdrawal_index = WithdrawalIndex(withdrawal.index + 1) + state.last_withdrawal_validator_index = withdrawal.validator_index ``` #### Modified `process_execution_payload` diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index 9bf70b56d..2f99b05b1 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -1,10 +1,14 @@ +import random from eth2spec.test.helpers.execution_payload import ( build_empty_execution_payload, ) from eth2spec.test.context import spec_state_test, expect_assertion_error, with_capella_and_later - from eth2spec.test.helpers.state import next_slot +from eth2spec.test.helpers.withdrawals import ( + set_validator_fully_withdrawable, + set_validator_partially_withdrawable, +) def prepare_withdrawal_queue(spec, state, num_withdrawals): @@ -22,6 +26,37 @@ def prepare_withdrawal_queue(spec, state, num_withdrawals): assert len(state.withdrawal_queue) == num_withdrawals + pre_queue_len +def prepare_expected_withdrawals(spec, state, + num_full_withdrawals=0, num_partial_withdrawals=0, rng=random.Random(5566)): + assert num_full_withdrawals + num_partial_withdrawals <= len(state.validators) + all_valdiator_indices = list(range(len(state.validators))) + sampled_indices = rng.sample(all_valdiator_indices, num_full_withdrawals + num_partial_withdrawals) + fully_withdrawable_indices = rng.sample(sampled_indices, num_full_withdrawals) + partial_withdrawals_indices = list(set(sampled_indices).difference(set(fully_withdrawable_indices))) + + for index in fully_withdrawable_indices: + set_validator_fully_withdrawable(spec, state, index) + for index in partial_withdrawals_indices: + set_validator_partially_withdrawable(spec, state, index) + + return fully_withdrawable_indices, partial_withdrawals_indices + + +def verify_post_state(state, spec, expected_withdrawals, + fully_withdrawable_indices, partial_withdrawals_indices): + expected_withdrawals_validator_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] + for index in fully_withdrawable_indices: + if index in expected_withdrawals_validator_indices: + assert state.balances[index] == 0 + else: + assert state.balances[index] > 0 + for index in partial_withdrawals_indices: + if index in expected_withdrawals_validator_indices: + assert state.balances[index] == spec.MAX_EFFECTIVE_BALANCE + else: + assert state.balances[index] > spec.MAX_EFFECTIVE_BALANCE + + def run_withdrawals_processing(spec, state, execution_payload, valid=True): """ Run ``process_execution_payload``, yielding: @@ -30,10 +65,10 @@ def run_withdrawals_processing(spec, state, execution_payload, valid=True): - post-state ('post'). If ``valid == False``, run expecting ``AssertionError`` """ + expected_withdrawals = spec.get_expected_withdrawals(state) + assert len(expected_withdrawals) <= spec.MAX_WITHDRAWALS_PER_PAYLOAD - pre_withdrawal_queue = state.withdrawal_queue.copy() - num_withdrawals = min(spec.MAX_WITHDRAWALS_PER_PAYLOAD, len(pre_withdrawal_queue)) - + pre_state = state.copy() yield 'pre', state yield 'execution_payload', execution_payload @@ -46,18 +81,22 @@ def run_withdrawals_processing(spec, state, execution_payload, valid=True): yield 'post', state - if len(pre_withdrawal_queue) == 0: - assert len(state.withdrawal_queue) == 0 - elif len(pre_withdrawal_queue) <= num_withdrawals: - assert len(state.withdrawal_queue) == 0 + if len(expected_withdrawals) == 0: + assert state == pre_state + elif len(expected_withdrawals) < spec.MAX_WITHDRAWALS_PER_PAYLOAD: + assert len(spec.get_expected_withdrawals(state)) == 0 + elif len(expected_withdrawals) == spec.MAX_WITHDRAWALS_PER_PAYLOAD: + assert len(spec.get_expected_withdrawals(state)) >= 0 else: - assert state.withdrawal_queue == pre_withdrawal_queue[num_withdrawals:] + raise ValueError('len(expected_withdrawals) should not be greater than MAX_WITHDRAWALS_PER_PAYLOAD') + + return expected_withdrawals @with_capella_and_later @spec_state_test -def test_success_empty_queue(spec, state): - assert len(state.withdrawal_queue) == 0 +def test_success_zero_expected_withdrawals(spec, state): + assert len(spec.get_expected_withdrawals(state)) == 0 next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -67,35 +106,81 @@ def test_success_empty_queue(spec, state): @with_capella_and_later @spec_state_test -def test_success_one_in_queue(spec, state): - prepare_withdrawal_queue(spec, state, 1) +def test_success_one_full_withdrawal(spec, state): + fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( + spec, state, num_full_withdrawals=1) + assert len(fully_withdrawable_indices) == 1 + assert len(partial_withdrawals_indices) == 0 next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) - yield from run_withdrawals_processing(spec, state, execution_payload) + expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) + + verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + + +@with_capella_and_later +@spec_state_test +def test_success_one_partial_withdrawal(spec, state): + fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( + spec, state, num_partial_withdrawals=1) + assert len(fully_withdrawable_indices) == 0 + assert len(partial_withdrawals_indices) == 1 + for index in partial_withdrawals_indices: + assert state.balances[index] != spec.MAX_EFFECTIVE_BALANCE + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + + expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) + + verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) @with_capella_and_later @spec_state_test def test_success_max_per_slot_in_queue(spec, state): - prepare_withdrawal_queue(spec, state, spec.MAX_WITHDRAWALS_PER_PAYLOAD) + num_full_withdrawals = spec.MAX_WITHDRAWALS_PER_PAYLOAD // 2 + num_partial_withdrawals = spec.MAX_WITHDRAWALS_PER_PAYLOAD - num_full_withdrawals + fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( + spec, state, + num_full_withdrawals=num_full_withdrawals, num_partial_withdrawals=num_partial_withdrawals) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) - yield from run_withdrawals_processing(spec, state, execution_payload) + expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) + + verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) @with_capella_and_later @spec_state_test -def test_success_a_lot_in_queue(spec, state): - prepare_withdrawal_queue(spec, state, spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) +def test_success_a_lot_full_withdrawal_in_queue(spec, state): + fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( + spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) - yield from run_withdrawals_processing(spec, state, execution_payload) + expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) + + verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + + +@with_capella_and_later +@spec_state_test +def test_success_a_lot_partial_withdrawal_in_queue(spec, state): + fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( + spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + + expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) + + verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) # @@ -105,8 +190,6 @@ def test_success_a_lot_in_queue(spec, state): @with_capella_and_later @spec_state_test def test_fail_empty_queue_non_empty_withdrawals(spec, state): - assert len(state.withdrawal_queue) == 0 - next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) withdrawal = spec.Withdrawal( @@ -122,8 +205,8 @@ def test_fail_empty_queue_non_empty_withdrawals(spec, state): @with_capella_and_later @spec_state_test -def test_fail_one_in_queue_none_in_withdrawals(spec, state): - prepare_withdrawal_queue(spec, state, 1) +def test_fail_one_expected_full_withdrawal_and_none_in_withdrawals(spec, state): + prepare_expected_withdrawals(spec, state, num_full_withdrawals=1) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -134,8 +217,20 @@ def test_fail_one_in_queue_none_in_withdrawals(spec, state): @with_capella_and_later @spec_state_test -def test_fail_one_in_queue_two_in_withdrawals(spec, state): - prepare_withdrawal_queue(spec, state, 1) +def test_fail_one_expected_partial_withdrawal_and_none_in_withdrawals(spec, state): + prepare_expected_withdrawals(spec, state, num_partial_withdrawals=1) + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + execution_payload.withdrawals = [] + + yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + + +@with_capella_and_later +@spec_state_test +def test_fail_one_expected_full_withdrawal_and_duplicate_in_withdrawals(spec, state): + prepare_expected_withdrawals(spec, state, num_full_withdrawals=2) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -146,8 +241,20 @@ def test_fail_one_in_queue_two_in_withdrawals(spec, state): @with_capella_and_later @spec_state_test -def test_fail_max_per_slot_in_queue_one_less_in_withdrawals(spec, state): - prepare_withdrawal_queue(spec, state, spec.MAX_WITHDRAWALS_PER_PAYLOAD) +def test_fail_one_expected_partial_withdrawal_and_duplicate_in_withdrawals(spec, state): + prepare_expected_withdrawals(spec, state, num_partial_withdrawals=2) + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + execution_payload.withdrawals.append(execution_payload.withdrawals[0].copy()) + + yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + + +@with_capella_and_later +@spec_state_test +def test_fail_max_per_slot_full_withdrawals_and_one_less_in_withdrawals(spec, state): + prepare_expected_withdrawals(spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -158,8 +265,32 @@ def test_fail_max_per_slot_in_queue_one_less_in_withdrawals(spec, state): @with_capella_and_later @spec_state_test -def test_fail_a_lot_in_queue_too_few_in_withdrawals(spec, state): - prepare_withdrawal_queue(spec, state, spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) +def test_fail_max_per_slot_partial_withdrawals_and_one_less_in_withdrawals(spec, state): + prepare_expected_withdrawals(spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD) + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + execution_payload.withdrawals = execution_payload.withdrawals[:-1] + + yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + + +@with_capella_and_later +@spec_state_test +def test_fail_a_lot_full_withdrawals_in_queue_too_few_in_withdrawals(spec, state): + prepare_expected_withdrawals(spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + execution_payload.withdrawals = execution_payload.withdrawals[:-1] + + yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + + +@with_capella_and_later +@spec_state_test +def test_fail_a_lot_partial_withdrawals_in_queue_too_few_in_withdrawals(spec, state): + prepare_expected_withdrawals(spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -175,7 +306,7 @@ def test_fail_a_lot_in_queue_too_few_in_withdrawals(spec, state): @with_capella_and_later @spec_state_test def test_fail_incorrect_dequeue_index(spec, state): - prepare_withdrawal_queue(spec, state, 1) + prepare_expected_withdrawals(spec, state, num_full_withdrawals=1) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -186,8 +317,8 @@ def test_fail_incorrect_dequeue_index(spec, state): @with_capella_and_later @spec_state_test -def test_fail_incorrect_dequeue_address(spec, state): - prepare_withdrawal_queue(spec, state, 1) +def test_fail_incorrect_dequeue_address_full(spec, state): + prepare_expected_withdrawals(spec, state, num_full_withdrawals=1) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -198,8 +329,20 @@ def test_fail_incorrect_dequeue_address(spec, state): @with_capella_and_later @spec_state_test -def test_fail_incorrect_dequeue_amount(spec, state): - prepare_withdrawal_queue(spec, state, 1) +def test_fail_incorrect_dequeue_address_partial(spec, state): + prepare_expected_withdrawals(spec, state, num_partial_withdrawals=1) + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + execution_payload.withdrawals[0].address = b'\xff' * 20 + + yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + + +@with_capella_and_later +@spec_state_test +def test_fail_incorrect_dequeue_amount_full(spec, state): + prepare_expected_withdrawals(spec, state, num_full_withdrawals=1) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -210,8 +353,20 @@ def test_fail_incorrect_dequeue_amount(spec, state): @with_capella_and_later @spec_state_test -def test_fail_one_of_many_dequeued_incorrectly(spec, state): - prepare_withdrawal_queue(spec, state, spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) +def test_fail_incorrect_dequeue_amount_partial(spec, state): + prepare_expected_withdrawals(spec, state, num_full_withdrawals=1) + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + execution_payload.withdrawals[0].amount += 1 + + yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + + +@with_capella_and_later +@spec_state_test +def test_fail_one_of_many_dequeued_incorrectly_full(spec, state): + prepare_expected_withdrawals(spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -228,8 +383,44 @@ def test_fail_one_of_many_dequeued_incorrectly(spec, state): @with_capella_and_later @spec_state_test -def test_fail_many_dequeued_incorrectly(spec, state): - prepare_withdrawal_queue(spec, state, spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) +def test_fail_one_of_many_dequeued_incorrectly_partial(spec, state): + prepare_expected_withdrawals(spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + num_withdrawals = len(execution_payload.withdrawals) + + # Pick withdrawal in middle of list and mutate + withdrawal = execution_payload.withdrawals[num_withdrawals // 2] + withdrawal.index += 1 + withdrawal.address = b'\x99' * 20 + withdrawal.amount += 4000000 + + yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + + +@with_capella_and_later +@spec_state_test +def test_fail_many_dequeued_incorrectly_full(spec, state): + prepare_expected_withdrawals(spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + for i, withdrawal in enumerate(execution_payload.withdrawals): + if i % 3 == 0: + withdrawal.index += 1 + elif i % 3 == 1: + withdrawal.address = i.to_bytes(20, 'big') + else: + withdrawal.amount += 1 + + yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + + +@with_capella_and_later +@spec_state_test +def test_fail_many_dequeued_incorrectly_partial(spec, state): + prepare_expected_withdrawals(spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) From 494cefcda6a2fee397a895f83e0cb26c8bed4805 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 3 Nov 2022 16:27:24 -0500 Subject: [PATCH 18/36] Fix sanity block tests --- .../eth2spec/test/capella/sanity/test_blocks.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py index 28c20a2cd..011f0606a 100644 --- a/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py @@ -45,6 +45,8 @@ def test_full_withdrawal_in_epoch_transition(spec, state): index = 0 current_epoch = spec.get_current_epoch(state) set_validator_fully_withdrawable(spec, state, index, current_epoch) + assert len(spec.get_expected_withdrawals(state)) == 1 + yield 'pre', state # trigger epoch transition @@ -55,6 +57,7 @@ def test_full_withdrawal_in_epoch_transition(spec, state): yield 'post', state assert state.balances[index] == 0 + assert len(spec.get_expected_withdrawals(state)) == 0 @with_capella_and_later @@ -63,7 +66,8 @@ def test_partial_withdrawal_in_epoch_transition(spec, state): index = state.next_withdrawal_index set_validator_partially_withdrawable(spec, state, index, excess_balance=1000000000000) pre_balance = state.balances[index] - pre_withdrawal_queue_len = len(state.withdrawal_queue) + + assert len(spec.get_expected_withdrawals(state)) == 1 yield 'pre', state @@ -77,21 +81,19 @@ def test_partial_withdrawal_in_epoch_transition(spec, state): assert state.balances[index] < pre_balance # Potentially less than due to sync committee penalty assert state.balances[index] <= spec.MAX_EFFECTIVE_BALANCE - # Withdrawal is processed within the context of the block so queue empty - assert len(state.withdrawal_queue) == pre_withdrawal_queue_len + assert len(spec.get_expected_withdrawals(state)) == 0 @with_capella_and_later @spec_state_test def test_many_partial_withdrawals_in_epoch_transition(spec, state): assert len(state.validators) > spec.MAX_WITHDRAWALS_PER_PAYLOAD - assert spec.MAX_PARTIAL_WITHDRAWALS_PER_EPOCH > spec.MAX_WITHDRAWALS_PER_PAYLOAD for i in range(spec.MAX_WITHDRAWALS_PER_PAYLOAD + 1): index = (i + state.next_withdrawal_index) % len(state.validators) set_validator_partially_withdrawable(spec, state, index, excess_balance=1000000000000) - pre_withdrawal_queue_len = len(state.withdrawal_queue) + assert len(spec.get_expected_withdrawals(state)) == spec.MAX_WITHDRAWALS_PER_PAYLOAD yield 'pre', state @@ -102,8 +104,7 @@ def test_many_partial_withdrawals_in_epoch_transition(spec, state): yield 'blocks', [signed_block] yield 'post', state - # All new partial withdrawals processed except 1 - assert len(state.withdrawal_queue) == pre_withdrawal_queue_len + 1 + assert len(spec.get_expected_withdrawals(state)) == 1 @with_capella_and_later From b530dc09aad70086a84208a09042b39970cb4a0e Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 3 Nov 2022 16:57:02 -0500 Subject: [PATCH 19/36] Move old withdrawal epoch_processing tests to block_processing --- .../test_process_withdrawals.py | 369 +++++++++++++++++- .../test/capella/epoch_processing/__init__.py | 0 .../test_process_full_withdrawals.py | 174 --------- .../test_process_partial_withdrawals.py | 262 ------------- 4 files changed, 365 insertions(+), 440 deletions(-) delete mode 100644 tests/core/pyspec/eth2spec/test/capella/epoch_processing/__init__.py delete mode 100644 tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_full_withdrawals.py delete mode 100644 tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_partial_withdrawals.py diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index 2f99b05b1..ca8753f1b 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -1,11 +1,24 @@ import random + +from eth2spec.test.context import ( + spec_state_test, + expect_assertion_error, + with_capella_and_later, + with_presets, +) +from eth2spec.test.helpers.constants import MINIMAL from eth2spec.test.helpers.execution_payload import ( build_empty_execution_payload, ) - -from eth2spec.test.context import spec_state_test, expect_assertion_error, with_capella_and_later -from eth2spec.test.helpers.state import next_slot +from eth2spec.test.helpers.random import ( + randomize_state, +) +from eth2spec.test.helpers.state import ( + next_epoch, + next_slot, +) from eth2spec.test.helpers.withdrawals import ( + set_eth1_withdrawal_credential_with_balance, set_validator_fully_withdrawable, set_validator_partially_withdrawable, ) @@ -57,7 +70,7 @@ def verify_post_state(state, spec, expected_withdrawals, assert state.balances[index] > spec.MAX_EFFECTIVE_BALANCE -def run_withdrawals_processing(spec, state, execution_payload, valid=True): +def run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=None, valid=True): """ Run ``process_execution_payload``, yielding: - pre-state ('pre') @@ -67,6 +80,8 @@ def run_withdrawals_processing(spec, state, execution_payload, valid=True): """ expected_withdrawals = spec.get_expected_withdrawals(state) assert len(expected_withdrawals) <= spec.MAX_WITHDRAWALS_PER_PAYLOAD + if num_expected_withdrawals is not None: + assert len(expected_withdrawals) == num_expected_withdrawals pre_state = state.copy() yield 'pre', state @@ -433,3 +448,349 @@ def test_fail_many_dequeued_incorrectly_partial(spec, state): withdrawal.amount += 1 yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + + +# +# More full withdrawal cases +# + +@with_capella_and_later +@spec_state_test +def test_withdrawable_epoch_but_0_balance(spec, state): + current_epoch = spec.get_current_epoch(state) + set_validator_fully_withdrawable(spec, state, 0, current_epoch) + + state.validators[0].effective_balance = 10000000000 + state.balances[0] = 0 + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=0) + + +@with_capella_and_later +@spec_state_test +def test_withdrawable_epoch_but_0_effective_balance_0_balance(spec, state): + current_epoch = spec.get_current_epoch(state) + set_validator_fully_withdrawable(spec, state, 0, current_epoch) + + state.validators[0].effective_balance = 0 + state.balances[0] = 0 + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=0) + + +@with_capella_and_later +@spec_state_test +def test_withdrawable_epoch_but_0_effective_balance_nonzero_balance(spec, state): + current_epoch = spec.get_current_epoch(state) + set_validator_fully_withdrawable(spec, state, 0, current_epoch) + + state.validators[0].effective_balance = 0 + state.balances[0] = 100000000 + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=1) + + +@with_capella_and_later +@spec_state_test +def test_no_withdrawals_but_some_next_epoch(spec, state): + current_epoch = spec.get_current_epoch(state) + + # Make a few validators withdrawable at the *next* epoch + for index in range(3): + set_validator_fully_withdrawable(spec, state, index, current_epoch + 1) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=0) + + +@with_capella_and_later +@spec_state_test +def test_all_withdrawal(spec, state): + # Make all validators withdrawable + for index in range(len(state.validators)): + set_validator_fully_withdrawable(spec, state, index) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing( + spec, state, execution_payload, + num_expected_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD) + + +def run_random_full_withdrawals_test(spec, state, rng): + randomize_state(spec, state, rng) + for index in range(len(state.validators)): + # 50% withdrawable + if rng.choice([True, False]): + set_validator_fully_withdrawable(spec, state, index) + validator = state.validators[index] + # 12.5% unset credentials + if rng.randint(0, 7) == 0: + validator.withdrawal_credentials = spec.BLS_WITHDRAWAL_PREFIX + validator.withdrawal_credentials[1:] + # 12.5% not enough balance + if rng.randint(0, 7) == 0: + state.balances[index] = 0 + # 12.5% not close enough epoch + if rng.randint(0, 7) == 0: + validator.withdrawable_epoch += 1 + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload) + + +@with_capella_and_later +@spec_state_test +def test_random_full_withdrawals_0(spec, state): + yield from run_random_full_withdrawals_test(spec, state, random.Random(444)) + + +@with_capella_and_later +@spec_state_test +def test_random_full_withdrawals_1(spec, state): + yield from run_random_full_withdrawals_test(spec, state, random.Random(420)) + + +@with_capella_and_later +@spec_state_test +def test_random_full_withdrawals_2(spec, state): + yield from run_random_full_withdrawals_test(spec, state, random.Random(200)) + + +@with_capella_and_later +@spec_state_test +def test_random_full_withdrawals_3(spec, state): + yield from run_random_full_withdrawals_test(spec, state, random.Random(2000000)) + + +# +# More partial withdrawal cases +# + +@with_capella_and_later +@spec_state_test +def test_success_no_max_effective_balance(spec, state): + validator_index = len(state.validators) // 2 + # To be partially withdrawable, the validator's effective balance must be maxed out + set_eth1_withdrawal_credential_with_balance(spec, state, validator_index, spec.MAX_EFFECTIVE_BALANCE - 1) + validator = state.validators[validator_index] + + assert validator.effective_balance < spec.MAX_EFFECTIVE_BALANCE + assert not spec.is_partially_withdrawable_validator(validator, state.balances[validator_index]) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=0) + + +@with_capella_and_later +@spec_state_test +def test_success_no_excess_balance(spec, state): + validator_index = len(state.validators) // 2 + # To be partially withdrawable, the validator needs an excess balance + set_eth1_withdrawal_credential_with_balance(spec, state, validator_index, spec.MAX_EFFECTIVE_BALANCE) + validator = state.validators[validator_index] + + assert validator.effective_balance == spec.MAX_EFFECTIVE_BALANCE + assert not spec.is_partially_withdrawable_validator(validator, state.balances[validator_index]) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=0) + + +@with_capella_and_later +@spec_state_test +def test_success_excess_balance_but_no_max_effective_balance(spec, state): + validator_index = len(state.validators) // 2 + set_validator_partially_withdrawable(spec, state, validator_index) + validator = state.validators[validator_index] + + # To be partially withdrawable, the validator needs both a maxed out effective balance and an excess balance + validator.effective_balance = spec.MAX_EFFECTIVE_BALANCE - 1 + + assert not spec.is_partially_withdrawable_validator(validator, state.balances[validator_index]) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=0) + + +@with_capella_and_later +@spec_state_test +def test_success_one_partial_withdrawable_not_yet_active(spec, state): + validator_index = len(state.validators) // 2 + state.validators[validator_index].activation_epoch += 4 + set_validator_partially_withdrawable(spec, state, validator_index) + + assert not spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state)) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=1) + + +@with_capella_and_later +@spec_state_test +def test_success_one_partial_withdrawable_in_exit_queue(spec, state): + validator_index = len(state.validators) // 2 + state.validators[validator_index].exit_epoch = spec.get_current_epoch(state) + 1 + set_validator_partially_withdrawable(spec, state, validator_index) + + assert spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state)) + assert not spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state) + 1) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=1) + + +@with_capella_and_later +@spec_state_test +def test_success_one_partial_withdrawable_exited(spec, state): + validator_index = len(state.validators) // 2 + state.validators[validator_index].exit_epoch = spec.get_current_epoch(state) + set_validator_partially_withdrawable(spec, state, validator_index) + + assert not spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state)) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=1) + + +@with_capella_and_later +@spec_state_test +def test_success_one_partial_withdrawable_active_and_slashed(spec, state): + validator_index = len(state.validators) // 2 + state.validators[validator_index].slashed = True + set_validator_partially_withdrawable(spec, state, validator_index) + + assert spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state)) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=1) + + +@with_capella_and_later +@spec_state_test +def test_success_one_partial_withdrawable_exited_and_slashed(spec, state): + validator_index = len(state.validators) // 2 + state.validators[validator_index].slashed = True + state.validators[validator_index].exit_epoch = spec.get_current_epoch(state) + set_validator_partially_withdrawable(spec, state, validator_index) + + assert not spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state)) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=1) + + +@with_capella_and_later +@spec_state_test +def test_success_two_partial_withdrawable(spec, state): + set_validator_partially_withdrawable(spec, state, 0) + set_validator_partially_withdrawable(spec, state, 1) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=2) + + +@with_capella_and_later +@spec_state_test +def test_success_max_partial_withdrawable(spec, state): + # Sanity check that this test works for this state + assert len(state.validators) >= spec.MAX_WITHDRAWALS_PER_PAYLOAD + + for i in range(spec.MAX_WITHDRAWALS_PER_PAYLOAD): + set_validator_partially_withdrawable(spec, state, i) + + execution_payload = build_empty_execution_payload(spec, state) + + yield from run_withdrawals_processing( + spec, state, execution_payload, num_expected_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD) + + +@with_capella_and_later +@with_presets([MINIMAL], reason="not enough validators with mainnet config") +@spec_state_test +def test_success_max_plus_one_withdrawable(spec, state): + # Sanity check that this test works for this state + assert len(state.validators) >= spec.MAX_WITHDRAWALS_PER_PAYLOAD + 1 + + # More than MAX_WITHDRAWALS_PER_PAYLOAD partially withdrawable + for i in range(spec.MAX_WITHDRAWALS_PER_PAYLOAD + 1): + set_validator_partially_withdrawable(spec, state, i) + + execution_payload = build_empty_execution_payload(spec, state) + + # Should only have MAX_WITHDRAWALS_PER_PAYLOAD withdrawals created + yield from run_withdrawals_processing( + spec, state, execution_payload, num_expected_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD) + + +def run_random_partial_withdrawals_test(spec, state, rng): + for _ in range(rng.randint(0, 2)): + next_epoch(spec, state) + randomize_state(spec, state, rng) + + num_validators = len(state.validators) + state.last_withdrawal_validator_index = rng.randint(0, num_validators - 1) + + num_partially_withdrawable = rng.randint(0, num_validators - 1) + partially_withdrawable_indices = rng.sample(range(num_validators), num_partially_withdrawable) + for index in partially_withdrawable_indices: + set_validator_partially_withdrawable(spec, state, index, excess_balance=rng.randint(1, 1000000000)) + + execution_payload = build_empty_execution_payload(spec, state) + + # Note: due to the randomness and other epoch processing, some of these set as "partially withdrawable" + # may not be partially withdrawable once we get to ``process_partial_withdrawals``, + # thus *not* using the optional third param in this call + yield from run_withdrawals_processing(spec, state, execution_payload) + + +@with_capella_and_later +@spec_state_test +def test_random_0(spec, state): + yield from run_random_partial_withdrawals_test(spec, state, random.Random(0)) + + +@with_capella_and_later +@spec_state_test +def test_random_partial_withdrawals_1(spec, state): + yield from run_random_partial_withdrawals_test(spec, state, random.Random(1)) + + +@with_capella_and_later +@spec_state_test +def test_random_partial_withdrawals_2(spec, state): + yield from run_random_partial_withdrawals_test(spec, state, random.Random(2)) + + +@with_capella_and_later +@spec_state_test +def test_random_partial_withdrawals_3(spec, state): + yield from run_random_partial_withdrawals_test(spec, state, random.Random(3)) + + +@with_capella_and_later +@spec_state_test +def test_random_partial_withdrawals_4(spec, state): + yield from run_random_partial_withdrawals_test(spec, state, random.Random(4)) + + +@with_capella_and_later +@spec_state_test +def test_random_partial_withdrawals_5(spec, state): + yield from run_random_partial_withdrawals_test(spec, state, random.Random(5)) diff --git a/tests/core/pyspec/eth2spec/test/capella/epoch_processing/__init__.py b/tests/core/pyspec/eth2spec/test/capella/epoch_processing/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_full_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_full_withdrawals.py deleted file mode 100644 index 35d2968cb..000000000 --- a/tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_full_withdrawals.py +++ /dev/null @@ -1,174 +0,0 @@ -from random import Random - -from eth2spec.test.context import ( - with_capella_and_later, - spec_state_test, -) -from eth2spec.test.helpers.random import ( - randomize_state, -) -from eth2spec.test.helpers.epoch_processing import ( - run_epoch_processing_to, -) -from eth2spec.test.helpers.withdrawals import ( - set_validator_fully_withdrawable, -) - - -def run_process_full_withdrawals(spec, state, num_expected_withdrawals=None): - run_epoch_processing_to(spec, state, 'process_full_withdrawals') - - pre_next_withdrawal_index = state.next_withdrawal_index - pre_withdrawal_queue = state.withdrawal_queue.copy() - to_be_withdrawn_indices = [ - index for index, validator in enumerate(state.validators) - if spec.is_fully_withdrawable_validator(validator, state.balances[index], spec.get_current_epoch(state)) - ] - - if num_expected_withdrawals is not None: - assert len(to_be_withdrawn_indices) == num_expected_withdrawals - else: - num_expected_withdrawals = len(to_be_withdrawn_indices) - - yield 'pre', state - spec.process_full_withdrawals(state) - yield 'post', state - - for index in to_be_withdrawn_indices: - assert state.balances[index] == 0 - - assert len(state.withdrawal_queue) == len(pre_withdrawal_queue) + num_expected_withdrawals - assert state.next_withdrawal_index == pre_next_withdrawal_index + num_expected_withdrawals - - -@with_capella_and_later -@spec_state_test -def test_no_withdrawable_validators(spec, state): - pre_validators = state.validators.copy() - yield from run_process_full_withdrawals(spec, state, 0) - - assert pre_validators == state.validators - - -@with_capella_and_later -@spec_state_test -def test_withdrawable_epoch_but_0_balance(spec, state): - current_epoch = spec.get_current_epoch(state) - set_validator_fully_withdrawable(spec, state, 0, current_epoch) - - state.validators[0].effective_balance = 10000000000 - state.balances[0] = 0 - - yield from run_process_full_withdrawals(spec, state, 0) - - -@with_capella_and_later -@spec_state_test -def test_withdrawable_epoch_but_0_effective_balance_0_balance(spec, state): - current_epoch = spec.get_current_epoch(state) - set_validator_fully_withdrawable(spec, state, 0, current_epoch) - - state.validators[0].effective_balance = 0 - state.balances[0] = 0 - - yield from run_process_full_withdrawals(spec, state, 0) - - -@with_capella_and_later -@spec_state_test -def test_withdrawable_epoch_but_0_effective_balance_nonzero_balance(spec, state): - current_epoch = spec.get_current_epoch(state) - set_validator_fully_withdrawable(spec, state, 0, current_epoch) - - state.validators[0].effective_balance = 0 - state.balances[0] = 100000000 - - yield from run_process_full_withdrawals(spec, state, 1) - - -@with_capella_and_later -@spec_state_test -def test_no_withdrawals_but_some_next_epoch(spec, state): - current_epoch = spec.get_current_epoch(state) - - # Make a few validators withdrawable at the *next* epoch - for index in range(3): - set_validator_fully_withdrawable(spec, state, index, current_epoch + 1) - - yield from run_process_full_withdrawals(spec, state, 0) - - -@with_capella_and_later -@spec_state_test -def test_single_withdrawal(spec, state): - # Make one validator withdrawable - set_validator_fully_withdrawable(spec, state, 0) - - assert state.next_withdrawal_index == 0 - yield from run_process_full_withdrawals(spec, state, 1) - - assert state.next_withdrawal_index == 1 - - -@with_capella_and_later -@spec_state_test -def test_multi_withdrawal(spec, state): - # Make a few validators withdrawable - for index in range(3): - set_validator_fully_withdrawable(spec, state, index) - - yield from run_process_full_withdrawals(spec, state, 3) - - -@with_capella_and_later -@spec_state_test -def test_all_withdrawal(spec, state): - # Make all validators withdrawable - for index in range(len(state.validators)): - set_validator_fully_withdrawable(spec, state, index) - - yield from run_process_full_withdrawals(spec, state, len(state.validators)) - - -def run_random_full_withdrawals_test(spec, state, rng): - randomize_state(spec, state, rng) - for index in range(len(state.validators)): - # 50% withdrawable - if rng.choice([True, False]): - set_validator_fully_withdrawable(spec, state, index) - validator = state.validators[index] - # 12.5% unset credentials - if rng.randint(0, 7) == 0: - validator.withdrawal_credentials = spec.BLS_WITHDRAWAL_PREFIX + validator.withdrawal_credentials[1:] - # 12.5% not enough balance - if rng.randint(0, 7) == 0: - state.balances[index] = 0 - # 12.5% not close enough epoch - if rng.randint(0, 7) == 0: - validator.withdrawable_epoch += 1 - - yield from run_process_full_withdrawals(spec, state, None) - - -@with_capella_and_later -@spec_state_test -def test_random_withdrawals_0(spec, state): - yield from run_random_full_withdrawals_test(spec, state, Random(444)) - - -@with_capella_and_later -@spec_state_test -def test_random_withdrawals_1(spec, state): - yield from run_random_full_withdrawals_test(spec, state, Random(420)) - - -@with_capella_and_later -@spec_state_test -def test_random_withdrawals_2(spec, state): - yield from run_random_full_withdrawals_test(spec, state, Random(200)) - - -@with_capella_and_later -@spec_state_test -def test_random_withdrawals_3(spec, state): - yield from run_random_full_withdrawals_test(spec, state, Random(2000000)) diff --git a/tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_partial_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_partial_withdrawals.py deleted file mode 100644 index 7569d2862..000000000 --- a/tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_partial_withdrawals.py +++ /dev/null @@ -1,262 +0,0 @@ -import random -from eth2spec.test.helpers.constants import MINIMAL -from eth2spec.test.context import ( - with_capella_and_later, - spec_state_test, - with_presets, -) -from eth2spec.test.helpers.epoch_processing import run_epoch_processing_to -from eth2spec.test.helpers.state import next_epoch -from eth2spec.test.helpers.random import randomize_state -from eth2spec.test.helpers.withdrawals import ( - set_validator_partially_withdrawable, - set_eth1_withdrawal_credential_with_balance, -) - - -def run_process_partial_withdrawals(spec, state, num_expected_withdrawals=None): - # Run rest of epoch processing before predicting partial withdrawals as - # balance changes can affect withdrawability - run_epoch_processing_to(spec, state, 'process_partial_withdrawals') - - pre_next_withdrawal_index = state.next_withdrawal_index - pre_withdrawal_queue = state.withdrawal_queue.copy() - - partially_withdrawable_indices = [ - index for index, validator in enumerate(state.validators) - if spec.is_partially_withdrawable_validator(validator, state.balances[index]) - ] - num_partial_withdrawals = min(len(partially_withdrawable_indices), spec.MAX_PARTIAL_WITHDRAWALS_PER_EPOCH) - - if num_expected_withdrawals is not None: - assert num_partial_withdrawals == num_expected_withdrawals - else: - num_expected_withdrawals = num_partial_withdrawals - - yield 'pre', state - spec.process_partial_withdrawals(state) - yield 'post', state - - post_partially_withdrawable_indices = [ - index for index, validator in enumerate(state.validators) - if spec.is_partially_withdrawable_validator(validator, state.balances[index]) - ] - - assert len(partially_withdrawable_indices) - num_partial_withdrawals == len(post_partially_withdrawable_indices) - - assert len(state.withdrawal_queue) == len(pre_withdrawal_queue) + num_expected_withdrawals - assert state.next_withdrawal_index == pre_next_withdrawal_index + num_expected_withdrawals - - -@with_capella_and_later -@spec_state_test -def test_success_no_withdrawable(spec, state): - pre_validators = state.validators.copy() - yield from run_process_partial_withdrawals(spec, state, 0) - - assert pre_validators == state.validators - - -@with_capella_and_later -@spec_state_test -def test_success_no_max_effective_balance(spec, state): - validator_index = len(state.validators) // 2 - # To be partially withdrawable, the validator's effective balance must be maxed out - set_eth1_withdrawal_credential_with_balance(spec, state, validator_index, spec.MAX_EFFECTIVE_BALANCE - 1) - validator = state.validators[validator_index] - - assert validator.effective_balance < spec.MAX_EFFECTIVE_BALANCE - assert not spec.is_partially_withdrawable_validator(validator, state.balances[validator_index]) - - yield from run_process_partial_withdrawals(spec, state, 0) - - -@with_capella_and_later -@spec_state_test -def test_success_no_excess_balance(spec, state): - validator_index = len(state.validators) // 2 - # To be partially withdrawable, the validator needs an excess balance - set_eth1_withdrawal_credential_with_balance(spec, state, validator_index, spec.MAX_EFFECTIVE_BALANCE) - validator = state.validators[validator_index] - - assert validator.effective_balance == spec.MAX_EFFECTIVE_BALANCE - assert not spec.is_partially_withdrawable_validator(validator, state.balances[validator_index]) - - yield from run_process_partial_withdrawals(spec, state, 0) - - -@with_capella_and_later -@spec_state_test -def test_success_excess_balance_but_no_max_effective_balance(spec, state): - validator_index = len(state.validators) // 2 - set_validator_partially_withdrawable(spec, state, validator_index) - validator = state.validators[validator_index] - - # To be partially withdrawable, the validator needs both a maxed out effective balance and an excess balance - validator.effective_balance = spec.MAX_EFFECTIVE_BALANCE - 1 - - assert not spec.is_partially_withdrawable_validator(validator, state.balances[validator_index]) - - yield from run_process_partial_withdrawals(spec, state, 0) - - -@with_capella_and_later -@spec_state_test -def test_success_one_partial_withdrawable(spec, state): - validator_index = len(state.validators) // 2 - set_validator_partially_withdrawable(spec, state, validator_index) - - yield from run_process_partial_withdrawals(spec, state, 1) - - -@with_capella_and_later -@spec_state_test -def test_success_one_partial_withdrawable_not_yet_active(spec, state): - validator_index = len(state.validators) // 2 - state.validators[validator_index].activation_epoch += 4 - set_validator_partially_withdrawable(spec, state, validator_index) - - assert not spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state)) - - yield from run_process_partial_withdrawals(spec, state, 1) - - -@with_capella_and_later -@spec_state_test -def test_success_one_partial_withdrawable_in_exit_queue(spec, state): - validator_index = len(state.validators) // 2 - state.validators[validator_index].exit_epoch = spec.get_current_epoch(state) + 1 - set_validator_partially_withdrawable(spec, state, validator_index) - - assert spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state)) - assert not spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state) + 1) - - yield from run_process_partial_withdrawals(spec, state, 1) - - -@with_capella_and_later -@spec_state_test -def test_success_one_partial_withdrawable_exited(spec, state): - validator_index = len(state.validators) // 2 - state.validators[validator_index].exit_epoch = spec.get_current_epoch(state) - set_validator_partially_withdrawable(spec, state, validator_index) - - assert not spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state)) - - yield from run_process_partial_withdrawals(spec, state, 1) - - -@with_capella_and_later -@spec_state_test -def test_success_one_partial_withdrawable_active_and_slashed(spec, state): - validator_index = len(state.validators) // 2 - state.validators[validator_index].slashed = True - set_validator_partially_withdrawable(spec, state, validator_index) - - assert spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state)) - - yield from run_process_partial_withdrawals(spec, state, 1) - - -@with_capella_and_later -@spec_state_test -def test_success_one_partial_withdrawable_exited_and_slashed(spec, state): - validator_index = len(state.validators) // 2 - state.validators[validator_index].slashed = True - state.validators[validator_index].exit_epoch = spec.get_current_epoch(state) - set_validator_partially_withdrawable(spec, state, validator_index) - - assert not spec.is_active_validator(state.validators[validator_index], spec.get_current_epoch(state)) - - yield from run_process_partial_withdrawals(spec, state, 1) - - -@with_capella_and_later -@spec_state_test -def test_success_two_partial_withdrawable(spec, state): - set_validator_partially_withdrawable(spec, state, 0) - set_validator_partially_withdrawable(spec, state, 1) - - yield from run_process_partial_withdrawals(spec, state, 2) - - -@with_capella_and_later -@spec_state_test -def test_success_max_partial_withdrawable(spec, state): - # Sanity check that this test works for this state - assert len(state.validators) >= spec.MAX_PARTIAL_WITHDRAWALS_PER_EPOCH - - for i in range(spec.MAX_PARTIAL_WITHDRAWALS_PER_EPOCH): - set_validator_partially_withdrawable(spec, state, i) - - yield from run_process_partial_withdrawals(spec, state, spec.MAX_PARTIAL_WITHDRAWALS_PER_EPOCH) - - -@with_capella_and_later -@with_presets([MINIMAL], reason="not enough validators with mainnet config") -@spec_state_test -def test_success_max_plus_one_withdrawable(spec, state): - # Sanity check that this test works for this state - assert len(state.validators) >= spec.MAX_PARTIAL_WITHDRAWALS_PER_EPOCH + 1 - - # More than MAX_PARTIAL_WITHDRAWALS_PER_EPOCH partially withdrawable - for i in range(spec.MAX_PARTIAL_WITHDRAWALS_PER_EPOCH + 1): - set_validator_partially_withdrawable(spec, state, i) - - # Should only have MAX_PARTIAL_WITHDRAWALS_PER_EPOCH withdrawals created - yield from run_process_partial_withdrawals(spec, state, spec.MAX_PARTIAL_WITHDRAWALS_PER_EPOCH) - - -def run_random_partial_withdrawals_test(spec, state, rng): - for _ in range(rng.randint(0, 2)): - next_epoch(spec, state) - randomize_state(spec, state, rng) - - num_validators = len(state.validators) - state.next_partial_withdrawal_validator_index = rng.randint(0, num_validators - 1) - - num_partially_withdrawable = rng.randint(0, num_validators - 1) - partially_withdrawable_indices = rng.sample(range(num_validators), num_partially_withdrawable) - for index in partially_withdrawable_indices: - set_validator_partially_withdrawable(spec, state, index, excess_balance=rng.randint(1, 1000000000)) - - # Note: due to the randomness and other epoch processing, some of these set as "partially withdrawable" - # may not be partially withdrawable once we get to ``process_partial_withdrawals``, - # thus *not* using the optional third param in this call - yield from run_process_partial_withdrawals(spec, state) - - -@with_capella_and_later -@spec_state_test -def test_random_0(spec, state): - yield from run_random_partial_withdrawals_test(spec, state, random.Random(0)) - - -@with_capella_and_later -@spec_state_test -def test_random_1(spec, state): - yield from run_random_partial_withdrawals_test(spec, state, random.Random(1)) - - -@with_capella_and_later -@spec_state_test -def test_random_2(spec, state): - yield from run_random_partial_withdrawals_test(spec, state, random.Random(2)) - - -@with_capella_and_later -@spec_state_test -def test_random_3(spec, state): - yield from run_random_partial_withdrawals_test(spec, state, random.Random(3)) - - -@with_capella_and_later -@spec_state_test -def test_random_4(spec, state): - yield from run_random_partial_withdrawals_test(spec, state, random.Random(4)) - - -@with_capella_and_later -@spec_state_test -def test_random_5(spec, state): - yield from run_random_partial_withdrawals_test(spec, state, random.Random(5)) From 6e913ecbd2b9b592269608bb445e8a76b859990a Mon Sep 17 00:00:00 2001 From: Potuz Date: Thu, 3 Nov 2022 20:23:35 -0300 Subject: [PATCH 20/36] rename to latest_withdrawal_validator_index --- specs/capella/beacon-chain.md | 6 +++--- specs/capella/fork.md | 3 ++- .../capella/block_processing/test_process_withdrawals.py | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index c93eaf150..36da72d72 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -221,7 +221,7 @@ class BeaconState(Container): latest_execution_payload_header: ExecutionPayloadHeader # Withdrawals next_withdrawal_index: WithdrawalIndex # [New in Capella] - last_withdrawal_validator_index: ValidatorIndex # [New in Capella] + latest_withdrawal_validator_index: ValidatorIndex # [New in Capella] ``` ## Helpers @@ -286,7 +286,7 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: epoch = get_current_epoch(state) withdrawal_index = state.next_withdrawal_index - index = state.last_withdrawal_validator_index + index = state.latest_withdrawal_validator_index withdrawals: List[Withdrawal] = [] for _ in range(len(state.validators)): index = ValidatorIndex((index + 1) % len(state.validators)) @@ -325,7 +325,7 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: decrease_balance(state, withdrawal.validator_index, withdrawal.amount) if len(expected_withdrawals) > 0: state.next_withdrawal_index = WithdrawalIndex(withdrawal.index + 1) - state.last_withdrawal_validator_index = withdrawal.validator_index + state.latest_withdrawal_validator_index = withdrawal.validator_index ``` #### Modified `process_execution_payload` diff --git a/specs/capella/fork.md b/specs/capella/fork.md index 27e2afcac..1ea43b463 100644 --- a/specs/capella/fork.md +++ b/specs/capella/fork.md @@ -129,7 +129,8 @@ def upgrade_to_capella(pre: bellatrix.BeaconState) -> BeaconState: # Execution-layer latest_execution_payload_header=latest_execution_payload_header, # Withdrawals - last_withdrawal_validator_index=ValidatorIndex(0), + next_withdrawal_index=WithdrawalIndex(0), + latest_withdrawal_validator_index=ValidatorIndex(0), ) return post diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index ca8753f1b..7f3e16cca 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -745,7 +745,7 @@ def run_random_partial_withdrawals_test(spec, state, rng): randomize_state(spec, state, rng) num_validators = len(state.validators) - state.last_withdrawal_validator_index = rng.randint(0, num_validators - 1) + state.latest_withdrawal_validator_index = rng.randint(0, num_validators - 1) num_partially_withdrawable = rng.randint(0, num_validators - 1) partially_withdrawable_indices = rng.sample(range(num_validators), num_partially_withdrawable) From 22f803a038ad98a55955379775a83d60dd0e6834 Mon Sep 17 00:00:00 2001 From: Potuz Date: Fri, 4 Nov 2022 07:20:26 -0300 Subject: [PATCH 21/36] name change and increment operator --- specs/capella/beacon-chain.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 36da72d72..1526d518a 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -286,28 +286,28 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: epoch = get_current_epoch(state) withdrawal_index = state.next_withdrawal_index - index = state.latest_withdrawal_validator_index + validator_index = state.latest_withdrawal_validator_index withdrawals: List[Withdrawal] = [] for _ in range(len(state.validators)): - index = ValidatorIndex((index + 1) % len(state.validators)) - val = state.validators[index] - balance = state.balances[index] + validator_index = ValidatorIndex((validator_index + 1) % len(state.validators)) + val = state.validators[validator_index] + balance = state.balances[validator_index] if is_fully_withdrawable_validator(val, balance, epoch): withdrawals.append(Withdrawal( index=withdrawal_index, - validator_index=index, + validator_index=validator_index, address=ExecutionAddress(val.withdrawal_credentials[12:]), amount=balance, )) - withdrawal_index = WithdrawalIndex(withdrawal_index + 1) + withdrawal_index += 1 elif is_partially_withdrawable_validator(val, balance): withdrawals.append(Withdrawal( index=withdrawal_index, - validator_index=index, + validator_index=validator_index, address=ExecutionAddress(val.withdrawal_credentials[12:]), amount=balance - MAX_EFFECTIVE_BALANCE, )) - withdrawal_index = WithdrawalIndex(withdrawal_index + 1) + withdrawal_index += 1 if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: break return withdrawals From 9973e9f29f7d5c05798e92404008fbe9916c6b01 Mon Sep 17 00:00:00 2001 From: Potuz Date: Fri, 4 Nov 2022 09:00:53 -0300 Subject: [PATCH 22/36] harden tests and add couple of cases --- .../test_process_withdrawals.py | 122 ++++++++++++------ 1 file changed, 83 insertions(+), 39 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index 7f3e16cca..7926e3426 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -23,27 +23,11 @@ from eth2spec.test.helpers.withdrawals import ( set_validator_partially_withdrawable, ) - -def prepare_withdrawal_queue(spec, state, num_withdrawals): - pre_queue_len = len(state.withdrawal_queue) - validator_len = len(state.validators) - for i in range(num_withdrawals): - withdrawal = spec.Withdrawal( - index=i + 5, - validator_index=(i + 1000) % validator_len, - address=b'\x42' * 20, - amount=200000 + i, - ) - state.withdrawal_queue.append(withdrawal) - - assert len(state.withdrawal_queue) == num_withdrawals + pre_queue_len - - def prepare_expected_withdrawals(spec, state, num_full_withdrawals=0, num_partial_withdrawals=0, rng=random.Random(5566)): assert num_full_withdrawals + num_partial_withdrawals <= len(state.validators) - all_valdiator_indices = list(range(len(state.validators))) - sampled_indices = rng.sample(all_valdiator_indices, num_full_withdrawals + num_partial_withdrawals) + all_validator_indices = list(range(len(state.validators))) + sampled_indices = rng.sample(all_validator_indices, num_full_withdrawals + num_partial_withdrawals) fully_withdrawable_indices = rng.sample(sampled_indices, num_full_withdrawals) partial_withdrawals_indices = list(set(sampled_indices).difference(set(fully_withdrawable_indices))) @@ -58,6 +42,8 @@ def prepare_expected_withdrawals(spec, state, def verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices): expected_withdrawals_validator_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] + assert state.next_withdrawal_index == expected_withdrawals[-1].index + 1 + assert state.latest_withdrawal_validator_index == expected_withdrawals_validator_indices[-1] for index in fully_withdrawable_indices: if index in expected_withdrawals_validator_indices: assert state.balances[index] == 0 @@ -100,9 +86,7 @@ def run_withdrawals_processing(spec, state, execution_payload, num_expected_with assert state == pre_state elif len(expected_withdrawals) < spec.MAX_WITHDRAWALS_PER_PAYLOAD: assert len(spec.get_expected_withdrawals(state)) == 0 - elif len(expected_withdrawals) == spec.MAX_WITHDRAWALS_PER_PAYLOAD: - assert len(spec.get_expected_withdrawals(state)) >= 0 - else: + elif len(expected_withdrawals) > spec.MAX_WITHDRAWALS_PER_PAYLOAD: raise ValueError('len(expected_withdrawals) should not be greater than MAX_WITHDRAWALS_PER_PAYLOAD') return expected_withdrawals @@ -155,7 +139,7 @@ def test_success_one_partial_withdrawal(spec, state): @with_capella_and_later @spec_state_test -def test_success_max_per_slot_in_queue(spec, state): +def test_success_max_per_slot(spec, state): num_full_withdrawals = spec.MAX_WITHDRAWALS_PER_PAYLOAD // 2 num_partial_withdrawals = spec.MAX_WITHDRAWALS_PER_PAYLOAD - num_full_withdrawals fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( @@ -172,9 +156,9 @@ def test_success_max_per_slot_in_queue(spec, state): @with_capella_and_later @spec_state_test -def test_success_a_lot_full_withdrawal_in_queue(spec, state): +def test_success_all_fully_withdrawable(spec, state): fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( - spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) + spec, state, num_full_withdrawals=len(state.validators)) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -186,9 +170,9 @@ def test_success_a_lot_full_withdrawal_in_queue(spec, state): @with_capella_and_later @spec_state_test -def test_success_a_lot_partial_withdrawal_in_queue(spec, state): +def test_success_all_partially_withdrawable(spec, state): fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( - spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) + spec, state, num_partial_withdrawals=len(state.validators)) next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) @@ -204,7 +188,7 @@ def test_success_a_lot_partial_withdrawal_in_queue(spec, state): @with_capella_and_later @spec_state_test -def test_fail_empty_queue_non_empty_withdrawals(spec, state): +def test_fail_non_withdrawable_non_empty_withdrawals(spec, state): next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) withdrawal = spec.Withdrawal( @@ -256,7 +240,7 @@ def test_fail_one_expected_full_withdrawal_and_duplicate_in_withdrawals(spec, st @with_capella_and_later @spec_state_test -def test_fail_one_expected_partial_withdrawal_and_duplicate_in_withdrawals(spec, state): +def test_fail_two_expected_partial_withdrawal_and_duplicate_in_withdrawals(spec, state): prepare_expected_withdrawals(spec, state, num_partial_withdrawals=2) next_slot(spec, state) @@ -292,7 +276,7 @@ def test_fail_max_per_slot_partial_withdrawals_and_one_less_in_withdrawals(spec, @with_capella_and_later @spec_state_test -def test_fail_a_lot_full_withdrawals_in_queue_too_few_in_withdrawals(spec, state): +def test_fail_a_lot_fully_withdrawable_too_few_in_withdrawals(spec, state): prepare_expected_withdrawals(spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) next_slot(spec, state) @@ -304,7 +288,7 @@ def test_fail_a_lot_full_withdrawals_in_queue_too_few_in_withdrawals(spec, state @with_capella_and_later @spec_state_test -def test_fail_a_lot_partial_withdrawals_in_queue_too_few_in_withdrawals(spec, state): +def test_fail_a_lot_partially_withdrawable_too_few_in_withdrawals(spec, state): prepare_expected_withdrawals(spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) next_slot(spec, state) @@ -313,6 +297,19 @@ def test_fail_a_lot_partial_withdrawals_in_queue_too_few_in_withdrawals(spec, st yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) +@with_capella_and_later +@spec_state_test +def test_fail_a_lot_mixed_withdrawable_in_queue_too_few_in_withdrawals(spec, state): + prepare_expected_withdrawals(spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4, + num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) + + next_slot(spec, state) + execution_payload = build_empty_execution_payload(spec, state) + execution_payload.withdrawals = execution_payload.withdrawals[:-1] + + yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + + # # Failure cases in which the withdrawals in the execution_payload are incorrect @@ -320,7 +317,7 @@ def test_fail_a_lot_partial_withdrawals_in_queue_too_few_in_withdrawals(spec, st @with_capella_and_later @spec_state_test -def test_fail_incorrect_dequeue_index(spec, state): +def test_fail_incorrect_withdrawal_index(spec, state): prepare_expected_withdrawals(spec, state, num_full_withdrawals=1) next_slot(spec, state) @@ -332,7 +329,7 @@ def test_fail_incorrect_dequeue_index(spec, state): @with_capella_and_later @spec_state_test -def test_fail_incorrect_dequeue_address_full(spec, state): +def test_fail_incorrect_address_full(spec, state): prepare_expected_withdrawals(spec, state, num_full_withdrawals=1) next_slot(spec, state) @@ -344,7 +341,7 @@ def test_fail_incorrect_dequeue_address_full(spec, state): @with_capella_and_later @spec_state_test -def test_fail_incorrect_dequeue_address_partial(spec, state): +def test_fail_incorrect_address_partial(spec, state): prepare_expected_withdrawals(spec, state, num_partial_withdrawals=1) next_slot(spec, state) @@ -356,7 +353,7 @@ def test_fail_incorrect_dequeue_address_partial(spec, state): @with_capella_and_later @spec_state_test -def test_fail_incorrect_dequeue_amount_full(spec, state): +def test_fail_incorrect_amount_full(spec, state): prepare_expected_withdrawals(spec, state, num_full_withdrawals=1) next_slot(spec, state) @@ -368,7 +365,7 @@ def test_fail_incorrect_dequeue_amount_full(spec, state): @with_capella_and_later @spec_state_test -def test_fail_incorrect_dequeue_amount_partial(spec, state): +def test_fail_incorrect_amount_partial(spec, state): prepare_expected_withdrawals(spec, state, num_full_withdrawals=1) next_slot(spec, state) @@ -380,7 +377,7 @@ def test_fail_incorrect_dequeue_amount_partial(spec, state): @with_capella_and_later @spec_state_test -def test_fail_one_of_many_dequeued_incorrectly_full(spec, state): +def test_fail_one_of_many_incorrectly_full(spec, state): prepare_expected_withdrawals(spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) next_slot(spec, state) @@ -398,7 +395,7 @@ def test_fail_one_of_many_dequeued_incorrectly_full(spec, state): @with_capella_and_later @spec_state_test -def test_fail_one_of_many_dequeued_incorrectly_partial(spec, state): +def test_fail_one_of_many_incorrectly_partial(spec, state): prepare_expected_withdrawals(spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) next_slot(spec, state) @@ -416,7 +413,7 @@ def test_fail_one_of_many_dequeued_incorrectly_partial(spec, state): @with_capella_and_later @spec_state_test -def test_fail_many_dequeued_incorrectly_full(spec, state): +def test_fail_many_incorrectly_full(spec, state): prepare_expected_withdrawals(spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) next_slot(spec, state) @@ -434,7 +431,7 @@ def test_fail_many_dequeued_incorrectly_full(spec, state): @with_capella_and_later @spec_state_test -def test_fail_many_dequeued_incorrectly_partial(spec, state): +def test_fail_many_incorrectly_partial(spec, state): prepare_expected_withdrawals(spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) next_slot(spec, state) @@ -794,3 +791,50 @@ def test_random_partial_withdrawals_4(spec, state): @spec_state_test def test_random_partial_withdrawals_5(spec, state): yield from run_random_partial_withdrawals_test(spec, state, random.Random(5)) + + +# Tests with multiple blocks + +@with_capella_and_later +@spec_state_test +def test_success_two_payloads(spec, state): + fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( + spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4, + num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) + + next_slot(spec, state) + next_withdrawal_index = state.next_withdrawal_index + execution_payload = build_empty_execution_payload(spec, state) + expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) + verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + withdrawn_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] + fully_withdrawable_indices = list(set(fully_withdrawable_indices).difference(set(withdrawn_indices))) + partial_withdrawals_indices = list(set(partial_withdrawals_indices).difference(set(withdrawn_indices))) + assert state.next_withdrawal_index == next_withdrawal_index + spec.MAX_WITHDRAWALS_PER_PAYLOAD + + execution_payload = build_empty_execution_payload(spec, state) + expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) + verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + assert state.next_withdrawal_index == next_withdrawal_index + spec.MAX_WITHDRAWALS_PER_PAYLOAD * 2 + +@with_capella_and_later +@spec_state_test +def test_fail_second_payload_isnt_compatible(spec, state): + fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( + spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4, + num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) + + next_slot(spec, state) + next_withdrawal_index = state.next_withdrawal_index + execution_payload = build_empty_execution_payload(spec, state) + expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) + verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + withdrawn_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] + fully_withdrawable_indices = list(set(fully_withdrawable_indices).difference(set(withdrawn_indices))) + partial_withdrawals_indices = list(set(partial_withdrawals_indices).difference(set(withdrawn_indices))) + assert state.next_withdrawal_index == next_withdrawal_index + spec.MAX_WITHDRAWALS_PER_PAYLOAD + + execution_payload = build_empty_execution_payload(spec, state) + state.next_withdrawal_index += 1 + yield from run_withdrawals_processing(spec, state, execution_payload) + From e15b02d16fa7da892ac28c35352a27bffec376b9 Mon Sep 17 00:00:00 2001 From: Potuz Date: Fri, 4 Nov 2022 09:13:21 -0300 Subject: [PATCH 23/36] lint --- specs/capella/beacon-chain.md | 4 ++-- .../capella/block_processing/test_process_withdrawals.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 1526d518a..e2001d777 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -299,7 +299,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: address=ExecutionAddress(val.withdrawal_credentials[12:]), amount=balance, )) - withdrawal_index += 1 + withdrawal_index += WithdrawalIndex(1) elif is_partially_withdrawable_validator(val, balance): withdrawals.append(Withdrawal( index=withdrawal_index, @@ -307,7 +307,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: address=ExecutionAddress(val.withdrawal_credentials[12:]), amount=balance - MAX_EFFECTIVE_BALANCE, )) - withdrawal_index += 1 + withdrawal_index += WithdrawalIndex(1) if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: break return withdrawals diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index 7926e3426..6474fb4fd 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -23,6 +23,7 @@ from eth2spec.test.helpers.withdrawals import ( set_validator_partially_withdrawable, ) + def prepare_expected_withdrawals(spec, state, num_full_withdrawals=0, num_partial_withdrawals=0, rng=random.Random(5566)): assert num_full_withdrawals + num_partial_withdrawals <= len(state.validators) @@ -297,6 +298,7 @@ def test_fail_a_lot_partially_withdrawable_too_few_in_withdrawals(spec, state): yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) + @with_capella_and_later @spec_state_test def test_fail_a_lot_mixed_withdrawable_in_queue_too_few_in_withdrawals(spec, state): @@ -310,7 +312,6 @@ def test_fail_a_lot_mixed_withdrawable_in_queue_too_few_in_withdrawals(spec, sta yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) - # # Failure cases in which the withdrawals in the execution_payload are incorrect # @@ -794,7 +795,6 @@ def test_random_partial_withdrawals_5(spec, state): # Tests with multiple blocks - @with_capella_and_later @spec_state_test def test_success_two_payloads(spec, state): @@ -817,6 +817,7 @@ def test_success_two_payloads(spec, state): verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) assert state.next_withdrawal_index == next_withdrawal_index + spec.MAX_WITHDRAWALS_PER_PAYLOAD * 2 + @with_capella_and_later @spec_state_test def test_fail_second_payload_isnt_compatible(spec, state): @@ -836,5 +837,4 @@ def test_fail_second_payload_isnt_compatible(spec, state): execution_payload = build_empty_execution_payload(spec, state) state.next_withdrawal_index += 1 - yield from run_withdrawals_processing(spec, state, execution_payload) - + yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) From 8488fb79d9b50df4d45ba7f2b90bcdae60a8de72 Mon Sep 17 00:00:00 2001 From: Potuz Date: Fri, 4 Nov 2022 18:47:56 -0300 Subject: [PATCH 24/36] Alex Stokes' review --- specs/capella/beacon-chain.md | 11 ++++++----- .../block_processing/test_process_withdrawals.py | 6 +++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index e2001d777..0e24fadda 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -290,21 +290,21 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: withdrawals: List[Withdrawal] = [] for _ in range(len(state.validators)): validator_index = ValidatorIndex((validator_index + 1) % len(state.validators)) - val = state.validators[validator_index] + validator = state.validators[validator_index] balance = state.balances[validator_index] - if is_fully_withdrawable_validator(val, balance, epoch): + if is_fully_withdrawable_validator(validator, balance, epoch): withdrawals.append(Withdrawal( index=withdrawal_index, validator_index=validator_index, - address=ExecutionAddress(val.withdrawal_credentials[12:]), + address=ExecutionAddress(validator.withdrawal_credentials[12:]), amount=balance, )) withdrawal_index += WithdrawalIndex(1) - elif is_partially_withdrawable_validator(val, balance): + elif is_partially_withdrawable_validator(validator, balance): withdrawals.append(Withdrawal( index=withdrawal_index, validator_index=validator_index, - address=ExecutionAddress(val.withdrawal_credentials[12:]), + address=ExecutionAddress(validator.withdrawal_credentials[12:]), amount=balance - MAX_EFFECTIVE_BALANCE, )) withdrawal_index += WithdrawalIndex(1) @@ -324,6 +324,7 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: assert withdrawal == expected_withdrawal decrease_balance(state, withdrawal.validator_index, withdrawal.amount) if len(expected_withdrawals) > 0: + # withdrawal holds the last withdrawal object in the payload. state.next_withdrawal_index = WithdrawalIndex(withdrawal.index + 1) state.latest_withdrawal_validator_index = withdrawal.validator_index ``` diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index 6474fb4fd..31fb18753 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -42,6 +42,10 @@ def prepare_expected_withdrawals(spec, state, def verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices): + # Consider verifying also the condition when no withdrawals are expected. + if len(expected_withdrawals) == 0: + return + expected_withdrawals_validator_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] assert state.next_withdrawal_index == expected_withdrawals[-1].index + 1 assert state.latest_withdrawal_validator_index == expected_withdrawals_validator_indices[-1] @@ -128,7 +132,7 @@ def test_success_one_partial_withdrawal(spec, state): assert len(fully_withdrawable_indices) == 0 assert len(partial_withdrawals_indices) == 1 for index in partial_withdrawals_indices: - assert state.balances[index] != spec.MAX_EFFECTIVE_BALANCE + assert state.balances[index] > spec.MAX_EFFECTIVE_BALANCE next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) From c8d1614e7eadc7cfe252783885d6e316515902b1 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Fri, 4 Nov 2022 16:19:26 -0600 Subject: [PATCH 25/36] Apply suggestions from code review --- specs/capella/beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 0e24fadda..c362539be 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -324,7 +324,7 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: assert withdrawal == expected_withdrawal decrease_balance(state, withdrawal.validator_index, withdrawal.amount) if len(expected_withdrawals) > 0: - # withdrawal holds the last withdrawal object in the payload. + # `withdrawal` holds the last withdrawal object in the payload. state.next_withdrawal_index = WithdrawalIndex(withdrawal.index + 1) state.latest_withdrawal_validator_index = withdrawal.validator_index ``` From 3fc1ebce06ef5a51a32bb6640507d596358dfc14 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 7 Nov 2022 08:39:10 -0500 Subject: [PATCH 26/36] Move some Capella operation tests to sanity/blocks tests --- .../test_process_withdrawals.py | 63 +--------------- .../test/capella/sanity/test_blocks.py | 73 ++++++++++++++++++- .../eth2spec/test/helpers/withdrawals.py | 19 +++++ tests/generators/epoch_processing/main.py | 8 +- 4 files changed, 95 insertions(+), 68 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index 31fb18753..30af9ab51 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -18,28 +18,13 @@ from eth2spec.test.helpers.state import ( next_slot, ) from eth2spec.test.helpers.withdrawals import ( + prepare_expected_withdrawals, set_eth1_withdrawal_credential_with_balance, set_validator_fully_withdrawable, set_validator_partially_withdrawable, ) -def prepare_expected_withdrawals(spec, state, - num_full_withdrawals=0, num_partial_withdrawals=0, rng=random.Random(5566)): - assert num_full_withdrawals + num_partial_withdrawals <= len(state.validators) - all_validator_indices = list(range(len(state.validators))) - sampled_indices = rng.sample(all_validator_indices, num_full_withdrawals + num_partial_withdrawals) - fully_withdrawable_indices = rng.sample(sampled_indices, num_full_withdrawals) - partial_withdrawals_indices = list(set(sampled_indices).difference(set(fully_withdrawable_indices))) - - for index in fully_withdrawable_indices: - set_validator_fully_withdrawable(spec, state, index) - for index in partial_withdrawals_indices: - set_validator_partially_withdrawable(spec, state, index) - - return fully_withdrawable_indices, partial_withdrawals_indices - - def verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices): # Consider verifying also the condition when no withdrawals are expected. @@ -796,49 +781,3 @@ def test_random_partial_withdrawals_4(spec, state): @spec_state_test def test_random_partial_withdrawals_5(spec, state): yield from run_random_partial_withdrawals_test(spec, state, random.Random(5)) - - -# Tests with multiple blocks -@with_capella_and_later -@spec_state_test -def test_success_two_payloads(spec, state): - fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( - spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4, - num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) - - next_slot(spec, state) - next_withdrawal_index = state.next_withdrawal_index - execution_payload = build_empty_execution_payload(spec, state) - expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) - verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) - withdrawn_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] - fully_withdrawable_indices = list(set(fully_withdrawable_indices).difference(set(withdrawn_indices))) - partial_withdrawals_indices = list(set(partial_withdrawals_indices).difference(set(withdrawn_indices))) - assert state.next_withdrawal_index == next_withdrawal_index + spec.MAX_WITHDRAWALS_PER_PAYLOAD - - execution_payload = build_empty_execution_payload(spec, state) - expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) - verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) - assert state.next_withdrawal_index == next_withdrawal_index + spec.MAX_WITHDRAWALS_PER_PAYLOAD * 2 - - -@with_capella_and_later -@spec_state_test -def test_fail_second_payload_isnt_compatible(spec, state): - fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( - spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4, - num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) - - next_slot(spec, state) - next_withdrawal_index = state.next_withdrawal_index - execution_payload = build_empty_execution_payload(spec, state) - expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) - verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) - withdrawn_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] - fully_withdrawable_indices = list(set(fully_withdrawable_indices).difference(set(withdrawn_indices))) - partial_withdrawals_indices = list(set(partial_withdrawals_indices).difference(set(withdrawn_indices))) - assert state.next_withdrawal_index == next_withdrawal_index + spec.MAX_WITHDRAWALS_PER_PAYLOAD - - execution_payload = build_empty_execution_payload(spec, state) - state.next_withdrawal_index += 1 - yield from run_withdrawals_processing(spec, state, execution_payload, valid=False) diff --git a/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py index 011f0606a..73a40e539 100644 --- a/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py @@ -6,12 +6,17 @@ from eth2spec.test.helpers.state import ( state_transition_and_sign_block, ) from eth2spec.test.helpers.block import ( - build_empty_block_for_next_slot, build_empty_block, + build_empty_block_for_next_slot, + build_empty_block, ) from eth2spec.test.helpers.bls_to_execution_changes import get_signed_address_change +from eth2spec.test.helpers.state import ( + next_slot, +) from eth2spec.test.helpers.withdrawals import ( set_validator_fully_withdrawable, set_validator_partially_withdrawable, + prepare_expected_withdrawals, ) from eth2spec.test.helpers.voluntary_exits import prepare_signed_exits @@ -134,3 +139,69 @@ def test_exit_and_bls_change(spec, state): assert not spec.is_fully_withdrawable_validator(validator, balance, current_epoch) assert validator.withdrawable_epoch < spec.FAR_FUTURE_EPOCH assert spec.is_fully_withdrawable_validator(validator, balance, validator.withdrawable_epoch) + + +def _valid_withdrawl(spec, state): + fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( + spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4, + num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) + + next_slot(spec, state) + pre_next_withdrawal_index = state.next_withdrawal_index + + expected_withdrawals = spec.get_expected_withdrawals(state) + + pre_state = state.copy() + + # Block 1 + block = build_empty_block_for_next_slot(spec, state) + signed_block_1 = state_transition_and_sign_block(spec, state, block) + + withdrawn_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] + fully_withdrawable_indices = list(set(fully_withdrawable_indices).difference(set(withdrawn_indices))) + partial_withdrawals_indices = list(set(partial_withdrawals_indices).difference(set(withdrawn_indices))) + assert state.next_withdrawal_index == pre_next_withdrawal_index + spec.MAX_WITHDRAWALS_PER_PAYLOAD + + withdrawn_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] + fully_withdrawable_indices = list(set(fully_withdrawable_indices).difference(set(withdrawn_indices))) + partial_withdrawals_indices = list(set(partial_withdrawals_indices).difference(set(withdrawn_indices))) + assert state.next_withdrawal_index == pre_next_withdrawal_index + spec.MAX_WITHDRAWALS_PER_PAYLOAD + + return pre_state, signed_block_1, pre_next_withdrawal_index + + +@with_capella_and_later +@spec_state_test +def test_withdrawal_success_two_blocks(spec, state): + pre_state, signed_block_1, pre_next_withdrawal_index = _valid_withdrawl(spec, state) + + yield 'pre', pre_state + + # Block 2 + block = build_empty_block_for_next_slot(spec, state) + signed_block_2 = state_transition_and_sign_block(spec, state, block) + + assert state.next_withdrawal_index == pre_next_withdrawal_index + spec.MAX_WITHDRAWALS_PER_PAYLOAD * 2 + + yield 'blocks', [signed_block_1, signed_block_2] + yield 'post', state + + +@with_capella_and_later +@spec_state_test +def test_withdrawal_fail_second_block_payload_isnt_compatible(spec, state): + _valid_withdrawl(spec, state) + + # Block 2 + block = build_empty_block_for_next_slot(spec, state) + + # Modify state.next_withdrawal_index to incorrect number + state.next_withdrawal_index += 1 + + # Only need to output the state transition of signed_block_2 + yield 'pre', state + + signed_block_2 = state_transition_and_sign_block(spec, state, block, expect_fail=True) + + yield 'blocks', [signed_block_2] + yield 'post', None diff --git a/tests/core/pyspec/eth2spec/test/helpers/withdrawals.py b/tests/core/pyspec/eth2spec/test/helpers/withdrawals.py index 526ac0caa..35349839b 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/helpers/withdrawals.py @@ -1,3 +1,6 @@ +import random + + def set_validator_fully_withdrawable(spec, state, index, withdrawable_epoch=None): if withdrawable_epoch is None: withdrawable_epoch = spec.get_current_epoch(state) @@ -29,3 +32,19 @@ def set_validator_partially_withdrawable(spec, state, index, excess_balance=1000 validator = state.validators[index] assert spec.is_partially_withdrawable_validator(validator, state.balances[index]) + + +def prepare_expected_withdrawals(spec, state, + num_full_withdrawals=0, num_partial_withdrawals=0, rng=random.Random(5566)): + assert num_full_withdrawals + num_partial_withdrawals <= len(state.validators) + all_validator_indices = list(range(len(state.validators))) + sampled_indices = rng.sample(all_validator_indices, num_full_withdrawals + num_partial_withdrawals) + fully_withdrawable_indices = rng.sample(sampled_indices, num_full_withdrawals) + partial_withdrawals_indices = list(set(sampled_indices).difference(set(fully_withdrawable_indices))) + + for index in fully_withdrawable_indices: + set_validator_fully_withdrawable(spec, state, index) + for index in partial_withdrawals_indices: + set_validator_partially_withdrawable(spec, state, index) + + return fully_withdrawable_indices, partial_withdrawals_indices diff --git a/tests/generators/epoch_processing/main.py b/tests/generators/epoch_processing/main.py index 946a6c2c0..fc57fbe4e 100644 --- a/tests/generators/epoch_processing/main.py +++ b/tests/generators/epoch_processing/main.py @@ -27,11 +27,9 @@ if __name__ == "__main__": # so no additional tests required. bellatrix_mods = altair_mods - _new_capella_mods = {key: 'eth2spec.test.capella.epoch_processing.test_process_' + key for key in [ - 'full_withdrawals', - 'partial_withdrawals', - ]} - capella_mods = combine_mods(_new_capella_mods, altair_mods) + # No epoch-processing changes in Capella and previous testing repeats with new types, + # so no additional tests required. + capella_mods = bellatrix_mods # TODO Custody Game testgen is disabled for now # custody_game_mods = {**{key: 'eth2spec.test.custody_game.epoch_processing.test_process_' + key for key in [ From dac756efee181d27449a30b62c0280178248ddd6 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 7 Nov 2022 09:09:09 -0500 Subject: [PATCH 27/36] Minor clean up --- presets/minimal/capella.yaml | 2 +- specs/capella/beacon-chain.md | 2 +- .../test/capella/block_processing/test_process_withdrawals.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/presets/minimal/capella.yaml b/presets/minimal/capella.yaml index 476738f3f..1ac8a806c 100644 --- a/presets/minimal/capella.yaml +++ b/presets/minimal/capella.yaml @@ -8,5 +8,5 @@ MAX_BLS_TO_EXECUTION_CHANGES: 16 # Execution # --------------------------------------------------------------- -# 2**2 (= 4) +# [customized] 2**2 (= 4) MAX_WITHDRAWALS_PER_PAYLOAD: 4 diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index c362539be..88777d1d6 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -276,7 +276,7 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: process_execution_payload(state, block.body.execution_payload, EXECUTION_ENGINE) # [Modified in Capella] process_randao(state, block.body) process_eth1_data(state, block.body) - process_operations(state, block.body) + process_operations(state, block.body) # [Modified in Capella] process_sync_aggregate(state, block.body.sync_aggregate) ``` diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index 30af9ab51..fbf0387b4 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -741,8 +741,8 @@ def run_random_partial_withdrawals_test(spec, state, rng): execution_payload = build_empty_execution_payload(spec, state) - # Note: due to the randomness and other epoch processing, some of these set as "partially withdrawable" - # may not be partially withdrawable once we get to ``process_partial_withdrawals``, + # Note: due to the randomness and other block processing, some of these set as "partially withdrawable" + # may not be partially withdrawable once we get to ``process_withdrawals``, # thus *not* using the optional third param in this call yield from run_withdrawals_processing(spec, state, execution_payload) From 2f89f5096d288c9b774e6bbf498bbaa817de5f46 Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 7 Nov 2022 17:29:56 -0300 Subject: [PATCH 28/36] Danny's review 1st pass --- specs/capella/beacon-chain.md | 6 +++--- .../core/pyspec/eth2spec/test/capella/sanity/test_blocks.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 88777d1d6..87dacc643 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -324,9 +324,9 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: assert withdrawal == expected_withdrawal decrease_balance(state, withdrawal.validator_index, withdrawal.amount) if len(expected_withdrawals) > 0: - # `withdrawal` holds the last withdrawal object in the payload. - state.next_withdrawal_index = WithdrawalIndex(withdrawal.index + 1) - state.latest_withdrawal_validator_index = withdrawal.validator_index + latest_withdrawal = payload.withdrawals[-1] + state.next_withdrawal_index = WithdrawalIndex(latest_withdrawal.index + 1) + state.latest_withdrawal_validator_index = latest_withdrawal.validator_index ``` #### Modified `process_execution_payload` diff --git a/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py index 73a40e539..0636dd58f 100644 --- a/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py @@ -141,7 +141,7 @@ def test_exit_and_bls_change(spec, state): assert spec.is_fully_withdrawable_validator(validator, balance, validator.withdrawable_epoch) -def _valid_withdrawl(spec, state): +def _valid_withdrawal(spec, state): fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) @@ -173,7 +173,7 @@ def _valid_withdrawl(spec, state): @with_capella_and_later @spec_state_test def test_withdrawal_success_two_blocks(spec, state): - pre_state, signed_block_1, pre_next_withdrawal_index = _valid_withdrawl(spec, state) + pre_state, signed_block_1, pre_next_withdrawal_index = _valid_withdrawal(spec, state) yield 'pre', pre_state @@ -190,7 +190,7 @@ def test_withdrawal_success_two_blocks(spec, state): @with_capella_and_later @spec_state_test def test_withdrawal_fail_second_block_payload_isnt_compatible(spec, state): - _valid_withdrawl(spec, state) + _valid_withdrawal(spec, state) # Block 2 block = build_empty_block_for_next_slot(spec, state) From ac670e2c4770086b68bf530e00b661778ff36d91 Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 7 Nov 2022 17:34:19 -0300 Subject: [PATCH 29/36] add phase0 sentence regarding process slots --- specs/capella/validator.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/specs/capella/validator.md b/specs/capella/validator.md index 4f9d538e0..bd91df49c 100644 --- a/specs/capella/validator.md +++ b/specs/capella/validator.md @@ -58,6 +58,9 @@ All validator responsibilities remain unchanged other than those noted below. expected withdrawals for the slot must be gathered from the `state` (utilizing the helper `get_expected_withdrawals`) and passed into the `ExecutionEngine` within `prepare_execution_payload`. +*Note*: In this section, `state` is the state of the slot for the block proposal _without_ the block yet applied. +That is, `state` is the `previous_state` processed through any empty slots up to the assigned slot using `process_slots(previous_state, slot)`. + *Note*: The only change made to `prepare_execution_payload` is to call `get_expected_withdrawals()` to set the new `withdrawals` field of `PayloadAttributes`. From 99e2704c2c73d259df4be706d28a5cb908379b1e Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 7 Nov 2022 17:56:01 -0300 Subject: [PATCH 30/36] make tests pass --- specs/capella/beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 87dacc643..c9c6a37e7 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -324,7 +324,7 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: assert withdrawal == expected_withdrawal decrease_balance(state, withdrawal.validator_index, withdrawal.amount) if len(expected_withdrawals) > 0: - latest_withdrawal = payload.withdrawals[-1] + latest_withdrawal = expected_withdrawals[-1] state.next_withdrawal_index = WithdrawalIndex(latest_withdrawal.index + 1) state.latest_withdrawal_validator_index = latest_withdrawal.validator_index ``` From 8f42e485c773a940181c94d1d5edb4e872142c69 Mon Sep 17 00:00:00 2001 From: Potuz Date: Tue, 8 Nov 2022 19:53:58 -0300 Subject: [PATCH 31/36] Add extra % --- specs/capella/beacon-chain.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index c9c6a37e7..a820d213d 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -286,10 +286,9 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: epoch = get_current_epoch(state) withdrawal_index = state.next_withdrawal_index - validator_index = state.latest_withdrawal_validator_index + validator_index = ValidatorIndex((state.latest_validator_index + 1) % len(state.validators)) withdrawals: List[Withdrawal] = [] for _ in range(len(state.validators)): - validator_index = ValidatorIndex((validator_index + 1) % len(state.validators)) validator = state.validators[validator_index] balance = state.balances[validator_index] if is_fully_withdrawable_validator(validator, balance, epoch): @@ -310,6 +309,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: withdrawal_index += WithdrawalIndex(1) if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: break + validator_index = ValidatorIndex((validator_index + 1) % len(state.validators)) return withdrawals ``` From 710b124cdc3c52681c7f93027f44573e08aea539 Mon Sep 17 00:00:00 2001 From: Potuz Date: Tue, 8 Nov 2022 21:51:54 -0300 Subject: [PATCH 32/36] fix last commit --- specs/capella/beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index a820d213d..e72a0dc1a 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -286,7 +286,7 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: epoch = get_current_epoch(state) withdrawal_index = state.next_withdrawal_index - validator_index = ValidatorIndex((state.latest_validator_index + 1) % len(state.validators)) + validator_index = ValidatorIndex((state.latest_withdrawal_validator_index + 1) % len(state.validators)) withdrawals: List[Withdrawal] = [] for _ in range(len(state.validators)): validator = state.validators[validator_index] From 087f785ee924edba32a8a7a598ed7133b649ca23 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 9 Nov 2022 17:15:00 -0500 Subject: [PATCH 33/36] PR feedback from @djrtwo --- .../test_process_withdrawals.py | 42 ++++++++++++------- .../test/capella/sanity/test_blocks.py | 6 +-- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index fbf0387b4..ae8e5af3c 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -46,7 +46,8 @@ def verify_post_state(state, spec, expected_withdrawals, assert state.balances[index] > spec.MAX_EFFECTIVE_BALANCE -def run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=None, valid=True): +def run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=None, + fully_withdrawable_indices=None, partial_withdrawals_indices=None, valid=True): """ Run ``process_execution_payload``, yielding: - pre-state ('pre') @@ -79,6 +80,9 @@ def run_withdrawals_processing(spec, state, execution_payload, num_expected_with elif len(expected_withdrawals) > spec.MAX_WITHDRAWALS_PER_PAYLOAD: raise ValueError('len(expected_withdrawals) should not be greater than MAX_WITHDRAWALS_PER_PAYLOAD') + if fully_withdrawable_indices is not None or partial_withdrawals_indices is not None: + verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + return expected_withdrawals @@ -104,9 +108,10 @@ def test_success_one_full_withdrawal(spec, state): next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) - expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) - - verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + yield from run_withdrawals_processing( + spec, state, execution_payload, + fully_withdrawable_indices=fully_withdrawable_indices, + partial_withdrawals_indices=partial_withdrawals_indices) @with_capella_and_later @@ -122,9 +127,11 @@ def test_success_one_partial_withdrawal(spec, state): next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) - expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) - - verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + yield from run_withdrawals_processing( + spec, state, execution_payload, + fully_withdrawable_indices=fully_withdrawable_indices, + partial_withdrawals_indices=partial_withdrawals_indices + ) @with_capella_and_later @@ -139,9 +146,10 @@ def test_success_max_per_slot(spec, state): next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) - expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) - - verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + yield from run_withdrawals_processing( + spec, state, execution_payload, + fully_withdrawable_indices=fully_withdrawable_indices, + partial_withdrawals_indices=partial_withdrawals_indices) @with_capella_and_later @@ -153,9 +161,10 @@ def test_success_all_fully_withdrawable(spec, state): next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) - expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) - - verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + yield from run_withdrawals_processing( + spec, state, execution_payload, + fully_withdrawable_indices=fully_withdrawable_indices, + partial_withdrawals_indices=partial_withdrawals_indices) @with_capella_and_later @@ -167,9 +176,10 @@ def test_success_all_partially_withdrawable(spec, state): next_slot(spec, state) execution_payload = build_empty_execution_payload(spec, state) - expected_withdrawals = yield from run_withdrawals_processing(spec, state, execution_payload) - - verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices) + yield from run_withdrawals_processing( + spec, state, execution_payload, + fully_withdrawable_indices=fully_withdrawable_indices, + partial_withdrawals_indices=partial_withdrawals_indices) # diff --git a/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py index 0636dd58f..f3ad843b1 100644 --- a/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py @@ -141,7 +141,7 @@ def test_exit_and_bls_change(spec, state): assert spec.is_fully_withdrawable_validator(validator, balance, validator.withdrawable_epoch) -def _valid_withdrawal(spec, state): +def _perform_valid_withdrawal(spec, state): fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals( spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4) @@ -173,7 +173,7 @@ def _valid_withdrawal(spec, state): @with_capella_and_later @spec_state_test def test_withdrawal_success_two_blocks(spec, state): - pre_state, signed_block_1, pre_next_withdrawal_index = _valid_withdrawal(spec, state) + pre_state, signed_block_1, pre_next_withdrawal_index = _perform_valid_withdrawal(spec, state) yield 'pre', pre_state @@ -190,7 +190,7 @@ def test_withdrawal_success_two_blocks(spec, state): @with_capella_and_later @spec_state_test def test_withdrawal_fail_second_block_payload_isnt_compatible(spec, state): - _valid_withdrawal(spec, state) + _perform_valid_withdrawal(spec, state) # Block 2 block = build_empty_block_for_next_slot(spec, state) From 7f266bcb0fc740f8b781e404c4836313e73c498d Mon Sep 17 00:00:00 2001 From: Potuz Date: Thu, 10 Nov 2022 08:33:11 -0300 Subject: [PATCH 34/36] Use next_validator_withdrawal_index --- specs/capella/beacon-chain.md | 7 ++++--- specs/capella/fork.md | 2 +- .../capella/block_processing/test_process_withdrawals.py | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index e72a0dc1a..90a64925f 100644 --- a/specs/capella/beacon-chain.md +++ b/specs/capella/beacon-chain.md @@ -221,7 +221,7 @@ class BeaconState(Container): latest_execution_payload_header: ExecutionPayloadHeader # Withdrawals next_withdrawal_index: WithdrawalIndex # [New in Capella] - latest_withdrawal_validator_index: ValidatorIndex # [New in Capella] + next_withdrawal_validator_index: ValidatorIndex # [New in Capella] ``` ## Helpers @@ -286,7 +286,7 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]: epoch = get_current_epoch(state) withdrawal_index = state.next_withdrawal_index - validator_index = ValidatorIndex((state.latest_withdrawal_validator_index + 1) % len(state.validators)) + validator_index = state.next_withdrawal_validator_index withdrawals: List[Withdrawal] = [] for _ in range(len(state.validators)): validator = state.validators[validator_index] @@ -326,7 +326,8 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: if len(expected_withdrawals) > 0: latest_withdrawal = expected_withdrawals[-1] state.next_withdrawal_index = WithdrawalIndex(latest_withdrawal.index + 1) - state.latest_withdrawal_validator_index = latest_withdrawal.validator_index + next_validator_index = ValidatorIndex((latest_withdrawal.validator_index + 1) % len(state.validators)) + state.next_withdrawal_validator_index = next_validator_index ``` #### Modified `process_execution_payload` diff --git a/specs/capella/fork.md b/specs/capella/fork.md index 1ea43b463..e2493d33b 100644 --- a/specs/capella/fork.md +++ b/specs/capella/fork.md @@ -130,7 +130,7 @@ def upgrade_to_capella(pre: bellatrix.BeaconState) -> BeaconState: latest_execution_payload_header=latest_execution_payload_header, # Withdrawals next_withdrawal_index=WithdrawalIndex(0), - latest_withdrawal_validator_index=ValidatorIndex(0), + next_withdrawal_validator_index=ValidatorIndex(0), ) return post diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index fbf0387b4..843753c6c 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -33,7 +33,7 @@ def verify_post_state(state, spec, expected_withdrawals, expected_withdrawals_validator_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] assert state.next_withdrawal_index == expected_withdrawals[-1].index + 1 - assert state.latest_withdrawal_validator_index == expected_withdrawals_validator_indices[-1] + assert state.next_withdrawal_validator_index == (expected_withdrawals_validator_indices[-1]+1) % len(state.validators) for index in fully_withdrawable_indices: if index in expected_withdrawals_validator_indices: assert state.balances[index] == 0 @@ -732,7 +732,7 @@ def run_random_partial_withdrawals_test(spec, state, rng): randomize_state(spec, state, rng) num_validators = len(state.validators) - state.latest_withdrawal_validator_index = rng.randint(0, num_validators - 1) + state.next_withdrawal_validator_index = rng.randint(0, num_validators - 1) num_partially_withdrawable = rng.randint(0, num_validators - 1) partially_withdrawable_indices = rng.sample(range(num_validators), num_partially_withdrawable) From 3d82a19b3b143033e96063bc6a4262a9f0d9e93d Mon Sep 17 00:00:00 2001 From: Potuz Date: Thu, 10 Nov 2022 08:41:55 -0300 Subject: [PATCH 35/36] whitespace --- .../test/capella/block_processing/test_process_withdrawals.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index 943277a83..bab60a03d 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -96,7 +96,6 @@ def test_success_zero_expected_withdrawals(spec, state): yield from run_withdrawals_processing(spec, state, execution_payload) - @with_capella_and_later @spec_state_test def test_success_one_full_withdrawal(spec, state): From c7d733303f8f9ef32244dec43cc5b26150acc7b8 Mon Sep 17 00:00:00 2001 From: Potuz Date: Thu, 10 Nov 2022 08:47:16 -0300 Subject: [PATCH 36/36] lint --- .../test/capella/block_processing/test_process_withdrawals.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py index bab60a03d..7b39f2b9d 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py @@ -33,7 +33,8 @@ def verify_post_state(state, spec, expected_withdrawals, expected_withdrawals_validator_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals] assert state.next_withdrawal_index == expected_withdrawals[-1].index + 1 - assert state.next_withdrawal_validator_index == (expected_withdrawals_validator_indices[-1]+1) % len(state.validators) + next_withdrawal_validator_index = (expected_withdrawals_validator_indices[-1] + 1) % len(state.validators) + assert state.next_withdrawal_validator_index == next_withdrawal_validator_index for index in fully_withdrawable_indices: if index in expected_withdrawals_validator_indices: assert state.balances[index] == 0 @@ -96,6 +97,7 @@ def test_success_zero_expected_withdrawals(spec, state): yield from run_withdrawals_processing(spec, state, execution_payload) + @with_capella_and_later @spec_state_test def test_success_one_full_withdrawal(spec, state):