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

log engine/builder API decisionmaking #5608

Merged
merged 1 commit into from
Dec 15, 2023
Merged

log engine/builder API decisionmaking #5608

merged 1 commit into from
Dec 15, 2023

Conversation

tersec
Copy link
Contributor

@tersec tersec commented Nov 19, 2023

If this PR is to be useful, it's not clear to me whether it has as much value at debug as info.

The general goal here is to clarify for for Nimbus end-users and for developers if/when Nimbus is properly comparing engine/builder bids and when it's inadvertently selecting one because the other didn't work. It furthermore shows even when selecting one, if both arrived, the value of the other bid as well.

I don't view this as critical functionality, but might be nice to have, and came up during some discussions/apropos concerns about what happened when Nimbus sometimes did not use a configured builder AP relay but fell back to the engine API

General reasons this can occur are relays being down, or more commonly, returning HTTP 204, indicating nothing interesting to suggest for that slot. It can also happen that they do suggest some block (header), but it's not particularly valuable.

The latter becomes potentially more interesting as the local block value boost is increased, because usually in a direct comparison, the builders and relays will create higher-value blocks than local EL clients, but might not if they have to be 15%+ higher-value.

Copy link

github-actions bot commented Nov 19, 2023

Unit Test Results

         9 files  ±0    1 098 suites  ±0   27m 17s ⏱️ -29s
  3 965 tests ±0    3 618 ✔️ ±0  347 💤 ±0  0 ±0 
16 090 runs  ±0  15 692 ✔️ ±0  398 💤 ±0  0 ±0 

Results for commit 6bc463b. ± Comparison against base commit df902fd.

♻️ This comment has been updated with latest results.

@tersec tersec force-pushed the KPv branch 2 times, most recently from c1c4fd3 to dff00c4 Compare December 1, 2023 18:59
@tersec tersec force-pushed the KPv branch 2 times, most recently from 3f08488 to 870712b Compare December 6, 2023 15:15
@tersec
Copy link
Contributor Author

tersec commented Dec 11, 2023

Of note, Lighthouse has something like this: sigp/lighthouse#4352

@zah zah merged commit cb6b54e into unstable Dec 15, 2023
11 checks passed
@zah zah deleted the KPv branch December 15, 2023 20:31
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.

2 participants