Skip to content
Open
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
36 changes: 27 additions & 9 deletions src/payment/unified.rs
Comment thread
elnafateh marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,22 @@ impl UnifiedPayment {

let payment_result = if let Ok(hrn) = HumanReadableName::from_encoded(uri_str) {
let hrn = maybe_wrap(hrn.clone());
self.bolt12_payment.send_using_amount_inner(&offer, amount_msat.unwrap_or(0), None, None, route_parameters, Some(hrn))
self.bolt12_payment.send_using_amount_inner(
&offer,
amount_msat.unwrap_or(0),
None,
None,
route_parameters,
Some(hrn),
)
} else if let Some(amount_msat) = amount_msat {
self.bolt12_payment.send_using_amount(&offer, amount_msat, None, None, route_parameters)
self.bolt12_payment.send_using_amount(
&offer,
amount_msat,
None,
None,
route_parameters,
)
} else {
self.bolt12_payment.send(&offer, None, None, route_parameters)
}
Expand All @@ -304,14 +317,19 @@ impl UnifiedPayment {
},
PaymentMethod::LightningBolt11(invoice) => {
let invoice = maybe_wrap(invoice.clone());
let payment_result = self.bolt11_invoice.send(&invoice, route_parameters)
.map_err(|e| {
let payment_result = self.bolt11_invoice.send(&invoice, route_parameters);

match payment_result {
Ok(payment_id) => {
return Ok(UnifiedPaymentResult::Bolt11 { payment_id });
},
Err(Error::DuplicatePayment) => {
log_error!(self.logger, "Failed to send BOLT11 invoice: DuplicatePayment. This is part of a unified payment. Aborting to avoid duplicate payment.");
return Err(Error::DuplicatePayment);
},
Err(e) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this error can be just a persistence error happening in send/send_internal, with the payment being initiated. Fall back would be a duplicate payment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, persistence failures return a separate PersistenceFailed variant, so aborting on DuplicatePayment here is safe.

log_error!(self.logger, "Failed to send BOLT11 invoice: {:?}. This is part of a unified payment. Falling back to the on-chain transaction.", e);
e
});

if let Ok(payment_id) = payment_result {
return Ok(UnifiedPaymentResult::Bolt11 { payment_id });
},
}
},
PaymentMethod::OnChain(address) => {
Expand Down
79 changes: 79 additions & 0 deletions tests/integration_tests_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2967,6 +2967,85 @@ async fn unified_send_receive_bip21_uri() {
assert_eq!(node_b.list_balances().total_lightning_balance_sats, 200_000);
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn unified_send_bolt11_duplicate_payment_no_onchain_fallback() {
// Regression test for https://github.com/lightningdevkit/ldk-node/issues/1033
//
// Sending a unified BIP21 payment that resolves to BOLT11 should return
// Error::DuplicatePayment on retry, not fall back to the on-chain method.

let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
let chain_source = random_chain_source(&bitcoind, &electrsd);

let (node_a, node_b) = setup_two_nodes(&chain_source, false, false);

let address_a = node_a.onchain_payment().new_address().unwrap();
let premined_sats = 5_000_000;

premine_and_distribute_funds(
&bitcoind.client,
&electrsd.client,
vec![address_a],
Amount::from_sat(premined_sats),
)
.await;

node_a.sync_wallets().unwrap();
open_channel(&node_a, &node_b, 4_000_000, true, &electrsd).await;
generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await;

node_a.sync_wallets().unwrap();
node_b.sync_wallets().unwrap();

expect_channel_ready_event!(node_a, node_b.node_id());
expect_channel_ready_event!(node_b, node_a.node_id());

// Sleep until we broadcast a node announcement.
while node_b.status().latest_node_announcement_broadcast_timestamp.is_none() {
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
}
tokio::time::sleep(std::time::Duration::from_secs(1)).await;

let expected_amount_sats = 100_000;
let expiry_sec = 4_000;

// Receive a unified payment on node_b — this will produce a URI with BOLT12 offer + BOLT11 invoice.
let uri_str =
node_b.unified_payment().receive(expected_amount_sats, "asdf", expiry_sec).unwrap();

// Strip the BOLT12 offer so the URI resolves to BOLT11 only (no BOLT12, no on-chain fallback).
let uri_str_bolt11_only = uri_str.split("&lno=").next().unwrap();

// First send: should succeed via BOLT11.
let first_result = node_a.unified_payment().send(uri_str_bolt11_only, None, None).await;
let first_payment_id = match first_result {
Ok(UnifiedPaymentResult::Bolt11 { payment_id }) => payment_id,
Ok(other) => panic!("Expected Bolt11 result on first send, got: {:?}", other),
Err(e) => panic!("Expected Bolt11 result on first send, got error: {:?}", e),
};
expect_payment_successful_event!(node_a, Some(first_payment_id), None);

// Second send with the same URI: should return DuplicatePayment, NOT fall back to on-chain.
let second_result = node_a.unified_payment().send(uri_str_bolt11_only, None, None).await;
match second_result {
Err(NodeError::DuplicatePayment) => {
// Expected — this is the fix for #1033.
},
Ok(UnifiedPaymentResult::Onchain { txid }) => {
panic!(
"Regression: Duplicate BOLT11 payment fell back to on-chain. txid={}. See #1033",
txid
);
},
Ok(other) => {
panic!("Expected DuplicatePayment error on retry, got: {:?}", other);
},
Err(other) => {
panic!("Expected DuplicatePayment error on retry, got: {:?}", other);
},
}
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn lsps2_client_service_integration() {
do_lsps2_client_service_integration(true).await;
Expand Down
Loading