Skip to content

Commit 08130b7

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 cb9f6e7 commit 08130b7

File tree

1 file changed

+60
-30
lines changed

1 file changed

+60
-30
lines changed

‎lightning-invoice/src/payment.rs

+60-30
Original file line numberDiff line numberDiff line change
@@ -1561,7 +1561,8 @@ mod tests {
15611561
assert_eq!(inflight_map.get(&(2, false)).unwrap().clone(), 64);
15621562

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

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

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

15801582
#[test]
@@ -1596,17 +1598,19 @@ mod tests {
15961598
let router = TestRouter {};
15971599
let scorer = RefCell::new(TestScorer::new()
15981600
// 1st invoice, 1st path
1599-
.expect_usage(ChannelUsage { amount_msat: 10, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1600-
.expect_usage(ChannelUsage { amount_msat: 20, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16011601
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1602+
.expect_usage(ChannelUsage { amount_msat: 84, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1603+
.expect_usage(ChannelUsage { amount_msat: 94, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16021604
// 1st invoice, 2nd path
16031605
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1606+
.expect_usage(ChannelUsage { amount_msat: 74, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16041607
// 2nd invoice, 1st path
1605-
.expect_usage(ChannelUsage { amount_msat: 10, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1606-
.expect_usage(ChannelUsage { amount_msat: 20, inflight_htlc_msat: 64, effective_capacity: EffectiveCapacity::Unknown } )
16071608
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1609+
.expect_usage(ChannelUsage { amount_msat: 84, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1610+
.expect_usage(ChannelUsage { amount_msat: 94, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16081611
// 2nd invoice, 2nd path
16091612
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 64, effective_capacity: EffectiveCapacity::Unknown } )
1613+
.expect_usage(ChannelUsage { amount_msat: 74, inflight_htlc_msat: 74, effective_capacity: EffectiveCapacity::Unknown } )
16101614
);
16111615
let logger = TestLogger::new();
16121616
let invoice_payer =
@@ -1641,26 +1645,31 @@ mod tests {
16411645
let payer = TestPayer::new()
16421646
.expect_send(Amount::ForInvoice(final_value_msat))
16431647
.expect_send(Amount::OnRetry(final_value_msat / 2))
1644-
.expect_send(Amount::OnRetry(final_value_msat / 2));
1648+
.expect_send(Amount::OnRetry(final_value_msat / 4));
16451649
let final_value_msat = payment_invoice.amount_milli_satoshis().unwrap();
16461650
let router = TestRouter {};
16471651
let scorer = RefCell::new(TestScorer::new()
16481652
// 1st invoice, 1st path
1649-
.expect_usage(ChannelUsage { amount_msat: 10, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1650-
.expect_usage(ChannelUsage { amount_msat: 20, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16511653
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1654+
.expect_usage(ChannelUsage { amount_msat: 84, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1655+
.expect_usage(ChannelUsage { amount_msat: 94, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
16521656
// 1st invoice, 2nd path
16531657
.expect_usage(ChannelUsage { amount_msat: 64, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1654-
// Retry 1
1655-
.expect_usage(ChannelUsage { amount_msat: 10, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1656-
.expect_usage(ChannelUsage { amount_msat: 20, inflight_htlc_msat: 64, effective_capacity: EffectiveCapacity::Unknown } )
1658+
.expect_usage(ChannelUsage { amount_msat: 74, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1659+
// Retry 1, 1st path
16571660
.expect_usage(ChannelUsage { amount_msat: 32, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1661+
.expect_usage(ChannelUsage { amount_msat: 52, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1662+
.expect_usage(ChannelUsage { amount_msat: 62, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1663+
// Retry 1, 2nd path
16581664
.expect_usage(ChannelUsage { amount_msat: 32, inflight_htlc_msat: 64, effective_capacity: EffectiveCapacity::Unknown } )
1659-
// Retry 2
1660-
.expect_usage(ChannelUsage { amount_msat: 10, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1661-
.expect_usage(ChannelUsage { amount_msat: 20, inflight_htlc_msat: 96, effective_capacity: EffectiveCapacity::Unknown } )
1662-
.expect_usage(ChannelUsage { amount_msat: 32, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1663-
.expect_usage(ChannelUsage { amount_msat: 32, inflight_htlc_msat: 96, effective_capacity: EffectiveCapacity::Unknown } )
1665+
.expect_usage(ChannelUsage { amount_msat: 42, inflight_htlc_msat: 64 + 10, effective_capacity: EffectiveCapacity::Unknown } )
1666+
// Retry 2, 1st path
1667+
.expect_usage(ChannelUsage { amount_msat: 16, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1668+
.expect_usage(ChannelUsage { amount_msat: 36, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1669+
.expect_usage(ChannelUsage { amount_msat: 46, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown } )
1670+
// Retry 2, 2nd path
1671+
.expect_usage(ChannelUsage { amount_msat: 16, inflight_htlc_msat: 64 + 32, effective_capacity: EffectiveCapacity::Unknown } )
1672+
.expect_usage(ChannelUsage { amount_msat: 26, inflight_htlc_msat: 74 + 32 + 10, effective_capacity: EffectiveCapacity::Unknown } )
16641673
);
16651674
let logger = TestLogger::new();
16661675
let invoice_payer =
@@ -1689,7 +1698,7 @@ mod tests {
16891698
path: TestRouter::path_for_value(final_value_msat / 2),
16901699
short_channel_id: None,
16911700
retry: Some(RouteParameters {
1692-
final_value_msat: final_value_msat / 2,
1701+
final_value_msat: final_value_msat / 4,
16931702
..TestRouter::retry_for_invoice(&payment_invoice)
16941703
}),
16951704
});
@@ -1724,7 +1733,7 @@ mod tests {
17241733

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

17301739
#[test]
@@ -1754,8 +1763,8 @@ mod tests {
17541763
invoice_payer.pay_invoice(&invoice_to_pay).unwrap();
17551764
let inflight_map = invoice_payer.create_inflight_map();
17561765

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

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

0 commit comments

Comments
 (0)