From: Jeffrey Czyz Date: Fri, 29 Oct 2021 13:52:27 +0000 (-0500) Subject: Parameterize Scorer by a Time trait X-Git-Tag: v0.0.103~2^2~1 X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=a8d3b5aabf88487cb72dccd4553ba1894166fbb9;p=rust-lightning Parameterize Scorer by a Time trait Scorer uses time to determine how much to penalize a channel after a failure occurs. Parameterizing it by time cleans up the code such that no-std support is in a single AlwaysPresent struct, which implements the Time trait. Time is implemented for std::time::Instant when std is available. This parameterization also allows for deterministic testing since a clock could be devised to advance forward as needed. --- diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 6a89f5e99..50743774b 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -312,7 +312,6 @@ mod tests { use lightning::ln::features::InitFeatures; use lightning::ln::msgs::{ChannelMessageHandler, Init}; use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler}; - use lightning::routing::scorer::Scorer; use lightning::routing::network_graph::{NetworkGraph, NetGraphMsgHandler}; use lightning::util::config::UserConfig; use lightning::util::events::{Event, MessageSendEventsProvider, MessageSendEvent}; @@ -635,7 +634,7 @@ mod tests { let data_dir = nodes[0].persister.get_data_dir(); let persister = move |node: &ChannelManager, Arc, Arc, Arc, Arc>| FilesystemPersister::persist_manager(data_dir.clone(), node); let router = DefaultRouter::new(Arc::clone(&nodes[0].network_graph), Arc::clone(&nodes[0].logger)); - let scorer = Arc::new(Mutex::new(Scorer::default())); + let scorer = Arc::new(Mutex::new(test_utils::TestScorer::default())); let invoice_payer = Arc::new(InvoicePayer::new(Arc::clone(&nodes[0].node), router, scorer, Arc::clone(&nodes[0].logger), |_: &_| {}, RetryAttempts(2))); let event_handler = Arc::clone(&invoice_payer); let bg_processor = BackgroundProcessor::start(persister, event_handler, nodes[0].chain_monitor.clone(), nodes[0].node.clone(), nodes[0].net_graph_msg_handler.clone(), nodes[0].peer_manager.clone(), nodes[0].logger.clone()); diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index d489cc69e..c7cd048be 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -157,7 +157,6 @@ mod test { use lightning::ln::features::InitFeatures; use lightning::ln::msgs::ChannelMessageHandler; use lightning::routing::router::{Payee, RouteParameters, find_route}; - use lightning::routing::scorer::Scorer; use lightning::util::events::MessageSendEventsProvider; use lightning::util::test_utils; #[test] @@ -183,7 +182,7 @@ mod test { let first_hops = nodes[0].node.list_usable_channels(); let network_graph = node_cfgs[0].network_graph; let logger = test_utils::TestLogger::new(); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = find_route( &nodes[0].node.get_our_node_id(), ¶ms, network_graph, Some(&first_hops.iter().collect::>()), &logger, &scorer, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f7441e0f1..17a287c74 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6087,9 +6087,9 @@ mod tests { use ln::msgs; use ln::msgs::ChannelMessageHandler; use routing::router::{Payee, RouteParameters, find_route}; - use routing::scorer::Scorer; use util::errors::APIError; use util::events::{Event, MessageSendEvent, MessageSendEventsProvider}; + use util::test_utils; #[cfg(feature = "std")] #[test] @@ -6325,7 +6325,7 @@ mod tests { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // To start (1), send a regular payment but don't claim it. let expected_route = [&nodes[1]]; @@ -6430,7 +6430,7 @@ mod tests { }; let network_graph = nodes[0].network_graph; let first_hops = nodes[0].node.list_usable_channels(); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = find_route( &payer_pubkey, ¶ms, network_graph, Some(&first_hops.iter().collect::>()), nodes[0].logger, &scorer @@ -6473,7 +6473,7 @@ mod tests { }; let network_graph = nodes[0].network_graph; let first_hops = nodes[0].node.list_usable_channels(); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = find_route( &payer_pubkey, ¶ms, network_graph, Some(&first_hops.iter().collect::>()), nodes[0].logger, &scorer diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index f605a63ab..515fc79e8 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -17,7 +17,6 @@ use ln::{PaymentPreimage, PaymentHash, PaymentSecret}; use ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, PaymentId}; use routing::network_graph::{NetGraphMsgHandler, NetworkGraph}; use routing::router::{Payee, Route, get_route}; -use routing::scorer::Scorer; use ln::features::{InitFeatures, InvoiceFeatures}; use ln::msgs; use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler}; @@ -1016,7 +1015,7 @@ macro_rules! get_route_and_payment_hash { let payee = $crate::routing::router::Payee::new($recv_node.node.get_our_node_id()) .with_features($crate::ln::features::InvoiceFeatures::known()) .with_route_hints($last_hops); - let scorer = ::routing::scorer::Scorer::with_fixed_penalty(0); + let scorer = ::util::test_utils::TestScorer::with_fixed_penalty(0); let route = ::routing::router::get_route( &$send_node.node.get_our_node_id(), &payee, $send_node.network_graph, Some(&$send_node.node.list_usable_channels().iter().collect::>()), @@ -1353,7 +1352,7 @@ 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()) .with_features(InvoiceFeatures::known()); - let scorer = Scorer::with_fixed_penalty(0); + 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, Some(&origin_node.node.list_usable_channels().iter().collect::>()), @@ -1371,7 +1370,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()) .with_features(InvoiceFeatures::known()); - let scorer = Scorer::with_fixed_penalty(0); + 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(); assert_eq!(route.paths.len(), 1); assert_eq!(route.paths[0].len(), expected_route.len()); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 8c0870afa..1bf981680 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -25,7 +25,6 @@ use ln::{chan_utils, onion_utils}; use ln::chan_utils::HTLC_SUCCESS_TX_WEIGHT; use routing::network_graph::{NetworkUpdate, RoutingFees}; use routing::router::{Payee, Route, RouteHop, RouteHint, RouteHintHop, RouteParameters, find_route, get_route}; -use routing::scorer::Scorer; use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures}; use ln::msgs; use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction}; @@ -7161,7 +7160,7 @@ fn test_check_htlc_underpaying() { // Create some initial channels create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); - let scorer = Scorer::with_fixed_penalty(0); + 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 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]); @@ -7561,7 +7560,7 @@ 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 scorer = Scorer::with_fixed_penalty(0); + 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; @@ -9061,7 +9060,7 @@ fn test_keysend_payments_to_public_node() { final_value_msat: 10000, final_cltv_expiry_delta: 40, }; - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = find_route(&payer_pubkey, ¶ms, network_graph, None, nodes[0].logger, &scorer).unwrap(); let test_preimage = PaymentPreimage([42; 32]); @@ -9095,7 +9094,7 @@ fn test_keysend_payments_to_private_node() { }; let network_graph = nodes[0].network_graph; let first_hops = nodes[0].node.list_usable_channels(); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = find_route( &payer_pubkey, ¶ms, network_graph, Some(&first_hops.iter().collect::>()), nodes[0].logger, &scorer diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index b766cefd7..f72f71459 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -15,7 +15,6 @@ use ln::{PaymentPreimage, PaymentHash}; use ln::channelmanager::PaymentSendFailure; use routing::router::{Payee, get_route}; use routing::network_graph::NetworkUpdate; -use routing::scorer::Scorer; use ln::features::{InitFeatures, InvoiceFeatures}; use ln::msgs; use ln::msgs::{ChannelMessageHandler, ErrorAction}; @@ -82,7 +81,7 @@ fn updates_shutdown_wait() { let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()); let logger = test_utils::TestLogger::new(); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let (our_payment_preimage, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000); diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 93d3dc8f1..633775bfe 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1473,7 +1473,6 @@ mod tests { use routing; use routing::network_graph::{NetworkGraph, NetGraphMsgHandler, NodeId}; use routing::router::{get_route, Payee, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees}; - use routing::scorer::Scorer; use chain::transaction::OutPoint; use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures}; use ln::msgs::{ErrorAction, LightningError, OptionalField, UnsignedChannelAnnouncement, ChannelAnnouncement, RoutingMessageHandler, @@ -1943,7 +1942,7 @@ mod tests { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); let payee = Payee::new(nodes[2]); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Simple route to 2 via 1 @@ -1974,7 +1973,7 @@ mod tests { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); let payee = Payee::new(nodes[2]); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Simple route to 2 via 1 @@ -1993,7 +1992,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 payee = Payee::new(nodes[2]); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Simple route to 2 via 1 @@ -2118,7 +2117,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 payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known()); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // A route to node#2 via two paths. // One path allows transferring 35-40 sats, another one also allows 35-40 sats. @@ -2254,7 +2253,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 payee = Payee::new(nodes[2]); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // // Disable channels 4 and 12 by flags=2 update_channel(&net_graph_msg_handler, &secp_ctx, &privkeys[1], UnsignedChannelUpdate { @@ -2312,7 +2311,7 @@ mod tests { 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 scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Disable nodes 1, 2, and 8 by requiring unknown feature bits let unknown_features = NodeFeatures::known().set_unknown_feature_required(); @@ -2353,7 +2352,7 @@ mod tests { fn our_chans_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let scorer = Scorer::with_fixed_penalty(0); + 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]); @@ -2482,7 +2481,7 @@ mod tests { fn partial_route_hint_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Simple test across 2, 3, 5, and 4 via a last_hop channel // Tests the behaviour when the RouteHint contains a suboptimal hop. @@ -2581,7 +2580,7 @@ mod tests { 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 scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Test handling of an empty RouteHint passed in Invoice. @@ -2663,7 +2662,7 @@ mod tests { 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 scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // Test through channels 2, 3, 5, 8. // Test shows that multiple hop hints are considered. @@ -2769,7 +2768,7 @@ mod tests { 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 scorer = Scorer::with_fixed_penalty(0); + 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. @@ -2818,7 +2817,7 @@ mod tests { fn our_chans_last_hop_connect_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // 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)]; @@ -2939,7 +2938,7 @@ mod tests { }]); let payee = Payee::new(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 = Scorer::with_fixed_penalty(0); + 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 +2992,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 = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known()); // We will use a simple single-path route from @@ -3265,7 +3264,7 @@ mod tests { // one of the latter hops is limited. 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 = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known()); // Path via {node7, node2, node4} is channels {12, 13, 6, 11}. @@ -3388,7 +3387,7 @@ mod tests { fn ignore_fee_first_hop_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 scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[2]); // Path via node0 is channels {1, 3}. Limit them to 100 and 50 sats (total limit 50). @@ -3434,7 +3433,7 @@ mod tests { fn simple_mpp_route_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 scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known()); // We need a route consisting of 3 paths: @@ -3565,7 +3564,7 @@ mod tests { fn long_mpp_route_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 scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known()); // We need a route consisting of 3 paths: @@ -3727,7 +3726,7 @@ mod tests { fn mpp_cheaper_route_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 scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known()); // This test checks that if we have two cheaper paths and one more expensive path, @@ -3894,7 +3893,7 @@ mod tests { // if the fee is not properly accounted for, the behavior is different. 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 = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known()); // We need a route consisting of 2 paths: @@ -4063,7 +4062,7 @@ mod tests { // path finding we realize that we found more capacity than we need. 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 = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known()); // We need a route consisting of 3 paths: @@ -4220,7 +4219,7 @@ mod tests { let network_graph = Arc::new(NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash())); 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 = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[6]); add_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, &privkeys[1], ChannelFeatures::from_le_bytes(id_to_feature_flags(6)), 6); @@ -4349,7 +4348,7 @@ mod tests { // we calculated fees on a higher value, resulting in us ignoring such paths. let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph(); let (our_privkey, our_id, _, nodes) = get_nodes(&secp_ctx); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[2]); // We modify the graph to set the htlc_maximum of channel 2 to below the value we wish to @@ -4411,7 +4410,7 @@ mod tests { // resulting in us thinking there is no possible path, even if other paths exist. 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 = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known()); // We modify the graph to set the htlc_minimum of channel 2 and 4 as needed - channel 2 @@ -4478,7 +4477,7 @@ mod tests { let (_, our_id, _, nodes) = get_nodes(&secp_ctx); let logger = Arc::new(test_utils::TestLogger::new()); let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let payee = Payee::new(nodes[0]).with_features(InvoiceFeatures::known()); { @@ -4519,7 +4518,7 @@ mod tests { let payee = Payee::new(nodes[6]).with_route_hints(last_hops(&nodes)); // Without penalizing each hop 100 msats, a longer path with lower fees is chosen. - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = get_route( &our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer @@ -4532,7 +4531,7 @@ mod tests { // Applying a 100 msat penalty to each hop results in taking channels 7 and 10 to nodes[6] // from nodes[2] rather than channel 6, 11, and 8, even though the longer path is cheaper. - let scorer = Scorer::with_fixed_penalty(100); + let scorer = test_utils::TestScorer::with_fixed_penalty(100); let route = get_route( &our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer @@ -4575,7 +4574,7 @@ mod tests { let payee = Payee::new(nodes[6]).with_route_hints(last_hops(&nodes)); // A path to nodes[6] exists when no penalties are applied to any channel. - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = get_route( &our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer @@ -4704,7 +4703,7 @@ mod tests { }, }; let graph = NetworkGraph::read(&mut d).unwrap(); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // First, get 100 (source, destination) pairs for which route-getting actually succeeds... let mut seed = random_init_seed() as usize; @@ -4735,7 +4734,7 @@ mod tests { }, }; let graph = NetworkGraph::read(&mut d).unwrap(); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // First, get 100 (source, destination) pairs for which route-getting actually succeeds... let mut seed = random_init_seed() as usize; diff --git a/lightning/src/routing/scorer.rs b/lightning/src/routing/scorer.rs index 9089a8932..d9b8da2dc 100644 --- a/lightning/src/routing/scorer.rs +++ b/lightning/src/routing/scorer.rs @@ -53,8 +53,6 @@ use routing::router::RouteHop; use prelude::*; use core::time::Duration; -#[cfg(not(feature = "no-std"))] -use std::time::Instant; /// [`routing::Score`] implementation that provides reasonable default behavior. /// @@ -64,10 +62,23 @@ use std::time::Instant; /// See [module-level documentation] for usage. /// /// [module-level documentation]: crate::routing::scorer -pub struct Scorer { +pub type Scorer = ScorerUsingTime::; + +/// Time used by [`Scorer`]. +#[cfg(not(feature = "no-std"))] +pub type DefaultTime = std::time::Instant; + +/// Time used by [`Scorer`]. +#[cfg(feature = "no-std")] +pub type DefaultTime = Eternity; + +/// [`routing::Score`] implementation parameterized by [`Time`]. +/// +/// See [`Scorer`] for details. +pub struct ScorerUsingTime { params: ScoringParameters, // TODO: Remove entries of closed channels. - channel_failures: HashMap, + channel_failures: HashMap>, } /// Parameters for configuring [`Scorer`]. @@ -86,6 +97,11 @@ pub struct ScoringParameters { /// The time required to elapse before any accumulated [`failure_penalty_msat`] penalties are /// cut in half. /// + /// # Note + /// + /// When time is an [`Eternity`], as is default when enabling feature `no-std`, it will never + /// elapse. Therefore, this penalty will never decay. + /// /// [`failure_penalty_msat`]: Self::failure_penalty_msat pub failure_penalty_half_life: Duration, } @@ -93,16 +109,24 @@ pub struct ScoringParameters { /// Accounting for penalties against a channel for failing to relay any payments. /// /// Penalties decay over time, though accumulate as more failures occur. -struct ChannelFailure { +struct ChannelFailure { /// Accumulated penalty in msats for the channel as of `last_failed`. undecayed_penalty_msat: u64, /// Last time the channel failed. Used to decay `undecayed_penalty_msat`. - #[cfg(not(feature = "no-std"))] - last_failed: Instant, + last_failed: T, } -impl Scorer { +/// A measurement of time. +pub trait Time { + /// Returns an instance corresponding to the current moment. + fn now() -> Self; + + /// Returns the amount of time elapsed since `self` was created. + fn elapsed(&self) -> Duration; +} + +impl ScorerUsingTime { /// Creates a new scorer using the given scoring parameters. pub fn new(params: ScoringParameters) -> Self { Self { @@ -122,42 +146,31 @@ impl Scorer { } } -impl ChannelFailure { +impl ChannelFailure { fn new(failure_penalty_msat: u64) -> Self { Self { undecayed_penalty_msat: failure_penalty_msat, - #[cfg(not(feature = "no-std"))] - last_failed: Instant::now(), + last_failed: T::now(), } } fn add_penalty(&mut self, failure_penalty_msat: u64, half_life: Duration) { self.undecayed_penalty_msat = self.decayed_penalty_msat(half_life) + failure_penalty_msat; - #[cfg(not(feature = "no-std"))] - { - self.last_failed = Instant::now(); - } + self.last_failed = T::now(); } fn decayed_penalty_msat(&self, half_life: Duration) -> u64 { - let decays = self.elapsed().as_secs().checked_div(half_life.as_secs()); + let decays = self.last_failed.elapsed().as_secs().checked_div(half_life.as_secs()); match decays { Some(decays) => self.undecayed_penalty_msat >> decays, None => 0, } } - - fn elapsed(&self) -> Duration { - #[cfg(not(feature = "no-std"))] - return self.last_failed.elapsed(); - #[cfg(feature = "no-std")] - return Duration::from_secs(0); - } } -impl Default for Scorer { +impl Default for ScorerUsingTime { fn default() -> Self { - Scorer::new(ScoringParameters::default()) + Self::new(ScoringParameters::default()) } } @@ -171,7 +184,7 @@ impl Default for ScoringParameters { } } -impl routing::Score for Scorer { +impl routing::Score for ScorerUsingTime { fn channel_penalty_msat( &self, short_channel_id: u64, _source: &NodeId, _target: &NodeId ) -> u64 { @@ -191,3 +204,27 @@ impl routing::Score for Scorer { .or_insert_with(|| ChannelFailure::new(failure_penalty_msat)); } } + +#[cfg(not(feature = "no-std"))] +impl Time for std::time::Instant { + fn now() -> Self { + std::time::Instant::now() + } + + fn elapsed(&self) -> Duration { + std::time::Instant::elapsed(self) + } +} + +/// A state in which time has no meaning. +pub struct Eternity; + +impl Time for Eternity { + fn now() -> Self { + Self + } + + fn elapsed(&self) -> Duration { + Duration::from_secs(0) + } +} diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 28f63e21e..4734a1cb4 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -21,6 +21,7 @@ use ln::features::{ChannelFeatures, InitFeatures}; use ln::msgs; use ln::msgs::OptionalField; use ln::script::ShutdownScript; +use routing::scorer::{Eternity, ScorerUsingTime}; use util::enforcing_trait_impls::{EnforcingSigner, EnforcementState}; use util::events; use util::logger::{Logger, Level, Record}; @@ -690,3 +691,6 @@ impl core::fmt::Debug for OnRegisterOutput { .finish() } } + +/// A scorer useful in testing, when the passage of time isn't a concern. +pub type TestScorer = ScorerUsingTime;