Skip to content

Commit a7ab041

Browse files
committed
Queue splice pending events
Store splice pending outpoints in a per-channel FIFO so concurrent splice consumers cannot overwrite each other before they observe their event. Add coverage for back-to-back splice-in and splice-out activity on the same channel. The test exercises the wallet path that previously depended on a single pending outpoint per channel.
1 parent a346206 commit a7ab041

2 files changed

Lines changed: 189 additions & 29 deletions

File tree

orange-sdk/src/lightning_wallet.rs

Lines changed: 73 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use ldk_node::{NodeError, UserChannelId};
2727

2828
use graduated_rebalancer::{LightningBalance, ReceivedLightningPayment};
2929

30-
use std::collections::HashMap;
30+
use std::collections::{HashMap, VecDeque};
3131
use std::fmt::Debug;
3232
use std::pin::Pin;
3333
use std::sync::{Arc, Mutex};
@@ -51,22 +51,6 @@ pub(crate) struct LightningWalletImpl {
5151
lsp_socket_addr: SocketAddress,
5252
}
5353

54-
/// One pending `SplicePending` event per `user_channel_id`, consumed by
55-
/// `await_splice_pending`. A queue rather than a `watch` so each consumer takes its own event:
56-
/// `watch` would let a second splice on the same channel observe the previous splice's stale
57-
/// outpoint instead of waiting for the new `SplicePending`.
58-
pub(crate) struct SplicePendingInbox {
59-
pub(crate) pending: Mutex<HashMap<u128, OutPoint>>,
60-
pub(crate) notify: Notify,
61-
}
62-
63-
impl SplicePendingInbox {
64-
pub(crate) fn deliver(&self, channel_id: u128, funding_txo: OutPoint) {
65-
self.pending.lock().unwrap().insert(channel_id, funding_txo);
66-
self.notify.notify_waiters();
67-
}
68-
}
69-
7054
pub(crate) struct LightningWallet {
7155
pub(crate) inner: Arc<LightningWalletImpl>,
7256
}
@@ -242,18 +226,7 @@ impl LightningWallet {
242226
}
243227

244228
pub(crate) async fn await_splice_pending(&self, channel_id: u128) -> OutPoint {
245-
let inbox = &self.inner.splice_pending_inbox;
246-
loop {
247-
// Register interest BEFORE checking the queue so a `notify_waiters` racing with the
248-
// check still wakes us up.
249-
let notified = inbox.notify.notified();
250-
tokio::pin!(notified);
251-
notified.as_mut().enable();
252-
if let Some(txo) = inbox.pending.lock().unwrap().remove(&channel_id) {
253-
return txo;
254-
}
255-
notified.await;
256-
}
229+
self.inner.splice_pending_inbox.wait_for(channel_id).await
257230
}
258231

259232
pub(crate) fn get_on_chain_address(&self) -> Result<Address, NodeError> {
@@ -607,3 +580,74 @@ impl From<&PaymentDetails> for PaymentType {
607580
}
608581
}
609582
}
583+
584+
/// Pending `SplicePending` events keyed by `user_channel_id`, consumed by
585+
/// `await_splice_pending`. A per-channel queue rather than a `watch` so each consumer takes its own
586+
/// event: `watch` would let a second splice on the same channel observe the previous splice's stale
587+
/// outpoint instead of waiting for the new `SplicePending`.
588+
pub(crate) struct SplicePendingInbox {
589+
pub(crate) pending: Mutex<HashMap<u128, VecDeque<OutPoint>>>,
590+
pub(crate) notify: Notify,
591+
}
592+
593+
impl SplicePendingInbox {
594+
pub(crate) fn deliver(&self, channel_id: u128, funding_txo: OutPoint) {
595+
self.pending.lock().unwrap().entry(channel_id).or_default().push_back(funding_txo);
596+
self.notify.notify_waiters();
597+
}
598+
599+
pub(crate) async fn wait_for(&self, channel_id: u128) -> OutPoint {
600+
loop {
601+
// Register interest BEFORE checking the queue so a `notify_waiters` racing with the
602+
// check still wakes us up.
603+
let notified = self.notify.notified();
604+
tokio::pin!(notified);
605+
notified.as_mut().enable();
606+
if let Some(txo) = {
607+
let mut pending = self.pending.lock().unwrap();
608+
if let Some(queue) = pending.get_mut(&channel_id) {
609+
let txo = queue.pop_front();
610+
if queue.is_empty() {
611+
pending.remove(&channel_id);
612+
}
613+
txo
614+
} else {
615+
None
616+
}
617+
} {
618+
return txo;
619+
}
620+
notified.await;
621+
}
622+
}
623+
}
624+
625+
#[cfg(test)]
626+
mod tests {
627+
use super::*;
628+
use ldk_node::bitcoin::Txid;
629+
630+
fn dummy_outpoint(seed: u8) -> OutPoint {
631+
let bytes = [seed; 32];
632+
OutPoint { txid: Txid::from_byte_array(bytes), vout: seed as u32 }
633+
}
634+
635+
#[test]
636+
fn splice_pending_inbox_preserves_distinct_events_for_same_channel() {
637+
let inbox =
638+
SplicePendingInbox { pending: Mutex::new(HashMap::new()), notify: Notify::new() };
639+
let channel_id = 7;
640+
let first = dummy_outpoint(0xaa);
641+
let second = dummy_outpoint(0xbb);
642+
643+
inbox.deliver(channel_id, first);
644+
inbox.deliver(channel_id, second);
645+
646+
let mut pending = inbox.pending.lock().unwrap();
647+
let queue = pending.get_mut(&channel_id).expect("channel should have pending events");
648+
649+
assert_eq!(queue.pop_front(), Some(first));
650+
assert_eq!(queue.pop_front(), Some(second));
651+
assert!(queue.is_empty());
652+
}
653+
}

orange-sdk/tests/integration_tests.rs

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,122 @@ async fn test_receive_to_onchain_with_channel() {
599599
.await;
600600
}
601601

602+
#[tokio::test(flavor = "multi_thread")]
603+
#[test_log::test]
604+
async fn test_concurrent_splice_in_and_out_preserve_pending_events() {
605+
test_utils::run_test(|params| async move {
606+
let wallet = Arc::clone(&params.wallet);
607+
let lsp = Arc::clone(&params.lsp);
608+
let bitcoind = Arc::clone(&params.bitcoind);
609+
let third_party = Arc::clone(&params.third_party);
610+
let electrsd = Arc::clone(&params.electrsd);
611+
612+
open_channel_from_lsp(&wallet, Arc::clone(&third_party)).await;
613+
614+
generate_blocks(&bitcoind, &electrsd, 6).await;
615+
test_utils::wait_for_condition("wallet sync after channel open", || async {
616+
wallet.channels().iter().any(|a| a.confirmations.is_some_and(|c| c > 0) && a.is_usable)
617+
})
618+
.await;
619+
620+
let recv_amt = Amount::from_sats(300_000).unwrap();
621+
let uri = wallet.get_single_use_receive_uri(Some(recv_amt)).await.unwrap();
622+
let sent_txid = third_party
623+
.onchain_payment()
624+
.send_to_address(&uri.address.unwrap(), recv_amt.sats().unwrap(), None)
625+
.unwrap();
626+
627+
wait_for_tx(&electrsd.client, sent_txid).await;
628+
generate_blocks(&bitcoind, &electrsd, 6).await;
629+
wallet.sync_ln_wallet().unwrap();
630+
631+
test_utils::wait_for_condition("pending balance to update", || async {
632+
wallet.get_balance().await.unwrap().pending_balance == recv_amt
633+
})
634+
.await;
635+
636+
let event = wait_next_event(&wallet).await;
637+
match event {
638+
Event::OnchainPaymentReceived { txid, amount_sat, status, .. } => {
639+
assert_eq!(txid, sent_txid);
640+
assert_eq!(amount_sat, recv_amt.sats().unwrap());
641+
assert!(matches!(status, ConfirmationStatus::Confirmed { .. }));
642+
},
643+
ev => panic!("Expected OnchainPaymentReceived event, got {ev:?}"),
644+
}
645+
646+
let first_splice = tokio::time::timeout(Duration::from_secs(60), wallet.next_event_async())
647+
.await
648+
.expect("timed out waiting for splice-in event");
649+
wallet.event_handled().unwrap();
650+
let first_splice = match first_splice {
651+
Event::SplicePending {
652+
user_channel_id, counterparty_node_id, new_funding_txo, ..
653+
} => {
654+
assert_eq!(counterparty_node_id, lsp.node_id());
655+
(user_channel_id, new_funding_txo)
656+
},
657+
ev => panic!("Expected first SplicePending event, got {ev:?}"),
658+
};
659+
660+
generate_blocks(&bitcoind, &electrsd, 6).await;
661+
wallet.sync_ln_wallet().unwrap();
662+
663+
let addr = third_party.onchain_payment().new_address().unwrap();
664+
let send_amount = Amount::from_sats(10_000).unwrap();
665+
let pay_wallet = Arc::clone(&wallet);
666+
let pay_task = tokio::spawn(async move {
667+
let instr =
668+
pay_wallet.parse_payment_instructions(addr.to_string().as_str()).await.unwrap();
669+
let info = PaymentInfo::build(instr, Some(send_amount)).unwrap();
670+
pay_wallet.pay(&info).await
671+
});
672+
673+
let second_splice = tokio::time::timeout(Duration::from_secs(60), async {
674+
loop {
675+
let event = wallet.next_event_async().await;
676+
wallet.event_handled().unwrap();
677+
if let Event::SplicePending {
678+
user_channel_id,
679+
counterparty_node_id,
680+
new_funding_txo,
681+
..
682+
} = event
683+
{
684+
assert_eq!(counterparty_node_id, lsp.node_id());
685+
break (user_channel_id, new_funding_txo);
686+
}
687+
}
688+
})
689+
.await
690+
.expect("timed out waiting for splice-out event");
691+
692+
assert_eq!(
693+
first_splice.0, second_splice.0,
694+
"both splices should target the same LSP channel"
695+
);
696+
assert_ne!(
697+
first_splice.1, second_splice.1,
698+
"splices should have distinct funding outpoints"
699+
);
700+
701+
tokio::time::timeout(Duration::from_secs(60), pay_task)
702+
.await
703+
.expect("splice-out payment hung waiting for its SplicePending")
704+
.expect("payment task panicked")
705+
.expect("splice-out payment failed");
706+
707+
test_utils::wait_for_condition("splice-in rebalance metadata", || async {
708+
wallet.list_transactions().await.unwrap().iter().any(|tx| {
709+
tx.payment_type == PaymentType::IncomingOnChain { txid: Some(sent_txid) }
710+
&& tx.fee.is_some_and(|fee| fee > Amount::ZERO)
711+
})
712+
})
713+
.await;
714+
})
715+
.await;
716+
}
717+
602718
async fn run_test_pay_lightning_from_self_custody(amountless: bool) {
603719
test_utils::run_test(move |params| async move {
604720
let wallet = Arc::clone(&params.wallet);

0 commit comments

Comments
 (0)