From e4dbe31e0a88e54331753323c6a5e4364b391d7c Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 7 Jan 2026 11:29:49 +0100 Subject: [PATCH 01/29] Set restrictive file permissions for seed file Previously, seed files were created using `fs::File::create()` which inherits the default umask, potentially making the sensitive seed material world-readable on Unix systems. This change: - Creates seed files with mode 0o400 (owner read only) on Unix - Uses `create_new` instead of `create` to atomically fail if the file already exists, providing defense-in-depth against TOCTOU race conditions Co-Authored-By: Claude AI Backport of e93844aa981fb0f80e588d49ff1357bff326ac2e Conflicts Resolved in src/io/utils.rs --- src/io/utils.rs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/io/utils.rs b/src/io/utils.rs index 1b4b02a827..e6fcc6ba1c 100644 --- a/src/io/utils.rs +++ b/src/io/utils.rs @@ -5,12 +5,15 @@ // http://opensource.org/licenses/MIT>, at your option. You may not use this file except in // accordance with one or both of these licenses. -use std::fs; +use std::fs::{self, OpenOptions}; use std::io::Write; use std::ops::Deref; use std::path::Path; use std::sync::Arc; +#[cfg(unix)] +use std::os::unix::fs::OpenOptionsExt; + use bdk_chain::indexer::keychain_txout::ChangeSet as BdkIndexerChangeSet; use bdk_chain::local_chain::ChangeSet as BdkLocalChainChangeSet; use bdk_chain::miniscript::{Descriptor, DescriptorPublicKey}; @@ -113,10 +116,23 @@ where })?; } - let mut f = fs::File::create(keys_seed_path).map_err(|e| { - log_error!(logger, "Failed to create keys seed file: {}", keys_seed_path); - e - })?; + #[cfg(unix)] + let mut f = OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o400) + .open(keys_seed_path) + .map_err(|e| { + log_error!(logger, "Failed to create keys seed file: {}", keys_seed_path); + e + })?; + + #[cfg(not(unix))] + let mut f = + OpenOptions::new().write(true).create_new(true).open(keys_seed_path).map_err(|e| { + log_error!(logger, "Failed to create keys seed file: {}", keys_seed_path); + e + })?; f.write_all(&key).map_err(|e| { log_error!(logger, "Failed to write node keys seed to disk: {}", keys_seed_path); From 042c5de0575ca226e28e9ceee5cb7453751b1c3e Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 2 Apr 2026 09:35:35 +0200 Subject: [PATCH 02/29] Run rate limiter garbage collection before inserting new user Move the GC pass from after insertion to before, so that stale entries are reclaimed before allocating a new bucket. This avoids unnecessary growth of the user map between GC cycles. AI tools were used in preparing this commit. Backport of 818cccddec27a4905dbd0e419bd3bde4cd583e13 --- src/payment/asynchronous/rate_limiter.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/payment/asynchronous/rate_limiter.rs b/src/payment/asynchronous/rate_limiter.rs index 671b1dc72a..bf12508927 100644 --- a/src/payment/asynchronous/rate_limiter.rs +++ b/src/payment/asynchronous/rate_limiter.rs @@ -23,6 +23,8 @@ pub(crate) struct RateLimiter { max_idle: Duration, } +const MAX_USERS: usize = 10_000; + struct Bucket { tokens: u32, last_refill: Instant, @@ -36,10 +38,19 @@ impl RateLimiter { pub(crate) fn allow(&mut self, user_id: &[u8]) -> bool { let now = Instant::now(); - let entry = self.users.entry(user_id.to_vec()); - let is_new_user = matches!(entry, std::collections::hash_map::Entry::Vacant(_)); + let is_new_user = !self.users.contains_key(user_id); + + if is_new_user { + self.garbage_collect(self.max_idle); + if self.users.len() >= MAX_USERS { + return false; + } + } - let bucket = entry.or_insert(Bucket { tokens: self.capacity, last_refill: now }); + let bucket = self + .users + .entry(user_id.to_vec()) + .or_insert(Bucket { tokens: self.capacity, last_refill: now }); let elapsed = now.duration_since(bucket.last_refill); let tokens_to_add = (elapsed.as_secs_f64() / self.refill_interval.as_secs_f64()) as u32; @@ -56,11 +67,6 @@ impl RateLimiter { false }; - // Each time a new user is added, we take the opportunity to clean up old rate limits. - if is_new_user { - self.garbage_collect(self.max_idle); - } - allow } From 618deed91baffae6fc0aa7c58045a2d6cb078e73 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 19 May 2026 14:04:01 +0200 Subject: [PATCH 03/29] Reject malformed (odd-length) hex strings Previously, we'd attempt to decode any hex string even if it had an odd number of input characters. Here, we check the length and return `None` in case it's odd. Signed-off-by: Elias Rohrer Backport of 818998443318a96b04838ad7ebf8ab48a20164b2 --- src/hex_utils.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/hex_utils.rs b/src/hex_utils.rs index d56c6fd992..054e8b1f28 100644 --- a/src/hex_utils.rs +++ b/src/hex_utils.rs @@ -9,6 +9,11 @@ use std::fmt::Write; #[cfg(feature = "uniffi")] pub fn to_vec(hex: &str) -> Option> { + // Reject malformed hex strings. + if hex.len() % 2 != 0 { + return None; + } + let mut out = Vec::with_capacity(hex.len() / 2); let mut b = 0; From 73a255b79b6025e97493c991bc458d5ab39ddeab Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 8 Oct 2026 12:41:55 +0200 Subject: [PATCH 04/29] Refuse skimmed fees on payments without a record The check that an LSP didn't withhold more than the agreed-upon JIT channel fee only ran for inbound payments we already had a record for. BOLT12 offer and spontaneous payments only get their record created inside the claim handler, so they skipped the check entirely and were claimed no matter how much the counterparty skimmed. Refuse any withheld fee for payments without a stored record, as we never authorized a skim for them. This matches the behavior on main, where the check moved to the authenticated BOLT11 payment metadata. Only this part of the upstream change is backported. Backport of 44146f9c8cb82b9f2946525736b4303195aba743 Conflicts Resolved in src/event.rs Co-Authored-By: HAL 9000 --- src/event.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/event.rs b/src/event.rs index f64710d508..ae94249e60 100644 --- a/src/event.rs +++ b/src/event.rs @@ -790,6 +790,18 @@ where }, _ => {}, } + } else if counterparty_skimmed_fee_msat > 0 { + // We only ever authorize an LSP to withhold a fee for JIT-channel payments we + // previously registered. Any other payment (e.g., BOLT12 offer or spontaneous + // payments, for which no record exists yet) must not have fees skimmed. + log_info!( + self.logger, + "Refusing inbound payment with hash {} as the counterparty withheld {}msat without a corresponding JIT-channel payment authorizing it", + hex_utils::to_string(&payment_hash.0), + counterparty_skimmed_fee_msat, + ); + self.channel_manager.fail_htlc_backwards(&payment_hash); + return Ok(()); } log_info!( From fbd3bd7449c705120858935485d50722880e18b5 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 10 Jun 2026 11:05:36 +0200 Subject: [PATCH 05/29] Reject oversized splice-out amounts Validate splice-out requests against outbound capacity after converting the requested satoshi amount to millisatoshis with overflow handling. This prevents values above the spendable channel balance from slipping past the guard due to a unit mismatch. Keep splice integration coverage aligned with the corrected capacity semantics by rejecting an amount one satoshi above outbound capacity and deriving the full-cycle splice-out amount from the channel's current spendable capacity. AI-Assisted-By: OpenAI Codex Co-Authored-By: HAL 9000 This finding was discovered by Project Loupe Backport of ca4d4c348fefa22779924068c3112d8c5bd62dfd --- src/lib.rs | 4 +++- tests/common/mod.rs | 4 +++- tests/integration_tests_rust.rs | 12 ++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 2fdcdddbf4..490c5b9afc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1375,7 +1375,9 @@ impl Node { if let Some(channel_details) = open_channels.iter().find(|c| c.user_channel_id == user_channel_id.0) { - if splice_amount_sats > channel_details.outbound_capacity_msat { + let splice_amount_msat = + splice_amount_sats.checked_mul(1_000).ok_or(Error::ChannelSplicingFailed)?; + if splice_amount_msat > channel_details.outbound_capacity_msat { return Err(Error::ChannelSplicingFailed); } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 699f8f1d0e..833dfecc21 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -1108,7 +1108,9 @@ pub(crate) async fn do_channel_full_cycle( println!("\nB splices out to pay A"); let addr_a = node_a.onchain_payment().new_address().unwrap(); - let splice_out_sat = funding_amount_sat / 2; + let available_splice_out_sat = node_b.list_channels()[0].outbound_capacity_msat / 1000; + let splice_out_sat = available_splice_out_sat / 2; + assert!(splice_out_sat > 500_000); node_b.splice_out(&user_channel_id_b, node_a.node_id(), &addr_a, splice_out_sat).unwrap(); expect_splice_pending_event!(node_a, node_b.node_id()); diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index d6c7c9447f..e34a5526f0 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -973,6 +973,18 @@ async fn splice_channel() { ); assert_eq!(node_b.list_balances().total_lightning_balance_sats, 0); + let address = node_a.onchain_payment().new_address().unwrap(); + let excessive_splice_out_sats = node_a.list_channels()[0].outbound_capacity_msat / 1000 + 1; + assert_eq!( + node_a.splice_out( + &user_channel_id_a, + node_b.node_id(), + &address, + excessive_splice_out_sats + ), + Err(NodeError::ChannelSplicingFailed), + ); + // Test that splicing and payments fail when there are insufficient funds let address = node_b.onchain_payment().new_address().unwrap(); let amount_msat = 400_000_000; From de12588555e5e2b911f7fabbaf9e41eedbfd3c1e Mon Sep 17 00:00:00 2001 From: benthecarman Date: Fri, 12 Jun 2026 14:29:30 -0500 Subject: [PATCH 06/29] Reject overflowing unified receive amounts Return InvalidAmount when converting the requested satoshi amount to millisatoshis would overflow. This keeps debug and release behavior consistent and avoids producing a URI whose on-chain amount differs from its Lightning payment amount. This commit was created with assistance from OpenAI Codex. This finding was discovered by Project Loupe Backport of 9ab21064d13cea191c4cd936ef686476f87bdd71 Conflicts Resolved in src/payment/unified_qr.rs, tests/integration_tests_rust.rs --- src/payment/unified_qr.rs | 4 ++-- tests/integration_tests_rust.rs | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/payment/unified_qr.rs b/src/payment/unified_qr.rs index d3c879cba1..84f784e46a 100644 --- a/src/payment/unified_qr.rs +++ b/src/payment/unified_qr.rs @@ -91,9 +91,9 @@ impl UnifiedQrPayment { pub fn receive( &self, amount_sats: u64, description: &str, expiry_sec: u32, ) -> Result { - let onchain_address = self.onchain_payment.new_address()?; + let amount_msats = amount_sats.checked_mul(1_000).ok_or(Error::InvalidAmount)?; - let amount_msats = amount_sats * 1_000; + let onchain_address = self.onchain_payment.new_address()?; let bolt12_offer = match self.bolt12_payment.receive_inner(amount_msats, description, None, None) { diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index e34a5526f0..d6022c8f96 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -1580,6 +1580,18 @@ async fn generate_bip21_uri() { assert!(uqr_payment.contains("lno=")); } +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn unified_receive_rejects_msat_overflow() { + let (_bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = TestChainSource::Esplora(&electrsd); + let node = setup_node(&chain_source, random_config(true), None); + + assert_eq!( + Err(NodeError::InvalidAmount), + node.unified_qr_payment().receive(u64::MAX, "asdf", 4_000) + ); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn unified_qr_send_receive() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); From 09f9bbbc6322c47dbb193d98b031213c874ed1aa Mon Sep 17 00:00:00 2001 From: benthecarman Date: Fri, 12 Jun 2026 14:52:23 -0500 Subject: [PATCH 07/29] Avoid Bolt11 claim amount underflow Use saturating arithmetic when accounting for skimmed JIT-channel fees while validating manually claimed payments. This prevents an oversized skimmed fee from underflowing the expected claimable amount. This commit was created with assistance from OpenAI Codex. This finding was discovered by Project Loupe Backport of 187056b750c926b6cf4bd5d9ffcfe9bc1125d520 --- src/payment/bolt11.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/payment/bolt11.rs b/src/payment/bolt11.rs index 60c313381a..0b938e7def 100644 --- a/src/payment/bolt11.rs +++ b/src/payment/bolt11.rs @@ -331,7 +331,7 @@ impl Bolt11Payment { _ => 0, }; if let Some(invoice_amount_msat) = details.amount_msat { - if claimable_amount_msat < invoice_amount_msat - skimmed_fee_msat { + if claimable_amount_msat < invoice_amount_msat.saturating_sub(skimmed_fee_msat) { log_error!( self.logger, "Failed to manually claim payment {} as the claimable amount is less than expected", From c4f3c17e6435115fecaaf7be5f2d16895e1cf0e0 Mon Sep 17 00:00:00 2001 From: benthecarman Date: Mon, 15 Jun 2026 15:26:43 -0500 Subject: [PATCH 08/29] Keep insert-or-update cache unchanged on persist failure Build the updated object separately and persist it before replacing the cached entry, so failed writes leave memory aligned with storage. This finding was discovered by Project Loupe AI-Assisted-By: OpenAI Codex Backport of 6560dffb931077f6e9914a229e3048d7e48d92b3 Conflicts Resolved in src/data_store.rs --- src/data_store.rs | 121 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 104 insertions(+), 17 deletions(-) diff --git a/src/data_store.rs b/src/data_store.rs index 87bd831c9b..89b899c81e 100644 --- a/src/data_store.rs +++ b/src/data_store.rs @@ -5,7 +5,7 @@ // http://opensource.org/licenses/MIT>, at your option. You may not use this file except in // accordance with one or both of these licenses. -use std::collections::{hash_map, HashMap}; +use std::collections::HashMap; use std::ops::Deref; use std::sync::{Arc, Mutex}; @@ -75,23 +75,23 @@ where pub(crate) fn insert_or_update(&self, object: SO) -> Result { let mut locked_objects = self.objects.lock().unwrap(); - let updated; - match locked_objects.entry(object.id()) { - hash_map::Entry::Occupied(mut e) => { - let update = object.to_update(); - updated = e.get_mut().update(&update); - if updated { - self.persist(&e.get())?; - } - }, - hash_map::Entry::Vacant(e) => { - e.insert(object.clone()); - self.persist(&object)?; - updated = true; - }, - } + let id = object.id(); + let updated_object = if let Some(existing_object) = locked_objects.get(&id) { + let mut updated_object = existing_object.clone(); + let updated = updated_object.update(&object.to_update()); + if updated { + updated_object + } else { + return Ok(false); + } + } else { + object + }; - Ok(updated) + // Persist first, so a failed write leaves the in-memory state aligned with storage. + self.persist(&updated_object)?; + locked_objects.insert(id, updated_object); + Ok(true) } pub(crate) fn remove(&self, id: &SO::Id) -> Result<(), Error> { @@ -171,7 +171,12 @@ where #[cfg(test)] mod tests { + use std::future::Future; + use std::pin::Pin; + use lightning::impl_writeable_tlv_based; + use lightning::io; + use lightning::util::persist::KVStore; use lightning::util::test_utils::TestLogger; use super::*; @@ -296,4 +301,86 @@ mod tests { new_iou_object.data[0] += 1; assert_eq!(Ok(true), data_store.insert_or_update(new_iou_object)); } + + struct FailingStore; + + impl KVStore for FailingStore { + fn read( + &self, _primary_namespace: &str, _secondary_namespace: &str, _key: &str, + ) -> Pin, io::Error>> + 'static + Send>> { + Box::pin(async { Err(io::Error::new(io::ErrorKind::Other, "read failed")) }) + } + + fn write( + &self, _primary_namespace: &str, _secondary_namespace: &str, _key: &str, _buf: Vec, + ) -> Pin> + 'static + Send>> { + Box::pin(async { Err(io::Error::new(io::ErrorKind::Other, "write failed")) }) + } + + fn remove( + &self, _primary_namespace: &str, _secondary_namespace: &str, _key: &str, _lazy: bool, + ) -> Pin> + 'static + Send>> { + Box::pin(async { Err(io::Error::new(io::ErrorKind::Other, "remove failed")) }) + } + + fn list( + &self, _primary_namespace: &str, _secondary_namespace: &str, + ) -> Pin, io::Error>> + 'static + Send>> { + Box::pin(async { Err(io::Error::new(io::ErrorKind::Other, "list failed")) }) + } + } + + impl KVStoreSync for FailingStore { + fn read( + &self, _primary_namespace: &str, _secondary_namespace: &str, _key: &str, + ) -> io::Result> { + Err(io::Error::new(io::ErrorKind::Other, "read failed")) + } + + fn write( + &self, _primary_namespace: &str, _secondary_namespace: &str, _key: &str, _buf: Vec, + ) -> io::Result<()> { + Err(io::Error::new(io::ErrorKind::Other, "write failed")) + } + + fn remove( + &self, _primary_namespace: &str, _secondary_namespace: &str, _key: &str, _lazy: bool, + ) -> io::Result<()> { + Err(io::Error::new(io::ErrorKind::Other, "remove failed")) + } + + fn list( + &self, _primary_namespace: &str, _secondary_namespace: &str, + ) -> io::Result> { + Err(io::Error::new(io::ErrorKind::Other, "list failed")) + } + } + + fn new_failing_data_store(objects: Vec) -> DataStore> { + let store: Arc = Arc::new(FailingStore); + let logger = Arc::new(TestLogger::new()); + DataStore::new( + objects, + "datastore_test_primary".to_string(), + "datastore_test_secondary".to_string(), + store, + logger, + ) + } + + #[test] + fn insert_or_update_does_not_mutate_memory_if_persist_fails() { + let existing_id = TestObjectId { id: [42u8; 4] }; + let existing_object = TestObject { id: existing_id, data: [23u8; 3] }; + let data_store = new_failing_data_store(vec![existing_object]); + + let updated_object = TestObject { id: existing_id, data: [24u8; 3] }; + assert_eq!(Err(Error::PersistenceFailed), data_store.insert_or_update(updated_object)); + assert_eq!(Some(existing_object), data_store.get(&existing_id)); + + let new_id = TestObjectId { id: [55u8; 4] }; + let new_object = TestObject { id: new_id, data: [34u8; 3] }; + assert_eq!(Err(Error::PersistenceFailed), data_store.insert_or_update(new_object)); + assert!(data_store.get(&new_id).is_none()); + } } From 5af74e4a47c8953b8db898f3b86fc03e87e75e28 Mon Sep 17 00:00:00 2001 From: benthecarman Date: Mon, 15 Jun 2026 15:27:27 -0500 Subject: [PATCH 09/29] Keep update cache unchanged on persist failure Apply updates to a cloned object and only replace the cached entry after the backing store write succeeds. This finding was discovered by Project Loupe AI-Assisted-By: OpenAI Codex Backport of e3793b858c8cb482a0e6db32176010d8349e8a49 Conflicts Resolved in src/data_store.rs --- src/data_store.rs | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/data_store.rs b/src/data_store.rs index 89b899c81e..e7b506c19b 100644 --- a/src/data_store.rs +++ b/src/data_store.rs @@ -127,17 +127,19 @@ where pub(crate) fn update(&self, update: &SO::Update) -> Result { let mut locked_objects = self.objects.lock().unwrap(); - if let Some(object) = locked_objects.get_mut(&update.id()) { - let updated = object.update(update); - if updated { - self.persist(&object)?; - Ok(DataStoreUpdateResult::Updated) - } else { - Ok(DataStoreUpdateResult::Unchanged) - } - } else { - Ok(DataStoreUpdateResult::NotFound) + let id = update.id(); + let Some(object) = locked_objects.get(&id) else { + return Ok(DataStoreUpdateResult::NotFound); + }; + let mut updated_object = object.clone(); + if !updated_object.update(update) { + return Ok(DataStoreUpdateResult::Unchanged); } + + // Persist first, so a failed write leaves the in-memory state aligned with storage. + self.persist(&updated_object)?; + locked_objects.insert(id, updated_object); + Ok(DataStoreUpdateResult::Updated) } pub(crate) fn list_filter bool>(&self, f: F) -> Vec { @@ -383,4 +385,15 @@ mod tests { assert_eq!(Err(Error::PersistenceFailed), data_store.insert_or_update(new_object)); assert!(data_store.get(&new_id).is_none()); } + + #[test] + fn update_does_not_mutate_memory_if_persist_fails() { + let id = TestObjectId { id: [42u8; 4] }; + let object = TestObject { id, data: [23u8; 3] }; + let data_store = new_failing_data_store(vec![object]); + + let update = TestObjectUpdate { id, data: [24u8; 3] }; + assert_eq!(Err(Error::PersistenceFailed), data_store.update(&update)); + assert_eq!(Some(object), data_store.get(&id)); + } } From 3bcef49c047b1092c38897d2d7de83e4e2fa3b6c Mon Sep 17 00:00:00 2001 From: benthecarman Date: Mon, 15 Jun 2026 15:28:11 -0500 Subject: [PATCH 10/29] Keep removed objects cached on persist failure Check for the object first, remove it from the backing store, and only then delete it from the in-memory map. This finding was discovered by Project Loupe AI-Assisted-By: OpenAI Codex Backport of 531d52d670b54f1762aaf4efac9f66bd3eb70729 Conflicts Resolved in src/data_store.rs --- src/data_store.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/data_store.rs b/src/data_store.rs index e7b506c19b..796623b601 100644 --- a/src/data_store.rs +++ b/src/data_store.rs @@ -95,8 +95,8 @@ where } pub(crate) fn remove(&self, id: &SO::Id) -> Result<(), Error> { - let removed = self.objects.lock().unwrap().remove(id).is_some(); - if removed { + let mut locked_objects = self.objects.lock().unwrap(); + if locked_objects.contains_key(id) { let store_key = id.encode_to_hex_str(); KVStoreSync::remove( &*self.kv_store, @@ -116,6 +116,8 @@ where ); Error::PersistenceFailed })?; + // Only drop the object from memory once the backing store removal succeeded. + locked_objects.remove(id); } Ok(()) } @@ -396,4 +398,14 @@ mod tests { assert_eq!(Err(Error::PersistenceFailed), data_store.update(&update)); assert_eq!(Some(object), data_store.get(&id)); } + + #[test] + fn remove_does_not_mutate_memory_if_persist_fails() { + let id = TestObjectId { id: [42u8; 4] }; + let object = TestObject { id, data: [23u8; 3] }; + let data_store = new_failing_data_store(vec![object]); + + assert_eq!(Err(Error::PersistenceFailed), data_store.remove(&id)); + assert_eq!(Some(object), data_store.get(&id)); + } } From 36c7208324d0b0b66e5053a1b510d4add913a421 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Sat, 27 Jun 2026 19:43:51 +0000 Subject: [PATCH 11/29] Fix anchor reserves when splicing in all funds We stop requiring that splice-ins leave an anchor reserve for an additional anchor channel on top of the existing set of anchor channels; after splice-ins, our anchor reserve only needs to cover the existing set of anchor channels. Backport of 4c8ecb28303f07ca7aebd42d1e9195dc074feedd Conflicts Resolved in src/lib.rs --- src/lib.rs | 66 +++++++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 490c5b9afc..633fc23651 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1084,7 +1084,7 @@ impl Node { })?; // Check funds availability after connection (includes anchor reserve calculation) - self.check_sufficient_funds_for_channel(channel_amount_sats, &node_id)?; + self.check_sufficient_onchain_funds(channel_amount_sats, &node_id, true)?; let mut user_config = default_user_config(&self.config); user_config.channel_handshake_config.announce_for_forwarding = announce_for_forwarding; @@ -1125,46 +1125,52 @@ impl Node { } } - fn check_sufficient_funds_for_channel( - &self, amount_sats: u64, peer_node_id: &PublicKey, + fn check_sufficient_onchain_funds( + &self, amount_sats: u64, peer_node_id: &PublicKey, for_new_channel: bool, ) -> Result<(), Error> { + let action_str = if for_new_channel { "create channel" } else { "splice-in" }; let cur_anchor_reserve_sats = total_anchor_channels_reserve_sats(&self.channel_manager, &self.config); let spendable_amount_sats = self.wallet.get_spendable_amount_sats(cur_anchor_reserve_sats).unwrap_or(0); - // Fail early if we have less than the channel value available. if spendable_amount_sats < amount_sats { - log_error!(self.logger, - "Unable to create channel due to insufficient funds. Available: {}sats, Required: {}sats", - spendable_amount_sats, amount_sats + log_error!( + self.logger, + "Unable to {} due to insufficient funds. Available: {}sats, Required: {}sats", + action_str, + spendable_amount_sats, + amount_sats ); return Err(Error::InsufficientFunds); } - // Fail if we have less than the channel value + anchor reserve available (if applicable). - let init_features = self - .peer_manager - .peer_by_node_id(peer_node_id) - .ok_or(Error::ConnectionFailed)? - .init_features; - let required_funds_sats = amount_sats - + self.config.anchor_channels_config.as_ref().map_or(0, |c| { - if init_features.requires_anchors_zero_fee_htlc_tx() - && !c.trusted_peers_no_reserve.contains(peer_node_id) - { - c.per_channel_reserve_sats - } else { - 0 - } - }); + if for_new_channel { + // Fail if we have less than the channel value + anchor reserve available (if + // applicable). + let init_features = self + .peer_manager + .peer_by_node_id(peer_node_id) + .ok_or(Error::ConnectionFailed)? + .init_features; + let required_funds_sats = amount_sats + + self.config.anchor_channels_config.as_ref().map_or(0, |c| { + if init_features.requires_anchors_zero_fee_htlc_tx() + && !c.trusted_peers_no_reserve.contains(peer_node_id) + { + c.per_channel_reserve_sats + } else { + 0 + } + }); - if spendable_amount_sats < required_funds_sats { - log_error!(self.logger, - "Unable to create channel due to insufficient funds. Available: {}sats, Required: {}sats", - spendable_amount_sats, required_funds_sats - ); - return Err(Error::InsufficientFunds); + if spendable_amount_sats < required_funds_sats { + log_error!(self.logger, + "Unable to create channel due to insufficient funds. Available: {}sats, Required: {}sats", + spendable_amount_sats, required_funds_sats + ); + return Err(Error::InsufficientFunds); + } } Ok(()) @@ -1260,7 +1266,7 @@ impl Node { if let Some(channel_details) = open_channels.iter().find(|c| c.user_channel_id == user_channel_id.0) { - self.check_sufficient_funds_for_channel(splice_amount_sats, &counterparty_node_id)?; + self.check_sufficient_onchain_funds(splice_amount_sats, &counterparty_node_id, false)?; const EMPTY_SCRIPT_SIG_WEIGHT: u64 = 1 /* empty script_sig */ * bitcoin::constants::WITNESS_SCALE_FACTOR as u64; From 1e48b30ebb663a271b7265042f1b68b5eb4721b5 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Fri, 26 Jun 2026 19:37:40 +0000 Subject: [PATCH 12/29] Reserve onchain funds for anchor channels when peer sets them optional When we are preparing to open a channel to a peer, we should reserve onchain funds for an anchor channel when the peer's init features signals anchor channels as optional, as channel negotiation with such a peer can result in an anchor channel. Tests written with codex. Backport of 2c2c2870fe755fd01641046a6801af51d3bd7154 Conflicts Resolved in src/lib.rs, src/liquidity.rs, tests/integration_tests_rust.rs --- src/lib.rs | 2 +- src/liquidity.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 633fc23651..0fd5720b09 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1155,7 +1155,7 @@ impl Node { .init_features; let required_funds_sats = amount_sats + self.config.anchor_channels_config.as_ref().map_or(0, |c| { - if init_features.requires_anchors_zero_fee_htlc_tx() + if init_features.supports_anchors_zero_fee_htlc_tx() && !c.trusted_peers_no_reserve.contains(peer_node_id) { c.per_channel_reserve_sats diff --git a/src/liquidity.rs b/src/liquidity.rs index 74e6098ddc..77c7e41835 100644 --- a/src/liquidity.rs +++ b/src/liquidity.rs @@ -749,7 +749,7 @@ where self.wallet.get_spendable_amount_sats(cur_anchor_reserve_sats).unwrap_or(0); let required_funds_sats = channel_amount_sats + self.config.anchor_channels_config.as_ref().map_or(0, |c| { - if init_features.requires_anchors_zero_fee_htlc_tx() + if init_features.supports_anchors_zero_fee_htlc_tx() && !c.trusted_peers_no_reserve.contains(&their_network_key) { c.per_channel_reserve_sats @@ -760,7 +760,7 @@ where if spendable_amount_sats < required_funds_sats { log_error!(self.logger, "Unable to create channel due to insufficient funds. Available: {}sats, Required: {}sats", - spendable_amount_sats, channel_amount_sats + spendable_amount_sats, required_funds_sats ); // TODO: We just silently fail here. Eventually we will need to remember // the pending requests and regularly retry opening the channel until we From 2ee491f87c2d1b4cff2ae0f567d32e60162fe5a8 Mon Sep 17 00:00:00 2001 From: JHB <16675200+jharveyb@users.noreply.github.com> Date: Fri, 24 Jul 2026 18:47:42 -0400 Subject: [PATCH 13/29] bitcoind: fix raw TX fetching when using REST Backport of 0757faad29d1885c576aa75841d4dea8ab22ac5e Conflicts Resolved in src/chain/bitcoind.rs --- src/chain/bitcoind.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/chain/bitcoind.rs b/src/chain/bitcoind.rs index b3d7880d61..ed209e31a1 100644 --- a/src/chain/bitcoind.rs +++ b/src/chain/bitcoind.rs @@ -1272,9 +1272,12 @@ pub(crate) struct GetRawTransactionResponse(pub Transaction); impl TryInto for JsonResponse { type Error = std::io::Error; fn try_into(self) -> std::io::Result { + // Accept both the bare hex-encoded TX from RPC, and the REST response + // of a JSON object with the hex-encoded TX in the 'hex' field. let tx = self .0 .as_str() + .or_else(|| self.0.get("hex").and_then(|v| v.as_str())) .ok_or(std::io::Error::new( std::io::ErrorKind::Other, "Failed to parse getrawtransaction response", @@ -1603,6 +1606,25 @@ mod tests { prop_assert_eq!(decoded.0, tx); } + #[test] + fn prop_get_raw_transaction_response_json_object_roundtrip(tx in arbitrary_transaction()) { + let hex = bitcoin::consensus::encode::serialize_hex(&tx); + let vsize = tx.vsize(); + let json_val = json!({ + "version": tx.version, + "vsize": vsize, + "hex": hex, + }); + + let resp = JsonResponse(json_val); + let decoded: GetRawTransactionResponse = resp.try_into().unwrap(); + + prop_assert_eq!(decoded.0.compute_txid(), tx.compute_txid()); + prop_assert_eq!(decoded.0.compute_wtxid(), tx.compute_wtxid()); + + prop_assert_eq!(decoded.0, tx); + } + #[test] fn prop_fee_response_roundtrip(fee_rate in any::()) { let fee_rate = fee_rate.abs(); From 47cc6749f2fc0419e291f08296f7e97d236f774e Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 5 Aug 2026 14:58:00 +0200 Subject: [PATCH 14/29] Follow reorgs with bitcoind REST Bitcoin Core's REST headers endpoint cannot serve headers from a stale branch. This prevented REST-backed nodes from finding a common ancestor and following the replacement chain after a reorg. In #1006 we enabled bitcoind REST chain source support in CI, which exposed this failure. Co-Authored-By: HAL 9000 Backport of 66d48d23e1833c52fe9cbbd1dab0b73b9e0a47c3 Conflicts Resolved in src/chain/bitcoind.rs, tests/reorg_test.rs --- src/chain/bitcoind.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/chain/bitcoind.rs b/src/chain/bitcoind.rs index ed209e31a1..96db488c2a 100644 --- a/src/chain/bitcoind.rs +++ b/src/chain/bitcoind.rs @@ -1194,9 +1194,14 @@ impl BlockSource for BitcoindClient { BitcoindClient::Rpc { rpc_client, .. } => { Box::pin(async move { rpc_client.get_header(header_hash, height_hint).await }) }, - BitcoindClient::Rest { rest_client, .. } => { - Box::pin(async move { rest_client.get_header(header_hash, height_hint).await }) - }, + BitcoindClient::Rest { rest_client, rpc_client, .. } => Box::pin(async move { + match rest_client.get_header(header_hash, height_hint).await { + Err(e) if e.kind() == BlockSourceErrorKind::Persistent => { + rpc_client.get_header(header_hash, height_hint).await + }, + result => result, + } + }), } } From b92fe5fb6108451ed33d0cd19994de49282dfa23 Mon Sep 17 00:00:00 2001 From: elnafateh Date: Sun, 9 Aug 2026 15:18:57 +0100 Subject: [PATCH 15/29] Stop logging payment preimages at INFO level Payment preimages are proof-of-payment and correlation material. The INFO log previously included the full preimage after a successful outbound payment, exposing it to log collectors and anyone with log access. Remove the preimage from the 'Successfully sent payment' log message in src/event.rs, retaining only the payment hash for diagnostics. The preimage remains accessible via the PaymentSuccessful event and payment store APIs. Backport of 8828e52b8f066a512d62f5451efef550f644eb79 Conflicts Resolved in src/event.rs --- src/event.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/event.rs b/src/event.rs index ae94249e60..92b65b5d07 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1062,8 +1062,7 @@ where self.payment_store.get(&payment_id).map(|payment| { log_info!( self.logger, - "Successfully sent payment of {}msat{} from \ - payment hash {:?} with preimage {:?}", + "Successfully sent payment of {}msat{} with payment hash {}", payment.amount_msat.unwrap(), if let Some(fee) = fee_paid_msat { format!(" (fee {} msat)", fee) @@ -1071,7 +1070,6 @@ where "".to_string() }, hex_utils::to_string(&payment_hash.0), - hex_utils::to_string(&payment_preimage.0) ); }); let event = Event::PaymentSuccessful { From 2ad1237159b7acfd90dc0bc2f4d0c0192bb1626f Mon Sep 17 00:00:00 2001 From: jolah1 Date: Thu, 6 Aug 2026 21:48:53 +0100 Subject: [PATCH 16/29] Retain Electrum `Filter` registrations across `stop`/`start` `ElectrumRuntimeStatus::stop` reset itself to `Stopped` with empty pending registration vectors, dropping the `ElectrumRuntimeClient` and, with it, the `ElectrumSyncClient` that owns the registered transactions and outputs. `start` then drained the pending vectors, so nothing survived even a single cycle. As `ChannelMonitor`s only register their watched transactions and outputs while being loaded in `Builder::build`, a `stop`/`start` cycle that doesn't rebuild the node left the chain source with no registrations at all, i.e., the node would no longer learn about confirmations or spends of, e.g., its funding outputs. Note we can't simply re-register from `ChannelMonitor::load_outputs_to_watch` on `start` either, as, e.g., the `OutputSweeper` also registers outputs it wants to see spent. Here we therefore keep a canonical, deduplicated registration inventory in `ElectrumRuntimeStatus` that is maintained whether we're started or not: `register_tx`/`register_output` always record the entry and additionally forward it to the client if one is live, `start` replays the inventory to the fresh client without consuming it, and `stop` merely drops the client. This also restores parity with the Esplora chain source, whose `tx_sync` is long-lived and hence never loses its registrations. Co-Authored-By: Claude Opus 5 Backport of af634d32a5e673fb6639102029c4a743a2111d6e Conflicts Resolved in src/chain/electrum.rs --- src/chain/electrum.rs | 95 ++++++++++++++++----------------- tests/integration_tests_rust.rs | 54 +++++++++++++++++++ 2 files changed, 101 insertions(+), 48 deletions(-) diff --git a/src/chain/electrum.rs b/src/chain/electrum.rs index 9e05dfaeed..8094f14eb9 100644 --- a/src/chain/electrum.rs +++ b/src/chain/electrum.rs @@ -5,7 +5,7 @@ // http://opensource.org/licenses/MIT>, at your option. You may not use this file except in // accordance with one or both of these licenses. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::{Arc, Mutex, RwLock}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; @@ -318,78 +318,77 @@ impl Filter for ElectrumChainSource { } } -enum ElectrumRuntimeStatus { - Started(Arc), - Stopped { - pending_registered_txs: Vec<(Txid, ScriptBuf)>, - pending_registered_outputs: Vec, - }, +struct ElectrumRuntimeStatus { + client: Option>, + // The canonical inventory of all `Filter` entries registered over this chain source's + // lifetime. + // + // We retain these even while started: the `ElectrumRuntimeClient` (and hence the + // `ElectrumSyncClient` owning its own copy of the registrations) is dropped on `stop`, so + // replaying the inventory on the next `start` is the only way to keep watching the same set + // of transactions and outputs across a `stop`/`start` cycle. Note that we can't simply + // re-register from `ChannelMonitor::load_outputs_to_watch` instead, as, e.g., the + // `OutputSweeper` also registers outputs it wants to see spent. + registered_txs: HashMap, + registered_outputs: HashSet, } impl ElectrumRuntimeStatus { fn new() -> Self { - let pending_registered_txs = Vec::new(); - let pending_registered_outputs = Vec::new(); - Self::Stopped { pending_registered_txs, pending_registered_outputs } + let client = None; + let registered_txs = HashMap::new(); + let registered_outputs = HashSet::new(); + Self { client, registered_txs, registered_outputs } } pub(super) fn start( &mut self, server_url: String, runtime: Arc, config: Arc, logger: Arc, ) -> Result<(), Error> { - match self { - Self::Stopped { pending_registered_txs, pending_registered_outputs } => { - let client = Arc::new(ElectrumRuntimeClient::new( - server_url.clone(), - runtime, - config, - logger, - )?); - - // Apply any pending `Filter` entries - for (txid, script_pubkey) in pending_registered_txs.drain(..) { - client.register_tx(&txid, &script_pubkey); - } - - for output in pending_registered_outputs.drain(..) { - client.register_output(output) - } - - *self = Self::Started(client); - }, - Self::Started(_) => { - debug_assert!(false, "We shouldn't call start if we're already started") - }, + if self.client.is_some() { + debug_assert!(false, "We shouldn't call start if we're already started"); + return Ok(()); + } + + let client = Arc::new(ElectrumRuntimeClient::new(server_url, runtime, config, logger)?); + + // (Re-)apply all known `Filter` entries to the fresh client. + for (txid, script_pubkey) in self.registered_txs.iter() { + client.register_tx(txid, script_pubkey); + } + + for output in self.registered_outputs.iter() { + client.register_output(output.clone()); } + + self.client = Some(client); + Ok(()) } pub(super) fn stop(&mut self) { - *self = Self::new() + // Drop the client, but retain the registration inventory so we can replay it if we're + // started again. + self.client = None; } fn client(&self) -> Option> { - match self { - Self::Started(client) => Some(Arc::clone(&client)), - Self::Stopped { .. } => None, - } + self.client.as_ref().map(Arc::clone) } fn register_tx(&mut self, txid: &Txid, script_pubkey: &Script) { - match self { - Self::Started(client) => client.register_tx(txid, script_pubkey), - Self::Stopped { pending_registered_txs, .. } => { - pending_registered_txs.push((*txid, script_pubkey.to_owned())) - }, + self.registered_txs.insert(*txid, script_pubkey.to_owned()); + + if let Some(client) = self.client.as_ref() { + client.register_tx(txid, script_pubkey); } } fn register_output(&mut self, output: lightning::chain::WatchedOutput) { - match self { - Self::Started(client) => client.register_output(output), - Self::Stopped { pending_registered_outputs, .. } => { - pending_registered_outputs.push(output) - }, + self.registered_outputs.insert(output.clone()); + + if let Some(client) = self.client.as_ref() { + client.register_output(output); } } } diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index d6022c8f96..ab526c66a1 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -315,6 +315,60 @@ async fn start_stop_reinit() { reinitialized_node.stop().unwrap(); } +// The Electrum chain source drops its runtime client - and with it the tx-sync client holding all +// `Filter` registrations - when stopped. As `ChannelMonitor`s only register their watched +// transactions and outputs while being loaded in `Builder::build`, nothing would re-register them +// on the next `start`, leaving the node blind to confirmations and spends of, e.g., its funding +// outputs. So here we assert the chain source replays its registrations across a `stop`/`start` +// cycle. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn electrum_registrations_survive_chain_source_restart() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = TestChainSource::Electrum(&electrsd); + let (node_a, node_b) = setup_two_nodes(&chain_source, false, false, false); + + let address_a = node_a.onchain_payment().new_address().unwrap(); + + let premine_amount_sat = 5_000_000; + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![address_a], + Amount::from_sat(premine_amount_sat), + ) + .await; + node_a.sync_wallets().unwrap(); + + // Opening the channel registers the funding transaction and output with the chain source's + // `Filter`. We leave it unconfirmed for now, so watching for it is still pending. + open_channel(&node_a, &node_b, 4_000_000, false, &electrsd).await; + + // Restart node A repeatedly, which tears down and recreates its Electrum chain source every time. + // Note that the `ChannelMonitor`s are not reloaded here, so the registrations have to survive in + // the chain source itself, and they have to survive more than a single cycle, i.e., replaying + // them mustn't consume them. + for _ in 0..3 { + node_a.stop().unwrap(); + node_a.start().unwrap(); + } + + // Reconnect eagerly rather than waiting on the background reconnection interval. + let node_addr_b = node_b.listening_addresses().unwrap().first().unwrap().clone(); + node_a.connect(node_b.node_id(), node_addr_b, false).unwrap(); + + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + // Node A can only learn that the funding transaction confirmed if its registrations survived + // the restart. + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + node_a.stop().unwrap(); + node_b.stop().unwrap(); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn onchain_send_receive() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); From b5fa4c2ec011887e11e2a91dc66b9fc1a2427ead Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Fri, 7 Aug 2026 11:14:07 +0200 Subject: [PATCH 17/29] Fail closed on manager read errors Only a missing ChannelManager should create a fresh node. Propagate other storage errors so transient failures cannot replace live channel state with an empty manager. Fixes #1026 Co-Authored-By: HAL 9000 Backport of 03dafa9c30f16e2dcc39f3eba0678347a2af136f Conflicts Resolved in src/builder.rs --- src/builder.rs | 107 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 105 insertions(+), 2 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 63e84db378..af6eeb5308 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1494,12 +1494,21 @@ fn build_with_store_internal( // Initialize the ChannelManager let channel_manager = { - if let Ok(res) = KVStoreSync::read( + let channel_manager_bytes = match KVStoreSync::read( &*kv_store, CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE, CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE, CHANNEL_MANAGER_PERSISTENCE_KEY, ) { + Ok(res) => Some(res), + Err(e) if e.kind() == lightning::io::ErrorKind::NotFound => None, + Err(e) => { + log_error!(logger, "Failed to read channel manager from store: {}", e); + return Err(BuildError::ReadFailed); + }, + }; + + if let Some(res) = channel_manager_bytes { let mut reader = Cursor::new(res); let channel_monitor_references = channel_monitors.iter().map(|(_, chanmon)| chanmon).collect(); @@ -1926,7 +1935,101 @@ pub(crate) fn sanitize_alias(alias_str: &str) -> Result { #[cfg(test)] mod tests { - use super::{sanitize_alias, BuildError, NodeAlias}; + use std::future::Future; + use std::pin::Pin; + use std::sync::Arc; + + use lightning::io; + use lightning::util::persist::{ + KVStore, KVStoreSync, CHANNEL_MANAGER_PERSISTENCE_KEY, + CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE, + CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE, + }; + + use super::{sanitize_alias, BuildError, NodeAlias, NodeBuilder}; + use crate::io::test_utils::InMemoryStore; + use crate::types::DynStore; + + struct ChannelManagerReadFailingStore(InMemoryStore); + + impl ChannelManagerReadFailingStore { + fn fails_read(primary_namespace: &str, secondary_namespace: &str, key: &str) -> bool { + primary_namespace == CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE + && secondary_namespace == CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE + && key == CHANNEL_MANAGER_PERSISTENCE_KEY + } + } + + impl KVStoreSync for ChannelManagerReadFailingStore { + fn read( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, + ) -> io::Result> { + if Self::fails_read(primary_namespace, secondary_namespace, key) { + return Err(io::Error::new(io::ErrorKind::Other, "channel manager read failed")); + } + KVStoreSync::read(&self.0, primary_namespace, secondary_namespace, key) + } + + fn write( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: Vec, + ) -> io::Result<()> { + KVStoreSync::write(&self.0, primary_namespace, secondary_namespace, key, buf) + } + + fn remove( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool, + ) -> io::Result<()> { + KVStoreSync::remove(&self.0, primary_namespace, secondary_namespace, key, lazy) + } + + fn list( + &self, primary_namespace: &str, secondary_namespace: &str, + ) -> io::Result> { + KVStoreSync::list(&self.0, primary_namespace, secondary_namespace) + } + } + + impl KVStore for ChannelManagerReadFailingStore { + fn read( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, + ) -> Pin, io::Error>> + 'static + Send>> { + let res = KVStoreSync::read(self, primary_namespace, secondary_namespace, key); + Box::pin(async move { res }) + } + + fn write( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: Vec, + ) -> Pin> + 'static + Send>> { + let res = KVStoreSync::write(self, primary_namespace, secondary_namespace, key, buf); + Box::pin(async move { res }) + } + + fn remove( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool, + ) -> Pin> + 'static + Send>> { + let res = KVStoreSync::remove(self, primary_namespace, secondary_namespace, key, lazy); + Box::pin(async move { res }) + } + + fn list( + &self, primary_namespace: &str, secondary_namespace: &str, + ) -> Pin, io::Error>> + 'static + Send>> { + let res = KVStoreSync::list(self, primary_namespace, secondary_namespace); + Box::pin(async move { res }) + } + } + + #[test] + fn channel_manager_read_failure_fails_build() { + let mut builder = NodeBuilder::new(); + builder.set_entropy_seed_bytes([42; 64]); + builder.set_log_facade_logger(); + let store: Arc = Arc::new(ChannelManagerReadFailingStore(InMemoryStore::new())); + + let result = builder.build_with_store(store); + + assert!(matches!(result, Err(BuildError::ReadFailed))); + } #[test] fn sanitize_empty_node_alias() { From 82aa471d6960b11447ac9dd9e1516c7cd9c50372 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Tue, 25 Aug 2026 05:26:38 +0000 Subject: [PATCH 18/29] Collect the raw mempool txids into a HashSet Mempool reconciliation checks every cached transaction against the raw mempool response. Using a Vec to store the raw mempool txids makes each membership check linear and the retention pass quadratic. Store the txids in a HashSet to make membership checks expected constant time and reconciliation expected linear. Co-Authored-By: HAL 9000 Backport of 8c632e56ac9b71cd5e916de4d4807239027eefa4 Conflicts Resolved in src/chain/bitcoind.rs --- src/chain/bitcoind.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/chain/bitcoind.rs b/src/chain/bitcoind.rs index 96db488c2a..81040503f7 100644 --- a/src/chain/bitcoind.rs +++ b/src/chain/bitcoind.rs @@ -5,7 +5,7 @@ // http://opensource.org/licenses/MIT>, at your option. You may not use this file except in // accordance with one or both of these licenses. -use std::collections::{HashMap, VecDeque}; +use std::collections::{HashMap, HashSet, VecDeque}; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, Mutex, RwLock}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; @@ -910,7 +910,7 @@ impl BitcoindClient { } /// Retrieves the raw mempool. - pub(crate) async fn get_raw_mempool(&self) -> std::io::Result> { + pub(crate) async fn get_raw_mempool(&self) -> std::io::Result> { match self { BitcoindClient::Rpc { rpc_client, .. } => { Self::get_raw_mempool_rpc(Arc::clone(rpc_client)).await @@ -922,7 +922,7 @@ impl BitcoindClient { } /// Retrieves the raw mempool via the RPC interface. - async fn get_raw_mempool_rpc(rpc_client: Arc) -> std::io::Result> { + async fn get_raw_mempool_rpc(rpc_client: Arc) -> std::io::Result> { let verbose_flag_json = serde_json::json!(false); rpc_client .call_method::("getrawmempool", &[verbose_flag_json]) @@ -931,7 +931,7 @@ impl BitcoindClient { } /// Retrieves the raw mempool via the REST interface. - async fn get_raw_mempool_rest(rest_client: Arc) -> std::io::Result> { + async fn get_raw_mempool_rest(rest_client: Arc) -> std::io::Result> { rest_client .request_resource::( "mempool/contents.json?verbose=false", @@ -1300,7 +1300,7 @@ impl TryInto for JsonResponse { } } -pub struct GetRawMempoolResponse(Vec); +pub struct GetRawMempoolResponse(HashSet); impl TryInto for JsonResponse { type Error = std::io::Error; @@ -1310,7 +1310,7 @@ impl TryInto for JsonResponse { "Failed to parse getrawmempool response", ))?; - let mut mempool_transactions = Vec::with_capacity(res.len()); + let mut mempool_transactions = HashSet::with_capacity(res.len()); for hex in res { let txid = if let Some(hex_str) = hex.as_str() { @@ -1330,7 +1330,7 @@ impl TryInto for JsonResponse { )); }; - mempool_transactions.push(txid); + mempool_transactions.insert(txid); } Ok(GetRawMempoolResponse(mempool_transactions)) @@ -1488,6 +1488,8 @@ impl std::fmt::Display for HttpError { #[cfg(test)] mod tests { + use std::collections::HashSet; + use bitcoin::hashes::Hash; use bitcoin::{FeeRate, OutPoint, ScriptBuf, Transaction, TxIn, TxOut, Txid, Witness}; use lightning_block_sync::http::JsonResponse; @@ -1566,10 +1568,10 @@ mod tests { #[test] fn prop_get_raw_mempool_response_roundtrip(txids in vec(any::<[u8;32]>(), 0..10)) { - let txid_vec: Vec = txids.into_iter().map(Txid::from_byte_array).collect(); - let original = GetRawMempoolResponse(txid_vec.clone()); + let txid_set: HashSet = txids.into_iter().map(Txid::from_byte_array).collect(); + let original = GetRawMempoolResponse(txid_set.clone()); - let json_vec: Vec = txid_vec.iter().map(|t| t.to_string()).collect(); + let json_vec: Vec = txid_set.iter().map(|t| t.to_string()).collect(); let json_val = serde_json::Value::Array(json_vec.iter().map(|s| json!(s)).collect()); let resp = JsonResponse(json_val); From 6ba5f1943457e87211e2273ed8d8830e7a0dcf51 Mon Sep 17 00:00:00 2001 From: elnafateh Date: Tue, 25 Aug 2026 11:08:44 +0000 Subject: [PATCH 19/29] Fix BOLT11 DuplicatePayment triggering on-chain fallback in unified payments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UnifiedPayment::send previously treated any error from the BOLT11 leg of a unified payment as non-terminal and fell through to the on-chain payment method. This meant a retried BOLT11 payment that returns Error::DuplicatePayment would still result in an on-chain transaction being broadcast for the same invoice — a duplicate payment. Error::DuplicatePayment is now terminal in UnifiedPayment::send: the unified payment aborts instead of falling back to on-chain. Fixes #1033. unified_send_receive_bip21_uri already funds two nodes, opens a channel, and sends a successful BOLT11 payment via uri_str_without_offer partway through. Add the regression assertion right there — retry the same uri_str_without_offer and assert DuplicatePayment, not a new on-chain payment — instead of duplicating that setup in a standalone test. Backport of 9c271d7f8d03dd605f5a0ce4c25074a7e88b4c0d Conflicts Resolved in src/payment/unified_qr.rs, tests/integration_tests_rust.rs --- src/payment/unified_qr.rs | 6 ++++++ tests/integration_tests_rust.rs | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/src/payment/unified_qr.rs b/src/payment/unified_qr.rs index 84f784e46a..b310caa93e 100644 --- a/src/payment/unified_qr.rs +++ b/src/payment/unified_qr.rs @@ -165,6 +165,12 @@ impl UnifiedQrPayment { let invoice = maybe_wrap(invoice); match self.bolt11_invoice.send(&invoice, route_parameters) { Ok(payment_id) => return Ok(QrPaymentResult::Bolt11 { payment_id }), + // A duplicate payment already exists, so falling back to the on-chain method + // would pay the same invoice a second time. + Err(Error::DuplicatePayment) => { + log_error!(self.logger, "Failed to send BOLT11 invoice: DuplicatePayment. This is part of a unified QR code payment. Aborting to avoid duplicate payment."); + return Err(Error::DuplicatePayment); + }, Err(e) => { log_error!(self.logger, "Failed to send BOLT11 invoice: {:?}. This is part of a unified QR code payment. Falling back to the on-chain transaction.", e); }, diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index ab526c66a1..4803a81964 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -1725,6 +1725,15 @@ async fn unified_qr_send_receive() { }; expect_payment_successful_event!(node_a, Some(invoice_payment_id), None); + // Retrying the same BOLT11 invoice must return DuplicatePayment, not fall back to on-chain. + match node_a.unified_qr_payment().send(uri_str_without_offer, None) { + Err(NodeError::DuplicatePayment) => {}, + Ok(QrPaymentResult::Onchain { txid }) => { + panic!("Regression: duplicate BOLT11 payment fell back to on-chain. txid={}", txid); + }, + other => panic!("Expected DuplicatePayment error on retry, got: {:?}", other), + } + let expect_onchain_amount_sats = 800_000; let onchain_uqr_payment = node_b.unified_qr_payment().receive(expect_onchain_amount_sats, "asdf", 4_000).unwrap(); From 75b089ff95cdad83b20f42cc8823880a63589efe Mon Sep 17 00:00:00 2001 From: elnafateh Date: Tue, 25 Aug 2026 11:10:28 +0000 Subject: [PATCH 20/29] Fix unified payment falling back to on-chain after PersistenceFailed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In UnifiedPayment::send, the BOLT11 leg's bolt11_invoice.send only returns Err(PersistenceFailed) *after* pay_for_bolt11_invoice has already succeeded and the Lightning payment is in-flight. The previous match treated every remaining error as a fall-through to the next payment method, so a persistence failure after initiation would broadcast an on-chain transaction for the same URI — a duplicate payment. Err(Error::PersistenceFailed) on the BOLT11 leg is now terminal, mirroring how DuplicatePayment is handled, and aborts the unified payment instead of falling back to on-chain. This is a regression hazard raised during review of the DuplicatePayment fix (PR #1038). It is pre-existing and orthogonal to #1033; tracked here as the unified variant of the broader post-commit persistence hazard. Per review discussion: rather than a dedicated node/channel fixture, build unified_send_receive_bip21_uri's node_a on a PaymentFailingStore (inert until armed) from the start, and add a PersistenceFailed assertion at the end of that test using a fresh BOLT11-only URI. This reuses the funding/channel/announcement setup and the successful-send flow the test already has, rather than duplicating it. Adds PaymentFailingStore (a KVStore wrapper that fails writes to the payments namespace on demand) and setup_two_nodes_with_failing_store_a (mirrors setup_two_nodes, but node_a is built on PaymentFailingStore). Backport of d7ee094c4873403adbfe2c3f7ee4a0abd7504ceb Conflicts Resolved in src/payment/unified_qr.rs, tests/common/mod.rs, tests/integration_tests_rust.rs --- src/payment/unified_qr.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/payment/unified_qr.rs b/src/payment/unified_qr.rs index b310caa93e..6a4311ea66 100644 --- a/src/payment/unified_qr.rs +++ b/src/payment/unified_qr.rs @@ -171,6 +171,14 @@ impl UnifiedQrPayment { log_error!(self.logger, "Failed to send BOLT11 invoice: DuplicatePayment. This is part of a unified QR code payment. Aborting to avoid duplicate payment."); return Err(Error::DuplicatePayment); }, + // A persistence failure may occur after the Lightning payment has already been + // initiated with the ChannelManager. Falling back to the on-chain method in that + // case would double-pay, so we abort instead of proceeding to the next payment + // method. + Err(Error::PersistenceFailed) => { + log_error!(self.logger, "Failed to send BOLT11 invoice: PersistenceFailed. This is part of a unified QR code payment. Aborting to avoid a potential duplicate payment."); + return Err(Error::PersistenceFailed); + }, Err(e) => { log_error!(self.logger, "Failed to send BOLT11 invoice: {:?}. This is part of a unified QR code payment. Falling back to the on-chain transaction.", e); }, From 833a715c7e9d1c02ab11bc459f52a1641881a59f Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 11 Sep 2026 11:27:26 -0500 Subject: [PATCH 21/29] Keep reserved change addresses marked during fee estimates Estimating the maximum funding amount, the maximum splice-in amount, or a send-all payment builds a temporary drain transaction whose anchor reserve output pays to the wallet's first internal address, then cancels it as though it were a real transaction. A drain transaction never reserves a change address, so the cancel has nothing of its own to undo and instead clears the usage mark on that internal address. If a pending channel funding or splice contribution had reserved the same address for its change, the wallet could hand it out again until the pending transaction is synced, causing address reuse. Fee estimates now leave the wallet untouched, since BDK neither marks an address nor locks an input for a transaction that drains to a fixed script. Co-Authored-By: Claude Backport of 7880030f99ae9a1ebbe81d1360bb35b450d11d33 Conflicts Resolved in src/wallet/mod.rs --- src/wallet/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 2f8daa5001..95d9c85016 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -420,8 +420,11 @@ impl Wallet { e })?; - // 'cancel' the transaction to free up any used change addresses - locked_wallet.cancel_tx(&tmp_tx); + // Note we intentionally don't `cancel_tx` the temporary transaction: draining + // to a fixed script means BDK neither reserves a change address nor locks + // inputs for it. Cancelling it would instead clear the usage mark on the + // anchor reserve placeholder address, which a pending transaction may have + // reserved as its change address, leading to address reuse. let estimated_spendable_amount = Amount::from_sat( spendable_amount_sats.saturating_sub(estimated_tx_fee.to_sat()), From 78e875e51dc965f1b1f726e2b9d16d51da2d5fe8 Mon Sep 17 00:00:00 2001 From: Bartok9 <259807879+Bartok9@users.noreply.github.com> Date: Sun, 13 Sep 2026 09:13:44 -0400 Subject: [PATCH 22/29] fix: BOLT12 arm of UnifiedPayment::send returns DuplicatePayment/PersistenceFailed errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The BOLT12 arm used .map_err() + if let Ok, so DuplicatePayment and PersistenceFailed fell through to BOLT11/onchain — risking double payment. Now treats both as terminal, mirroring the BOLT11 fix in #1038. Fixes: #1060 AI-assisted: generated by Sera (Hermes Agent), verified manually. Signed-off-by: Bartok9 <259807879+Bartok9@users.noreply.github.com> Backport of 3c98b1c2dada8e12ec6a1b4754f467e697fbd2d8 Conflicts Resolved in src/payment/unified_qr.rs --- src/payment/unified_qr.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/payment/unified_qr.rs b/src/payment/unified_qr.rs index 6a4311ea66..f139f450e4 100644 --- a/src/payment/unified_qr.rs +++ b/src/payment/unified_qr.rs @@ -155,6 +155,20 @@ impl UnifiedQrPayment { let offer = maybe_wrap(offer); match self.bolt12_payment.send(&offer, None, None, route_parameters) { Ok(payment_id) => return Ok(QrPaymentResult::Bolt12 { payment_id }), + // A duplicate payment already exists, so falling back to the BOLT11 invoice + // would pay the same offer a second time. + Err(Error::DuplicatePayment) => { + log_error!(self.logger, "Failed to send BOLT12 offer: DuplicatePayment. This is part of a unified QR code payment. Aborting to avoid duplicate payment."); + return Err(Error::DuplicatePayment); + }, + // A persistence failure may occur after the Lightning payment has already been + // initiated with the ChannelManager. Falling back to the BOLT11 invoice in that + // case would double-pay, so we abort instead of proceeding to the next payment + // method. + Err(Error::PersistenceFailed) => { + log_error!(self.logger, "Failed to send BOLT12 offer: PersistenceFailed. This is part of a unified QR code payment. Aborting to avoid a potential duplicate payment."); + return Err(Error::PersistenceFailed); + }, Err(e) => { log_error!(self.logger, "Failed to send BOLT12 offer: {:?}. This is part of a unified QR code payment. Falling back to the BOLT11 invoice.", e); }, From c43be1a3f9a1a5b66d527596c1591acb962761d1 Mon Sep 17 00:00:00 2001 From: tsk Date: Tue, 22 Sep 2026 12:11:53 +0000 Subject: [PATCH 23/29] fix(payments): ignore late failures for successful Lightning payments Ignore PaymentFailed when an outbound Lightning payment has already succeeded. Check and update the record atomically, preserving its metadata and suppressing the contradictory failure notification. Continue emitting failure events for unchanged or missing records so replay can recover from event-queue persistence failures. Developed with assistance from OpenAI Codex. Backport of 1bb6aa38641c30687dbf6509abac5c3a06820e5c Conflicts Resolved in src/event.rs, src/payment/store.rs --- src/event.rs | 16 ++++++++++++++++ src/payment/store.rs | 13 +++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/event.rs b/src/event.rs index 92b65b5d07..5bfeb8a8d7 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1088,6 +1088,22 @@ where }; }, LdkEvent::PaymentFailed { payment_id, payment_hash, reason, .. } => { + // LDK may emit PaymentFailed after PaymentSent. Ignore the entire late failure so + // it cannot alter payment metadata or emit a contradictory event. + if let Some(payment) = self.payment_store.get(&payment_id) { + if payment.direction == PaymentDirection::Outbound + && payment.kind.is_lightning_payment() + && payment.status == PaymentStatus::Succeeded + { + log_info!( + self.logger, + "Ignoring late payment failure for already-succeeded payment with ID {}.", + payment_id + ); + return Ok(()); + } + } + log_info!( self.logger, "Failed to send payment with ID {} due to {:?}.", diff --git a/src/payment/store.rs b/src/payment/store.rs index 184de2ea97..e574f75242 100644 --- a/src/payment/store.rs +++ b/src/payment/store.rs @@ -445,6 +445,19 @@ pub enum PaymentKind { }, } +impl PaymentKind { + pub(crate) fn is_lightning_payment(&self) -> bool { + match self { + Self::Onchain { .. } => false, + Self::Bolt11 { .. } + | Self::Bolt11Jit { .. } + | Self::Bolt12Offer { .. } + | Self::Bolt12Refund { .. } + | Self::Spontaneous { .. } => true, + } + } +} + impl_writeable_tlv_based_enum!(PaymentKind, (0, Onchain) => { (0, txid, required), From 1cbcd617a6353d286438009a972e86724d2e36e6 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Tue, 6 Oct 2026 15:53:55 +0000 Subject: [PATCH 24/29] Count unknown channel types in anchor reserve Conservatively include channels whose type is not yet known when calculating the anchor reserve. Otherwise, opening or accepting new channels, or splicing funds into existing channels, can leave too little reserved for channels still being negotiated. Do not count channels with unknown channel types when assessing whether 0FC support is required from the chain source. This part is all user-controlled, and we do not want to require the chain source to support 0FC if startup happens with an in-flight channel open, and no other 0FC chain source support predicates are true. Reported by Project Loupe Backport of 87c3182d06bf0df2e85d16a1128c841d283712e6 Conflicts Resolved in src/lib.rs --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 0fd5720b09..a324f5b951 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1837,7 +1837,7 @@ pub(crate) fn total_anchor_channels_reserve_sats( .map_or(true, |s| s != ChannelShutdownState::ShutdownComplete) && c.channel_type .as_ref() - .map_or(false, |t| t.requires_anchors_zero_fee_htlc_tx()) + .map_or(true, |t| t.requires_anchors_zero_fee_htlc_tx()) }) .count() as u64 * anchor_channels_config.per_channel_reserve_sats From ad36b93c3567dfd8609c3fe662f45363eb850ca2 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 13 Sep 2026 16:55:16 +0000 Subject: [PATCH 25/29] Drop spurious outbound payment status update on inb payment If we have sent an outbound payment with a given `payment_hash` and we receive an inbound payment with a matching `payment_hash`, its possible this is an indication of a payment loop, but its also possible its simply a random HTLC with the same hash. We should, thus, not update the outbound payment's status, as it might still be pending and complete. We can detect the payment's failure when the HTLC failure comes back to us as the sender. Backport of c49b86dd50214e3b4cb759fe48794d0144b6f92f Conflicts Resolved in src/event.rs --- src/event.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/event.rs b/src/event.rs index 5bfeb8a8d7..d7e27e961b 100644 --- a/src/event.rs +++ b/src/event.rs @@ -652,18 +652,7 @@ where payment_id ); self.channel_manager.fail_htlc_backwards(&payment_hash); - - let update = PaymentDetailsUpdate { - status: Some(PaymentStatus::Failed), - ..PaymentDetailsUpdate::new(payment_id) - }; - match self.payment_store.update(&update) { - Ok(_) => return Ok(()), - Err(e) => { - log_error!(self.logger, "Failed to access payment store: {}", e); - return Err(ReplayEvent()); - }, - }; + return Ok(()); } if info.status == PaymentStatus::Succeeded From 9517adf909138549d829d5135cadb7fb7a29480a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 13 Sep 2026 16:57:26 +0000 Subject: [PATCH 26/29] Do not mark a successful inbound payment as failed on a dup HTLC If we have already received a payment and marked it as `PaymentStatus::Succeeded` before receiving a second HTLC with the same `payment_hash`, we should leave the payment at `PaymentStatus::Succeeded` rather than reverting it to failed. Backport of a176d513c44060b4fdd3ed074ad9a92f28271142 Conflicts Resolved in src/event.rs --- src/event.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/event.rs b/src/event.rs index d7e27e961b..9f224beb03 100644 --- a/src/event.rs +++ b/src/event.rs @@ -665,18 +665,7 @@ where amount_msat, ); self.channel_manager.fail_htlc_backwards(&payment_hash); - - let update = PaymentDetailsUpdate { - status: Some(PaymentStatus::Failed), - ..PaymentDetailsUpdate::new(payment_id) - }; - match self.payment_store.update(&update) { - Ok(_) => return Ok(()), - Err(e) => { - log_error!(self.logger, "Failed to access payment store: {}", e); - return Err(ReplayEvent()); - }, - }; + return Ok(()); } let max_total_opening_fee_msat = match info.kind { From 68d21422ddab849d761f91c6451695d8751adca5 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 13 Sep 2026 17:04:06 +0000 Subject: [PATCH 27/29] Reject payments with unknown required TLVs `ChannelManager::claim_funds` fails if there are any custom even TLVs. While this is documented, its somewhat deadly for swap services - they might release the preimage in response to a `PaymentClaimable` event and assume they can definitely claim the payment, but LDK will refuse to do so. The `lightning` crate really needs to fix this API for the next release, but for now we should avoid having such an obvious API hole in `ldk-node`. Backport of 8dd1c9148ab73097f5c961216418303803f222fc Conflicts Resolved in src/event.rs --- src/event.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/event.rs b/src/event.rs index 9f224beb03..1e5f06e806 100644 --- a/src/event.rs +++ b/src/event.rs @@ -19,7 +19,7 @@ use lightning::events::{ ClosureReason, Event as LdkEvent, PaymentFailureReason, PaymentPurpose, ReplayEvent, }; use lightning::impl_writeable_tlv_based_enum; -use lightning::ln::channelmanager::PaymentId; +use lightning::ln::channelmanager::{FailureCode, PaymentId}; use lightning::ln::types::ChannelId; use lightning::routing::gossip::NodeId; use lightning::util::config::{ @@ -644,6 +644,22 @@ where .. } => { let payment_id = PaymentId(payment_hash.0); + + if let Some((tlv_type, _)) = onion_fields + .as_ref() + .and_then(|fields| fields.custom_tlvs().iter().find(|(id, _)| id % 2 == 0)) + { + log_info!( + self.logger, + "Refusing inbound payment with hash {} as the sender included unknown required TLVs (incl {})", + hex_utils::to_string(&payment_hash.0), + tlv_type, + ); + let error_code = FailureCode::InvalidOnionPayload(Some((*tlv_type, 0))); + self.channel_manager.fail_htlc_backwards_with_reason(&payment_hash, error_code); + return Ok(()); + } + if let Some(info) = self.payment_store.get(&payment_id) { if info.direction == PaymentDirection::Outbound { log_info!( From 81eb99440c7d40ee373ebc2151725a3fdd495115 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 7 Oct 2026 15:51:49 +0200 Subject: [PATCH 28/29] Reject underpaid BOLT11 JIT invoices BOLT11 JIT invoices did not enforce their advertised amount at the recipient. A sender could bypass the LSP's intercept SCID and have a smaller payment accepted over an existing channel. Require the full sender-declared invoice amount for generated and caller-supplied payment hashes. Preserve negotiated LSP fee deductions and cover both invoice paths with an underpayment regression. Fixes #609 Co-Authored-By: HAL 9000 Backport of b5c731d4257bd0bdac86655e6bd39af43708e3ae Conflicts Resolved in src/liquidity.rs, tests/integration_tests_rust.rs --- src/liquidity.rs | 4 +-- tests/integration_tests_rust.rs | 61 +++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/liquidity.rs b/src/liquidity.rs index 77c7e41835..a9bb44dad3 100644 --- a/src/liquidity.rs +++ b/src/liquidity.rs @@ -1278,7 +1278,7 @@ where .channel_manager .create_inbound_payment_for_hash( payment_hash, - None, + amount_msat, expiry_secs, Some(min_final_cltv_expiry_delta), ) @@ -1290,7 +1290,7 @@ where }, None => self .channel_manager - .create_inbound_payment(None, expiry_secs, Some(min_final_cltv_expiry_delta)) + .create_inbound_payment(amount_msat, expiry_secs, Some(min_final_cltv_expiry_delta)) .map_err(|e| { log_error!(self.logger, "Failed to register inbound payment: {:?}", e); Error::InvoiceCreationFailed diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 4803a81964..8ce90c54d2 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -1806,6 +1806,19 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { let client_config = random_config(true); setup_builder!(client_builder, client_config.node_config); + let client_seed = [42u8; 64]; + #[cfg(feature = "uniffi")] + client_builder.set_entropy_seed_bytes(client_seed.to_vec()).unwrap(); + #[cfg(not(feature = "uniffi"))] + client_builder.set_entropy_seed_bytes(client_seed); + let client_master_key = + bitcoin::bip32::Xpriv::new_master(client_config.node_config.network, &client_seed).unwrap(); + let client_keys = lightning::sign::KeysManager::new( + &client_master_key.private_key.secret_bytes(), + 0, + 0, + true, + ); client_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config)); client_builder.set_liquidity_source_lsps2(service_node_id, service_addr, None); let client_node = client_builder.build().unwrap(); @@ -1898,6 +1911,54 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { expect_event!(service_node, PaymentForwarded); expect_payment_received_event!(client_node, amount_msat); + // Bypass the intercept SCID using the existing service -> client channel. Re-signing a + // smaller invoice only bypasses the sending API's amount check; the recipient gets the + // original payment hash and secret, as it would from a malicious payer. + for payment_hash in [None, Some(PaymentHash::from(PaymentPreimage([44; 32])))] { + let invoice = match payment_hash { + None => client_node.bolt11_payment().receive_via_jit_channel( + jit_amount_msat, + &invoice_description, + 1024, + None, + ), + Some(hash) => client_node.bolt11_payment().receive_via_jit_channel_for_hash( + jit_amount_msat, + &invoice_description, + 1024, + None, + hash, + ), + } + .unwrap(); + let invoice: &lightning_invoice::Bolt11Invoice = &invoice; + let underpayment = lightning_invoice::InvoiceBuilder::new(invoice.currency()) + .description("Underpay JIT invoice".to_owned()) + .payment_hash(*invoice.payment_hash()) + .payment_secret(*invoice.payment_secret()) + .amount_milli_satoshis(1_000_000) + .current_timestamp() + .min_final_cltv_expiry_delta(invoice.min_final_cltv_expiry_delta()) + .build_signed(|hash| { + bitcoin::secp256k1::Secp256k1::new() + .sign_ecdsa_recoverable(hash, &client_keys.get_node_secret_key()) + }) + .unwrap(); + assert_eq!(underpayment.recover_payee_pub_key(), client_node.node_id()); + #[cfg(feature = "uniffi")] + let underpayment = Arc::new(underpayment.into()); + let payment_id = service_node.bolt11_payment().send(&underpayment, None).unwrap(); + tokio::select! { + _ = async { + expect_event!(service_node, PaymentFailed); + assert_eq!(service_node.payment(&payment_id).unwrap().status, PaymentStatus::Failed); + } => {}, + event = client_node.next_event_async() => { + panic!("JIT invoice accepted an underpayment (custom hash: {}): {:?}", payment_hash.is_some(), event); + }, + } + } + //////////////////////////////////////////////////////////////////////////// // receive_via_jit_channel_for_hash and claim_for_hash //////////////////////////////////////////////////////////////////////////// From 11e3da95ea3eb7fee92004e7e1cf6e453ed857f8 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 8 Oct 2026 12:45:43 +0200 Subject: [PATCH 29/29] Bump version to v0.7.1 and update CHANGELOG Bump the crate and bindings version numbers to v0.7.1 and add a CHANGELOG section listing the fixes backported for this patch release. Co-Authored-By: HAL 9000 --- CHANGELOG.md | 38 +++++++++++++++++++ Cargo.toml | 2 +- Package.swift | 2 +- .../kotlin/ldk-node-android/gradle.properties | 2 +- .../kotlin/ldk-node-jvm/gradle.properties | 2 +- bindings/python/pyproject.toml | 2 +- 6 files changed, 43 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38b7d6de55..efc9c6ad58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,41 @@ +# 0.7.1 - Oct. 12, 2026 +This patch release backports a number of bug fixes and security-relevant hardening. + +## Bug Fixes and Improvements +- Inbound payments with a counterparty-withheld fee are now refused unless they are JIT-channel + payments for which we authorized the fee, closing a gap for BOLT12 and spontaneous payments. (#899) +- Unified (BIP21) payments no longer fall back to the next payment method when the BOLT12 or BOLT11 + leg fails with `DuplicatePayment` or `PersistenceFailed`, avoiding potential double payments. + (#1038, #1098) +- A late `PaymentFailed` event no longer marks an already-succeeded outbound payment as failed. + (#1065) +- An inbound HTLC reusing the payment hash of an outbound payment no longer marks that outbound + payment as failed. (#1157) +- A duplicate HTLC for an already-received payment no longer marks that payment as failed. (#1157) +- Inbound payments carrying unknown even-typed custom TLVs are now failed back instead of being + surfaced as claimable, as LDK would refuse to claim them. (#1157) +- BOLT11 JIT invoices now enforce their amount at the recipient, so a payer can no longer bypass + the LSP and have an underpayment accepted over an existing channel. (#1156) +- `Builder::build` now fails instead of starting a fresh node when the `ChannelManager` can't be + read from the store for reasons other than it being absent. (#1032) +- In-memory payment data is no longer updated when the corresponding persistence operation fails. + (#933) +- An anchor reserve is now kept for peers that merely support (rather than require) anchor + channels, while splice-ins no longer require an additional reserve. (#660) +- Channels whose type is not yet negotiated are now counted toward the anchor reserve, so + concurrent channel opens can no longer leave too little reserved. (#1152) +- Electrum `Filter` registrations are now retained across `Node::stop`/`Node::start` cycles. + (#1025) +- Raw transaction fetching and reorg handling via Bitcoin Core REST have been fixed. (#1006, #1022) +- Fee estimation for send-all operations no longer clears a reserved change address, avoiding + address reuse. (#1096) +- Seed files are now created with owner-only permissions. (#741) +- Payment preimages are no longer logged on successful sends. (#1035) +- Several unchecked arithmetic operations have been fixed. (#927, #940) +- The async payments rate limiter now caps the number of tracked users. (#866) +- `bitcoind` mempool reconciliation now runs in linear rather than quadratic time. (#1068) +- Odd-length hex strings are now rejected when parsing hex in the bindings. (#909) + # 0.7.0 - Dec. 3, 2025 This seventh minor release introduces numerous new features, bug fixes, and API improvements. In particular, it adds support for channel Splicing, Async Payments, as well as sourcing chain data from a Bitcoin Core REST backend. diff --git a/Cargo.toml b/Cargo.toml index 27be74f3f4..09bce27f38 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ldk-node" -version = "0.7.0" +version = "0.7.1" authors = ["Elias Rohrer "] homepage = "https://lightningdevkit.org/" license = "MIT OR Apache-2.0" diff --git a/Package.swift b/Package.swift index ed3a97ea04..9f25cddb14 100644 --- a/Package.swift +++ b/Package.swift @@ -3,7 +3,7 @@ import PackageDescription -let tag = "v0.7.0" +let tag = "v0.7.1" let checksum = "37e909987c285ddaaabf7caede58f0695491398acd4561987914996f4623a3c3" let url = "https://github.com/lightningdevkit/ldk-node/releases/download/\(tag)/LDKNodeFFI.xcframework.zip" diff --git a/bindings/kotlin/ldk-node-android/gradle.properties b/bindings/kotlin/ldk-node-android/gradle.properties index fa772eb702..4872c02824 100644 --- a/bindings/kotlin/ldk-node-android/gradle.properties +++ b/bindings/kotlin/ldk-node-android/gradle.properties @@ -2,4 +2,4 @@ org.gradle.jvmargs=-Xmx1536m android.useAndroidX=true android.enableJetifier=true kotlin.code.style=official -libraryVersion=0.7.0 +libraryVersion=0.7.1 diff --git a/bindings/kotlin/ldk-node-jvm/gradle.properties b/bindings/kotlin/ldk-node-jvm/gradle.properties index d24edea2e6..3401039465 100644 --- a/bindings/kotlin/ldk-node-jvm/gradle.properties +++ b/bindings/kotlin/ldk-node-jvm/gradle.properties @@ -1,3 +1,3 @@ org.gradle.jvmargs=-Xmx1536m kotlin.code.style=official -libraryVersion=0.7.0 +libraryVersion=0.7.1 diff --git a/bindings/python/pyproject.toml b/bindings/python/pyproject.toml index 18ba319c40..8ead54bbd1 100644 --- a/bindings/python/pyproject.toml +++ b/bindings/python/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "ldk_node" -version = "0.7.0" +version = "0.7.1" authors = [ { name="Elias Rohrer", email="dev@tnull.de" }, ]