From b2ce9d387480c7e4131c9e7aa5047c7711aa3cf7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 31 Oct 2021 18:19:39 +0000 Subject: [PATCH 1/6] Remove trailing ;s from macro calls to silence new rustc warnings --- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/util/macro_logger.rs | 2 +- lightning/src/util/ser_macros.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 17a287c74..4ebbc3e55 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1141,7 +1141,7 @@ macro_rules! handle_monitor_err { res } }; ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr) => { - handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, Vec::new()); + handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, Vec::new()) } } diff --git a/lightning/src/util/macro_logger.rs b/lightning/src/util/macro_logger.rs index 788bc5eca..747752613 100644 --- a/lightning/src/util/macro_logger.rs +++ b/lightning/src/util/macro_logger.rs @@ -213,6 +213,6 @@ macro_rules! log_debug { #[macro_export] macro_rules! log_trace { ($logger: expr, $($arg:tt)*) => ( - log_given_level!($logger, $crate::util::logger::Level::Trace, $($arg)*); + log_given_level!($logger, $crate::util::logger::Level::Trace, $($arg)*) ) } diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 0827a023a..165d1f1ed 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -310,7 +310,7 @@ macro_rules! write_ver_prefix { /// correctly. macro_rules! write_tlv_fields { ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => { - encode_varint_length_prefixed_tlv!($stream, {$(($type, $field, $fieldty)),*}); + encode_varint_length_prefixed_tlv!($stream, {$(($type, $field, $fieldty)),*}) } } From 2ec427f1486684db31eeb01671fa36a78404bab3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 31 Oct 2021 18:20:27 +0000 Subject: [PATCH 2/6] Rename Payee::new to Payee::from_node_id to clarify it somewhat This also differentiates it from the bindings default-constructed `new` method which is constructed when all fields are exposed and of mappable types. --- fuzz/src/full_stack.rs | 4 +- fuzz/src/router.rs | 2 +- lightning-invoice/src/payment.rs | 8 +-- lightning-invoice/src/utils.rs | 2 +- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/ln/functional_test_utils.rs | 6 +- lightning/src/ln/functional_tests.rs | 6 +- lightning/src/ln/shutdown_tests.rs | 4 +- lightning/src/routing/router.rs | 76 +++++++++++------------ 9 files changed, 55 insertions(+), 55 deletions(-) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 790178692..829ef20b0 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -438,7 +438,7 @@ pub fn do_test(data: &[u8], logger: &Arc) { }, 4 => { let final_value_msat = slice_to_be24(get_slice!(3)) as u64; - let payee = Payee::new(get_pubkey!()); + let payee = Payee::from_node_id(get_pubkey!()); let params = RouteParameters { payee, final_value_msat, @@ -461,7 +461,7 @@ pub fn do_test(data: &[u8], logger: &Arc) { }, 15 => { let final_value_msat = slice_to_be24(get_slice!(3)) as u64; - let payee = Payee::new(get_pubkey!()); + let payee = Payee::from_node_id(get_pubkey!()); let params = RouteParameters { payee, final_value_msat, diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index abd83fa58..8c9b4b7d8 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -251,7 +251,7 @@ pub fn do_test(data: &[u8], out: Out) { let scorer = Scorer::with_fixed_penalty(0); for target in node_pks.iter() { let params = RouteParameters { - payee: Payee::new(*target).with_route_hints(last_hops.clone()), + payee: Payee::from_node_id(*target).with_route_hints(last_hops.clone()), final_value_msat: slice_to_be64(get_slice!(8)), final_cltv_expiry_delta: slice_to_be32(get_slice!(4)), }; diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index a5160ea9b..b29fe18f5 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -262,7 +262,7 @@ where match payment_cache.entry(payment_hash) { hash_map::Entry::Vacant(entry) => { let payer = self.payer.node_id(); - let mut payee = Payee::new(invoice.recover_payee_pub_key()) + let mut payee = Payee::from_node_id(invoice.recover_payee_pub_key()) .with_expiry_time(expiry_time_from_unix_epoch(&invoice).as_secs()) .with_route_hints(invoice.route_hints()); if let Some(features) = invoice.features() { @@ -1034,7 +1034,7 @@ mod tests { } fn retry_for_invoice(invoice: &Invoice) -> RouteParameters { - let mut payee = Payee::new(invoice.recover_payee_pub_key()) + let mut payee = Payee::from_node_id(invoice.recover_payee_pub_key()) .with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs()) .with_route_hints(invoice.route_hints()); if let Some(features) = invoice.features() { @@ -1266,7 +1266,7 @@ mod tests { cltv_expiry_delta: 100, }], ], - payee: Some(Payee::new(nodes[1].node.get_our_node_id())), + payee: Some(Payee::from_node_id(nodes[1].node.get_our_node_id())), }; let router = ManualRouter(RefCell::new(VecDeque::new())); router.expect_find_route(Ok(route.clone())); @@ -1309,7 +1309,7 @@ mod tests { cltv_expiry_delta: 100, }], ], - payee: Some(Payee::new(nodes[1].node.get_our_node_id())), + payee: Some(Payee::from_node_id(nodes[1].node.get_our_node_id())), }; let router = ManualRouter(RefCell::new(VecDeque::new())); router.expect_find_route(Ok(route.clone())); diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index c7cd048be..35a74b6a5 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -171,7 +171,7 @@ mod test { assert_eq!(invoice.min_final_cltv_expiry(), MIN_FINAL_CLTV_EXPIRY as u64); assert_eq!(invoice.description(), InvoiceDescription::Direct(&Description("test".to_string()))); - let payee = Payee::new(invoice.recover_payee_pub_key()) + let payee = Payee::from_node_id(invoice.recover_payee_pub_key()) .with_features(invoice.features().unwrap().clone()) .with_route_hints(invoice.route_hints()); let params = RouteParameters { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 4ebbc3e55..a5f84abfd 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6646,7 +6646,7 @@ pub mod bench { macro_rules! send_payment { ($node_a: expr, $node_b: expr) => { let usable_channels = $node_a.list_usable_channels(); - let payee = Payee::new($node_b.get_our_node_id()) + let payee = Payee::from_node_id($node_b.get_our_node_id()) .with_features(InvoiceFeatures::known()); let scorer = Scorer::with_fixed_penalty(0); let route = get_route(&$node_a.get_our_node_id(), &payee, &dummy_graph, diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 515fc79e8..e6b246707 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1012,7 +1012,7 @@ macro_rules! get_route_and_payment_hash { }}; ($send_node: expr, $recv_node: expr, $last_hops: expr, $recv_value: expr, $cltv: expr) => {{ let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!($recv_node, Some($recv_value)); - let payee = $crate::routing::router::Payee::new($recv_node.node.get_our_node_id()) + let payee = $crate::routing::router::Payee::from_node_id($recv_node.node.get_our_node_id()) .with_features($crate::ln::features::InvoiceFeatures::known()) .with_route_hints($last_hops); let scorer = ::util::test_utils::TestScorer::with_fixed_penalty(0); @@ -1350,7 +1350,7 @@ pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: pub const TEST_FINAL_CLTV: u32 = 70; pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) { - let payee = Payee::new(expected_route.last().unwrap().node.get_our_node_id()) + let payee = Payee::from_node_id(expected_route.last().unwrap().node.get_our_node_id()) .with_features(InvoiceFeatures::known()); let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = get_route( @@ -1368,7 +1368,7 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: } pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) { - let payee = Payee::new(expected_route.last().unwrap().node.get_our_node_id()) + let payee = Payee::from_node_id(expected_route.last().unwrap().node.get_our_node_id()) .with_features(InvoiceFeatures::known()); let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = get_route(&origin_node.node.get_our_node_id(), &payee, origin_node.network_graph, None, recv_value, TEST_FINAL_CLTV, origin_node.logger, &scorer).unwrap(); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 1bf981680..5e87aaff3 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -7161,7 +7161,7 @@ fn test_check_htlc_underpaying() { create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[1].node.get_our_node_id()).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[1].node.get_our_node_id()).with_features(InvoiceFeatures::known()); let route = get_route(&nodes[0].node.get_our_node_id(), &payee, nodes[0].network_graph, None, 10_000, TEST_FINAL_CLTV, nodes[0].logger, &scorer).unwrap(); let (_, our_payment_hash, _) = get_payment_preimage_hash!(nodes[0]); let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200, 0).unwrap(); @@ -7559,12 +7559,12 @@ fn test_bump_penalty_txn_on_revoked_htlcs() { let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 59000000, InitFeatures::known(), InitFeatures::known()); // Lock HTLC in both directions (using a slightly lower CLTV delay to provide timely RBF bumps) - let payee = Payee::new(nodes[1].node.get_our_node_id()).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[1].node.get_our_node_id()).with_features(InvoiceFeatures::known()); let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = get_route(&nodes[0].node.get_our_node_id(), &payee, &nodes[0].network_graph, None, 3_000_000, 50, nodes[0].logger, &scorer).unwrap(); let payment_preimage = send_along_route(&nodes[0], route, &[&nodes[1]], 3_000_000).0; - let payee = Payee::new(nodes[0].node.get_our_node_id()).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[0].node.get_our_node_id()).with_features(InvoiceFeatures::known()); let route = get_route(&nodes[1].node.get_our_node_id(), &payee, nodes[1].network_graph, None, 3_000_000, 50, nodes[0].logger, &scorer).unwrap(); send_along_route(&nodes[1], route, &[&nodes[0]], 3_000_000); diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index f72f71459..fcec4703e 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -96,9 +96,9 @@ fn updates_shutdown_wait() { let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[0]); - let payee_1 = Payee::new(nodes[1].node.get_our_node_id()).with_features(InvoiceFeatures::known()); + let payee_1 = Payee::from_node_id(nodes[1].node.get_our_node_id()).with_features(InvoiceFeatures::known()); let route_1 = get_route(&nodes[0].node.get_our_node_id(), &payee_1, nodes[0].network_graph, None, 100000, TEST_FINAL_CLTV, &logger, &scorer).unwrap(); - let payee_2 = Payee::new(nodes[0].node.get_our_node_id()).with_features(InvoiceFeatures::known()); + let payee_2 = Payee::from_node_id(nodes[0].node.get_our_node_id()).with_features(InvoiceFeatures::known()); let route_2 = get_route(&nodes[1].node.get_our_node_id(), &payee_2, nodes[1].network_graph, None, 100000, TEST_FINAL_CLTV, &logger, &scorer).unwrap(); unwrap_send_err!(nodes[0].node.send_payment(&route_1, payment_hash, &Some(payment_secret)), true, APIError::ChannelUnavailable {..}, {}); unwrap_send_err!(nodes[1].node.send_payment(&route_2, payment_hash, &Some(payment_secret)), true, APIError::ChannelUnavailable {..}, {}); diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 633775bfe..724765a18 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -203,7 +203,7 @@ impl_writeable_tlv_based!(Payee, { impl Payee { /// Creates a payee with the node id of the given `pubkey`. - pub fn new(pubkey: PublicKey) -> Self { + pub fn from_node_id(pubkey: PublicKey) -> Self { Self { pubkey, features: None, @@ -214,7 +214,7 @@ impl Payee { /// Creates a payee with the node id of the given `pubkey` to use for keysend payments. pub fn for_keysend(pubkey: PublicKey) -> Self { - Self::new(pubkey).with_features(InvoiceFeatures::for_keysend()) + Self::from_node_id(pubkey).with_features(InvoiceFeatures::for_keysend()) } /// Includes the payee's features. @@ -1941,7 +1941,7 @@ mod tests { fn simple_route_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[2]); + let payee = Payee::from_node_id(nodes[2]); let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Simple route to 2 via 1 @@ -1972,7 +1972,7 @@ mod tests { fn invalid_first_hop_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[2]); + let payee = Payee::from_node_id(nodes[2]); let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Simple route to 2 via 1 @@ -1991,7 +1991,7 @@ mod tests { fn htlc_minimum_test() { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[2]); + let payee = Payee::from_node_id(nodes[2]); let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Simple route to 2 via 1 @@ -2116,7 +2116,7 @@ mod tests { fn htlc_minimum_overpay_test() { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[2]).with_features(InvoiceFeatures::known()); let scorer = test_utils::TestScorer::with_fixed_penalty(0); // A route to node#2 via two paths. @@ -2252,7 +2252,7 @@ mod tests { fn disable_channels_test() { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[2]); + let payee = Payee::from_node_id(nodes[2]); let scorer = test_utils::TestScorer::with_fixed_penalty(0); // // Disable channels 4 and 12 by flags=2 @@ -2310,7 +2310,7 @@ mod tests { fn disable_node_test() { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[2]); + let payee = Payee::from_node_id(nodes[2]); let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Disable nodes 1, 2, and 8 by requiring unknown feature bits @@ -2355,7 +2355,7 @@ mod tests { let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Route to 1 via 2 and 3 because our channel to 1 is disabled - let payee = Payee::new(nodes[0]); + let payee = Payee::from_node_id(nodes[0]); let route = get_route(&our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer).unwrap(); assert_eq!(route.paths[0].len(), 3); @@ -2381,7 +2381,7 @@ mod tests { assert_eq!(route.paths[0][2].channel_features.le_flags(), &id_to_feature_flags(3)); // If we specify a channel to node7, that overrides our local channel view and that gets used - let payee = Payee::new(nodes[2]); + let payee = Payee::from_node_id(nodes[2]); let our_chans = vec![get_channel_details(Some(42), nodes[7].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)]; let route = get_route(&our_id, &payee, &network_graph, Some(&our_chans.iter().collect::>()), 100, 42, Arc::clone(&logger), &scorer).unwrap(); assert_eq!(route.paths[0].len(), 2); @@ -2503,13 +2503,13 @@ mod tests { let mut invalid_last_hops = last_hops_multi_private_channels(&nodes); invalid_last_hops.push(invalid_last_hop); { - let payee = Payee::new(nodes[6]).with_route_hints(invalid_last_hops); + let payee = Payee::from_node_id(nodes[6]).with_route_hints(invalid_last_hops); if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer) { assert_eq!(err, "Route hint cannot have the payee as the source."); } else { panic!(); } } - let payee = Payee::new(nodes[6]).with_route_hints(last_hops_multi_private_channels(&nodes)); + let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops_multi_private_channels(&nodes)); let route = get_route(&our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer).unwrap(); assert_eq!(route.paths[0].len(), 5); @@ -2579,7 +2579,7 @@ mod tests { fn ignores_empty_last_hops_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[6]).with_route_hints(empty_last_hop(&nodes)); + let payee = Payee::from_node_id(nodes[6]).with_route_hints(empty_last_hop(&nodes)); let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Test handling of an empty RouteHint passed in Invoice. @@ -2661,7 +2661,7 @@ mod tests { fn multi_hint_last_hops_test() { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[6]).with_route_hints(multi_hint_last_hops(&nodes)); + let payee = Payee::from_node_id(nodes[6]).with_route_hints(multi_hint_last_hops(&nodes)); let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Test through channels 2, 3, 5, 8. // Test shows that multiple hop hints are considered. @@ -2767,7 +2767,7 @@ mod tests { fn last_hops_with_public_channel_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[6]).with_route_hints(last_hops_with_public_channel(&nodes)); + let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops_with_public_channel(&nodes)); let scorer = test_utils::TestScorer::with_fixed_penalty(0); // This test shows that public routes can be present in the invoice // which would be handled in the same manner. @@ -2822,7 +2822,7 @@ mod tests { // Simple test with outbound channel to 4 to test that last_hops and first_hops connect let our_chans = vec![get_channel_details(Some(42), nodes[3].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)]; let mut last_hops = last_hops(&nodes); - let payee = Payee::new(nodes[6]).with_route_hints(last_hops.clone()); + let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops.clone()); let route = get_route(&our_id, &payee, &network_graph, Some(&our_chans.iter().collect::>()), 100, 42, Arc::clone(&logger), &scorer).unwrap(); assert_eq!(route.paths[0].len(), 2); @@ -2843,7 +2843,7 @@ mod tests { last_hops[0].0[0].fees.base_msat = 1000; // Revert to via 6 as the fee on 8 goes up - let payee = Payee::new(nodes[6]).with_route_hints(last_hops); + let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops); let route = get_route(&our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer).unwrap(); assert_eq!(route.paths[0].len(), 4); @@ -2936,7 +2936,7 @@ mod tests { htlc_minimum_msat: None, htlc_maximum_msat: last_hop_htlc_max, }]); - let payee = Payee::new(target_node_id).with_route_hints(vec![last_hops]); + let payee = Payee::from_node_id(target_node_id).with_route_hints(vec![last_hops]); let our_chans = vec![get_channel_details(Some(42), middle_node_id, InitFeatures::from_le_bytes(vec![0b11]), outbound_capacity_msat)]; let scorer = test_utils::TestScorer::with_fixed_penalty(0); get_route(&source_node_id, &payee, &NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()), Some(&our_chans.iter().collect::>()), route_val, 42, &test_utils::TestLogger::new(), &scorer) @@ -2993,7 +2993,7 @@ mod tests { let (secp_ctx, network_graph, mut net_graph_msg_handler, chain_monitor, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[2]).with_features(InvoiceFeatures::known()); // We will use a simple single-path route from // our node to node2 via node0: channels {1, 3}. @@ -3265,7 +3265,7 @@ mod tests { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[3]).with_features(InvoiceFeatures::known()); // Path via {node7, node2, node4} is channels {12, 13, 6, 11}. // {12, 13, 11} have the capacities of 100, {6} has a capacity of 50. @@ -3388,7 +3388,7 @@ mod tests { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[2]); + let payee = Payee::from_node_id(nodes[2]); // Path via node0 is channels {1, 3}. Limit them to 100 and 50 sats (total limit 50). update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate { @@ -3434,7 +3434,7 @@ mod tests { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[2]).with_features(InvoiceFeatures::known()); // We need a route consisting of 3 paths: // From our node to node2 via node0, node7, node1 (three paths one hop each). @@ -3565,7 +3565,7 @@ mod tests { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[3]).with_features(InvoiceFeatures::known()); // We need a route consisting of 3 paths: // From our node to node3 via {node0, node2}, {node7, node2, node4} and {node7, node2}. @@ -3727,7 +3727,7 @@ mod tests { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[3]).with_features(InvoiceFeatures::known()); // This test checks that if we have two cheaper paths and one more expensive path, // so that liquidity-wise any 2 of 3 combination is sufficient, @@ -3894,7 +3894,7 @@ mod tests { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[3]).with_features(InvoiceFeatures::known()); // We need a route consisting of 2 paths: // From our node to node3 via {node0, node2} and {node7, node2, node4}. @@ -4063,7 +4063,7 @@ mod tests { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[2]).with_features(InvoiceFeatures::known()); // We need a route consisting of 3 paths: // From our node to node2 via node0, node7, node1 (three paths one hop each). @@ -4220,7 +4220,7 @@ mod tests { let net_graph_msg_handler = NetGraphMsgHandler::new(Arc::clone(&network_graph), None, Arc::clone(&logger)); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[6]); + let payee = Payee::from_node_id(nodes[6]); add_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, &privkeys[1], ChannelFeatures::from_le_bytes(id_to_feature_flags(6)), 6); update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate { @@ -4349,7 +4349,7 @@ mod tests { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, _, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[2]); + let payee = Payee::from_node_id(nodes[2]); // We modify the graph to set the htlc_maximum of channel 2 to below the value we wish to // send. @@ -4411,7 +4411,7 @@ mod tests { let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[2]).with_features(InvoiceFeatures::known()); // We modify the graph to set the htlc_minimum of channel 2 and 4 as needed - channel 2 // gets an htlc_maximum_msat of 80_000 and channel 4 an htlc_minimum_msat of 90_000. We @@ -4478,7 +4478,7 @@ mod tests { let logger = Arc::new(test_utils::TestLogger::new()); let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()); let scorer = test_utils::TestScorer::with_fixed_penalty(0); - let payee = Payee::new(nodes[0]).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(nodes[0]).with_features(InvoiceFeatures::known()); { let route = get_route(&our_id, &payee, &network_graph, Some(&[ @@ -4515,7 +4515,7 @@ mod tests { fn prefers_shorter_route_with_higher_fees() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[6]).with_route_hints(last_hops(&nodes)); + let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes)); // Without penalizing each hop 100 msats, a longer path with lower fees is chosen. let scorer = test_utils::TestScorer::with_fixed_penalty(0); @@ -4571,7 +4571,7 @@ mod tests { fn avoids_routing_through_bad_channels_and_nodes() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payee = Payee::new(nodes[6]).with_route_hints(last_hops(&nodes)); + let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes)); // A path to nodes[6] exists when no penalties are applied to any channel. let scorer = test_utils::TestScorer::with_fixed_penalty(0); @@ -4714,7 +4714,7 @@ mod tests { let src = &PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); seed = seed.overflowing_mul(0xdeadbeef).0; let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - let payee = Payee::new(dst); + let payee = Payee::from_node_id(dst); let amt = seed as u64 % 200_000_000; if get_route(src, &payee, &graph, None, amt, 42, &test_utils::TestLogger::new(), &scorer).is_ok() { continue 'load_endpoints; @@ -4745,7 +4745,7 @@ mod tests { let src = &PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); seed = seed.overflowing_mul(0xdeadbeef).0; let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - let payee = Payee::new(dst).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(dst).with_features(InvoiceFeatures::known()); let amt = seed as u64 % 200_000_000; if get_route(src, &payee, &graph, None, amt, 42, &test_utils::TestLogger::new(), &scorer).is_ok() { continue 'load_endpoints; @@ -4811,7 +4811,7 @@ mod benches { let src = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); seed *= 0xdeadbeef; let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - let payee = Payee::new(dst); + let payee = Payee::from_node_id(dst); let amt = seed as u64 % 1_000_000; if get_route(&src, &payee, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok() { path_endpoints.push((src, dst, amt)); @@ -4824,7 +4824,7 @@ mod benches { let mut idx = 0; bench.iter(|| { let (src, dst, amt) = path_endpoints[idx % path_endpoints.len()]; - let payee = Payee::new(dst); + let payee = Payee::from_node_id(dst); assert!(get_route(&src, &payee, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok()); idx += 1; }); @@ -4846,7 +4846,7 @@ mod benches { let src = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); seed *= 0xdeadbeef; let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - let payee = Payee::new(dst).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(dst).with_features(InvoiceFeatures::known()); let amt = seed as u64 % 1_000_000; if get_route(&src, &payee, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok() { path_endpoints.push((src, dst, amt)); @@ -4859,7 +4859,7 @@ mod benches { let mut idx = 0; bench.iter(|| { let (src, dst, amt) = path_endpoints[idx % path_endpoints.len()]; - let payee = Payee::new(dst).with_features(InvoiceFeatures::known()); + let payee = Payee::from_node_id(dst).with_features(InvoiceFeatures::known()); assert!(get_route(&src, &payee, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok()); idx += 1; }); From 51d146c56641b42ae1b1c3d89a2d3ce5033f5578 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 31 Oct 2021 18:21:46 +0000 Subject: [PATCH 3/6] Make payment_path_failed path type bindings-mappable The bindings don't currently support passing `Vec`s of objects which it mappes as "opaque types". This is because it will require clones to convert its own list of references to Rust's list of objects. In the near future we should resolve this limitation, allowing us to revert this (and make `find_route`'s method signature similarly cleaner), but for now we must avoid `Vec`. --- lightning-invoice/src/payment.rs | 7 ++++--- lightning/src/routing/mod.rs | 4 ++-- lightning/src/routing/router.rs | 4 ++-- lightning/src/routing/scorer.rs | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index b29fe18f5..075559bfd 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -71,7 +71,7 @@ //! # fn channel_penalty_msat( //! # &self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId //! # ) -> u64 { 0 } -//! # fn payment_path_failed(&mut self, _path: &Vec, _short_channel_id: u64) {} +//! # fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} //! # } //! # //! # struct FakeLogger {}; @@ -415,7 +415,8 @@ where all_paths_failed, payment_id, payment_hash, rejected_by_dest, path, short_channel_id, retry, .. } => { if let Some(short_channel_id) = short_channel_id { - self.scorer.lock().payment_path_failed(path, *short_channel_id); + let t = path.iter().collect::>(); + self.scorer.lock().payment_path_failed(&t, *short_channel_id); } if *rejected_by_dest { @@ -1099,7 +1100,7 @@ mod tests { &self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId ) -> u64 { 0 } - fn payment_path_failed(&mut self, _path: &Vec, short_channel_id: u64) { + fn payment_path_failed(&mut self, _path: &[&RouteHop], short_channel_id: u64) { if let Some(expected_short_channel_id) = self.expectations.pop_front() { assert_eq!(short_channel_id, expected_short_channel_id); } diff --git a/lightning/src/routing/mod.rs b/lightning/src/routing/mod.rs index d6c016468..a2454f6a3 100644 --- a/lightning/src/routing/mod.rs +++ b/lightning/src/routing/mod.rs @@ -30,7 +30,7 @@ pub trait Score { fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId) -> u64; /// Handles updating channel penalties after failing to route through a channel. - fn payment_path_failed(&mut self, path: &Vec, short_channel_id: u64); + fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64); } /// A scorer that is accessed under a lock. @@ -70,7 +70,7 @@ impl> Score for T { self.deref().channel_penalty_msat(short_channel_id, source, target) } - fn payment_path_failed(&mut self, path: &Vec, short_channel_id: u64) { + fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) { self.deref_mut().payment_path_failed(path, short_channel_id) } } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 724765a18..6caba6778 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -4552,7 +4552,7 @@ mod tests { if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 } } - fn payment_path_failed(&mut self, _path: &Vec, _short_channel_id: u64) {} + fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} } struct BadNodeScorer { @@ -4564,7 +4564,7 @@ mod tests { if *target == self.node_id { u64::max_value() } else { 0 } } - fn payment_path_failed(&mut self, _path: &Vec, _short_channel_id: u64) {} + fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} } #[test] diff --git a/lightning/src/routing/scorer.rs b/lightning/src/routing/scorer.rs index 2aa0b4c14..956b09a54 100644 --- a/lightning/src/routing/scorer.rs +++ b/lightning/src/routing/scorer.rs @@ -220,7 +220,7 @@ impl routing::Score for ScorerUsingTime { self.params.base_penalty_msat + failure_penalty_msat } - fn payment_path_failed(&mut self, _path: &Vec, short_channel_id: u64) { + fn payment_path_failed(&mut self, _path: &[&RouteHop], short_channel_id: u64) { let failure_penalty_msat = self.params.failure_penalty_msat; let half_life = self.params.failure_penalty_half_life; self.channel_failures From 0f0530a67aabcfedc9587c9a33cd8c34802a120d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 31 Oct 2021 00:26:54 +0000 Subject: [PATCH 4/6] Remove now-unused import in routing/mod.rs --- lightning/src/routing/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/lightning/src/routing/mod.rs b/lightning/src/routing/mod.rs index a2454f6a3..3a48ffe93 100644 --- a/lightning/src/routing/mod.rs +++ b/lightning/src/routing/mod.rs @@ -16,7 +16,6 @@ pub mod scorer; use routing::network_graph::NodeId; use routing::router::RouteHop; -use prelude::*; use core::cell::{RefCell, RefMut}; use core::ops::DerefMut; use sync::{Mutex, MutexGuard}; From 80802006ab35a139008a71cff8addf62ea69e72f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 1 Nov 2021 04:23:30 +0000 Subject: [PATCH 5/6] Add `(C-not exported)` tag to a `Payee` modifier with move semantics This matches the other `Payee` move-modifier functions. --- lightning/src/routing/router.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 6caba6778..974ae74e4 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -232,6 +232,8 @@ impl Payee { } /// Includes a payment expiration in seconds relative to the UNIX epoch. + /// + /// (C-not exported) since bindings don't support move semantics pub fn with_expiry_time(self, expiry_time: u64) -> Self { Self { expiry_time: Some(expiry_time), ..self } } From 0c1b70c16193acee843d823c74d2839b6cea32f6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 1 Nov 2021 22:01:57 +0000 Subject: [PATCH 6/6] Add `(C-not exported)` tags as required in tuple types This prepares us for C bindings auto-exporting tuple type fields. --- lightning-invoice/src/lib.rs | 3 ++- lightning/src/util/logger.rs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 3492b74f4..04ba08bd8 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -379,7 +379,8 @@ pub enum TaggedField { /// SHA-256 hash #[derive(Clone, Debug, Hash, Eq, PartialEq)] -pub struct Sha256(pub sha256::Hash); +pub struct Sha256(/// (C-not exported) as the native hash types are not currently mapped + pub sha256::Hash); /// Description string /// diff --git a/lightning/src/util/logger.rs b/lightning/src/util/logger.rs index 01b024065..2717effb2 100644 --- a/lightning/src/util/logger.rs +++ b/lightning/src/util/logger.rs @@ -121,6 +121,7 @@ pub trait Logger { } /// Wrapper for logging byte slices in hex format. +/// (C-not exported) as fmt can't be used in C #[doc(hidden)] pub struct DebugBytes<'a>(pub &'a [u8]); impl<'a> core::fmt::Display for DebugBytes<'a> {