Skip to content

Commit c60e390

Browse files
committed
Fix race condition in rebalances
Before we would return a successful rebalance on when we received the final payment, but we did not wait for the trusted wallet to get the succcess back. This could cause a race where if we called list_transactions() after the rebalance but before the trusted wallet got the success event, list_transactions would not correctly reflect the rebalance and would hit a debug_assert. This fixes by waiting for the payment success event on the trusted side.
1 parent 0acfff6 commit c60e390

7 files changed

Lines changed: 195 additions & 27 deletions

File tree

graduated-rebalancer/src/lib.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,10 @@ pub trait TrustedWallet: Send + Sync {
7979
&self, method: PaymentMethod, amount: Amount,
8080
) -> Pin<Box<dyn Future<Output = Result<[u8; 32], Self::Error>> + Send + '_>>;
8181

82-
/// Get transaction fee paid
83-
fn get_tx_fee(&self, id: [u8; 32])
84-
-> Pin<Box<dyn Future<Output = Option<Amount>> + Send + '_>>;
82+
/// Wait for a payment success notification
83+
fn await_payment_success(
84+
&self, payment_hash: [u8; 32],
85+
) -> Pin<Box<dyn Future<Output = Option<ReceivedLightningPayment>> + Send + '_>>;
8586
}
8687

8788
/// Trait representing a lightning wallet backend
@@ -265,10 +266,6 @@ where
265266
amount_msat: transfer_amt.milli_sats(),
266267
});
267268

268-
// get the fee of the rebalance transaction
269-
let rebalance_tx_fee =
270-
self.trusted.get_tx_fee(rebalance_id).await.unwrap_or(Amount::ZERO);
271-
272269
let ln_payment = match self
273270
.ln_wallet
274271
.await_payment_receipt(expected_hash.to_byte_array())
@@ -281,6 +278,18 @@ where
281278
},
282279
};
283280

281+
let trusted_payment = match self
282+
.trusted
283+
.await_payment_success(expected_hash.to_byte_array())
284+
.await
285+
{
286+
Some(success) => success,
287+
None => {
288+
log_error!(self.logger, "Failed to send rebalance payment!");
289+
return;
290+
},
291+
};
292+
284293
log_info!(
285294
self.logger,
286295
"Rebalance succeeded. Sent trusted tx {} to lightning tx {}",
@@ -294,7 +303,7 @@ where
294303
ln_rebalance_payment_id: ln_payment.id,
295304
amount_msat: transfer_amt.milli_sats(),
296305
fee_msat: ln_payment.fee_paid_msat.unwrap_or_default()
297-
+ rebalance_tx_fee.milli_sats(),
306+
+ trusted_payment.fee_paid_msat.unwrap_or_default(),
298307
});
299308
},
300309
Err(e) => {

orange-sdk/Cargo.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@ reqwest = { version = "0.12.23", default-features = false, features = ["rustls-t
2626
breez-sdk-spark = { git = "https://github.com/breez/spark-sdk.git", rev = "41212dfcfe36e22a55ac224c791b326f259f90d6", default-features = false, features = ["rustls-tls"], optional = true }
2727
tokio = { version = "1.0", default-features = false, features = ["rt-multi-thread", "sync"] }
2828
uuid = { version = "1.0", default-features = false, optional = true }
29-
cdk = { git = "https://github.com/benthecarman/cdk.git", rev = "7d25e9ae5ed7f47f9ae7e87d8a9ee16797fee8cd", default-features = false, features = ["wallet"], optional = true }
29+
cdk = { git = "https://github.com/benthecarman/cdk.git", rev = "39c1206a4a1dda2adc1f3e23628136ef645f6c6b", default-features = false, features = ["wallet"], optional = true }
3030
serde_json = { version = "1.0", optional = true }
3131
async-trait = "0.1"
3232
log = "0.4.28"
3333

3434
corepc-node = { version = "0.8.0", features = ["29_0", "download"], optional = true }
35-
cdk-ldk-node = { git = "https://github.com/benthecarman/cdk.git", rev = "7d25e9ae5ed7f47f9ae7e87d8a9ee16797fee8cd", optional = true }
36-
cdk-sqlite = { git = "https://github.com/benthecarman/cdk.git", rev = "7d25e9ae5ed7f47f9ae7e87d8a9ee16797fee8cd", optional = true }
37-
cdk-axum = { git = "https://github.com/benthecarman/cdk.git", rev = "7d25e9ae5ed7f47f9ae7e87d8a9ee16797fee8cd", optional = true }
35+
cdk-ldk-node = { git = "https://github.com/benthecarman/cdk.git", rev = "39c1206a4a1dda2adc1f3e23628136ef645f6c6b", optional = true }
36+
cdk-sqlite = { git = "https://github.com/benthecarman/cdk.git", rev = "39c1206a4a1dda2adc1f3e23628136ef645f6c6b", optional = true }
37+
cdk-axum = { git = "https://github.com/benthecarman/cdk.git", rev = "39c1206a4a1dda2adc1f3e23628136ef645f6c6b", optional = true }
3838
axum = { version = "0.8.1", optional = true }
3939

4040
uniffi = { version = "0.29", features = ["cli", "tokio"], optional = true }

orange-sdk/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -887,10 +887,10 @@ impl Wallet {
887887
}
888888
}
889889

890-
for (_, tx_info) in internal_transfers {
890+
for (id, tx_info) in internal_transfers {
891891
debug_assert!(
892892
tx_info.send_fee.is_some(),
893-
"Internal transfers must have a send fee, got {tx_info:?}",
893+
"Internal transfers must have a send fee, got {id}: {tx_info:?}",
894894
);
895895
debug_assert!(tx_info.transaction.is_some());
896896

orange-sdk/src/trusted_wallet/cashu/mod.rs

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! An implementation of `TrustedWalletInterface` using the Cashu (CDK) SDK.
22
3+
use crate::bitcoin::hex::DisplayHex;
34
use crate::logging::Logger;
45
use crate::store::{PaymentId, TxMetadataStore, TxStatus};
56
use crate::trusted_wallet::{Payment, TrustedError, TrustedWalletInterface};
@@ -26,8 +27,11 @@ use cdk::wallet::Wallet;
2627
use cdk::wallet::types::{Transaction, TransactionDirection};
2728
use cdk::{Amount as CdkAmount, StreamExt};
2829

30+
use graduated_rebalancer::ReceivedLightningPayment;
31+
2932
use tokio::sync::{mpsc, watch};
3033

34+
use std::collections::HashMap;
3135
use std::future::Future;
3236
use std::pin::Pin;
3337
use std::str::FromStr;
@@ -55,6 +59,8 @@ pub struct Cashu {
5559
cashu_wallet: Arc<Wallet>,
5660
unit: CurrencyUnit,
5761
shutdown_sender: watch::Sender<()>,
62+
payment_success_sender: watch::Sender<()>,
63+
payment_success_flag: watch::Receiver<()>,
5864
logger: Arc<Logger>,
5965
supports_bolt12: bool,
6066
mint_quote_sender: mpsc::Sender<MintQuote>,
@@ -291,8 +297,14 @@ impl TrustedWalletInterface for Cashu {
291297
let event_queue = Arc::clone(&self.event_queue);
292298
let tx_metadata = self.tx_metadata.clone();
293299
let quote_id = quote.id.clone();
300+
let payment_success_sender = self.payment_success_sender.clone();
294301
self.runtime.spawn(async move {
295-
match cashu_wallet.melt(&quote_id).await {
302+
let mut metadata = HashMap::new();
303+
if let Some(hash) = &payment_hash {
304+
metadata.insert(PAYMENT_HASH_METADATA_KEY.to_string(), hash.to_string());
305+
}
306+
307+
match cashu_wallet.melt_with_metadata(&quote_id, metadata).await {
296308
Ok(res) => {
297309
match res.state {
298310
MeltQuoteState::Paid => {
@@ -304,6 +316,8 @@ impl TrustedWalletInterface for Cashu {
304316
map.get(&payment_id).is_some_and(|m| m.ty.is_rebalance())
305317
};
306318
if is_rebalance {
319+
// make sure we still send payment success
320+
payment_success_sender.send(()).unwrap();
307321
return;
308322
}
309323

@@ -369,6 +383,8 @@ impl TrustedWalletInterface for Cashu {
369383
payment_preimage,
370384
fee_paid_msat: Some(fee_paid_sat * 1_000), // convert to msats
371385
});
386+
387+
payment_success_sender.send(()).unwrap();
372388
},
373389
MeltQuoteState::Failed => {
374390
log_error!(logger, "Melt failed for quote: {quote_id}");
@@ -418,6 +434,35 @@ impl TrustedWalletInterface for Cashu {
418434
})
419435
}
420436

437+
fn await_payment_success(
438+
&self, payment_hash: [u8; 32],
439+
) -> Pin<Box<dyn Future<Output = Option<ReceivedLightningPayment>> + Send + '_>> {
440+
Box::pin(async move {
441+
loop {
442+
let txs = self
443+
.cashu_wallet
444+
.list_transactions(Some(TransactionDirection::Outgoing))
445+
.await
446+
.ok()?;
447+
448+
let hex = payment_hash.to_lower_hex_string();
449+
let tx = txs.iter().find(|tx| {
450+
tx.metadata.get(PAYMENT_HASH_METADATA_KEY).is_some_and(|h| h == &hex)
451+
});
452+
453+
if let Some(tx) = tx {
454+
let payment_id = Self::id_to_32_byte_array(tx.quote_id.as_ref().expect("safe"));
455+
return Some(ReceivedLightningPayment {
456+
id: payment_id,
457+
fee_paid_msat: Some(convert_amount(tx.fee, &self.unit).ok()?.milli_sats()),
458+
});
459+
}
460+
461+
self.await_payment_success().await;
462+
}
463+
})
464+
}
465+
421466
fn stop(&self) -> Pin<Box<dyn Future<Output = ()> + Send + '_>> {
422467
Box::pin(async move {
423468
log_info!(self.logger, "Stopping Cashu wallet");
@@ -426,6 +471,8 @@ impl TrustedWalletInterface for Cashu {
426471
}
427472
}
428473

474+
const PAYMENT_HASH_METADATA_KEY: &str = "payment_hash";
475+
429476
impl Cashu {
430477
pub(crate) async fn init(
431478
config: &WalletConfig, cashu_config: CashuConfig, store: Arc<dyn KVStore + Sync + Send>,
@@ -485,6 +532,7 @@ impl Cashu {
485532
.unwrap_or(false);
486533

487534
let (shutdown_sender, mut shutdown_receiver) = watch::channel::<()>(());
535+
let (payment_success_sender, payment_success_flag) = watch::channel(());
488536

489537
// Create channel for mint quote monitoring with bounded capacity
490538
let (mint_quote_sender, mut mint_quote_receiver) = mpsc::channel::<MintQuote>(32);
@@ -554,6 +602,8 @@ impl Cashu {
554602
cashu_wallet,
555603
unit: cashu_config.unit,
556604
shutdown_sender,
605+
payment_success_sender,
606+
payment_success_flag,
557607
logger,
558608
supports_bolt12,
559609
mint_quote_sender,
@@ -644,6 +694,12 @@ impl Cashu {
644694
}
645695
Ok(())
646696
}
697+
698+
pub(crate) async fn await_payment_success(&self) {
699+
let mut flag = self.payment_success_flag.clone();
700+
flag.mark_unchanged();
701+
let _ = flag.changed().await;
702+
}
647703
}
648704

649705
fn convert_amount(cdk_amount: CdkAmount, unit: &CurrencyUnit) -> Result<Amount, TrustedError> {

orange-sdk/src/trusted_wallet/dummy.rs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@ use bitcoin_payment_instructions::PaymentMethod;
88
use bitcoin_payment_instructions::amount::Amount;
99
use corepc_node::client::bitcoin::Network;
1010
use corepc_node::{Node as Bitcoind, get_available_port};
11+
use graduated_rebalancer::ReceivedLightningPayment;
12+
use ldk_node::lightning::ln::channelmanager;
1113
use ldk_node::lightning::ln::msgs::SocketAddress;
1214
use ldk_node::lightning_invoice::{Bolt11Invoice, Bolt11InvoiceDescription, Description};
15+
use ldk_node::payment::{PaymentKind, PaymentStatus};
1316
use ldk_node::{Event, Node};
1417
use rand::RngCore;
1518
use std::env::temp_dir;
@@ -18,6 +21,7 @@ use std::sync::atomic::{AtomicU64, Ordering};
1821
use std::sync::{Arc, RwLock};
1922
use std::time::Duration;
2023
use tokio::runtime::Runtime;
24+
use tokio::sync::watch;
2125
use uuid::Uuid;
2226

2327
/// A dummy implementation of `TrustedWalletInterface` for testing purposes.
@@ -28,6 +32,7 @@ pub(crate) struct DummyTrustedWallet {
2832
current_bal_msats: Arc<AtomicU64>,
2933
payments: Arc<RwLock<Vec<Payment>>>,
3034
ldk_node: Arc<Node>,
35+
payment_success_flag: watch::Receiver<()>,
3136
}
3237

3338
#[derive(Clone)]
@@ -78,6 +83,8 @@ impl DummyTrustedWallet {
7883
let current_bal_msats = Arc::new(AtomicU64::new(0));
7984
let payments: Arc<RwLock<Vec<Payment>>> = Arc::new(RwLock::new(vec![]));
8085

86+
let (payment_success_sender, payment_success_flag) = watch::channel(());
87+
8188
let events_ref = Arc::clone(&ldk_node);
8289
let bal = Arc::clone(&current_bal_msats);
8390
let pays = Arc::clone(&payments);
@@ -126,6 +133,8 @@ impl DummyTrustedWallet {
126133
})
127134
.unwrap();
128135
}
136+
137+
payment_success_sender.send(()).unwrap();
129138
},
130139
Event::PaymentFailed { payment_id, payment_hash, reason } => {
131140
// convert id
@@ -241,7 +250,13 @@ impl DummyTrustedWallet {
241250
panic!("No usable channels found {channels:?}");
242251
}
243252

244-
DummyTrustedWallet { current_bal_msats, payments, ldk_node }
253+
DummyTrustedWallet { current_bal_msats, payments, ldk_node, payment_success_flag }
254+
}
255+
256+
pub(crate) async fn await_payment_success(&self) {
257+
let mut flag = self.payment_success_flag.clone();
258+
flag.mark_unchanged();
259+
let _ = flag.changed().await;
245260
}
246261
}
247262

@@ -359,6 +374,40 @@ impl TrustedWalletInterface for DummyTrustedWallet {
359374
})
360375
}
361376

377+
fn await_payment_success(
378+
&self, payment_hash: [u8; 32],
379+
) -> Pin<Box<dyn Future<Output = Option<ReceivedLightningPayment>> + Send + '_>> {
380+
Box::pin(async move {
381+
let id = channelmanager::PaymentId(payment_hash);
382+
loop {
383+
if let Some(payment) = self.ldk_node.payment(&id) {
384+
let counterparty_skimmed_fee_msat = match payment.kind {
385+
PaymentKind::Bolt11 { hash, .. } => {
386+
debug_assert!(hash.0 == payment_hash, "Payment Hash mismatch");
387+
None
388+
},
389+
PaymentKind::Bolt11Jit { hash, counterparty_skimmed_fee_msat, .. } => {
390+
debug_assert!(hash.0 == payment_hash, "Payment Hash mismatch");
391+
counterparty_skimmed_fee_msat
392+
},
393+
_ => return None, // Ignore other payment kinds, we only care about the one we just sent.
394+
};
395+
match payment.status {
396+
PaymentStatus::Succeeded => {
397+
return Some(ReceivedLightningPayment {
398+
id: payment.id.0,
399+
fee_paid_msat: counterparty_skimmed_fee_msat,
400+
});
401+
},
402+
PaymentStatus::Pending => {},
403+
PaymentStatus::Failed => return None,
404+
}
405+
}
406+
self.await_payment_success().await;
407+
}
408+
})
409+
}
410+
362411
fn stop(&self) -> Pin<Box<dyn Future<Output = ()> + Send + '_>> {
363412
Box::pin(async move {
364413
let _ = self.ldk_node.stop();

orange-sdk/src/trusted_wallet/mod.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ use ldk_node::lightning_invoice::Bolt11Invoice;
77
use bitcoin_payment_instructions::PaymentMethod;
88
use bitcoin_payment_instructions::amount::Amount;
99

10+
use graduated_rebalancer::ReceivedLightningPayment;
11+
1012
use std::future::Future;
1113
use std::pin::Pin;
1214
use std::sync::Arc;
@@ -82,6 +84,12 @@ pub trait TrustedWalletInterface: Send + Sync + private::Sealed {
8284
&self, method: PaymentMethod, amount: Amount,
8385
) -> Pin<Box<dyn Future<Output = Result<[u8; 32], TrustedError>> + Send + '_>>;
8486

87+
/// Waits for a payment with the given payment hash to succeed.
88+
/// Returns the `ReceivedLightningPayment` if successful, or `None` if it fails or times out.
89+
fn await_payment_success(
90+
&self, payment_hash: [u8; 32],
91+
) -> Pin<Box<dyn Future<Output = Option<ReceivedLightningPayment>> + Send + '_>>;
92+
8593
/// Stops the wallet, cleaning up any resources.
8694
/// This is typically used to gracefully shut down the wallet.
8795
fn stop(&self) -> Pin<Box<dyn Future<Output = ()> + Send + '_>>;
@@ -110,13 +118,10 @@ impl<T: ?Sized + TrustedWalletInterface> graduated_rebalancer::TrustedWallet for
110118
Box::pin(async move { self.0.pay(method, amount).await })
111119
}
112120

113-
fn get_tx_fee(
114-
&self, id: [u8; 32],
115-
) -> Pin<Box<dyn Future<Output = Option<Amount>> + Send + '_>> {
116-
Box::pin(async move {
117-
let trusted_txs = self.0.list_payments().await.unwrap_or_default();
118-
trusted_txs.iter().find(|p| p.id == id).map(|p| p.fee)
119-
})
121+
fn await_payment_success(
122+
&self, payment_hash: [u8; 32],
123+
) -> Pin<Box<dyn Future<Output = Option<ReceivedLightningPayment>> + Send + '_>> {
124+
Box::pin(async move { self.0.await_payment_success(payment_hash).await })
120125
}
121126
}
122127

0 commit comments

Comments
 (0)