From 45a361581670f179234433d9fcbcddff354bcbbb Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 12 Sep 2023 15:19:26 +1000 Subject: [PATCH 01/49] Allow honest validators to reorg late blocks --- specs/bellatrix/fork-choice.md | 96 ++++++++++++++++++++++++++++++++++ specs/phase0/fork-choice.md | 92 ++++++++++++++++++++++++++++---- specs/phase0/validator.md | 21 +++++--- 3 files changed, 193 insertions(+), 16 deletions(-) diff --git a/specs/bellatrix/fork-choice.md b/specs/bellatrix/fork-choice.md index c8475195fc..fbfc03e14c 100644 --- a/specs/bellatrix/fork-choice.md +++ b/specs/bellatrix/fork-choice.md @@ -11,6 +11,7 @@ - [`ExecutionEngine`](#executionengine) - [`notify_forkchoice_updated`](#notify_forkchoice_updated) - [`safe_block_hash`](#safe_block_hash) + - [`should_override_forkchoice_update`](#should_override_forkchoice_update) - [Helpers](#helpers) - [`PayloadAttributes`](#payloadattributes) - [`PowBlock`](#powblock) @@ -76,6 +77,101 @@ As per EIP-3675, before a post-transition block is finalized, `notify_forkchoice The `safe_block_hash` parameter MUST be set to return value of [`get_safe_execution_payload_hash(store: Store)`](../../fork_choice/safe-block.md#get_safe_execution_payload_hash) function. +##### `should_override_forkchoice_update` + +If proposer boost re-orgs are implemented and enabled (see `get_proposer_head`) then additional care +must be taken to ensure that the proposer is able to build an execution payload. + +If a beacon node knows it will propose the next block then it SHOULD NOT call +`notify_forkchoice_updated` if it detects the current head to be weak and potentially capable of +being re-orged. Complete information for evaluating `get_proposer_head` _will not_ be available +immediately after the receipt of a new block, so an approximation of those conditions should be +used when deciding whether to send or suppress a fork choice notification. The exact conditions +used may be implementation-specific, a suggested implementation is below. + +Let `validator_is_connected` be a function that indicates whether the validator with +`validator_index` is connected to the node (e.g. has sent an unexpired proposer preparation +message). + +```python +def validator_is_connected(_validator_index: ValidatorIndex) -> boolean: + ... +``` + +```python +def should_override_forkchoice_update( + store: Store, + head_root: Root, +) -> boolean: + justified_state = store.checkpoint_states[store.justified_checkpoint] + head_block = store.blocks[head_root] + parent_root = head_block.parent_root + parent_block = store.blocks[parent_root] + current_slot = get_current_slot(store) + proposal_slot = head_block.slot + Slot(1) + + # Only re-org the head_block block if it arrived later than the attestation deadline. + head_late = store.block_timeliness.get(head_root) is False + + # Only suppress the fork choice update if we are confident that we will propose the next block. + parent_state_advanced = store.block_states[parent_root] + process_slots(parent_state_advanced, proposal_slot) + proposer_index = get_beacon_proposer_index(parent_state_advanced) + proposing_reorg_slot = validator_is_connected(proposer_index) + + # Do not re-org if the chain is not finalizing with acceptable frequency. + proposal_epoch = compute_epoch_at_slot(proposal_slot) + epochs_since_finalization = proposal_epoch - store.finalized_checkpoint.epoch + finalization_ok = epochs_since_finalization <= REORG_MAX_EPOCHS_SINCE_FINALIZATION + + # Single slot re-org. + parent_slot_ok = parent_block.slot + 1 == head_block.slot + time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT + current_time_ok = (head_block.slot == current_slot or + (proposal_slot == current_slot and + time_into_slot <= SECONDS_PER_SLOT // INTERVALS_PER_SLOT // 2)) + single_slot_reorg = parent_slot_ok and current_time_ok + + # Shuffling stable. + shuffling_stable = proposal_slot % SLOTS_PER_EPOCH != 0 + + # FFG information of the new head_block will be competitive with the current head. + ffg_competitive = (store.unrealized_justifications[parent_root] == + store.unrealized_justifications[head_root]) + + # Check the head weight only if the attestations from the head slot have already been applied. + # Implementations may want to do this in different ways, e.g. by advancing + # `store.time` early, or by counting queued attestations during the head block's slot. + if current_slot > head_block.slot: + head_weight = get_weight(store, head_root) + reorg_threshold = calculate_committee_fraction(justified_state, REORG_WEIGHT_THRESHOLD) + head_weak = head_weight < reorg_threshold + + parent_weight = get_weight(store, parent_root) + parent_threshold = calculate_committee_fraction(justified_state, REORG_PARENT_WEIGHT_THRESHOLD) + parent_strong = parent_weight > parent_threshold + else: + head_weak = True + parent_strong = True + + return all([head_late, proposing_reorg_slot, finalization_ok, single_slot_reorg, + shuffling_stable, ffg_competitive, head_weak, parent_strong]) +``` + +> Note that the ordering of conditions is a suggestion only. Implementations are free to +optimize by re-ordering the conditions from least to most expensive and by returning early if +any of the early conditions are `False`. + +In case `should_override_forkchoice_update` returns `True`, a node SHOULD instead call +`notify_forkchoice_updated` with parameters appropriate for building upon the parent block. Care +must be taken to compute the correct `payload_attributes`, as they may change depending on the slot +of the block to be proposed (due to withdrawals). + +If `should_override_forkchoice_update` returns `True` but `get_proposer_head` later chooses the +canonical head rather than its parent, then this is a misprediction that will cause the node +to construct a payload with less notice. The result of `get_proposer_head` MUST be honoured in +preference to the heuristic method. + ## Helpers ### `PayloadAttributes` diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 8b52186dda..50655bc485 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -18,12 +18,14 @@ - [`get_current_slot`](#get_current_slot) - [`compute_slots_since_epoch_start`](#compute_slots_since_epoch_start) - [`get_ancestor`](#get_ancestor) + - [`calculate_committee_fraction`](#calculate_committee_fraction) - [`get_checkpoint_block`](#get_checkpoint_block) - [`get_weight`](#get_weight) - [`get_voting_source`](#get_voting_source) - [`filter_block_tree`](#filter_block_tree) - [`get_filtered_block_tree`](#get_filtered_block_tree) - [`get_head`](#get_head) + - [`get_proposer_head`](#get_proposer_head) - [`update_checkpoints`](#update_checkpoints) - [`update_unrealized_checkpoints`](#update_unrealized_checkpoints) - [Pull-up tip helpers](#pull-up-tip-helpers) @@ -76,11 +78,16 @@ Any of the above handlers that trigger an unhandled exception (e.g. a failed ass ### Configuration -| Name | Value | -| ---------------------- | ------------ | -| `PROPOSER_SCORE_BOOST` | `uint64(40)` | +| Name | Value | +| ------------------------------------- | ------------ | +| `PROPOSER_SCORE_BOOST` | `uint64(40)` | +| `REORG_WEIGHT_THRESHOLD` | `uint64(20)` | +| `REORG_PARENT_WEIGHT_THRESHOLD` | `uint64(160)`| +| `REORG_MAX_EPOCHS_SINCE_FINALIZATION` | `Epoch(2)` | -- The proposer score boost is worth `PROPOSER_SCORE_BOOST` percentage of the committee's weight, i.e., for slot with committee weight `committee_weight` the boost weight is equal to `(committee_weight * PROPOSER_SCORE_BOOST) // 100`. +- The proposer score boost and re-org weight threshold are percentage + values that are measured with respect to the weight of a single committee. See + `calculate_committee_fraction`. ### Helpers @@ -115,6 +122,7 @@ class Store(object): equivocating_indices: Set[ValidatorIndex] blocks: Dict[Root, BeaconBlock] = field(default_factory=dict) block_states: Dict[Root, BeaconState] = field(default_factory=dict) + block_timeliness: Dict[Root, boolean] = field(default_factory=dict) checkpoint_states: Dict[Checkpoint, BeaconState] = field(default_factory=dict) latest_messages: Dict[ValidatorIndex, LatestMessage] = field(default_factory=dict) unrealized_justifications: Dict[Root, Checkpoint] = field(default_factory=dict) @@ -191,6 +199,14 @@ def get_ancestor(store: Store, root: Root, slot: Slot) -> Root: return root ``` +#### `calculate_committee_fraction` + +```python +def calculate_committee_fraction(state: BeaconState, committee_percent: uint64) -> Gwei: + committee_weight = get_total_active_balance(state) // SLOTS_PER_EPOCH + return Gwei((committee_weight * committee_percent) // 100) +``` + #### `get_checkpoint_block` ```python @@ -225,8 +241,7 @@ def get_weight(store: Store, root: Root) -> Gwei: proposer_score = Gwei(0) # Boost is applied if ``root`` is an ancestor of ``proposer_boost_root`` if get_ancestor(store, store.proposer_boost_root, store.blocks[root].slot) == root: - committee_weight = get_total_active_balance(state) // SLOTS_PER_EPOCH - proposer_score = (committee_weight * PROPOSER_SCORE_BOOST) // 100 + proposer_score = calculate_committee_fraction(state, PROPOSER_SCORE_BOOST) return attestation_score + proposer_score ``` @@ -247,7 +262,6 @@ def get_voting_source(store: Store, block_root: Root) -> Checkpoint: # The block is not from a prior epoch, therefore the voting source is not pulled up head_state = store.block_states[block_root] return head_state.current_justified_checkpoint - ``` #### `filter_block_tree` @@ -342,6 +356,62 @@ def get_head(store: Store) -> Root: head = max(children, key=lambda root: (get_weight(store, root), root)) ``` +#### `get_proposer_head` + +_Implementing `get_proposer_head` is optional_. + +```python +def get_proposer_head(store: Store, head_root: Root, slot: Slot) -> Root: + justified_state = store.checkpoint_states[store.justified_checkpoint] + head_block = store.blocks[head_root] + parent_root = head_block.parent_root + parent_block = store.blocks[parent_root] + + # Only re-org the head block if it arrived later than the attestation deadline. + head_late = store.block_timeliness.get(head_root) is False + + # Do not re-org if the chain is not finalizing with acceptable frequency. + epochs_since_finalization = compute_epoch_at_slot(slot) - store.finalized_checkpoint.epoch + finalization_ok = epochs_since_finalization <= REORG_MAX_EPOCHS_SINCE_FINALIZATION + + # Only re-org a single slot at most. + single_slot_reorg = parent_block.slot + 1 == head_block.slot and head_block.slot + 1 == slot + + # Only re-org if we are proposing on-time. + time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT + proposing_on_time = time_into_slot <= SECONDS_PER_SLOT // INTERVALS_PER_SLOT // 2 + + # Do not re-org on an epoch boundary where the proposer shuffling could change. + shuffling_stable = slot % SLOTS_PER_EPOCH != 0 + + # Ensure that the FFG information of the new head will be competitive with the current head. + ffg_competitive = (store.unrealized_justifications[parent_root] == + store.unrealized_justifications[head_root]) + + # Check that the head has few enough votes to be overpowered by our proposer boost. + assert store.proposer_boost_root != head_root # ensure boost has worn off + head_weight = get_weight(store, head_root) + reorg_threshold = calculate_committee_fraction(justified_state, REORG_WEIGHT_THRESHOLD) + head_weak = head_weight < reorg_threshold + + # Check that the missing votes are assigned to the parent and not being hoarded. + parent_weight = get_weight(store, parent_root) + parent_threshold = calculate_committee_fraction(justified_state, REORG_PARENT_WEIGHT_THRESHOLD) + parent_strong = parent_weight > parent_threshold + + if all([head_late, finalization_ok, single_slot_reorg, proposing_on_time, shuffling_stable, + ffg_competitive, head_weak, parent_strong]): + # We can re-org the current head by building upon its parent block. + return parent_root + else: + return head_root +``` + +> Note that the ordering of conditions is a suggestion only. Implementations are free to +optimize by re-ordering the conditions from least to most expensive and by returning early if +any of the early conditions are `False`. + + #### `update_checkpoints` ```python @@ -536,11 +606,15 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Add new state for this block to the store store.block_states[block_root] = state - # Add proposer score boost if the block is timely + # Add block timeliness to the store time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT is_before_attesting_interval = time_into_slot < SECONDS_PER_SLOT // INTERVALS_PER_SLOT + is_timely = get_current_slot(store) == block.slot and is_before_attesting_interval + store.block_timeliness[hash_tree_root(block)] = is_timely + + # Add proposer score boost if the block is timely and not conflicting with an existing block is_first_block = store.proposer_boost_root == Root() - if get_current_slot(store) == block.slot and is_before_attesting_interval and is_first_block: + if is_timely and is_first_block: store.proposer_boost_root = hash_tree_root(block) # Update checkpoints in store if necessary diff --git a/specs/phase0/validator.md b/specs/phase0/validator.md index 86b230654c..533cdeabcc 100644 --- a/specs/phase0/validator.md +++ b/specs/phase0/validator.md @@ -274,15 +274,22 @@ A validator has two primary responsibilities to the beacon chain: [proposing blo A validator is expected to propose a [`SignedBeaconBlock`](./beacon-chain.md#signedbeaconblock) at the beginning of any `slot` during which `is_proposer(state, validator_index)` returns `True`. -To propose, the validator selects the `BeaconBlock`, `parent` which: - -1. In their view of fork choice is the head of the chain at the start of - `slot`, after running `on_tick` and applying any queued attestations from `slot - 1`. -2. Is from a slot strictly less than the slot of the block about to be proposed, - i.e. `parent.slot < slot`. +To propose, the validator selects a `BeaconBlock`, `parent` using this process: + +1. Compute fork choice's view of the head at the start of `slot`, after running + `on_tick` and applying any queued attestations from `slot - 1`. + Set `head_root = get_head(store)`. +2. Compute the _proposer head_, which is the head upon which the proposer SHOULD build in order to + incentivise timely block propagation by other validators. + Set `parent_root = get_proposer_head(store, head_root, slot)`. + A proposer may set `parent_root == head_root` if proposer re-orgs are not implemented or have + been disabled. +3. Let `parent` be the block with `parent_root`. The validator creates, signs, and broadcasts a `block` that is a child of `parent` -that satisfies a valid [beacon chain state transition](./beacon-chain.md#beacon-chain-state-transition-function). +and satisfies a valid [beacon chain state transition](./beacon-chain.md#beacon-chain-state-transition-function). +Note that the parent's slot must be strictly less than the slot of the block about to be proposed, +i.e. `parent.slot < slot`. There is one proposer per slot, so if there are N active validators any individual validator will on average be assigned to propose once per N slots (e.g. at 312,500 validators = 10 million ETH, that's once per ~6 weeks). From 651db2f8585bff75f7cad8e0210ef9353c039335 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 19 Oct 2023 19:15:49 +0800 Subject: [PATCH 02/49] Refactoring --- pysetup/spec_builders/bellatrix.py | 7 +- specs/bellatrix/fork-choice.md | 60 ++++++-------- specs/phase0/fork-choice.md | 121 ++++++++++++++++++++++------- 3 files changed, 122 insertions(+), 66 deletions(-) diff --git a/pysetup/spec_builders/bellatrix.py b/pysetup/spec_builders/bellatrix.py index c5753d7df0..1f49a0029a 100644 --- a/pysetup/spec_builders/bellatrix.py +++ b/pysetup/spec_builders/bellatrix.py @@ -27,7 +27,12 @@ def get_execution_state(_execution_state_root: Bytes32) -> ExecutionState: def get_pow_chain_head() -> PowBlock: - pass""" + pass + + +def validator_is_connected(validator_index: ValidatorIndex) -> bool: + # pylint: disable=unused-argument + return True""" @classmethod def execution_engine_cls(cls) -> str: diff --git a/specs/bellatrix/fork-choice.md b/specs/bellatrix/fork-choice.md index fbfc03e14c..88f4e5c393 100644 --- a/specs/bellatrix/fork-choice.md +++ b/specs/bellatrix/fork-choice.md @@ -89,21 +89,12 @@ immediately after the receipt of a new block, so an approximation of those condi used when deciding whether to send or suppress a fork choice notification. The exact conditions used may be implementation-specific, a suggested implementation is below. -Let `validator_is_connected` be a function that indicates whether the validator with -`validator_index` is connected to the node (e.g. has sent an unexpired proposer preparation -message). +Let `validator_is_connected(validator_index: ValidatorIndex) -> bool` be a function that indicates +whether the validator with `validator_index` is connected to the node (e.g. has sent an unexpired +proposer preparation message). ```python -def validator_is_connected(_validator_index: ValidatorIndex) -> boolean: - ... -``` - -```python -def should_override_forkchoice_update( - store: Store, - head_root: Root, -) -> boolean: - justified_state = store.checkpoint_states[store.justified_checkpoint] +def should_override_forkchoice_update(store: Store, head_root: Root) -> bool: head_block = store.blocks[head_root] parent_root = head_block.parent_root parent_block = store.blocks[parent_root] @@ -111,7 +102,13 @@ def should_override_forkchoice_update( proposal_slot = head_block.slot + Slot(1) # Only re-org the head_block block if it arrived later than the attestation deadline. - head_late = store.block_timeliness.get(head_root) is False + head_late = is_head_late(store, head_root) + + # Shuffling stable. + shuffling_stable = is_shuffling_stable(proposal_slot) + + # FFG information of the new head_block will be competitive with the current head. + ffg_competitive = is_ffg_competitive(store, head_root, parent_root) # Only suppress the fork choice update if we are confident that we will propose the next block. parent_state_advanced = store.block_states[parent_root] @@ -120,45 +117,32 @@ def should_override_forkchoice_update( proposing_reorg_slot = validator_is_connected(proposer_index) # Do not re-org if the chain is not finalizing with acceptable frequency. - proposal_epoch = compute_epoch_at_slot(proposal_slot) - epochs_since_finalization = proposal_epoch - store.finalized_checkpoint.epoch - finalization_ok = epochs_since_finalization <= REORG_MAX_EPOCHS_SINCE_FINALIZATION + finalization_ok = is_finalization_ok(store, proposal_slot) # Single slot re-org. parent_slot_ok = parent_block.slot + 1 == head_block.slot - time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT - current_time_ok = (head_block.slot == current_slot or - (proposal_slot == current_slot and - time_into_slot <= SECONDS_PER_SLOT // INTERVALS_PER_SLOT // 2)) - single_slot_reorg = parent_slot_ok and current_time_ok - - # Shuffling stable. - shuffling_stable = proposal_slot % SLOTS_PER_EPOCH != 0 + proposing_on_time = is_proposing_on_time(store) - # FFG information of the new head_block will be competitive with the current head. - ffg_competitive = (store.unrealized_justifications[parent_root] == - store.unrealized_justifications[head_root]) + # Note that this condition is different from `get_proposer_head` + current_time_ok = (head_block.slot == current_slot + or (proposal_slot == current_slot and proposing_on_time)) + single_slot_reorg = parent_slot_ok and current_time_ok # Check the head weight only if the attestations from the head slot have already been applied. # Implementations may want to do this in different ways, e.g. by advancing # `store.time` early, or by counting queued attestations during the head block's slot. if current_slot > head_block.slot: - head_weight = get_weight(store, head_root) - reorg_threshold = calculate_committee_fraction(justified_state, REORG_WEIGHT_THRESHOLD) - head_weak = head_weight < reorg_threshold - - parent_weight = get_weight(store, parent_root) - parent_threshold = calculate_committee_fraction(justified_state, REORG_PARENT_WEIGHT_THRESHOLD) - parent_strong = parent_weight > parent_threshold + head_weak = is_head_weak(store, head_root) + parent_strong = is_parent_strong(store, parent_root) else: head_weak = True parent_strong = True - return all([head_late, proposing_reorg_slot, finalization_ok, single_slot_reorg, - shuffling_stable, ffg_competitive, head_weak, parent_strong]) + return all([head_late, shuffling_stable, proposing_reorg_slot, ffg_competitive, finalization_ok, + single_slot_reorg, head_weak, parent_strong]) ``` -> Note that the ordering of conditions is a suggestion only. Implementations are free to +*Note*: The ordering of conditions is a suggestion only. Implementations are free to optimize by re-ordering the conditions from least to most expensive and by returning early if any of the early conditions are `False`. diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 50655bc485..5dea585d1f 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -25,7 +25,16 @@ - [`filter_block_tree`](#filter_block_tree) - [`get_filtered_block_tree`](#get_filtered_block_tree) - [`get_head`](#get_head) - - [`get_proposer_head`](#get_proposer_head) + - [Proposer head and reorg helpers](#proposer-head-and-reorg-helpers) + - [`is_head_late`](#is_head_late) + - [`is_shuffling_stable`](#is_shuffling_stable) + - [`is_shuffling_stable`](#is_shuffling_stable-1) + - [`is_ffg_competitive`](#is_ffg_competitive) + - [`is_finalization_ok`](#is_finalization_ok) + - [`is_proposing_on_time`](#is_proposing_on_time) + - [`is_head_weak`](#is_head_weak) + - [`is_parent_strong`](#is_parent_strong) + - [`get_proposer_head`](#get_proposer_head) - [`update_checkpoints`](#update_checkpoints) - [`update_unrealized_checkpoints`](#update_unrealized_checkpoints) - [Pull-up tip helpers](#pull-up-tip-helpers) @@ -356,58 +365,116 @@ def get_head(store: Store) -> Root: head = max(children, key=lambda root: (get_weight(store, root), root)) ``` -#### `get_proposer_head` +#### Proposer head and reorg helpers -_Implementing `get_proposer_head` is optional_. +_Implementing these helpers is optional_. +##### `is_head_late` ```python -def get_proposer_head(store: Store, head_root: Root, slot: Slot) -> Root: +def is_head_late(store: Store, head_root: Root) -> bool: + return not store.block_timeliness.get(head_root) +``` + +##### `is_shuffling_stable` +```python +def is_shuffling_stable(slot: Slot) -> bool: + return slot % SLOTS_PER_EPOCH != 0 +``` + +##### `is_shuffling_stable` +```python +def is_shuffling_stable(slot: Slot) -> bool: + return slot % SLOTS_PER_EPOCH != 0 +``` + +##### `is_ffg_competitive` + +```python +def is_ffg_competitive(store: Store, head_root: Root, parent_root: Root) -> bool: + return (store.unrealized_justifications[head_root] == store.unrealized_justifications[parent_root]) +``` + +##### `is_finalization_ok` + +```python +def is_finalization_ok(store: Store, slot: Slot) -> bool: + epochs_since_finalization = compute_epoch_at_slot(slot) - store.finalized_checkpoint.epoch + return epochs_since_finalization <= REORG_MAX_EPOCHS_SINCE_FINALIZATION +``` + +##### `is_proposing_on_time` + +```python +def is_proposing_on_time(store: Store) -> bool: + time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT + proposer_reorg_cutoff = SECONDS_PER_SLOT // INTERVALS_PER_SLOT // 2 + return time_into_slot <= proposer_reorg_cutoff +``` + +##### `is_head_weak` + +```python +def is_head_weak(store: Store, head_root: Root) -> bool: justified_state = store.checkpoint_states[store.justified_checkpoint] + reorg_threshold = calculate_committee_fraction(justified_state, REORG_WEIGHT_THRESHOLD) + head_weight = get_weight(store, head_root) + return head_weight < reorg_threshold +``` + +##### `is_parent_strong` + +```python +def is_parent_strong(store: Store, parent_root: Root) -> bool: + justified_state = store.checkpoint_states[store.justified_checkpoint] + parent_threshold = calculate_committee_fraction(justified_state, REORG_PARENT_WEIGHT_THRESHOLD) + parent_weight = get_weight(store, parent_root) + return parent_weight > parent_threshold +``` + +##### `get_proposer_head` + +```python +def get_proposer_head(store: Store, head_root: Root, slot: Slot) -> Root: head_block = store.blocks[head_root] parent_root = head_block.parent_root parent_block = store.blocks[parent_root] # Only re-org the head block if it arrived later than the attestation deadline. - head_late = store.block_timeliness.get(head_root) is False + head_late = is_head_late(store, head_root) - # Do not re-org if the chain is not finalizing with acceptable frequency. - epochs_since_finalization = compute_epoch_at_slot(slot) - store.finalized_checkpoint.epoch - finalization_ok = epochs_since_finalization <= REORG_MAX_EPOCHS_SINCE_FINALIZATION + # Do not re-org on an epoch boundary where the proposer shuffling could change. + shuffling_stable = is_shuffling_stable(slot) - # Only re-org a single slot at most. - single_slot_reorg = parent_block.slot + 1 == head_block.slot and head_block.slot + 1 == slot + # Ensure that the FFG information of the new head will be competitive with the current head. + ffg_competitive = is_ffg_competitive(store, head_root, parent_root) - # Only re-org if we are proposing on-time. - time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT - proposing_on_time = time_into_slot <= SECONDS_PER_SLOT // INTERVALS_PER_SLOT // 2 + # Do not re-org if the chain is not finalizing with acceptable frequency. + finalization_ok = is_finalization_ok(store, slot) - # Do not re-org on an epoch boundary where the proposer shuffling could change. - shuffling_stable = slot % SLOTS_PER_EPOCH != 0 + # Only re-org if we are proposing on-time. + proposing_on_time = is_proposing_on_time(store) - # Ensure that the FFG information of the new head will be competitive with the current head. - ffg_competitive = (store.unrealized_justifications[parent_root] == - store.unrealized_justifications[head_root]) + # Only re-org a single slot at most. + parent_slot_ok = parent_block.slot + 1 == head_block.slot + current_time_ok = head_block.slot + 1 == slot + single_slot_reorg = parent_slot_ok and current_time_ok # Check that the head has few enough votes to be overpowered by our proposer boost. assert store.proposer_boost_root != head_root # ensure boost has worn off - head_weight = get_weight(store, head_root) - reorg_threshold = calculate_committee_fraction(justified_state, REORG_WEIGHT_THRESHOLD) - head_weak = head_weight < reorg_threshold + head_weak = is_head_weak(store, head_root) # Check that the missing votes are assigned to the parent and not being hoarded. - parent_weight = get_weight(store, parent_root) - parent_threshold = calculate_committee_fraction(justified_state, REORG_PARENT_WEIGHT_THRESHOLD) - parent_strong = parent_weight > parent_threshold + parent_strong = is_parent_strong(store, parent_root) - if all([head_late, finalization_ok, single_slot_reorg, proposing_on_time, shuffling_stable, - ffg_competitive, head_weak, parent_strong]): + if all([head_late, shuffling_stable, ffg_competitive, finalization_ok, + proposing_on_time, single_slot_reorg, head_weak, parent_strong]): # We can re-org the current head by building upon its parent block. return parent_root else: return head_root ``` -> Note that the ordering of conditions is a suggestion only. Implementations are free to +*Note*: The ordering of conditions is a suggestion only. Implementations are free to optimize by re-ordering the conditions from least to most expensive and by returning early if any of the early conditions are `False`. From 991d6d5609c6b4b429524a6e809218214bd021af Mon Sep 17 00:00:00 2001 From: danny Date: Thu, 19 Oct 2023 20:58:20 -0600 Subject: [PATCH 03/49] add note about not invalidating valid and available blocks --- specs/deneb/fork-choice.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specs/deneb/fork-choice.md b/specs/deneb/fork-choice.md index 5a700cc7a6..e0b0c0fef9 100644 --- a/specs/deneb/fork-choice.md +++ b/specs/deneb/fork-choice.md @@ -47,6 +47,8 @@ Initially, verification requires every verifying actor to retrieve all matching The block MUST NOT be considered valid until all valid `Blob`s have been downloaded. Blocks that have been previously validated as available SHOULD be considered available even if the associated `Blob`s have subsequently been pruned. +*Note*: Extraneous or invalid Blobs received on the p2p network MUST NOT invalidate a block that is otherwise valid and available. + ```python def is_data_available(beacon_block_root: Root, blob_kzg_commitments: Sequence[KZGCommitment]) -> bool: # `retrieve_blobs_and_proofs` is implementation and context dependent From 0f61819e5cb1ad7348e152b4392e13b6079fe9cb Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 20 Oct 2023 14:41:51 +0800 Subject: [PATCH 04/49] Update other specs accordingly and re-order "Proposer head and reorg helpers" section --- specs/bellatrix/fork-choice.md | 8 +++- specs/capella/fork-choice.md | 8 +++- specs/deneb/fork-choice.md | 8 +++- specs/phase0/fork-choice.md | 80 +++++++++++++++------------------- 4 files changed, 53 insertions(+), 51 deletions(-) diff --git a/specs/bellatrix/fork-choice.md b/specs/bellatrix/fork-choice.md index 88f4e5c393..fd70cfff4d 100644 --- a/specs/bellatrix/fork-choice.md +++ b/specs/bellatrix/fork-choice.md @@ -271,11 +271,15 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Add new state for this block to the store store.block_states[block_root] = state - # Add proposer score boost if the block is timely + # Add block timeliness to the store time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT is_before_attesting_interval = time_into_slot < SECONDS_PER_SLOT // INTERVALS_PER_SLOT + is_timely = get_current_slot(store) == block.slot and is_before_attesting_interval + store.block_timeliness[hash_tree_root(block)] = is_timely + + # Add proposer score boost if the block is timely and not conflicting with an existing block is_first_block = store.proposer_boost_root == Root() - if get_current_slot(store) == block.slot and is_before_attesting_interval and is_first_block: + if is_timely and is_first_block: store.proposer_boost_root = hash_tree_root(block) # Update checkpoints in store if necessary diff --git a/specs/capella/fork-choice.md b/specs/capella/fork-choice.md index a830080c11..ded9782ccf 100644 --- a/specs/capella/fork-choice.md +++ b/specs/capella/fork-choice.md @@ -103,11 +103,15 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Add new state for this block to the store store.block_states[block_root] = state - # Add proposer score boost if the block is timely + # Add block timeliness to the store time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT is_before_attesting_interval = time_into_slot < SECONDS_PER_SLOT // INTERVALS_PER_SLOT + is_timely = get_current_slot(store) == block.slot and is_before_attesting_interval + store.block_timeliness[hash_tree_root(block)] = is_timely + + # Add proposer score boost if the block is timely and not conflicting with an existing block is_first_block = store.proposer_boost_root == Root() - if get_current_slot(store) == block.slot and is_before_attesting_interval and is_first_block: + if is_timely and is_first_block: store.proposer_boost_root = hash_tree_root(block) # Update checkpoints in store if necessary diff --git a/specs/deneb/fork-choice.md b/specs/deneb/fork-choice.md index 5a700cc7a6..03b3bf8fe3 100644 --- a/specs/deneb/fork-choice.md +++ b/specs/deneb/fork-choice.md @@ -103,11 +103,15 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Add new state for this block to the store store.block_states[block_root] = state - # Add proposer score boost if the block is timely + # Add block timeliness to the store time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT is_before_attesting_interval = time_into_slot < SECONDS_PER_SLOT // INTERVALS_PER_SLOT + is_timely = get_current_slot(store) == block.slot and is_before_attesting_interval + store.block_timeliness[hash_tree_root(block)] = is_timely + + # Add proposer score boost if the block is timely and not conflicting with an existing block is_first_block = store.proposer_boost_root == Root() - if get_current_slot(store) == block.slot and is_before_attesting_interval and is_first_block: + if is_timely and is_first_block: store.proposer_boost_root = hash_tree_root(block) # Update checkpoints in store if necessary diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 5dea585d1f..077209d5fc 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -25,18 +25,17 @@ - [`filter_block_tree`](#filter_block_tree) - [`get_filtered_block_tree`](#get_filtered_block_tree) - [`get_head`](#get_head) + - [`update_checkpoints`](#update_checkpoints) + - [`update_unrealized_checkpoints`](#update_unrealized_checkpoints) - [Proposer head and reorg helpers](#proposer-head-and-reorg-helpers) - [`is_head_late`](#is_head_late) - [`is_shuffling_stable`](#is_shuffling_stable) - - [`is_shuffling_stable`](#is_shuffling_stable-1) - [`is_ffg_competitive`](#is_ffg_competitive) - [`is_finalization_ok`](#is_finalization_ok) - [`is_proposing_on_time`](#is_proposing_on_time) - [`is_head_weak`](#is_head_weak) - [`is_parent_strong`](#is_parent_strong) - [`get_proposer_head`](#get_proposer_head) - - [`update_checkpoints`](#update_checkpoints) - - [`update_unrealized_checkpoints`](#update_unrealized_checkpoints) - [Pull-up tip helpers](#pull-up-tip-helpers) - [`compute_pulled_up_tip`](#compute_pulled_up_tip) - [`on_tick` helpers](#on_tick-helpers) @@ -365,6 +364,38 @@ def get_head(store: Store) -> Root: head = max(children, key=lambda root: (get_weight(store, root), root)) ``` +#### `update_checkpoints` + +```python +def update_checkpoints(store: Store, justified_checkpoint: Checkpoint, finalized_checkpoint: Checkpoint) -> None: + """ + Update checkpoints in store if necessary + """ + # Update justified checkpoint + if justified_checkpoint.epoch > store.justified_checkpoint.epoch: + store.justified_checkpoint = justified_checkpoint + + # Update finalized checkpoint + if finalized_checkpoint.epoch > store.finalized_checkpoint.epoch: + store.finalized_checkpoint = finalized_checkpoint +``` + +#### `update_unrealized_checkpoints` + +```python +def update_unrealized_checkpoints(store: Store, unrealized_justified_checkpoint: Checkpoint, + unrealized_finalized_checkpoint: Checkpoint) -> None: + """ + Update unrealized checkpoints in store if necessary + """ + # Update unrealized justified checkpoint + if unrealized_justified_checkpoint.epoch > store.unrealized_justified_checkpoint.epoch: + store.unrealized_justified_checkpoint = unrealized_justified_checkpoint + + # Update unrealized finalized checkpoint + if unrealized_finalized_checkpoint.epoch > store.unrealized_finalized_checkpoint.epoch: + store.unrealized_finalized_checkpoint = unrealized_finalized_checkpoint +``` #### Proposer head and reorg helpers _Implementing these helpers is optional_. @@ -372,13 +403,7 @@ _Implementing these helpers is optional_. ##### `is_head_late` ```python def is_head_late(store: Store, head_root: Root) -> bool: - return not store.block_timeliness.get(head_root) -``` - -##### `is_shuffling_stable` -```python -def is_shuffling_stable(slot: Slot) -> bool: - return slot % SLOTS_PER_EPOCH != 0 + return not store.block_timeliness[head_root] ``` ##### `is_shuffling_stable` @@ -478,41 +503,6 @@ def get_proposer_head(store: Store, head_root: Root, slot: Slot) -> Root: optimize by re-ordering the conditions from least to most expensive and by returning early if any of the early conditions are `False`. - -#### `update_checkpoints` - -```python -def update_checkpoints(store: Store, justified_checkpoint: Checkpoint, finalized_checkpoint: Checkpoint) -> None: - """ - Update checkpoints in store if necessary - """ - # Update justified checkpoint - if justified_checkpoint.epoch > store.justified_checkpoint.epoch: - store.justified_checkpoint = justified_checkpoint - - # Update finalized checkpoint - if finalized_checkpoint.epoch > store.finalized_checkpoint.epoch: - store.finalized_checkpoint = finalized_checkpoint -``` - -#### `update_unrealized_checkpoints` - -```python -def update_unrealized_checkpoints(store: Store, unrealized_justified_checkpoint: Checkpoint, - unrealized_finalized_checkpoint: Checkpoint) -> None: - """ - Update unrealized checkpoints in store if necessary - """ - # Update unrealized justified checkpoint - if unrealized_justified_checkpoint.epoch > store.unrealized_justified_checkpoint.epoch: - store.unrealized_justified_checkpoint = unrealized_justified_checkpoint - - # Update unrealized finalized checkpoint - if unrealized_finalized_checkpoint.epoch > store.unrealized_finalized_checkpoint.epoch: - store.unrealized_finalized_checkpoint = unrealized_finalized_checkpoint -``` - - #### Pull-up tip helpers ##### `compute_pulled_up_tip` From 16498c60aab06fabeb9d86175e79ee4aa12540af Mon Sep 17 00:00:00 2001 From: danny Date: Fri, 20 Oct 2023 12:06:47 -0600 Subject: [PATCH 05/49] Update fork-choice.md --- specs/deneb/fork-choice.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/specs/deneb/fork-choice.md b/specs/deneb/fork-choice.md index e0b0c0fef9..501ce95bbe 100644 --- a/specs/deneb/fork-choice.md +++ b/specs/deneb/fork-choice.md @@ -47,7 +47,7 @@ Initially, verification requires every verifying actor to retrieve all matching The block MUST NOT be considered valid until all valid `Blob`s have been downloaded. Blocks that have been previously validated as available SHOULD be considered available even if the associated `Blob`s have subsequently been pruned. -*Note*: Extraneous or invalid Blobs received on the p2p network MUST NOT invalidate a block that is otherwise valid and available. +*Note*: Extraneous or invalid Blobs (in addition to KZG expected/referenced valid blobs) received on the p2p network MUST NOT invalidate a block that is otherwise valid and available. ```python def is_data_available(beacon_block_root: Root, blob_kzg_commitments: Sequence[KZGCommitment]) -> bool: @@ -95,6 +95,9 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # If not, this block MAY be queued and subsequently considered when blob data becomes available assert is_data_available(hash_tree_root(block), block.body.blob_kzg_commitments) + # *Note*: Extraneous or invalid Blobs (in addition to the expected/referenced valid blobs) + # received on the p2p network MUST NOT invalidate a block that is otherwise valid and available + # Check the block is valid and compute the post-state state = pre_state.copy() block_root = hash_tree_root(block) From ce387d65c9b0b9ac73afabd1f2e00a19e2210e33 Mon Sep 17 00:00:00 2001 From: danny Date: Mon, 23 Oct 2023 09:15:06 -0600 Subject: [PATCH 06/49] shift wehre warning is in comments --- specs/deneb/fork-choice.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/specs/deneb/fork-choice.md b/specs/deneb/fork-choice.md index 501ce95bbe..10cf2bacd2 100644 --- a/specs/deneb/fork-choice.md +++ b/specs/deneb/fork-choice.md @@ -93,10 +93,9 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # [New in Deneb:EIP4844] # Check if blob data is available # If not, this block MAY be queued and subsequently considered when blob data becomes available - assert is_data_available(hash_tree_root(block), block.body.blob_kzg_commitments) - # *Note*: Extraneous or invalid Blobs (in addition to the expected/referenced valid blobs) # received on the p2p network MUST NOT invalidate a block that is otherwise valid and available + assert is_data_available(hash_tree_root(block), block.body.blob_kzg_commitments) # Check the block is valid and compute the post-state state = pre_state.copy() From 24b4d469032c73b073d689cfe49a9afcbb7f5cd4 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 23 Oct 2023 16:49:52 +0800 Subject: [PATCH 07/49] Add `get_proposer_head` and `should_override_forkchoice_update` tests --- .../test/bellatrix/fork_choice/test_reorg.py | 162 ++++++++++++++++++ .../eth2spec/test/helpers/attestations.py | 24 +-- .../eth2spec/test/helpers/fork_choice.py | 13 +- .../fork_choice/test_get_proposer_head.py | 154 +++++++++++++++++ 4 files changed, 334 insertions(+), 19 deletions(-) create mode 100644 tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_reorg.py create mode 100644 tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_proposer_head.py diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_reorg.py b/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_reorg.py new file mode 100644 index 0000000000..cb7adac1c2 --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_reorg.py @@ -0,0 +1,162 @@ +from eth2spec.test.context import ( + spec_state_test, + with_bellatrix_and_later, + with_presets, +) +from eth2spec.test.helpers.constants import ( + MINIMAL, +) +from eth2spec.test.helpers.attestations import ( + get_valid_attestation_at_slot, +) +from eth2spec.test.helpers.block import ( + build_empty_block_for_next_slot, +) +from eth2spec.test.helpers.fork_choice import ( + apply_next_epoch_with_attestations, + apply_next_slots_with_attestations, + get_genesis_forkchoice_store_and_block, + on_tick_and_append_step, + tick_and_add_block, + tick_and_run_on_attestation, +) +from eth2spec.test.helpers.state import ( + state_transition_and_sign_block, + next_epoch, + next_slot, +) + + +@with_bellatrix_and_later +@spec_state_test +@with_presets([MINIMAL], reason="too slow") +def test_should_override_forkchoice_update__false(spec, state): + 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 = build_empty_block_for_next_slot(spec, state) + signed_block = state_transition_and_sign_block(spec, state, block) + yield from tick_and_add_block(spec, store, signed_block, test_steps) + assert spec.get_head(store) == signed_block.message.hash_tree_root() + + # Proposer of next slot + head_root = spec.get_head(store) + + # Next slot + next_slot(spec, state) + slot = state.slot + + current_time = slot * spec.config.SECONDS_PER_SLOT + store.genesis_time + on_tick_and_append_step(spec, store, current_time, test_steps) + + assert not spec.should_override_forkchoice_update(store, head_root) + + yield 'steps', test_steps + + +@with_bellatrix_and_later +@spec_state_test +def test_should_override_forkchoice_update__true(spec, state): + 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 + + next_epoch(spec, state) + on_tick_and_append_step(spec, store, store.genesis_time + state.slot * spec.config.SECONDS_PER_SLOT, test_steps) + + # Fill epoch 1 to 3 + for _ in range(3): + state, store, _ = yield from apply_next_epoch_with_attestations( + spec, state, store, True, True, test_steps=test_steps) + + assert spec.compute_epoch_at_slot(spec.get_current_slot(store)) == 4 + assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 3 + assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2 + + # Make an empty block + block = build_empty_block_for_next_slot(spec, state) + signed_block = state_transition_and_sign_block(spec, state, block) + yield from tick_and_add_block(spec, store, signed_block, test_steps) + + # Fill a slot (parent) + state, store, signed_parent_block = yield from apply_next_slots_with_attestations( + spec, state, store, 1, True, True, test_steps) + + # Fill a slot with attestations to its parent + block = build_empty_block_for_next_slot(spec, state) + parent_block_slot = block.slot - 1 + block.body.attestations = get_valid_attestation_at_slot( + state, + spec, + parent_block_slot, + ) + signed_block = state_transition_and_sign_block(spec, state, block) + + # Make the head block late + attesting_cutoff = spec.config.SECONDS_PER_SLOT // spec.INTERVALS_PER_SLOT + current_time = state.slot * spec.config.SECONDS_PER_SLOT + store.genesis_time + attesting_cutoff + on_tick_and_append_step(spec, store, current_time, test_steps) + assert store.time == current_time + + yield from tick_and_add_block(spec, store, signed_block, test_steps) + assert spec.get_current_slot(store) == block.slot + + # Check conditions + head_root = spec.get_head(store) + head_block = store.blocks[head_root] + parent_root = head_block.parent_root + assert parent_root == signed_parent_block.message.hash_tree_root() + parent_block = store.blocks[parent_root] + + # Add attestations to the parent block + temp_state = state.copy() + next_slot(spec, temp_state) + current_time = state.slot * spec.config.SECONDS_PER_SLOT + store.genesis_time + 1 + on_tick_and_append_step(spec, store, current_time, test_steps) + attestations = get_valid_attestation_at_slot( + temp_state, + spec, + slot_to_attest=temp_state.slot - 1, + beacon_block_root=parent_root, + ) + current_slot = spec.get_current_slot(store) + for attestation in attestations: + yield from tick_and_run_on_attestation(spec, store, attestation, test_steps) + + current_slot = spec.get_current_slot(store) + proposal_slot = head_block.slot + 1 + + # The conditions in `get_proposer_head` + assert spec.is_head_late(store, head_root) + assert spec.is_shuffling_stable(proposal_slot) + assert spec.is_ffg_competitive(store, head_root, parent_root) + assert spec.is_finalization_ok(store, proposal_slot) + + # TODO: proposing_reorg_slot + + # Single slot re-org. + parent_slot_ok = parent_block.slot + 1 == head_block.slot + proposing_on_time = spec.is_proposing_on_time(store) + assert proposing_on_time + assert parent_slot_ok and proposal_slot == current_slot and proposing_on_time + + assert spec.is_head_weak(store, head_root) + assert spec.is_parent_strong(store, parent_root) + + assert spec.should_override_forkchoice_update(store, head_root) + + # TODO: export the `should_override_forkchoice_update` result to test vectors? + + yield 'steps', test_steps diff --git a/tests/core/pyspec/eth2spec/test/helpers/attestations.py b/tests/core/pyspec/eth2spec/test/helpers/attestations.py index 4899e62243..6cd35c5380 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/attestations.py +++ b/tests/core/pyspec/eth2spec/test/helpers/attestations.py @@ -51,19 +51,21 @@ def run_attestation_processing(spec, state, attestation, valid=True): yield 'post', state -def build_attestation_data(spec, state, slot, index, shard=None): +def build_attestation_data(spec, state, slot, index, beacon_block_root=None, shard=None): assert state.slot >= slot - if slot == state.slot: - block_root = build_empty_block_for_next_slot(spec, state).parent_root + if beacon_block_root is not None: + pass + elif slot == state.slot: + beacon_block_root = build_empty_block_for_next_slot(spec, state).parent_root else: - block_root = spec.get_block_root_at_slot(state, slot) + beacon_block_root = spec.get_block_root_at_slot(state, slot) current_epoch_start_slot = spec.compute_start_slot_at_epoch(spec.get_current_epoch(state)) if slot < current_epoch_start_slot: epoch_boundary_root = spec.get_block_root(state, spec.get_previous_epoch(state)) elif slot == current_epoch_start_slot: - epoch_boundary_root = block_root + epoch_boundary_root = beacon_block_root else: epoch_boundary_root = spec.get_block_root(state, spec.get_current_epoch(state)) @@ -77,7 +79,7 @@ def build_attestation_data(spec, state, slot, index, shard=None): data = spec.AttestationData( slot=slot, index=index, - beacon_block_root=block_root, + beacon_block_root=beacon_block_root, source=spec.Checkpoint(epoch=source_epoch, root=source_root), target=spec.Checkpoint(epoch=spec.compute_epoch_at_slot(slot), root=epoch_boundary_root), ) @@ -91,6 +93,7 @@ def get_valid_attestation(spec, slot=None, index=None, filter_participant_set=None, + beacon_block_root=None, signed=False): # If filter_participant_set filters everything, the attestation has 0 participants, and cannot be signed. # Thus strictly speaking invalid when no participant is added later. @@ -99,9 +102,7 @@ def get_valid_attestation(spec, if index is None: index = 0 - attestation_data = build_attestation_data( - spec, state, slot=slot, index=index - ) + attestation_data = build_attestation_data(spec, state, slot=slot, index=index, beacon_block_root=beacon_block_root) beacon_committee = spec.get_beacon_committee( state, @@ -195,7 +196,7 @@ def add_attestations_to_state(spec, state, attestations, slot): spec.process_attestation(state, attestation) -def get_valid_attestation_at_slot(state, spec, slot_to_attest, participation_fn=None): +def get_valid_attestation_at_slot(state, spec, slot_to_attest, participation_fn=None, beacon_block_root=None): committees_per_slot = spec.get_committee_count_per_slot(state, spec.compute_epoch_at_slot(slot_to_attest)) for index in range(committees_per_slot): def participants_filter(comm): @@ -210,7 +211,8 @@ def participants_filter(comm): slot_to_attest, index=index, signed=True, - filter_participant_set=participants_filter + filter_participant_set=participants_filter, + beacon_block_root=beacon_block_root, ) diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py index e0e3547222..ef80a3ab6f 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py @@ -92,14 +92,11 @@ def add_attestations(spec, store, attestations, test_steps, is_from_block=False) def tick_and_run_on_attestation(spec, store, attestation, test_steps, is_from_block=False): - parent_block = store.blocks[attestation.data.beacon_block_root] - pre_state = store.block_states[spec.hash_tree_root(parent_block)] - block_time = pre_state.genesis_time + parent_block.slot * spec.config.SECONDS_PER_SLOT - next_epoch_time = block_time + spec.SLOTS_PER_EPOCH * spec.config.SECONDS_PER_SLOT - - if store.time < next_epoch_time: - spec.on_tick(store, next_epoch_time) - test_steps.append({'tick': int(next_epoch_time)}) + # Make get_current_slot(store) >= attestation.data.slot + 1 + min_time_to_include = (attestation.data.slot + 1) * spec.config.SECONDS_PER_SLOT + if store.time < min_time_to_include: + spec.on_tick(store, min_time_to_include) + test_steps.append({'tick': int(min_time_to_include)}) yield from add_attestation(spec, store, attestation, test_steps, is_from_block) diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_proposer_head.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_proposer_head.py new file mode 100644 index 0000000000..083b292c0f --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_proposer_head.py @@ -0,0 +1,154 @@ +from eth2spec.test.context import ( + spec_state_test, + with_altair_and_later, +) +from eth2spec.test.helpers.attestations import ( + get_valid_attestation_at_slot, +) +from eth2spec.test.helpers.block import ( + build_empty_block_for_next_slot, +) +from eth2spec.test.helpers.fork_choice import ( + apply_next_epoch_with_attestations, + apply_next_slots_with_attestations, + get_genesis_forkchoice_store_and_block, + on_tick_and_append_step, + tick_and_add_block, + tick_and_run_on_attestation, +) +from eth2spec.test.helpers.state import ( + next_epoch, + next_slot, + state_transition_and_sign_block, +) + + +@with_altair_and_later +@spec_state_test +def test_basic_is_head_root(spec, state): + 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 = build_empty_block_for_next_slot(spec, state) + signed_block = state_transition_and_sign_block(spec, state, block) + yield from tick_and_add_block(spec, store, signed_block, test_steps) + assert spec.get_head(store) == signed_block.message.hash_tree_root() + + # Proposer of next slot + head_root = spec.get_head(store) + + # Proposing next slot + next_slot(spec, state) + slot = state.slot + + current_time = slot * spec.config.SECONDS_PER_SLOT + store.genesis_time + on_tick_and_append_step(spec, store, current_time, test_steps) + proposer_head = spec.get_proposer_head(store, head_root, slot) + + assert proposer_head == head_root + + yield 'steps', test_steps + + +@with_altair_and_later +@spec_state_test +def test_basic_is_parent_root(spec, state): + 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 + + next_epoch(spec, state) + on_tick_and_append_step(spec, store, store.genesis_time + state.slot * spec.config.SECONDS_PER_SLOT, test_steps) + + # Fill epoch 1 to 3 + for _ in range(3): + state, store, _ = yield from apply_next_epoch_with_attestations( + spec, state, store, True, True, test_steps=test_steps) + + assert spec.compute_epoch_at_slot(spec.get_current_slot(store)) == 4 + assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 3 + assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2 + + # Make an empty block + block = build_empty_block_for_next_slot(spec, state) + signed_block = state_transition_and_sign_block(spec, state, block) + yield from tick_and_add_block(spec, store, signed_block, test_steps) + + # Fill a slot (parent) + state, store, signed_parent_block = yield from apply_next_slots_with_attestations( + spec, state, store, 1, True, True, test_steps) + + # Fill a slot with attestations to its parent + block = build_empty_block_for_next_slot(spec, state) + parent_block_slot = block.slot - 1 + block.body.attestations = get_valid_attestation_at_slot( + state, + spec, + parent_block_slot, + ) + signed_block = state_transition_and_sign_block(spec, state, block) + + # Make the head block late + attesting_cutoff = spec.config.SECONDS_PER_SLOT // spec.INTERVALS_PER_SLOT + current_time = state.slot * spec.config.SECONDS_PER_SLOT + store.genesis_time + attesting_cutoff + on_tick_and_append_step(spec, store, current_time, test_steps) + assert store.time == current_time + + yield from tick_and_add_block(spec, store, signed_block, test_steps) + + # Check conditions + head_root = spec.get_head(store) + head_block = store.blocks[head_root] + parent_root = head_block.parent_root + assert parent_root == signed_parent_block.message.hash_tree_root() + parent_block = store.blocks[parent_root] + + # Proposing next slot + next_slot(spec, state) + slot = state.slot + + # Add attestations to the parent block + current_time = slot * spec.config.SECONDS_PER_SLOT + store.genesis_time + on_tick_and_append_step(spec, store, current_time, test_steps) + attestations = get_valid_attestation_at_slot( + state, + spec, + slot_to_attest=slot - 1, + beacon_block_root=parent_root, + ) + for attestation in attestations: + yield from tick_and_run_on_attestation(spec, store, attestation, test_steps) + + # The conditions in `get_proposer_head` + assert spec.is_head_late(store, head_root) + assert spec.is_shuffling_stable(slot) + assert spec.is_ffg_competitive(store, head_root, parent_root) + assert spec.is_finalization_ok(store, slot) + assert spec.is_proposing_on_time(store) + + parent_slot_ok = parent_block.slot + 1 == head_block.slot + current_time_ok = head_block.slot + 1 == slot + single_slot_reorg = parent_slot_ok and current_time_ok + assert single_slot_reorg + + assert spec.is_head_weak(store, head_root) + assert spec.is_parent_strong(store, parent_root) + + proposer_head = spec.get_proposer_head(store, head_root, state.slot) + assert proposer_head == parent_root + + # TODO: export the `proposer_head` result to test vectors? + + yield 'steps', test_steps From 22215b8fe467d16ced6698e6d99c0204fb28165c Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 25 Oct 2023 01:34:57 +0800 Subject: [PATCH 08/49] Fix `parent_state_advanced` side effect and reorder the conditions --- specs/bellatrix/fork-choice.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/specs/bellatrix/fork-choice.md b/specs/bellatrix/fork-choice.md index fd70cfff4d..d78862037b 100644 --- a/specs/bellatrix/fork-choice.md +++ b/specs/bellatrix/fork-choice.md @@ -110,15 +110,15 @@ def should_override_forkchoice_update(store: Store, head_root: Root) -> bool: # FFG information of the new head_block will be competitive with the current head. ffg_competitive = is_ffg_competitive(store, head_root, parent_root) + # Do not re-org if the chain is not finalizing with acceptable frequency. + finalization_ok = is_finalization_ok(store, proposal_slot) + # Only suppress the fork choice update if we are confident that we will propose the next block. - parent_state_advanced = store.block_states[parent_root] + parent_state_advanced = store.block_states[parent_root].copy() process_slots(parent_state_advanced, proposal_slot) proposer_index = get_beacon_proposer_index(parent_state_advanced) proposing_reorg_slot = validator_is_connected(proposer_index) - # Do not re-org if the chain is not finalizing with acceptable frequency. - finalization_ok = is_finalization_ok(store, proposal_slot) - # Single slot re-org. parent_slot_ok = parent_block.slot + 1 == head_block.slot proposing_on_time = is_proposing_on_time(store) @@ -138,8 +138,9 @@ def should_override_forkchoice_update(store: Store, head_root: Root) -> bool: head_weak = True parent_strong = True - return all([head_late, shuffling_stable, proposing_reorg_slot, ffg_competitive, finalization_ok, - single_slot_reorg, head_weak, parent_strong]) + return all([head_late, shuffling_stable, ffg_competitive, finalization_ok, + proposing_reorg_slot, single_slot_reorg, + head_weak, parent_strong]) ``` *Note*: The ordering of conditions is a suggestion only. Implementations are free to From 3f8d8feb69a7562bb5c48b482ea3f2f4bd4069e5 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 25 Oct 2023 01:42:30 +0800 Subject: [PATCH 09/49] Update config files --- configs/mainnet.yaml | 7 +++++++ configs/minimal.yaml | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/configs/mainnet.yaml b/configs/mainnet.yaml index bcd18e5cdc..d0d5fc496c 100644 --- a/configs/mainnet.yaml +++ b/configs/mainnet.yaml @@ -93,6 +93,13 @@ CHURN_LIMIT_QUOTIENT: 65536 # --------------------------------------------------------------- # 40% PROPOSER_SCORE_BOOST: 40 +# 20% +REORG_WEIGHT_THRESHOLD: 20 +# 160% +REORG_PARENT_WEIGHT_THRESHOLD: 160 +# `2` epochs +REORG_MAX_EPOCHS_SINCE_FINALIZATION: 2 + # Deposit contract # --------------------------------------------------------------- diff --git a/configs/minimal.yaml b/configs/minimal.yaml index d23ca7adb2..8c09c7e37d 100644 --- a/configs/minimal.yaml +++ b/configs/minimal.yaml @@ -92,6 +92,12 @@ CHURN_LIMIT_QUOTIENT: 32 # --------------------------------------------------------------- # 40% PROPOSER_SCORE_BOOST: 40 +# 20% +REORG_WEIGHT_THRESHOLD: 20 +# 160% +REORG_PARENT_WEIGHT_THRESHOLD: 160 +# `2` epochs +REORG_MAX_EPOCHS_SINCE_FINALIZATION: 2 # Deposit contract From 35d444b9d225d9a18a1d3fc6e87c2294bd74ab05 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 25 Oct 2023 02:10:55 +0800 Subject: [PATCH 10/49] Update test format and output `should_override_forkchoice_update` result --- .../test/bellatrix/fork_choice/test_reorg.py | 21 ++++++++++++++++--- tests/formats/fork_choice/README.md | 1 + tests/generators/fork_choice/main.py | 2 ++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_reorg.py b/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_reorg.py index cb7adac1c2..617e0909c8 100644 --- a/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_reorg.py +++ b/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_reorg.py @@ -17,6 +17,7 @@ apply_next_slots_with_attestations, get_genesis_forkchoice_store_and_block, on_tick_and_append_step, + output_store_checks, tick_and_add_block, tick_and_run_on_attestation, ) @@ -56,7 +57,15 @@ def test_should_override_forkchoice_update__false(spec, state): current_time = slot * spec.config.SECONDS_PER_SLOT + store.genesis_time on_tick_and_append_step(spec, store, current_time, test_steps) - assert not spec.should_override_forkchoice_update(store, head_root) + should_override = spec.should_override_forkchoice_update(store, head_root) + assert not should_override + + output_store_checks(spec, store, test_steps) + test_steps.append({ + 'checks': { + 'should_override_forkchoice_update': should_override, + } + }) yield 'steps', test_steps @@ -155,8 +164,14 @@ def test_should_override_forkchoice_update__true(spec, state): assert spec.is_head_weak(store, head_root) assert spec.is_parent_strong(store, parent_root) - assert spec.should_override_forkchoice_update(store, head_root) + should_override = spec.should_override_forkchoice_update(store, head_root) + assert should_override - # TODO: export the `should_override_forkchoice_update` result to test vectors? + output_store_checks(spec, store, test_steps) + test_steps.append({ + 'checks': { + 'should_override_forkchoice_update': should_override, + } + }) yield 'steps', test_steps diff --git a/tests/formats/fork_choice/README.md b/tests/formats/fork_choice/README.md index d23de865b3..3bd773f524 100644 --- a/tests/formats/fork_choice/README.md +++ b/tests/formats/fork_choice/README.md @@ -177,6 +177,7 @@ finalized_checkpoint: { root: string, -- Encoded 32-byte value from store.finalized_checkpoint.root } proposer_boost_root: string -- Encoded 32-byte value from store.proposer_boost_root +should_override_forkchoice_update: bool -- [New in Bellatrix] the result of `should_override_forkchoice_update(store, head_root)` ``` For example: diff --git a/tests/generators/fork_choice/main.py b/tests/generators/fork_choice/main.py index 7ff028cd80..e8d19eb099 100644 --- a/tests/generators/fork_choice/main.py +++ b/tests/generators/fork_choice/main.py @@ -10,12 +10,14 @@ 'ex_ante', 'reorg', 'withholding', + 'get_proposer_head', ]} # For merge `on_merge_block` test kind added with `pow_block_N.ssz` files with several # PowBlock's which should be resolved by `get_pow_block(hash: Hash32) -> PowBlock` function _new_bellatrix_mods = {key: 'eth2spec.test.bellatrix.fork_choice.test_' + key for key in [ 'on_merge_block', + 'reorg', ]} bellatrix_mods = combine_mods(_new_bellatrix_mods, altair_mods) capella_mods = bellatrix_mods # No additional Capella specific fork choice tests From 1a53dbbf96944f88412fbe3f0394b15264774dc2 Mon Sep 17 00:00:00 2001 From: Nishant Das Date: Wed, 25 Oct 2023 22:05:57 +0800 Subject: [PATCH 11/49] Update p2p-interface.md --- specs/deneb/p2p-interface.md | 1 + 1 file changed, 1 insertion(+) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index a81e5d0bb6..831f906062 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -155,6 +155,7 @@ The following validations MUST pass before forwarding the `signed_blob_sidecar` - _[IGNORE]_ The sidecar's block's parent (defined by `sidecar.block_parent_root`) has been seen (via both gossip and non-gossip sources) (a client MAY queue sidecars for processing once the parent block is retrieved). - _[REJECT]_ The sidecar's block's parent (defined by `sidecar.block_parent_root`) passes validation. - _[REJECT]_ The sidecar is from a higher slot than the sidecar's block's parent (defined by `sidecar.block_parent_root`). +- _[REJECT]_ The current finalized_checkpoint is an ancestor of the sidecar's block's parent -- i.e. `get_checkpoint_block(store, sidecar.block_parent_root, store.finalized_checkpoint.epoch) == store.finalized_checkpoint.root`. - _[REJECT]_ The proposer signature, `signed_blob_sidecar.signature`, is valid as verified by `verify_blob_sidecar_signature`. - _[IGNORE]_ The sidecar is the only sidecar with valid signature received for the tuple `(sidecar.block_root, sidecar.index)`. - _[REJECT]_ The sidecar is proposed by the expected `proposer_index` for the block's slot in the context of the current shuffling (defined by `block_parent_root`/`slot`). From 3f1bc2051294d7ea632b5010500f98a331119d39 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 26 Oct 2023 17:44:48 +1100 Subject: [PATCH 12/49] Clarifications for proposer boost reorgs --- configs/mainnet.yaml | 2 +- configs/minimal.yaml | 2 +- specs/bellatrix/fork-choice.md | 4 ++-- specs/phase0/fork-choice.md | 5 +++-- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/configs/mainnet.yaml b/configs/mainnet.yaml index d0d5fc496c..aacd2a96e1 100644 --- a/configs/mainnet.yaml +++ b/configs/mainnet.yaml @@ -94,7 +94,7 @@ CHURN_LIMIT_QUOTIENT: 65536 # 40% PROPOSER_SCORE_BOOST: 40 # 20% -REORG_WEIGHT_THRESHOLD: 20 +REORG_HEAD_WEIGHT_THRESHOLD: 20 # 160% REORG_PARENT_WEIGHT_THRESHOLD: 160 # `2` epochs diff --git a/configs/minimal.yaml b/configs/minimal.yaml index 8c09c7e37d..592ad281b6 100644 --- a/configs/minimal.yaml +++ b/configs/minimal.yaml @@ -93,7 +93,7 @@ CHURN_LIMIT_QUOTIENT: 32 # 40% PROPOSER_SCORE_BOOST: 40 # 20% -REORG_WEIGHT_THRESHOLD: 20 +REORG_HEAD_WEIGHT_THRESHOLD: 20 # 160% REORG_PARENT_WEIGHT_THRESHOLD: 160 # `2` epochs diff --git a/specs/bellatrix/fork-choice.md b/specs/bellatrix/fork-choice.md index d78862037b..7bf607d6e9 100644 --- a/specs/bellatrix/fork-choice.md +++ b/specs/bellatrix/fork-choice.md @@ -154,8 +154,8 @@ of the block to be proposed (due to withdrawals). If `should_override_forkchoice_update` returns `True` but `get_proposer_head` later chooses the canonical head rather than its parent, then this is a misprediction that will cause the node -to construct a payload with less notice. The result of `get_proposer_head` MUST be honoured in -preference to the heuristic method. +to construct a payload with less notice. The result of `get_proposer_head` MUST be preferred over +the result of `should_override_forkchoice_update` (when proposer reorgs are enabled). ## Helpers diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 077209d5fc..6a54371152 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -89,7 +89,7 @@ Any of the above handlers that trigger an unhandled exception (e.g. a failed ass | Name | Value | | ------------------------------------- | ------------ | | `PROPOSER_SCORE_BOOST` | `uint64(40)` | -| `REORG_WEIGHT_THRESHOLD` | `uint64(20)` | +| `REORG_HEAD_WEIGHT_THRESHOLD` | `uint64(20)` | | `REORG_PARENT_WEIGHT_THRESHOLD` | `uint64(160)`| | `REORG_MAX_EPOCHS_SINCE_FINALIZATION` | `Epoch(2)` | @@ -431,6 +431,7 @@ def is_finalization_ok(store: Store, slot: Slot) -> bool: ```python def is_proposing_on_time(store: Store) -> bool: + # Use half `SECONDS_PER_SLOT // INTERVALS_PER_SLOT` as the proposer reorg deadline time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT proposer_reorg_cutoff = SECONDS_PER_SLOT // INTERVALS_PER_SLOT // 2 return time_into_slot <= proposer_reorg_cutoff @@ -441,7 +442,7 @@ def is_proposing_on_time(store: Store) -> bool: ```python def is_head_weak(store: Store, head_root: Root) -> bool: justified_state = store.checkpoint_states[store.justified_checkpoint] - reorg_threshold = calculate_committee_fraction(justified_state, REORG_WEIGHT_THRESHOLD) + reorg_threshold = calculate_committee_fraction(justified_state, REORG_HEAD_WEIGHT_THRESHOLD) head_weight = get_weight(store, head_root) return head_weight < reorg_threshold ``` From 9ce8e3d6e97bad6c5bb4378605dcdf82c734e783 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 26 Oct 2023 17:44:48 +1100 Subject: [PATCH 13/49] Update test format. Add `get_proposer_head` --- ...> test_should_override_forkchoice_update.py} | 0 .../fork_choice/test_get_proposer_head.py | 17 +++++++++++++++-- tests/formats/fork_choice/README.md | 12 ++++++++++-- tests/generators/fork_choice/main.py | 2 +- 4 files changed, 26 insertions(+), 5 deletions(-) rename tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/{test_reorg.py => test_should_override_forkchoice_update.py} (100%) diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_reorg.py b/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_should_override_forkchoice_update.py similarity index 100% rename from tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_reorg.py rename to tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_should_override_forkchoice_update.py diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_proposer_head.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_proposer_head.py index 083b292c0f..249e76b08a 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_proposer_head.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_proposer_head.py @@ -1,3 +1,4 @@ +from eth_utils import encode_hex from eth2spec.test.context import ( spec_state_test, with_altair_and_later, @@ -13,6 +14,7 @@ apply_next_slots_with_attestations, get_genesis_forkchoice_store_and_block, on_tick_and_append_step, + output_store_checks, tick_and_add_block, tick_and_run_on_attestation, ) @@ -51,9 +53,15 @@ def test_basic_is_head_root(spec, state): current_time = slot * spec.config.SECONDS_PER_SLOT + store.genesis_time on_tick_and_append_step(spec, store, current_time, test_steps) proposer_head = spec.get_proposer_head(store, head_root, slot) - assert proposer_head == head_root + output_store_checks(spec, store, test_steps) + test_steps.append({ + 'checks': { + 'get_proposer_head': encode_hex(proposer_head), + } + }) + yield 'steps', test_steps @@ -149,6 +157,11 @@ def test_basic_is_parent_root(spec, state): proposer_head = spec.get_proposer_head(store, head_root, state.slot) assert proposer_head == parent_root - # TODO: export the `proposer_head` result to test vectors? + output_store_checks(spec, store, test_steps) + test_steps.append({ + 'checks': { + 'get_proposer_head': encode_hex(proposer_head), + } + }) yield 'steps', test_steps diff --git a/tests/formats/fork_choice/README.md b/tests/formats/fork_choice/README.md index 3bd773f524..1d8b9cfb39 100644 --- a/tests/formats/fork_choice/README.md +++ b/tests/formats/fork_choice/README.md @@ -159,7 +159,7 @@ The checks to verify the current status of `store`. checks: {: value} -- the assertions. ``` -`` is the field member or property of [`Store`](../../../specs/phase0/fork-choice.md#store) object that maintained by client implementation. Currently, the possible fields included: +`` is the field member or property of [`Store`](../../../specs/phase0/fork-choice.md#store) object that maintained by client implementation. The fields include: ```yaml head: { @@ -177,7 +177,13 @@ finalized_checkpoint: { root: string, -- Encoded 32-byte value from store.finalized_checkpoint.root } proposer_boost_root: string -- Encoded 32-byte value from store.proposer_boost_root -should_override_forkchoice_update: bool -- [New in Bellatrix] the result of `should_override_forkchoice_update(store, head_root)` +``` + +Additionally, these fields if `get_proposer_head` and `should_override_forkchoice_update` features are implemented: + +```yaml +get_proposer_head: string -- Encoded 32-byte value from get_proposer_head(store) +should_override_forkchoice_update: bool -- [New in Bellatrix] the result of `should_override_forkchoice_update(store, head_root)`, where head_root is the result value from get_head(store) ``` For example: @@ -188,6 +194,8 @@ For example: justified_checkpoint: {epoch: 3, root: '0xc25faab4acab38d3560864ca01e4d5cc4dc2cd473da053fbc03c2669143a2de4'} finalized_checkpoint: {epoch: 2, root: '0x40d32d6283ec11c53317a46808bc88f55657d93b95a1af920403187accf48f4f'} proposer_boost_root: '0xdaa1d49d57594ced0c35688a6da133abb086d191a2ebdfd736fad95299325aeb' + get_proposer_head: '0xdaa1d49d57594ced0c35688a6da133abb086d191a2ebdfd736fad95299325aeb' + should_override_forkchoice_update: false ``` *Note*: Each `checks` step may include one or multiple items. Each item has to be checked against the current store. diff --git a/tests/generators/fork_choice/main.py b/tests/generators/fork_choice/main.py index e8d19eb099..df117ccd16 100644 --- a/tests/generators/fork_choice/main.py +++ b/tests/generators/fork_choice/main.py @@ -17,7 +17,7 @@ # PowBlock's which should be resolved by `get_pow_block(hash: Hash32) -> PowBlock` function _new_bellatrix_mods = {key: 'eth2spec.test.bellatrix.fork_choice.test_' + key for key in [ 'on_merge_block', - 'reorg', + 'should_override_forkchoice_update', ]} bellatrix_mods = combine_mods(_new_bellatrix_mods, altair_mods) capella_mods = bellatrix_mods # No additional Capella specific fork choice tests From b9285b8fe75da43f9142c6643cfac60f8ebf179b Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 26 Oct 2023 22:39:24 +0800 Subject: [PATCH 14/49] Add `validator_is_connected` --- .../test_should_override_forkchoice_update.py | 15 ++++++++++++--- tests/formats/fork_choice/README.md | 11 +++++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_should_override_forkchoice_update.py b/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_should_override_forkchoice_update.py index 617e0909c8..465a00f1a9 100644 --- a/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_should_override_forkchoice_update.py +++ b/tests/core/pyspec/eth2spec/test/bellatrix/fork_choice/test_should_override_forkchoice_update.py @@ -63,7 +63,10 @@ def test_should_override_forkchoice_update__false(spec, state): output_store_checks(spec, store, test_steps) test_steps.append({ 'checks': { - 'should_override_forkchoice_update': should_override, + 'should_override_forkchoice_update': { + 'validator_is_connected': True, + 'result': should_override, + }, } }) @@ -153,7 +156,10 @@ def test_should_override_forkchoice_update__true(spec, state): assert spec.is_ffg_competitive(store, head_root, parent_root) assert spec.is_finalization_ok(store, proposal_slot) - # TODO: proposing_reorg_slot + parent_state_advanced = store.block_states[parent_root].copy() + spec.process_slots(parent_state_advanced, proposal_slot) + proposer_index = spec.get_beacon_proposer_index(parent_state_advanced) + assert spec.validator_is_connected(proposer_index) # Single slot re-org. parent_slot_ok = parent_block.slot + 1 == head_block.slot @@ -170,7 +176,10 @@ def test_should_override_forkchoice_update__true(spec, state): output_store_checks(spec, store, test_steps) test_steps.append({ 'checks': { - 'should_override_forkchoice_update': should_override, + 'should_override_forkchoice_update': { + 'validator_is_connected': True, + 'result': should_override, + }, } }) diff --git a/tests/formats/fork_choice/README.md b/tests/formats/fork_choice/README.md index 1d8b9cfb39..1258a66c06 100644 --- a/tests/formats/fork_choice/README.md +++ b/tests/formats/fork_choice/README.md @@ -142,7 +142,7 @@ Optional step for optimistic sync tests. } ``` -This step sets the [`payloadStatus`](https://github.com/ethereum/execution-apis/blob/main/src/engine/specification.md#PayloadStatusV1) +This step sets the [`payloadStatus`](https://github.com/ethereum/execution-apis/blob/main/src/engine/paris.md#payloadstatusv1) value that Execution Layer client mock returns in responses to the following Engine API calls: * [`engine_newPayloadV1(payload)`](https://github.com/ethereum/execution-apis/blob/main/src/engine/paris.md#engine_newpayloadv1) if `payload.blockHash == payload_info.block_hash` * [`engine_forkchoiceUpdatedV1(forkchoiceState, ...)`](https://github.com/ethereum/execution-apis/blob/main/src/engine/paris.md#engine_forkchoiceupdatedv1) if `forkchoiceState.headBlockHash == payload_info.block_hash` @@ -182,8 +182,11 @@ proposer_boost_root: string -- Encoded 32-byte value from store.proposer_boost Additionally, these fields if `get_proposer_head` and `should_override_forkchoice_update` features are implemented: ```yaml -get_proposer_head: string -- Encoded 32-byte value from get_proposer_head(store) -should_override_forkchoice_update: bool -- [New in Bellatrix] the result of `should_override_forkchoice_update(store, head_root)`, where head_root is the result value from get_head(store) +get_proposer_head: string -- Encoded 32-byte value from get_proposer_head(store) +should_override_forkchoice_update: { -- [New in Bellatrix] + validator_is_connected: bool, -- The mocking result of `validator_is_connected(proposer_index)` in this call + result: bool, -- The result of `should_override_forkchoice_update(store, head_root)`, where head_root is the result value from get_head(store) +} ``` For example: @@ -195,7 +198,7 @@ For example: finalized_checkpoint: {epoch: 2, root: '0x40d32d6283ec11c53317a46808bc88f55657d93b95a1af920403187accf48f4f'} proposer_boost_root: '0xdaa1d49d57594ced0c35688a6da133abb086d191a2ebdfd736fad95299325aeb' get_proposer_head: '0xdaa1d49d57594ced0c35688a6da133abb086d191a2ebdfd736fad95299325aeb' - should_override_forkchoice_update: false + should_override_forkchoice_update: {validator_is_connected: false, result: false} ``` *Note*: Each `checks` step may include one or multiple items. Each item has to be checked against the current store. From 530efa88aa262f4b90c8680a7ee709ff98183725 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 12:10:29 +0300 Subject: [PATCH 15/49] Add blob sidecar inclusion proof --- specs/deneb/p2p-interface.md | 79 +++++++++++++++++++++++------------- specs/deneb/validator.md | 44 ++++++++++---------- 2 files changed, 71 insertions(+), 52 deletions(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index a81e5d0bb6..feda3563b6 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -14,10 +14,11 @@ The specification of these changes continues in the same format as the network s - [Configuration](#configuration) - [Containers](#containers) - [`BlobSidecar`](#blobsidecar) - - [`SignedBlobSidecar`](#signedblobsidecar) - [`BlobIdentifier`](#blobidentifier) - [Helpers](#helpers) - [`verify_blob_sidecar_signature`](#verify_blob_sidecar_signature) + - [`verify_blob_sidecar_inclusion_proof`](#verify_blob_sidecar_inclusion_proof) + - [`is_valid_merkle_path`](#is_valid_merkle_path) - [The gossip domain: gossipsub](#the-gossip-domain-gossipsub) - [Topics and messages](#topics-and-messages) - [Global topics](#global-topics) @@ -51,6 +52,8 @@ The specification of these changes continues in the same format as the network s | `MAX_REQUEST_BLOB_SIDECARS` | `MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK` | Maximum number of blob sidecars in a single request | | `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` | `2**12` (= 4096 epochs, ~18 days) | The minimum epoch range over which a node must serve blob sidecars | | `BLOB_SIDECAR_SUBNET_COUNT` | `6` | The number of blob sidecar subnets used in the gossipsub protocol. | +| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `17` | Merkle proof for `blob_kzg_commitments` list item | +| `BLOB_KZG_COMMITMENT_GINDEX` | `27` | Gindex path to `blob_kzg_commitments` on `BeaconBlockBody` container ### Containers @@ -60,24 +63,12 @@ The specification of these changes continues in the same format as the network s ```python class BlobSidecar(Container): - block_root: Root index: BlobIndex # Index of blob in block - slot: Slot - block_parent_root: Root # Proposer shuffling determinant - proposer_index: ValidatorIndex blob: Blob kzg_commitment: KZGCommitment kzg_proof: KZGProof # Allows for quick verification of kzg_commitment -``` - -#### `SignedBlobSidecar` - -*[New in Deneb:EIP4844]* - -```python -class SignedBlobSidecar(Container): - message: BlobSidecar - signature: BLSSignature + commitment_inclusion_proof: [Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH] + signed_block_header: SignedBeaconBlockHeader ``` #### `BlobIdentifier` @@ -95,10 +86,39 @@ class BlobIdentifier(Container): ##### `verify_blob_sidecar_signature` ```python -def verify_blob_sidecar_signature(state: BeaconState, signed_blob_sidecar: SignedBlobSidecar) -> bool: - proposer = state.validators[signed_blob_sidecar.message.proposer_index] - signing_root = compute_signing_root(signed_blob_sidecar.message, get_domain(state, DOMAIN_BLOB_SIDECAR)) - return bls.Verify(proposer.pubkey, signing_root, signed_blob_sidecar.signature) +def verify_blob_sidecar_signature(state: BeaconState, blob_sidecar: BlobSidecar) -> bool: + block_header = blob_sidecar.signed_block_header.message + proposer = state.validators[block_header.proposer_index] + domain = get_domain(state, DOMAIN_BEACON_PROPOSER, compute_epoch_at_slot(block_header.slot)) + signing_root = compute_signing_root(block_header, domain) + return bls.Verify(proposer.pubkey, signing_root, blob_sidecar.signed_block_header.signature) +``` + +##### `verify_blob_sidecar_inclusion_proof` + +```python +def verify_blob_sidecar_inclusion_proof(blob_sidecar: BlobSidecar) -> bool: + commitment_item_gindex = MAX_BLOB_COMMITMENTS_PER_BLOCK + blob_sidecar.index + gindex = BLOB_KZG_COMMITMENT_GINDEX + commitment_item_gindex << floorlog2(BLOB_ZKG_COMMITMENT_GINDEX) + return is_valid_merkle_path( + leaf=blob_sidecar.kzg_commitment.hash_tree_root(), + branch=blob_sidecar.commitment_inclusion_proof, + gindex=gindex, + root=blob_sidecar.signed_block_header.message.body_root, + ) +``` + +#### `is_valid_merkle_path` + +```python +def is_valid_merkle_path(leaf: Bytes32, branch: Sequence[Bytes32], gindex: int, root: Root) -> bool: + value = leaf + for i in range(len(branch)): + if (gindex >> i) & 1 == 0: + value = hash(branch[i] + value) + else: + value = hash(value + branch[i]) + return value == root ``` ### The gossip domain: gossipsub @@ -123,7 +143,7 @@ The new topics along with the type of the `data` field of a gossipsub message ar | Name | Message Type | | - | - | -| `blob_sidecar_{subnet_id}` | `SignedBlobSidecar` [New in Deneb:EIP4844] | +| `blob_sidecar_{subnet_id}` | `BlobSidecar` [New in Deneb:EIP4844] | ##### Global topics @@ -146,18 +166,19 @@ New validation: This topic is used to propagate signed blob sidecars, where each blob index maps to some `subnet_id`. -The following validations MUST pass before forwarding the `signed_blob_sidecar` on the network, assuming the alias `sidecar = signed_blob_sidecar.message`: +The following validations MUST pass before forwarding the `blob_sidecar` on the network, assuming the alias `sidecar = blob_sidecar` and `block_header = blob_sidecar.signed_block_header.message`: - _[REJECT]_ The sidecar's index is consistent with `MAX_BLOBS_PER_BLOCK` -- i.e. `sidecar.index < MAX_BLOBS_PER_BLOCK`. - _[REJECT]_ The sidecar is for the correct subnet -- i.e. `compute_subnet_for_blob_sidecar(sidecar.index) == subnet_id`. -- _[IGNORE]_ The sidecar is not from a future slot (with a `MAXIMUM_GOSSIP_CLOCK_DISPARITY` allowance) -- i.e. validate that `sidecar.slot <= current_slot` (a client MAY queue future sidecars for processing at the appropriate slot). -- _[IGNORE]_ The sidecar is from a slot greater than the latest finalized slot -- i.e. validate that `sidecar.slot > compute_start_slot_at_epoch(state.finalized_checkpoint.epoch)` -- _[IGNORE]_ The sidecar's block's parent (defined by `sidecar.block_parent_root`) has been seen (via both gossip and non-gossip sources) (a client MAY queue sidecars for processing once the parent block is retrieved). -- _[REJECT]_ The sidecar's block's parent (defined by `sidecar.block_parent_root`) passes validation. -- _[REJECT]_ The sidecar is from a higher slot than the sidecar's block's parent (defined by `sidecar.block_parent_root`). -- _[REJECT]_ The proposer signature, `signed_blob_sidecar.signature`, is valid as verified by `verify_blob_sidecar_signature`. -- _[IGNORE]_ The sidecar is the only sidecar with valid signature received for the tuple `(sidecar.block_root, sidecar.index)`. -- _[REJECT]_ The sidecar is proposed by the expected `proposer_index` for the block's slot in the context of the current shuffling (defined by `block_parent_root`/`slot`). +- _[IGNORE]_ The sidecar is not from a future slot (with a `MAXIMUM_GOSSIP_CLOCK_DISPARITY` allowance) -- i.e. validate that `block_header.slot <= current_slot` (a client MAY queue future sidecars for processing at the appropriate slot). +- _[IGNORE]_ The sidecar is from a slot greater than the latest finalized slot -- i.e. validate that `block_header.slot > compute_start_slot_at_epoch(state.finalized_checkpoint.epoch)` +- _[IGNORE]_ The sidecar's block's parent (defined by `block_header.parent_root`) has been seen (via both gossip and non-gossip sources) (a client MAY queue sidecars for processing once the parent block is retrieved). +- _[REJECT]_ The sidecar's block's parent (defined by `block_header.parent_root`) passes validation. +- _[REJECT]_ The sidecar is from a higher slot than the sidecar's block's parent (defined by `block_header.parent_root`). +- _[REJECT]_ The proposer signature in `blob_sidecar.signed_block_header`, is valid as verified by `verify_blob_sidecar_signature`. +- _[REJECT]_ The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof`. +- _[IGNORE]_ The sidecar is the only sidecar with valid signature received for the tuple `(hash_tree_root(block_header), sidecar.index)`. +- _[REJECT]_ The sidecar is proposed by the expected `proposer_index` for the block's slot in the context of the current shuffling (defined by `parent_root`/`slot`). If the `proposer_index` cannot immediately be verified against the expected shuffling, the sidecar MAY be queued for later processing while proposers for the block's branch are calculated -- in such a case _do not_ `REJECT`, instead `IGNORE` this message. ###### `beacon_aggregate_and_proof` diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index 7b8a81b19a..aa9e4a83c5 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -21,8 +21,8 @@ - [Constructing the `BeaconBlockBody`](#constructing-the-beaconblockbody) - [ExecutionPayload](#executionpayload) - [Blob KZG commitments](#blob-kzg-commitments) - - [Constructing the `SignedBlobSidecar`s](#constructing-the-signedblobsidecars) - - [Sidecar](#sidecar) + - [Constructing the `BlobSidecar`s](#constructing-the-blobsidecars) + - [BlobSidecar](#blob-sidecar) @@ -133,49 +133,47 @@ use the `payload_id` to retrieve `blobs`, `blob_kzg_commitments`, and `blob_kzg_ via `get_payload(payload_id).blobs_bundle`. 2. Set `block.body.blob_kzg_commitments = blob_kzg_commitments`. -#### Constructing the `SignedBlobSidecar`s +#### Constructing the `BlobSidecar`s *[New in Deneb:EIP4844]* -To construct a `SignedBlobSidecar`, a `signed_blob_sidecar` is defined with the necessary context for block and sidecar proposal. +To construct a `BlobSidecar`, a `blob_sidecar` is defined with the necessary context for block and sidecar proposal. ##### Sidecar -Blobs associated with a block are packaged into sidecar objects for distribution to the network. +Blobs associated with a block are packaged into sidecar objects for distribution to the associated sidecar topic, the `blob_sidecar_{subnet_id}` pubsub topic. Each `sidecar` is obtained from: ```python -def get_blob_sidecars(block: BeaconBlock, +def get_blob_sidecars(signed_block: SignedBeaconBlock, blobs: Sequence[Blob], blob_kzg_proofs: Sequence[KZGProof]) -> Sequence[BlobSidecar]: + block_header = BeaconBlockHeader( + slot=block.slot, + proposer_index=block.proposer_index, + parent_root=block.parent_root, + state_root=Bytes32(), # Overwritten in the next process_slot call + body_root=hash_tree_root(block.body), + ) + signed_block_header = SignedBeaconBlockHeader(message=block_header, signature=signed_block.signature) return [ BlobSidecar( - block_root=hash_tree_root(block), index=index, - slot=block.slot, - block_parent_root=block.parent_root, blob=blob, - kzg_commitment=block.body.blob_kzg_commitments[index], + kzg_commitment=signed_block.message.body.blob_kzg_commitments[index], kzg_proof=blob_kzg_proofs[index], + commitment_inclusion_proof=compute_commitment_inclusion_proof( + signed_block.message.body, + signed_block.message.body.blob_kzg_commitments[index], + index, + ), + signed_block_header=signed_block_header, ) for index, blob in enumerate(blobs) ] ``` -Then for each sidecar, `signed_sidecar = SignedBlobSidecar(message=sidecar, signature=signature)` is constructed and published to the associated sidecar topic, the `blob_sidecar_{subnet_id}` pubsub topic. - -`signature` is obtained from: - -```python -def get_blob_sidecar_signature(state: BeaconState, - sidecar: BlobSidecar, - privkey: int) -> BLSSignature: - domain = get_domain(state, DOMAIN_BLOB_SIDECAR, compute_epoch_at_slot(sidecar.slot)) - signing_root = compute_signing_root(sidecar, domain) - return bls.Sign(privkey, signing_root) -``` - The `subnet_id` for the `signed_sidecar` is calculated with: - Let `blob_index = signed_sidecar.message.index`. - Let `subnet_id = compute_subnet_for_blob_sidecar(blob_index)`. From e8bccecba5493269755b340db1947b84c2cac65e Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 15:47:33 +0300 Subject: [PATCH 16/49] remove verify_blob_sidecar_signature --- specs/deneb/p2p-interface.md | 14 +------------- specs/deneb/validator.md | 2 +- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index feda3563b6..5772589395 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -16,7 +16,6 @@ The specification of these changes continues in the same format as the network s - [`BlobSidecar`](#blobsidecar) - [`BlobIdentifier`](#blobidentifier) - [Helpers](#helpers) - - [`verify_blob_sidecar_signature`](#verify_blob_sidecar_signature) - [`verify_blob_sidecar_inclusion_proof`](#verify_blob_sidecar_inclusion_proof) - [`is_valid_merkle_path`](#is_valid_merkle_path) - [The gossip domain: gossipsub](#the-gossip-domain-gossipsub) @@ -83,17 +82,6 @@ class BlobIdentifier(Container): #### Helpers -##### `verify_blob_sidecar_signature` - -```python -def verify_blob_sidecar_signature(state: BeaconState, blob_sidecar: BlobSidecar) -> bool: - block_header = blob_sidecar.signed_block_header.message - proposer = state.validators[block_header.proposer_index] - domain = get_domain(state, DOMAIN_BEACON_PROPOSER, compute_epoch_at_slot(block_header.slot)) - signing_root = compute_signing_root(block_header, domain) - return bls.Verify(proposer.pubkey, signing_root, blob_sidecar.signed_block_header.signature) -``` - ##### `verify_blob_sidecar_inclusion_proof` ```python @@ -175,7 +163,7 @@ The following validations MUST pass before forwarding the `blob_sidecar` on the - _[IGNORE]_ The sidecar's block's parent (defined by `block_header.parent_root`) has been seen (via both gossip and non-gossip sources) (a client MAY queue sidecars for processing once the parent block is retrieved). - _[REJECT]_ The sidecar's block's parent (defined by `block_header.parent_root`) passes validation. - _[REJECT]_ The sidecar is from a higher slot than the sidecar's block's parent (defined by `block_header.parent_root`). -- _[REJECT]_ The proposer signature in `blob_sidecar.signed_block_header`, is valid as verified by `verify_blob_sidecar_signature`. +- _[REJECT]_ The proposer signature in `blob_sidecar.signed_block_header`, is valid with respect to the `proposer_index` pubkey. - _[REJECT]_ The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof`. - _[IGNORE]_ The sidecar is the only sidecar with valid signature received for the tuple `(hash_tree_root(block_header), sidecar.index)`. - _[REJECT]_ The sidecar is proposed by the expected `proposer_index` for the block's slot in the context of the current shuffling (defined by `parent_root`/`slot`). diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index aa9e4a83c5..f80e015f77 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -152,7 +152,7 @@ def get_blob_sidecars(signed_block: SignedBeaconBlock, slot=block.slot, proposer_index=block.proposer_index, parent_root=block.parent_root, - state_root=Bytes32(), # Overwritten in the next process_slot call + state_root=block.state_root, body_root=hash_tree_root(block.body), ) signed_block_header = SignedBeaconBlockHeader(message=block_header, signature=signed_block.signature) From 29bbdf439331483da5eff39c414f600441891bf2 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 16:14:17 +0300 Subject: [PATCH 17/49] compute KZG_COMMITMENT_INCLUSION_PROOF_DEPTH --- specs/deneb/p2p-interface.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 5772589395..e7fd55847f 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -51,8 +51,8 @@ The specification of these changes continues in the same format as the network s | `MAX_REQUEST_BLOB_SIDECARS` | `MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK` | Maximum number of blob sidecars in a single request | | `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` | `2**12` (= 4096 epochs, ~18 days) | The minimum epoch range over which a node must serve blob sidecars | | `BLOB_SIDECAR_SUBNET_COUNT` | `6` | The number of blob sidecar subnets used in the gossipsub protocol. | -| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `17` | Merkle proof for `blob_kzg_commitments` list item | -| `BLOB_KZG_COMMITMENT_GINDEX` | `27` | Gindex path to `blob_kzg_commitments` on `BeaconBlockBody` container +| `BLOB_KZG_COMMITMENTS_GINDEX` | `4 ** 2 + 11` (= 27) | `blob_kzg_commitments` field gindex on `BeaconBlockBody` container | +| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK) + 1` | Merkle proof for `blob_kzg_commitments` list item | ### Containers @@ -87,7 +87,7 @@ class BlobIdentifier(Container): ```python def verify_blob_sidecar_inclusion_proof(blob_sidecar: BlobSidecar) -> bool: commitment_item_gindex = MAX_BLOB_COMMITMENTS_PER_BLOCK + blob_sidecar.index - gindex = BLOB_KZG_COMMITMENT_GINDEX + commitment_item_gindex << floorlog2(BLOB_ZKG_COMMITMENT_GINDEX) + gindex = BLOB_KZG_COMMITMENTS_GINDEX + commitment_item_gindex << floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) return is_valid_merkle_path( leaf=blob_sidecar.kzg_commitment.hash_tree_root(), branch=blob_sidecar.commitment_inclusion_proof, From a4a29a164fe62a460a2605d580a16a4b668d311a Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 16:15:06 +0300 Subject: [PATCH 18/49] List typo --- specs/deneb/p2p-interface.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index e7fd55847f..023cd29c40 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -66,7 +66,7 @@ class BlobSidecar(Container): blob: Blob kzg_commitment: KZGCommitment kzg_proof: KZGProof # Allows for quick verification of kzg_commitment - commitment_inclusion_proof: [Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH] + commitment_inclusion_proof: List[Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH] signed_block_header: SignedBeaconBlockHeader ``` From 3dbe54e4b859dc47907bb011e3bad96756c3024e Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 16:16:10 +0300 Subject: [PATCH 19/49] doctoc --- specs/deneb/p2p-interface.md | 2 +- specs/deneb/validator.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 023cd29c40..7846c9a2a3 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -17,7 +17,7 @@ The specification of these changes continues in the same format as the network s - [`BlobIdentifier`](#blobidentifier) - [Helpers](#helpers) - [`verify_blob_sidecar_inclusion_proof`](#verify_blob_sidecar_inclusion_proof) - - [`is_valid_merkle_path`](#is_valid_merkle_path) + - [`is_valid_merkle_path`](#is_valid_merkle_path) - [The gossip domain: gossipsub](#the-gossip-domain-gossipsub) - [Topics and messages](#topics-and-messages) - [Global topics](#global-topics) diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index f80e015f77..49e1136000 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -22,7 +22,7 @@ - [ExecutionPayload](#executionpayload) - [Blob KZG commitments](#blob-kzg-commitments) - [Constructing the `BlobSidecar`s](#constructing-the-blobsidecars) - - [BlobSidecar](#blob-sidecar) + - [Sidecar](#sidecar) From c2a64a18f0deb318c81ffc4dfa9fa96ab2370cdf Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 17:07:50 +0300 Subject: [PATCH 20/49] pass lint --- pysetup/spec_builders/deneb.py | 13 +++- specs/deneb/p2p-interface.md | 2 +- specs/deneb/validator.md | 8 +-- .../unittests/validator/test_validator.py | 69 ++++++++++++------- 4 files changed, 63 insertions(+), 29 deletions(-) diff --git a/pysetup/spec_builders/deneb.py b/pysetup/spec_builders/deneb.py index c32bee8305..612ae6cf67 100644 --- a/pysetup/spec_builders/deneb.py +++ b/pysetup/spec_builders/deneb.py @@ -23,7 +23,17 @@ def sundry_functions(cls) -> str: return ''' def retrieve_blobs_and_proofs(beacon_block_root: Root) -> Tuple[Sequence[Blob], Sequence[KZGProof]]: # pylint: disable=unused-argument - return [], []''' + return [], [] + + +def compute_commitment_inclusion_proof( + body: BeaconBlockBody, + blob_kzg_commitments: Sequence[KZGCommitment], + index: int +) -> List[Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH]: + # pylint: disable=unused-argument + return [] +''' @classmethod def execution_engine_cls(cls) -> str: @@ -68,4 +78,5 @@ def hardcoded_custom_type_dep_constants(cls, spec_object) -> str: 'BYTES_PER_FIELD_ELEMENT': spec_object.constant_vars['BYTES_PER_FIELD_ELEMENT'].value, 'FIELD_ELEMENTS_PER_BLOB': spec_object.preset_vars['FIELD_ELEMENTS_PER_BLOB'].value, 'MAX_BLOBS_PER_BLOCK': spec_object.preset_vars['MAX_BLOBS_PER_BLOCK'].value, + 'MAX_BLOB_COMMITMENTS_PER_BLOCK': spec_object.preset_vars['MAX_BLOB_COMMITMENTS_PER_BLOCK'].value, } diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 7846c9a2a3..c7044dc1ed 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -52,7 +52,7 @@ The specification of these changes continues in the same format as the network s | `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` | `2**12` (= 4096 epochs, ~18 days) | The minimum epoch range over which a node must serve blob sidecars | | `BLOB_SIDECAR_SUBNET_COUNT` | `6` | The number of blob sidecar subnets used in the gossipsub protocol. | | `BLOB_KZG_COMMITMENTS_GINDEX` | `4 ** 2 + 11` (= 27) | `blob_kzg_commitments` field gindex on `BeaconBlockBody` container | -| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK) + 1` | Merkle proof for `blob_kzg_commitments` list item | +| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK) # noqa: E501` | Merkle proof for `blob_kzg_commitments` list item | ### Containers diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index 49e1136000..13b816b6f5 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -148,6 +148,7 @@ Each `sidecar` is obtained from: def get_blob_sidecars(signed_block: SignedBeaconBlock, blobs: Sequence[Blob], blob_kzg_proofs: Sequence[KZGProof]) -> Sequence[BlobSidecar]: + block = signed_block.message block_header = BeaconBlockHeader( slot=block.slot, proposer_index=block.proposer_index, @@ -160,18 +161,17 @@ def get_blob_sidecars(signed_block: SignedBeaconBlock, BlobSidecar( index=index, blob=blob, - kzg_commitment=signed_block.message.body.blob_kzg_commitments[index], + kzg_commitment=block.body.blob_kzg_commitments[index], kzg_proof=blob_kzg_proofs[index], commitment_inclusion_proof=compute_commitment_inclusion_proof( - signed_block.message.body, - signed_block.message.body.blob_kzg_commitments[index], + block.body, + block.body.blob_kzg_commitments[index], index, ), signed_block_header=signed_block_header, ) for index, blob in enumerate(blobs) ] - ``` The `subnet_id` for the `signed_sidecar` is calculated with: diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py index 876824107a..ef031693d0 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py @@ -12,16 +12,48 @@ from eth2spec.test.helpers.block import ( build_empty_block_for_next_slot ) -from eth2spec.test.helpers.keys import ( - pubkey_to_privkey -) +from tests.core.pyspec.eth2spec.utils.ssz.ssz_impl import hash_tree_root + + +def get_blob_sidecars(spec, signed_block, blobs, blob_kzg_proofs): + block = signed_block.message + block_header = spec.BeaconBlockHeader( + slot=block.slot, + proposer_index=block.proposer_index, + parent_root=block.parent_root, + state_root=block.state_root, + body_root=hash_tree_root(block.body), + ) + signed_block_header = spec.SignedBeaconBlockHeader(message=block_header, signature=signed_block.signature) + return [ + spec.BlobSidecar( + index=index, + blob=blob, + kzg_commitment=signed_block.message.body.blob_kzg_commitments[index], + kzg_proof=blob_kzg_proofs[index], + commitment_inclusion_proof=compute_commitment_inclusion_proof( + spec, + signed_block.message.body, + signed_block.message.body.blob_kzg_commitments[index], + index, + ), + signed_block_header=signed_block_header, + ) + for index, blob in enumerate(blobs) + ] + + +def compute_commitment_inclusion_proof(spec, body, kzg_commitment, index): + gindex = (spec.BeaconBlockBody / 'blob_kzg_commitments' / index).gindex() + raise Exception('todo, does remerkleable expose an API to compute proofs?') + return gindex @with_deneb_and_later @spec_state_test -def test_blob_sidecar_signature(spec, state): +def test_blob_sidecar_inclusion_proof(spec, state): """ - Test `get_blob_sidecar_signature` + Test `verify_blob_sidecar_inclusion_proof` """ blob_count = 4 block = build_empty_block_for_next_slot(spec, state) @@ -30,22 +62,16 @@ def test_blob_sidecar_signature(spec, state): block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - blob_sidecars = spec.get_blob_sidecars(block, blobs, proofs) - proposer = state.validators[blob_sidecars[1].proposer_index] - privkey = pubkey_to_privkey[proposer.pubkey] - sidecar_signature = spec.get_blob_sidecar_signature(state, - blob_sidecars[1], - privkey) - - signed_blob_sidecar = spec.SignedBlobSidecar(message=blob_sidecars[1], signature=sidecar_signature) + blob_sidecars = spec.get_blob_sidecars(spec, block, blobs, proofs) - assert spec.verify_blob_sidecar_signature(state, signed_blob_sidecar) + for blob_sidecar in blob_sidecars: + assert spec.verify_blob_sidecar_inclusion_proof(blob_sidecar) @with_deneb_and_later @spec_state_test @always_bls -def test_blob_sidecar_signature_incorrect(spec, state): +def test_blob_sidecar_inclusion_proof_incorrect(spec, state): """ Test `get_blob_sidecar_signature` """ @@ -56,12 +82,9 @@ def test_blob_sidecar_signature_incorrect(spec, state): block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - blob_sidecars = spec.get_blob_sidecars(block, blobs, proofs) - - sidecar_signature = spec.get_blob_sidecar_signature(state, - blob_sidecars[1], - 123) - - signed_blob_sidecar = spec.SignedBlobSidecar(message=blob_sidecars[1], signature=sidecar_signature) + blob_sidecars = spec.get_blob_sidecars(spec, block, blobs, proofs) - assert not spec.verify_blob_sidecar_signature(state, signed_blob_sidecar) + for blob_sidecar in blob_sidecars: + block = blob_sidecar.signed_block_header.message + block = block.body_root = hash_tree_root(block.body_root) # mutate body root to break proof + assert not spec.verify_blob_sidecar_inclusion_proof(blob_sidecar) From caa79a5a02d5a421d820f121b3bee7075f6f2752 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 18:08:05 +0300 Subject: [PATCH 21/49] build tree --- pysetup/spec_builders/deneb.py | 8 +++----- specs/deneb/beacon-chain.md | 13 +++++++++++++ specs/deneb/p2p-interface.md | 19 ++----------------- specs/deneb/validator.md | 3 +-- .../unittests/validator/test_validator.py | 8 +++----- 5 files changed, 22 insertions(+), 29 deletions(-) diff --git a/pysetup/spec_builders/deneb.py b/pysetup/spec_builders/deneb.py index 612ae6cf67..14087702a9 100644 --- a/pysetup/spec_builders/deneb.py +++ b/pysetup/spec_builders/deneb.py @@ -28,11 +28,9 @@ def retrieve_blobs_and_proofs(beacon_block_root: Root) -> Tuple[Sequence[Blob], def compute_commitment_inclusion_proof( body: BeaconBlockBody, - blob_kzg_commitments: Sequence[KZGCommitment], - index: int -) -> List[Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH]: - # pylint: disable=unused-argument - return [] + index: GeneralizedIndex +) -> Sequence[Bytes32]: + return build_proof(body.get_backing(), index) ''' @classmethod diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index b98ac12592..64bcca5c28 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -187,6 +187,19 @@ def kzg_commitment_to_versioned_hash(kzg_commitment: KZGCommitment) -> Versioned return VERSIONED_HASH_VERSION_KZG + hash(kzg_commitment)[1:] ``` +#### `is_valid_merkle_path` + +```python +def is_valid_merkle_path(leaf: Bytes32, branch: Sequence[Bytes32], gindex: int, root: Root) -> bool: + value = leaf + for i in range(len(branch)): + if (gindex >> i) & 1 == 0: + value = hash(branch[i] + value) + else: + value = hash(value + branch[i]) + return value == root +``` + ### Beacon state accessors #### Modified `get_attestation_participation_flag_indices` diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index c7044dc1ed..13ac98f643 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -51,7 +51,7 @@ The specification of these changes continues in the same format as the network s | `MAX_REQUEST_BLOB_SIDECARS` | `MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK` | Maximum number of blob sidecars in a single request | | `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` | `2**12` (= 4096 epochs, ~18 days) | The minimum epoch range over which a node must serve blob sidecars | | `BLOB_SIDECAR_SUBNET_COUNT` | `6` | The number of blob sidecar subnets used in the gossipsub protocol. | -| `BLOB_KZG_COMMITMENTS_GINDEX` | `4 ** 2 + 11` (= 27) | `blob_kzg_commitments` field gindex on `BeaconBlockBody` container | +| `BLOB_KZG_COMMITMENTS_GINDEX` | `get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')` (= 27) | `blob_kzg_commitments` field gindex on `BeaconBlockBody` container | | `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK) # noqa: E501` | Merkle proof for `blob_kzg_commitments` list item | ### Containers @@ -86,29 +86,14 @@ class BlobIdentifier(Container): ```python def verify_blob_sidecar_inclusion_proof(blob_sidecar: BlobSidecar) -> bool: - commitment_item_gindex = MAX_BLOB_COMMITMENTS_PER_BLOCK + blob_sidecar.index - gindex = BLOB_KZG_COMMITMENTS_GINDEX + commitment_item_gindex << floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) return is_valid_merkle_path( leaf=blob_sidecar.kzg_commitment.hash_tree_root(), branch=blob_sidecar.commitment_inclusion_proof, - gindex=gindex, + gindex=get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', blob_sidecar.index), root=blob_sidecar.signed_block_header.message.body_root, ) ``` -#### `is_valid_merkle_path` - -```python -def is_valid_merkle_path(leaf: Bytes32, branch: Sequence[Bytes32], gindex: int, root: Root) -> bool: - value = leaf - for i in range(len(branch)): - if (gindex >> i) & 1 == 0: - value = hash(branch[i] + value) - else: - value = hash(value + branch[i]) - return value == root -``` - ### The gossip domain: gossipsub Some gossip meshes are upgraded in the fork of Deneb to support upgraded types. diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index 13b816b6f5..ba41adf0b9 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -165,8 +165,7 @@ def get_blob_sidecars(signed_block: SignedBeaconBlock, kzg_proof=blob_kzg_proofs[index], commitment_inclusion_proof=compute_commitment_inclusion_proof( block.body, - block.body.blob_kzg_commitments[index], - index, + get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', index), ), signed_block_header=signed_block_header, ) diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py index ef031693d0..a45acb0710 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py @@ -34,7 +34,6 @@ def get_blob_sidecars(spec, signed_block, blobs, blob_kzg_proofs): commitment_inclusion_proof=compute_commitment_inclusion_proof( spec, signed_block.message.body, - signed_block.message.body.blob_kzg_commitments[index], index, ), signed_block_header=signed_block_header, @@ -43,10 +42,9 @@ def get_blob_sidecars(spec, signed_block, blobs, blob_kzg_proofs): ] -def compute_commitment_inclusion_proof(spec, body, kzg_commitment, index): - gindex = (spec.BeaconBlockBody / 'blob_kzg_commitments' / index).gindex() - raise Exception('todo, does remerkleable expose an API to compute proofs?') - return gindex +def compute_commitment_inclusion_proof(spec, body, index): + gindex = spec.get_generalized_index(spec.BeaconBlockBody, 'blob_kzg_commitments', index) + return spec.build_proof(body, gindex) @with_deneb_and_later From 8712451c43c60095eee2e10aaf996cf63710632a Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 18:09:01 +0300 Subject: [PATCH 22/49] Update specs/deneb/p2p-interface.md Co-authored-by: Mikhail Kalinin --- specs/deneb/p2p-interface.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 13ac98f643..346810295c 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -151,7 +151,7 @@ The following validations MUST pass before forwarding the `blob_sidecar` on the - _[REJECT]_ The proposer signature in `blob_sidecar.signed_block_header`, is valid with respect to the `proposer_index` pubkey. - _[REJECT]_ The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof`. - _[IGNORE]_ The sidecar is the only sidecar with valid signature received for the tuple `(hash_tree_root(block_header), sidecar.index)`. -- _[REJECT]_ The sidecar is proposed by the expected `proposer_index` for the block's slot in the context of the current shuffling (defined by `parent_root`/`slot`). +- _[REJECT]_ The sidecar is proposed by the expected `proposer_index` for the block's slot in the context of the current shuffling (defined by `block_header.parent_root`/`block_header.slot`). If the `proposer_index` cannot immediately be verified against the expected shuffling, the sidecar MAY be queued for later processing while proposers for the block's branch are calculated -- in such a case _do not_ `REJECT`, instead `IGNORE` this message. ###### `beacon_aggregate_and_proof` From f2649f65ba36d3ea6bc0234547759245622226bc Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 18:51:43 +0300 Subject: [PATCH 23/49] fix unit tests --- specs/deneb/p2p-interface.md | 4 +- specs/deneb/validator.md | 2 +- .../unittests/validator/test_validator.py | 43 +++---------------- 3 files changed, 10 insertions(+), 39 deletions(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 346810295c..5ace66fde0 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -51,8 +51,8 @@ The specification of these changes continues in the same format as the network s | `MAX_REQUEST_BLOB_SIDECARS` | `MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK` | Maximum number of blob sidecars in a single request | | `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` | `2**12` (= 4096 epochs, ~18 days) | The minimum epoch range over which a node must serve blob sidecars | | `BLOB_SIDECAR_SUBNET_COUNT` | `6` | The number of blob sidecar subnets used in the gossipsub protocol. | -| `BLOB_KZG_COMMITMENTS_GINDEX` | `get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')` (= 27) | `blob_kzg_commitments` field gindex on `BeaconBlockBody` container | -| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK) # noqa: E501` | Merkle proof for `blob_kzg_commitments` list item | +| `BLOB_KZG_COMMITMENTS_GINDEX` | `2**4 + 11` (= 27) | `blob_kzg_commitments` field gindex on `BeaconBlockBody` container | +| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `4 + 1 + floorlog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` | Merkle proof for `blob_kzg_commitments` list item | ### Containers diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index ba41adf0b9..6b21ea30fd 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -165,7 +165,7 @@ def get_blob_sidecars(signed_block: SignedBeaconBlock, kzg_proof=blob_kzg_proofs[index], commitment_inclusion_proof=compute_commitment_inclusion_proof( block.body, - get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', index), + get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', index), # type: ignore ), signed_block_header=signed_block_header, ) diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py index a45acb0710..52374d42f5 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py @@ -10,46 +10,15 @@ get_sample_opaque_tx, ) from eth2spec.test.helpers.block import ( - build_empty_block_for_next_slot + build_empty_block_for_next_slot, + sign_block ) from tests.core.pyspec.eth2spec.utils.ssz.ssz_impl import hash_tree_root -def get_blob_sidecars(spec, signed_block, blobs, blob_kzg_proofs): - block = signed_block.message - block_header = spec.BeaconBlockHeader( - slot=block.slot, - proposer_index=block.proposer_index, - parent_root=block.parent_root, - state_root=block.state_root, - body_root=hash_tree_root(block.body), - ) - signed_block_header = spec.SignedBeaconBlockHeader(message=block_header, signature=signed_block.signature) - return [ - spec.BlobSidecar( - index=index, - blob=blob, - kzg_commitment=signed_block.message.body.blob_kzg_commitments[index], - kzg_proof=blob_kzg_proofs[index], - commitment_inclusion_proof=compute_commitment_inclusion_proof( - spec, - signed_block.message.body, - index, - ), - signed_block_header=signed_block_header, - ) - for index, blob in enumerate(blobs) - ] - - -def compute_commitment_inclusion_proof(spec, body, index): - gindex = spec.get_generalized_index(spec.BeaconBlockBody, 'blob_kzg_commitments', index) - return spec.build_proof(body, gindex) - - @with_deneb_and_later @spec_state_test -def test_blob_sidecar_inclusion_proof(spec, state): +def test_blob_sidecar_inclusion_proof_correct(spec, state): """ Test `verify_blob_sidecar_inclusion_proof` """ @@ -60,7 +29,8 @@ def test_blob_sidecar_inclusion_proof(spec, state): block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - blob_sidecars = spec.get_blob_sidecars(spec, block, blobs, proofs) + signed_block = sign_block(spec, state, block, proposer_index=0) + blob_sidecars = spec.get_blob_sidecars(signed_block, blobs, proofs) for blob_sidecar in blob_sidecars: assert spec.verify_blob_sidecar_inclusion_proof(blob_sidecar) @@ -80,7 +50,8 @@ def test_blob_sidecar_inclusion_proof_incorrect(spec, state): block.body.execution_payload.transactions = [opaque_tx] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - blob_sidecars = spec.get_blob_sidecars(spec, block, blobs, proofs) + signed_block = sign_block(spec, state, block, proposer_index=0) + blob_sidecars = spec.get_blob_sidecars(signed_block, blobs, proofs) for blob_sidecar in blob_sidecars: block = blob_sidecar.signed_block_header.message From 83e5930b5ca98a9993e285f4dc92768d548c5599 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 20:08:40 +0300 Subject: [PATCH 24/49] doctoc --- specs/deneb/beacon-chain.md | 1 + specs/deneb/p2p-interface.md | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index 64bcca5c28..fde75b0ed3 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -25,6 +25,7 @@ - [Helper functions](#helper-functions) - [Misc](#misc) - [`kzg_commitment_to_versioned_hash`](#kzg_commitment_to_versioned_hash) + - [`is_valid_merkle_path`](#is_valid_merkle_path) - [Beacon state accessors](#beacon-state-accessors) - [Modified `get_attestation_participation_flag_indices`](#modified-get_attestation_participation_flag_indices) - [New `get_validator_activation_churn_limit`](#new-get_validator_activation_churn_limit) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 5ace66fde0..3a4721cee8 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -17,7 +17,6 @@ The specification of these changes continues in the same format as the network s - [`BlobIdentifier`](#blobidentifier) - [Helpers](#helpers) - [`verify_blob_sidecar_inclusion_proof`](#verify_blob_sidecar_inclusion_proof) - - [`is_valid_merkle_path`](#is_valid_merkle_path) - [The gossip domain: gossipsub](#the-gossip-domain-gossipsub) - [Topics and messages](#topics-and-messages) - [Global topics](#global-topics) From 0bf9e75964515e62fb999f931138c948bff89ad7 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 27 Oct 2023 20:22:17 +0300 Subject: [PATCH 25/49] review PR --- pysetup/spec_builders/deneb.py | 5 +---- specs/deneb/p2p-interface.md | 8 ++++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pysetup/spec_builders/deneb.py b/pysetup/spec_builders/deneb.py index 14087702a9..9d976c451e 100644 --- a/pysetup/spec_builders/deneb.py +++ b/pysetup/spec_builders/deneb.py @@ -26,10 +26,7 @@ def retrieve_blobs_and_proofs(beacon_block_root: Root) -> Tuple[Sequence[Blob], return [], [] -def compute_commitment_inclusion_proof( - body: BeaconBlockBody, - index: GeneralizedIndex -) -> Sequence[Bytes32]: +def compute_commitment_inclusion_proof(body: BeaconBlockBody, index: GeneralizedIndex) -> Sequence[Bytes32]: return build_proof(body.get_backing(), index) ''' diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 3a4721cee8..b3c3850864 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -51,7 +51,7 @@ The specification of these changes continues in the same format as the network s | `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` | `2**12` (= 4096 epochs, ~18 days) | The minimum epoch range over which a node must serve blob sidecars | | `BLOB_SIDECAR_SUBNET_COUNT` | `6` | The number of blob sidecar subnets used in the gossipsub protocol. | | `BLOB_KZG_COMMITMENTS_GINDEX` | `2**4 + 11` (= 27) | `blob_kzg_commitments` field gindex on `BeaconBlockBody` container | -| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `4 + 1 + floorlog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` | Merkle proof for `blob_kzg_commitments` list item | +| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `4 + 1 + floorlog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` | Merkle proof depth for `blob_kzg_commitments` list item | ### Containers @@ -65,8 +65,8 @@ class BlobSidecar(Container): blob: Blob kzg_commitment: KZGCommitment kzg_proof: KZGProof # Allows for quick verification of kzg_commitment - commitment_inclusion_proof: List[Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH] signed_block_header: SignedBeaconBlockHeader + commitment_inclusion_proof: List[Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH] ``` #### `BlobIdentifier` @@ -147,9 +147,9 @@ The following validations MUST pass before forwarding the `blob_sidecar` on the - _[IGNORE]_ The sidecar's block's parent (defined by `block_header.parent_root`) has been seen (via both gossip and non-gossip sources) (a client MAY queue sidecars for processing once the parent block is retrieved). - _[REJECT]_ The sidecar's block's parent (defined by `block_header.parent_root`) passes validation. - _[REJECT]_ The sidecar is from a higher slot than the sidecar's block's parent (defined by `block_header.parent_root`). -- _[REJECT]_ The proposer signature in `blob_sidecar.signed_block_header`, is valid with respect to the `proposer_index` pubkey. +- _[REJECT]_ The proposer signature of `blob_sidecar.signed_block_header`, is valid with respect to the `block_header.proposer_index` pubkey. - _[REJECT]_ The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof`. -- _[IGNORE]_ The sidecar is the only sidecar with valid signature received for the tuple `(hash_tree_root(block_header), sidecar.index)`. +- _[IGNORE]_ The sidecar is the first sidecar for the tuple (block_header.slot, block_header.proposer_index, sidecar.index) with valid header signature and sidecar inclusion proof - _[REJECT]_ The sidecar is proposed by the expected `proposer_index` for the block's slot in the context of the current shuffling (defined by `block_header.parent_root`/`block_header.slot`). If the `proposer_index` cannot immediately be verified against the expected shuffling, the sidecar MAY be queued for later processing while proposers for the block's branch are calculated -- in such a case _do not_ `REJECT`, instead `IGNORE` this message. From 26516ec3e96648b09e1b8ce978abb5d67bdc3332 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sat, 28 Oct 2023 03:53:16 +0800 Subject: [PATCH 26/49] Move `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` to preset and cast `int()` --- presets/mainnet/deneb.yaml | 2 ++ presets/minimal/deneb.yaml | 2 ++ specs/deneb/p2p-interface.md | 10 ++++++++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/presets/mainnet/deneb.yaml b/presets/mainnet/deneb.yaml index 23889fd18e..38db7e90a0 100644 --- a/presets/mainnet/deneb.yaml +++ b/presets/mainnet/deneb.yaml @@ -8,3 +8,5 @@ FIELD_ELEMENTS_PER_BLOB: 4096 MAX_BLOB_COMMITMENTS_PER_BLOCK: 4096 # `uint64(6)` MAX_BLOBS_PER_BLOCK: 6 +# `4 + 1 + floorlog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 12 = 17 +KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 17 diff --git a/presets/minimal/deneb.yaml b/presets/minimal/deneb.yaml index 9721bec14a..cac61bdbf3 100644 --- a/presets/minimal/deneb.yaml +++ b/presets/minimal/deneb.yaml @@ -8,3 +8,5 @@ FIELD_ELEMENTS_PER_BLOB: 4096 MAX_BLOB_COMMITMENTS_PER_BLOCK: 16 # `uint64(6)` MAX_BLOBS_PER_BLOCK: 6 +# [customized] `4 + 1 + floorlog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 4 = 9 +KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 9 diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index b3c3850864..fa4378387a 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -40,6 +40,14 @@ The specification of these changes continues in the same format as the network s ## Modifications in Deneb +### Preset + +*[New in Deneb:EIP4844]* + +| Name | Value | Description | +|------------------------------------------|-----------------------------------|---------------------------------------------------------------------| +| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `int(4 + 1 + floorlog2(MAX_BLOB_COMMITMENTS_PER_BLOCK))` | Merkle proof depth for `blob_kzg_commitments` list item | + ### Configuration *[New in Deneb:EIP4844]* @@ -50,8 +58,6 @@ The specification of these changes continues in the same format as the network s | `MAX_REQUEST_BLOB_SIDECARS` | `MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK` | Maximum number of blob sidecars in a single request | | `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` | `2**12` (= 4096 epochs, ~18 days) | The minimum epoch range over which a node must serve blob sidecars | | `BLOB_SIDECAR_SUBNET_COUNT` | `6` | The number of blob sidecar subnets used in the gossipsub protocol. | -| `BLOB_KZG_COMMITMENTS_GINDEX` | `2**4 + 11` (= 27) | `blob_kzg_commitments` field gindex on `BeaconBlockBody` container | -| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `4 + 1 + floorlog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` | Merkle proof depth for `blob_kzg_commitments` list item | ### Containers From 56f8c2bb06a99fac7aebbacf5e8a3de2398b2dc0 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Sat, 28 Oct 2023 10:39:54 -0600 Subject: [PATCH 27/49] Update validator.md Align block production description using the latest APIs with corresponding symbols. --- specs/deneb/validator.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index 7b8a81b19a..2a8c832c73 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -128,10 +128,8 @@ def prepare_execution_payload(state: BeaconState, *[New in Deneb:EIP4844]* -1. After retrieving the execution payload from the execution engine as specified above, -use the `payload_id` to retrieve `blobs`, `blob_kzg_commitments`, and `blob_kzg_proofs` -via `get_payload(payload_id).blobs_bundle`. -2. Set `block.body.blob_kzg_commitments = blob_kzg_commitments`. +1. The execution payload is obtained from the execution engine as defined above using `payload_id`. The response also includes a `blobs_bundle` entry containing the corresponding `blobs`, `commitments`, and `proofs`. +2. Set `block.body.blob_kzg_commitments = commitments`. #### Constructing the `SignedBlobSidecar`s From 35e22702f34a612e1c0a7f1c39bc10c1aff81e01 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Sat, 28 Oct 2023 11:11:27 -0600 Subject: [PATCH 28/49] remove `BLOB_TX_TYPE` from spec (and move to tests) --- specs/deneb/beacon-chain.md | 1 - tests/core/pyspec/eth2spec/test/helpers/sharding.py | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index b98ac12592..852f98308f 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -77,7 +77,6 @@ Deneb is a consensus-layer upgrade containing a number of features. Including: | Name | Value | | - | - | -| `BLOB_TX_TYPE` | `uint8(0x03)` | | `VERSIONED_HASH_VERSION_KZG` | `Bytes1('0x01')` | ## Preset diff --git a/tests/core/pyspec/eth2spec/test/helpers/sharding.py b/tests/core/pyspec/eth2spec/test/helpers/sharding.py index 25b4dea599..c06b1aa9cb 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/sharding.py +++ b/tests/core/pyspec/eth2spec/test/helpers/sharding.py @@ -7,6 +7,7 @@ Union, boolean, uint256, uint64, + uint8, ) from eth2spec.utils.ssz.ssz_impl import serialize @@ -20,6 +21,9 @@ MAX_ACCESS_LIST_SIZE = 2**24 +BLOB_TX_TYPE = uint8(0x03) + + class AccessTuple(Container): address: Bytes20 # Address = Bytes20 storage_keys: List[Bytes32, MAX_ACCESS_LIST_STORAGE_KEYS] @@ -120,5 +124,5 @@ def get_sample_opaque_tx(spec, blob_count=1, rng=random.Random(5566), is_valid_b ) ) serialized_tx = serialize(signed_blob_tx) - opaque_tx = spec.uint_to_bytes(spec.BLOB_TX_TYPE) + serialized_tx + opaque_tx = spec.uint_to_bytes(BLOB_TX_TYPE) + serialized_tx return opaque_tx, blobs, blob_kzg_commitments, blob_kzg_proofs From f214e25509a7f4626e5ba513ff878edd73208f73 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Sat, 28 Oct 2023 11:41:27 -0600 Subject: [PATCH 29/49] Update p2p-interface.md Suggest readers of `BlobSidecarsByRoot` also verify KZG proofs of blob data. --- specs/deneb/p2p-interface.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index a81e5d0bb6..7b8bacd253 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -280,6 +280,8 @@ It may be less in the case that the responding peer is missing blocks or sidecar The response is unsigned, i.e. `BlobSidecar`, as the signature of the beacon block proposer may not be available beyond the initial distribution via gossip. +Before consuming the next response chunk, the response reader SHOULD verify the blob sidecar is well-formatted and correct w.r.t. the expected KZG commitments through `verify_blob_kzg_proof_batch`. + No more than `MAX_REQUEST_BLOB_SIDECARS` may be requested at a time. `BlobSidecarsByRoot` is primarily used to recover recent blobs (e.g. when receiving a block with a transaction whose corresponding blob is missing). From 1657d16afd5d8def1b4f33999e7043f9ca7cd407 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sun, 29 Oct 2023 02:33:07 +0800 Subject: [PATCH 30/49] Add `BLOB_KZG_COMMITMENTS_GINDEX` to "Constant". Use pyspec parser trick to add assertion in pyspec --- presets/mainnet/deneb.yaml | 4 +++- presets/minimal/deneb.yaml | 4 +++- pysetup/helpers.py | 7 ++++++- pysetup/spec_builders/base.py | 4 ++++ pysetup/spec_builders/deneb.py | 15 ++++++++++++++- pysetup/typing.py | 1 + setup.py | 15 +++++++++++++++ specs/deneb/p2p-interface.md | 10 +++++++++- 8 files changed, 55 insertions(+), 5 deletions(-) diff --git a/presets/mainnet/deneb.yaml b/presets/mainnet/deneb.yaml index 38db7e90a0..2a1e02e1ca 100644 --- a/presets/mainnet/deneb.yaml +++ b/presets/mainnet/deneb.yaml @@ -8,5 +8,7 @@ FIELD_ELEMENTS_PER_BLOB: 4096 MAX_BLOB_COMMITMENTS_PER_BLOCK: 4096 # `uint64(6)` MAX_BLOBS_PER_BLOCK: 6 -# `4 + 1 + floorlog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 12 = 17 +# `get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')` = 27 +BLOB_KZG_COMMITMENTS_GINDEX: 27 +# `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 12 = 17 KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 17 diff --git a/presets/minimal/deneb.yaml b/presets/minimal/deneb.yaml index cac61bdbf3..19c22cf946 100644 --- a/presets/minimal/deneb.yaml +++ b/presets/minimal/deneb.yaml @@ -8,5 +8,7 @@ FIELD_ELEMENTS_PER_BLOB: 4096 MAX_BLOB_COMMITMENTS_PER_BLOCK: 16 # `uint64(6)` MAX_BLOBS_PER_BLOCK: 6 -# [customized] `4 + 1 + floorlog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 4 = 9 +# `get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')` = 27 +BLOB_KZG_COMMITMENTS_GINDEX: 27 +# [customized] `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 4 = 9 KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 9 diff --git a/pysetup/helpers.py b/pysetup/helpers.py index 692aaa0d7e..cfb4dec4e9 100644 --- a/pysetup/helpers.py +++ b/pysetup/helpers.py @@ -111,8 +111,9 @@ def format_constant(name: str, vardef: VariableDefinition) -> str: return out # Merge all constant objects - hardcoded_ssz_dep_constants = reduce(lambda obj, builder: {**obj, **builder.hardcoded_ssz_dep_constants()}, builders, {}) + hardcoded_ssz_dep_constants = reduce(lambda obj, builder: {**obj, **builder.hardcoded_ssz_dep_constants()}, builders, {}) hardcoded_custom_type_dep_constants = reduce(lambda obj, builder: {**obj, **builder.hardcoded_custom_type_dep_constants(spec_object)}, builders, {}) + hardcoded_func_dep_presets = reduce(lambda obj, builder: {**obj, **builder.hardcoded_func_dep_presets(spec_object)}, builders, {}) # Concatenate all strings imports = reduce(lambda txt, builder: (txt + "\n\n" + builder.imports(preset_name) ).strip("\n"), builders, "") preparations = reduce(lambda txt, builder: (txt + "\n\n" + builder.preparations() ).strip("\n"), builders, "") @@ -126,6 +127,7 @@ def format_constant(name: str, vardef: VariableDefinition) -> str: ssz_dep_constants = '\n'.join(map(lambda x: '%s = %s' % (x, hardcoded_ssz_dep_constants[x]), hardcoded_ssz_dep_constants)) ssz_dep_constants_verification = '\n'.join(map(lambda x: 'assert %s == %s' % (x, spec_object.ssz_dep_constants[x]), hardcoded_ssz_dep_constants)) custom_type_dep_constants = '\n'.join(map(lambda x: '%s = %s' % (x, hardcoded_custom_type_dep_constants[x]), hardcoded_custom_type_dep_constants)) + func_dep_presets_verification = '\n'.join(map(lambda x: 'assert %s == %s # noqa: E501' % (x, spec_object.func_dep_presets[x]), hardcoded_func_dep_presets)) spec_strs = [ imports, preparations, @@ -147,6 +149,7 @@ def format_constant(name: str, vardef: VariableDefinition) -> str: # Since some constants are hardcoded in setup.py, the following assertions verify that the hardcoded constants are # as same as the spec definition. ssz_dep_constants_verification, + func_dep_presets_verification, ] return "\n\n\n".join([str.strip("\n") for str in spec_strs if str]) + "\n" @@ -223,6 +226,7 @@ def combine_spec_objects(spec0: SpecObject, spec1: SpecObject) -> SpecObject: preset_vars = combine_dicts(spec0.preset_vars, spec1.preset_vars) config_vars = combine_dicts(spec0.config_vars, spec1.config_vars) ssz_dep_constants = combine_dicts(spec0.ssz_dep_constants, spec1.ssz_dep_constants) + func_dep_presets = combine_dicts(spec0.func_dep_presets, spec1.func_dep_presets) ssz_objects = combine_ssz_objects(spec0.ssz_objects, spec1.ssz_objects, custom_types) dataclasses = combine_dicts(spec0.dataclasses, spec1.dataclasses) return SpecObject( @@ -233,6 +237,7 @@ def combine_spec_objects(spec0: SpecObject, spec1: SpecObject) -> SpecObject: preset_vars=preset_vars, config_vars=config_vars, ssz_dep_constants=ssz_dep_constants, + func_dep_presets=func_dep_presets, ssz_objects=ssz_objects, dataclasses=dataclasses, ) diff --git a/pysetup/spec_builders/base.py b/pysetup/spec_builders/base.py index 44743682ab..ad9a2cb4c4 100644 --- a/pysetup/spec_builders/base.py +++ b/pysetup/spec_builders/base.py @@ -47,6 +47,10 @@ def hardcoded_custom_type_dep_constants(cls, spec_object) -> Dict[str, str]: # """ return {} + @classmethod + def hardcoded_func_dep_presets(cls, spec_object) -> Dict[str, str]: + return {} + @classmethod def implement_optimizations(cls, functions: Dict[str, str]) -> Dict[str, str]: return functions diff --git a/pysetup/spec_builders/deneb.py b/pysetup/spec_builders/deneb.py index 9d976c451e..febcd2742c 100644 --- a/pysetup/spec_builders/deneb.py +++ b/pysetup/spec_builders/deneb.py @@ -1,3 +1,4 @@ +from typing import Dict from .base import BaseSpecBuilder from ..constants import DENEB @@ -68,10 +69,22 @@ def verify_and_notify_new_payload(self: ExecutionEngine, @classmethod - def hardcoded_custom_type_dep_constants(cls, spec_object) -> str: + def hardcoded_custom_type_dep_constants(cls, spec_object) -> Dict[str, str]: return { 'BYTES_PER_FIELD_ELEMENT': spec_object.constant_vars['BYTES_PER_FIELD_ELEMENT'].value, 'FIELD_ELEMENTS_PER_BLOB': spec_object.preset_vars['FIELD_ELEMENTS_PER_BLOB'].value, 'MAX_BLOBS_PER_BLOCK': spec_object.preset_vars['MAX_BLOBS_PER_BLOCK'].value, 'MAX_BLOB_COMMITMENTS_PER_BLOCK': spec_object.preset_vars['MAX_BLOB_COMMITMENTS_PER_BLOCK'].value, } + + @classmethod + def hardcoded_ssz_dep_constants(cls) -> Dict[str, str]: + return { + 'BLOB_KZG_COMMITMENTS_GINDEX': 'GeneralizedIndex(27)', + } + + @classmethod + def hardcoded_func_dep_presets(cls, spec_object) -> Dict[str, str]: + return { + 'KZG_COMMITMENT_INCLUSION_PROOF_DEPTH': spec_object.preset_vars['KZG_COMMITMENT_INCLUSION_PROOF_DEPTH'].value, + } diff --git a/pysetup/typing.py b/pysetup/typing.py index 56f0cf3b10..28c2220c50 100644 --- a/pysetup/typing.py +++ b/pysetup/typing.py @@ -22,6 +22,7 @@ class SpecObject(NamedTuple): preset_vars: Dict[str, VariableDefinition] config_vars: Dict[str, VariableDefinition] ssz_dep_constants: Dict[str, str] # the constants that depend on ssz_objects + func_dep_presets: Dict[str, str] # the constants that depend on functions ssz_objects: Dict[str, str] dataclasses: Dict[str, str] diff --git a/setup.py b/setup.py index f540094348..87a3dc3ca8 100644 --- a/setup.py +++ b/setup.py @@ -162,6 +162,7 @@ def get_spec(file_name: Path, preset: Dict[str, str], config: Dict[str, str], pr preset_vars: Dict[str, VariableDefinition] = {} config_vars: Dict[str, VariableDefinition] = {} ssz_dep_constants: Dict[str, str] = {} + func_dep_presets: Dict[str, str] = {} ssz_objects: Dict[str, str] = {} dataclasses: Dict[str, str] = {} custom_types: Dict[str, str] = {} @@ -214,6 +215,16 @@ def get_spec(file_name: Path, preset: Dict[str, str], config: Dict[str, str], pr value_cell = cells[1] value = value_cell.children[0].children + + description = None + if len(cells) >= 3: + description_cell = cells[2] + if len(description_cell.children) > 0: + description = description_cell.children[0].children + if isinstance(description, list): + # marko parses `**X**` as a list containing a X + description = description[0].children + if isinstance(value, list): # marko parses `**X**` as a list containing a X value = value[0].children @@ -228,6 +239,9 @@ def get_spec(file_name: Path, preset: Dict[str, str], config: Dict[str, str], pr ssz_dep_constants[name] = value continue + if description is not None and description.startswith(""): + func_dep_presets[name] = value + value_def = _parse_value(name, value) if name in preset: preset_vars[name] = VariableDefinition(value_def.type_name, preset[name], value_def.comment, None) @@ -256,6 +270,7 @@ def get_spec(file_name: Path, preset: Dict[str, str], config: Dict[str, str], pr preset_vars=preset_vars, config_vars=config_vars, ssz_dep_constants=ssz_dep_constants, + func_dep_presets=func_dep_presets, ssz_objects=ssz_objects, dataclasses=dataclasses, ) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index fa4378387a..05c6b568d2 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -40,13 +40,21 @@ The specification of these changes continues in the same format as the network s ## Modifications in Deneb +### Constant + +*[New in Deneb:EIP4844]* + +| Name | Value | Description | +|------------------------------------------|-----------------------------------|---------------------------------------------------------------------| +| `BLOB_KZG_COMMITMENTS_GINDEX` | `get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')` (= 27) | `blob_kzg_commitments` field gindex on `BeaconBlockBody` container | + ### Preset *[New in Deneb:EIP4844]* | Name | Value | Description | |------------------------------------------|-----------------------------------|---------------------------------------------------------------------| -| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `int(4 + 1 + floorlog2(MAX_BLOB_COMMITMENTS_PER_BLOCK))` | Merkle proof depth for `blob_kzg_commitments` list item | +| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `uint64(floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK))` (= 17) | Merkle proof depth for `blob_kzg_commitments` list item | ### Configuration From de3b6a24dc107834dce3e195a243b261d740a72a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sun, 29 Oct 2023 02:33:39 +0800 Subject: [PATCH 31/49] Fix toc --- specs/deneb/p2p-interface.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 05c6b568d2..9fd61c7edf 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -11,6 +11,8 @@ The specification of these changes continues in the same format as the network s - [Modifications in Deneb](#modifications-in-deneb) + - [Constant](#constant) + - [Preset](#preset) - [Configuration](#configuration) - [Containers](#containers) - [`BlobSidecar`](#blobsidecar) From b7e0b88cb18a9e726af9ff1e7828c778de899d77 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sun, 29 Oct 2023 03:28:57 +0800 Subject: [PATCH 32/49] Fix test --- specs/deneb/p2p-interface.md | 6 ++++-- .../test/deneb/unittests/validator/test_validator.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 9fd61c7edf..5d640ddf54 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -101,10 +101,12 @@ class BlobIdentifier(Container): ```python def verify_blob_sidecar_inclusion_proof(blob_sidecar: BlobSidecar) -> bool: - return is_valid_merkle_path( + gindex = get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', blob_sidecar.index) + return is_valid_merkle_branch( leaf=blob_sidecar.kzg_commitment.hash_tree_root(), branch=blob_sidecar.commitment_inclusion_proof, - gindex=get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', blob_sidecar.index), + depth=floorlog2(gindex), + index=get_subtree_index(gindex), root=blob_sidecar.signed_block_header.message.body_root, ) ``` diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py index 52374d42f5..8adea7d31d 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py @@ -55,5 +55,5 @@ def test_blob_sidecar_inclusion_proof_incorrect(spec, state): for blob_sidecar in blob_sidecars: block = blob_sidecar.signed_block_header.message - block = block.body_root = hash_tree_root(block.body_root) # mutate body root to break proof + block.body_root = spec.hash(block.body_root) # mutate body root to break proof assert not spec.verify_blob_sidecar_inclusion_proof(blob_sidecar) From b018fbc46a37c23ff57b27d0a9678cbf0082561a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sun, 29 Oct 2023 03:39:24 +0800 Subject: [PATCH 33/49] Remove `BLOB_KZG_COMMITMENTS_GINDEX` from the preset files --- presets/mainnet/deneb.yaml | 2 -- presets/minimal/deneb.yaml | 2 -- 2 files changed, 4 deletions(-) diff --git a/presets/mainnet/deneb.yaml b/presets/mainnet/deneb.yaml index 2a1e02e1ca..6d2fb4abde 100644 --- a/presets/mainnet/deneb.yaml +++ b/presets/mainnet/deneb.yaml @@ -8,7 +8,5 @@ FIELD_ELEMENTS_PER_BLOB: 4096 MAX_BLOB_COMMITMENTS_PER_BLOCK: 4096 # `uint64(6)` MAX_BLOBS_PER_BLOCK: 6 -# `get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')` = 27 -BLOB_KZG_COMMITMENTS_GINDEX: 27 # `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 12 = 17 KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 17 diff --git a/presets/minimal/deneb.yaml b/presets/minimal/deneb.yaml index 19c22cf946..340e8d8d5f 100644 --- a/presets/minimal/deneb.yaml +++ b/presets/minimal/deneb.yaml @@ -8,7 +8,5 @@ FIELD_ELEMENTS_PER_BLOB: 4096 MAX_BLOB_COMMITMENTS_PER_BLOCK: 16 # `uint64(6)` MAX_BLOBS_PER_BLOCK: 6 -# `get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')` = 27 -BLOB_KZG_COMMITMENTS_GINDEX: 27 # [customized] `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 4 = 9 KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 9 From ae6a9eba264c890179ad4b1c6b50debdd8d0519d Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sun, 29 Oct 2023 03:39:39 +0800 Subject: [PATCH 34/49] Fix lint --- .../eth2spec/test/deneb/unittests/validator/test_validator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py index 8adea7d31d..6cd3011fcb 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py @@ -13,7 +13,6 @@ build_empty_block_for_next_slot, sign_block ) -from tests.core.pyspec.eth2spec.utils.ssz.ssz_impl import hash_tree_root @with_deneb_and_later From 0e4737eba29c6bba500cc5c1bc7b49990b7cf37a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 30 Oct 2023 14:05:09 +0800 Subject: [PATCH 35/49] Add a general `compute_merkle_proof` helper to replace container-specific helpers --- pysetup/helpers.py | 3 +-- pysetup/spec_builders/altair.py | 6 +++--- pysetup/spec_builders/capella.py | 9 --------- pysetup/spec_builders/deneb.py | 4 ---- specs/altair/light-client/full-node.md | 18 +++++++++--------- specs/capella/light-client/full-node.md | 11 +---------- specs/deneb/light-client/full-node.md | 2 +- specs/deneb/validator.md | 2 +- .../light_client/test_single_merkle_proof.py | 9 +++------ .../light_client/test_single_merkle_proof.py | 3 +-- .../eth2spec/test/helpers/light_client.py | 6 ++---- 11 files changed, 22 insertions(+), 51 deletions(-) diff --git a/pysetup/helpers.py b/pysetup/helpers.py index cfb4dec4e9..b2fe00f75e 100644 --- a/pysetup/helpers.py +++ b/pysetup/helpers.py @@ -68,8 +68,7 @@ def format_protocol(protocol_name: str, protocol_def: ProtocolDefinition) -> str if k in [ "ceillog2", "floorlog2", - "compute_merkle_proof_for_block_body", - "compute_merkle_proof_for_state", + "compute_merkle_proof", ]: del spec_object.functions[k] diff --git a/pysetup/spec_builders/altair.py b/pysetup/spec_builders/altair.py index 4b35380de9..ff667b19ce 100644 --- a/pysetup/spec_builders/altair.py +++ b/pysetup/spec_builders/altair.py @@ -34,9 +34,9 @@ def get_generalized_index(ssz_class: Any, *path: Sequence[PyUnion[int, SSZVariab return GeneralizedIndex(ssz_path.gindex()) -def compute_merkle_proof_for_state(state: BeaconState, - index: GeneralizedIndex) -> Sequence[Bytes32]: - return build_proof(state.get_backing(), index)''' +def compute_merkle_proof(object: SSZObject, + index: GeneralizedIndex) -> Sequence[Bytes32]: + return build_proof(object.get_backing(), index)''' @classmethod diff --git a/pysetup/spec_builders/capella.py b/pysetup/spec_builders/capella.py index 03b619b66a..080bdb537f 100644 --- a/pysetup/spec_builders/capella.py +++ b/pysetup/spec_builders/capella.py @@ -13,15 +13,6 @@ def imports(cls, preset_name: str): from eth2spec.bellatrix import {preset_name} as bellatrix ''' - - @classmethod - def sundry_functions(cls) -> str: - return ''' -def compute_merkle_proof_for_block_body(body: BeaconBlockBody, - index: GeneralizedIndex) -> Sequence[Bytes32]: - return build_proof(body.get_backing(), index)''' - - @classmethod def hardcoded_ssz_dep_constants(cls) -> Dict[str, str]: return { diff --git a/pysetup/spec_builders/deneb.py b/pysetup/spec_builders/deneb.py index febcd2742c..486018f868 100644 --- a/pysetup/spec_builders/deneb.py +++ b/pysetup/spec_builders/deneb.py @@ -25,10 +25,6 @@ def sundry_functions(cls) -> str: def retrieve_blobs_and_proofs(beacon_block_root: Root) -> Tuple[Sequence[Blob], Sequence[KZGProof]]: # pylint: disable=unused-argument return [], [] - - -def compute_commitment_inclusion_proof(body: BeaconBlockBody, index: GeneralizedIndex) -> Sequence[Bytes32]: - return build_proof(body.get_backing(), index) ''' @classmethod diff --git a/specs/altair/light-client/full-node.md b/specs/altair/light-client/full-node.md index 7dc25448c1..27651af01f 100644 --- a/specs/altair/light-client/full-node.md +++ b/specs/altair/light-client/full-node.md @@ -10,7 +10,7 @@ - [Introduction](#introduction) - [Helper functions](#helper-functions) - - [`compute_merkle_proof_for_state`](#compute_merkle_proof_for_state) + - [`compute_merkle_proof`](#compute_merkle_proof) - [`block_to_light_client_header`](#block_to_light_client_header) - [Deriving light client data](#deriving-light-client-data) - [`create_light_client_bootstrap`](#create_light_client_bootstrap) @@ -27,11 +27,13 @@ This document provides helper functions to enable full nodes to serve light clie ## Helper functions -### `compute_merkle_proof_for_state` +### `compute_merkle_proof` + +This function return the Merkle proof of the given SSZ object `object` at generalized index `index`. ```python -def compute_merkle_proof_for_state(state: BeaconState, - index: GeneralizedIndex) -> Sequence[Bytes32]: +def compute_merkle_proof(object: SSZObject, + index: GeneralizedIndex) -> Sequence[Bytes32]: ... ``` @@ -73,7 +75,7 @@ def create_light_client_bootstrap(state: BeaconState, return LightClientBootstrap( header=block_to_light_client_header(block), current_sync_committee=state.current_sync_committee, - current_sync_committee_branch=compute_merkle_proof_for_state(state, CURRENT_SYNC_COMMITTEE_INDEX), + current_sync_committee_branch=compute_merkle_proof(state, CURRENT_SYNC_COMMITTEE_INDEX), ) ``` @@ -120,8 +122,7 @@ def create_light_client_update(state: BeaconState, # `next_sync_committee` is only useful if the message is signed by the current sync committee if update_attested_period == update_signature_period: update.next_sync_committee = attested_state.next_sync_committee - update.next_sync_committee_branch = compute_merkle_proof_for_state( - attested_state, NEXT_SYNC_COMMITTEE_INDEX) + update.next_sync_committee_branch = compute_merkle_proof(attested_state, NEXT_SYNC_COMMITTEE_INDEX) # Indicate finality whenever possible if finalized_block is not None: @@ -130,8 +131,7 @@ def create_light_client_update(state: BeaconState, assert hash_tree_root(update.finalized_header.beacon) == attested_state.finalized_checkpoint.root else: assert attested_state.finalized_checkpoint.root == Bytes32() - update.finality_branch = compute_merkle_proof_for_state( - attested_state, FINALIZED_ROOT_INDEX) + update.finality_branch = compute_merkle_proof(attested_state, FINALIZED_ROOT_INDEX) update.sync_aggregate = block.message.body.sync_aggregate update.signature_slot = block.message.slot diff --git a/specs/capella/light-client/full-node.md b/specs/capella/light-client/full-node.md index a406cd771b..c59af8ec74 100644 --- a/specs/capella/light-client/full-node.md +++ b/specs/capella/light-client/full-node.md @@ -10,7 +10,6 @@ - [Introduction](#introduction) - [Helper functions](#helper-functions) - - [`compute_merkle_proof_for_block_body`](#compute_merkle_proof_for_block_body) - [Modified `block_to_light_client_header`](#modified-block_to_light_client_header) @@ -22,14 +21,6 @@ This upgrade adds information about the execution payload to light client data a ## Helper functions -### `compute_merkle_proof_for_block_body` - -```python -def compute_merkle_proof_for_block_body(body: BeaconBlockBody, - index: GeneralizedIndex) -> Sequence[Bytes32]: - ... -``` - ### Modified `block_to_light_client_header` ```python @@ -55,7 +46,7 @@ def block_to_light_client_header(block: SignedBeaconBlock) -> LightClientHeader: transactions_root=hash_tree_root(payload.transactions), withdrawals_root=hash_tree_root(payload.withdrawals), ) - execution_branch = compute_merkle_proof_for_block_body(block.message.body, EXECUTION_PAYLOAD_INDEX) + execution_branch = compute_merkle_proof(block.message.body, EXECUTION_PAYLOAD_INDEX) else: # Note that during fork transitions, `finalized_header` may still point to earlier forks. # While Bellatrix blocks also contain an `ExecutionPayload` (minus `withdrawals_root`), diff --git a/specs/deneb/light-client/full-node.md b/specs/deneb/light-client/full-node.md index 281348167a..18b97ae43a 100644 --- a/specs/deneb/light-client/full-node.md +++ b/specs/deneb/light-client/full-node.md @@ -52,7 +52,7 @@ def block_to_light_client_header(block: SignedBeaconBlock) -> LightClientHeader: execution_header.blob_gas_used = payload.blob_gas_used execution_header.excess_blob_gas = payload.excess_blob_gas - execution_branch = compute_merkle_proof_for_block_body(block.message.body, EXECUTION_PAYLOAD_INDEX) + execution_branch = compute_merkle_proof(block.message.body, EXECUTION_PAYLOAD_INDEX) else: # Note that during fork transitions, `finalized_header` may still point to earlier forks. # While Bellatrix blocks also contain an `ExecutionPayload` (minus `withdrawals_root`), diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index 6b21ea30fd..e5141746a6 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -163,7 +163,7 @@ def get_blob_sidecars(signed_block: SignedBeaconBlock, blob=blob, kzg_commitment=block.body.blob_kzg_commitments[index], kzg_proof=blob_kzg_proofs[index], - commitment_inclusion_proof=compute_commitment_inclusion_proof( + commitment_inclusion_proof=compute_merkle_proof( block.body, get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', index), # type: ignore ), diff --git a/tests/core/pyspec/eth2spec/test/altair/light_client/test_single_merkle_proof.py b/tests/core/pyspec/eth2spec/test/altair/light_client/test_single_merkle_proof.py index 7e9c6b7e2f..104d4774cf 100644 --- a/tests/core/pyspec/eth2spec/test/altair/light_client/test_single_merkle_proof.py +++ b/tests/core/pyspec/eth2spec/test/altair/light_client/test_single_merkle_proof.py @@ -10,8 +10,7 @@ @spec_state_test def test_current_sync_committee_merkle_proof(spec, state): yield "object", state - current_sync_committee_branch = spec.compute_merkle_proof_for_state( - state, spec.CURRENT_SYNC_COMMITTEE_INDEX) + current_sync_committee_branch = spec.compute_merkle_proof(state, spec.CURRENT_SYNC_COMMITTEE_INDEX) yield "proof", { "leaf": "0x" + state.current_sync_committee.hash_tree_root().hex(), "leaf_index": spec.CURRENT_SYNC_COMMITTEE_INDEX, @@ -31,8 +30,7 @@ def test_current_sync_committee_merkle_proof(spec, state): @spec_state_test def test_next_sync_committee_merkle_proof(spec, state): yield "object", state - next_sync_committee_branch = spec.compute_merkle_proof_for_state( - state, spec.NEXT_SYNC_COMMITTEE_INDEX) + next_sync_committee_branch = spec.compute_merkle_proof(state, spec.NEXT_SYNC_COMMITTEE_INDEX) yield "proof", { "leaf": "0x" + state.next_sync_committee.hash_tree_root().hex(), "leaf_index": spec.NEXT_SYNC_COMMITTEE_INDEX, @@ -52,8 +50,7 @@ def test_next_sync_committee_merkle_proof(spec, state): @spec_state_test def test_finality_root_merkle_proof(spec, state): yield "object", state - finality_branch = spec.compute_merkle_proof_for_state( - state, spec.FINALIZED_ROOT_INDEX) + finality_branch = spec.compute_merkle_proof(state, spec.FINALIZED_ROOT_INDEX) yield "proof", { "leaf": "0x" + state.finalized_checkpoint.root.hex(), "leaf_index": spec.FINALIZED_ROOT_INDEX, diff --git a/tests/core/pyspec/eth2spec/test/capella/light_client/test_single_merkle_proof.py b/tests/core/pyspec/eth2spec/test/capella/light_client/test_single_merkle_proof.py index 8d3bf8e3cb..8401a94442 100644 --- a/tests/core/pyspec/eth2spec/test/capella/light_client/test_single_merkle_proof.py +++ b/tests/core/pyspec/eth2spec/test/capella/light_client/test_single_merkle_proof.py @@ -15,8 +15,7 @@ def test_execution_merkle_proof(spec, state): block = state_transition_with_full_block(spec, state, True, False) yield "object", block.message.body - execution_branch = spec.compute_merkle_proof_for_block_body( - block.message.body, spec.EXECUTION_PAYLOAD_INDEX) + execution_branch = spec.compute_merkle_proof(block.message.body, spec.EXECUTION_PAYLOAD_INDEX) yield "proof", { "leaf": "0x" + block.message.body.execution_payload.hash_tree_root().hex(), "leaf_index": spec.EXECUTION_PAYLOAD_INDEX, diff --git a/tests/core/pyspec/eth2spec/test/helpers/light_client.py b/tests/core/pyspec/eth2spec/test/helpers/light_client.py index ceca145e94..1878832c37 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/light_client.py +++ b/tests/core/pyspec/eth2spec/test/helpers/light_client.py @@ -56,13 +56,11 @@ def create_update(spec, if with_next: update.next_sync_committee = attested_state.next_sync_committee - update.next_sync_committee_branch = spec.compute_merkle_proof_for_state( - attested_state, spec.NEXT_SYNC_COMMITTEE_INDEX) + update.next_sync_committee_branch = spec.compute_merkle_proof(attested_state, spec.NEXT_SYNC_COMMITTEE_INDEX) if with_finality: update.finalized_header = spec.block_to_light_client_header(finalized_block) - update.finality_branch = spec.compute_merkle_proof_for_state( - attested_state, spec.FINALIZED_ROOT_INDEX) + update.finality_branch = spec.compute_merkle_proof(attested_state, spec.FINALIZED_ROOT_INDEX) update.sync_aggregate, update.signature_slot = get_sync_aggregate( spec, attested_state, num_participants) From c680212183e67ff5895e6532861f85b4b4a9d557 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:33:49 +0200 Subject: [PATCH 36/49] drop is_valid_merkle_path --- specs/deneb/beacon-chain.md | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/specs/deneb/beacon-chain.md b/specs/deneb/beacon-chain.md index fde75b0ed3..b98ac12592 100644 --- a/specs/deneb/beacon-chain.md +++ b/specs/deneb/beacon-chain.md @@ -25,7 +25,6 @@ - [Helper functions](#helper-functions) - [Misc](#misc) - [`kzg_commitment_to_versioned_hash`](#kzg_commitment_to_versioned_hash) - - [`is_valid_merkle_path`](#is_valid_merkle_path) - [Beacon state accessors](#beacon-state-accessors) - [Modified `get_attestation_participation_flag_indices`](#modified-get_attestation_participation_flag_indices) - [New `get_validator_activation_churn_limit`](#new-get_validator_activation_churn_limit) @@ -188,19 +187,6 @@ def kzg_commitment_to_versioned_hash(kzg_commitment: KZGCommitment) -> Versioned return VERSIONED_HASH_VERSION_KZG + hash(kzg_commitment)[1:] ``` -#### `is_valid_merkle_path` - -```python -def is_valid_merkle_path(leaf: Bytes32, branch: Sequence[Bytes32], gindex: int, root: Root) -> bool: - value = leaf - for i in range(len(branch)): - if (gindex >> i) & 1 == 0: - value = hash(branch[i] + value) - else: - value = hash(value + branch[i]) - return value == root -``` - ### Beacon state accessors #### Modified `get_attestation_participation_flag_indices` From 126e807d193229f56f4e21543dfc14ba3cbf00ea Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:34:51 +0200 Subject: [PATCH 37/49] Update specs/deneb/p2p-interface.md Co-authored-by: Mikhail Kalinin --- specs/deneb/p2p-interface.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 5d640ddf54..65334c740b 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -82,7 +82,7 @@ class BlobSidecar(Container): kzg_commitment: KZGCommitment kzg_proof: KZGProof # Allows for quick verification of kzg_commitment signed_block_header: SignedBeaconBlockHeader - commitment_inclusion_proof: List[Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH] + commitment_inclusion_proof: Vector[Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH] ``` #### `BlobIdentifier` From b803f1ceeb04b5a4e6d0589eb905f86b18f40563 Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:38:48 +0200 Subject: [PATCH 38/49] Update specs/deneb/p2p-interface.md Co-authored-by: Pop Chunhapanya --- specs/deneb/p2p-interface.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 65334c740b..349b4ec8c5 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -48,7 +48,7 @@ The specification of these changes continues in the same format as the network s | Name | Value | Description | |------------------------------------------|-----------------------------------|---------------------------------------------------------------------| -| `BLOB_KZG_COMMITMENTS_GINDEX` | `get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')` (= 27) | `blob_kzg_commitments` field gindex on `BeaconBlockBody` container | +| `BLOB_KZG_COMMITMENTS_GINDEX` | `get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')` (= 27) | `blob_kzg_commitments` field generalized index on `BeaconBlockBody` container | ### Preset From d323f0568fb286d732a3deb66758bf75151a7444 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:37:17 +0200 Subject: [PATCH 39/49] drop sidecar alias --- specs/deneb/p2p-interface.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 349b4ec8c5..b712c1679b 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -156,10 +156,10 @@ New validation: This topic is used to propagate signed blob sidecars, where each blob index maps to some `subnet_id`. -The following validations MUST pass before forwarding the `blob_sidecar` on the network, assuming the alias `sidecar = blob_sidecar` and `block_header = blob_sidecar.signed_block_header.message`: +The following validations MUST pass before forwarding the `blob_sidecar` on the network, assuming the alias `block_header = blob_sidecar.signed_block_header.message`: -- _[REJECT]_ The sidecar's index is consistent with `MAX_BLOBS_PER_BLOCK` -- i.e. `sidecar.index < MAX_BLOBS_PER_BLOCK`. -- _[REJECT]_ The sidecar is for the correct subnet -- i.e. `compute_subnet_for_blob_sidecar(sidecar.index) == subnet_id`. +- _[REJECT]_ The sidecar's index is consistent with `MAX_BLOBS_PER_BLOCK` -- i.e. `blob_sidecar.index < MAX_BLOBS_PER_BLOCK`. +- _[REJECT]_ The sidecar is for the correct subnet -- i.e. `compute_subnet_for_blob_sidecar(blob_sidecar.index) == subnet_id`. - _[IGNORE]_ The sidecar is not from a future slot (with a `MAXIMUM_GOSSIP_CLOCK_DISPARITY` allowance) -- i.e. validate that `block_header.slot <= current_slot` (a client MAY queue future sidecars for processing at the appropriate slot). - _[IGNORE]_ The sidecar is from a slot greater than the latest finalized slot -- i.e. validate that `block_header.slot > compute_start_slot_at_epoch(state.finalized_checkpoint.epoch)` - _[IGNORE]_ The sidecar's block's parent (defined by `block_header.parent_root`) has been seen (via both gossip and non-gossip sources) (a client MAY queue sidecars for processing once the parent block is retrieved). @@ -167,7 +167,7 @@ The following validations MUST pass before forwarding the `blob_sidecar` on the - _[REJECT]_ The sidecar is from a higher slot than the sidecar's block's parent (defined by `block_header.parent_root`). - _[REJECT]_ The proposer signature of `blob_sidecar.signed_block_header`, is valid with respect to the `block_header.proposer_index` pubkey. - _[REJECT]_ The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof`. -- _[IGNORE]_ The sidecar is the first sidecar for the tuple (block_header.slot, block_header.proposer_index, sidecar.index) with valid header signature and sidecar inclusion proof +- _[IGNORE]_ The sidecar is the first sidecar for the tuple (block_header.slot, block_header.proposer_index, blob_sidecar.index) with valid header signature and sidecar inclusion proof - _[REJECT]_ The sidecar is proposed by the expected `proposer_index` for the block's slot in the context of the current shuffling (defined by `block_header.parent_root`/`block_header.slot`). If the `proposer_index` cannot immediately be verified against the expected shuffling, the sidecar MAY be queued for later processing while proposers for the block's branch are calculated -- in such a case _do not_ `REJECT`, instead `IGNORE` this message. From a12441486c5a0ab1376b64b1d6cb8308bee63823 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 30 Oct 2023 17:42:32 +0800 Subject: [PATCH 40/49] Enhance `blob_sidecar_inclusion_proof` tests --- .../unittests/validator/test_validator.py | 62 +++++++++++-------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py index 6cd3011fcb..77ae900671 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py @@ -1,5 +1,5 @@ +import random from eth2spec.test.context import ( - always_bls, spec_state_test, with_deneb_and_later, ) @@ -15,21 +15,30 @@ ) -@with_deneb_and_later -@spec_state_test -def test_blob_sidecar_inclusion_proof_correct(spec, state): - """ - Test `verify_blob_sidecar_inclusion_proof` - """ - blob_count = 4 +def _get_sample_sidecars(spec, state, rng): block = build_empty_block_for_next_slot(spec, state) - opaque_tx, blobs, blob_kzg_commitments, proofs = get_sample_opaque_tx(spec, blob_count=blob_count) - block.body.blob_kzg_commitments = blob_kzg_commitments - block.body.execution_payload.transactions = [opaque_tx] + + # 2 txs, each has 2 blobs + blob_count = 2 + opaque_tx_1, blobs_1, blob_kzg_commitments_1, proofs_1 = get_sample_opaque_tx(spec, blob_count=blob_count, rng=rng) + opaque_tx_2, blobs_2, blob_kzg_commitments_2, proofs_2 = get_sample_opaque_tx(spec, blob_count=blob_count, rng=rng) + assert opaque_tx_1 != opaque_tx_2 + + block.body.blob_kzg_commitments = blob_kzg_commitments_1 + blob_kzg_commitments_2 + block.body.execution_payload.transactions = [opaque_tx_1, opaque_tx_2] block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) + blobs = blobs_1 + blobs_2 + proofs = proofs_1 + proofs_2 signed_block = sign_block(spec, state, block, proposer_index=0) - blob_sidecars = spec.get_blob_sidecars(signed_block, blobs, proofs) + return spec.get_blob_sidecars(signed_block, blobs, proofs) + + +@with_deneb_and_later +@spec_state_test +def test_blob_sidecar_inclusion_proof_correct(spec, state): + rng = random.Random(1234) + blob_sidecars = _get_sample_sidecars(spec, state, rng) for blob_sidecar in blob_sidecars: assert spec.verify_blob_sidecar_inclusion_proof(blob_sidecar) @@ -37,22 +46,23 @@ def test_blob_sidecar_inclusion_proof_correct(spec, state): @with_deneb_and_later @spec_state_test -@always_bls -def test_blob_sidecar_inclusion_proof_incorrect(spec, state): - """ - Test `get_blob_sidecar_signature` - """ - blob_count = 4 - block = build_empty_block_for_next_slot(spec, state) - opaque_tx, blobs, blob_kzg_commitments, proofs = get_sample_opaque_tx(spec, blob_count=blob_count) - block.body.blob_kzg_commitments = blob_kzg_commitments - block.body.execution_payload.transactions = [opaque_tx] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) - - signed_block = sign_block(spec, state, block, proposer_index=0) - blob_sidecars = spec.get_blob_sidecars(signed_block, blobs, proofs) +def test_blob_sidecar_inclusion_proof_incorrect_wrong_body(spec, state): + rng = random.Random(1234) + blob_sidecars = _get_sample_sidecars(spec, state, rng) for blob_sidecar in blob_sidecars: block = blob_sidecar.signed_block_header.message block.body_root = spec.hash(block.body_root) # mutate body root to break proof assert not spec.verify_blob_sidecar_inclusion_proof(blob_sidecar) + + +@with_deneb_and_later +@spec_state_test +def test_blob_sidecar_inclusion_proof_incorrect_wrong_proof(spec, state): + rng = random.Random(1234) + blob_sidecars = _get_sample_sidecars(spec, state, rng) + + for blob_sidecar in blob_sidecars: + # wrong proof + blob_sidecar.commitment_inclusion_proof = spec.compute_merkle_proof(spec.BeaconBlockBody(), 0) + assert not spec.verify_blob_sidecar_inclusion_proof(blob_sidecar) From 51343f54fe73eaffa550d78414faed6e63df5b62 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 30 Oct 2023 18:24:19 +0800 Subject: [PATCH 41/49] Fix typing and delete the `signed_sidecar` --- pysetup/spec_builders/altair.py | 2 +- specs/deneb/validator.md | 8 ++++---- ssz/merkle-proofs.md | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pysetup/spec_builders/altair.py b/pysetup/spec_builders/altair.py index ff667b19ce..abd9b56c40 100644 --- a/pysetup/spec_builders/altair.py +++ b/pysetup/spec_builders/altair.py @@ -27,7 +27,7 @@ def preparations(cls): @classmethod def sundry_functions(cls) -> str: return ''' -def get_generalized_index(ssz_class: Any, *path: Sequence[PyUnion[int, SSZVariableName]]) -> GeneralizedIndex: +def get_generalized_index(ssz_class: Any, *path: PyUnion[int, SSZVariableName]) -> GeneralizedIndex: ssz_path = Path(ssz_class) for item in path: ssz_path = ssz_path / item diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index e5141746a6..cb87e13225 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -163,18 +163,18 @@ def get_blob_sidecars(signed_block: SignedBeaconBlock, blob=blob, kzg_commitment=block.body.blob_kzg_commitments[index], kzg_proof=blob_kzg_proofs[index], + signed_block_header=signed_block_header, commitment_inclusion_proof=compute_merkle_proof( block.body, - get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', index), # type: ignore + get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', index), ), - signed_block_header=signed_block_header, ) for index, blob in enumerate(blobs) ] ``` -The `subnet_id` for the `signed_sidecar` is calculated with: -- Let `blob_index = signed_sidecar.message.index`. +The `subnet_id` for the `blob_sidecar` is calculated with: +- Let `blob_index = blob_sidecar.index`. - Let `subnet_id = compute_subnet_for_blob_sidecar(blob_index)`. ```python diff --git a/ssz/merkle-proofs.md b/ssz/merkle-proofs.md index 6772026fe1..919ff07f59 100644 --- a/ssz/merkle-proofs.md +++ b/ssz/merkle-proofs.md @@ -167,7 +167,7 @@ def get_item_position(typ: SSZType, index_or_variable_name: Union[int, SSZVariab ``` ```python -def get_generalized_index(typ: SSZType, path: Sequence[Union[int, SSZVariableName]]) -> GeneralizedIndex: +def get_generalized_index(typ: SSZType, *path: PyUnion[int, SSZVariableName]) -> GeneralizedIndex: """ Converts a path (eg. `[7, "foo", 3]` for `x[7].foo[3]`, `[12, "bar", "__len__"]` for `len(x[12].bar)`) into the generalized index representing its position in the Merkle tree. From 1bac25a6230c9c509ae1058d35c7dadb2e7a6b01 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 31 Oct 2023 00:53:53 +0800 Subject: [PATCH 42/49] Add Merkle proof test --- .../test/deneb/merkle_proof/__init__.py | 0 .../merkle_proof/test_single_merkle_proof.py | 46 +++++++++++++++++++ tests/formats/merkle_proof/README.md | 4 ++ tests/generators/merkle_proof/README.md | 5 ++ tests/generators/merkle_proof/__init__.py | 0 tests/generators/merkle_proof/main.py | 14 ++++++ .../generators/merkle_proof/requirements.txt | 2 + 7 files changed, 71 insertions(+) create mode 100644 tests/core/pyspec/eth2spec/test/deneb/merkle_proof/__init__.py create mode 100644 tests/core/pyspec/eth2spec/test/deneb/merkle_proof/test_single_merkle_proof.py create mode 100644 tests/formats/merkle_proof/README.md create mode 100644 tests/generators/merkle_proof/README.md create mode 100644 tests/generators/merkle_proof/__init__.py create mode 100644 tests/generators/merkle_proof/main.py create mode 100644 tests/generators/merkle_proof/requirements.txt diff --git a/tests/core/pyspec/eth2spec/test/deneb/merkle_proof/__init__.py b/tests/core/pyspec/eth2spec/test/deneb/merkle_proof/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/core/pyspec/eth2spec/test/deneb/merkle_proof/test_single_merkle_proof.py b/tests/core/pyspec/eth2spec/test/deneb/merkle_proof/test_single_merkle_proof.py new file mode 100644 index 0000000000..bcc25a89d4 --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/deneb/merkle_proof/test_single_merkle_proof.py @@ -0,0 +1,46 @@ +from eth2spec.test.context import ( + spec_state_test, + with_deneb_and_later, + with_test_suite_name, +) +from eth2spec.test.helpers.block import ( + build_empty_block_for_next_slot, + sign_block +) +from eth2spec.test.helpers.execution_payload import ( + compute_el_block_hash, +) +from eth2spec.test.helpers.sharding import ( + get_sample_opaque_tx, +) + + +@with_test_suite_name("BeaconBlockBody") +@with_deneb_and_later +@spec_state_test +def test_blob_kzg_commitment_merkle_proof(spec, state): + opaque_tx, blobs, blob_kzg_commitments, proofs = get_sample_opaque_tx(spec, blob_count=1) + block = build_empty_block_for_next_slot(spec, state) + block.body.blob_kzg_commitments = blob_kzg_commitments + block.body.execution_payload.transactions = [opaque_tx] + block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload) + signed_block = sign_block(spec, state, block, proposer_index=0) + blob_sidecars = spec.get_blob_sidecars(signed_block, blobs, proofs) + blob_index = 0 + blob_sidecar = blob_sidecars[blob_index] + + yield "object", block.body + commitment_inclusion_proof = blob_sidecar.commitment_inclusion_proof + gindex = spec.get_generalized_index(spec.BeaconBlockBody, 'blob_kzg_commitments', blob_index) + yield "proof", { + "leaf": "0x" + blob_sidecar.kzg_commitment.hash_tree_root().hex(), + "leaf_index": gindex, + "branch": ['0x' + root.hex() for root in commitment_inclusion_proof] + } + assert spec.is_valid_merkle_branch( + leaf=blob_sidecar.kzg_commitment.hash_tree_root(), + branch=blob_sidecar.commitment_inclusion_proof, + depth=spec.floorlog2(gindex), + index=spec.get_subtree_index(gindex), + root=blob_sidecar.signed_block_header.message.body_root, + ) diff --git a/tests/formats/merkle_proof/README.md b/tests/formats/merkle_proof/README.md new file mode 100644 index 0000000000..77822daabe --- /dev/null +++ b/tests/formats/merkle_proof/README.md @@ -0,0 +1,4 @@ +# Merkle proof tests + +Handlers: +- `single_merkle_proof`: see [Single leaf merkle proof test format](../light_client/single_merkle_proof.md) diff --git a/tests/generators/merkle_proof/README.md b/tests/generators/merkle_proof/README.md new file mode 100644 index 0000000000..fb4d05fda8 --- /dev/null +++ b/tests/generators/merkle_proof/README.md @@ -0,0 +1,5 @@ +# Merkle proof tests + +The purpose of this test-generator is to provide test-vectors for validating the correct implementation of the Merkle proof verification. + +Test-format documentation can be found [here](../../formats/merkle_proof/README.md). diff --git a/tests/generators/merkle_proof/__init__.py b/tests/generators/merkle_proof/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/generators/merkle_proof/main.py b/tests/generators/merkle_proof/main.py new file mode 100644 index 0000000000..59a0e8ce65 --- /dev/null +++ b/tests/generators/merkle_proof/main.py @@ -0,0 +1,14 @@ +from eth2spec.test.helpers.constants import DENEB +from eth2spec.gen_helpers.gen_from_tests.gen import run_state_test_generators + + +if __name__ == "__main__": + deneb_mods = {key: 'eth2spec.test.deneb.merkle_proof.test_' + key for key in [ + 'single_merkle_proof', + ]} + + all_mods = { + DENEB: deneb_mods, + } + + run_state_test_generators(runner_name="merkle_proof", all_mods=all_mods) diff --git a/tests/generators/merkle_proof/requirements.txt b/tests/generators/merkle_proof/requirements.txt new file mode 100644 index 0000000000..1822486863 --- /dev/null +++ b/tests/generators/merkle_proof/requirements.txt @@ -0,0 +1,2 @@ +pytest>=4.4 +../../../[generator] From 19883ec5922e3cd3b906f08c5293b30e311dbea0 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Tue, 31 Oct 2023 11:39:57 +0200 Subject: [PATCH 43/49] Add verify_blob_kzg_proof condition --- specs/deneb/p2p-interface.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index b712c1679b..586c909e4e 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -167,7 +167,8 @@ The following validations MUST pass before forwarding the `blob_sidecar` on the - _[REJECT]_ The sidecar is from a higher slot than the sidecar's block's parent (defined by `block_header.parent_root`). - _[REJECT]_ The proposer signature of `blob_sidecar.signed_block_header`, is valid with respect to the `block_header.proposer_index` pubkey. - _[REJECT]_ The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof`. -- _[IGNORE]_ The sidecar is the first sidecar for the tuple (block_header.slot, block_header.proposer_index, blob_sidecar.index) with valid header signature and sidecar inclusion proof +- _[REJECT]_ The sidecar's blob is valid as verified by `verify_blob_kzg_proof(blob_sidecar.blob, blob_sidecar.kzg_commitment, blob_sidecar.kzg_proof)`. +- _[IGNORE]_ The sidecar is the first sidecar for the tuple (block_header.slot, block_header.proposer_index, blob_sidecar.index) with valid header signature, sidecar inclusion proof, and kzg proof. - _[REJECT]_ The sidecar is proposed by the expected `proposer_index` for the block's slot in the context of the current shuffling (defined by `block_header.parent_root`/`block_header.slot`). If the `proposer_index` cannot immediately be verified against the expected shuffling, the sidecar MAY be queued for later processing while proposers for the block's branch are calculated -- in such a case _do not_ `REJECT`, instead `IGNORE` this message. From 4a609cec574eca635a9e0980fe448089f8690ce1 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Wed, 1 Nov 2023 15:57:41 +0200 Subject: [PATCH 44/49] rename to kzg_commitment_inclusion_proof --- specs/deneb/p2p-interface.md | 4 ++-- specs/deneb/validator.md | 2 +- .../test/deneb/merkle_proof/test_single_merkle_proof.py | 6 +++--- .../test/deneb/unittests/validator/test_validator.py | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 2cfdcde748..748a3f0d8d 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -82,7 +82,7 @@ class BlobSidecar(Container): kzg_commitment: KZGCommitment kzg_proof: KZGProof # Allows for quick verification of kzg_commitment signed_block_header: SignedBeaconBlockHeader - commitment_inclusion_proof: Vector[Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH] + kzg_commitment_inclusion_proof: Vector[Bytes32, KZG_COMMITMENT_INCLUSION_PROOF_DEPTH] ``` #### `BlobIdentifier` @@ -104,7 +104,7 @@ def verify_blob_sidecar_inclusion_proof(blob_sidecar: BlobSidecar) -> bool: gindex = get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', blob_sidecar.index) return is_valid_merkle_branch( leaf=blob_sidecar.kzg_commitment.hash_tree_root(), - branch=blob_sidecar.commitment_inclusion_proof, + branch=blob_sidecar.kzg_commitment_inclusion_proof, depth=floorlog2(gindex), index=get_subtree_index(gindex), root=blob_sidecar.signed_block_header.message.body_root, diff --git a/specs/deneb/validator.md b/specs/deneb/validator.md index 3dc2e90f0d..3e2c91f817 100644 --- a/specs/deneb/validator.md +++ b/specs/deneb/validator.md @@ -162,7 +162,7 @@ def get_blob_sidecars(signed_block: SignedBeaconBlock, kzg_commitment=block.body.blob_kzg_commitments[index], kzg_proof=blob_kzg_proofs[index], signed_block_header=signed_block_header, - commitment_inclusion_proof=compute_merkle_proof( + kzg_commitment_inclusion_proof=compute_merkle_proof( block.body, get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', index), ), diff --git a/tests/core/pyspec/eth2spec/test/deneb/merkle_proof/test_single_merkle_proof.py b/tests/core/pyspec/eth2spec/test/deneb/merkle_proof/test_single_merkle_proof.py index bcc25a89d4..75bcacc7fc 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/merkle_proof/test_single_merkle_proof.py +++ b/tests/core/pyspec/eth2spec/test/deneb/merkle_proof/test_single_merkle_proof.py @@ -30,16 +30,16 @@ def test_blob_kzg_commitment_merkle_proof(spec, state): blob_sidecar = blob_sidecars[blob_index] yield "object", block.body - commitment_inclusion_proof = blob_sidecar.commitment_inclusion_proof + kzg_commitment_inclusion_proof = blob_sidecar.kzg_commitment_inclusion_proof gindex = spec.get_generalized_index(spec.BeaconBlockBody, 'blob_kzg_commitments', blob_index) yield "proof", { "leaf": "0x" + blob_sidecar.kzg_commitment.hash_tree_root().hex(), "leaf_index": gindex, - "branch": ['0x' + root.hex() for root in commitment_inclusion_proof] + "branch": ['0x' + root.hex() for root in kzg_commitment_inclusion_proof] } assert spec.is_valid_merkle_branch( leaf=blob_sidecar.kzg_commitment.hash_tree_root(), - branch=blob_sidecar.commitment_inclusion_proof, + branch=blob_sidecar.kzg_commitment_inclusion_proof, depth=spec.floorlog2(gindex), index=spec.get_subtree_index(gindex), root=blob_sidecar.signed_block_header.message.body_root, diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py index 77ae900671..7ed88cba45 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/validator/test_validator.py @@ -64,5 +64,5 @@ def test_blob_sidecar_inclusion_proof_incorrect_wrong_proof(spec, state): for blob_sidecar in blob_sidecars: # wrong proof - blob_sidecar.commitment_inclusion_proof = spec.compute_merkle_proof(spec.BeaconBlockBody(), 0) + blob_sidecar.kzg_commitment_inclusion_proof = spec.compute_merkle_proof(spec.BeaconBlockBody(), 0) assert not spec.verify_blob_sidecar_inclusion_proof(blob_sidecar) From 71106f1d6486743cac6ecb415c0f03012398b337 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 2 Nov 2023 21:38:01 +0700 Subject: [PATCH 45/49] Remove `BLOB_KZG_COMMITMENTS_GINDEX` --- presets/mainnet/deneb.yaml | 2 +- presets/minimal/deneb.yaml | 2 +- pysetup/spec_builders/deneb.py | 6 ------ specs/deneb/p2p-interface.md | 11 +++-------- .../test/deneb/unittests/test_config_invariants.py | 3 +++ 5 files changed, 8 insertions(+), 16 deletions(-) diff --git a/presets/mainnet/deneb.yaml b/presets/mainnet/deneb.yaml index 6d2fb4abde..0f56b8bdfa 100644 --- a/presets/mainnet/deneb.yaml +++ b/presets/mainnet/deneb.yaml @@ -8,5 +8,5 @@ FIELD_ELEMENTS_PER_BLOB: 4096 MAX_BLOB_COMMITMENTS_PER_BLOCK: 4096 # `uint64(6)` MAX_BLOBS_PER_BLOCK: 6 -# `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 12 = 17 +# `floorlog2(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 12 = 17 KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 17 diff --git a/presets/minimal/deneb.yaml b/presets/minimal/deneb.yaml index 340e8d8d5f..bc4fe4369a 100644 --- a/presets/minimal/deneb.yaml +++ b/presets/minimal/deneb.yaml @@ -8,5 +8,5 @@ FIELD_ELEMENTS_PER_BLOB: 4096 MAX_BLOB_COMMITMENTS_PER_BLOCK: 16 # `uint64(6)` MAX_BLOBS_PER_BLOCK: 6 -# [customized] `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 4 = 9 +# [customized] `floorlog2(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 4 = 9 KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 9 diff --git a/pysetup/spec_builders/deneb.py b/pysetup/spec_builders/deneb.py index 486018f868..e1ad051849 100644 --- a/pysetup/spec_builders/deneb.py +++ b/pysetup/spec_builders/deneb.py @@ -73,12 +73,6 @@ def hardcoded_custom_type_dep_constants(cls, spec_object) -> Dict[str, str]: 'MAX_BLOB_COMMITMENTS_PER_BLOCK': spec_object.preset_vars['MAX_BLOB_COMMITMENTS_PER_BLOCK'].value, } - @classmethod - def hardcoded_ssz_dep_constants(cls) -> Dict[str, str]: - return { - 'BLOB_KZG_COMMITMENTS_GINDEX': 'GeneralizedIndex(27)', - } - @classmethod def hardcoded_func_dep_presets(cls, spec_object) -> Dict[str, str]: return { diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 748a3f0d8d..f8124c92d3 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -46,17 +46,13 @@ The specification of these changes continues in the same format as the network s *[New in Deneb:EIP4844]* -| Name | Value | Description | -|------------------------------------------|-----------------------------------|---------------------------------------------------------------------| -| `BLOB_KZG_COMMITMENTS_GINDEX` | `get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')` (= 27) | `blob_kzg_commitments` field generalized index on `BeaconBlockBody` container | - ### Preset *[New in Deneb:EIP4844]* | Name | Value | Description | |------------------------------------------|-----------------------------------|---------------------------------------------------------------------| -| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `uint64(floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK))` (= 17) | Merkle proof depth for `blob_kzg_commitments` list item | +| `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` | `uint64(floorlog2(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK))` (= 17) | Merkle proof depth for `blob_kzg_commitments` list item | ### Configuration @@ -101,12 +97,11 @@ class BlobIdentifier(Container): ```python def verify_blob_sidecar_inclusion_proof(blob_sidecar: BlobSidecar) -> bool: - gindex = get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', blob_sidecar.index) return is_valid_merkle_branch( leaf=blob_sidecar.kzg_commitment.hash_tree_root(), branch=blob_sidecar.kzg_commitment_inclusion_proof, - depth=floorlog2(gindex), - index=get_subtree_index(gindex), + depth=KZG_COMMITMENT_INCLUSION_PROOF_DEPTH, + index=get_subtree_index(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', blob_sidecar.index)), root=blob_sidecar.signed_block_header.message.body_root, ) ``` diff --git a/tests/core/pyspec/eth2spec/test/deneb/unittests/test_config_invariants.py b/tests/core/pyspec/eth2spec/test/deneb/unittests/test_config_invariants.py index 087bd63c60..f3fa956d0a 100644 --- a/tests/core/pyspec/eth2spec/test/deneb/unittests/test_config_invariants.py +++ b/tests/core/pyspec/eth2spec/test/deneb/unittests/test_config_invariants.py @@ -20,3 +20,6 @@ def test_networking(spec): assert spec.config.MAX_REQUEST_BLOB_SIDECARS == spec.config.MAX_REQUEST_BLOCKS_DENEB * spec.MAX_BLOBS_PER_BLOCK # Start with the same size, but `BLOB_SIDECAR_SUBNET_COUNT` could potentially increase later. assert spec.config.BLOB_SIDECAR_SUBNET_COUNT == spec.MAX_BLOBS_PER_BLOCK + for i in range(spec.MAX_BLOB_COMMITMENTS_PER_BLOCK): + gindex = spec.get_generalized_index(spec.BeaconBlockBody, 'blob_kzg_commitments', i) + assert spec.floorlog2(gindex) == spec.KZG_COMMITMENT_INCLUSION_PROOF_DEPTH From 3492c0a99a88d43fe3e4fb99f0fade0431822825 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 2 Nov 2023 22:10:06 +0700 Subject: [PATCH 46/49] minor refactoring --- specs/deneb/p2p-interface.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index f8124c92d3..eedc2e661d 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -97,11 +97,12 @@ class BlobIdentifier(Container): ```python def verify_blob_sidecar_inclusion_proof(blob_sidecar: BlobSidecar) -> bool: + gindex = get_subtree_index(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', blob_sidecar.index)) return is_valid_merkle_branch( leaf=blob_sidecar.kzg_commitment.hash_tree_root(), branch=blob_sidecar.kzg_commitment_inclusion_proof, depth=KZG_COMMITMENT_INCLUSION_PROOF_DEPTH, - index=get_subtree_index(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments', blob_sidecar.index)), + index=gindex, root=blob_sidecar.signed_block_header.message.body_root, ) ``` From 7118c30b672eb5654926c580f903f8bcdfb989b1 Mon Sep 17 00:00:00 2001 From: danny Date: Thu, 2 Nov 2023 09:40:49 -0600 Subject: [PATCH 47/49] a few cleanups to sidecar gossip conditions --- specs/deneb/p2p-interface.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index eedc2e661d..532246ecef 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -158,12 +158,12 @@ The following validations MUST pass before forwarding the `blob_sidecar` on the - _[REJECT]_ The sidecar is for the correct subnet -- i.e. `compute_subnet_for_blob_sidecar(blob_sidecar.index) == subnet_id`. - _[IGNORE]_ The sidecar is not from a future slot (with a `MAXIMUM_GOSSIP_CLOCK_DISPARITY` allowance) -- i.e. validate that `block_header.slot <= current_slot` (a client MAY queue future sidecars for processing at the appropriate slot). - _[IGNORE]_ The sidecar is from a slot greater than the latest finalized slot -- i.e. validate that `block_header.slot > compute_start_slot_at_epoch(state.finalized_checkpoint.epoch)` +- _[REJECT]_ The proposer signature of `blob_sidecar.signed_block_header`, is valid with respect to the `block_header.proposer_index` pubkey. - _[IGNORE]_ The sidecar's block's parent (defined by `block_header.parent_root`) has been seen (via both gossip and non-gossip sources) (a client MAY queue sidecars for processing once the parent block is retrieved). - _[REJECT]_ The sidecar's block's parent (defined by `block_header.parent_root`) passes validation. - _[REJECT]_ The sidecar is from a higher slot than the sidecar's block's parent (defined by `block_header.parent_root`). -- _[REJECT]_ The current finalized_checkpoint is an ancestor of the sidecar's block's parent -- i.e. `get_checkpoint_block(store, sidecar.block_parent_root, store.finalized_checkpoint.epoch) == store.finalized_checkpoint.root`. -- _[REJECT]_ The proposer signature of `blob_sidecar.signed_block_header`, is valid with respect to the `block_header.proposer_index` pubkey. -- _[REJECT]_ The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof`. +- _[REJECT]_ The current finalized_checkpoint is an ancestor of the sidecar's block -- i.e. `get_checkpoint_block(store, block_header.parent_root, store.finalized_checkpoint.epoch) == store.finalized_checkpoint.root`. +- _[REJECT]_ The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof(blob_sidecar)`. - _[REJECT]_ The sidecar's blob is valid as verified by `verify_blob_kzg_proof(blob_sidecar.blob, blob_sidecar.kzg_commitment, blob_sidecar.kzg_proof)`. - _[IGNORE]_ The sidecar is the first sidecar for the tuple (block_header.slot, block_header.proposer_index, blob_sidecar.index) with valid header signature, sidecar inclusion proof, and kzg proof. - _[REJECT]_ The sidecar is proposed by the expected `proposer_index` for the block's slot in the context of the current shuffling (defined by `block_header.parent_root`/`block_header.slot`). From e0b87a665d9e714f2ec334f63f503fb5ef6ddc12 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 2 Nov 2023 22:41:47 +0700 Subject: [PATCH 48/49] Bump v1.4.0-beta.4 --- tests/core/pyspec/eth2spec/VERSION.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/VERSION.txt b/tests/core/pyspec/eth2spec/VERSION.txt index 71f00e8222..744da44f28 100644 --- a/tests/core/pyspec/eth2spec/VERSION.txt +++ b/tests/core/pyspec/eth2spec/VERSION.txt @@ -1 +1 @@ -1.4.0-beta.3 +1.4.0-beta.4 From e21ed6de4f81b0ce9115694753f12f19fafd5fc0 Mon Sep 17 00:00:00 2001 From: danny Date: Thu, 2 Nov 2023 10:03:56 -0600 Subject: [PATCH 49/49] clarify/fix things in blob requests --- specs/deneb/p2p-interface.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/specs/deneb/p2p-interface.md b/specs/deneb/p2p-interface.md index 17e673c50e..0e53af83a3 100644 --- a/specs/deneb/p2p-interface.md +++ b/specs/deneb/p2p-interface.md @@ -286,10 +286,7 @@ Requests sidecars by block root and index. The response is a list of `BlobSidecar` whose length is less than or equal to the number of requests. It may be less in the case that the responding peer is missing blocks or sidecars. -The response is unsigned, i.e. `BlobSidecar`, as the signature of the beacon block proposer -may not be available beyond the initial distribution via gossip. - -Before consuming the next response chunk, the response reader SHOULD verify the blob sidecar is well-formatted and correct w.r.t. the expected KZG commitments through `verify_blob_kzg_proof_batch`. +Before consuming the next response chunk, the response reader SHOULD verify the blob sidecar is well-formatted, has valid inclusion proof, and is correct w.r.t. the expected KZG commitments through `verify_blob_kzg_proof`. No more than `MAX_REQUEST_BLOB_SIDECARS` may be requested at a time. @@ -334,9 +331,7 @@ Response Content: Requests blob sidecars in the slot range `[start_slot, start_slot + count)`, leading up to the current head block as selected by fork choice. -The response is unsigned, i.e. `BlobSidecarsByRange`, as the signature of the beacon block proposer may not be available beyond the initial distribution via gossip. - -Before consuming the next response chunk, the response reader SHOULD verify the blob sidecar is well-formatted and correct w.r.t. the expected KZG commitments through `verify_blob_kzg_proof_batch`. +Before consuming the next response chunk, the response reader SHOULD verify the blob sidecar is well-formatted, has valid inclusion proof, and is correct w.r.t. the expected KZG commitments through `verify_blob_kzg_proof`. `BlobSidecarsByRange` is primarily used to sync blobs that may have been missed on gossip and to sync within the `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS` window.