Log structured peer, channel and hash fields #4217

joostjager · 2025-11-11T12:52:28Z

This PR improves channel ID logging consistency and clarity.

Removes redundant channel ID text from log messages where the ID is already included as a structured field. This reduces duplication and makes it easier to filter logs by channel. De-duplication of peer and payment hash is left for a follow up.
Adds a Display implementation for log Records that outputs the structured fields. The fields are truncated to six (hex) characters for readability. In tests, the layout is changed slightly to accommodate for narrower terminal windows.

Together, these changes standardize how channel IDs, peers and payment hashes appear in logs and make them easier to parse and analyze.

Example output (prod):

node 0 TRACE [lightning::ln::channelmanager:5304]      ch:ae3367 p:0355f8 h:544e62 Attempting to send payment with payment hash 544e62cee8033709e389e5b2755343d0d0fa8c4850215cfb6331717e80d1aea3 along path with next hop 1099511627776
node 0 DEBUG [lightning::ln::channel:12560]            ch:ae3367 p:0355f8 h:544e62 Pushing new outbound HTLC with hash 544e62cee8033709e389e5b2755343d0d0fa8c4850215cfb6331717e80d1aea3 for 10000 msat to peer
node 0 TRACE [lightning::ln::channel:12640]            ch:ae3367 p:0355f8 h:544e62 Updating HTLC state for a newly-sent commitment_signed...
node 0 TRACE [lightning::ln::channel:5267]             ch:ae3367 p:0355f8 h:544e62 Building commitment transaction number 281474976710654 (really 1 xor 53018734980789) for channel ae3367da2c13bc1ceb86bf56418f62828f7ce9d6bfb15a46af5ba1f1ed8b124f for remote, generated by us with fee 253...
node 0 TRACE [lightning::ln::channel:5313]             ch:ae3367 p:0355f8 h:544e62    ...including outbound LocalAnnounced HTLC 0 (hash 544e62cee8033709e389e5b2755343d0d0fa8c4850215cfb6331717e80d1aea3) with value 10000
node 0 TRACE [lightning::sign::tx_builder:362]         ch:ae3367 p:0355f8 h:544e62    ...trimming outbound HTLC with value 10sat, hash 544e62cee8033709e389e5b2755343d0d0fa8c4850215cfb6331717e80d1aea3, due to dust limit 354
node 0 TRACE [lightning::sign::tx_builder:435]         ch:ae3367 p:0355f8 h:544e62    ...including to_local output with value 99796
node 0 TRACE [lightning::chain::chainmonitor:1407]     ch:ae3367 p:0355f8          Updating ChannelMonitor to id 1

Example output (test):

node 0 TRACE [lightning::ln::channelmanager:5304]      ch:ae3367 Attempting to send payment with payment hash 544e62cee8033709e389e5b2755343d0d0fa8c4850215cfb6331717e80d1aea3 along path with next hop 1099511627776 [p:0355f8 h:544e62]
node 0 DEBUG [lightning::ln::channel:12560]            ch:ae3367 Pushing new outbound HTLC with hash 544e62cee8033709e389e5b2755343d0d0fa8c4850215cfb6331717e80d1aea3 for 10000 msat to peer [p:0355f8 h:544e62]
node 0 TRACE [lightning::ln::channel:12640]            ch:ae3367 Updating HTLC state for a newly-sent commitment_signed... [p:0355f8 h:544e62]
node 0 TRACE [lightning::ln::channel:5267]             ch:ae3367 Building commitment transaction number 281474976710654 (really 1 xor 53018734980789) for channel ae3367da2c13bc1ceb86bf56418f62828f7ce9d6bfb15a46af5ba1f1ed8b124f for remote, generated by us with fee 253... [p:0355f8 h:544e62]
node 0 TRACE [lightning::ln::channel:5313]             ch:ae3367    ...including outbound LocalAnnounced HTLC 0 (hash 544e62cee8033709e389e5b2755343d0d0fa8c4850215cfb6331717e80d1aea3) with value 10000 [p:0355f8 h:544e62]
node 0 TRACE [lightning::sign::tx_builder:362]         ch:ae3367    ...trimming outbound HTLC with value 10sat, hash 544e62cee8033709e389e5b2755343d0d0fa8c4850215cfb6331717e80d1aea3, due to dust limit 354 [p:0355f8 h:544e62]
node 0 TRACE [lightning::sign::tx_builder:435]         ch:ae3367    ...including to_local output with value 99796 [p:0355f8 h:544e62]
node 0 TRACE [lightning::chain::chainmonitor:1407]     ch:ae3367 Updating ChannelMonitor to id 1 [p:0355f8]

ldk-reviews-bot · 2025-11-11T12:52:31Z

👋 Thanks for assigning @valentinewallace as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

lightning/src/util/test_utils.rs

codecov · 2025-11-12T09:27:16Z

Codecov Report

❌ Patch coverage is 88.99083% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.33%. Comparing base (d4828c0) to head (053933b).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/logger.rs	75.38%	0 Missing and 16 partials ⚠️
lightning/src/ln/channelmanager.rs	83.67%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4217      +/-   ##
==========================================
- Coverage   89.36%   89.33%   -0.03%     
==========================================
  Files         180      180              
  Lines      138369   138341      -28     
  Branches   138369   138341      -28     
==========================================
- Hits       123652   123587      -65     
- Misses      12114    12135      +21     
- Partials     2603     2619      +16

Flag	Coverage Δ
fuzzing	`35.91% <53.80%> (+0.03%)`	⬆️
tests	`88.70% <88.99%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

fuzz/src/onion_message.rs

lightning/src/util/logger.rs

TheBlueMatt

LGTM, needs rebase though.

lightning/src/util/logger.rs

As a preparatory step for unifying test log output.

Makes sure that 4 and 5 character log levels line up.

Now that we have Display implemented on Record, the various test logger implementations can make use of that.

joostjager · 2025-11-14T10:50:42Z

Rebased

joostjager · 2025-11-14T15:09:18Z

Named SubstringFormatter struct: https://github.com/lightningdevkit/rust-lightning/compare/2febae0fb29e9e72294d8b31b4e6a335270ac0db..e76d5643a729f9f854796218086235f6a2e0794b

valentinewallace

I would prefer putting the peer and payment hash at the end of the log, at least in test output. I work on a laptop somewhat regularly and this change pushes the actual log content very far to the side, particularly since the payment_hash field doesn't seem to be set the vast majority of the time.

We could also maybe do this in addition:

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 643c1825f..a0cd881f1 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -4360,7 +4360,7 @@ pub fn create_chanmon_cfgs_with_legacy_keys(
                let tx_broadcaster = test_utils::TestBroadcaster::new(Network::Testnet);
                let fee_estimator = test_utils::TestFeeEstimator::new(253);
                let chain_source = test_utils::TestChainSource::new(Network::Testnet);
-               let logger = test_utils::TestLogger::with_id(format!("node {}", i));
+               let logger = test_utils::TestLogger::with_id(format!("n{}", i));
                let persister = test_utils::TestPersister::new();
                let seed = [i as u8; 32];
                let keys_manager = if predefined_keys_ids.is_some() {
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 5f408a9e8..d414b980f 100644
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -1698,7 +1698,7 @@ impl TestLogger {

 impl Logger for TestLogger {
        fn log(&self, record: Record) {
-               let s = format!("{:<6} {}", self.id, record);
+               let s = format!("{:<2} {}", self.id, record);
                #[cfg(ldk_bench)]
                {
                        // When benchmarking, we don't actually want to print logs, but we do want to format

lightning/src/ln/channelmanager.rs

valentinewallace · 2025-11-14T16:29:51Z

lightning/src/ln/invoice_utils.rs

 					entry.insert(channel);
 				} else {
 					log_trace!(logger,
-						"Preferring counterparty {} channel {} (SCID {:?}, {} msats) over {} (SCID {:?}, {} msats) for invoice route hints",


The word "over" got deleted, may need to tweak the log slightly to keep the original meaning as well

Reverted these two log lines. As they are making a comparison, it may be a bit weird to rely on the structured field only.

joostjager · 2025-11-14T18:19:37Z

We could also maybe do this in addition:

In some tests, the nodes are called 'alice' and 'bob', so we'd need to change that to 'a' and 'b' perhaps? Maybe just one character is also sufficient. Starting everyline with n is probably unnecessary?

ldk-reviews-bot · 2025-11-17T00:00:11Z

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

joostjager · 2025-11-17T11:37:47Z

Applied the smaller suggestions https://github.com/lightningdevkit/rust-lightning/compare/73ea2568a2f4ab6f2e3809a2acf07ad111cad231~1..65f7f7f and added a fixup commit to differentiate output between test and prod. Single-char node ids I didn't do yet because we also have longer ids.

Previously, log messages often included the channel ID both in the message text and in the structured `channel_id` field. This led to redundant information and made it difficult to quickly identify which log lines pertain to a given channel, as the ID could appear in different positions or sometimes not at all. This change removes the channel ID from the message in all cases where it is already present in the structured field. A test run was used to verify that the structured field always appeared in these log calls. Some exceptions remain—for example, calls where the structured field contains a temporary channel ID and the message contains the final ID were left unchanged.

joostjager · 2025-11-17T14:38:25Z

Reminder: if rebasing on top of #4224, chan ids can be removed.

TheBlueMatt

Feel free to squash imo

valentinewallace

LGTM, feel free to squash

Since most log messages no longer include the channel ID in their text, this commit updates the test logger to output the structured channel ID instead. The ID is truncated for readability. Similarly peer id and payment hash are logged. Co-authored-by: Matt Corallo <[email protected]>

joostjager · 2025-11-17T16:18:34Z

Squashed without changes

valentinewallace

This has the side effect of reducing the rustfmt vertical-ness of many logs within the code👌

TheBlueMatt · 2025-11-17T18:05:33Z

lightning/src/ln/channelmanager.rs

The format! can go away and can be inlined into the log_err now that this differs from what we put in APIError::ChannelUnavailable below.

TheBlueMatt · 2025-11-17T18:06:52Z

lightning/src/ln/channelmanager.rs

 						};
-					log_trace!(logger, "Forwarding HTLC from SCID {} with payment_hash {} and next hop SCID {} over {} channel {} with corresponding peer {}",
-						prev_outbound_scid_alias, &payment_hash, short_chan_id, channel_description, optimal_channel.context.channel_id(), &counterparty_node_id);
+					log_trace!(logger, "Forwarding HTLC from SCID {} with payment_hash {} and next hop SCID {} over {} with corresponding peer {}",


We now also log peer so no need for that either (plus corresponding should have been removed).

Suggested change

log_trace!(logger, "Forwarding HTLC from SCID {} with payment_hash {} and next hop SCID {} over {} with corresponding peer {}",

log_trace!(logger, "Forwarding HTLC from SCID {} with payment_hash {} and next hop SCID {} over {}",

TheBlueMatt · 2025-11-17T18:31:50Z

lightning/src/ln/peer_handler.rs

+									Some(msg.channel_id),
+									None
+								),
+								"Handling Shutdown event in peer_handler for node {}",


There's plenty of room to remove node_ids in various places.

joostjager · 2025-11-17T20:10:29Z

I intentionally left removal of redundant node ids and payment hashes for a follow up. Wanted to land this first.

joostjager force-pushed the chan-logging branch 2 times, most recently from 5bae991 to bf104c0 Compare November 11, 2025 13:11

joostjager marked this pull request as ready for review November 11, 2025 13:45

joostjager requested a review from valentinewallace November 11, 2025 13:45

TheBlueMatt reviewed Nov 11, 2025

View reviewed changes

lightning/src/util/test_utils.rs Outdated Show resolved Hide resolved

joostjager force-pushed the chan-logging branch from bf104c0 to 9491fa7 Compare November 11, 2025 13:56

joostjager removed the request for review from valentinewallace November 11, 2025 14:00

joostjager force-pushed the chan-logging branch 3 times, most recently from 44dec3f to 2f6b50a Compare November 12, 2025 08:13

joostjager requested a review from TheBlueMatt November 12, 2025 10:29

joostjager force-pushed the chan-logging branch from 2f6b50a to f2483eb Compare November 12, 2025 10:36

joostjager requested a review from valentinewallace November 12, 2025 16:22

TheBlueMatt reviewed Nov 12, 2025

View reviewed changes

fuzz/src/onion_message.rs Show resolved Hide resolved

lightning/src/util/logger.rs Outdated Show resolved Hide resolved

joostjager force-pushed the chan-logging branch 4 times, most recently from 61b1780 to 3506f92 Compare November 13, 2025 10:29

joostjager added this to Weekly Goals Nov 13, 2025

joostjager self-assigned this Nov 13, 2025

TheBlueMatt reviewed Nov 13, 2025

View reviewed changes

lightning/src/util/logger.rs Outdated Show resolved Hide resolved

joostjager added 3 commits November 14, 2025 11:44

Implement Display for Record

621f909

As a preparatory step for unifying test log output.

Align log levels

3fc5090

Makes sure that 4 and 5 character log levels line up.

Unify test logger output

30715f5

Now that we have Display implemented on Record, the various test logger implementations can make use of that.

joostjager force-pushed the chan-logging branch from 3506f92 to 2febae0 Compare November 14, 2025 10:50

joostjager requested a review from TheBlueMatt November 14, 2025 10:50

joostjager changed the title ~~Remove redundant channel ID logging~~ Log structured peer, channel and hash fields Nov 14, 2025

joostjager force-pushed the chan-logging branch from 2febae0 to e76d564 Compare November 14, 2025 15:08

joostjager force-pushed the chan-logging branch from e76d564 to 73ea256 Compare November 14, 2025 15:16

valentinewallace reviewed Nov 14, 2025

View reviewed changes

joostjager force-pushed the chan-logging branch from 73ea256 to e069743 Compare November 14, 2025 19:16

joostjager mentioned this pull request Nov 14, 2025

Store and check channel data in channel monitors #4218

Draft

joostjager force-pushed the chan-logging branch from 3f2926c to 1e1df23 Compare November 17, 2025 11:33

joostjager force-pushed the chan-logging branch from 1e1df23 to a21fd62 Compare November 17, 2025 11:41

joostjager mentioned this pull request Nov 17, 2025

Functionize additional macro code in channelmanager.rs #4224

Open

joostjager requested a review from valentinewallace November 17, 2025 14:04

TheBlueMatt reviewed Nov 17, 2025

View reviewed changes

valentinewallace reviewed Nov 17, 2025

View reviewed changes

joostjager force-pushed the chan-logging branch from a21fd62 to 053933b Compare November 17, 2025 16:14

valentinewallace approved these changes Nov 17, 2025

View reviewed changes

joostjager merged commit bda19d6 into lightningdevkit:main Nov 17, 2025
26 checks passed

github-project-automation bot moved this to Done in Weekly Goals Nov 17, 2025

TheBlueMatt reviewed Nov 17, 2025

View reviewed changes

	log_trace!(logger, "Forwarding HTLC from SCID {} with payment_hash {} and next hop SCID {} over {} with corresponding peer {}",
	log_trace!(logger, "Forwarding HTLC from SCID {} with payment_hash {} and next hop SCID {} over {}",

Log structured peer, channel and hash fields #4217

Log structured peer, channel and hash fields #4217

Conversation

joostjager commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joostjager commented Nov 14, 2025

Uh oh!

joostjager commented Nov 14, 2025

Uh oh!

valentinewallace left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

valentinewallace Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

joostjager Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

joostjager commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Nov 17, 2025

Uh oh!

joostjager commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joostjager commented Nov 17, 2025

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

valentinewallace left a comment

Choose a reason for hiding this comment

Uh oh!

joostjager commented Nov 17, 2025

Uh oh!

valentinewallace left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheBlueMatt Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

joostjager commented Nov 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

joostjager commented Nov 11, 2025 •

edited

Loading

ldk-reviews-bot commented Nov 11, 2025 •

edited

Loading

codecov bot commented Nov 12, 2025 •

edited

Loading

joostjager commented Nov 14, 2025 •

edited

Loading

joostjager commented Nov 17, 2025 •

edited

Loading