Include block root in publish block logs #8111

dapplion · 2025-09-24T13:32:27Z

Issue Addressed

Debugging #8104 it would have been helpful to quickly see in the logs that a specific block was submitted into the HTTP API.

Because we want to optimize the block root computation we don't include it in the logs, and just log the block slot. I believe we can take a minute performance hit to have the block root in all the logs during block publishing.

jimmygchen · 2025-09-24T13:41:08Z

beacon_node/http_api/src/publish_blocks.rs

    current_span.record("provenance", provenance);

    let block = unverified_block.inner_block();
+    let block_root = block.canonical_root();


We can do block_root.unwrap_or_else here, as block root is already calculated when calling from the publish blinded block path.

If you record this to the span, the block_root field will be available to both the span and all logs in this function:

current_span.record("block_root", ?block_root);

mergify · 2025-10-25T03:17:22Z

Hi @dapplion, this pull request has been closed automatically due to 30 days of inactivity. If you’d like to continue working on it, feel free to reopen at any time.

jimmygchen · 2025-11-05T00:30:20Z

Hey @dapplion I think this is useful so I'm going to revive this one!

…-log

dapplion

✅ looks good! Thanks for continuing it @jimmygchen

@michaelsproul

* Optimise `state_root_at_slot` for finalized slot (sigp#8353) This is an optimisation targeted at Fulu networks in non-finality. While debugging on Holesky, we found that `state_root_at_slot` was being called from `prepare_beacon_proposer` a lot, for the finalized state: https://github.com/sigp/lighthouse/blob/2c9b670f5d313450252c6cb40a5ee34802d54fef/beacon_node/http_api/src/lib.rs#L3860-L3861 This was causing `prepare_beacon_proposer` calls to take upwards of 5 seconds, sometimes 10 seconds, because it would trigger _multiple_ beacon state loads in order to iterate back to the finalized slot. Ideally, loading the finalized state should be quick because we keep it cached in the state cache (technically we keep the split state, but they usually coincide). Instead we are computing the finalized state root separately (slow), and then loading the state from the cache (fast). Although it would be possible to make the API faster by removing the `state_root_at_slot` call, I believe it's simpler to change `state_root_at_slot` itself and remove the footgun. Devs rightly expect operations involving the finalized state to be fast. Co-Authored-By: Michael Sproul <michael@sigmaprime.io> * Remove Windows CI jobs (sigp#8362) Remove all Windows-related CI jobs Co-Authored-By: antondlr <anton@sigmaprime.io> * Update proposer-only section in the documentation (sigp#8358) Co-Authored-By: Tan Chee Keong <tanck@sigmaprime.io> Co-Authored-By: Michael Sproul <michaelsproul@users.noreply.github.com> * Fix unaggregated delay metric (sigp#8366) while working on this sigp#7892 @michaelsproul pointed it might be a good metric to measure the delay from start of the slot instead of the current `slot_duration / 3`, since the attestations duties start before the `1/3rd` mark now with the change in the link PR. Co-Authored-By: hopinheimer <knmanas6@gmail.com> Co-Authored-By: hopinheimer <48147533+hopinheimer@users.noreply.github.com> * Downgrade and remove unnecessary logs (sigp#8367) ### Downgrade a non error to `Debug` I noticed this error on one of our hoodi nodes: ``` Nov 04 05:13:38.892 ERROR Error during data column reconstruction block_root: 0x4271b9efae7deccec3989bd2418e998b83ce8144210c2b17200abb62b7951190, error: DuplicateFullyImported(0x4271b9efae7deccec3989bd2418e998b83ce8144210c2b17200abb62b7951190) ``` This shouldn't be logged as an error and it's due to a normal race condition, and it doesn't impact the node negatively. ### Remove spammy logs This logs is filling up the log files quite quickly and it is also something we'd expect during normal operation - getting columns via EL before gossip. We haven't found this debug log to be useful, so I propose we remove it to avoid spamming debug logs. ``` Received already available column sidecar. Ignoring the column sidecar ``` In the process of removing this, I noticed we aren't propagating the validation result, which I think we should so I've added this. The impact should be quite minimal - the message will stay in the gossip memcache for a bit longer but should be evicted in the next heartbeat. Co-Authored-By: Jimmy Chen <jchen.tc@gmail.com> * Prepare `sensitive_url` for `crates.io` (sigp#8223) Another good candidate for publishing separately from Lighthouse is `sensitive_url` as it's a general utility crate and not related to Ethereum. This PR prepares it to be spun out into its own crate. I've made the `full` field on `SensitiveUrl` private and instead provided an explicit getter called `.expose_full()`. It's a bit ugly for the diff but I prefer the explicit nature of the getter. I've also added some extra tests and doc strings along with feature gating `Serialize` and `Deserialize` implementations behind the `serde` feature. Co-Authored-By: Mac L <mjladson@pm.me> * Remove ecdsa feature of libp2p (sigp#8374) This compiles, is there any reason to keep `ecdsa`? CC @jxs Co-Authored-By: Michael Sproul <michael@sigmaprime.io> * CI workflows to use warpbuild ci runner (sigp#8343) Self hosted GitHub Runners review and improvements local testnet workflow now uses warpbuild ci runner Co-Authored-By: lemon <snyxmk@gmail.com> Co-Authored-By: antondlr <anton@sigmaprime.io> * Remove `sensitive_url` and import from `crates.io` (sigp#8377) Use the recently published `sensitive_url` and remove it from Lighthouse Co-Authored-By: Mac L <mjladson@pm.me> * Migrate derivative to educe (sigp#8125) Fixes sigp#7001. Mostly mechanical replacement of `derivative` attributes with `educe` ones. ### **Attribute Syntax Changes** ```rust // Bounds: = "..." → (...) #[derivative(Hash(bound = "E: EthSpec"))] #[educe(Hash(bound(E: EthSpec)))] // Ignore: = "ignore" → (ignore) #[derivative(PartialEq = "ignore")] #[educe(PartialEq(ignore))] // Default values: value = "..." → expression = ... #[derivative(Default(value = "ForkName::Base"))] #[educe(Default(expression = ForkName::Base))] // Methods: format_with/compare_with = "..." → method(...) #[derivative(Debug(format_with = "fmt_peer_set_as_len"))] #[educe(Debug(method(fmt_peer_set_as_len)))] // Empty bounds: removed entirely, educe can infer appropriate bounds #[derivative(Default(bound = ""))] #[educe(Default)] // Transparent debug: manual implementation (educe doesn't support it) #[derivative(Debug = "transparent")] // Replaced with manual Debug impl that delegates to inner field ``` **Note**: Some bounds use strings (`bound("E: EthSpec")`) for superstruct compatibility (`expected ','` errors). Co-Authored-By: Javier Chávarri <javier.chavarri@gmail.com> Co-Authored-By: Mac L <mjladson@pm.me> * Fix flaky reconstruction test (sigp#8321) FIx flaky tests that depends on timing. Previously the test processes all 128 columns and expect reconstruction to happen after all columns are processed. There is a race here, and reconstruction could be triggered before all columns are processed. I've updated the tests to process 64 columns, just enough for reconstruction and wait for 50ms for reconstruction to be triggered. This PR requires the change made in sigp#8194 for the test to pass consistently (blob count set to 1 for all blocks instead of random blob count between 0..max) Co-Authored-By: Jimmy Chen <jchen.tc@gmail.com> Co-Authored-By: Jimmy Chen <jimmy@sigmaprime.io> * Remove `ethers-core` from `execution_layer` (sigp#8149) sigp#6022 Use `alloy_rpc_types::Transaction` to replace the `ethers_core::Transaction` inside the execution block generator. Co-Authored-By: Mac L <mjladson@pm.me> * Include block root in publish block logs (sigp#8111) Debugging sigp#8104 it would have been helpful to quickly see in the logs that a specific block was submitted into the HTTP API. Because we want to optimize the block root computation we don't include it in the logs, and just log the block slot. I believe we can take a minute performance hit to have the block root in all the logs during block publishing. Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com> Co-Authored-By: Jimmy Chen <jchen.tc@gmail.com> * fix: clarify `bb` vs `bl` variable names in BeaconProcessorQueue (sigp#8315) since block and blob both start with `bl`, it was not clear how to differentiate between `blbroots_queue` and `bbroots_queue` After renaming, there also seems to be a discrepancy Co-Authored-By: Kevaundray Wedderburn <kevtheappdev@gmail.com> * Migrate the `deposit_contract` crate to `alloy` (sigp#8139) sigp#6022 Switches the `deposit_contract` crate to use the `alloy` ecosystem and removes the dependency on `ethabi` Co-Authored-By: Mac L <mjladson@pm.me> --------- Co-authored-by: Michael Sproul <michaelsproul@users.noreply.github.com> Co-authored-by: Michael Sproul <michael@sigmaprime.io> Co-authored-by: antondlr <anton@sigmaprime.io> Co-authored-by: chonghe <44791194+chong-he@users.noreply.github.com> Co-authored-by: hopinheimer <48147533+hopinheimer@users.noreply.github.com> Co-authored-by: Jimmy Chen <jimmy@sigmaprime.io> Co-authored-by: Jimmy Chen <jchen.tc@gmail.com> Co-authored-by: Mac L <mjladson@pm.me> Co-authored-by: lmnzx <snyxmk@gmail.com> Co-authored-by: Javier Chávarri <javier.chavarri@gmail.com> Co-authored-by: Lion - dapplion <35266934+dapplion@users.noreply.github.com>

Include block root in import log

69c03f3

dapplion changed the title ~~Include block root in import log~~ Include block root in publish block logs Sep 24, 2025

jimmygchen reviewed Sep 24, 2025

View reviewed changes

jimmygchen added waiting-on-author The reviewer has suggested changes and awaits thier implementation. UX-and-logs tracing labels Sep 25, 2025

mergify bot closed this Oct 25, 2025

mergify bot added the stale Stale PRs that have been inactive and is now outdated label Oct 25, 2025

jimmygchen reopened this Nov 5, 2025

jimmygchen added 2 commits November 5, 2025 11:37

Merge remote-tracking branch 'origin/unstable' into block-import-root…

71816fc

…-log

Record block root and slot on publish_block span.

8bd2211

jimmygchen added ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Nov 5, 2025

dapplion commented Nov 10, 2025

View reviewed changes

jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Nov 11, 2025

jimmygchen approved these changes Nov 11, 2025

View reviewed changes

mergify bot added the queued label Nov 11, 2025

mergify bot added a commit that referenced this pull request Nov 11, 2025

Merge of #8111

785d4fc

mergify bot mentioned this pull request Nov 11, 2025

merge queue: embarking unstable (93b8f46) and #8111 together #8394

Closed

6 tasks

mergify bot merged commit 22dea2b into sigp:unstable Nov 11, 2025
37 checks passed

mergify bot removed the queued label Nov 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Include block root in publish block logs #8111

Include block root in publish block logs #8111

Uh oh!

dapplion commented Sep 24, 2025

Uh oh!

jimmygchen Sep 24, 2025

Uh oh!

mergify bot commented Oct 25, 2025

Uh oh!

jimmygchen commented Nov 5, 2025

Uh oh!

dapplion left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Include block root in publish block logs #8111

Include block root in publish block logs #8111

Uh oh!

Conversation

dapplion commented Sep 24, 2025

Issue Addressed

Uh oh!

jimmygchen Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Oct 25, 2025

Uh oh!

jimmygchen commented Nov 5, 2025

Uh oh!

dapplion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants