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" }, ] 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() { diff --git a/src/chain/bitcoind.rs b/src/chain/bitcoind.rs index b3d7880d61..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", @@ -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, + } + }), } } @@ -1272,9 +1277,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", @@ -1292,7 +1300,7 @@ impl TryInto for JsonResponse { } } -pub struct GetRawMempoolResponse(Vec); +pub struct GetRawMempoolResponse(HashSet); impl TryInto for JsonResponse { type Error = std::io::Error; @@ -1302,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() { @@ -1322,7 +1330,7 @@ impl TryInto for JsonResponse { )); }; - mempool_transactions.push(txid); + mempool_transactions.insert(txid); } Ok(GetRawMempoolResponse(mempool_transactions)) @@ -1480,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; @@ -1558,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); @@ -1603,6 +1613,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(); 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/src/data_store.rs b/src/data_store.rs index 87bd831c9b..796623b601 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,28 +75,28 @@ 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> { - 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(()) } @@ -127,17 +129,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 { @@ -171,7 +175,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 +305,107 @@ 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()); + } + + #[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)); + } + + #[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)); + } } diff --git a/src/event.rs b/src/event.rs index f64710d508..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!( @@ -652,18 +668,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 @@ -676,18 +681,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 { @@ -790,6 +784,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!( @@ -1050,8 +1056,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) @@ -1059,7 +1064,6 @@ where "".to_string() }, hex_utils::to_string(&payment_hash.0), - hex_utils::to_string(&payment_preimage.0) ); }); let event = Event::PaymentSuccessful { @@ -1078,6 +1082,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/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; 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); diff --git a/src/lib.rs b/src/lib.rs index 2fdcdddbf4..a324f5b951 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.supports_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; @@ -1375,7 +1381,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); } @@ -1829,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 diff --git a/src/liquidity.rs b/src/liquidity.rs index 74e6098ddc..a9bb44dad3 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 @@ -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/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 } 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", 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), diff --git a/src/payment/unified_qr.rs b/src/payment/unified_qr.rs index d3c879cba1..f139f450e4 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) { @@ -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); }, @@ -165,6 +179,20 @@ 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); + }, + // 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); }, 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()), 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..8ce90c54d2 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(); @@ -973,6 +1027,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; @@ -1568,6 +1634,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(); @@ -1647,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(); @@ -1719,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(); @@ -1811,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 ////////////////////////////////////////////////////////////////////////////