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

Enhance block by root log #13472

Merged
merged 1 commit into from
Jan 18, 2024
Merged

Conversation

terencechain
Copy link
Member

Currently, if a block's parent or attestation head vote is not in the database, the beacon node logs a request message for it. This message can be either "Requesting block for pending attestation" or "Requesting parent block"'. However, these logs are misleading because they are generated before the requested root is filtered by s.seenPendingBlocks[r] || s.cfg.chain.BlockBeingSynced(r). It would be more accurate to log these messages after applying the filter. Although logging later in the function means losing information like attSlot, attCount, or slot, these details are likely irrelevant for a debug log, and changing the debug log format should not be a concern...

@terencechain terencechain added the Ready For Review A pull request ready for code review label Jan 16, 2024
@terencechain terencechain requested a review from a team as a code owner January 16, 2024 20:06
@terencechain terencechain force-pushed the enhance-request-block-by-root-log branch 2 times, most recently from a2db288 to 0f37570 Compare January 16, 2024 20:09
@terencechain terencechain force-pushed the enhance-request-block-by-root-log branch from 0f37570 to af0d9a3 Compare January 17, 2024 18:43
@terencechain terencechain added this pull request to the merge queue Jan 18, 2024
Merged via the queue into develop with commit f3ef1b6 Jan 18, 2024
17 checks passed
@terencechain terencechain deleted the enhance-request-block-by-root-log branch January 18, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants