Skip to content

Commit

Permalink
Get rid of unnecessary result in create_inbound_payment_for_hash
Browse files Browse the repository at this point in the history
  • Loading branch information
valentinewallace committed Nov 24, 2021
1 parent 2dfcd3f commit e52ef48
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 27 deletions.
20 changes: 6 additions & 14 deletions fuzz/src/chanmon_consistency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,22 +283,15 @@ fn check_payment_err(send_err: PaymentSendFailure) {
type ChanMan = ChannelManager<EnforcingSigner, Arc<TestChainMonitor>, Arc<TestBroadcaster>, Arc<KeyProvider>, Arc<FuzzEstimator>, Arc<dyn Logger>>;

#[inline]
fn get_payment_secret_hash(dest: &ChanMan, payment_id: &mut u8) -> Option<(PaymentSecret, PaymentHash)> {
let mut payment_hash;
for _ in 0..256 {
payment_hash = PaymentHash(Sha256::hash(&[*payment_id; 1]).into_inner());
if let Ok(payment_secret) = dest.create_inbound_payment_for_hash(payment_hash, None, 3600) {
return Some((payment_secret, payment_hash));
}
*payment_id = payment_id.wrapping_add(1);
}
None
fn get_payment_secret_hash(dest: &ChanMan, payment_id: &mut u8) -> (PaymentSecret, PaymentHash) {
let payment_hash = PaymentHash(Sha256::hash(&[*payment_id; 1]).into_inner());
let payment_secret = dest.create_inbound_payment_for_hash(payment_hash, None, 3600);
(payment_secret, payment_hash)
}

#[inline]
fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, payment_id: &mut u8) -> bool {
let (payment_secret, payment_hash) =
if let Some((secret, hash)) = get_payment_secret_hash(dest, payment_id) { (secret, hash) } else { return true; };
let (payment_secret, payment_hash) = get_payment_secret_hash(dest, payment_id);
if let Err(err) = source.send_payment(&Route {
paths: vec![vec![RouteHop {
pubkey: dest.get_our_node_id(),
Expand All @@ -316,8 +309,7 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p
}
#[inline]
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 {
let (payment_secret, payment_hash) =
if let Some((secret, hash)) = get_payment_secret_hash(dest, payment_id) { (secret, hash) } else { return true; };
let (payment_secret, payment_hash) = get_payment_secret_hash(dest, payment_id);
if let Err(err) = source.send_payment(&Route {
paths: vec![vec![RouteHop {
pubkey: middle.get_our_node_id(),
Expand Down
6 changes: 3 additions & 3 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4701,7 +4701,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
/// [`create_inbound_payment`]: Self::create_inbound_payment
/// [`PaymentReceived`]: events::Event::PaymentReceived
// For details on the implementation of this method, see `verify_inbound_payment_data`.
pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> PaymentSecret {
let (metadata_key, user_pmt_hash_key, _) = hkdf_extract_expand(&vec![0], &self.inbound_payment_key[..]);
let mut min_amt_msat_bytes: [u8; 8] = match min_value_msat {
Some(amt) => amt.to_be_bytes(),
Expand Down Expand Up @@ -4741,7 +4741,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
encrypted_metadata_slice[i] = chacha_block[i] ^ metadata_bytes[i];
}
}
Ok(PaymentSecret(payment_secret_bytes))
PaymentSecret(payment_secret_bytes)
}

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

$node_a.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
let payment_event = SendEvent::from_event($node_a.get_and_clear_pending_msg_events().pop().unwrap());
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,7 @@ macro_rules! get_payment_preimage_hash {
let payment_preimage = PaymentPreimage([*payment_count; 32]);
*payment_count += 1;
let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200).unwrap();
let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200);
(payment_preimage, payment_hash, payment_secret)
}
}
Expand Down
18 changes: 9 additions & 9 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,7 @@ fn test_duplicate_htlc_different_direction_onchain() {
let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &vec!(&nodes[1])[..], 900_000);

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

// Provide preimage to node 0 by claiming payment
Expand Down Expand Up @@ -5012,7 +5012,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {

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

let payment_secret = nodes[3].node.create_inbound_payment_for_hash(duplicate_payment_hash, None, 7200).unwrap();
let payment_secret = nodes[3].node.create_inbound_payment_for_hash(duplicate_payment_hash, None, 7200);
// We reduce the final CLTV here by a somewhat arbitrary constant to keep it under the one-byte
// script push size limit so that the below script length checks match
// ACCEPTED_HTLC_SCRIPT_WEIGHT.
Expand Down Expand Up @@ -5215,30 +5215,30 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
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
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], ds_dust_limit*1000);
// 2nd HTLC:
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
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
// 3rd HTLC:
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
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
// 4th HTLC:
let (_, payment_hash_3, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], 1000000);
// 5th HTLC:
let (_, payment_hash_4, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], 1000000);
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], 1000000);
// 6th HTLC:
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());
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));
// 7th HTLC:
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());
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));

// 8th HTLC:
let (_, payment_hash_5, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], 1000000);
// 9th HTLC:
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], ds_dust_limit*1000);
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
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

// 10th HTLC:
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
// 11th HTLC:
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], 1000000);
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());
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));

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

Expand Down

0 comments on commit e52ef48

Please sign in to comment.