-
Notifications
You must be signed in to change notification settings - Fork 380
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
Small API cleanups pre-0.0.119 #2798
Small API cleanups pre-0.0.119 #2798
Conversation
CI is sad |
1350daa
to
759a213
Compare
Oops, fixed. |
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2798 +/- ##
=======================================
Coverage 88.43% 88.43%
=======================================
Files 114 114
Lines 91718 91718
Branches 91718 91718
=======================================
+ Hits 81109 81112 +3
- Misses 8126 8127 +1
+ Partials 2483 2479 -4 ☔ View full report in Codecov by Sentry. |
When we make the `PrivateRoute` inner `RouteHint` `pub`, we failed to note that the `PrivateRoute::new` constructor actually verifies a length invariant. Thus, we un-export the inner field and force users to go back through the `new` fn.
Since `lightning-invoice` now depends on the `bitcoin` crate directly, also depending on the `bitcoin_hashes` crate is redundant and just means we confuse users by setting the `std` flag only on `bitcoin`. Thus, we drop the explicit dependency here and replace it with `bitcoin::hashes`.
In 4b5db8c, `channelmanager::PendingHTLCRouting` was made public, exposing a `FinalOnionHopData` field to the world. However, `FinalOnionHopData` was left crate-private, making the enum impossible to construct. There isn't a strong reason for this (even though the `FinalOnionHopData` API is somewhat confusing, being separated from the rest of the onion structs), so we expose it here.
62f8669 added two `htlc_maximum_msat.unwrap_or`s, but used a default value of 0, spuriously causing all HTLCs to fail if we don't have an htlc maximum value. This should be mostly harmless, but we should fix it anyway.
759a213
to
642240e
Compare
Oops, added one more fixup from #2781. |
...due to a transitive dependency of the `bitcoind` crate.
Also pushed a last-minute MSRV break in a dev-dep. |
@@ -102,7 +102,7 @@ impl<G: Deref<Target = NetworkGraph<L>> + Clone, L: Deref, S: Deref, SP: Sized, | |||
.filter(|details| details.counterparty.features.supports_route_blinding()) | |||
.filter(|details| amount_msats <= details.inbound_capacity_msat) | |||
.filter(|details| amount_msats >= details.inbound_htlc_minimum_msat.unwrap_or(0)) | |||
.filter(|details| amount_msats <= details.inbound_htlc_maximum_msat.unwrap_or(0)) | |||
.filter(|details| amount_msats <= details.inbound_htlc_maximum_msat.unwrap_or(u64::MAX)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... I think my reasoning here was that inbound_htlc_maximum_msat
should be set for usable channels. So if it is not set, using zero would cause us to not consider this channel.
It will be Some
if ChannelContext::counterparty_selected_channel_reserve_satoshis
is Some
, which seems to be when accept_channel
is received:
rust-lightning/lightning/src/ln/channel.rs
Line 6324 in b9797eb
counterparty_selected_channel_reserve_satoshis: None, // Filled in in accept_channel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yea, I mean it won't be set except if the channel isn't usable, but I'd kinda rather lean on get_inbound_payment_scid()
to detect "is channel available for inbound usage" rather than htlc_max, but it doesn't really matter that much.
A few small API cleanups that came up when generating bindings.