Closed
Conversation
This comment has been minimized.
This comment has been minimized.
Member
|
Hey @nerolation please run |
potuz
reviewed
Jul 31, 2025
| # Update the builder payment weight | ||
| if flag_index == TIMELY_HEAD_FLAG_INDEX and is_attestation_same_slot(state, data): | ||
| payment.weight += state.validators[index].effective_balance | ||
| elif flag_index == TIMELY_TARGET_FLAG_INDEX and is_attestation_same_slot(state, data): |
Contributor
There was a problem hiding this comment.
This will add the payment twice, once for the TIMELY_HEAD_FLAG_INDEX and another one for the TIMELY_TARGET_FLAG_INDEX. Perhaps the simplest is to just remove the if/elif statements and just do this:
if weight == 0 and is_attestation_same_slot(state, data):
is_correct_block = data.beacon_block_root == get_block_root_at_slot(state, Slot(data.slot))
is_correct_payload = data.index == 0 # Same-slot attestations must have index=0
if is_correct_block and is_correct_payload:
weight = state.validators[index].effective_balance
And then outside of the flag loop payment.weight += weight
Contributor
Author
There was a problem hiding this comment.
It might be confusing that we use payment.weight and have a separate weight variable for the attestation weights
Contributor
Author
There was a problem hiding this comment.
weight is never 0
This PR:
- Reverts the fork-choice to today's behavior in terms of boost
(proposer boost = 40, no other boosts), and by removing (block, slot)
voting
- Starting from today's fork-choice, it allows for decoupled beacon
block and payload by working on a tree of `ForkChoiceNode`s instead of
blocks.
- A `ForkChoiceNode` has `(root, payload_status)`, where
`payload_status` is either `PENDING`, `EMPTY` or `FULL`.
Pending is for the beacon block *before making a decision on the
payload*, the other two for the payload decision. As long as a `PENDING`
`ForkChoiceNode` is chosen by `get_head`, the corresponding beacon block
is part of the canonical chain, though the payload may or may not be
part of it.
- `FULL` and `EMPTY` are considered children of the `PENDING`
`ForkChoiceNode`, so all weight from the `EMPTY`
and `FULL` variants is inherited by the `PENDING` variant.
- the `FULL` variant is only considered if the payload is locally
available (is imported in `store.execution_payload_states`, which
requires satisfying `is_data_available`)
- When running `get_head`, we alternate choosing between beacon blocks,
i.e., `PENDING` `ForkChoiceNode`s, and then making a a decision about
the `EMPTY` and `FULL` children of the current beacon block:
```
(B, PENDING)
├── (B, EMPTY)
└── (B, FULL)
(B, PENDING)
├─→ (B, EMPTY)
│ ├─→ (C, PENDING)
│ │ ├─→ (C, EMPTY)
│ │ └─→ (C, FULL)
│ └─→ (D, PENDING)
└X→ (B, FULL) (unavailable)
```
- Still,`get_head` remains nearly identical to today, by adding a
`get_fork_choice_children` function that substitutes the current
explicit choice of children of the current `head` and abstracts away the
alternating logic of choosing between empty/full payloads and actual
beacon blocks
- `LatestMessage`s are augmented with a `payload_present` boolean
recording the payload presence indicated by (the overloading of)
`data.index` (set to `data.index == 1`). This is useful when `data.slot
> block.slot`, when the attestation expresses an opinion on the
payload's presence, as it tells us whether to assign weight to the
`EMPTY` or `FULL` variant (which also trickles down to the `PENDING`
variant since they are children of it), whereas it is irrelevant when
`data.slot == block.slot` (and should always be set to `False` because
`data.index` can only be `0` then)
- `should_extend_payload` handles the remaining decision, about the
payload status of a block *from the previous slot*, when we don't have
any regular attestations *with information about the payload*. If the
payload is locally available, the `FULL` variant is chosen:
- Whenever a majority of the PTC has voted for it
- If the current proposer has extended anything but the EMPTY variant
In other words, the next proposer is allowed to reorg the previous
payload only if it receives less than half of the PTC votes, and
otherwise we always default to FULL
---------
Co-authored-by: Justin Traglia <95511699+jtraglia@users.noreply.github.com>
Co-authored-by: Potuz <potuz@prysmaticlabs.com>
- Add specification on constructing `slot` and `execution_requests` in execution payload envelope - Remove spec related to `payload_withheld` --------- Co-authored-by: Justin Traglia <jtraglia@pm.me>
For consistency, we should state if these functions are new or modified.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continuing from my discord message here:
In line 1030, the payment weight is only incremented when
flag_index == TIMELY_HEAD_FLAG_INDEX. ButTIMELY_HEAD_FLAG_INDEXis only set (lines 503–504) wheninclusion_delay == IN_ATTESTATION_INCLUSION_DELAY, which means the attestation must be included in the next slot.If the next slot is missed (no block proposed), attestations from the previous slot cannot be included with
inclusion_delay == 1, so they won't haveTIMELY_HEAD_FLAG_INDEXset. This means their weight won't count toward the quorum, potentially preventing the builder from being required to make payment, even if the proposer's block received substantial attestations.