Skip to content

Commit e54a49b

Browse files
committed
Parameterize Scorer by a Clock trait
Scorer uses time to determine how much to penalize a channel after a failure occurs. Parameterizing it by a clock cleans up the code such that no-std support is in a single AlwaysPresent struct, which implements the Clock trait. Clock is implemented for std::time::Instant when std is available. This parameterization also allows for deterministic testing since a Clock for testing could be devised to advance forward as needed.
1 parent e8c7243 commit e54a49b

File tree

11 files changed

+99
-82
lines changed

11 files changed

+99
-82
lines changed

fuzz/src/full_stack.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
382382
let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret());
383383
let network_graph = NetworkGraph::new(genesis_block(network).block_hash());
384384
let net_graph_msg_handler = Arc::new(NetGraphMsgHandler::new(network_graph, None, Arc::clone(&logger)));
385-
let scorer = Scorer::with_fixed_penalty(0);
385+
let scorer = Scorer::<std::time::Instant>::with_fixed_penalty(0);
386386

387387
let peers = RefCell::new([false; 256]);
388388
let mut loss_detector = MoneyLossDetector::new(&peers, channelmanager.clone(), monitor.clone(), PeerManager::new(MessageHandler {

fuzz/src/router.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
248248
}]));
249249
}
250250
}
251-
let scorer = Scorer::with_fixed_penalty(0);
251+
let scorer = Scorer::<std::time::Instant>::with_fixed_penalty(0);
252252
for target in node_pks.iter() {
253253
let params = RouteParameters {
254254
payee: Payee::new(*target).with_route_hints(last_hops.clone()),

lightning-background-processor/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,6 @@ mod tests {
309309
use lightning::ln::features::InitFeatures;
310310
use lightning::ln::msgs::{ChannelMessageHandler, Init};
311311
use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler};
312-
use lightning::routing::scorer::Scorer;
313312
use lightning::routing::network_graph::{NetworkGraph, NetGraphMsgHandler};
314313
use lightning::util::config::UserConfig;
315314
use lightning::util::events::{Event, MessageSendEventsProvider, MessageSendEvent};
@@ -632,7 +631,7 @@ mod tests {
632631
let persister = move |node: &ChannelManager<InMemorySigner, Arc<ChainMonitor>, Arc<test_utils::TestBroadcaster>, Arc<KeysManager>, Arc<test_utils::TestFeeEstimator>, Arc<test_utils::TestLogger>>| FilesystemPersister::persist_manager(data_dir.clone(), node);
633632
let network_graph = Arc::new(NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()));
634633
let router = DefaultRouter::new(network_graph, Arc::clone(&nodes[0].logger));
635-
let scorer = Arc::new(Mutex::new(Scorer::default()));
634+
let scorer = Arc::new(Mutex::new(test_utils::TestScorer::default()));
636635
let invoice_payer = Arc::new(InvoicePayer::new(Arc::clone(&nodes[0].node), router, scorer, Arc::clone(&nodes[0].logger), |_: &_| {}, RetryAttempts(2)));
637636
let event_handler = Arc::clone(&invoice_payer);
638637
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());

lightning-invoice/src/utils.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,6 @@ mod test {
157157
use lightning::ln::features::InitFeatures;
158158
use lightning::ln::msgs::ChannelMessageHandler;
159159
use lightning::routing::router::{Payee, RouteParameters, find_route};
160-
use lightning::routing::scorer::Scorer;
161160
use lightning::util::events::MessageSendEventsProvider;
162161
use lightning::util::test_utils;
163162
#[test]
@@ -183,7 +182,7 @@ mod test {
183182
let first_hops = nodes[0].node.list_usable_channels();
184183
let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
185184
let logger = test_utils::TestLogger::new();
186-
let scorer = Scorer::with_fixed_penalty(0);
185+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
187186
let route = find_route(
188187
&nodes[0].node.get_our_node_id(), &params, network_graph,
189188
Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer,

lightning/src/ln/channelmanager.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6061,9 +6061,9 @@ mod tests {
60616061
use ln::msgs;
60626062
use ln::msgs::ChannelMessageHandler;
60636063
use routing::router::{Payee, RouteParameters, find_route};
6064-
use routing::scorer::Scorer;
60656064
use util::errors::APIError;
60666065
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
6066+
use util::test_utils;
60676067

60686068
#[cfg(feature = "std")]
60696069
#[test]
@@ -6299,7 +6299,7 @@ mod tests {
62996299
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
63006300
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
63016301
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
6302-
let scorer = Scorer::with_fixed_penalty(0);
6302+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
63036303

63046304
// To start (1), send a regular payment but don't claim it.
63056305
let expected_route = [&nodes[1]];
@@ -6404,7 +6404,7 @@ mod tests {
64046404
};
64056405
let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
64066406
let first_hops = nodes[0].node.list_usable_channels();
6407-
let scorer = Scorer::with_fixed_penalty(0);
6407+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
64086408
let route = find_route(
64096409
&payer_pubkey, &params, network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
64106410
nodes[0].logger, &scorer
@@ -6447,7 +6447,7 @@ mod tests {
64476447
};
64486448
let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
64496449
let first_hops = nodes[0].node.list_usable_channels();
6450-
let scorer = Scorer::with_fixed_penalty(0);
6450+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
64516451
let route = find_route(
64526452
&payer_pubkey, &params, network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
64536453
nodes[0].logger, &scorer
@@ -6622,7 +6622,7 @@ pub mod bench {
66226622
let usable_channels = $node_a.list_usable_channels();
66236623
let payee = Payee::new($node_b.get_our_node_id())
66246624
.with_features(InvoiceFeatures::known());
6625-
let scorer = Scorer::with_fixed_penalty(0);
6625+
let scorer = Scorer::<std::time::Instant>::with_fixed_penalty(0);
66266626
let route = get_route(&$node_a.get_our_node_id(), &payee, &dummy_graph,
66276627
Some(&usable_channels.iter().map(|r| r).collect::<Vec<_>>()), 10_000, TEST_FINAL_CLTV, &logger_a, &scorer).unwrap();
66286628

lightning/src/ln/functional_test_utils.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
1717
use ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, PaymentId};
1818
use routing::network_graph::{NetGraphMsgHandler, NetworkGraph};
1919
use routing::router::{Payee, Route, get_route};
20-
use routing::scorer::Scorer;
2120
use ln::features::{InitFeatures, InvoiceFeatures};
2221
use ln::msgs;
2322
use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler};
@@ -1015,7 +1014,7 @@ macro_rules! get_route_and_payment_hash {
10151014
.with_features($crate::ln::features::InvoiceFeatures::known())
10161015
.with_route_hints($last_hops);
10171016
let net_graph_msg_handler = &$send_node.net_graph_msg_handler;
1018-
let scorer = ::routing::scorer::Scorer::with_fixed_penalty(0);
1017+
let scorer = ::util::test_utils::TestScorer::with_fixed_penalty(0);
10191018
let route = ::routing::router::get_route(
10201019
&$send_node.node.get_our_node_id(), &payee, &net_graph_msg_handler.network_graph,
10211020
Some(&$send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
@@ -1353,7 +1352,7 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
13531352
let payee = Payee::new(expected_route.last().unwrap().node.get_our_node_id())
13541353
.with_features(InvoiceFeatures::known());
13551354
let net_graph_msg_handler = &origin_node.net_graph_msg_handler;
1356-
let scorer = Scorer::with_fixed_penalty(0);
1355+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
13571356
let route = get_route(
13581357
&origin_node.node.get_our_node_id(), &payee, &net_graph_msg_handler.network_graph,
13591358
Some(&origin_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
@@ -1372,7 +1371,7 @@ pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
13721371
let payee = Payee::new(expected_route.last().unwrap().node.get_our_node_id())
13731372
.with_features(InvoiceFeatures::known());
13741373
let net_graph_msg_handler = &origin_node.net_graph_msg_handler;
1375-
let scorer = Scorer::with_fixed_penalty(0);
1374+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
13761375
let route = get_route(&origin_node.node.get_our_node_id(), &payee, &net_graph_msg_handler.network_graph, None, recv_value, TEST_FINAL_CLTV, origin_node.logger, &scorer).unwrap();
13771376
assert_eq!(route.paths.len(), 1);
13781377
assert_eq!(route.paths[0].len(), expected_route.len());

lightning/src/ln/functional_tests.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ use ln::{chan_utils, onion_utils};
2525
use ln::chan_utils::HTLC_SUCCESS_TX_WEIGHT;
2626
use routing::network_graph::{NetworkUpdate, RoutingFees};
2727
use routing::router::{Payee, Route, RouteHop, RouteHint, RouteHintHop, RouteParameters, find_route, get_route};
28-
use routing::scorer::Scorer;
2928
use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
3029
use ln::msgs;
3130
use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
@@ -7161,7 +7160,7 @@ fn test_check_htlc_underpaying() {
71617160
// Create some initial channels
71627161
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
71637162

7164-
let scorer = Scorer::with_fixed_penalty(0);
7163+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
71657164
let payee = Payee::new(nodes[1].node.get_our_node_id()).with_features(InvoiceFeatures::known());
71667165
let route = get_route(&nodes[0].node.get_our_node_id(), &payee, &nodes[0].net_graph_msg_handler.network_graph, None, 10_000, TEST_FINAL_CLTV, nodes[0].logger, &scorer).unwrap();
71677166
let (_, our_payment_hash, _) = get_payment_preimage_hash!(nodes[0]);
@@ -7561,7 +7560,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
75617560
let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 59000000, InitFeatures::known(), InitFeatures::known());
75627561
// Lock HTLC in both directions (using a slightly lower CLTV delay to provide timely RBF bumps)
75637562
let payee = Payee::new(nodes[1].node.get_our_node_id()).with_features(InvoiceFeatures::known());
7564-
let scorer = Scorer::with_fixed_penalty(0);
7563+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
75657564
let route = get_route(&nodes[0].node.get_our_node_id(), &payee, &nodes[0].net_graph_msg_handler.network_graph,
75667565
None, 3_000_000, 50, nodes[0].logger, &scorer).unwrap();
75677566
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() {
90619060
final_value_msat: 10000,
90629061
final_cltv_expiry_delta: 40,
90639062
};
9064-
let scorer = Scorer::with_fixed_penalty(0);
9063+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
90659064
let route = find_route(&payer_pubkey, &params, &network_graph, None, nodes[0].logger, &scorer).unwrap();
90669065

90679066
let test_preimage = PaymentPreimage([42; 32]);
@@ -9095,7 +9094,7 @@ fn test_keysend_payments_to_private_node() {
90959094
};
90969095
let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
90979096
let first_hops = nodes[0].node.list_usable_channels();
9098-
let scorer = Scorer::with_fixed_penalty(0);
9097+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
90999098
let route = find_route(
91009099
&payer_pubkey, &params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
91019100
nodes[0].logger, &scorer

lightning/src/ln/shutdown_tests.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use ln::{PaymentPreimage, PaymentHash};
1515
use ln::channelmanager::PaymentSendFailure;
1616
use routing::router::{Payee, get_route};
1717
use routing::network_graph::NetworkUpdate;
18-
use routing::scorer::Scorer;
1918
use ln::features::{InitFeatures, InvoiceFeatures};
2019
use ln::msgs;
2120
use ln::msgs::{ChannelMessageHandler, ErrorAction};
@@ -82,7 +81,7 @@ fn updates_shutdown_wait() {
8281
let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
8382
let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
8483
let logger = test_utils::TestLogger::new();
85-
let scorer = Scorer::with_fixed_penalty(0);
84+
let scorer = test_utils::TestScorer::with_fixed_penalty(0);
8685

8786
let (our_payment_preimage, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000);
8887

0 commit comments

Comments
 (0)