Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[VC] Add builderBoostFactor support. #6294

Merged
merged 7 commits into from
May 19, 2024
Merged

[VC] Add builderBoostFactor support. #6294

merged 7 commits into from
May 19, 2024

Conversation

cheatfate
Copy link
Contributor

No description provided.

@cheatfate cheatfate requested a review from tersec May 16, 2024 11:37
builderValue: UInt256, engineValue: Wei): bool =
if builderBoostFactor == 0'u64:
false
elif builderBoostFactor == 100'u64:
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like a special case which shouldn't be needed. Possibly slightly faster, but also means that the interesting codepath isn't executed as much.

Copy link
Contributor Author

@cheatfate cheatfate May 16, 2024

Choose a reason for hiding this comment

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

According to

builder_boost_factor

Percentage multiplier to apply to the builder's payload value when choosing between a builder payload header and payload from the paired execution node. This parameter is only relevant if the beacon node is connected to a builder, deems it safe to produce a builder payload, and receives valid responses from both the builder endpoint and the paired execution node. When these preconditions are met, the server MUST act as follows:

if exec_node_payload_value >= builder_boost_factor * (builder_payload_value // 100), then return a full (unblinded) block containing the execution node payload.
otherwise, return a blinded block containing the builder payload header.

Servers must support the following values of the boost factor which encode common preferences:

builder_boost_factor=0: prefer the execution node payload unless an error makes it unviable.
builder_boost_factor=100: default profit maximization mode; choose whichever payload pays more.
builder_boost_factor=2**64 - 1: prefer the builder payload unless an error or beacon node health check makes it unviable.

Servers should use saturating arithmetic or another technique to ensure that large values of the builder_boost_factor do not trigger overflows or errors. If this parameter is provided and the beacon node is not configured with a builder then the beacon node MUST respond with a full block, which the caller can choose to reject if it wishes. If this parameter is not provided then it should be treated as having the default value of 100. If the value is provided but out of range for a 64-bit unsigned integer, then an error response with status code 400 MUST be returned.

In case of 100 we just multiplying with 1, so i do not see any reasons to make multiplication and overflow check which involves division.

Copy link

github-actions bot commented May 16, 2024

Unit Test Results

         9 files  ±0    1 322 suites  +3   32m 22s ⏱️ +28s
  4 983 tests +1    4 635 ✔️ +1  348 💤 ±0  0 ±0 
20 805 runs  +3  20 401 ✔️ +3  404 💤 ±0  0 ±0 

Results for commit 6a8f10f. ± Comparison against base commit 4355f81.

♻️ This comment has been updated with latest results.

@cheatfate cheatfate changed the title [BN+VC] Replace localBlockValueBoost for builderBoostFactor. [VC] Add builderBoostFactor support. May 16, 2024
@tersec tersec merged commit d7c5bc0 into unstable May 19, 2024
14 checks passed
@tersec tersec deleted the vc-boost-factor branch May 19, 2024 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants