Skip to content

Add PaymentId in ChannelManager.list_recent_payments() #2567

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.

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 1 commit into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions lightning/src/chain/chainmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -854,8 +854,8 @@ mod tests {
create_announced_chan_between_nodes(&nodes, 0, 1);

// Route two payments to be claimed at the same time.
let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_2, payment_hash_2, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clear();
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
Expand Down Expand Up @@ -962,7 +962,7 @@ mod tests {
let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);

// First route a payment that we will claim on chain and give the recipient the preimage.
let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
nodes[1].node.claim_funds(payment_preimage);
expect_payment_claimed!(nodes[1], payment_hash, 1_000_000);
nodes[1].node.get_and_clear_pending_msg_events();
Expand Down
30 changes: 15 additions & 15 deletions lightning/src/ln/chanmon_update_fail_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ fn test_monitor_and_persister_update_fail() {
send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000);

// Route an HTLC from node 0 to node 1 (but don't settle)
let (preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);
let (preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);

// Make a copy of the ChainMonitor so we can capture the error it returns on a
// bogus update. Note that if instead we updated the nodes[0]'s ChainMonitor
Expand Down Expand Up @@ -286,7 +286,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;

let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

// Now try to send a second payment which will fail to send
let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand Down Expand Up @@ -858,7 +858,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

// Route a first payment that we'll fail backwards
let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
let (_, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);

// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
nodes[2].node.fail_htlc_backwards(&payment_hash_1);
Expand Down Expand Up @@ -1128,7 +1128,7 @@ fn test_monitor_update_fail_reestablish() {
let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1);
create_announced_chan_between_nodes(&nodes, 1, 2);

let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
let (payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);

nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id());
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
Expand Down Expand Up @@ -1344,7 +1344,7 @@ fn claim_while_disconnected_monitor_update_fail() {
let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;

// Forward a payment for B to claim
let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id());
Expand Down Expand Up @@ -1644,7 +1644,7 @@ fn test_monitor_update_fail_claim() {
// Rebalance a bit so that we can send backwards from 3 to 2.
send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
nodes[1].node.claim_funds(payment_preimage_1);
Expand Down Expand Up @@ -1763,7 +1763,7 @@ fn test_monitor_update_on_pending_forwards() {
// Rebalance a bit so that we can send backwards from 3 to 1.
send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
let (_, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
nodes[2].node.fail_htlc_backwards(&payment_hash_1);
expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[2], vec![HTLCDestination::FailedPayment { payment_hash: payment_hash_1 }]);
check_added_monitors!(nodes[2], 1);
Expand Down Expand Up @@ -1835,7 +1835,7 @@ fn monitor_update_claim_fail_no_response() {
let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;

// Forward a payment for B to claim
let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

// Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand Down Expand Up @@ -2177,7 +2177,7 @@ fn test_fail_htlc_on_broadcast_after_claim() {
create_announced_chan_between_nodes(&nodes, 0, 1);
let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2).2;

let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000);
let (payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000);

let bs_txn = get_local_commitment_txn!(nodes[2], chan_id_2);
assert_eq!(bs_txn.len(), 1);
Expand Down Expand Up @@ -2343,7 +2343,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
//
// Note that because, at the end, MonitorUpdateInProgress is still set, the HTLC generated in
// (c) will not be freed from the holding cell.
let (payment_preimage_0, payment_hash_0, _) = route_payment(&nodes[1], &[&nodes[0]], 100_000);
let (payment_preimage_0, payment_hash_0, ..) = route_payment(&nodes[1], &[&nodes[0]], 100_000);

nodes[0].node.send_payment_with_route(&route, payment_hash_1,
RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0)).unwrap();
Expand Down Expand Up @@ -2517,7 +2517,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
create_announced_chan_between_nodes(&nodes, 0, 1);
let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2).2;

let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100_000);
let (payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100_000);

let mut as_raa = None;
if htlc_status == HTLCStatusAtDupClaim::HoldingCell {
Expand Down Expand Up @@ -2747,8 +2747,8 @@ fn double_temp_error() {

let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1);

let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_2, payment_hash_2, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
// `claim_funds` results in a ChannelMonitorUpdate.
Expand Down Expand Up @@ -3045,8 +3045,8 @@ fn test_blocked_chan_preimage_release() {

// Tee up two payments in opposite directions across nodes[1], one it sent to generate a
// PaymentSent event and one it forwards.
let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[1], &[&nodes[2]], 1_000_000);
let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[2], &[&nodes[1], &nodes[0]], 1_000_000);
let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[1], &[&nodes[2]], 1_000_000);
let (payment_preimage_2, payment_hash_2, ..) = route_payment(&nodes[2], &[&nodes[1], &nodes[0]], 1_000_000);

// Claim the first payment to get a `PaymentSent` event (but don't handle it yet).
nodes[2].node.claim_funds(payment_preimage_1);
Expand Down
22 changes: 17 additions & 5 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,8 @@ impl From<&ClaimableHTLC> for events::ClaimedHTLC {
}
}

/// A payment identifier used to uniquely identify a payment to LDK.
/// A user-provided identifier in [`ChannelManager::send_payment`] used to uniquely identify
/// a payment and ensure idempotency in LDK.
///
/// This is not exported to bindings users as we just use [u8; 32] directly
#[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -1669,11 +1670,15 @@ pub enum ChannelShutdownState {
pub enum RecentPaymentDetails {
/// When an invoice was requested and thus a payment has not yet been sent.
AwaitingInvoice {
/// Identifier for the payment to ensure idempotency.
/// A user-provided identifier in [`ChannelManager::send_payment`] used to uniquely identify
/// a payment and ensure idempotency in LDK.
payment_id: PaymentId,
},
/// When a payment is still being sent and awaiting successful delivery.
Pending {
/// A user-provided identifier in [`ChannelManager::send_payment`] used to uniquely identify
/// a payment and ensure idempotency in LDK.
payment_id: PaymentId,
/// Hash of the payment that is currently being sent but has yet to be fulfilled or
/// abandoned.
payment_hash: PaymentHash,
Expand All @@ -1685,6 +1690,9 @@ pub enum RecentPaymentDetails {
/// been resolved. Upon receiving [`Event::PaymentSent`], we delay for a few minutes before the
/// payment is removed from tracking.
Fulfilled {
/// A user-provided identifier in [`ChannelManager::send_payment`] used to uniquely identify
/// a payment and ensure idempotency in LDK.
payment_id: PaymentId,
/// Hash of the payment that was claimed. `None` for serializations of [`ChannelManager`]
/// made before LDK version 0.0.104.
payment_hash: Option<PaymentHash>,
Expand All @@ -1693,6 +1701,9 @@ pub enum RecentPaymentDetails {
/// abandoned via [`ChannelManager::abandon_payment`], it is marked as abandoned until all
/// pending HTLCs for this payment resolve and an [`Event::PaymentFailed`] is generated.
Abandoned {
/// A user-provided identifier in [`ChannelManager::send_payment`] used to uniquely identify
/// a payment and ensure idempotency in LDK.
payment_id: PaymentId,
/// Hash of the payment that we have given up trying to send.
payment_hash: PaymentHash,
},
Expand Down Expand Up @@ -2429,15 +2440,16 @@ where
},
PendingOutboundPayment::Retryable { payment_hash, total_msat, .. } => {
Some(RecentPaymentDetails::Pending {
payment_id: *payment_id,
payment_hash: *payment_hash,
total_msat: *total_msat,
})
},
PendingOutboundPayment::Abandoned { payment_hash, .. } => {
Some(RecentPaymentDetails::Abandoned { payment_hash: *payment_hash })
Some(RecentPaymentDetails::Abandoned { payment_id: *payment_id, payment_hash: *payment_hash })
},
PendingOutboundPayment::Fulfilled { payment_hash, .. } => {
Some(RecentPaymentDetails::Fulfilled { payment_hash: *payment_hash })
Some(RecentPaymentDetails::Fulfilled { payment_id: *payment_id, payment_hash: *payment_hash })
},
PendingOutboundPayment::Legacy { .. } => None
})
Expand Down Expand Up @@ -9824,7 +9836,7 @@ mod tests {

// To start (1), send a regular payment but don't claim it.
let expected_route = [&nodes[1]];
let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &expected_route, 100_000);
let (payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &expected_route, 100_000);

// Next, attempt a keysend payment and make sure it fails.
let route_params = RouteParameters::from_payment_params_and_value(
Expand Down
6 changes: 3 additions & 3 deletions lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2467,7 +2467,7 @@ pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:

pub const TEST_FINAL_CLTV: u32 = 70;

pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret, PaymentId) {
let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV)
.with_bolt11_features(expected_route.last().unwrap().node.invoice_features()).unwrap();
let route_params = RouteParameters::from_payment_params_and_value(payment_params, recv_value);
Expand All @@ -2479,7 +2479,7 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
}

let res = send_along_route(origin_node, route, expected_route, recv_value);
(res.0, res.1, res.2)
(res.0, res.1, res.2, res.3)
}

pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) {
Expand All @@ -2506,7 +2506,7 @@ pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
assert!(err.contains("Cannot send value that would put us over the max HTLC value in flight our peer will accept")));
}

pub fn send_payment<'a, 'b, 'c>(origin: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
pub fn send_payment<'a, 'b, 'c>(origin: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret, PaymentId) {
let res = route_payment(&origin, expected_route, recv_value);
claim_payment(&origin, expected_route, res.0);
res
Expand Down
Loading