Skip to content

Commit 20f79b3

Browse files
Get rid of unnecessary result in create_inbound_payment_for_hash
1 parent ab238c7 commit 20f79b3

File tree

4 files changed

+19
-27
lines changed

4 files changed

+19
-27
lines changed

fuzz/src/chanmon_consistency.rs

+6-14
Original file line numberDiff line numberDiff line change
@@ -280,22 +280,15 @@ fn check_payment_err(send_err: PaymentSendFailure) {
280280
type ChanMan = ChannelManager<EnforcingSigner, Arc<TestChainMonitor>, Arc<TestBroadcaster>, Arc<KeyProvider>, Arc<FuzzEstimator>, Arc<dyn Logger>>;
281281

282282
#[inline]
283-
fn get_payment_secret_hash(dest: &ChanMan, payment_id: &mut u8) -> Option<(PaymentSecret, PaymentHash)> {
284-
let mut payment_hash;
285-
for _ in 0..256 {
286-
payment_hash = PaymentHash(Sha256::hash(&[*payment_id; 1]).into_inner());
287-
if let Ok(payment_secret) = dest.create_inbound_payment_for_hash(payment_hash, None, 3600) {
288-
return Some((payment_secret, payment_hash));
289-
}
290-
*payment_id = payment_id.wrapping_add(1);
291-
}
292-
None
283+
fn get_payment_secret_hash(dest: &ChanMan, payment_id: &mut u8) -> (PaymentSecret, PaymentHash) {
284+
let payment_hash = PaymentHash(Sha256::hash(&[*payment_id; 1]).into_inner());
285+
let payment_secret = dest.create_inbound_payment_for_hash(payment_hash, None, 3600);
286+
(payment_secret, payment_hash)
293287
}
294288

295289
#[inline]
296290
fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, payment_id: &mut u8) -> bool {
297-
let (payment_secret, payment_hash) =
298-
if let Some((secret, hash)) = get_payment_secret_hash(dest, payment_id) { (secret, hash) } else { return true; };
291+
let (payment_secret, payment_hash) = get_payment_secret_hash(dest, payment_id);
299292
if let Err(err) = source.send_payment(&Route {
300293
paths: vec![vec![RouteHop {
301294
pubkey: dest.get_our_node_id(),
@@ -313,8 +306,7 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p
313306
}
314307
#[inline]
315308
fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, dest: &ChanMan, dest_chan_id: u64, amt: u64, payment_id: &mut u8) -> bool {
316-
let (payment_secret, payment_hash) =
317-
if let Some((secret, hash)) = get_payment_secret_hash(dest, payment_id) { (secret, hash) } else { return true; };
309+
let (payment_secret, payment_hash) = get_payment_secret_hash(dest, payment_id);
318310
if let Err(err) = source.send_payment(&Route {
319311
paths: vec![vec![RouteHop {
320312
pubkey: middle.get_our_node_id(),

lightning/src/ln/channelmanager.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -4710,7 +4710,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
47104710
//
47114711
// Then on payment receipt, we verify in `verify_inbound_payment_data` that the payment preimage
47124712
// and payment secret match what was constructed in `create_inbound_payment`.
4713-
pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
4713+
pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> PaymentSecret {
47144714
let mut min_amt_msat_bytes: [u8; 8] = match min_value_msat {
47154715
Some(amt) => amt.to_be_bytes(),
47164716
None => [0; 8],
@@ -4753,7 +4753,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
47534753
encrypted_metadata_slice[i] = chacha_bytes[i] ^ metadata_bytes[i];
47544754
}
47554755
}
4756-
Ok(PaymentSecret(payment_secret_bytes))
4756+
PaymentSecret(payment_secret_bytes)
47574757
}
47584758

47594759
#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]
@@ -6833,7 +6833,7 @@ pub mod bench {
68336833
payment_preimage.0[0..8].copy_from_slice(&payment_count.to_le_bytes());
68346834
payment_count += 1;
68356835
let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
6836-
let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200).unwrap();
6836+
let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200);
68376837

68386838
$node_a.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
68396839
let payment_event = SendEvent::from_event($node_a.get_and_clear_pending_msg_events().pop().unwrap());

lightning/src/ln/functional_test_utils.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -999,7 +999,7 @@ macro_rules! get_payment_preimage_hash {
999999
let payment_preimage = PaymentPreimage([*payment_count; 32]);
10001000
*payment_count += 1;
10011001
let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
1002-
let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200).unwrap();
1002+
let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200);
10031003
(payment_preimage, payment_hash, payment_secret)
10041004
}
10051005
}

lightning/src/ln/functional_tests.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -1152,7 +1152,7 @@ fn test_duplicate_htlc_different_direction_onchain() {
11521152
let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &vec!(&nodes[1])[..], 900_000);
11531153

11541154
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[0], 800_000);
1155-
let node_a_payment_secret = nodes[0].node.create_inbound_payment_for_hash(payment_hash, None, 7200).unwrap();
1155+
let node_a_payment_secret = nodes[0].node.create_inbound_payment_for_hash(payment_hash, None, 7200);
11561156
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[0]]], 800_000, payment_hash, node_a_payment_secret);
11571157

11581158
// Provide preimage to node 0 by claiming payment
@@ -4957,7 +4957,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
49574957

49584958
let (our_payment_preimage, duplicate_payment_hash, _) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 900000);
49594959

4960-
let payment_secret = nodes[3].node.create_inbound_payment_for_hash(duplicate_payment_hash, None, 7200).unwrap();
4960+
let payment_secret = nodes[3].node.create_inbound_payment_for_hash(duplicate_payment_hash, None, 7200);
49614961
// We reduce the final CLTV here by a somewhat arbitrary constant to keep it under the one-byte
49624962
// script push size limit so that the below script length checks match
49634963
// ACCEPTED_HTLC_SCRIPT_WEIGHT.
@@ -5160,30 +5160,30 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
51605160
let (_, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], ds_dust_limit*1000); // not added < dust limit + HTLC tx fee
51615161
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], ds_dust_limit*1000);
51625162
// 2nd HTLC:
5163-
send_along_route_with_secret(&nodes[1], route.clone(), &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_1, nodes[5].node.create_inbound_payment_for_hash(payment_hash_1, None, 7200).unwrap()); // not added < dust limit + HTLC tx fee
5163+
send_along_route_with_secret(&nodes[1], route.clone(), &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_1, nodes[5].node.create_inbound_payment_for_hash(payment_hash_1, None, 7200)); // not added < dust limit + HTLC tx fee
51645164
// 3rd HTLC:
5165-
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_2, nodes[5].node.create_inbound_payment_for_hash(payment_hash_2, None, 7200).unwrap()); // not added < dust limit + HTLC tx fee
5165+
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_2, nodes[5].node.create_inbound_payment_for_hash(payment_hash_2, None, 7200)); // not added < dust limit + HTLC tx fee
51665166
// 4th HTLC:
51675167
let (_, payment_hash_3, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], 1000000);
51685168
// 5th HTLC:
51695169
let (_, payment_hash_4, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], 1000000);
51705170
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], 1000000);
51715171
// 6th HTLC:
5172-
send_along_route_with_secret(&nodes[1], route.clone(), &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_3, nodes[5].node.create_inbound_payment_for_hash(payment_hash_3, None, 7200).unwrap());
5172+
send_along_route_with_secret(&nodes[1], route.clone(), &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_3, nodes[5].node.create_inbound_payment_for_hash(payment_hash_3, None, 7200));
51735173
// 7th HTLC:
5174-
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_4, nodes[5].node.create_inbound_payment_for_hash(payment_hash_4, None, 7200).unwrap());
5174+
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_4, nodes[5].node.create_inbound_payment_for_hash(payment_hash_4, None, 7200));
51755175

51765176
// 8th HTLC:
51775177
let (_, payment_hash_5, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], 1000000);
51785178
// 9th HTLC:
51795179
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], ds_dust_limit*1000);
5180-
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_5, nodes[5].node.create_inbound_payment_for_hash(payment_hash_5, None, 7200).unwrap()); // not added < dust limit + HTLC tx fee
5180+
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_5, nodes[5].node.create_inbound_payment_for_hash(payment_hash_5, None, 7200)); // not added < dust limit + HTLC tx fee
51815181

51825182
// 10th HTLC:
51835183
let (_, payment_hash_6, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], ds_dust_limit*1000); // not added < dust limit + HTLC tx fee
51845184
// 11th HTLC:
51855185
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], 1000000);
5186-
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_6, nodes[5].node.create_inbound_payment_for_hash(payment_hash_6, None, 7200).unwrap());
5186+
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_6, nodes[5].node.create_inbound_payment_for_hash(payment_hash_6, None, 7200));
51875187

51885188
// Double-check that six of the new HTLC were added
51895189
// We now have six HTLCs pending over the dust limit and six HTLCs under the dust limit (ie,
@@ -7164,7 +7164,7 @@ fn test_check_htlc_underpaying() {
71647164
let payee = Payee::from_node_id(nodes[1].node.get_our_node_id()).with_features(InvoiceFeatures::known());
71657165
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();
71667166
let (_, our_payment_hash, _) = get_payment_preimage_hash!(nodes[0]);
7167-
let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200).unwrap();
7167+
let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200);
71687168
nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
71697169
check_added_monitors!(nodes[0], 1);
71707170

0 commit comments

Comments
 (0)