Skip to content

1916 Followups Part 1 #1994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 2, 2023
Merged

1916 Followups Part 1 #1994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fbc0847
Move the final CLTV delta to `PaymentParameters` from `RouteParams`
TheBlueMatt Jan 27, 2023
2b13557
Use the `PaymentParameter` final CLTV delta over `RouteParameters`
TheBlueMatt Jan 27, 2023
5ce4bfc
Test the `RouteParameters` passed to `TestRouter`
TheBlueMatt Jan 27, 2023
ddde63e
Log more information when retrying a payment attempt fails
TheBlueMatt Jan 27, 2023
8af05e0
Move retry-limiting to `retry_payment_with_route`
TheBlueMatt Jan 27, 2023
0712972
Use only the failed amount when retrying payments, not the full amt
TheBlueMatt Jan 28, 2023
0765600
Reduce the code in the commitment_signed_dance macro
TheBlueMatt Jan 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions fuzz/src/full_stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
},
4 => {
let final_value_msat = slice_to_be24(get_slice!(3)) as u64;
let payment_params = PaymentParameters::from_node_id(get_pubkey!());
let payment_params = PaymentParameters::from_node_id(get_pubkey!(), 42);
let params = RouteParameters {
payment_params,
final_value_msat,
Expand All @@ -536,7 +536,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
},
15 => {
let final_value_msat = slice_to_be24(get_slice!(3)) as u64;
let payment_params = PaymentParameters::from_node_id(get_pubkey!());
let payment_params = PaymentParameters::from_node_id(get_pubkey!(), 42);
let params = RouteParameters {
payment_params,
final_value_msat,
Expand Down
9 changes: 6 additions & 3 deletions fuzz/src/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,13 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
let scorer = FixedPenaltyScorer::with_penalty(0);
let random_seed_bytes: [u8; 32] = [get_slice!(1)[0]; 32];
for target in node_pks.iter() {
let final_value_msat = slice_to_be64(get_slice!(8));
let final_cltv_expiry_delta = slice_to_be32(get_slice!(4));
let route_params = RouteParameters {
payment_params: PaymentParameters::from_node_id(*target).with_route_hints(last_hops.clone()),
final_value_msat: slice_to_be64(get_slice!(8)),
final_cltv_expiry_delta: slice_to_be32(get_slice!(4)),
payment_params: PaymentParameters::from_node_id(*target, final_cltv_expiry_delta)
.with_route_hints(last_hops.clone()),
final_value_msat,
final_cltv_expiry_delta,
};
let _ = find_route(&our_pubkey, &route_params, &net_graph,
first_hops.map(|c| c.iter().collect::<Vec<_>>()).as_ref().map(|a| a.as_slice()),
Expand Down
16 changes: 9 additions & 7 deletions lightning-invoice/src/payment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,8 @@ where
};

let payment_secret = Some(invoice.payment_secret().clone());
let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(),
invoice.min_final_cltv_expiry_delta() as u32)
.with_expiry_time(expiry_time_from_unix_epoch(&invoice).as_secs())
.with_route_hints(invoice.route_hints());
if let Some(features) = invoice.features() {
Expand Down Expand Up @@ -486,7 +487,7 @@ where
};

let route_params = RouteParameters {
payment_params: PaymentParameters::for_keysend(pubkey),
payment_params: PaymentParameters::for_keysend(pubkey, final_cltv_expiry_delta),
final_value_msat: amount_msats,
final_cltv_expiry_delta,
};
Expand Down Expand Up @@ -1377,7 +1378,7 @@ mod tests {
assert_eq!(*payer.attempts.borrow(), 1);

let retry = RouteParameters {
payment_params: PaymentParameters::for_keysend(pubkey),
payment_params: PaymentParameters::for_keysend(pubkey, final_cltv_expiry_delta),
final_value_msat,
final_cltv_expiry_delta,
};
Expand Down Expand Up @@ -1655,7 +1656,8 @@ mod tests {
}

fn retry_for_invoice(invoice: &Invoice) -> RouteParameters {
let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
let mut payment_params = PaymentParameters::from_node_id(
invoice.recover_payee_pub_key(), invoice.min_final_cltv_expiry_delta() as u32)
.with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs())
.with_route_hints(invoice.route_hints());
if let Some(features) = invoice.features() {
Expand Down Expand Up @@ -2071,7 +2073,7 @@ mod tests {
cltv_expiry_delta: 100,
}],
],
payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 100)),
};
let router = ManualRouter(RefCell::new(VecDeque::new()));
router.expect_find_route(Ok(route.clone()));
Expand Down Expand Up @@ -2114,7 +2116,7 @@ mod tests {
cltv_expiry_delta: 100,
}],
],
payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 100)),
};
let router = ManualRouter(RefCell::new(VecDeque::new()));
router.expect_find_route(Ok(route.clone()));
Expand Down Expand Up @@ -2194,7 +2196,7 @@ mod tests {
cltv_expiry_delta: 100,
}]
],
payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())),
payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 100)),
};
let router = ManualRouter(RefCell::new(VecDeque::new()));
router.expect_find_route(Ok(route.clone()));
Expand Down
6 changes: 4 additions & 2 deletions lightning-invoice/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,8 @@ mod test {
assert_eq!(invoice.route_hints()[0].0[0].htlc_minimum_msat, chan.inbound_htlc_minimum_msat);
assert_eq!(invoice.route_hints()[0].0[0].htlc_maximum_msat, chan.inbound_htlc_maximum_msat);

let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(),
invoice.min_final_cltv_expiry_delta() as u32)
.with_features(invoice.features().unwrap().clone())
.with_route_hints(invoice.route_hints());
let route_params = RouteParameters {
Expand Down Expand Up @@ -1087,7 +1088,8 @@ mod test {
assert_eq!(invoice.expiry_time(), Duration::from_secs(non_default_invoice_expiry_secs.into()));
assert!(!invoice.features().unwrap().supports_basic_mpp());

let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(),
invoice.min_final_cltv_expiry_delta() as u32)
.with_features(invoice.features().unwrap().clone())
.with_route_hints(invoice.route_hints());
let params = RouteParameters {
Expand Down
20 changes: 12 additions & 8 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,10 @@ pub(crate) enum HTLCSource {
first_hop_htlc_msat: u64,
payment_id: PaymentId,
payment_secret: Option<PaymentSecret>,
/// Note that this is now "deprecated" - we write it for forwards (and read it for
/// backwards) compatibility reasons, but prefer to use the data in the
/// [`super::outbound_payment`] module, which stores per-payment data once instead of in
/// each HTLC.
payment_params: Option<PaymentParameters>,
},
}
Expand Down Expand Up @@ -2473,7 +2477,7 @@ where
self.pending_outbound_payments
.send_payment(payment_hash, payment_secret, payment_id, retry_strategy, route_params,
&self.router, self.list_usable_channels(), self.compute_inflight_htlcs(),
&self.entropy_source, &self.node_signer, best_block_height,
&self.entropy_source, &self.node_signer, best_block_height, &self.logger,
|path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv|
self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv))
}
Expand All @@ -2489,7 +2493,7 @@ where
#[cfg(test)]
pub(crate) fn test_add_new_pending_payment(&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId, route: &Route) -> Result<Vec<[u8; 32]>, PaymentSendFailure> {
let best_block_height = self.best_block.read().unwrap().height();
self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, payment_secret, payment_id, route, Retry::Attempts(0), &self.entropy_source, best_block_height)
self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, payment_secret, payment_id, route, None, &self.entropy_source, best_block_height)
}


Expand Down Expand Up @@ -7357,9 +7361,9 @@ where
hash_map::Entry::Vacant(entry) => {
let path_fee = path.get_path_fees();
entry.insert(PendingOutboundPayment::Retryable {
retry_strategy: Retry::Attempts(0),
retry_strategy: None,
attempts: PaymentAttempts::new(),
route_params: None,
payment_params: None,
session_privs: [session_priv_bytes].iter().map(|a| *a).collect(),
payment_hash: htlc.payment_hash,
payment_secret,
Expand Down Expand Up @@ -7871,7 +7875,7 @@ mod tests {

// Next, attempt a keysend payment and make sure it fails.
let route_params = RouteParameters {
payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id()),
payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV),
final_value_msat: 100_000,
final_cltv_expiry_delta: TEST_FINAL_CLTV,
};
Expand Down Expand Up @@ -7964,7 +7968,7 @@ mod tests {

let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
let route_params = RouteParameters {
payment_params: PaymentParameters::for_keysend(payee_pubkey),
payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
final_value_msat: 10_000,
final_cltv_expiry_delta: 40,
};
Expand Down Expand Up @@ -8009,7 +8013,7 @@ mod tests {

let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
let route_params = RouteParameters {
payment_params: PaymentParameters::for_keysend(payee_pubkey),
payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
final_value_msat: 10_000,
final_cltv_expiry_delta: 40,
};
Expand Down Expand Up @@ -8574,7 +8578,7 @@ pub mod bench {
macro_rules! send_payment {
($node_a: expr, $node_b: expr) => {
let usable_channels = $node_a.list_usable_channels();
let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id())
let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id(), TEST_FINAL_CLTV)
.with_features($node_b.invoice_features());
let scorer = test_utils::TestScorer::with_penalty(0);
let seed = [3u8; 32];
Expand Down
Loading