Set dont_forward on private channel updates and add tests

[Thrishalmadasu]

Fixes #1672.

Changes:

• Set the dont_forward bit (message_flags bit 1) on channel_update messages for private channels
• Filter out inbound channel_update gossip messages that have dont_forward set in P2PGossipSync
• Log a debug message when receiving a channel_update with dont_forward set for an unknown channel, suggesting the user check the tx_index passed to chain::Confirm::transactions_confirmed

Details

Outbound flag setting:

• In ChannelManager::get_channel_update_for_unicast, we now set:
  • message_flags = 1 | (1 << 1) for private channels (must_be_one + dont_forward)
  • message_flags = 1 for public channels (must_be_one only)

Inbound filtering:

• In P2PGossipSync::handle_channel_update, channel updates with the dont_forward bit set are rejected with ErrorAction::IgnoreAndLog(Level::Debug) to prevent them from polluting the network graph

Diagnostic logging:

• In internal_channel_update, if we receive a channel_update for an unknown SCID with dont_forward set, we log a debug message suggesting the user verify the tx_index passed to chain::Confirm::transactions_confirmed

Tests

• handling_channel_update_with_dont_forward_flag - verifies gossip layer rejects updates with dont_forward bit
• test_channel_update_dont_forward_flag - verifies private channels set the flag, public channels don't
• test_unknown_channel_update_with_dont_forward_logs_debug - verifies debug logging for unknown channel updates

Based on lightning/bolts#999: BOLT 7 dont_forward flag for SCID aliases.

[ldk-reviews-bot]

👋 I see @wpaulino was un-assigned.
If you'd like another reviewer assignment, please click here.

[codecov]

Codecov Report

❌ Patch coverage is 97.77778% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.00%. Comparing base (8cdc86a) to head (5e32d69).
⚠️ Report is 69 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/routing/gossip.rs	97.61%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4340      +/-   ##
==========================================
- Coverage   86.53%   86.00%   -0.54%     
==========================================
  Files         158      156       -2     
  Lines      103190   102652     -538     
  Branches   103190   102652     -538     
==========================================
- Hits        89300    88285    -1015     
- Misses      11469    11857     +388     
- Partials     2421     2510      +89     

Flag	Coverage Δ
fuzzing	?
tests	86.00% <97.77%> (+0.16%)	⬆️

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.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @wpaulino! 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.

[TheBlueMatt]

We should also ignore updates we receive with the bit set in our network graph/p2pgossipmessagehandler.

[TheBlueMatt]

Thanks!

[TheBlueMatt]

There's really no need to include both a comment and assertion text that say the same thing. In general code should be self-explanatory and excess comments and assertion text is just distracting. LLMs love to be super duper verbose which often only serves to obscure what's going on.

[Thrishalmadasu]

Thanks for the review and merging! I'll be more mindful about avoiding redundant comments and assertion text going forward. I'll also be more careful with my use of tools.