[minor] Add Display to Payment ID types

[optout21]

Minor convenience/housekeeping change:

• Added Display trait to PaymentHash, PaymentId, PaymentPreimage
• Reduced usage of log_bytes!()

Almost all remaining log_bytes! usage is for ChannelId, which is addressed in #2485 .
(The log_bytes! macro is OK, but its usage suggests non-type-safe usage, plus it reduces readablity a bit).

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.53%. Comparing base (8f02430) to head (4146264).
⚠️ Report is 5553 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	77.77%	6 Missing ⚠️
lightning/src/ln/inbound_payment.rs	50.00%	3 Missing ⚠️
lightning/src/ln/outbound_payment.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channel.rs	87.50%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2492 +/- ##
==========================================
- Coverage 90.59% 90.53% -0.07% 
==========================================
Files 106 107 +1 Lines 56525 56923 +398 Branches 56525 56923 +398 ==========================================
+ Hits 51207 51533 +326 - Misses 5318 5390 +72 

☔ 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.

[TheBlueMatt]

LGTM, except the allocations.

[TheBlueMatt]

Feel free to squash the fixup commit down into the original commit to not have two commits back-to-back that edit the same code.

[tnull]

LGTM after squash.

[optout21]

Rebased and reorganized commits, as suggested, to avoid touching the same code from multiple commits. Commits: add Display to PaymentHash (most frequently used), adapt logging, add Display to PaymentId and PaymentPreimage, adapt logging.
Please reconsider for review.

[valentinewallace]

Missed a few in channelmanager:

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 51f9b7914..db8de796f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -6331,10 +6331,10 @@ where
match monitor_event {
MonitorEvent::HTLCEvent(htlc_update) => {
if let Some(preimage) = htlc_update.payment_preimage {
- log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0));
+ log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", preimage);
self.claim_funds_internal(htlc_update.source, preimage, htlc_update.htlc_value_satoshis.map(|v| v * 1000), true, funding_outpoint);
} else {
- log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
+ log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", htlc_update.payment_hash);
let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);

[optout21]

Missed a few in channelmanager:

Indeed, good catch! I've searched through all log_bytes! occurences, but apparently I've missed these two (maybe changed with last rebase). Fixed (included in same commit).

[jbesraa]

looks good!
feel free to squash "add" and "use" commits

[tnull]

LGTM

[optout21]

Minor convenience/housekeeping change:

• Added Display trait to PaymentHash, PaymentId, PaymentPreimage
• Reduced usage of log_bytes!()

Almost all remaining log_bytes! usage is for ChannelId, which is addressed in #2485 .
(The log_bytes! macro is OK, but its usage suggests non-type-safe usage, plus it reduces readablity a bit).

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.53%. Comparing base (8f02430) to head (4146264).
⚠️ Report is 5553 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	77.77%	6 Missing ⚠️
lightning/src/ln/inbound_payment.rs	50.00%	3 Missing ⚠️
lightning/src/ln/outbound_payment.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channel.rs	87.50%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2492 +/- ##
==========================================
- Coverage 90.59% 90.53% -0.07% 
==========================================
Files 106 107 +1 Lines 56525 56923 +398 Branches 56525 56923 +398 ==========================================
+ Hits 51207 51533 +326 - Misses 5318 5390 +72 

☔ 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.

[TheBlueMatt]

LGTM, except the allocations.

[TheBlueMatt]

Feel free to squash the fixup commit down into the original commit to not have two commits back-to-back that edit the same code.

[tnull]

LGTM after squash.

[optout21]

Rebased and reorganized commits, as suggested, to avoid touching the same code from multiple commits. Commits: add Display to PaymentHash (most frequently used), adapt logging, add Display to PaymentId and PaymentPreimage, adapt logging.
Please reconsider for review.

[valentinewallace]

Missed a few in channelmanager:

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 51f9b7914..db8de796f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -6331,10 +6331,10 @@ where
match monitor_event {
MonitorEvent::HTLCEvent(htlc_update) => {
if let Some(preimage) = htlc_update.payment_preimage {
- log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0));
+ log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", preimage);
self.claim_funds_internal(htlc_update.source, preimage, htlc_update.htlc_value_satoshis.map(|v| v * 1000), true, funding_outpoint);
} else {
- log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
+ log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", htlc_update.payment_hash);
let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);

[optout21]

Missed a few in channelmanager:

Indeed, good catch! I've searched through all log_bytes! occurences, but apparently I've missed these two (maybe changed with last rebase). Fixed (included in same commit).

[jbesraa]

looks good!
feel free to squash "add" and "use" commits

[tnull]

LGTM

[optout21]

Minor convenience/housekeeping change:

• Added Display trait to PaymentHash, PaymentId, PaymentPreimage
• Reduced usage of log_bytes!()

Almost all remaining log_bytes! usage is for ChannelId, which is addressed in #2485 .
(The log_bytes! macro is OK, but its usage suggests non-type-safe usage, plus it reduces readablity a bit).

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.53%. Comparing base (8f02430) to head (4146264).
⚠️ Report is 5553 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	77.77%	6 Missing ⚠️
lightning/src/ln/inbound_payment.rs	50.00%	3 Missing ⚠️
lightning/src/ln/outbound_payment.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channel.rs	87.50%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2492 +/- ##
==========================================
- Coverage 90.59% 90.53% -0.07% 
==========================================
Files 106 107 +1 Lines 56525 56923 +398 Branches 56525 56923 +398 ==========================================
+ Hits 51207 51533 +326 - Misses 5318 5390 +72 

☔ 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.

[TheBlueMatt]

LGTM, except the allocations.

[TheBlueMatt]

Feel free to squash the fixup commit down into the original commit to not have two commits back-to-back that edit the same code.

[tnull]

LGTM after squash.

[optout21]

Rebased and reorganized commits, as suggested, to avoid touching the same code from multiple commits. Commits: add Display to PaymentHash (most frequently used), adapt logging, add Display to PaymentId and PaymentPreimage, adapt logging.
Please reconsider for review.

[valentinewallace]

Missed a few in channelmanager:

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 51f9b7914..db8de796f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -6331,10 +6331,10 @@ where
match monitor_event {
MonitorEvent::HTLCEvent(htlc_update) => {
if let Some(preimage) = htlc_update.payment_preimage {
- log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0));
+ log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", preimage);
self.claim_funds_internal(htlc_update.source, preimage, htlc_update.htlc_value_satoshis.map(|v| v * 1000), true, funding_outpoint);
} else {
- log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
+ log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", htlc_update.payment_hash);
let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);

[optout21]

Missed a few in channelmanager:

Indeed, good catch! I've searched through all log_bytes! occurences, but apparently I've missed these two (maybe changed with last rebase). Fixed (included in same commit).

[jbesraa]

looks good!
feel free to squash "add" and "use" commits

[tnull]

LGTM

[optout21]

Minor convenience/housekeeping change:

• Added Display trait to PaymentHash, PaymentId, PaymentPreimage
• Reduced usage of log_bytes!()

Almost all remaining log_bytes! usage is for ChannelId, which is addressed in #2485 .
(The log_bytes! macro is OK, but its usage suggests non-type-safe usage, plus it reduces readablity a bit).

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.53%. Comparing base (8f02430) to head (4146264).
⚠️ Report is 5553 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	77.77%	6 Missing ⚠️
lightning/src/ln/inbound_payment.rs	50.00%	3 Missing ⚠️
lightning/src/ln/outbound_payment.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channel.rs	87.50%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2492 +/- ##
==========================================
- Coverage 90.59% 90.53% -0.07% 
==========================================
Files 106 107 +1 Lines 56525 56923 +398 Branches 56525 56923 +398 ==========================================
+ Hits 51207 51533 +326 - Misses 5318 5390 +72 

☔ 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.

[TheBlueMatt]

LGTM, except the allocations.

[TheBlueMatt]

Feel free to squash the fixup commit down into the original commit to not have two commits back-to-back that edit the same code.

[tnull]

LGTM after squash.

[optout21]

Rebased and reorganized commits, as suggested, to avoid touching the same code from multiple commits. Commits: add Display to PaymentHash (most frequently used), adapt logging, add Display to PaymentId and PaymentPreimage, adapt logging.
Please reconsider for review.

[valentinewallace]

Missed a few in channelmanager:

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 51f9b7914..db8de796f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -6331,10 +6331,10 @@ where
match monitor_event {
MonitorEvent::HTLCEvent(htlc_update) => {
if let Some(preimage) = htlc_update.payment_preimage {
- log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0));
+ log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", preimage);
self.claim_funds_internal(htlc_update.source, preimage, htlc_update.htlc_value_satoshis.map(|v| v * 1000), true, funding_outpoint);
} else {
- log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
+ log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", htlc_update.payment_hash);
let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);

[optout21]

Missed a few in channelmanager:

Indeed, good catch! I've searched through all log_bytes! occurences, but apparently I've missed these two (maybe changed with last rebase). Fixed (included in same commit).

[jbesraa]

looks good!
feel free to squash "add" and "use" commits

[tnull]

LGTM

[optout21]

Minor convenience/housekeeping change:

• Added Display trait to PaymentHash, PaymentId, PaymentPreimage
• Reduced usage of log_bytes!()

Almost all remaining log_bytes! usage is for ChannelId, which is addressed in #2485 .
(The log_bytes! macro is OK, but its usage suggests non-type-safe usage, plus it reduces readablity a bit).

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.53%. Comparing base (8f02430) to head (4146264).
⚠️ Report is 5553 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	77.77%	6 Missing ⚠️
lightning/src/ln/inbound_payment.rs	50.00%	3 Missing ⚠️
lightning/src/ln/outbound_payment.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channel.rs	87.50%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2492 +/- ##
==========================================
- Coverage 90.59% 90.53% -0.07% 
==========================================
Files 106 107 +1 Lines 56525 56923 +398 Branches 56525 56923 +398 ==========================================
+ Hits 51207 51533 +326 - Misses 5318 5390 +72 

☔ 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.

[TheBlueMatt]

LGTM, except the allocations.

[TheBlueMatt]

Feel free to squash the fixup commit down into the original commit to not have two commits back-to-back that edit the same code.

[tnull]

LGTM after squash.

[optout21]

Rebased and reorganized commits, as suggested, to avoid touching the same code from multiple commits. Commits: add Display to PaymentHash (most frequently used), adapt logging, add Display to PaymentId and PaymentPreimage, adapt logging.
Please reconsider for review.

[valentinewallace]

Missed a few in channelmanager:

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 51f9b7914..db8de796f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -6331,10 +6331,10 @@ where
match monitor_event {
MonitorEvent::HTLCEvent(htlc_update) => {
if let Some(preimage) = htlc_update.payment_preimage {
- log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0));
+ log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", preimage);
self.claim_funds_internal(htlc_update.source, preimage, htlc_update.htlc_value_satoshis.map(|v| v * 1000), true, funding_outpoint);
} else {
- log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
+ log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", htlc_update.payment_hash);
let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);

[optout21]

Missed a few in channelmanager:

Indeed, good catch! I've searched through all log_bytes! occurences, but apparently I've missed these two (maybe changed with last rebase). Fixed (included in same commit).

[jbesraa]

looks good!
feel free to squash "add" and "use" commits

[tnull]

LGTM

[optout21]

Minor convenience/housekeeping change:

• Added Display trait to PaymentHash, PaymentId, PaymentPreimage
• Reduced usage of log_bytes!()

Almost all remaining log_bytes! usage is for ChannelId, which is addressed in #2485 .
(The log_bytes! macro is OK, but its usage suggests non-type-safe usage, plus it reduces readablity a bit).

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.53%. Comparing base (8f02430) to head (4146264).
⚠️ Report is 5553 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	77.77%	6 Missing ⚠️
lightning/src/ln/inbound_payment.rs	50.00%	3 Missing ⚠️
lightning/src/ln/outbound_payment.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channel.rs	87.50%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2492 +/- ##
==========================================
- Coverage 90.59% 90.53% -0.07% 
==========================================
Files 106 107 +1 Lines 56525 56923 +398 Branches 56525 56923 +398 ==========================================
+ Hits 51207 51533 +326 - Misses 5318 5390 +72 

☔ 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.

[TheBlueMatt]

LGTM, except the allocations.

[TheBlueMatt]

Feel free to squash the fixup commit down into the original commit to not have two commits back-to-back that edit the same code.

[tnull]

LGTM after squash.

[optout21]

Rebased and reorganized commits, as suggested, to avoid touching the same code from multiple commits. Commits: add Display to PaymentHash (most frequently used), adapt logging, add Display to PaymentId and PaymentPreimage, adapt logging.
Please reconsider for review.

[valentinewallace]

Missed a few in channelmanager:

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 51f9b7914..db8de796f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -6331,10 +6331,10 @@ where
match monitor_event {
MonitorEvent::HTLCEvent(htlc_update) => {
if let Some(preimage) = htlc_update.payment_preimage {
- log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0));
+ log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", preimage);
self.claim_funds_internal(htlc_update.source, preimage, htlc_update.htlc_value_satoshis.map(|v| v * 1000), true, funding_outpoint);
} else {
- log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
+ log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", htlc_update.payment_hash);
let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);

[optout21]

Missed a few in channelmanager:

Indeed, good catch! I've searched through all log_bytes! occurences, but apparently I've missed these two (maybe changed with last rebase). Fixed (included in same commit).

[jbesraa]

looks good!
feel free to squash "add" and "use" commits

[tnull]

LGTM

[optout21]

Minor convenience/housekeeping change:

• Added Display trait to PaymentHash, PaymentId, PaymentPreimage
• Reduced usage of log_bytes!()

Almost all remaining log_bytes! usage is for ChannelId, which is addressed in #2485 .
(The log_bytes! macro is OK, but its usage suggests non-type-safe usage, plus it reduces readablity a bit).

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.53%. Comparing base (8f02430) to head (4146264).
⚠️ Report is 5553 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	77.77%	6 Missing ⚠️
lightning/src/ln/inbound_payment.rs	50.00%	3 Missing ⚠️
lightning/src/ln/outbound_payment.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channel.rs	87.50%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2492 +/- ##
==========================================
- Coverage 90.59% 90.53% -0.07% 
==========================================
Files 106 107 +1 Lines 56525 56923 +398 Branches 56525 56923 +398 ==========================================
+ Hits 51207 51533 +326 - Misses 5318 5390 +72 

☔ 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.

[TheBlueMatt]

LGTM, except the allocations.

[TheBlueMatt]

Feel free to squash the fixup commit down into the original commit to not have two commits back-to-back that edit the same code.

[tnull]

LGTM after squash.

[optout21]

Rebased and reorganized commits, as suggested, to avoid touching the same code from multiple commits. Commits: add Display to PaymentHash (most frequently used), adapt logging, add Display to PaymentId and PaymentPreimage, adapt logging.
Please reconsider for review.

[valentinewallace]

Missed a few in channelmanager:

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 51f9b7914..db8de796f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -6331,10 +6331,10 @@ where
match monitor_event {
MonitorEvent::HTLCEvent(htlc_update) => {
if let Some(preimage) = htlc_update.payment_preimage {
- log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0));
+ log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", preimage);
self.claim_funds_internal(htlc_update.source, preimage, htlc_update.htlc_value_satoshis.map(|v| v * 1000), true, funding_outpoint);
} else {
- log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
+ log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", htlc_update.payment_hash);
let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);

[optout21]

Missed a few in channelmanager:

Indeed, good catch! I've searched through all log_bytes! occurences, but apparently I've missed these two (maybe changed with last rebase). Fixed (included in same commit).

[jbesraa]

looks good!
feel free to squash "add" and "use" commits

[tnull]

LGTM

[optout21]

Minor convenience/housekeeping change:

• Added Display trait to PaymentHash, PaymentId, PaymentPreimage
• Reduced usage of log_bytes!()

Almost all remaining log_bytes! usage is for ChannelId, which is addressed in #2485 .
(The log_bytes! macro is OK, but its usage suggests non-type-safe usage, plus it reduces readablity a bit).

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.53%. Comparing base (8f02430) to head (4146264).
⚠️ Report is 5553 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	77.77%	6 Missing ⚠️
lightning/src/ln/inbound_payment.rs	50.00%	3 Missing ⚠️
lightning/src/ln/outbound_payment.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channel.rs	87.50%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2492 +/- ##
==========================================
- Coverage 90.59% 90.53% -0.07% 
==========================================
Files 106 107 +1 Lines 56525 56923 +398 Branches 56525 56923 +398 ==========================================
+ Hits 51207 51533 +326 - Misses 5318 5390 +72 

☔ 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.

[TheBlueMatt]

LGTM, except the allocations.

[TheBlueMatt]

Feel free to squash the fixup commit down into the original commit to not have two commits back-to-back that edit the same code.

[tnull]

LGTM after squash.

[optout21]

Rebased and reorganized commits, as suggested, to avoid touching the same code from multiple commits. Commits: add Display to PaymentHash (most frequently used), adapt logging, add Display to PaymentId and PaymentPreimage, adapt logging.
Please reconsider for review.

[valentinewallace]

Missed a few in channelmanager:

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 51f9b7914..db8de796f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -6331,10 +6331,10 @@ where
match monitor_event {
MonitorEvent::HTLCEvent(htlc_update) => {
if let Some(preimage) = htlc_update.payment_preimage {
- log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0));
+ log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", preimage);
self.claim_funds_internal(htlc_update.source, preimage, htlc_update.htlc_value_satoshis.map(|v| v * 1000), true, funding_outpoint);
} else {
- log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
+ log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", htlc_update.payment_hash);
let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);

[optout21]

Missed a few in channelmanager:

Indeed, good catch! I've searched through all log_bytes! occurences, but apparently I've missed these two (maybe changed with last rebase). Fixed (included in same commit).

[jbesraa]

looks good!
feel free to squash "add" and "use" commits

[tnull]

LGTM