Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge test vectors: enable phase0 tests for Merge + start on new testing #2380

Merged
merged 17 commits into from
May 10, 2021
Merged

Merge test vectors: enable phase0 tests for Merge + start on new testing #2380

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f37f9a3
include merge in generators
protolambda May 4, 2021
ab693c9
update spec test constants for merge
protolambda May 4, 2021
521cffc
update execution-payload processing to isolate payload from block body
protolambda May 4, 2021
25d0d67
start testing of merge functionality
protolambda May 4, 2021
36032fd
update doc about format
protolambda May 4, 2021
79fc411
Adjust is_transition_block call in fork-choice
mkalinin May 5, 2021
e2be761
introduce merge fork version
protolambda May 5, 2021
470c6dc
update test runner to handle merge phase
protolambda May 5, 2021
ff3a82e
fix transactions field in exec payload helper
protolambda May 5, 2021
865d7db
update altair tests to not collide with Merge + fix merge test triggers
protolambda May 5, 2021
cc11328
fix merge forkchoice tests with mock get_pow_block
protolambda May 5, 2021
00cd1c3
fix forkchoice unittest not recognizing merge spec
protolambda May 5, 2021
2ef6291
Minimal execution payload test, more merge-specific testing in later PR
protolambda May 5, 2021
56bcb63
Lint fixes for merge testing update
protolambda May 5, 2021
8ac59b7
fix old ssz-static todo comment
protolambda May 5, 2021
76b5974
is_execution_enabled function + misc review fixes
protolambda May 6, 2021
42733b7
remove merge-test exec-payload trigger debug helper
protolambda May 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ def sundry_functions(cls) -> str:


def get_pow_block(hash: Bytes32) -> PowBlock:
pass
return PowBlock(block_hash=hash, is_valid=True, is_processed=True, total_difficulty=TRANSITION_TOTAL_DIFFICULTY)


def get_execution_state(execution_state_root: Bytes32) -> ExecutionState:
Expand Down
34 changes: 17 additions & 17 deletions specs/merge/beacon-chain.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
- [Introduction](#introduction)
- [Custom types](#custom-types)
- [Constants](#constants)
- [Transition](#transition)
- [Execution](#execution)
- [Configuration](#configuration)
- [Containers](#containers)
- [Extended containers](#extended-containers)
- [`BeaconBlockBody`](#beaconblockbody)
Expand Down Expand Up @@ -50,12 +50,6 @@ We define the following Python custom types for type hinting and readability:

## Constants

### Transition

| Name | Value |
| - | - |
| `TRANSITION_TOTAL_DIFFICULTY` | **TBD** |

### Execution

| Name | Value |
Expand All @@ -64,6 +58,16 @@ We define the following Python custom types for type hinting and readability:
| `MAX_EXECUTION_TRANSACTIONS` | `uint64(2**14)` (= 16,384) |
| `BYTES_PER_LOGS_BLOOM` | `uint64(2**8)` (= 256) |

## Configuration

Warning: this configuration is not definitive.

| Name | Value |
| - | - |
| `MERGE_FORK_VERSION` | `Version('0x02000000')` |
| `MERGE_FORK_EPOCH` | `Epoch(18446744073709551615)` **TBD** |
| `TRANSITION_TOTAL_DIFFICULTY` | **TBD** |

## Containers

### Extended containers
Expand Down Expand Up @@ -146,8 +150,8 @@ def is_transition_completed(state: BeaconState) -> bool:
#### `is_transition_block`

```python
def is_transition_block(state: BeaconState, block_body: BeaconBlockBody) -> bool:
return not is_transition_completed(state) and block_body.execution_payload != ExecutionPayload()
def is_transition_block(state: BeaconState, block: BeaconBlock) -> bool:
return not is_transition_completed(state) and block.body.execution_payload != ExecutionPayload()
```

#### `compute_time_at_slot`
Expand All @@ -168,7 +172,9 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None:
process_randao(state, block.body)
process_eth1_data(state, block.body)
process_operations(state, block.body)
process_execution_payload(state, block.body) # [New in Merge]
# Pre-merge, skip execution payload processing
if is_transition_completed(state) or is_transition_block(state, block):
process_execution_payload(state, block.body.execution_payload) # [New in Merge]
Copy link
Contributor

@hwwhww hwwhww May 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two issues:

  1. Looking at tests/formats/operations/README.md and I just realized that we put sync_aggregate as part of "operation" tests but process_sync_committee is not part of the process_operations function like execution_payload.
    1. IMO it's good to consider sync_aggregate as an operation.
    2. If we want to simplify it, I think execution_payload could also be considered as an operation since it's essentially "transactions".
  2. I can see why it looks good to extract the is_transition_completed(state) or is_transition_block(state, block) condition checks from process_execution_payload, but it makes it a bit unclear where/how to test these conditions in test cases.

What do you think about:

  1. Move process_sync_committee and process_execution_payload calls into process_operations.
  2. Revert the if is_transition_completed(state) or is_transition_block(state, block) checks extraction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Regarding process_sync_committee: sure, but let's keep that separate from merge-testing for now. Moving the process_execution_payload to process_operations would be good too. I'm honestly surprised we didn't do the same for the Randao and Eth1Data functionality.
  2. The if-statement was purposefully extracted from the process_execution_payload function: this way the function input can just be an ExecutionPayload, and not a complete beacon block body. This helps with future compatibility and reduces test complexity (We don't have to set up a complete beacon block for every single test case). We can test that one if-statement as part of the regular full beacon block testing. And in future phases after the merge, we may even remove the if-statement entirely, since the merge-transition check will always be true.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For (2), it might make sense to put it behind a function is_execution_enabled(state, block). This is semantically cleaner for any chain/setting that doesn't have the transition. They can just override the function to return True and as you mentioned, in the future we can just remove it.

```

#### Execution payload processing
Expand All @@ -181,16 +187,10 @@ The body of the function is implementation dependent.
##### `process_execution_payload`

```python
def process_execution_payload(state: BeaconState, body: BeaconBlockBody) -> None:
def process_execution_payload(state: BeaconState, execution_payload: ExecutionPayload) -> None:
"""
Note: This function is designed to be able to be run in parallel with the other `process_block` sub-functions
"""
# Pre-merge, skip processing
if not is_transition_completed(state) and not is_transition_block(state, body):
return

execution_payload = body.execution_payload

if is_transition_completed(state):
assert execution_payload.parent_hash == state.latest_execution_payload_header.block_hash
assert execution_payload.number == state.latest_execution_payload_header.number + 1
Expand Down
2 changes: 1 addition & 1 deletion specs/merge/fork-choice.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None:
assert get_ancestor(store, block.parent_root, finalized_slot) == store.finalized_checkpoint.root

# [New in Merge]
if is_transition_block(pre_state, block.body):
if is_transition_block(pre_state, block):
# Delay consideration of block until PoW block is processed by the PoW node
pow_block = get_pow_block(block.body.execution_payload.parent_hash)
assert pow_block.is_processed
Expand Down
21 changes: 10 additions & 11 deletions tests/core/pyspec/eth2spec/test/altair/block_processing/test_process_sync_committee.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@
transition_to,
)
from eth2spec.test.helpers.constants import (
PHASE0,
MAINNET, MINIMAL,
)
from eth2spec.test.helpers.sync_committee import (
compute_aggregate_sync_committee_signature,
)
from eth2spec.test.context import (
expect_assertion_error,
with_all_phases_except,
with_altair_and_later,
with_configs,
spec_state_test,
always_bls,
Expand Down Expand Up @@ -62,7 +61,7 @@ def get_committee_indices(spec, state, duplicates=False):
state.randao_mixes[randao_index] = hash(state.randao_mixes[randao_index])


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
@always_bls
def test_invalid_signature_missing_participant(spec, state):
Expand All @@ -84,7 +83,7 @@ def test_invalid_signature_missing_participant(spec, state):
yield from run_sync_committee_processing(spec, state, block, expect_exception=True)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
@always_bls
def test_invalid_signature_extra_participant(spec, state):
Expand Down Expand Up @@ -197,7 +196,7 @@ def run_successful_sync_committee_test(spec, state, committee, committee_bits):
)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@with_configs([MINIMAL], reason="to create nonduplicate committee")
@spec_state_test
def test_sync_committee_rewards_nonduplicate_committee(spec, state):
Expand All @@ -213,7 +212,7 @@ def test_sync_committee_rewards_nonduplicate_committee(spec, state):
yield from run_successful_sync_committee_test(spec, state, committee, committee_bits)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@with_configs([MAINNET], reason="to create duplicate committee")
@spec_state_test
def test_sync_committee_rewards_duplicate_committee(spec, state):
Expand All @@ -229,7 +228,7 @@ def test_sync_committee_rewards_duplicate_committee(spec, state):
yield from run_successful_sync_committee_test(spec, state, committee, committee_bits)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
@always_bls
def test_sync_committee_rewards_not_full_participants(spec, state):
Expand All @@ -240,7 +239,7 @@ def test_sync_committee_rewards_not_full_participants(spec, state):
yield from run_successful_sync_committee_test(spec, state, committee, committee_bits)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
@always_bls
def test_sync_committee_rewards_empty_participants(spec, state):
Expand All @@ -250,7 +249,7 @@ def test_sync_committee_rewards_empty_participants(spec, state):
yield from run_successful_sync_committee_test(spec, state, committee, committee_bits)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
@always_bls
def test_invalid_signature_past_block(spec, state):
Expand Down Expand Up @@ -289,7 +288,7 @@ def test_invalid_signature_past_block(spec, state):
yield from run_sync_committee_processing(spec, state, invalid_block, expect_exception=True)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@with_configs([MINIMAL], reason="to produce different committee sets")
@spec_state_test
@always_bls
Expand Down Expand Up @@ -326,7 +325,7 @@ def test_invalid_signature_previous_committee(spec, state):
yield from run_sync_committee_processing(spec, state, block, expect_exception=True)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
@always_bls
@with_configs([MINIMAL], reason="too slow")
Expand Down
13 changes: 5 additions & 8 deletions .../core/pyspec/eth2spec/test/altair/epoch_processing/test_process_sync_committee_updates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,13 @@
always_bls,
spec_state_test,
spec_test,
with_all_phases_except,
with_altair_and_later,
with_configs,
with_custom_state,
single_phase,
misc_balances,
)
from eth2spec.test.helpers.constants import (
PHASE0,
MINIMAL,
)
from eth2spec.test.helpers.constants import MINIMAL
from eth2spec.test.helpers.state import transition_to
from eth2spec.test.helpers.epoch_processing import (
run_epoch_processing_with,
Expand Down Expand Up @@ -49,7 +46,7 @@ def run_sync_committees_progress_test(spec, state):
assert state.next_sync_committee == third_sync_committee


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
@always_bls
@with_configs([MINIMAL], reason="too slow")
Expand All @@ -60,7 +57,7 @@ def test_sync_committees_progress_genesis(spec, state):
yield from run_sync_committees_progress_test(spec, state)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
@always_bls
@with_configs([MINIMAL], reason="too slow")
Expand All @@ -73,7 +70,7 @@ def test_sync_committees_progress_not_genesis(spec, state):
yield from run_sync_committees_progress_test(spec, state)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@with_custom_state(balances_fn=misc_balances, threshold_fn=lambda spec: spec.EJECTION_BALANCE)
@spec_test
@single_phase
Expand Down
17 changes: 8 additions & 9 deletions tests/core/pyspec/eth2spec/test/altair/sanity/test_blocks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@
from eth2spec.test.helpers.sync_committee import (
compute_aggregate_sync_committee_signature,
)
from eth2spec.test.helpers.constants import PHASE0
from eth2spec.test.context import (
with_all_phases_except,
with_altair_and_later,
spec_state_test,
)

Expand All @@ -40,46 +39,46 @@ def run_sync_committee_sanity_test(spec, state, fraction_full=1.0):
yield 'post', state


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
def test_full_sync_committee_committee(spec, state):
next_epoch(spec, state)
yield from run_sync_committee_sanity_test(spec, state, fraction_full=1.0)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
def test_half_sync_committee_committee(spec, state):
next_epoch(spec, state)
yield from run_sync_committee_sanity_test(spec, state, fraction_full=0.5)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
def test_empty_sync_committee_committee(spec, state):
next_epoch(spec, state)
yield from run_sync_committee_sanity_test(spec, state, fraction_full=0.0)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
def test_full_sync_committee_committee_genesis(spec, state):
yield from run_sync_committee_sanity_test(spec, state, fraction_full=1.0)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
def test_half_sync_committee_committee_genesis(spec, state):
yield from run_sync_committee_sanity_test(spec, state, fraction_full=0.5)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
def test_empty_sync_committee_committee_genesis(spec, state):
yield from run_sync_committee_sanity_test(spec, state, fraction_full=0.0)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@spec_state_test
def test_inactivity_scores(spec, state):
for _ in range(spec.MIN_EPOCHS_TO_INACTIVITY_PENALTY + 2):
Expand Down
9 changes: 4 additions & 5 deletions tests/core/pyspec/eth2spec/test/altair/unittests/validator/test_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
from eth2spec.utils import bls
from eth2spec.utils.bls import only_with_bls
from eth2spec.test.context import (
PHASE0,
with_all_phases_except,
with_altair_and_later,
with_state,
)

Expand All @@ -25,7 +24,7 @@ def ensure_assignments_in_sync_committee(
assert spec.is_assigned_to_sync_committee(state, epoch, validator_index)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@with_state
def test_is_assigned_to_sync_committee(phases, spec, state):
epoch = spec.get_current_epoch(state)
Expand Down Expand Up @@ -91,7 +90,7 @@ def _get_sync_committee_signature(


@only_with_bls()
@with_all_phases_except([PHASE0])
@with_altair_and_later
@with_state
def test_process_sync_committee_contributions(phases, spec, state):
# skip over slots at genesis
Expand Down Expand Up @@ -144,7 +143,7 @@ def _subnet_for_sync_committee_index(spec, i):
return i // (spec.SYNC_COMMITTEE_SIZE // spec.SYNC_COMMITTEE_SUBNET_COUNT)


@with_all_phases_except([PHASE0])
@with_altair_and_later
@with_state
def test_compute_subnets_for_sync_committee(state, spec, phases):
some_sync_committee_members = list(
Expand Down
Loading