Uh oh!
There was an error while loading. Please reload this page.
Channelmanager: improve block connection logging - #4586
Conversation
Upgrade best_block_updated log from TRACE to INFO so chain tip updates are visible without enabling full trace logging. Add per-txid DEBUG logs in transactions_confirmed to make it easier to identify which transactions triggered channel updates.
I've assigned @valentinewallace as a reviewer! |
| for (_, tx) in txdata.iter() { | ||
| log_debug!(self.logger, "Confirmed transaction {} in block {} at height {}", tx.compute_txid(), block_hash, height); | ||
| } |
There was a problem hiding this comment.
compute_txid() performs a double-SHA256 over the serialized transaction, and the log_given_level! macro evaluates its arguments unconditionally (the level check is inside the Logger::log implementation, not before argument evaluation). This means the loop runs and hashes every transaction even when the logger's level is above DEBUG.
Consider either:
- Guarding the loop behind a level check, or
- Folding these per-txid details into the existing TRACE log, or
- Using a lazy wrapper (e.g., a
Displayimpl that computes onfmt) — though this still wouldn't avoid the loop itself.
While txdata only contains matching transactions (not the full block), computing txids unconditionally on every block connection is unnecessary overhead for users who don't have DEBUG enabled.
Both prior issues are resolved in the latest commit:
No new issues found. The code is clean. No issues found. Both previously flagged issues have been addressed in the latest commit (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #4586 +/- ##
==========================================
+ Coverage 86.99% 87.16% +0.17%
==========================================
Files 163 161 -2 Lines 109008 109266 +258 Branches 109008 109266 +258 ==========================================
+ Hits 94828 95247 +419 + Misses 11696 11544 -152 + Partials 2484 2475 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| let block_hash = header.block_hash(); | ||
| log_trace!(self.logger, "{} transactions included in block {} at height {} provided", txdata.len(), block_hash, height); | ||
| struct LazyTxid<'a>(&'a chain::transaction::Transaction); |
There was a problem hiding this comment.
chain::transaction::Transaction references a private import — chain/transaction.rs has use bitcoin::transaction::Transaction; (no pub), so this name is not accessible from outside that module. This should fail to compile with error[E0603]: import 'Transaction' is private.
Transaction is already imported at line 24 of this file via use bitcoin::transaction::Transaction;, so the fix is simply:
| structLazyTxid<'a>(&'achain::transaction::Transaction); | |
| structLazyTxid<'a>(&'aTransaction); |
…gging Use a lazy Display wrapper to format confirmed txids, preventing unconditional hashing when DEBUG logging is disabled.
d78977d to
985a82bCompareldk-reviews-bot
commented
Jun 8, 2026
🔔 1st Reminder Hey @valentinewallace! This PR has been waiting for your review. |
valentinewallace
commented
Jun 8, 2026
Superseded by #4659 |
TheBlueMatt
commented
Jun 9, 2026
reopening per #4659 (comment) |
kamal-ogtl
commented
Jun 9, 2026
Thanks for reopening! |
Channelmanager: improve block connection logging
Fixes#2348. Supersedes #4420.
best_block_updatedwas logging at TRACE, making it impossible to track chain tip progress without enabling full trace logging. This upgrades it to INFO.Also adds per-txid DEBUG logs in
transactions_confirmedso individual transactions triggering channel updates are visible at a useful log level.The original PR #4420 also added an INFO log in
filtered_block_connected, but that duplicates the one inbest_block_updated(which filtered_block_connected calls directly). That log is left out here.