From e8363b9c1ce99164e01fa8c20d6e779d2c56607b Mon Sep 17 00:00:00 2001 From: benthecarman Date: Fri, 12 Jun 2026 14:29:30 -0500 Subject: [PATCH 1/4] 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. --- src/payment/unified.rs | 4 ++-- tests/integration_tests_rust.rs | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/payment/unified.rs b/src/payment/unified.rs index 3708afe8e6..2ad77f7728 100644 --- a/src/payment/unified.rs +++ b/src/payment/unified.rs @@ -129,9 +129,9 @@ impl UnifiedPayment { 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 309d5bf4d3..76835b38a8 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -1680,6 +1680,18 @@ async fn generate_bip21_uri() { assert!(uni_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 = random_chain_source(&bitcoind, &electrsd); + let node = setup_node(&chain_source, random_config(true)); + + assert_eq!( + Err(NodeError::InvalidAmount), + node.unified_payment().receive(u64::MAX, "asdf", 4_000) + ); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn unified_send_receive_bip21_uri() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); From 588ae2ff9294e93b10e8e1614847dd88bd3c2371 Mon Sep 17 00:00:00 2001 From: benthecarman Date: Fri, 12 Jun 2026 14:39:37 -0500 Subject: [PATCH 2/4] Clamp Electrum fee estimates Limit fee estimates returned by Electrum servers before they enter the fee-rate cache, and warn when either the raw estimate or adjusted value has to be capped. This prevents extreme server-provided values from flowing into wallet transaction construction. This commit was created with assistance from OpenAI Codex. --- src/chain/electrum.rs | 116 ++++++++++++++++++++++++++++++++++++------ src/logger.rs | 2 +- 2 files changed, 101 insertions(+), 17 deletions(-) diff --git a/src/chain/electrum.rs b/src/chain/electrum.rs index ad0ef1b7ba..9a2533deeb 100644 --- a/src/chain/electrum.rs +++ b/src/chain/electrum.rs @@ -31,7 +31,7 @@ use crate::fee_estimator::{ ConfirmationTarget, OnchainFeeEstimator, }; use crate::io::utils::update_and_persist_node_metrics; -use crate::logger::{log_bytes, log_debug, log_error, log_trace, LdkLogger, Logger}; +use crate::logger::{log_bytes, log_debug, log_error, log_trace, log_warn, LdkLogger, Logger}; use crate::runtime::Runtime; use crate::types::{ChainMonitor, ChannelManager, DynStore, Sweeper, Wallet}; use crate::PersistedNodeMetrics; @@ -39,6 +39,48 @@ use crate::PersistedNodeMetrics; const BDK_ELECTRUM_CLIENT_BATCH_SIZE: usize = 5; const ELECTRUM_CLIENT_NUM_RETRIES: u8 = 3; +// Electrum returns fee estimates in BTC/kvB. Values below this fall back to 1 sat/vB. +const ELECTRUM_MIN_FEE_RATE_BTC_PER_KVB: f64 = 0.00001; +// Convert BTC/kvB to sat/kwu: 100_000_000 sats per BTC divided by 4 weight units per vbyte. +const ELECTRUM_BTC_PER_KVB_TO_SAT_PER_KWU: f64 = 25_000_000.0; +// Cap estimates from the Electrum server at 10,000 sat/vB. +const ELECTRUM_MAX_FEE_RATE_SAT_PER_KWU: u64 = 2_500_000; + +fn fee_rate_from_electrum_estimate( + raw_fee_rate_btc_per_kvb: &serde_json::Value, target: ConfirmationTarget, logger: &Logger, +) -> FeeRate { + // Parse the retrieved serde_json::Value and fall back to 1 sat/vb (10^3 / 10^8 = 10^-5 + // = 0.00001 btc/kvb) if we fail or it yields less than that. This is mostly necessary + // to continue on `signet`/`regtest` where we might not get estimates (or bogus values). + let fee_rate_btc_per_kvb = raw_fee_rate_btc_per_kvb + .as_f64() + .filter(|converted| converted.is_finite()) + .map_or(ELECTRUM_MIN_FEE_RATE_BTC_PER_KVB, |converted| { + converted.max(ELECTRUM_MIN_FEE_RATE_BTC_PER_KVB) + }); + + // Electrum, just like Bitcoin Core, gives us a feerate in BTC/KvB. Thus, we multiply by + // 25_000_000 (10^8 / 4) to get satoshis/kwu. + let rounded_fee_rate_sat_per_kwu = + (fee_rate_btc_per_kvb * ELECTRUM_BTC_PER_KVB_TO_SAT_PER_KWU).round(); + let fee_rate_was_clamped = + rounded_fee_rate_sat_per_kwu > ELECTRUM_MAX_FEE_RATE_SAT_PER_KWU as f64; + let clamped_fee_rate_sat_per_kwu = + rounded_fee_rate_sat_per_kwu.min(ELECTRUM_MAX_FEE_RATE_SAT_PER_KWU as f64) as u64; + if fee_rate_was_clamped { + log_warn!( + logger, + "Clamped Electrum fee rate estimate for {target:?} to {ELECTRUM_MAX_FEE_RATE_SAT_PER_KWU} sats/kwu" + ); + } + + FeeRate::from_sat_per_kwu(clamped_fee_rate_sat_per_kwu) +} + +fn clamp_electrum_fee_rate(fee_rate: FeeRate) -> FeeRate { + FeeRate::from_sat_per_kwu(fee_rate.to_sat_per_kwu().min(ELECTRUM_MAX_FEE_RATE_SAT_PER_KWU)) +} + pub(super) struct ElectrumChainSource { server_url: String, pub(super) sync_config: ElectrumSyncConfig, @@ -643,24 +685,21 @@ impl ElectrumRuntimeClient { for (target, raw_fee_rate_btc_per_kvb) in confirmation_targets.into_iter().zip(raw_estimates_btc_kvb.into_iter()) { - // Parse the retrieved serde_json::Value and fall back to 1 sat/vb (10^3 / 10^8 = 10^-5 - // = 0.00001 btc/kvb) if we fail or it yields less than that. This is mostly necessary - // to continue on `signet`/`regtest` where we might not get estimates (or bogus - // values). - let fee_rate_btc_per_kvb = raw_fee_rate_btc_per_kvb - .as_f64() - .map_or(0.00001, |converted| converted.max(0.00001)); - - // Electrum, just like Bitcoin Core, gives us a feerate in BTC/KvB. - // Thus, we multiply by 25_000_000 (10^8 / 4) to get satoshis/kwu. - let fee_rate = { - let fee_rate_sat_per_kwu = (fee_rate_btc_per_kvb * 25_000_000.0).round() as u64; - FeeRate::from_sat_per_kwu(fee_rate_sat_per_kwu) - }; + let fee_rate = + fee_rate_from_electrum_estimate(&raw_fee_rate_btc_per_kvb, target, &self.logger); // LDK 0.0.118 introduced changes to the `ConfirmationTarget` semantics that // require some post-estimation adjustments to the fee rates, which we do here. - let adjusted_fee_rate = apply_post_estimation_adjustments(target, fee_rate); + let unclamped_adjusted_fee_rate = apply_post_estimation_adjustments(target, fee_rate); + let adjusted_fee_rate = clamp_electrum_fee_rate(unclamped_adjusted_fee_rate); + if adjusted_fee_rate != unclamped_adjusted_fee_rate { + log_warn!( + self.logger, + "Clamped adjusted Electrum fee rate estimate for {:?} to {} sats/kwu", + target, + ELECTRUM_MAX_FEE_RATE_SAT_PER_KWU + ); + } new_fee_rate_cache.insert(target, adjusted_fee_rate); @@ -684,3 +723,48 @@ impl Filter for ElectrumRuntimeClient { self.tx_sync.register_output(output) } } + +#[cfg(test)] +mod tests { + use super::*; + use lightning::chain::chaininterface::ConfirmationTarget as LdkConfirmationTarget; + use serde_json::json; + + #[test] + fn electrum_fee_rate_estimate_clamps_excessive_values() { + let logger = Logger::new_log_facade(); + let fee_rate = fee_rate_from_electrum_estimate( + &json!(1.0e20), + ConfirmationTarget::OnchainPayment, + &logger, + ); + + assert_eq!(fee_rate.to_sat_per_kwu(), ELECTRUM_MAX_FEE_RATE_SAT_PER_KWU); + } + + #[test] + fn electrum_fee_rate_estimate_rejects_invalid_values() { + let logger = Logger::new_log_facade(); + let fee_rate = fee_rate_from_electrum_estimate( + &json!(null), + ConfirmationTarget::OnchainPayment, + &logger, + ); + + assert_eq!(fee_rate.to_sat_per_kwu(), 250); + } + + #[test] + fn electrum_fee_rate_adjustment_preserves_ceiling() { + let fee_rate = FeeRate::from_sat_per_kwu(ELECTRUM_MAX_FEE_RATE_SAT_PER_KWU); + let adjusted_fee_rate = apply_post_estimation_adjustments( + LdkConfirmationTarget::MaximumFeeEstimate.into(), + fee_rate, + ); + + assert_eq!( + clamp_electrum_fee_rate(adjusted_fee_rate).to_sat_per_kwu(), + ELECTRUM_MAX_FEE_RATE_SAT_PER_KWU + ); + } +} diff --git a/src/logger.rs b/src/logger.rs index 3ef939b6d4..c5a4584a18 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -19,7 +19,7 @@ use lightning::ln::types::ChannelId; use lightning::types::payment::PaymentHash; pub use lightning::util::logger::Level as LogLevel; pub(crate) use lightning::util::logger::{Logger as LdkLogger, Record as LdkRecord}; -pub(crate) use lightning::{log_bytes, log_debug, log_error, log_info, log_trace}; +pub(crate) use lightning::{log_bytes, log_debug, log_error, log_info, log_trace, log_warn}; use log::{Level as LogFacadeLevel, Record as LogFacadeRecord}; /// A unit of logging output with metadata to enable filtering `module_path`, From c4d1e49a4623b472e8d0cfe7d0ae15497b585f9d Mon Sep 17 00:00:00 2001 From: benthecarman Date: Fri, 12 Jun 2026 14:52:23 -0500 Subject: [PATCH 3/4] 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. --- 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 068269997f..e3cb948a1b 100644 --- a/src/payment/bolt11.rs +++ b/src/payment/bolt11.rs @@ -539,7 +539,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 66a8c03ba23eb12e9a442bb3adf3d6f1848e37c9 Mon Sep 17 00:00:00 2001 From: benthecarman Date: Fri, 12 Jun 2026 15:42:11 -0500 Subject: [PATCH 4/4] Deduplicate registered chain txids Track registered transaction IDs in a set so repeated filter registrations do not grow the collection or slow block-connected checks. This keeps the wallet's registered-transaction lookup bounded by unique transaction IDs. This commit was created with assistance from OpenAI Codex. --- src/chain/mod.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/chain/mod.rs b/src/chain/mod.rs index 92c4bdb641..5a326be97b 100644 --- a/src/chain/mod.rs +++ b/src/chain/mod.rs @@ -9,7 +9,7 @@ pub(crate) mod bitcoind; mod electrum; mod esplora; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -84,7 +84,7 @@ impl WalletSyncStatus { pub(crate) struct ChainSource { kind: ChainSourceKind, - registered_txids: Mutex>, + registered_txids: Mutex>, tx_broadcaster: Arc, logger: Arc, } @@ -113,7 +113,7 @@ impl ChainSource { node_metrics, )?; let kind = ChainSourceKind::Esplora(esplora_chain_source); - let registered_txids = Mutex::new(Vec::new()); + let registered_txids = Mutex::new(HashSet::new()); Ok((Self { kind, registered_txids, tx_broadcaster, logger }, None)) } @@ -133,7 +133,7 @@ impl ChainSource { node_metrics, ); let kind = ChainSourceKind::Electrum(electrum_chain_source); - let registered_txids = Mutex::new(Vec::new()); + let registered_txids = Mutex::new(HashSet::new()); (Self { kind, registered_txids, tx_broadcaster, logger }, None) } @@ -156,7 +156,7 @@ impl ChainSource { ); let best_block = bitcoind_chain_source.poll_best_block().await.ok(); let kind = ChainSourceKind::Bitcoind(bitcoind_chain_source); - let registered_txids = Mutex::new(Vec::new()); + let registered_txids = Mutex::new(HashSet::new()); (Self { kind, registered_txids, tx_broadcaster, logger }, best_block) } @@ -180,7 +180,7 @@ impl ChainSource { ); let best_block = bitcoind_chain_source.poll_best_block().await.ok(); let kind = ChainSourceKind::Bitcoind(bitcoind_chain_source); - let registered_txids = Mutex::new(Vec::new()); + let registered_txids = Mutex::new(HashSet::new()); (Self { kind, registered_txids, tx_broadcaster, logger }, best_block) } @@ -214,7 +214,7 @@ impl ChainSource { } } - pub(crate) fn registered_txids(&self) -> Vec { + pub(crate) fn registered_txids(&self) -> HashSet { self.registered_txids.lock().expect("lock").clone() } @@ -472,7 +472,7 @@ impl ChainSource { impl Filter for ChainSource { fn register_tx(&self, txid: &Txid, script_pubkey: &Script) { - self.registered_txids.lock().expect("lock").push(*txid); + self.registered_txids.lock().expect("lock").insert(*txid); match &self.kind { ChainSourceKind::Esplora(esplora_chain_source) => { esplora_chain_source.register_tx(txid, script_pubkey)