-
Notifications
You must be signed in to change notification settings - Fork 402
Have get_route
and Route
take RouteParameters
#2555
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
Have get_route
and Route
take RouteParameters
#2555
Conversation
e0c032e
to
b092157
Compare
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #2555 +/- ##
==========================================
+ Coverage 90.59% 91.15% +0.55%
==========================================
Files 110 110
Lines 57509 61782 +4273
Branches 57509 61782 +4273
==========================================
+ Hits 52101 56315 +4214
- Misses 5408 5467 +59
☔ View full report in Codecov by Sentry. |
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.
Nothing blocking, thanks for undertaking this annoying sidequest 😛
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.
Also nothing worth blocking.
543674f
b092157
to
543674f
Compare
Addressed feedback and force-pushed with the following changes: > git diff-tree -U2 b0921575 543674f1
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 16e05db6..a43cfae9 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1856,6 +1856,6 @@ pub fn get_route(send_node: &Node, route_params: &RouteParameters) -> Result<Rou
macro_rules! get_route {
($send_node: expr, $payment_params: expr, $recv_value: expr) => {{
- let route_params = $crate::routing::router::RouteParameters::from_payment_params_and_value($payment_params, $recv_value);
- $crate::ln::functional_test_utils::get_route(&$send_node, &route_params)
+ let route_params = $crate::routing::router::RouteParameters::from_payment_params_and_value($payment_params, $recv_value);
+ $crate::ln::functional_test_utils::get_route(&$send_node, &route_params)
}}
}
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 63b2238d..9c5fe8e1 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -338,7 +338,9 @@ pub struct Route {
/// the same.
pub paths: Vec<Path>,
- /// The `route_params` parameter passed [`find_route`].
+ /// The `route_params` parameter passed to [`find_route`].
///
/// This is used by `ChannelManager` to track information which may be required for retries.
+ ///
+ /// Will be `None` for objects serialized with LDK versions prior to 0.0.117.
pub route_params: Option<RouteParameters>,
}
@@ -349,5 +351,5 @@ impl Route {
/// For objects serialized with LDK 0.0.117 and after, this includes any extra payment made to
/// the recipient, which can happen in excess of the amount passed to [`find_route`] via
- /// [`RouteParameters::final_value_msat`], if we had to reach the [`htlc_minimum_msat`] limit.
+ /// [`RouteParameters::final_value_msat`], if we had to reach the [`htlc_minimum_msat`] limits.
///
/// [`htlc_minimum_msat`]: https://github.com/lightning/bolts/blob/master/07-routing-gossip.md#the-channel_update-message
@@ -361,5 +363,5 @@ impl Route {
///
/// Might be more than requested as part of the given [`RouteParameters::final_value_msat`] if
- /// we had to reach the [`htlc_minimum_msat`] limit.
+ /// we had to reach the [`htlc_minimum_msat`] limits.
///
/// [`htlc_minimum_msat`]: https://github.com/lightning/bolts/blob/master/07-routing-gossip.md#the-channel_update-message
@@ -393,7 +395,7 @@ impl Writeable for Route {
}
write_tlv_fields!(writer, {
- // For compatibility with LDK versions prior to 0.0.117, we the individual
+ // For compatibility with LDK versions prior to 0.0.117, we take the individual
// RouteParameters' fields and reconstruct them on read.
- (1, self.route_params.as_ref().map(|p| p.payment_params.clone()), option),
+ (1, self.route_params.as_ref().map(|p| &p.payment_params), option),
(2, blinded_tails, optional_vec),
(3, self.route_params.as_ref().map(|p| p.final_value_msat), option),
@@ -434,5 +436,5 @@ impl Readable for Route {
}
- // If we previously wrote, the corresponding fields, reconstruct RouteParameters.
+ // If we previously wrote the corresponding fields, reconstruct RouteParameters.
let route_params = match (payment_params, final_value_msat) {
(Some(payment_params), Some(final_value_msat)) => {
diff --git a/pending_changelog/routes_route_params.txt b/pending_changelog/routes_route_params.txt
new file mode 100644
index 00000000..e88a1c78
--- /dev/null
+++ b/pending_changelog/routes_route_params.txt
@@ -0,0 +1,3 @@
+# Backwards Compatibility
+
+- `Route` objects written with LDK versions prior to 0.0.117 won't be retryable after being deserialized with LDK 0.0.117 or above. |
These are the first few commits of #2417 split out to it's own PR in order to avoid having too many rebase conflicts.