Skip to content

Use channel's real funding amounts when processing RGS data #3924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheBlueMatt
Copy link
Collaborator

Neither channel_announcements nor channel_updates contain a channel's actual funding amount, complicating scoring as nodes may set an htlc_maximum_msat to something less than the funding amount. When scoring, we use htlc_maximum_msat anyway if we don't know the funding amount, but its not a perfect proxy.

In
lightningdevkit/rapid-gossip-sync-server#102 we started including a channel's real funding amount in RGS data, and here we start parsing it and including it in our network graph.

Fixes #1559

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jul 12, 2025

👋 Thanks for assigning @jkczyz 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.

@TheBlueMatt TheBlueMatt requested a review from jkczyz July 12, 2025 17:30
Copy link

codecov bot commented Jul 12, 2025

Codecov Report

Attention: Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.56%. Comparing base (d67bd0f) to head (7ac02a6).
Report is 22 commits behind head on main.

Files with missing lines Patch % Lines
lightning-rapid-gossip-sync/src/processing.rs 88.23% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3924      +/-   ##
==========================================
+ Coverage   88.83%   89.56%   +0.73%     
==========================================
  Files         166      166              
  Lines      119259   128662    +9403     
  Branches   119259   128662    +9403     
==========================================
+ Hits       105940   115241    +9301     
- Misses      10992    11032      +40     
- Partials     2327     2389      +62     

☔ 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
Copy link

🔔 1st Reminder

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

@ldk-reviews-bot
Copy link

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

@TheBlueMatt TheBlueMatt force-pushed the 2025-07-rgs-use-funding-amt branch from 7ac02a6 to 7ad6364 Compare July 16, 2025 19:11
@TheBlueMatt
Copy link
Collaborator Author

Fixed the spurious error acceptance:

$ git diff-tree -U2 7ac02a625 7ad6364ab
diff --git a/lightning-rapid-gossip-sync/src/processing.rs b/lightning-rapid-gossip-sync/src/processing.rs
index 7670147ef7..002807414b 100644
--- a/lightning-rapid-gossip-sync/src/processing.rs
+++ b/lightning-rapid-gossip-sync/src/processing.rs
@@ -275,5 +275,6 @@ where
 				let mut cursor = &additional_data[..];
 				let funding_sats_read: Result<BigSize, _> = Readable::read(&mut cursor);
-				funding_sats = funding_sats_read.map(|sats| Some(sats.0)).unwrap_or(None);
+				funding_sats =
+					funding_sats_read.map(|sats| Some(sats.0)).ok_or(DecodeError::InvalidValue)?;
 				if !cursor.is_empty() {
 					log_gossip!(

@TheBlueMatt TheBlueMatt requested a review from jkczyz July 16, 2025 19:11
jkczyz
jkczyz previously approved these changes Jul 16, 2025
Copy link
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems CI is unhappy:

error[E0599]: no method named `ok_or` found for enum `Result` in the current scope
   --> lightning-rapid-gossip-sync/src/processing.rs:278:49
    |
278 |                     funding_sats_read.map(|sats| Some(sats.0)).ok_or(DecodeError::InvalidValue)?;
    |                                                                ^^^^^ method not found in `Result<std::option::Option<u64>, lightning::ln::msgs::DecodeError>`
    |
note: the method `ok_or` exists on the type `std::option::Option<u64>`
help: use the `?` operator to extract the `std::option::Option<u64>` value, propagating a `Result::Err` value to the caller
    |
278 |                     funding_sats_read.map(|sats| Some(sats.0))?.ok_or(DecodeError::InvalidValue)?;
    |                                                               +

For more information about this error, try `rustc --explain E0599`.
error: could not compile `lightning-rapid-gossip-sync` due to previous error

Neither `channel_announcement`s nor `channel_update`s contain a
channel's actual funding amount, complicating scoring as nodes may
set an `htlc_maximum_msat` to something less than the funding
amount. When scoring, we use `htlc_maximum_msat` anyway if we don't
know the funding amount, but its not a perfect proxy.

In
lightningdevkit/rapid-gossip-sync-server#102
we started including a channel's real funding amount in RGS data,
and here we start parsing it and including it in our network graph.

Fixes lightningdevkit#1559
@TheBlueMatt
Copy link
Collaborator Author

Ugh, sorry:

$ git diff-tree -U2 7ad6364ab 37756867a
diff --git a/lightning-rapid-gossip-sync/src/processing.rs b/lightning-rapid-gossip-sync/src/processing.rs
index 002807414b..8319506b57 100644
--- a/lightning-rapid-gossip-sync/src/processing.rs
+++ b/lightning-rapid-gossip-sync/src/processing.rs
@@ -274,7 +274,6 @@ where
 				let additional_data: Vec<u8> = Readable::read(read_cursor)?;
 				let mut cursor = &additional_data[..];
-				let funding_sats_read: Result<BigSize, _> = Readable::read(&mut cursor);
-				funding_sats =
-					funding_sats_read.map(|sats| Some(sats.0)).ok_or(DecodeError::InvalidValue)?;
+				let funding_sats_read: BigSize = Readable::read(&mut cursor)?;
+				funding_sats = Some(funding_sats_read.0);
 				if !cursor.is_empty() {
 					log_gossip!(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle htlc_max != channel capacity better in RGS
4 participants