Add allowed and reserved list of builder versions#5422
Conversation
|
The way this is written is a bit confusing to me. It reads like there's a single allowed version and a single reserved version. Also, generic constants for builders shouldn't include "payload" in the name. To be clear, maybe there should be |
| assert len(post_state.builders) == 1 | ||
| assert post_state.builders[0].pubkey == builder_pubkey | ||
| assert post_state.builders[0].version == post_spec.PAYLOAD_BUILDER_VERSION | ||
| assert post_state.builders[0].version == spec.uint8(spec.PAYLOAD_BUILDERS_RESERVED_VERSION) |
There was a problem hiding this comment.
There's an outdated reference to PAYLOAD_BUILDERS_RESERVED_VERSION here.
| assert is_active_builder(state, builder_index) | ||
| # Verify that the builder is a payload builder | ||
| assert state.builders[builder_index].version == PAYLOAD_BUILDER_VERSION | ||
| assert state.builders[builder_index].version <= BUILDER_MAX_VERSION |
There was a problem hiding this comment.
I do not like that we allow builderV1 to submit execution payload bids; only builderV0 should be able to do this. We do not know what builderV1 will be responsible for. I suspect builderV1 will be added with mandatory execution proofs, when there needs to be some in-protocol entity responsible for making proofs.
There was a problem hiding this comment.
To be clear, I would like for us to do this:
| assert state.builders[builder_index].version <= BUILDER_MAX_VERSION | |
| assert state.builders[builder_index].version == uint8(0) |
There was a problem hiding this comment.
Yeah, agree with this, having some allowlist to try to anticipate a scenario that ends up as pure technical dead weight unless it's exactly matched, and therefore also require some reserved list, seems excessive.
The explanation that
Rationale for the reserved list:
It allows the protocol to make certain that no builder with version larger than
PAYLOAD_BUILDER_RESERVED_VERSIONexists on state, thus, if in future forks we want to create a special type of builder that requires that no such builder existed, we still have this option.Rationale for the allowed list.
It allows the protocol to onboard builders with different versions that are able to send bids during the current fork. Thus, if we ever want to create some custom type of builders in the future that requires at fork N + 1 to have had landed a bid on fork N, this can be achieved.
is backwards: without the allowed list to begin with, the reserved list is pointless. So the motivating structure of this PR is that it wants an allowed list to happen. Without that the entire thing dissolves, including the reserved list.
| prefix = uint8(request.withdrawal_credentials[0]) | ||
| is_valid_version = prefix >= BUILDER_MIN_WITHDRAWAL_PREFIX | ||
| version = prefix - BUILDER_MIN_WITHDRAWAL_PREFIX | ||
| if is_valid_builder_deposit_signature(request) and is_valid_version: |
There was a problem hiding this comment.
We should swap these so the cheap one is performed first.
| if is_valid_builder_deposit_signature(request) and is_valid_version: | |
| if is_valid_version and is_valid_builder_deposit_signature(request): |
|
|
||
| | Name | Value | | ||
| | ------------------------- | ---------- | | ||
| | `PAYLOAD_BUILDER_VERSION` | `uint8(0)` | |
There was a problem hiding this comment.
I'm okay with deleting this constant for now, but we may introduce new constants for this later when another builder version is added. It's difficult to name these when we don't know what the future holds.
3 participants
This PR adds two different lists. A list of allowed builder versions and a list of reserved builder versions.
Any builder deposit for a version that is reserved is ignored at deposit processing time. This means that a builder that sends a version that is larger than
PAYLOAD_BUILDER_RESERVED_VERSIONwill lose its deposit.Any builder with a version that is above
PAYLOAD_BUILDER_ALLOWED_VERSIONwill not be able to land bids on chain. Their bids are ignored over P2P and blocks that include such bids are invalid.Rationale for the reserved list:
It allows the protocol to make certain that no builder with version larger than
PAYLOAD_BUILDER_RESERVED_VERSIONexists on state, thus, if in future forks we want to create a special type of builder that requires that no such builder existed, we still have this option.Rationale for the allowed list.
It allows the protocol to onboard builders with different versions that are able to send bids during the current fork. Thus, if we ever want to create some custom type of builders in the future that requires at fork N + 1 to have had landed a bid on fork N, this can be achieved.
Rationale for these lists to be different
We want allowed to be a strict subset of not reserved to
Protocols that prove against beacon block roots have guarantees that the bid's builder version lives on each of the above lists because of these features and because all these lists are separated: