Skip to content

Commit 80daf94

Browse files
jurvisjurvis
committed
Make payment tests more realistic
Made sure that every hop has a unique receipient. When we simulate calling `channel_penalty_msat` in `TestRouter`’s find route, use actual previous node ids instead of just using the payer’s.
1 parent c70bd1f commit 80daf94

File tree

1 file changed

+60
-30
lines changed

1 file changed

+60
-30
lines changed

‎lightning-invoice/src/payment.rs‎

Lines changed: 60 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1560,7 +1560,8 @@ mod tests {
15601560
assert_eq!(inflight_map.get(&(2, false)).unwrap().clone(), 64);
15611561

15621562
// Second path check
1563-
assert_eq!(inflight_map.get(&(1, false)).unwrap().clone(), 64);
1563+
assert_eq!(inflight_map.get(&(3, false)).unwrap().clone(), 74);
1564+
assert_eq!(inflight_map.get(&(4, false)).unwrap().clone(), 64);
15641565

15651566
invoice_payer.handle_event(&Event::PaymentPathSuccessful {
15661567
payment_id, payment_hash, path: route.paths[0].clone()
@@ -1573,7 +1574,8 @@ mod tests {
15731574
assert_eq!(inflight_map.get(&(2, false)), None);
15741575

15751576
// Second path should still be inflight
1576-
assert_eq!(inflight_map.get(&(1, false)).unwrap().clone(), 64)
1577+
assert_eq!(inflight_map.get(&(3, false)).unwrap().clone(), 74);
1578+
assert_eq!(inflight_map.get(&(4, false)).unwrap().clone(), 64)
15771579
}
15781580

15791581
#[test]
@@ -1595,17 +1597,19 @@ mod tests {
15951597
let router = TestRouter {};
15961598
let scorer = RefCell::new(TestScorer::new()
15971599
// 1st invoice, 1st path
1598-
.expect_usage(ChannelUsage { amount_msat: 10, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1599-
.expect_usage(ChannelUsage { amount_msat: 20, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16001600
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1601+
.expect_usage(ChannelUsage { amount_msat: 84, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1602+
.expect_usage(ChannelUsage { amount_msat: 94, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16011603
// 1st invoice, 2nd path
16021604
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1605+
.expect_usage(ChannelUsage { amount_msat: 74, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16031606
// 2nd invoice, 1st path
1604-
.expect_usage(ChannelUsage { amount_msat: 10, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1605-
.expect_usage(ChannelUsage { amount_msat: 20, inflight_htlc_msat: 64, effective_capacity: EffectiveCapacity::Unknown } )
16061607
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1608+
.expect_usage(ChannelUsage { amount_msat: 84, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1609+
.expect_usage(ChannelUsage { amount_msat: 94, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16071610
// 2nd invoice, 2nd path
16081611
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 64, effective_capacity: EffectiveCapacity::Unknown } )
1612+
.expect_usage(ChannelUsage { amount_msat: 74, inflight_htlc_msat: 74, effective_capacity: EffectiveCapacity::Unknown } )
16091613
);
16101614
let logger = TestLogger::new();
16111615
let invoice_payer =
@@ -1640,26 +1644,31 @@ mod tests {
16401644
let payer = TestPayer::new()
16411645
.expect_send(Amount::ForInvoice(final_value_msat))
16421646
.expect_send(Amount::OnRetry(final_value_msat / 2))
1643-
.expect_send(Amount::OnRetry(final_value_msat / 2));
1647+
.expect_send(Amount::OnRetry(final_value_msat / 4));
16441648
let final_value_msat = payment_invoice.amount_milli_satoshis().unwrap();
16451649
let router = TestRouter {};
16461650
let scorer = RefCell::new(TestScorer::new()
16471651
// 1st invoice, 1st path
1648-
.expect_usage(ChannelUsage { amount_msat: 10, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1649-
.expect_usage(ChannelUsage { amount_msat: 20, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16501652
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1653+
.expect_usage(ChannelUsage { amount_msat: 84, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1654+
.expect_usage(ChannelUsage { amount_msat: 94, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16511655
// 1st invoice, 2nd path
16521656
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1653-
// Retry 1
1654-
.expect_usage(ChannelUsage { amount_msat: 10, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1655-
.expect_usage(ChannelUsage { amount_msat: 20, inflight_htlc_msat: 64, effective_capacity: EffectiveCapacity::Unknown } )
1657+
.expect_usage(ChannelUsage { amount_msat: 74, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1658+
// Retry 1, 1st path
16561659
.expect_usage(ChannelUsage { amount_msat: 32, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1660+
.expect_usage(ChannelUsage { amount_msat: 52, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1661+
.expect_usage(ChannelUsage { amount_msat: 62, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1662+
// Retry 1, 2nd path
16571663
.expect_usage(ChannelUsage { amount_msat: 32, inflight_htlc_msat: 64, effective_capacity: EffectiveCapacity::Unknown } )
1658-
// Retry 2
1659-
.expect_usage(ChannelUsage { amount_msat: 10, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1660-
.expect_usage(ChannelUsage { amount_msat: 20, inflight_htlc_msat: 96, effective_capacity: EffectiveCapacity::Unknown } )
1661-
.expect_usage(ChannelUsage { amount_msat: 32, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1662-
.expect_usage(ChannelUsage { amount_msat: 32, inflight_htlc_msat: 96, effective_capacity: EffectiveCapacity::Unknown } )
1664+
.expect_usage(ChannelUsage { amount_msat: 42, inflight_htlc_msat: 64 + 10, effective_capacity: EffectiveCapacity::Unknown } )
1665+
// Retry 2, 1st path
1666+
.expect_usage(ChannelUsage { amount_msat: 16, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1667+
.expect_usage(ChannelUsage { amount_msat: 36, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1668+
.expect_usage(ChannelUsage { amount_msat: 46, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1669+
// Retry 2, 2nd path
1670+
.expect_usage(ChannelUsage { amount_msat: 16, inflight_htlc_msat: 64 + 32, effective_capacity: EffectiveCapacity::Unknown } )
1671+
.expect_usage(ChannelUsage { amount_msat: 26, inflight_htlc_msat: 74 + 32 + 10, effective_capacity: EffectiveCapacity::Unknown } )
16631672
);
16641673
let logger = TestLogger::new();
16651674
let invoice_payer =
@@ -1688,7 +1697,7 @@ mod tests {
16881697
path: TestRouter::path_for_value(final_value_msat / 2),
16891698
short_channel_id: None,
16901699
retry: Some(RouteParameters {
1691-
final_value_msat: final_value_msat / 2,
1700+
final_value_msat: final_value_msat / 4,
16921701
..TestRouter::retry_for_invoice(&payment_invoice)
16931702
}),
16941703
});
@@ -1723,7 +1732,7 @@ mod tests {
17231732

17241733
// Only the second path, which failed with `MonitorUpdateFailed` should be added to our
17251734
// inflight map because retries are disabled.
1726-
assert_eq!(inflight_map.len(), 1);
1735+
assert_eq!(inflight_map.len(), 2);
17271736
}
17281737

17291738
#[test]
@@ -1753,8 +1762,8 @@ mod tests {
17531762
invoice_payer.pay_invoice(&invoice_to_pay).unwrap();
17541763
let inflight_map = invoice_payer.create_inflight_map();
17551764

1756-
// All paths successful, hence we check of the existence of all 4 hops.
1757-
assert_eq!(inflight_map.len(), 4);
1765+
// All paths successful, hence we check of the existence of all 5 hops.
1766+
assert_eq!(inflight_map.len(), 5);
17581767
}
17591768

17601769
struct TestRouter;
@@ -1789,12 +1798,24 @@ mod tests {
17891798
cltv_expiry_delta: 0
17901799
},
17911800
],
1792-
vec![RouteHop {
1793-
pubkey: PublicKey::from_slice(&hex::decode("0324653eac434488002cc06bbfb7f10fe18991e35f9fe4302dbea6d2353dc0ab1c").unwrap()[..]).unwrap(),
1794-
channel_features: ChannelFeatures::empty(),
1795-
node_features: NodeFeatures::empty(),
1796-
short_channel_id: 1, fee_msat: final_value_msat / 2, cltv_expiry_delta: 144
1797-
}],
1801+
vec![
1802+
RouteHop {
1803+
pubkey: PublicKey::from_slice(&hex::decode("029e03a901b85534ff1e92c43c74431f7ce72046060fcf7a95c37e148f78c77255").unwrap()[..]).unwrap(),
1804+
channel_features: ChannelFeatures::empty(),
1805+
node_features: NodeFeatures::empty(),
1806+
short_channel_id: 3,
1807+
fee_msat: 10,
1808+
cltv_expiry_delta: 144
1809+
},
1810+
RouteHop {
1811+
pubkey: PublicKey::from_slice(&hex::decode("027f31ebc5462c1fdce1b737ecff52d37d75dea43ce11c74d25aa297165faa2007").unwrap()[..]).unwrap(),
1812+
channel_features: ChannelFeatures::empty(),
1813+
node_features: NodeFeatures::empty(),
1814+
short_channel_id: 4,
1815+
fee_msat: final_value_msat / 2,
1816+
cltv_expiry_delta: 144
1817+
}
1818+
],
17981819
],
17991820
payment_params: None,
18001821
}
@@ -1828,13 +1849,22 @@ mod tests {
18281849
// Simulate calling the Scorer just as you would in find_route
18291850
let route = Self::route_for_value(route_params.final_value_msat);
18301851
for path in route.paths {
1831-
for hop in path {
1852+
let mut aggregate_msat = 0u64;
1853+
for (idx, hop) in path.iter().rev().enumerate() {
1854+
aggregate_msat += hop.fee_msat;
18321855
let usage = ChannelUsage {
1833-
amount_msat: hop.fee_msat,
1856+
amount_msat: aggregate_msat,
18341857
inflight_htlc_msat: 0,
18351858
effective_capacity: EffectiveCapacity::Unknown,
18361859
};
1837-
scorer.channel_penalty_msat(hop.short_channel_id, &NodeId::from_pubkey(payer), &NodeId::from_pubkey(&hop.pubkey), usage);
1860+
1861+
// Since the path is reversed, the last element in our iteration is the first
1862+
// hop.
1863+
if idx == path.len() - 1 {
1864+
scorer.channel_penalty_msat(hop.short_channel_id, &NodeId::from_pubkey(payer), &NodeId::from_pubkey(&hop.pubkey), usage);
1865+
} else {
1866+
scorer.channel_penalty_msat(hop.short_channel_id, &NodeId::from_pubkey(&path[idx + 1].pubkey), &NodeId::from_pubkey(&hop.pubkey), usage);
1867+
}
18381868
}
18391869
}
18401870

0 commit comments

Comments
 (0)