Skip to content

Remove pre-capella check in BTEC gossip handler #5205

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

Closed
jtraglia wants to merge 1 commit into from
Closed

Remove pre-capella check in BTEC gossip handler #5205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 0 additions & 9 deletions specs/capella/p2p-interface.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ def validate_bls_to_execution_change_gossip(
seen: Seen,
state: BeaconState,
signed_bls_to_execution_change: SignedBLSToExecutionChange,
current_time_ms: uint64,
) -> None:
"""
Validate a SignedBLSToExecutionChange for gossip propagation.
Expand All @@ -217,14 +216,6 @@ def validate_bls_to_execution_change_gossip(
bls_to_execution_change = signed_bls_to_execution_change.message
validator_index = bls_to_execution_change.validator_index

# [IGNORE] The current epoch is at or after the Capella fork epoch
# (where current_epoch is defined by the current wall-clock time)
time_since_genesis_ms = current_time_ms - state.genesis_time * 1000
current_slot = Slot(time_since_genesis_ms // SLOT_DURATION_MS)
current_epoch = compute_epoch_at_slot(current_slot)
if current_epoch < CAPELLA_FORK_EPOCH:
raise GossipIgnore("current epoch is pre-capella")

# [IGNORE] This is the first valid bls_to_execution_change received for the validator
if validator_index in seen.bls_to_execution_change_indices:
raise GossipIgnore("already seen BLS to execution change for this validator")
Expand Down
92 changes: 16 additions & 76 deletions ...pyspec/eth_consensus_specs/test/capella/networking/test_gossip_bls_to_execution_change.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from eth_consensus_specs.test.context import (
always_bls,
spec_configured_state_test,
spec_state_test,
with_capella_and_later,
)
from eth_consensus_specs.test.helpers.bls_to_execution_changes import (
Expand All @@ -10,35 +10,23 @@
from eth_consensus_specs.test.helpers.keys import pubkeys


def run_validate_bls_to_execution_change_gossip(
spec, seen, state, signed_bls_to_execution_change, current_time_ms
):
def run_validate_bls_to_execution_change_gossip(spec, seen, state, signed_bls_to_execution_change):
"""
Run validate_bls_to_execution_change_gossip and return the result.
Returns: tuple of (result, reason) where result is "valid", "ignore", or "reject"
and reason is the exception message (or None for valid).
"""
try:
spec.validate_bls_to_execution_change_gossip(
seen, state, signed_bls_to_execution_change, current_time_ms
)
spec.validate_bls_to_execution_change_gossip(seen, state, signed_bls_to_execution_change)
return "valid", None
except spec.GossipIgnore as e:
return "ignore", str(e)
except spec.GossipReject as e:
return "reject", str(e)


def get_capella_fork_time_ms(spec, state):
"""
Return the current time in milliseconds at the Capella fork epoch.
"""
capella_slot = spec.compute_start_slot_at_epoch(spec.config.CAPELLA_FORK_EPOCH)
return spec.compute_time_at_slot_ms(state, capella_slot)


@with_capella_and_later
@spec_configured_state_test({"CAPELLA_FORK_EPOCH": 0})
@spec_state_test
def test_gossip_bls_to_execution_change__valid(spec, state):
"""
Test that a valid `bls_to_execution_change` passes gossip validation.
Expand All @@ -48,13 +36,11 @@ def test_gossip_bls_to_execution_change__valid(spec, state):

seen = get_seen(spec)
signed_bls_to_execution_change = get_signed_bls_to_execution_change(spec, state)
current_time_ms = get_capella_fork_time_ms(spec, state)

yield get_filename(signed_bls_to_execution_change), signed_bls_to_execution_change
yield "current_time_ms", "meta", int(current_time_ms)

result, reason = run_validate_bls_to_execution_change_gossip(
spec, seen, state, signed_bls_to_execution_change, current_time_ms
spec, seen, state, signed_bls_to_execution_change
)
assert result == "valid"
assert reason is None
Expand All @@ -73,43 +59,7 @@ def test_gossip_bls_to_execution_change__valid(spec, state):


@with_capella_and_later
@spec_configured_state_test({"CAPELLA_FORK_EPOCH": 1})
def test_gossip_bls_to_execution_change__ignore_pre_capella(spec, state):
"""
Test that a `bls_to_execution_change` before the Capella fork is ignored.
"""
yield "topic", "meta", "bls_to_execution_change"
yield "state", state

seen = get_seen(spec)
signed_bls_to_execution_change = get_signed_bls_to_execution_change(spec, state)
current_time_ms = spec.compute_time_at_slot_ms(state, spec.Slot(0))

yield get_filename(signed_bls_to_execution_change), signed_bls_to_execution_change
yield "current_time_ms", "meta", int(current_time_ms)

result, reason = run_validate_bls_to_execution_change_gossip(
spec, seen, state, signed_bls_to_execution_change, current_time_ms
)
assert result == "ignore"
assert reason == "current epoch is pre-capella"

yield (
"messages",
"meta",
[
{
"offset_ms": 0,
"message": get_filename(signed_bls_to_execution_change),
"expected": "ignore",
"reason": reason,
}
],
)


@with_capella_and_later
@spec_configured_state_test({"CAPELLA_FORK_EPOCH": 0})
@spec_state_test
def test_gossip_bls_to_execution_change__ignore_already_seen(spec, state):
"""
Test that a duplicate `bls_to_execution_change` is ignored.
Expand All @@ -120,13 +70,11 @@ def test_gossip_bls_to_execution_change__ignore_already_seen(spec, state):
messages = []
seen = get_seen(spec)
signed_bls_to_execution_change = get_signed_bls_to_execution_change(spec, state)
current_time_ms = get_capella_fork_time_ms(spec, state)

yield get_filename(signed_bls_to_execution_change), signed_bls_to_execution_change
yield "current_time_ms", "meta", int(current_time_ms)

result, reason = run_validate_bls_to_execution_change_gossip(
spec, seen, state, signed_bls_to_execution_change, current_time_ms
spec, seen, state, signed_bls_to_execution_change
)
assert result == "valid"
assert reason is None
Expand All @@ -139,7 +87,7 @@ def test_gossip_bls_to_execution_change__ignore_already_seen(spec, state):
)

result, reason = run_validate_bls_to_execution_change_gossip(
spec, seen, state, signed_bls_to_execution_change, current_time_ms
spec, seen, state, signed_bls_to_execution_change
)
assert result == "ignore"
assert reason == "already seen BLS to execution change for this validator"
Expand All @@ -156,7 +104,7 @@ def test_gossip_bls_to_execution_change__ignore_already_seen(spec, state):


@with_capella_and_later
@spec_configured_state_test({"CAPELLA_FORK_EPOCH": 0})
@spec_state_test
def test_gossip_bls_to_execution_change__reject_validator_index_out_of_range(spec, state):
"""
Test that a `bls_to_execution_change` with validator index out of range is rejected.
Expand All @@ -168,13 +116,11 @@ def test_gossip_bls_to_execution_change__reject_validator_index_out_of_range(spe
signed_bls_to_execution_change = get_signed_bls_to_execution_change(
spec, state, validator_index=len(state.validators)
)
current_time_ms = get_capella_fork_time_ms(spec, state)

yield get_filename(signed_bls_to_execution_change), signed_bls_to_execution_change
yield "current_time_ms", "meta", int(current_time_ms)

result, reason = run_validate_bls_to_execution_change_gossip(
spec, seen, state, signed_bls_to_execution_change, current_time_ms
spec, seen, state, signed_bls_to_execution_change
)
assert result == "reject"
assert reason == "validator index out of range"
Expand All @@ -194,7 +140,7 @@ def test_gossip_bls_to_execution_change__reject_validator_index_out_of_range(spe


@with_capella_and_later
@spec_configured_state_test({"CAPELLA_FORK_EPOCH": 0})
@spec_state_test
def test_gossip_bls_to_execution_change__reject_not_bls_credentials(spec, state):
"""
Test that a `bls_to_execution_change` for a validator without BLS credentials is rejected.
Expand All @@ -209,13 +155,11 @@ def test_gossip_bls_to_execution_change__reject_not_bls_credentials(spec, state)
signed_bls_to_execution_change = get_signed_bls_to_execution_change(
spec, state, validator_index=validator_index
)
current_time_ms = get_capella_fork_time_ms(spec, state)

yield get_filename(signed_bls_to_execution_change), signed_bls_to_execution_change
yield "current_time_ms", "meta", int(current_time_ms)

result, reason = run_validate_bls_to_execution_change_gossip(
spec, seen, state, signed_bls_to_execution_change, current_time_ms
spec, seen, state, signed_bls_to_execution_change
)
assert result == "reject"
assert reason == "validator does not have BLS withdrawal credentials"
Expand All @@ -235,7 +179,7 @@ def test_gossip_bls_to_execution_change__reject_not_bls_credentials(spec, state)


@with_capella_and_later
@spec_configured_state_test({"CAPELLA_FORK_EPOCH": 0})
@spec_state_test
def test_gossip_bls_to_execution_change__reject_pubkey_mismatch(spec, state):
"""
Test that a `bls_to_execution_change` with the wrong withdrawal pubkey is rejected.
Expand All @@ -251,13 +195,11 @@ def test_gossip_bls_to_execution_change__reject_pubkey_mismatch(spec, state):
validator_index=validator_index,
withdrawal_pubkey=pubkeys[0],
)
current_time_ms = get_capella_fork_time_ms(spec, state)

yield get_filename(signed_bls_to_execution_change), signed_bls_to_execution_change
yield "current_time_ms", "meta", int(current_time_ms)

result, reason = run_validate_bls_to_execution_change_gossip(
spec, seen, state, signed_bls_to_execution_change, current_time_ms
spec, seen, state, signed_bls_to_execution_change
)
assert result == "reject"
assert reason == "pubkey does not match validator withdrawal credentials"
Expand All @@ -277,7 +219,7 @@ def test_gossip_bls_to_execution_change__reject_pubkey_mismatch(spec, state):


@with_capella_and_later
@spec_configured_state_test({"CAPELLA_FORK_EPOCH": 0})
@spec_state_test
@always_bls
def test_gossip_bls_to_execution_change__reject_bad_signature(spec, state):
"""
Expand All @@ -289,13 +231,11 @@ def test_gossip_bls_to_execution_change__reject_bad_signature(spec, state):
seen = get_seen(spec)
signed_bls_to_execution_change = get_signed_bls_to_execution_change(spec, state)
signed_bls_to_execution_change.signature = spec.BLSSignature(b"\x42" * 96)
current_time_ms = get_capella_fork_time_ms(spec, state)

yield get_filename(signed_bls_to_execution_change), signed_bls_to_execution_change
yield "current_time_ms", "meta", int(current_time_ms)

result, reason = run_validate_bls_to_execution_change_gossip(
spec, seen, state, signed_bls_to_execution_change, current_time_ms
spec, seen, state, signed_bls_to_execution_change
)
assert result == "reject"
assert reason == "invalid BLS to execution change signature"
Expand Down