From f03b7cd448ab1b9f7613f964bdea7c5c1a00da3c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 6 Feb 2023 22:12:09 +0000 Subject: [PATCH] Remove the `final_cltv_expiry_delta` in `RouteParameters` entirely fbc08477e8dcdd8f3f2ada8ca77388b6185febe2 purported to "move" the `final_cltv_expiry_delta` field to `PaymentParamters` from `RouteParameters`. However, for naive backwards-compatibility reasons it left the existing on in place and only added a new, redundant field in `PaymentParameters`. It turns out there's really no reason for this - if we take a more critical eye towards backwards compatibility we can figure out the correct value in every `PaymentParameters` while deserializing. We do this here - making `PaymentParameters` a `ReadableArgs` taking a "default" `cltv_expiry_delta` when it goes to read. This allows existing `RouteParameters` objects to pass the read `final_cltv_expiry_delta` field in to be used if the new field wasn't present. --- fuzz/src/full_stack.rs | 2 - fuzz/src/router.rs | 1 - lightning-invoice/src/payment.rs | 1 - lightning-invoice/src/utils.rs | 2 - lightning/src/ln/channelmanager.rs | 12 +-- lightning/src/ln/functional_tests.rs | 2 - lightning/src/ln/outbound_payment.rs | 27 +----- lightning/src/ln/payment_tests.rs | 26 ++---- lightning/src/routing/router.rs | 124 +++++++++++++++++++-------- 9 files changed, 106 insertions(+), 91 deletions(-) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index e7d10a34..b96de0b9 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -513,7 +513,6 @@ pub fn do_test(data: &[u8], logger: &Arc) { let params = RouteParameters { payment_params, final_value_msat, - final_cltv_expiry_delta: 42, }; let random_seed_bytes: [u8; 32] = keys_manager.get_secure_random_bytes(); let route = match find_route(&our_id, ¶ms, &network_graph, None, Arc::clone(&logger), &scorer, &random_seed_bytes) { @@ -537,7 +536,6 @@ pub fn do_test(data: &[u8], logger: &Arc) { let params = RouteParameters { payment_params, final_value_msat, - final_cltv_expiry_delta: 42, }; let random_seed_bytes: [u8; 32] = keys_manager.get_secure_random_bytes(); let mut route = match find_route(&our_id, ¶ms, &network_graph, None, Arc::clone(&logger), &scorer, &random_seed_bytes) { diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 93de35c5..4c228cc7 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -301,7 +301,6 @@ pub fn do_test(data: &[u8], out: Out) { payment_params: PaymentParameters::from_node_id(*target, final_cltv_expiry_delta) .with_route_hints(last_hops.clone()), final_value_msat, - final_cltv_expiry_delta, }; let _ = find_route(&our_pubkey, &route_params, &net_graph, first_hops.map(|c| c.iter().collect::>()).as_ref().map(|a| a.as_slice()), diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index db47f995..981fd2e6 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -156,7 +156,6 @@ fn pay_invoice_using_amount( let route_params = RouteParameters { payment_params, final_value_msat: amount_msats, - final_cltv_expiry_delta: invoice.min_final_cltv_expiry_delta() as u32, }; payer.send_payment(payment_hash, &payment_secret, payment_id, route_params, retry_strategy) diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index 868f0b29..11a779b1 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -686,7 +686,6 @@ mod test { let route_params = RouteParameters { payment_params, final_value_msat: invoice.amount_milli_satoshis().unwrap(), - final_cltv_expiry_delta: invoice.min_final_cltv_expiry_delta() as u32, }; let first_hops = nodes[0].node.list_usable_channels(); let network_graph = &node_cfgs[0].network_graph; @@ -1050,7 +1049,6 @@ mod test { let params = RouteParameters { payment_params, final_value_msat: invoice.amount_milli_satoshis().unwrap(), - final_cltv_expiry_delta: invoice.min_final_cltv_expiry_delta() as u32, }; let first_hops = nodes[0].node.list_usable_channels(); let network_graph = &node_cfgs[0].network_graph; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5d09741f..80cea29f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6748,14 +6748,14 @@ impl Readable for HTLCSource { let mut path: Option> = Some(Vec::new()); let mut payment_id = None; let mut payment_secret = None; - let mut payment_params = None; + let mut payment_params: Option = None; read_tlv_fields!(reader, { (0, session_priv, required), (1, payment_id, option), (2, first_hop_htlc_msat, required), (3, payment_secret, option), (4, path, vec_type), - (5, payment_params, option), + (5, payment_params, (option: ReadableArgs, 0)), }); if payment_id.is_none() { // For backwards compat, if there was no payment_id written, use the session_priv bytes @@ -6766,6 +6766,11 @@ impl Readable for HTLCSource { return Err(DecodeError::InvalidValue); } let path = path.unwrap(); + if let Some(params) = payment_params.as_mut() { + if params.final_cltv_expiry_delta == 0 { + params.final_cltv_expiry_delta = path.last().unwrap().cltv_expiry_delta; + } + } Ok(HTLCSource::OutboundRoute { session_priv: session_priv.0.unwrap(), first_hop_htlc_msat, @@ -7967,7 +7972,6 @@ mod tests { let route_params = RouteParameters { payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV), final_value_msat: 100_000, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; let route = find_route( &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph, @@ -8058,7 +8062,6 @@ mod tests { let route_params = RouteParameters { payment_params: PaymentParameters::for_keysend(payee_pubkey, 40), final_value_msat: 10_000, - final_cltv_expiry_delta: 40, }; let network_graph = nodes[0].network_graph.clone(); let first_hops = nodes[0].node.list_usable_channels(); @@ -8101,7 +8104,6 @@ mod tests { let route_params = RouteParameters { payment_params: PaymentParameters::for_keysend(payee_pubkey, 40), final_value_msat: 10_000, - final_cltv_expiry_delta: 40, }; let network_graph = nodes[0].network_graph.clone(); let first_hops = nodes[0].node.list_usable_channels(); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 538f51c3..e16b20b8 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -9241,7 +9241,6 @@ fn test_keysend_payments_to_public_node() { let route_params = RouteParameters { payment_params: PaymentParameters::for_keysend(payee_pubkey, 40), final_value_msat: 10000, - final_cltv_expiry_delta: 40, }; let scorer = test_utils::TestScorer::new(); let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes(); @@ -9272,7 +9271,6 @@ fn test_keysend_payments_to_private_node() { let route_params = RouteParameters { payment_params: PaymentParameters::for_keysend(payee_pubkey, 40), final_value_msat: 10000, - final_cltv_expiry_delta: 40, }; let network_graph = nodes[0].network_graph.clone(); let first_hops = nodes[0].node.list_usable_channels(); diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 15bba61d..33ccecb4 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -16,7 +16,6 @@ use bitcoin::secp256k1::{self, Secp256k1, SecretKey}; use crate::chain::keysinterface::{EntropySource, NodeSigner, Recipient}; use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId}; -use crate::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA as LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA; use crate::ln::onion_utils::HTLCFailReason; use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router}; use crate::util::errors::APIError; @@ -25,6 +24,7 @@ use crate::util::logger::Logger; use crate::util::time::Time; #[cfg(all(not(feature = "no-std"), test))] use crate::util::time::tests::SinceEpoch; +use crate::util::ser::ReadableArgs; use core::cmp; use core::fmt::{self, Display, Formatter}; @@ -528,12 +528,6 @@ impl OutboundPayments { if pending_amt_msat < total_msat { retry_id_route_params = Some((*pmt_id, RouteParameters { final_value_msat: *total_msat - *pending_amt_msat, - final_cltv_expiry_delta: - if let Some(delta) = params.final_cltv_expiry_delta { delta } - else { - debug_assert!(false, "We always set the final_cltv_expiry_delta when a path fails"); - LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA.into() - }, payment_params: params.clone(), })); break @@ -976,9 +970,6 @@ impl OutboundPayments { Some(RouteParameters { payment_params: payment_params.clone(), final_value_msat: pending_amt_unsent, - final_cltv_expiry_delta: - if let Some(delta) = payment_params.final_cltv_expiry_delta { delta } - else { max_unsent_cltv_delta }, }) } else { None } } else { None }, @@ -1179,23 +1170,14 @@ impl OutboundPayments { // `payment_params`) back to the user. let path_last_hop = path.last().expect("Outbound payments must have had a valid path"); if let Some(params) = payment.get_mut().payment_parameters() { - if params.final_cltv_expiry_delta.is_none() { - // This should be rare, but a user could provide None for the payment data, and - // we need it when we go to retry the payment, so fill it in. - params.final_cltv_expiry_delta = Some(path_last_hop.cltv_expiry_delta); - } retry = Some(RouteParameters { payment_params: params.clone(), final_value_msat: path_last_hop.fee_msat, - final_cltv_expiry_delta: params.final_cltv_expiry_delta.unwrap(), }); } else if let Some(params) = payment_params { retry = Some(RouteParameters { payment_params: params.clone(), final_value_msat: path_last_hop.fee_msat, - final_cltv_expiry_delta: - if let Some(delta) = params.final_cltv_expiry_delta { delta } - else { path_last_hop.cltv_expiry_delta }, }); } @@ -1330,7 +1312,9 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, (0, session_privs, required), (1, pending_fee_msat, option), (2, payment_hash, required), - (3, payment_params, option), + // Note that while we "default" payment_param's final CLTV expiry delta to 0 we should + // never see it - `payment_params` was added here after the field was added/required. + (3, payment_params, (option: ReadableArgs, 0)), (4, payment_secret, option), (5, keysend_preimage, option), (6, total_msat, required), @@ -1386,7 +1370,6 @@ mod tests { let expired_route_params = RouteParameters { payment_params, final_value_msat: 0, - final_cltv_expiry_delta: 0, }; let pending_events = Mutex::new(Vec::new()); if on_retry { @@ -1428,7 +1411,6 @@ mod tests { let route_params = RouteParameters { payment_params, final_value_msat: 0, - final_cltv_expiry_delta: 0, }; router.expect_find_route(route_params.clone(), Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError })); @@ -1471,7 +1453,6 @@ mod tests { let route_params = RouteParameters { payment_params: payment_params.clone(), final_value_msat: 0, - final_cltv_expiry_delta: 0, }; let failed_scid = 42; let route = Route { diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 4aa14dfa..72513087 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -98,7 +98,6 @@ fn mpp_retry() { let mut route_params = RouteParameters { payment_params: route.payment_params.clone().unwrap(), final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone())); @@ -297,7 +296,6 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) { let route_params = RouteParameters { payment_params: route.payment_params.clone().unwrap(), final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); check_added_monitors!(nodes[0], 1); @@ -1387,12 +1385,12 @@ fn do_test_intercepted_payment(test: InterceptTest) { let route_params = RouteParameters { payment_params, final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; let route = get_route( &nodes[0].node.get_our_node_id(), &route_params.payment_params, &nodes[0].network_graph.read_only(), None, route_params.final_value_msat, - route_params.final_cltv_expiry_delta, nodes[0].logger, &scorer, &random_seed_bytes + route_params.payment_params.final_cltv_expiry_delta, nodes[0].logger, &scorer, + &random_seed_bytes, ).unwrap(); let (payment_hash, payment_secret) = nodes[2].node.create_inbound_payment(Some(amt_msat), 60 * 60, None).unwrap(); @@ -1577,7 +1575,6 @@ fn do_automatic_retries(test: AutoRetry) { let route_params = RouteParameters { payment_params, final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; let (_, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], amt_msat); @@ -1787,7 +1784,6 @@ fn auto_retry_partial_failure() { let route_params = RouteParameters { payment_params, final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; // Ensure the first monitor update (for the initial send path1 over chan_1) succeeds, but the @@ -1860,12 +1856,12 @@ fn auto_retry_partial_failure() { let mut payment_params = route_params.payment_params.clone(); payment_params.previously_failed_channels.push(chan_2_id); nodes[0].router.expect_find_route(RouteParameters { - payment_params, final_value_msat: amt_msat / 2, final_cltv_expiry_delta: TEST_FINAL_CLTV + payment_params, final_value_msat: amt_msat / 2, }, Ok(retry_1_route)); let mut payment_params = route_params.payment_params.clone(); payment_params.previously_failed_channels.push(chan_3_id); nodes[0].router.expect_find_route(RouteParameters { - payment_params, final_value_msat: amt_msat / 4, final_cltv_expiry_delta: TEST_FINAL_CLTV + payment_params, final_value_msat: amt_msat / 4, }, Ok(retry_2_route)); // Send a payment that will partially fail on send, then partially fail on retry, then succeed. @@ -1999,7 +1995,6 @@ fn auto_retry_zero_attempts_send_error() { let route_params = RouteParameters { payment_params, final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure); @@ -2039,7 +2034,6 @@ fn fails_paying_after_rejected_by_payee() { let route_params = RouteParameters { payment_params, final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); @@ -2086,7 +2080,6 @@ fn retry_multi_path_single_failed_payment() { let route_params = RouteParameters { payment_params: payment_params.clone(), final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; let chans = nodes[0].node.list_usable_channels(); @@ -2121,7 +2114,7 @@ fn retry_multi_path_single_failed_payment() { payment_params: pay_params, // Note that the second request here requests the amount we originally failed to send, // not the amount remaining on the full payment, which should be changed. - final_value_msat: 100_000_001, final_cltv_expiry_delta: TEST_FINAL_CLTV + final_value_msat: 100_000_001, }, Ok(route.clone())); { @@ -2180,7 +2173,6 @@ fn immediate_retry_on_failure() { let route_params = RouteParameters { payment_params, final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; let chans = nodes[0].node.list_usable_channels(); @@ -2207,7 +2199,6 @@ fn immediate_retry_on_failure() { pay_params.previously_failed_channels.push(chans[0].short_channel_id.unwrap()); nodes[0].router.expect_find_route(RouteParameters { payment_params: pay_params, final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV }, Ok(route.clone())); nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); @@ -2270,7 +2261,6 @@ fn no_extra_retries_on_back_to_back_fail() { let route_params = RouteParameters { payment_params, final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; let mut route = Route { @@ -2316,7 +2306,7 @@ fn no_extra_retries_on_back_to_back_fail() { route.paths[0][1].fee_msat = amt_msat; nodes[0].router.expect_find_route(RouteParameters { payment_params: second_payment_params, - final_value_msat: amt_msat, final_cltv_expiry_delta: TEST_FINAL_CLTV, + final_value_msat: amt_msat, }, Ok(route.clone())); nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); @@ -2471,7 +2461,6 @@ fn test_simple_partial_retry() { let route_params = RouteParameters { payment_params, final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; let mut route = Route { @@ -2516,7 +2505,7 @@ fn test_simple_partial_retry() { route.paths.remove(0); nodes[0].router.expect_find_route(RouteParameters { payment_params: second_payment_params, - final_value_msat: amt_msat / 2, final_cltv_expiry_delta: TEST_FINAL_CLTV, + final_value_msat: amt_msat / 2, }, Ok(route.clone())); nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); @@ -2637,7 +2626,6 @@ fn test_threaded_payment_retries() { let mut route_params = RouteParameters { payment_params, final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, }; let mut route = Route { diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 0a3c5406..9682503e 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -19,7 +19,7 @@ use crate::ln::features::{ChannelFeatures, InvoiceFeatures, NodeFeatures}; use crate::ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT}; use crate::routing::gossip::{DirectedChannelInfo, EffectiveCapacity, ReadOnlyNetworkGraph, NetworkGraph, NodeId, RoutingFees}; use crate::routing::scoring::{ChannelUsage, LockableScore, Score}; -use crate::util::ser::{Writeable, Readable, Writer}; +use crate::util::ser::{Writeable, Readable, ReadableArgs, Writer}; use crate::util::logger::{Level, Logger}; use crate::util::chacha20::ChaCha20; @@ -315,18 +315,21 @@ impl Readable for Route { let path_count: u64 = Readable::read(reader)?; if path_count == 0 { return Err(DecodeError::InvalidValue); } let mut paths = Vec::with_capacity(cmp::min(path_count, 128) as usize); + let mut min_final_cltv_expiry_delta = u32::max_value(); for _ in 0..path_count { let hop_count: u8 = Readable::read(reader)?; - let mut hops = Vec::with_capacity(hop_count as usize); + let mut hops: Vec = Vec::with_capacity(hop_count as usize); for _ in 0..hop_count { hops.push(Readable::read(reader)?); } if hops.is_empty() { return Err(DecodeError::InvalidValue); } + min_final_cltv_expiry_delta = + cmp::min(min_final_cltv_expiry_delta, hops.last().unwrap().cltv_expiry_delta); paths.push(hops); } let mut payment_params = None; read_tlv_fields!(reader, { - (1, payment_params, option), + (1, payment_params, (option: ReadableArgs, min_final_cltv_expiry_delta)), }); Ok(Route { paths, payment_params }) } @@ -345,19 +348,38 @@ pub struct RouteParameters { /// The amount in msats sent on the failed payment path. pub final_value_msat: u64, +} - /// The CLTV on the final hop of the failed payment path. - /// - /// This field is deprecated, [`PaymentParameters::final_cltv_expiry_delta`] should be used - /// instead, if available. - pub final_cltv_expiry_delta: u32, +impl Writeable for RouteParameters { + fn write(&self, writer: &mut W) -> Result<(), io::Error> { + write_tlv_fields!(writer, { + (0, self.payment_params, required), + (2, self.final_value_msat, required), + // LDK versions prior to 0.0.114 had the `final_cltv_expiry_delta` parameter in + // `RouteParameters` directly. For compatibility, we write it here. + (4, self.payment_params.final_cltv_expiry_delta, required), + }); + Ok(()) + } } -impl_writeable_tlv_based!(RouteParameters, { - (0, payment_params, required), - (2, final_value_msat, required), - (4, final_cltv_expiry_delta, required), -}); +impl Readable for RouteParameters { + fn read(reader: &mut R) -> Result { + _init_and_read_tlv_fields!(reader, { + (0, payment_params, (required: ReadableArgs, 0)), + (2, final_value_msat, required), + (4, final_cltv_expiry_delta, required), + }); + let mut payment_params: PaymentParameters = payment_params.0.unwrap(); + if payment_params.final_cltv_expiry_delta == 0 { + payment_params.final_cltv_expiry_delta = final_cltv_expiry_delta.0.unwrap(); + } + Ok(Self { + payment_params, + final_value_msat: final_value_msat.0.unwrap(), + }) + } +} /// Maximum total CTLV difference we allow for a full payment path. pub const DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA: u32 = 1008; @@ -431,23 +453,54 @@ pub struct PaymentParameters { /// these SCIDs. pub previously_failed_channels: Vec, - /// The minimum CLTV delta at the end of the route. - /// - /// This field should always be set to `Some` and may be required in a future release. - pub final_cltv_expiry_delta: Option, + /// The minimum CLTV delta at the end of the route. This value must not be zero. + pub final_cltv_expiry_delta: u32, +} + +impl Writeable for PaymentParameters { + fn write(&self, writer: &mut W) -> Result<(), io::Error> { + write_tlv_fields!(writer, { + (0, self.payee_pubkey, required), + (1, self.max_total_cltv_expiry_delta, required), + (2, self.features, option), + (3, self.max_path_count, required), + (4, self.route_hints, vec_type), + (5, self.max_channel_saturation_power_of_half, required), + (6, self.expiry_time, option), + (7, self.previously_failed_channels, vec_type), + (9, self.final_cltv_expiry_delta, required), + }); + Ok(()) + } +} + +impl ReadableArgs for PaymentParameters { + fn read(reader: &mut R, default_final_cltv_expiry_delta: u32) -> Result { + _init_and_read_tlv_fields!(reader, { + (0, payee_pubkey, required), + (1, max_total_cltv_expiry_delta, (default_value, DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA)), + (2, features, option), + (3, max_path_count, (default_value, DEFAULT_MAX_PATH_COUNT)), + (4, route_hints, vec_type), + (5, max_channel_saturation_power_of_half, (default_value, 2)), + (6, expiry_time, option), + (7, previously_failed_channels, vec_type), + (9, final_cltv_expiry_delta, (default_value, default_final_cltv_expiry_delta)), + }); + Ok(Self { + payee_pubkey: _init_tlv_based_struct_field!(payee_pubkey, required), + max_total_cltv_expiry_delta: _init_tlv_based_struct_field!(max_total_cltv_expiry_delta, (default_value, unused)), + features, + max_path_count: _init_tlv_based_struct_field!(max_path_count, (default_value, unused)), + route_hints: route_hints.unwrap_or(Vec::new()), + max_channel_saturation_power_of_half: _init_tlv_based_struct_field!(max_channel_saturation_power_of_half, (default_value, unused)), + expiry_time, + previously_failed_channels: previously_failed_channels.unwrap_or(Vec::new()), + final_cltv_expiry_delta: _init_tlv_based_struct_field!(final_cltv_expiry_delta, (default_value, unused)), + }) + } } -impl_writeable_tlv_based!(PaymentParameters, { - (0, payee_pubkey, required), - (1, max_total_cltv_expiry_delta, (default_value, DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA)), - (2, features, option), - (3, max_path_count, (default_value, DEFAULT_MAX_PATH_COUNT)), - (4, route_hints, vec_type), - (5, max_channel_saturation_power_of_half, (default_value, 2)), - (6, expiry_time, option), - (7, previously_failed_channels, vec_type), - (9, final_cltv_expiry_delta, option), -}); impl PaymentParameters { /// Creates a payee with the node id of the given `pubkey`. @@ -464,7 +517,7 @@ impl PaymentParameters { max_path_count: DEFAULT_MAX_PATH_COUNT, max_channel_saturation_power_of_half: 2, previously_failed_channels: Vec::new(), - final_cltv_expiry_delta: Some(final_cltv_expiry_delta), + final_cltv_expiry_delta, } } @@ -939,9 +992,7 @@ pub fn find_route( ) -> Result where L::Target: Logger, GL::Target: Logger { let graph_lock = network_graph.read_only(); - let final_cltv_expiry_delta = - if let Some(delta) = route_params.payment_params.final_cltv_expiry_delta { delta } - else { route_params.final_cltv_expiry_delta }; + let final_cltv_expiry_delta = route_params.payment_params.final_cltv_expiry_delta; let mut route = get_route(our_node_pubkey, &route_params.payment_params, &graph_lock, first_hops, route_params.final_value_msat, final_cltv_expiry_delta, logger, scorer, random_seed_bytes)?; @@ -980,9 +1031,9 @@ where L::Target: Logger { if payment_params.max_total_cltv_expiry_delta <= final_cltv_expiry_delta { return Err(LightningError{err: "Can't find a route where the maximum total CLTV expiry delta is below the final CLTV expiry.".to_owned(), action: ErrorAction::IgnoreError}); } - if let Some(delta) = payment_params.final_cltv_expiry_delta { - debug_assert_eq!(delta, final_cltv_expiry_delta); - } + + // TODO: Remove the explicit final_cltv_expiry_delta parameter + debug_assert_eq!(final_cltv_expiry_delta, payment_params.final_cltv_expiry_delta); // The general routing idea is the following: // 1. Fill first/last hops communicated by the caller. @@ -2021,7 +2072,8 @@ where L::Target: Logger, GL::Target: Logger { let graph_lock = network_graph.read_only(); let mut route = build_route_from_hops_internal( our_node_pubkey, hops, &route_params.payment_params, &graph_lock, - route_params.final_value_msat, route_params.final_cltv_expiry_delta, logger, random_seed_bytes)?; + route_params.final_value_msat, route_params.payment_params.final_cltv_expiry_delta, + logger, random_seed_bytes)?; add_random_cltv_offset(&mut route, &route_params.payment_params, &graph_lock, random_seed_bytes); Ok(route) } -- 2.30.2