Skip to content

Commit 2edb3f1

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #2020 from valentinewallace/2023-02-test-inflight-scoring
Fix and test in-flight HTLC scoring in between retries
2 parents 9f10203 + 2b2965f commit 2edb3f1

14 files changed

+200
-128
lines changed

‎fuzz/src/chanmon_consistency.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,6 @@ impl Router for FuzzRouter {
9797
action: msgs::ErrorAction::IgnoreError
9898
})
9999
}
100-
fn notify_payment_path_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
101-
fn notify_payment_path_successful(&self, _path: &[&RouteHop]) {}
102-
fn notify_payment_probe_successful(&self, _path: &[&RouteHop]) {}
103-
fn notify_payment_probe_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
104100
}
105101

106102
pub struct TestBroadcaster {}

‎fuzz/src/full_stack.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,6 @@ impl Router for FuzzRouter {
140140
action: msgs::ErrorAction::IgnoreError
141141
})
142142
}
143-
fn notify_payment_path_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
144-
fn notify_payment_path_successful(&self, _path: &[&RouteHop]) {}
145-
fn notify_payment_probe_successful(&self, _path: &[&RouteHop]) {}
146-
fn notify_payment_probe_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
147143
}
148144

149145
struct TestBroadcaster {

‎lightning-invoice/src/utils.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ mod test {
691691
let first_hops = nodes[0].node.list_usable_channels();
692692
let network_graph = &node_cfgs[0].network_graph;
693693
let logger = test_utils::TestLogger::new();
694-
let scorer = test_utils::TestScorer::with_penalty(0);
694+
let scorer = test_utils::TestScorer::new();
695695
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
696696
let route = find_route(
697697
&nodes[0].node.get_our_node_id(), &route_params, &network_graph,
@@ -1055,7 +1055,7 @@ mod test {
10551055
let first_hops = nodes[0].node.list_usable_channels();
10561056
let network_graph = &node_cfgs[0].network_graph;
10571057
let logger = test_utils::TestLogger::new();
1058-
let scorer = test_utils::TestScorer::with_penalty(0);
1058+
let scorer = test_utils::TestScorer::new();
10591059
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
10601060
let route = find_route(
10611061
&nodes[0].node.get_our_node_id(), &params, &network_graph,

‎lightning/src/ln/channelmanager.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2537,7 +2537,7 @@ where
25372537
let best_block_height = self.best_block.read().unwrap().height();
25382538
self.pending_outbound_payments
25392539
.send_payment(payment_hash, payment_secret, payment_id, retry_strategy, route_params,
2540-
&self.router, self.list_usable_channels(), self.compute_inflight_htlcs(),
2540+
&self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
25412541
&self.entropy_source, &self.node_signer, best_block_height, &self.logger,
25422542
|path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv|
25432543
self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv))
@@ -2637,7 +2637,7 @@ where
26372637
let best_block_height = self.best_block.read().unwrap().height();
26382638
self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, payment_id,
26392639
retry_strategy, route_params, &self.router, self.list_usable_channels(),
2640-
self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height,
2640+
|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height,
26412641
&self.logger,
26422642
|path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv|
26432643
self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv))
@@ -7997,7 +7997,7 @@ mod tests {
79977997
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
79987998
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
79997999
create_announced_chan_between_nodes(&nodes, 0, 1);
8000-
let scorer = test_utils::TestScorer::with_penalty(0);
8000+
let scorer = test_utils::TestScorer::new();
80018001
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
80028002

80038003
// To start (1), send a regular payment but don't claim it.
@@ -8103,7 +8103,7 @@ mod tests {
81038103
};
81048104
let network_graph = nodes[0].network_graph.clone();
81058105
let first_hops = nodes[0].node.list_usable_channels();
8106-
let scorer = test_utils::TestScorer::with_penalty(0);
8106+
let scorer = test_utils::TestScorer::new();
81078107
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
81088108
let route = find_route(
81098109
&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
@@ -8146,7 +8146,7 @@ mod tests {
81468146
};
81478147
let network_graph = nodes[0].network_graph.clone();
81488148
let first_hops = nodes[0].node.list_usable_channels();
8149-
let scorer = test_utils::TestScorer::with_penalty(0);
8149+
let scorer = test_utils::TestScorer::new();
81508150
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
81518151
let route = find_route(
81528152
&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
@@ -8516,7 +8516,8 @@ pub mod bench {
85168516
let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))};
85178517
let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
85188518
let logger_a = test_utils::TestLogger::with_id("node a".to_owned());
8519-
let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(genesis_hash, &logger_a)));
8519+
let scorer = Mutex::new(test_utils::TestScorer::new());
8520+
let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(genesis_hash, &logger_a)), &scorer);
85208521

85218522
let mut config: UserConfig = Default::default();
85228523
config.channel_handshake_config.minimum_depth = 1;
@@ -8607,7 +8608,7 @@ pub mod bench {
86078608
let usable_channels = $node_a.list_usable_channels();
86088609
let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id(), TEST_FINAL_CLTV)
86098610
.with_features($node_b.invoice_features());
8610-
let scorer = test_utils::TestScorer::with_penalty(0);
8611+
let scorer = test_utils::TestScorer::new();
86118612
let seed = [3u8; 32];
86128613
let keys_manager = KeysManager::new(&seed, 42, 42);
86138614
let random_seed_bytes = keys_manager.get_secure_random_bytes();

‎lightning/src/ln/functional_test_utils.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ pub struct TestChanMonCfg {
305305
pub persister: test_utils::TestPersister,
306306
pub logger: test_utils::TestLogger,
307307
pub keys_manager: test_utils::TestKeysInterface,
308+
pub scorer: Mutex<test_utils::TestScorer>,
308309
}
309310

310311
pub struct NodeCfg<'a> {
@@ -427,6 +428,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
427428
channel_monitors.insert(monitor.get_funding_txo().0, monitor);
428429
}
429430

431+
let scorer = Mutex::new(test_utils::TestScorer::new());
430432
let mut w = test_utils::TestVecWriter(Vec::new());
431433
self.node.write(&mut w).unwrap();
432434
<(BlockHash, ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>)>::read(&mut io::Cursor::new(w.0), ChannelManagerReadArgs {
@@ -435,7 +437,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
435437
node_signer: self.keys_manager,
436438
signer_provider: self.keys_manager,
437439
fee_estimator: &test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) },
438-
router: &test_utils::TestRouter::new(Arc::new(network_graph)),
440+
router: &test_utils::TestRouter::new(Arc::new(network_graph), &scorer),
439441
chain_monitor: self.chain_monitor,
440442
tx_broadcaster: &broadcaster,
441443
logger: &self.logger,
@@ -1568,7 +1570,7 @@ macro_rules! get_payment_preimage_hash {
15681570
macro_rules! get_route {
15691571
($send_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{
15701572
use $crate::chain::keysinterface::EntropySource;
1571-
let scorer = $crate::util::test_utils::TestScorer::with_penalty(0);
1573+
let scorer = $crate::util::test_utils::TestScorer::new();
15721574
let keys_manager = $crate::util::test_utils::TestKeysInterface::new(&[0u8; 32], bitcoin::network::constants::Network::Testnet);
15731575
let random_seed_bytes = keys_manager.get_secure_random_bytes();
15741576
$crate::routing::router::get_route(
@@ -2120,7 +2122,7 @@ pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
21202122
let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV)
21212123
.with_features(expected_route.last().unwrap().node.invoice_features());
21222124
let network_graph = origin_node.network_graph.read_only();
2123-
let scorer = test_utils::TestScorer::with_penalty(0);
2125+
let scorer = test_utils::TestScorer::new();
21242126
let seed = [0u8; 32];
21252127
let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
21262128
let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2285,8 +2287,9 @@ pub fn create_chanmon_cfgs(node_count: usize) -> Vec<TestChanMonCfg> {
22852287
let persister = test_utils::TestPersister::new();
22862288
let seed = [i as u8; 32];
22872289
let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
2290+
let scorer = Mutex::new(test_utils::TestScorer::new());
22882291

2289-
chan_mon_cfgs.push(TestChanMonCfg { tx_broadcaster, fee_estimator, chain_source, logger, persister, keys_manager });
2292+
chan_mon_cfgs.push(TestChanMonCfg { tx_broadcaster, fee_estimator, chain_source, logger, persister, keys_manager, scorer });
22902293
}
22912294

22922295
chan_mon_cfgs
@@ -2304,7 +2307,7 @@ pub fn create_node_cfgs<'a>(node_count: usize, chanmon_cfgs: &'a Vec<TestChanMon
23042307
logger: &chanmon_cfgs[i].logger,
23052308
tx_broadcaster: &chanmon_cfgs[i].tx_broadcaster,
23062309
fee_estimator: &chanmon_cfgs[i].fee_estimator,
2307-
router: test_utils::TestRouter::new(network_graph.clone()),
2310+
router: test_utils::TestRouter::new(network_graph.clone(), &chanmon_cfgs[i].scorer),
23082311
chain_monitor,
23092312
keys_manager: &chanmon_cfgs[i].keys_manager,
23102313
node_seed: seed,

‎lightning/src/ln/functional_tests.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5258,7 +5258,8 @@ fn test_key_derivation_params() {
52585258
let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
52595259
let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[0].chain_source), &chanmon_cfgs[0].tx_broadcaster, &chanmon_cfgs[0].logger, &chanmon_cfgs[0].fee_estimator, &chanmon_cfgs[0].persister, &keys_manager);
52605260
let network_graph = Arc::new(NetworkGraph::new(chanmon_cfgs[0].chain_source.genesis_hash, &chanmon_cfgs[0].logger));
5261-
let router = test_utils::TestRouter::new(network_graph.clone());
5261+
let scorer = Mutex::new(test_utils::TestScorer::new());
5262+
let router = test_utils::TestRouter::new(network_graph.clone(), &scorer);
52625263
let node = NodeCfg { chain_source: &chanmon_cfgs[0].chain_source, logger: &chanmon_cfgs[0].logger, tx_broadcaster: &chanmon_cfgs[0].tx_broadcaster, fee_estimator: &chanmon_cfgs[0].fee_estimator, router, chain_monitor, keys_manager: &keys_manager, network_graph, node_seed: seed, override_init_features: alloc::rc::Rc::new(core::cell::RefCell::new(None)) };
52635264
let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
52645265
node_cfgs.remove(0);
@@ -6926,7 +6927,7 @@ fn test_check_htlc_underpaying() {
69266927
// Create some initial channels
69276928
create_announced_chan_between_nodes(&nodes, 0, 1);
69286929

6929-
let scorer = test_utils::TestScorer::with_penalty(0);
6930+
let scorer = test_utils::TestScorer::new();
69306931
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
69316932
let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[1].node.invoice_features());
69326933
let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None, 10_000, TEST_FINAL_CLTV, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
@@ -7176,7 +7177,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
71767177
let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 59000000);
71777178
// Lock HTLC in both directions (using a slightly lower CLTV delay to provide timely RBF bumps)
71787179
let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 50).with_features(nodes[1].node.invoice_features());
7179-
let scorer = test_utils::TestScorer::with_penalty(0);
7180+
let scorer = test_utils::TestScorer::new();
71807181
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
71817182
let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None,
71827183
3_000_000, 50, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
@@ -9187,7 +9188,7 @@ fn test_keysend_payments_to_public_node() {
91879188
final_value_msat: 10000,
91889189
final_cltv_expiry_delta: 40,
91899190
};
9190-
let scorer = test_utils::TestScorer::with_penalty(0);
9191+
let scorer = test_utils::TestScorer::new();
91919192
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
91929193
let route = find_route(&payer_pubkey, &route_params, &network_graph, None, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
91939194

@@ -9220,7 +9221,7 @@ fn test_keysend_payments_to_private_node() {
92209221
};
92219222
let network_graph = nodes[0].network_graph.clone();
92229223
let first_hops = nodes[0].node.list_usable_channels();
9223-
let scorer = test_utils::TestScorer::with_penalty(0);
9224+
let scorer = test_utils::TestScorer::new();
92249225
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
92259226
let route = find_route(
92269227
&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),

‎lightning/src/ln/onion_route_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -928,7 +928,7 @@ macro_rules! get_phantom_route {
928928
htlc_maximum_msat: None,
929929
}
930930
])]);
931-
let scorer = test_utils::TestScorer::with_penalty(0);
931+
let scorer = test_utils::TestScorer::new();
932932
let network_graph = $nodes[0].network_graph.read_only();
933933
(get_route(
934934
&$nodes[0].node.get_our_node_id(), &payment_params, &network_graph,

0 commit comments

Comments
 (0)