Correctly emit PaymentReceivedEvent on rebalances

Previously we did not do this under bad assumptions. We were able to do
this only because in tests we could guaranatee the payment ids were the
same across trusted and self custodial because we were using ldk-node's
payment id. However, with actual trusted backends we can't do this so we
have no way to see if the payment is a rebalance until after we've
received it.

To prevent this from happening again we now mangle the id in
DummyTrustedWallet so we don't accidentially have tests assuming that.

Author: benthecarman

orange-sdk/src/event.rs

@@ -359,23 +359,14 @@ impl LdkEventHandler {
 					}
 				});
 
-				let payment_id = PaymentId::Trusted(payment_id.0);
-				let is_rebalance = {
-					let map = self.tx_metadata.read();
-					map.get(&payment_id).is_some_and(|m| m.ty.is_rebalance())
-				};
-
-				// If this is a rebalance payment, we do not emit a PaymentReceived event.
-				if !is_rebalance {
-					if let Err(e) = self.event_queue.add_event(Event::PaymentReceived {
-						payment_id,
-						payment_hash,
-						amount_msat,
-						custom_records,
-						lsp_fee_msats,
-					}) {
-						log_error!(self.logger, "Failed to add PaymentReceived event: {e:?}");
-					}
+				if let Err(e) = self.event_queue.add_event(Event::PaymentReceived {
+					payment_id: PaymentId::SelfCustodial(payment_id.0),
+					payment_hash,
+					amount_msat,
+					custom_records,
+					lsp_fee_msats,
+				}) {
+					log_error!(self.logger, "Failed to add PaymentReceived event: {e:?}");
 				}
 				let _ = self.payment_receipt_sender.send(());
 			},

orange-sdk/src/lib.rs

@@ -20,7 +20,7 @@ pub use bitcoin_payment_instructions::PaymentMethod;
 use bitcoin_payment_instructions::amount::Amount;
 
 use crate::rebalancer::{OrangeRebalanceEventHandler, OrangeTrigger};
-use crate::store::{PaymentId, TxMetadata, TxMetadataStore, TxType};
+use crate::store::{TxMetadata, TxMetadataStore, TxType};
 #[cfg(feature = "cashu")]
 use crate::trusted_wallet::cashu::Cashu;
 #[cfg(feature = "_test-utils")]
@@ -74,7 +74,7 @@ pub use ldk_node::bitcoin;
 pub use ldk_node::payment::ConfirmationStatus;
 #[cfg(feature = "spark")]
 pub use spark_wallet::{OperatorPoolConfig, ServiceProviderConfig, SparkWalletConfig};
-pub use store::{PaymentType, Transaction, TxStatus};
+pub use store::{PaymentId, PaymentType, Transaction, TxStatus};
 pub use trusted_wallet::ExtraConfig;
 
 #[cfg(feature = "uniffi")]

orange-sdk/src/store.rs

@@ -130,7 +130,9 @@ impl From<Transaction> for StoreTransaction {
 /// the payment to the user.
 #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
 pub enum PaymentId {
+	/// A self-custodial payment identifier.
 	SelfCustodial([u8; 32]),
+	/// A trusted payment identifier.
 	Trusted([u8; 32]),
 }
 

orange-sdk/src/trusted_wallet/dummy.rs

@@ -92,7 +92,7 @@ impl DummyTrustedWallet {
 						payment_preimage,
 					} => {
 						// convert id
-						let id = payment_id.unwrap().0;
+						let id = mangle_payment_id(payment_id.unwrap().0);
 
 						let mut payments = pays.write().unwrap();
 						let item = payments.iter_mut().find(|p| p.id == id);
@@ -123,7 +123,7 @@ impl DummyTrustedWallet {
 					},
 					Event::PaymentFailed { payment_id, payment_hash, reason } => {
 						// convert id
-						let id = payment_id.unwrap().0;
+						let id = mangle_payment_id(payment_id.unwrap().0);
 
 						let mut payments = pays.write().unwrap();
 						let item = payments.iter().cloned().enumerate().find(|(_, p)| p.id == id);
@@ -152,7 +152,7 @@ impl DummyTrustedWallet {
 					},
 					Event::PaymentReceived { payment_id, amount_msat, payment_hash, .. } => {
 						// convert id
-						let id = payment_id.unwrap().0;
+						let id = mangle_payment_id(payment_id.unwrap().0);
 
 						let mut payments = pays.write().unwrap();
 						// We create invoices on the fly without adding the payment to our list
@@ -301,18 +301,24 @@ impl TrustedWalletInterface for DummyTrustedWallet {
 		Box::pin(async move {
 			let id = match method {
 				PaymentMethod::LightningBolt11(inv) => {
-					self.ldk_node
+					let id = self
+						.ldk_node
 						.bolt11_payment()
 						.send_using_amount(&inv, amount.milli_sats(), None)
 						.unwrap()
-						.0
+						.0;
+
+					mangle_payment_id(id)
 				},
 				PaymentMethod::LightningBolt12(offer) => {
-					self.ldk_node
+					let id = self
+						.ldk_node
 						.bolt12_payment()
 						.send_using_amount(&offer, amount.milli_sats(), None, None)
 						.unwrap()
-						.0
+						.0;
+
+					mangle_payment_id(id)
 				},
 				PaymentMethod::OnChain(address) => {
 					let txid = self
@@ -353,3 +359,13 @@ impl TrustedWalletInterface for DummyTrustedWallet {
 		})
 	}
 }
+
+// we don't want our payment ids to be the same as LDK's, this ended up not testing
+// bad assumptions properly. So we mangle the payment id a bit here to avoid collisions.
+fn mangle_payment_id(id: [u8; 32]) -> [u8; 32] {
+	let mut mangled = id;
+	for i in 0..32 {
+		mangled[i] = mangled[i].wrapping_add(0x42);
+	}
+	mangled
+}

orange-sdk/tests/integration_tests.rs

@@ -218,18 +218,23 @@ fn test_sweep_to_ln() {
 			_ => panic!("Expected ChannelOpened event"),
 		}
 
-		// because of an issue in CDK, we skip the rest of this test
-		// otherwise it will fail for now
-		// TODO REMOVE ME
-		if cfg!(feature = "_cashu-tests") {
-			return;
+		let expect_amt = intermediate_amt.saturating_add(recv_amt);
+
+		let event = wait_next_event(&wallet).await;
+		match event {
+			Event::PaymentReceived { payment_id, amount_msat, lsp_fee_msats, .. } => {
+				assert!(matches!(payment_id, orange_sdk::PaymentId::SelfCustodial(_)));
+				assert!(lsp_fee_msats.is_some());
+				assert_eq!(amount_msat, expect_amt.milli_sats() - lsp_fee_msats.unwrap());
+			},
+			e => panic!("Expected RebalanceSuccessful event, got {e:?}"),
 		}
 
 		let event = wait_next_event(&wallet).await;
 		match event {
 			Event::RebalanceSuccessful { amount_msat, fee_msat, .. } => {
 				assert!(fee_msat > 0);
-				assert_eq!(amount_msat, intermediate_amt.saturating_add(recv_amt).milli_sats());
+				assert_eq!(amount_msat, expect_amt.milli_sats());
 			},
 			e => panic!("Expected RebalanceSuccessful event, got {e:?}"),
 		}