Merge pull request #2006 from TheBlueMatt/2023-02-no-recursive-read-locks v0.0.114-beta
authorWilmer Paulino <9447167+wpaulino@users.noreply.github.com>
Tue, 28 Feb 2023 08:24:16 +0000 (00:24 -0800)
committerGitHub <noreply@github.com>
Tue, 28 Feb 2023 08:24:16 +0000 (00:24 -0800)
Refuse recursive read locks

fuzz/src/full_stack.rs
fuzz/src/router.rs
lightning-invoice/src/payment.rs
lightning-invoice/src/utils.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/outbound_payment.rs
lightning/src/ln/payment_tests.rs
lightning/src/routing/router.rs
lightning/src/util/ser.rs
lightning/src/util/ser_macros.rs

index e7d10a341629db7d5e190945e51f8e7959fc02a2..b96de0b9782443f9c4fd72da7b861a2c6727a420 100644 (file)
@@ -513,7 +513,6 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
                                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, &params, &network_graph, None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
@@ -537,7 +536,6 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
                                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, &params, &network_graph, None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
index 93de35c5d094783d8bdc4e9b6c6a935b39088f34..4c228cc731bc2d0981ad77c9a0e2ff444f243302 100644 (file)
@@ -301,7 +301,6 @@ pub fn do_test<Out: test_logger::Output>(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::<Vec<_>>()).as_ref().map(|a| a.as_slice()),
index db47f9954800a2296785bdabd622aa31b7bd41c4..981fd2e6ba49225c0e41fb2923ee56e480aea0b7 100644 (file)
@@ -156,7 +156,6 @@ fn pay_invoice_using_amount<P: Deref>(
        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)
index 868f0b297b1162da0551c8923b4ced23ab880ccb..11a779b1028a8ddceb825cc833cceb57856d52f0 100644 (file)
@@ -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;
index 1045e77ad04f32a22d00263e33cbdb83b2adc77e..0757e117ce2e7660648856053070c12773b2b6b1 100644 (file)
@@ -6774,27 +6774,36 @@ impl Readable for HTLCSource {
                        0 => {
                                let mut session_priv: crate::util::ser::RequiredWrapper<SecretKey> = crate::util::ser::RequiredWrapper(None);
                                let mut first_hop_htlc_msat: u64 = 0;
-                               let mut path = Some(Vec::new());
+                               let mut path: Option<Vec<RouteHop>> = Some(Vec::new());
                                let mut payment_id = None;
                                let mut payment_secret = None;
-                               let mut payment_params = None;
+                               let mut payment_params: Option<PaymentParameters> = 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
                                        // instead.
                                        payment_id = Some(PaymentId(*session_priv.0.unwrap().as_ref()));
                                }
+                               if path.is_none() || path.as_ref().unwrap().is_empty() {
+                                       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,
-                                       path: path.unwrap(),
+                                       path,
                                        payment_id: payment_id.unwrap(),
                                        payment_secret,
                                        payment_params,
@@ -7998,7 +8007,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,
@@ -8089,7 +8097,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();
@@ -8132,7 +8139,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();
index 36f10f742be8c4c808e868d445a39f5b2f88994e..454fbe2b7819889e3246314a90ac82eed17f40f9 100644 (file)
@@ -9244,7 +9244,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();
@@ -9275,7 +9274,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();
index 15bba61dda408e208e6f9d9bf5d341ce116812be..33ccecb4ca858b17c45321b3a17c2303e6931587 100644 (file)
@@ -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 {
index 4dd1b9009763a3bdfae43e3944d9c30f29ea5dc4..c4cd0fc1b09d1ca691654c7fbc32e256bb39ed41 100644 (file)
@@ -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);
@@ -1388,12 +1386,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();
@@ -1578,7 +1576,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);
 
@@ -1788,7 +1785,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
@@ -1861,12 +1857,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.
@@ -2000,7 +1996,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);
@@ -2040,7 +2035,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();
@@ -2087,7 +2081,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();
@@ -2122,7 +2115,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()));
 
        {
@@ -2181,7 +2174,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();
@@ -2208,7 +2200,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();
@@ -2271,7 +2262,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 {
@@ -2317,7 +2307,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();
@@ -2472,7 +2462,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 {
@@ -2517,7 +2506,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();
@@ -2638,7 +2627,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 {
index b456e15a2d13cdce9b6090889e1e886ae17a997c..9682503ee45c22d5062a099f5c1af015bbe3ffc0 100644 (file)
@@ -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;
 
@@ -313,18 +313,23 @@ impl Readable for Route {
        fn read<R: io::Read>(reader: &mut R) -> Result<Route, DecodeError> {
                let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
                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<RouteHop> = 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 })
        }
@@ -343,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<W: Writer>(&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<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+               _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;
@@ -429,23 +453,54 @@ pub struct PaymentParameters {
        /// these SCIDs.
        pub previously_failed_channels: Vec<u64>,
 
-       /// 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<u32>,
+       /// 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<W: Writer>(&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<u32> for PaymentParameters {
+       fn read<R: io::Read>(reader: &mut R, default_final_cltv_expiry_delta: u32) -> Result<Self, DecodeError> {
+               _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`.
@@ -462,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,
                }
        }
 
@@ -937,9 +992,7 @@ pub fn find_route<L: Deref, GL: Deref, S: Score>(
 ) -> Result<Route, LightningError>
 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)?;
@@ -978,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.
@@ -2019,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)
 }
index 5adf5758131bb65e9929042a267162e3665c58e2..14c25775174b19dc50f4dabe5ecb434ce0ea259b 100644 (file)
@@ -289,17 +289,23 @@ impl<T: Readable> MaybeReadable for T {
 }
 
 /// Wrapper to read a required (non-optional) TLV record.
-pub struct RequiredWrapper<T: Readable>(pub Option<T>);
+pub struct RequiredWrapper<T>(pub Option<T>);
 impl<T: Readable> Readable for RequiredWrapper<T> {
        #[inline]
        fn read<R: Read>(reader: &mut R) -> Result<Self, DecodeError> {
                Ok(Self(Some(Readable::read(reader)?)))
        }
 }
+impl<A, T: ReadableArgs<A>> ReadableArgs<A> for RequiredWrapper<T> {
+       #[inline]
+       fn read<R: Read>(reader: &mut R, args: A) -> Result<Self, DecodeError> {
+               Ok(Self(Some(ReadableArgs::read(reader, args)?)))
+       }
+}
 /// When handling `default_values`, we want to map the default-value T directly
 /// to a `RequiredWrapper<T>` in a way that works for `field: T = t;` as
 /// well. Thus, we assume `Into<T> for T` does nothing and use that.
-impl<T: Readable> From<T> for RequiredWrapper<T> {
+impl<T> From<T> for RequiredWrapper<T> {
        fn from(t: T) -> RequiredWrapper<T> { RequiredWrapper(Some(t)) }
 }
 
index 5e55ab1d3b8182baf80f87e0e1a929d90380057c..59402f6acc8e0eab43810eb896a678186e2e9efc 100644 (file)
@@ -51,6 +51,10 @@ macro_rules! _encode_tlv {
        ($stream: expr, $type: expr, $field: expr, (option, encoding: $fieldty: ty)) => {
                $crate::_encode_tlv!($stream, $type, $field, option);
        };
+       ($stream: expr, $type: expr, $field: expr, (option: $trait: ident $(, $read_arg: expr)?)) => {
+               // Just a read-mapped type
+               $crate::_encode_tlv!($stream, $type, $field, option);
+       };
 }
 
 /// Panics if the last seen TLV type is not numerically less than the TLV type currently being checked.
@@ -161,6 +165,9 @@ macro_rules! _get_varint_length_prefixed_tlv_length {
                        $len.0 += field_len;
                }
        };
+       ($len: expr, $type: expr, $field: expr, (option: $trait: ident $(, $read_arg: expr)?)) => {
+               $crate::_get_varint_length_prefixed_tlv_length!($len, $type, $field, option);
+       };
        ($len: expr, $type: expr, $field: expr, upgradable_required) => {
                $crate::_get_varint_length_prefixed_tlv_length!($len, $type, $field, required);
        };
@@ -210,6 +217,9 @@ macro_rules! _check_decoded_tlv_order {
                        return Err(DecodeError::InvalidValue);
                }
        }};
+       ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (required: $trait: ident $(, $read_arg: expr)?)) => {{
+               $crate::_check_decoded_tlv_order!($last_seen_type, $typ, $type, $field, required);
+       }};
        ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, option) => {{
                // no-op
        }};
@@ -252,6 +262,9 @@ macro_rules! _check_missing_tlv {
                        return Err(DecodeError::InvalidValue);
                }
        }};
+       ($last_seen_type: expr, $type: expr, $field: ident, (required: $trait: ident $(, $read_arg: expr)?)) => {{
+               $crate::_check_missing_tlv!($last_seen_type, $type, $field, required);
+       }};
        ($last_seen_type: expr, $type: expr, $field: ident, vec_type) => {{
                // no-op
        }};
@@ -285,6 +298,9 @@ macro_rules! _decode_tlv {
        ($reader: expr, $field: ident, required) => {{
                $field = $crate::util::ser::Readable::read(&mut $reader)?;
        }};
+       ($reader: expr, $field: ident, (required: $trait: ident $(, $read_arg: expr)?)) => {{
+               $field = $trait::read(&mut $reader $(, $read_arg)*)?;
+       }};
        ($reader: expr, $field: ident, vec_type) => {{
                let f: $crate::util::ser::WithoutLength<Vec<_>> = $crate::util::ser::Readable::read(&mut $reader)?;
                $field = Some(f.0);
@@ -644,6 +660,9 @@ macro_rules! _init_tlv_based_struct_field {
        ($field: ident, option) => {
                $field
        };
+       ($field: ident, (option: $trait: ident $(, $read_arg: expr)?)) => {
+               $crate::_init_tlv_based_struct_field!($field, option)
+       };
        ($field: ident, upgradable_required) => {
                $field.0.unwrap()
        };
@@ -673,12 +692,18 @@ macro_rules! _init_tlv_field_var {
        ($field: ident, required) => {
                let mut $field = $crate::util::ser::RequiredWrapper(None);
        };
+       ($field: ident, (required: $trait: ident $(, $read_arg: expr)?)) => {
+               $crate::_init_tlv_field_var!($field, required);
+       };
        ($field: ident, vec_type) => {
                let mut $field = Some(Vec::new());
        };
        ($field: ident, option) => {
                let mut $field = None;
        };
+       ($field: ident, (option: $trait: ident $(, $read_arg: expr)?)) => {
+               $crate::_init_tlv_field_var!($field, option);
+       };
        ($field: ident, upgradable_required) => {
                let mut $field = $crate::util::ser::UpgradableRequired(None);
        };