Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 2 additions & 2 deletions pysetup/spec_builders/deneb.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ def preparations(cls):
@classmethod
def sundry_functions(cls) -> str:
return '''
def retrieve_blobs_and_proofs(beacon_block_root: Root) -> PyUnion[Tuple[Blob, KZGProof], Tuple[str, str]]:
def retrieve_blobs_and_proofs(beacon_block_root: Root) -> Tuple[Sequence[Blob], Sequence[KZGProof]]:
# pylint: disable=unused-argument
return ("TEST", "TEST")'''
return [], []'''

@classmethod
def execution_engine_cls(cls) -> str:
Expand Down
5 changes: 0 additions & 5 deletions specs/deneb/fork-choice.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ def is_data_available(beacon_block_root: Root, blob_kzg_commitments: Sequence[KZ
# `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS`
blobs, proofs = retrieve_blobs_and_proofs(beacon_block_root)

# For testing, `retrieve_blobs_and_proofs` returns ("TEST", "TEST").
# TODO: Remove it once we have a way to inject `BlobSidecar` into tests.
if isinstance(blobs, str) or isinstance(proofs, str):
return True

return verify_blob_kzg_proof_batch(blobs, blob_kzg_commitments, proofs)
```

Expand Down
Empty file.
136 changes: 136 additions & 0 deletions tests/core/pyspec/eth2spec/test/deneb/fork_choice/test_on_block.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
from random import Random

from eth2spec.test.context import (
spec_state_test,
with_deneb_and_later,
)

from eth2spec.test.helpers.block import (
build_empty_block_for_next_slot,
)
from eth2spec.test.helpers.fork_choice import (
BlobData,
get_genesis_forkchoice_store_and_block,
on_tick_and_append_step,
tick_and_add_block,
with_blob_data,
)
from eth2spec.test.helpers.state import (
state_transition_and_sign_block,
)
from eth2spec.test.helpers.sharding import (
get_sample_opaque_tx

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
get_sample_opaque_tx
get_sample_opaque_tx,

)


def get_block_with_blob(spec, state, rng=None):
block = build_empty_block_for_next_slot(spec, state)
opaque_tx, blobs, blob_kzg_commitments, blob_kzg_proofs = get_sample_opaque_tx(spec, blob_count=1, rng=rng)
block.body.execution_payload.transactions = [opaque_tx]
# block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this intentionally left here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, it should have been uncommented although it's unrelated here.

block.body.blob_kzg_commitments = blob_kzg_commitments
return block, blobs, blob_kzg_proofs


@with_deneb_and_later
@spec_state_test
def test_simple_blob_data(spec, state):
rng = Random(1234)

test_steps = []
# Initialization
store, anchor_block = get_genesis_forkchoice_store_and_block(spec, state)
yield 'anchor_state', state
yield 'anchor_block', anchor_block
current_time = state.slot * spec.config.SECONDS_PER_SLOT + store.genesis_time
on_tick_and_append_step(spec, store, current_time, test_steps)
assert store.time == current_time

# On receiving a block of `GENESIS_SLOT + 1` slot
block, blobs, blob_kzg_proofs = get_block_with_blob(spec, state, rng=rng)
signed_block = state_transition_and_sign_block(spec, state, block)
blob_data = BlobData(blobs, blob_kzg_proofs)

def run_func_1():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe run_tick_and_add_block_with_data_1 -- a little long... just a bit hesitant on run_func_1 generic name not explaining what's going on

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Or maybe we should hide with_blob_data usage with a helper -- tick_and_add_block_with_data -- that does the meta-programming so you don't have to explicitly do it each time. Seems like the exact same usage is used over and over in this file so I think helper to hide makes sense

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good suggestion 👍

yield from tick_and_add_block(spec, store, signed_block, test_steps, blob_data=blob_data)

yield from with_blob_data(spec, blob_data, run_func_1)

assert spec.get_head(store) == signed_block.message.hash_tree_root()

# On receiving a block of next epoch
store.time = current_time + spec.config.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't there be a on_tick_and_append_step to run tick processing in fork choice?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Without the on_tick_and_append_step, the followup checks in steps.yaml will fail as the time doesn't match with the test runner fork choice. test runner fork choice needs to be told manually to advance to the target time, via steps.yaml

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ahh nice catch! I think this line should be removed because tick_and_add_block_with_data would call on_tick properly.

block, blobs, blob_kzg_proofs = get_block_with_blob(spec, state, rng=rng)
signed_block = state_transition_and_sign_block(spec, state, block)
blob_data = BlobData(blobs, blob_kzg_proofs)

def run_func_2():
yield from tick_and_add_block(spec, store, signed_block, test_steps, blob_data=blob_data)

yield from with_blob_data(spec, blob_data, run_func_2)

assert spec.get_head(store) == signed_block.message.hash_tree_root()

yield 'steps', test_steps


@with_deneb_and_later
@spec_state_test
def test_invalid_incorrect_proof(spec, state):
rng = Random(1234)

test_steps = []
# Initialization
store, anchor_block = get_genesis_forkchoice_store_and_block(spec, state)
yield 'anchor_state', state
yield 'anchor_block', anchor_block
current_time = state.slot * spec.config.SECONDS_PER_SLOT + store.genesis_time
on_tick_and_append_step(spec, store, current_time, test_steps)
assert store.time == current_time

# On receiving a block of `GENESIS_SLOT + 1` slot
block, blobs, _ = get_block_with_blob(spec, state, rng=rng)
signed_block = state_transition_and_sign_block(spec, state, block)
# Insert incorrect proof
blob_kzg_proofs = [b'\xc0' + b'\x00' * 47]
blob_data = BlobData(blobs, blob_kzg_proofs)

def run_func_1():
yield from tick_and_add_block(spec, store, signed_block, test_steps, blob_data=blob_data, valid=False)

yield from with_blob_data(spec, blob_data, run_func_1)

assert spec.get_head(store) != signed_block.message.hash_tree_root()

yield 'steps', test_steps


@with_deneb_and_later
@spec_state_test
def test_invalid_data_unavailable(spec, state):
rng = Random(1234)

test_steps = []
# Initialization
store, anchor_block = get_genesis_forkchoice_store_and_block(spec, state)
yield 'anchor_state', state
yield 'anchor_block', anchor_block
current_time = state.slot * spec.config.SECONDS_PER_SLOT + store.genesis_time
on_tick_and_append_step(spec, store, current_time, test_steps)
assert store.time == current_time

# On receiving a block of `GENESIS_SLOT + 1` slot
block, _, _ = get_block_with_blob(spec, state, rng=rng)
signed_block = state_transition_and_sign_block(spec, state, block)

# data unavailable
blob_data = BlobData([], [])

def run_func_1():
yield from tick_and_add_block(spec, store, signed_block, test_steps, blob_data=blob_data, valid=False)

yield from with_blob_data(spec, blob_data, run_func_1)

assert spec.get_head(store) != signed_block.message.hash_tree_root()

yield 'steps', test_steps
87 changes: 76 additions & 11 deletions tests/core/pyspec/eth2spec/test/helpers/fork_choice.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import NamedTuple, Sequence, Any

from eth_utils import encode_hex
from eth2spec.test.exceptions import BlockNotFoundException
from eth2spec.test.helpers.attestations import (
Expand All @@ -7,6 +9,40 @@
)


class BlobData(NamedTuple):
"""
The return values of ``retrieve_blobs_and_proofs`` helper.
"""
blobs: Sequence[Any]
proofs: Sequence[bytes]


def with_blob_data(spec, blob_data, func):
"""
This helper runs the given ``func`` with monkeypatched ``retrieve_blobs_and_proofs``

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fancy.

that returns ``blob_data.blobs, blob_data.proofs``.
"""
def retrieve_blobs_and_proofs(beacon_block_root):
return blob_data.blobs, blob_data.proofs

retrieve_blobs_and_proofs_backup = spec.retrieve_blobs_and_proofs
spec.retrieve_blobs_and_proofs = retrieve_blobs_and_proofs

class AtomicBoolean():
value = False
is_called = AtomicBoolean()

def wrap(flag: AtomicBoolean):
yield from func()
flag.value = True

try:
yield from wrap(is_called)
finally:
spec.retrieve_blobs_and_proofs = retrieve_blobs_and_proofs_backup
assert is_called.value


def get_anchor_root(spec, state):
anchor_block_header = state.latest_block_header.copy()
if anchor_block_header.state_root == spec.Bytes32():
Expand All @@ -15,7 +51,8 @@ def get_anchor_root(spec, state):


def tick_and_add_block(spec, store, signed_block, test_steps, valid=True,
merge_block=False, block_not_found=False, is_optimistic=False):
merge_block=False, block_not_found=False, is_optimistic=False,
blob_data=None):
pre_state = store.block_states[signed_block.message.parent_root]
if merge_block:
assert spec.is_merge_transition_block(pre_state, signed_block.message.body)
Expand All @@ -30,6 +67,7 @@ def tick_and_add_block(spec, store, signed_block, test_steps, valid=True,
valid=valid,
block_not_found=block_not_found,
is_optimistic=is_optimistic,
blob_data=blob_data,
)

return post_state
Expand Down Expand Up @@ -94,6 +132,13 @@ def get_attester_slashing_file_name(attester_slashing):
return f"attester_slashing_{encode_hex(attester_slashing.hash_tree_root())}"


def get_blobs_file_name(blobs=None, blobs_root=None):
if blobs:
return f"blobs_{encode_hex(blobs.hash_tree_root())}"
else:
return f"blobs_{encode_hex(blobs_root)}"


def on_tick_and_append_step(spec, store, time, test_steps):
spec.on_tick(store, time)
test_steps.append({'tick': int(time)})
Expand All @@ -119,35 +164,55 @@ def add_block(spec,
test_steps,
valid=True,
block_not_found=False,
is_optimistic=False):
is_optimistic=False,
blob_data=None):
"""
Run on_block and on_attestation
"""
yield get_block_file_name(signed_block), signed_block

if not valid:
if is_optimistic:
run_on_block(spec, store, signed_block, valid=True)
# Check blob_data
if blob_data is not None:
blobs = spec.List[spec.Blob, spec.MAX_BLOBS_PER_BLOCK](blob_data.blobs)
if len(blobs) > 0:
blobs_root = blobs.hash_tree_root()
yield get_blobs_file_name(blobs_root=blobs_root), blobs
else:
blobs_root = None

is_blob_data_test = blob_data is not None

def _append_step(is_blob_data_test, valid=True):
if is_blob_data_test:
test_steps.append({
'block': get_block_file_name(signed_block),
'valid': False,
'blobs': None if blobs_root is None else get_blobs_file_name(blobs_root=blobs_root),
'proofs': [encode_hex(proof) for proof in blob_data.proofs],
'valid': valid,
})
else:
test_steps.append({
'block': get_block_file_name(signed_block),
'valid': valid,
})

if not valid:
if is_optimistic:
run_on_block(spec, store, signed_block, valid=True)
_append_step(is_blob_data_test, valid=False)
else:
try:
run_on_block(spec, store, signed_block, valid=True)
except (AssertionError, BlockNotFoundException) as e:
if isinstance(e, BlockNotFoundException) and not block_not_found:
assert False
test_steps.append({
'block': get_block_file_name(signed_block),
'valid': False,
})
_append_step(is_blob_data_test, valid=False)
return
else:
assert False
else:
run_on_block(spec, store, signed_block, valid=True)
test_steps.append({'block': get_block_file_name(signed_block)})
_append_step(is_blob_data_test)

# An on_block step implies receiving block's attestations
for attestation in signed_block.message.body.attestations:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@
)


rng = random.Random(1001)


@with_altair_and_later
@spec_state_test
def test_genesis(spec, state):
Expand Down Expand Up @@ -271,6 +268,7 @@ def test_proposer_boost_correct_head(spec, state):
next_slots(spec, state_2, 2)
block_2 = build_empty_block_for_next_slot(spec, state_2)
signed_block_2 = state_transition_and_sign_block(spec, state_2.copy(), block_2)
rng = random.Random(1001)
while spec.hash_tree_root(block_1) >= spec.hash_tree_root(block_2):
block_2.body.graffiti = spec.Bytes32(hex(rng.getrandbits(8 * 32))[2:].zfill(64))
signed_block_2 = state_transition_and_sign_block(spec, state_2.copy(), block_2)
Expand Down Expand Up @@ -339,6 +337,7 @@ def test_discard_equivocations_on_attester_slashing(spec, state):
next_slots(spec, state_2, 2)
block_2 = build_empty_block_for_next_slot(spec, state_2)
signed_block_2 = state_transition_and_sign_block(spec, state_2.copy(), block_2)
rng = random.Random(1001)
while spec.hash_tree_root(block_1) >= spec.hash_tree_root(block_2):
block_2.body.graffiti = spec.Bytes32(hex(rng.getrandbits(8 * 32))[2:].zfill(64))
signed_block_2 = state_transition_and_sign_block(spec, state_2.copy(), block_2)
Expand Down
39 changes: 35 additions & 4 deletions tests/formats/fork_choice/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,30 @@

The aim of the fork choice tests is to provide test coverage of the various components of the fork choice.

## Table of contents
<!-- TOC -->
<!-- START doctoc generated TOC please keep comment here to allow auto update -->
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->

- [Test case format](#test-case-format)
- [`meta.yaml`](#metayaml)
- [`anchor_state.ssz_snappy`](#anchor_statessz_snappy)
- [`anchor_block.ssz_snappy`](#anchor_blockssz_snappy)
- [`steps.yaml`](#stepsyaml)
- [`on_tick` execution step](#on_tick-execution-step)
- [`on_attestation` execution step](#on_attestation-execution-step)
- [`on_block` execution step](#on_block-execution-step)
- [`on_merge_block` execution step](#on_merge_block-execution-step)
- [`on_attester_slashing` execution step](#on_attester_slashing-execution-step)
- [`on_payload_info` execution step](#on_payload_info-execution-step)
- [Checks step](#checks-step)
- [`attestation_<32-byte-root>.ssz_snappy`](#attestation_32-byte-rootssz_snappy)
- [`block_<32-byte-root>.ssz_snappy`](#block_32-byte-rootssz_snappy)
- [Condition](#condition)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->
<!-- /TOC -->

## Test case format

### `meta.yaml`
Expand Down Expand Up @@ -59,14 +83,21 @@ The parameter that is required for executing `on_block(store, block)`.

```yaml
{
block: string -- the name of the `block_<32-byte-root>.ssz_snappy` file.
To execute `on_block(store, block)` with the given attestation.
valid: bool -- optional, default to `true`.
If it's `false`, this execution step is expected to be invalid.
block: string -- the name of the `block_<32-byte-root>.ssz_snappy` file.
To execute `on_block(store, block)` with the given attestation.
blobs: string or `null` -- optional, the name of the `blobs_<32-byte-root>.ssz_snappy` file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why null instead of just an empty list in the file? seems like that would reduce the exceptional cases here.

The blobs file content is a `List[Blob, MAX_BLOBS_PER_BLOCK]` SSZ object.
If it's `null`, `blobs` is an empty list.
proofs: array of byte48 hex string -- optional, the proofs of blob commitments.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if one of blobs or proofs exists but the other is not included? Is that an invalid test? or does one just consider the missing key as empty list?

I think it makes sense that either both keys should be there or both should be absent. And if so, maybe a note below about the relationship between the option keys

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And must these two lists have same length? or should they be able to handle if you have more of one than other?

If the latter, should add a test of mismatched lengths

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I changed it back to allow empty blobs SSZ file.

I think it makes sense that either both keys should be there or both should be absent.

Agreed. This is also the is_blob_data_test variable indicated in fork_choice.py::add_block.

And must these two lists have same length?

So the verify_blob_kzg_proof_batch API in is_data_available would check the length consistency and throw invalid.

If the latter, should add a test of mismatched lengths

👍 added these tests.

valid: bool -- optional, default to `true`.
If it's `false`, this execution step is expected to be invalid.
}
```

The file is located in the same folder (see below).

`blobs` and `proofs` are new fields from Deneb EIP-4844. These are the expected values from `retrieve_blobs_and_proofs()` helper inside `is_data_available()` helper.

After this step, the `store` object may have been updated.

#### `on_merge_block` execution step
Expand Down
Loading