From 8a5572630fa5a1c2fdbf200346e1a69c63028568 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 30 Apr 2026 15:33:44 +0200 Subject: [PATCH 01/17] lightning: introduce singular claim requests Have ChannelMonitor hand singular ClaimRequests to OnchainTxHandler. Convert them to PackageTemplates only after duplicate filtering. This makes the single-outpoint invariant explicit at that boundary. --- lightning/src/chain/channelmonitor.rs | 53 ++++++++++++++------------- lightning/src/chain/onchaintx.rs | 31 ++++++---------- lightning/src/chain/package.rs | 42 +++++++++++++++++++++ 3 files changed, 81 insertions(+), 45 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a2412bbaf5e..57a8da47352 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -38,8 +38,8 @@ use crate::chain::chaininterface::{ }; use crate::chain::onchaintx::{ClaimEvent, FeerateStrategy, OnchainTxHandler}; use crate::chain::package::{ - CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, HolderFundingOutput, - HolderHTLCOutput, PackageSolvingData, PackageTemplate, RevokedHTLCOutput, RevokedOutput, + ClaimRequest, CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, + HolderFundingOutput, HolderHTLCOutput, PackageSolvingData, RevokedHTLCOutput, RevokedOutput, }; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::chain::{BlockLocator, WatchedOutput}; @@ -3879,7 +3879,7 @@ impl ChannelMonitorImpl { fn generate_claimable_outpoints_and_watch_outputs( &mut self, generate_monitor_event_with_reason: Option, require_funding_seen: bool, - ) -> (Vec, Vec) { + ) -> (Vec, Vec) { let funding = get_confirmed_funding_scope!(self); let holder_commitment_tx = &funding.current_holder_commitment_tx; let funding_outp = HolderFundingOutput::build( @@ -3887,7 +3887,7 @@ impl ChannelMonitorImpl { funding.channel_parameters.clone(), ); let funding_outpoint = funding.funding_outpoint(); - let commitment_package = PackageTemplate::build_package( + let commitment_package = ClaimRequest::new( funding_outpoint.txid.clone(), funding_outpoint.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), self.best_block.height, @@ -3926,9 +3926,9 @@ impl ChannelMonitorImpl { let zero_fee_commitments = self.channel_type_features().supports_anchor_zero_fee_commitments(); if !zero_fee_htlcs && !zero_fee_commitments { - // Because we're broadcasting a commitment transaction, we should construct the package - // assuming it gets confirmed in the next block. Sadly, we have code which considers - // "not yet confirmed" things as discardable, so we cannot do that here. + // Because we're broadcasting a commitment transaction, we should construct claim + // requests assuming it gets confirmed in the next block. Sadly, we have code which + // considers "not yet confirmed" things as discardable, so we cannot do that here. let (mut new_outpoints, _) = self.get_broadcasted_holder_claims( funding, holder_commitment_tx, self.best_block.height, ); @@ -4806,11 +4806,11 @@ impl ChannelMonitorImpl { /// height > height + CLTV_SHARED_CLAIM_BUFFER. In any case, will install monitoring for /// HTLC-Success/HTLC-Timeout transactions. /// - /// Returns packages to claim the revoked output(s) and general information about the output that - /// is to the counterparty in the commitment transaction. + /// Returns claim requests for the revoked output(s) and general information about the output + /// that is to the counterparty in the commitment transaction. #[rustfmt::skip] fn check_spend_counterparty_transaction(&mut self, commitment_txid: Txid, commitment_tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &L) - -> (Vec, CommitmentTxCounterpartyOutputInfo) + -> (Vec, CommitmentTxCounterpartyOutputInfo) { // Most secp and related errors trying to create keys means we have no hope of constructing // a spend transaction...so we return no transactions to broadcast @@ -4850,7 +4850,7 @@ impl ChannelMonitorImpl { per_commitment_point, per_commitment_key, outp.value, funding_spent.channel_parameters.clone(), height, ); - let justice_package = PackageTemplate::build_package( + let justice_package = ClaimRequest::new( commitment_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp), height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32, @@ -4879,7 +4879,7 @@ impl ChannelMonitorImpl { } else { height }; - let justice_package = PackageTemplate::build_package( + let justice_package = ClaimRequest::new( commitment_txid, transaction_output_index, PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp), @@ -4968,7 +4968,7 @@ impl ChannelMonitorImpl { commitment_txid: Txid, per_commitment_option: Option<&Vec<(HTLCOutputInCommitment, Option>)>>, confirmation_height: Option, - ) -> Vec { + ) -> Vec { let per_commitment_claimable_data = match per_commitment_option { Some(outputs) => outputs, None => return Vec::new(), @@ -4993,7 +4993,7 @@ impl ChannelMonitorImpl { confirmation_height, ), ); - Some(PackageTemplate::build_package( + Some(ClaimRequest::new( commitment_txid, transaction_output_index, htlc_data, @@ -5009,13 +5009,13 @@ impl ChannelMonitorImpl { .collect() } - /// Returns the HTLC claim package templates and the counterparty output info + /// Returns the HTLC claim requests and the counterparty output info. fn get_counterparty_output_claim_info( &self, funding_spent: &FundingScope, commitment_number: u64, commitment_txid: Txid, tx: &Transaction, per_commitment_claimable_data: &[(HTLCOutputInCommitment, Option>)], confirmation_height: Option, - ) -> (Vec, CommitmentTxCounterpartyOutputInfo) { + ) -> (Vec, CommitmentTxCounterpartyOutputInfo) { let mut claimable_outpoints = Vec::new(); let mut to_counterparty_output_info: CommitmentTxCounterpartyOutputInfo = None; @@ -5086,7 +5086,7 @@ impl ChannelMonitorImpl { ), ) }; - let counterparty_package = PackageTemplate::build_package( + let counterparty_package = ClaimRequest::new( commitment_txid, transaction_output_index, counterparty_htlc_outp, @@ -5104,7 +5104,7 @@ impl ChannelMonitorImpl { #[rustfmt::skip] fn check_spend_counterparty_htlc( &mut self, tx: &Transaction, commitment_number: u64, commitment_txid: &Txid, height: u32, logger: &L - ) -> (Vec, Option) { + ) -> (Vec, Option) { let secret = if let Some(secret) = self.get_secret(commitment_number) { secret } else { return (Vec::new(), None); }; let per_commitment_key = match SecretKey::from_slice(&secret) { Ok(key) => key, @@ -5135,7 +5135,7 @@ impl ChannelMonitorImpl { per_commitment_point, per_commitment_key, tx.output[idx].value, self.funding.channel_parameters.clone(), height, ); - let justice_package = PackageTemplate::build_package( + let justice_package = ClaimRequest::new( htlc_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp), height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32, ); @@ -5187,13 +5187,14 @@ impl ChannelMonitorImpl { htlcs } - // Returns (1) `PackageTemplate`s that can be given to the OnchainTxHandler, so that the handler can - // broadcast transactions claiming holder HTLC commitment outputs and (2) a holder revokable - // script so we can detect whether a holder transaction has been seen on-chain. + // Returns (1) `ClaimRequest`s that can be given to the OnchainTxHandler, so that the + // handler can broadcast transactions claiming holder HTLC commitment outputs and (2) a + // holder revokable script so we can detect whether a holder transaction has been seen + // on-chain. #[rustfmt::skip] fn get_broadcasted_holder_claims( &self, funding: &FundingScope, holder_tx: &HolderCommitmentTransaction, conf_height: u32, - ) -> (Vec, Option<(ScriptBuf, PublicKey, RevocationKey)>) { + ) -> (Vec, Option<(ScriptBuf, PublicKey, RevocationKey)>) { let tx = holder_tx.trust(); let keys = tx.keys(); let redeem_script = chan_utils::get_revokeable_redeemscript( @@ -5212,7 +5213,7 @@ impl ChannelMonitorImpl { }; let transaction_output_index = htlc_descriptor.htlc.transaction_output_index .expect("Expected transaction output index for non-dust HTLC"); - PackageTemplate::build_package( + ClaimRequest::new( tx.txid(), transaction_output_index, PackageSolvingData::HolderHTLCOutput(HolderHTLCOutput::build(htlc_descriptor, conf_height)), counterparty_spendable_height, @@ -5248,7 +5249,7 @@ impl ChannelMonitorImpl { fn check_spend_holder_transaction( &mut self, commitment_txid: Txid, commitment_tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &L, - ) -> Option<(Vec, TransactionOutputs)> { + ) -> Option<(Vec, TransactionOutputs)> { let funding_spent = get_confirmed_funding_scope!(self); // HTLCs set may differ between last and previous holder commitment txn, in case of one them hitting chain, ensure we cancel all HTLCs backward @@ -5759,7 +5760,7 @@ impl ChannelMonitorImpl { conf_hash: BlockHash, txn_matched: Vec<&Transaction>, mut watch_outputs: Vec, - mut claimable_outpoints: Vec, + mut claimable_outpoints: Vec, broadcaster: &B, fee_estimator: &LowerBoundedFeeEstimator, logger: &WithContext, diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 3eb6d64f3a2..823b81936ce 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -27,7 +27,7 @@ use crate::chain::chaininterface::{ BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator, TransactionType, }; use crate::chain::channelmonitor::ANTI_REORG_DELAY; -use crate::chain::package::{PackageSolvingData, PackageTemplate}; +use crate::chain::package::{ClaimRequest, PackageSolvingData, PackageTemplate}; use crate::chain::transaction::MaybeSignedTransaction; use crate::chain::ClaimId; use crate::ln::chan_utils::{ @@ -791,7 +791,7 @@ impl OnchainTxHandler { /// `cur_height`, however it must never be higher than `cur_height`. #[rustfmt::skip] pub(super) fn update_claims_view_from_requests( - &mut self, mut requests: Vec, conf_height: u32, cur_height: u32, + &mut self, mut requests: Vec, conf_height: u32, cur_height: u32, broadcaster: &B, conf_target: ConfirmationTarget, destination_script: &Script, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, ) { @@ -801,33 +801,26 @@ impl OnchainTxHandler { // First drop any duplicate claims. requests.retain(|req| { - debug_assert_eq!( - req.outpoints().len(), - 1, - "Claims passed to `update_claims_view_from_requests` should not be aggregated" - ); - let mut all_outpoints_claiming = true; - for outpoint in req.outpoints() { - if self.claimable_outpoints.get(outpoint).is_none() { - all_outpoints_claiming = false; - } - } - if all_outpoints_claiming { + let outpoint = req.outpoint(); + if self.claimable_outpoints.get(outpoint).is_some() { log_info!(logger, "Ignoring second claim for outpoint {}:{}, already registered its claiming request", - req.outpoints()[0].txid, req.outpoints()[0].vout); + outpoint.txid, outpoint.vout); false } else { let timelocked_equivalent_package = self.locktimed_packages.iter().map(|v| v.1.iter()).flatten() - .find(|locked_package| locked_package.outpoints() == req.outpoints()); + .find(|locked_package| locked_package.outpoints().len() == 1 && locked_package.contains_outpoint(outpoint)); if let Some(package) = timelocked_equivalent_package { log_info!(logger, "Ignoring second claim for outpoint {}:{}, we already have one which we're waiting on a timelock at {} for.", - req.outpoints()[0].txid, req.outpoints()[0].vout, package.package_locktime(cur_height)); + outpoint.txid, outpoint.vout, package.package_locktime(cur_height)); false } else { true } } }); + let mut requests = requests.into_iter() + .map(ClaimRequest::into_package_template) + .collect::>(); // Then try to maximally aggregate `requests`. for i in (1..requests.len()).rev() { @@ -1290,7 +1283,7 @@ mod tests { use types::features::ChannelTypeFeatures; use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator}; - use crate::chain::package::{HolderHTLCOutput, PackageSolvingData, PackageTemplate}; + use crate::chain::package::{ClaimRequest, HolderHTLCOutput, PackageSolvingData}; use crate::chain::transaction::OutPoint; use crate::ln::chan_utils::{ ChannelPublicKeys, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, @@ -1412,7 +1405,7 @@ mod tests { let holder_commit_txid = holder_commit.trust().txid(); let mut requests = Vec::new(); for (htlc, counterparty_sig) in holder_commit.nondust_htlcs().iter().zip(holder_commit.counterparty_htlc_sigs.iter()) { - requests.push(PackageTemplate::build_package( + requests.push(ClaimRequest::new( holder_commit_txid, htlc.transaction_output_index.unwrap(), PackageSolvingData::HolderHTLCOutput(HolderHTLCOutput::build(HTLCDescriptor { diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index 269a8dd1d7d..f28ed72dbb2 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -1097,6 +1097,19 @@ enum PackageMalleability { Untractable, } +/// A single on-chain output claim generated by [`ChannelMonitor`]. +/// +/// These requests are converted to [`PackageTemplate`]s once [`OnchainTxHandler`] has deduplicated +/// them and is ready to aggregate compatible claims. +/// +/// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor +/// [`OnchainTxHandler`]: crate::chain::onchaintx::OnchainTxHandler +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ClaimRequest { + input: (BitcoinOutPoint, PackageSolvingData), + counterparty_spendable_height: u32, +} + /// A structure to describe a package content that is generated by ChannelMonitor and /// used by OnchainTxHandler to generate and broadcast transactions settling onchain claims. /// @@ -1179,6 +1192,32 @@ impl PartialEq for PackageTemplate { } } +impl ClaimRequest { + pub(crate) fn new( + txid: Txid, vout: u32, input_solving_data: PackageSolvingData, + counterparty_spendable_height: u32, + ) -> Self { + Self { + input: (BitcoinOutPoint { txid, vout }, input_solving_data), + counterparty_spendable_height, + } + } + + pub(crate) fn outpoint(&self) -> &BitcoinOutPoint { + &self.input.0 + } + + pub(crate) fn into_package_template(self) -> PackageTemplate { + let (outpoint, input_solving_data) = self.input; + PackageTemplate::build_package( + outpoint.txid, + outpoint.vout, + input_solving_data, + self.counterparty_spendable_height, + ) + } +} + impl PackageTemplate { #[rustfmt::skip] pub(crate) fn can_merge_with(&self, other: &PackageTemplate, cur_height: u32) -> bool { @@ -1265,6 +1304,9 @@ impl PackageTemplate { pub(crate) fn outpoints(&self) -> Vec<&BitcoinOutPoint> { self.inputs.iter().map(|(o, _)| o).collect() } + pub(crate) fn contains_outpoint(&self, outpoint: &BitcoinOutPoint) -> bool { + self.inputs.iter().any(|(input, _)| input == outpoint) + } pub(crate) fn outpoints_and_creation_heights( &self, ) -> impl Iterator)> { From 3abeb61e10a767600dcdd7e022d2d7102169f2b2 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 4 May 2026 10:36:53 +0200 Subject: [PATCH 02/17] lightning: clarify channelmonitor event thresholds Clarify ChannelMonitor comments around on-chain event thresholds. Some events only wait for anti-reorg finality, while CSV-delayed outputs wait until spendable through the same threshold queue. --- lightning/src/chain/channelmonitor.rs | 51 ++++++++++++++------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 57a8da47352..441defcea0d 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -473,7 +473,8 @@ impl Readable for CounterpartyCommitmentParameters { /// An entry for an [`OnchainEvent`], stating the block height and hash when the event was /// observed, as well as the transaction causing it. /// -/// Used to determine when the on-chain event can be considered safe from a chain reorganization. +/// Used to determine when the on-chain event can be considered safe from a chain reorganization +/// or, for CSV-delayed outputs, spendable. #[derive(Clone, PartialEq, Eq)] struct OnchainEventEntry { txid: Txid, @@ -491,14 +492,14 @@ impl OnchainEventEntry { OnchainEvent::MaturingOutput { descriptor: SpendableOutputDescriptor::DelayedPaymentOutput(ref descriptor) } => { - // A CSV'd transaction is confirmable in block (input height) + CSV delay, which means - // it's broadcastable when we see the previous block. + // A CSV-delayed output is spendable in block (input height) + CSV delay, which + // means we can hand it upstream when we see the previous block. conf_threshold = cmp::max(conf_threshold, self.height + descriptor.to_self_delay as u32 - 1); }, OnchainEvent::FundingSpendConfirmation { on_local_output_csv: Some(csv), .. } | OnchainEvent::HTLCSpendConfirmation { on_to_local_output_csv: Some(csv), .. } => { - // A CSV'd transaction is confirmable in block (input height) + CSV delay, which means - // it's broadcastable when we see the previous block. + // A CSV-delayed output is spendable in block (input height) + CSV delay, which + // means we can act on the event when we see the previous block. conf_threshold = cmp::max(conf_threshold, self.height + csv as u32 - 1); }, _ => {}, @@ -517,7 +518,7 @@ impl OnchainEventEntry { type CommitmentTxCounterpartyOutputInfo = Option<(u32, Amount)>; /// Upon discovering of some classes of onchain tx by ChannelMonitor, we may have to take actions on it -/// once they mature to enough confirmations (ANTI_REORG_DELAY) +/// once they reach anti-reorg finality or, for CSV-delayed outputs, CSV maturity. #[derive(Clone, PartialEq, Eq)] enum OnchainEvent { /// An outbound HTLC failing after a transaction is confirmed. Used @@ -534,8 +535,8 @@ enum OnchainEvent { /// transaction which appeared on chain. commitment_tx_output_idx: Option, }, - /// An output waiting on [`ANTI_REORG_DELAY`] confirmations before we hand the user the - /// [`SpendableOutputDescriptor`]. + /// An output waiting until it is anti-reorg final and, for CSV-delayed outputs, spendable + /// before we hand the user the [`SpendableOutputDescriptor`]. MaturingOutput { descriptor: SpendableOutputDescriptor }, /// A spend of the funding output, either a commitment transaction or a cooperative closing /// transaction. @@ -566,8 +567,8 @@ enum OnchainEvent { /// If the claim was made by either party with a preimage, this is filled in preimage: Option, /// If the claim was made by us on an inbound HTLC against a local commitment transaction, - /// we set this to the output CSV value which we will have to wait until to spend the - /// output (and generate a SpendableOutput event). + /// this records the CSV delay for the delayed output. While present, the event reaches + /// its threshold once the output is spendable. on_to_local_output_csv: Option, }, /// An alternative funding transaction (due to a splice/RBF) has confirmed but can no longer be @@ -1003,7 +1004,7 @@ impl Balance { } } -/// An HTLC which has been irrevocably resolved on-chain, and has reached ANTI_REORG_DELAY. +/// An HTLC whose on-chain outcome has reached the threshold for irrevocable resolution. #[derive(Clone, PartialEq, Eq)] struct IrrevocablyResolvedHTLC { commitment_tx_output_idx: Option, @@ -1301,8 +1302,9 @@ pub(crate) struct ChannelMonitorImpl { pub(super) is_processing_pending_events: bool, // Used to track on-chain events (i.e., transactions part of channels confirmed on chain) on - // which to take actions once they reach enough confirmations. Each entry includes the - // transaction's id and the height when the transaction was confirmed on chain. + // which to take actions once they reach anti-reorg finality or, for CSV-delayed outputs, + // CSV maturity. Each entry includes the transaction's id and the height when the transaction + // was confirmed on chain. onchain_events_awaiting_threshold_conf: Vec, // If we get serialized out and re-read, we need to make sure that the chain monitoring @@ -1339,8 +1341,8 @@ pub(crate) struct ChannelMonitorImpl { /// Added in 0.0.124. holder_pays_commitment_tx_fee: Option, - /// Set to `Some` of the confirmed transaction spending the funding input of the channel after - /// reaching `ANTI_REORG_DELAY` confirmations. + /// Set to `Some` once the confirmed transaction spending the funding input of the channel has + /// reached its event threshold. funding_spend_confirmed: Option, confirmed_commitment_tx_counterparty_output: CommitmentTxCounterpartyOutputInfo, @@ -2763,11 +2765,10 @@ impl ChannelMonitorImpl { source: BalanceSource::Htlc, }); } else if htlc_resolved && !htlc_output_spend_pending { - // Funding transaction spends should be fully confirmed by the time any - // HTLC transactions are resolved, unless we're talking about a holder - // commitment tx, whose resolution is delayed until the CSV timeout is - // reached, even though HTLCs may be resolved after only - // ANTI_REORG_DELAY confirmations. + // Funding transaction spends should have reached their event threshold by the time any + // HTLC transactions are irrevocably resolved, unless we're talking about a holder + // commitment tx, whose resolution is delayed until CSV maturity, even though HTLCs + // may be resolved after anti-reorg finality. debug_assert!(holder_commitment || self.funding_spend_confirmed.is_some()); } else if counterparty_revoked_commitment { let htlc_output_claim_pending = self.onchain_events_awaiting_threshold_conf.iter().any(|event| { @@ -2889,7 +2890,7 @@ impl ChannelMonitor { }); if let Some((txid, conf_thresh)) = funding_spend_pending { debug_assert!(us.funding_spend_confirmed.is_none(), - "We have a pending funding spend awaiting anti-reorg confirmation, we can't have confirmed it already!"); + "We have a pending funding spend awaiting its event threshold, it cannot have reached it already!"); confirmed_txid = Some(txid); pending_commitment_tx_conf_thresh = Some(conf_thresh); } @@ -3347,7 +3348,7 @@ macro_rules! fail_unbroadcast_htlcs { commitment_tx_output_idx: None, }, }; - log_trace!($logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of {} commitment transaction {}, waiting for confirmation (at height {})", + log_trace!($logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of {} commitment transaction {}, event reaches threshold at height {}", &htlc.payment_hash, $commitment_tx, $commitment_tx_type, $commitment_txid_confirmed, entry.confirmation_threshold()); $self.onchain_events_awaiting_threshold_conf.push(entry); @@ -4513,7 +4514,7 @@ impl ChannelMonitorImpl { // event for the same source. self.failed_back_htlc_ids.insert(SentHTLCId::from_source(source)); if let Some(confirmed_txid) = self.funding_spend_confirmed { - // Funding spend already confirmed past ANTI_REORG_DELAY: resolve immediately. + // Funding spend already reached its event threshold: resolve immediately. log_trace!( logger, "Failing HTLC from late counterparty commitment update immediately \ @@ -4549,7 +4550,7 @@ impl ChannelMonitorImpl { log_trace!( logger, "Failing HTLC from late counterparty commitment update, \ - waiting for confirmation (at height {})", + event reaches threshold at height {}", entry.confirmation_threshold() ); self.onchain_events_awaiting_threshold_conf.push(entry); @@ -6403,7 +6404,7 @@ impl ChannelMonitorImpl { commitment_tx_output_idx: Some(input.previous_output.vout), }, }; - log_info!(logger, "Failing HTLC with payment_hash {} timeout by a spend tx, waiting for confirmation (at height {})", &payment_hash, entry.confirmation_threshold()); + log_info!(logger, "Failing HTLC with payment_hash {} timeout by a spend tx, event reaches threshold at height {}", &payment_hash, entry.confirmation_threshold()); self.onchain_events_awaiting_threshold_conf.push(entry); } } From 0b29a408337eba48087523764471f070acc1cfa9 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 30 Apr 2026 16:02:00 +0200 Subject: [PATCH 03/17] lightning: refactor onchain tx handler tests Move repeated OnchainTxHandler setup into shared test helpers so the claim-replay coverage can focus on the behavior under test. --- lightning/src/chain/onchaintx.rs | 111 +++++++++++++++++-------------- 1 file changed, 62 insertions(+), 49 deletions(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 823b81936ce..e559f093922 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -1298,12 +1298,9 @@ mod tests { use super::OnchainTxHandler; - // Test that all claims with locktime equal to or less than the current height are broadcast - // immediately while claims with locktime greater than the current height are only broadcast - // once the locktime is reached. - #[test] - #[rustfmt::skip] - fn test_broadcast_height() { + fn new_test_tx_handler( + channel_type_features: ChannelTypeFeatures, nondust_htlcs: Vec, + ) -> OnchainTxHandler { let secp_ctx = Secp256k1::new(); let signer = InMemorySigner::new( SecretKey::from_slice(&[41; 32]).unwrap(), @@ -1340,9 +1337,6 @@ mod tests { )), }; let funding_outpoint = OutPoint { txid: Txid::all_zeros(), index: u16::MAX }; - - // Use non-anchor channels so that HTLC-Timeouts are broadcast immediately instead of sent - // to the user for external funding. let chan_params = ChannelTransactionParameters { holder_pubkeys: signer.pubkeys(&secp_ctx), holder_selected_contest_delay: 66, @@ -1353,66 +1347,45 @@ mod tests { }), funding_outpoint: Some(funding_outpoint), splice_parent_funding_txid: None, - channel_type_features: ChannelTypeFeatures::only_static_remote_key(), + channel_type_features, channel_value_satoshis: 0, }; - - // Create an OnchainTxHandler for a commitment containing HTLCs with CLTV expiries of 0, 1, - // and 2 blocks. - let mut nondust_htlcs = Vec::new(); - for i in 0..3 { - let preimage = PaymentPreimage([i; 32]); - let hash = PaymentHash(Sha256::hash(&preimage.0[..]).to_byte_array()); - nondust_htlcs.push( - HTLCOutputInCommitment { - offered: true, - amount_msat: 10000, - cltv_expiry: i as u32, - payment_hash: hash, - transaction_output_index: Some(i as u32), - } - ); - } - let holder_commit = HolderCommitmentTransaction::dummy(1000000, funding_outpoint, nondust_htlcs); - let destination_script = ScriptBuf::new(); + let holder_commit = + HolderCommitmentTransaction::dummy(1000000, funding_outpoint, nondust_htlcs); let counterparty_node_id = PublicKey::from_slice(&[2; 33]).unwrap(); - let mut tx_handler = OnchainTxHandler::new( + OnchainTxHandler::new( ChannelId::from_bytes([0; 32]), counterparty_node_id, 1000000, [0; 32], - destination_script.clone(), + ScriptBuf::new(), signer, chan_params, holder_commit, secp_ctx, - ); - - // Create a broadcaster with current block height 1. - let broadcaster = TestBroadcaster::new(Network::Testnet); - { - let mut blocks = broadcaster.blocks.lock().unwrap(); - let genesis_hash = blocks[0].0.block_hash(); - blocks.push((create_dummy_block(genesis_hash, 0, Vec::new()), 1)); - } - - let fee_estimator = TestFeeEstimator::new(253); - let fee_estimator = LowerBoundedFeeEstimator::new(&fee_estimator); - let logger = TestLogger::new(); + ) + } - // Request claiming of each HTLC on the holder's commitment, with current block height 1. + fn build_offered_holder_htlc_requests( + tx_handler: &OnchainTxHandler, + ) -> Vec { let holder_commit = tx_handler.current_holder_commitment_tx(); let holder_commit_txid = holder_commit.trust().txid(); let mut requests = Vec::new(); - for (htlc, counterparty_sig) in holder_commit.nondust_htlcs().iter().zip(holder_commit.counterparty_htlc_sigs.iter()) { + for (htlc, counterparty_sig) in + holder_commit.nondust_htlcs().iter().zip(holder_commit.counterparty_htlc_sigs.iter()) + { requests.push(ClaimRequest::new( holder_commit_txid, htlc.transaction_output_index.unwrap(), - PackageSolvingData::HolderHTLCOutput(HolderHTLCOutput::build(HTLCDescriptor { + PackageSolvingData::HolderHTLCOutput(HolderHTLCOutput::build( + HTLCDescriptor { channel_derivation_parameters: ChannelDerivationParameters { value_satoshis: tx_handler.channel_value_satoshis, keys_id: tx_handler.channel_keys_id, - transaction_parameters: tx_handler.channel_transaction_parameters.clone(), + transaction_parameters: tx_handler + .channel_transaction_parameters + .clone(), }, commitment_txid: holder_commit_txid, per_commitment_number: holder_commit.commitment_number(), @@ -1422,11 +1395,51 @@ mod tests { preimage: None, counterparty_sig: *counterparty_sig, }, - 0 + 0, )), 0, )); } + requests + } + + // Test that all claims with locktime equal to or less than the current height are broadcast + // immediately while claims with locktime greater than the current height are only broadcast + // once the locktime is reached. + #[test] + fn test_broadcast_height() { + // Create an OnchainTxHandler for a commitment containing HTLCs with CLTV expiries of 0, 1, + // and 2 blocks. + let mut nondust_htlcs = Vec::new(); + for i in 0..3 { + let preimage = PaymentPreimage([i; 32]); + let hash = PaymentHash(Sha256::hash(&preimage.0[..]).to_byte_array()); + nondust_htlcs.push(HTLCOutputInCommitment { + offered: true, + amount_msat: 10000, + cltv_expiry: i as u32, + payment_hash: hash, + transaction_output_index: Some(i as u32), + }); + } + let destination_script = ScriptBuf::new(); + let mut tx_handler = + new_test_tx_handler(ChannelTypeFeatures::only_static_remote_key(), nondust_htlcs); + + // Create a broadcaster with current block height 1. + let broadcaster = TestBroadcaster::new(Network::Testnet); + { + let mut blocks = broadcaster.blocks.lock().unwrap(); + let genesis_hash = blocks[0].0.block_hash(); + blocks.push((create_dummy_block(genesis_hash, 0, Vec::new()), 1)); + } + + let fee_estimator = TestFeeEstimator::new(253); + let fee_estimator = LowerBoundedFeeEstimator::new(&fee_estimator); + let logger = TestLogger::new(); + + // Request claiming of each HTLC on the holder's commitment, with current block height 1. + let requests = build_offered_holder_htlc_requests(&tx_handler); tx_handler.update_claims_view_from_requests( requests, 1, From fa9d45e10848386902dc3d730f918a8ad3da0dac Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 4 May 2026 11:10:15 +0200 Subject: [PATCH 04/17] lightning: cover delayed preimage claim balance Add a monitor test for an inbound HTLC claimed by preimage from a holder commitment. Confirm that the claimable balance remains unchanged after the HTLC-success spend reaches anti-reorg finality but before the CSV-delayed output is spendable. --- lightning/src/ln/monitor_tests.rs | 67 +++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index f52f093917b..4fd40df3a45 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -115,6 +115,73 @@ fn test_spendable_output<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, spendable_t } else { panic!(); } } +#[test] +fn preimage_claim_balance_unchanged_between_anti_reorg_and_csv() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let legacy_cfg = test_legacy_channel_config(); + let node_chanmgrs = + create_node_chanmgrs(2, &node_cfgs, &[Some(legacy_cfg.clone()), Some(legacy_cfg)]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let (_, _, chan_id, funding_tx) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000); + // Route an inbound HTLC to node 0 so its preimage claim spends an HTLC output from node 0's + // holder commitment and creates a CSV-delayed output. + let (route, payment_hash, payment_preimage, payment_secret) = + get_route_and_payment_hash!(nodes[1], nodes[0], 12_000_000); + nodes[1].node.send_payment_with_route(route, payment_hash, + RecipientOnionFields::secret_only(payment_secret, 12_000_000), PaymentId(payment_hash.0)).unwrap(); + check_added_monitors(&nodes[1], 1); + let updates = get_htlc_update_msgs(&nodes[1], &nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), &updates.update_add_htlcs[0]); + do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false); + expect_and_process_pending_htlcs(&nodes[0], false); + expect_payment_claimable!(nodes[0], payment_hash, payment_secret, 12_000_000); + + // Confirm node 0's holder commitment before claiming the HTLC so the preimage claim has a + // delayed output that remains tracked as an HTLC balance until it becomes spendable. + let message = "Channel force-closed".to_owned(); + nodes[0].node.force_close_broadcasting_latest_txn(&chan_id, &nodes[1].node.get_our_node_id(), message.clone()).unwrap(); + check_added_monitors(&nodes[0], 1); + check_closed_broadcast(&nodes[0], 1, true); + let reason = ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message }; + check_closed_event(&nodes[0], 1, reason, &[nodes[1].node.get_our_node_id()], 1000000); + let commitment_txn = nodes[0].tx_broadcaster.unique_txn_broadcast(); + assert_eq!(commitment_txn.len(), 1); + check_spends!(commitment_txn[0], funding_tx); + mine_transaction(&nodes[0], &commitment_txn[0]); + nodes[0].tx_broadcaster.clear(); + + // Claiming the HTLC with the preimage broadcasts the HTLC-Success transaction. Once it + // confirms, the resulting delayed output should be reported as an HTLC balance awaiting + // confirmations. + nodes[0].node.claim_funds(payment_preimage); + check_added_monitors(&nodes[0], 1); + expect_payment_claimed!(nodes[0], payment_hash, 12_000_000); + let htlc_claim_txn = nodes[0].tx_broadcaster.unique_txn_broadcast(); + assert_eq!(htlc_claim_txn.len(), 1); + check_spends!(htlc_claim_txn[0], commitment_txn[0]); + mine_transaction(&nodes[0], &htlc_claim_txn[0]); + + let htlc_claim_balances = sorted_vec(nodes[0].chain_monitor.chain_monitor + .get_monitor(chan_id).unwrap().get_claimable_balances()); + assert!(htlc_claim_balances.iter().any(|balance| matches!(balance, + Balance::ClaimableAwaitingConfirmations { + amount_satoshis: 12_000, + source: BalanceSource::Htlc, + .. + } + ))); + + // Advance only to anti-reorg finality for the HTLC-Success transaction. The CSV-delayed + // output is not spendable yet, so the claimable HTLC balance should remain unchanged. + connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1); + assert!(nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty()); + assert_eq!(htlc_claim_balances, sorted_vec(nodes[0].chain_monitor.chain_monitor + .get_monitor(chan_id).unwrap().get_claimable_balances())); +} + #[test] fn revoked_output_htlc_resolution_timing() { // Tests that HTLCs which were present in a broadcasted remote revoked commitment transaction From f2bb8ed583e51f84a7e8e974f195e4da4640e6a8 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 4 May 2026 09:45:31 +0200 Subject: [PATCH 05/17] lightning: resolve HTLC spends at anti-reorg finality Treat HTLCSpendConfirmation entries as irrevocably resolved once the commitment HTLC output spend reaches anti-reorg finality. Do not wait for CSV maturity of any delayed output created by that spend. Delayed outputs remain tracked separately as MaturingOutput entries, keeping claimable balances alive until they are CSV-mature and can be surfaced as SpendableOutputs. --- lightning/src/chain/channelmonitor.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 441defcea0d..2629e45402c 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -496,8 +496,7 @@ impl OnchainEventEntry { // means we can hand it upstream when we see the previous block. conf_threshold = cmp::max(conf_threshold, self.height + descriptor.to_self_delay as u32 - 1); }, - OnchainEvent::FundingSpendConfirmation { on_local_output_csv: Some(csv), .. } | - OnchainEvent::HTLCSpendConfirmation { on_to_local_output_csv: Some(csv), .. } => { + OnchainEvent::FundingSpendConfirmation { on_local_output_csv: Some(csv), .. } => { // A CSV-delayed output is spendable in block (input height) + CSV delay, which // means we can act on the event when we see the previous block. conf_threshold = cmp::max(conf_threshold, self.height + csv as u32 - 1); @@ -567,8 +566,9 @@ enum OnchainEvent { /// If the claim was made by either party with a preimage, this is filled in preimage: Option, /// If the claim was made by us on an inbound HTLC against a local commitment transaction, - /// this records the CSV delay for the delayed output. While present, the event reaches - /// its threshold once the output is spendable. + /// this records the CSV delay for the delayed output. The CSV-mature output remains + /// tracked via the corresponding [`OnchainEvent::MaturingOutput`]; the HTLC spend itself + /// reaches anti-reorg finality. on_to_local_output_csv: Option, }, /// An alternative funding transaction (due to a splice/RBF) has confirmed but can no longer be @@ -1346,9 +1346,10 @@ pub(crate) struct ChannelMonitorImpl { funding_spend_confirmed: Option, confirmed_commitment_tx_counterparty_output: CommitmentTxCounterpartyOutputInfo, - /// The set of HTLCs which have been either claimed or failed on chain and have reached - /// the requisite confirmations on the claim/fail transaction (either ANTI_REORG_DELAY or the - /// spending CSV for revocable outputs). + /// The set of HTLCs whose on-chain claim or fail outcome is irrevocably resolved because the + /// commitment transaction HTLC output spend has reached anti-reorg finality. Any resulting + /// output that is still waiting on CSV maturity is tracked separately as an + /// [`OnchainEvent::MaturingOutput`]. htlcs_resolved_on_chain: Vec, /// When a payment is resolved through an on-chain transaction, we tell the `ChannelManager` @@ -6298,10 +6299,9 @@ impl ChannelMonitorImpl { commitment_tx_output_idx: input.previous_output.vout, preimage: if accepted_preimage_claim || offered_preimage_claim { Some(payment_preimage) } else { None }, - // If this is a payment to us (ie !outbound_htlc), wait for - // the CSV delay before dropping the HTLC from claimable - // balance if the claim was an HTLC-Success transaction (ie - // accepted_preimage_claim). + // If this is a payment to us (ie !outbound_htlc), keep a + // record of the CSV delay. The delayed output is tracked + // separately as a MaturingOutput until it is spendable. on_to_local_output_csv: if accepted_preimage_claim && !outbound_htlc { Some(self.on_holder_tx_csv) } else { None }, }, From 9ac2cc167cabe37b6a8dd50d7d531234409a0649 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 6 May 2026 17:05:12 +0200 Subject: [PATCH 06/17] f: assert delayed output for HTLC spends Check that any HTLCSpendConfirmation carrying a local-output CSV has a matching delayed MaturingOutput. Scan spendable outputs before recording HTLC spend confirmations so the invariant is present when the assertion runs. --- lightning/src/chain/channelmonitor.rs | 32 ++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 2629e45402c..0e536a9a647 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -5727,9 +5727,9 @@ impl ChannelMonitorImpl { break; } } - self.is_resolving_htlc_output(&tx, height, &block_hash, logger); - self.check_tx_and_push_spendable_outputs(&tx, height, &block_hash, logger); + + self.is_resolving_htlc_output(&tx, height, &block_hash, logger); } } @@ -6207,6 +6207,7 @@ impl ChannelMonitorImpl { &mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &WithContext, ) { let funding_spent = get_confirmed_funding_scope!(self); + let txid = tx.compute_txid(); 'outer_loop: for input in &tx.input { let mut payment_data = None; @@ -6293,8 +6294,17 @@ impl ChannelMonitorImpl { if payment_data.is_none() { log_claim!($tx_info, $holder_tx, htlc_output, false); let outbound_htlc = $holder_tx == htlc_output.offered; + let on_to_local_output_csv = if accepted_preimage_claim && !outbound_htlc { + Some(self.on_holder_tx_csv) } else { None }; + #[cfg(debug_assertions)] + if let Some(csv) = on_to_local_output_csv { + debug_assert!( + self.has_delayed_maturing_output_for_tx(txid, csv), + "CSV-delayed HTLC spend confirmation should have a matching MaturingOutput" + ); + } self.onchain_events_awaiting_threshold_conf.push(OnchainEventEntry { - txid: tx.compute_txid(), height, block_hash: Some(*block_hash), transaction: Some(tx.clone()), + txid, height, block_hash: Some(*block_hash), transaction: Some(tx.clone()), event: OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx: input.previous_output.vout, preimage: if accepted_preimage_claim || offered_preimage_claim { @@ -6302,8 +6312,7 @@ impl ChannelMonitorImpl { // If this is a payment to us (ie !outbound_htlc), keep a // record of the CSV delay. The delayed output is tracked // separately as a MaturingOutput until it is spendable. - on_to_local_output_csv: if accepted_preimage_claim && !outbound_htlc { - Some(self.on_holder_tx_csv) } else { None }, + on_to_local_output_csv, }, }); continue 'outer_loop; @@ -6456,6 +6465,19 @@ impl ChannelMonitorImpl { spendable_outputs } + #[cfg(debug_assertions)] + fn has_delayed_maturing_output_for_tx(&self, txid: Txid, csv: u16) -> bool { + self.onchain_events_awaiting_threshold_conf.iter().any(|entry| { + entry.txid == txid + && match &entry.event { + OnchainEvent::MaturingOutput { + descriptor: SpendableOutputDescriptor::DelayedPaymentOutput(descriptor), + } => descriptor.to_self_delay == csv, + _ => false, + } + }) + } + /// Checks if the confirmed transaction is paying funds back to some address we can assume to /// own. #[rustfmt::skip] From f21ad3a3af83379ef2118fbd098ecbaec1f8d81b Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 30 Apr 2026 14:21:04 +0200 Subject: [PATCH 07/17] lightning: dedupe delayed claims by outpoint coverage A replayed holder HTLC claim may arrive as a single-outpoint request after earlier requests were merged into a delayed package. Check whether an existing delayed package already covers the new request instead of requiring exact outpoint-set equality. Add focused OnchainTxHandler coverage and a ChannelMonitor regression through claim_funds for both current anchor variants. --- lightning/src/chain/onchaintx.rs | 95 ++++++++++++++++++++++++- lightning/src/ln/monitor_tests.rs | 113 ++++++++++++++++++++++++++++++ 2 files changed, 205 insertions(+), 3 deletions(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index e559f093922..2cdad08de1e 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -807,9 +807,10 @@ impl OnchainTxHandler { outpoint.txid, outpoint.vout); false } else { - let timelocked_equivalent_package = self.locktimed_packages.iter().map(|v| v.1.iter()).flatten() - .find(|locked_package| locked_package.outpoints().len() == 1 && locked_package.contains_outpoint(outpoint)); - if let Some(package) = timelocked_equivalent_package { + let timelocked_covering_package = self.locktimed_packages.values() + .flat_map(|packages| packages.iter()) + .find(|locked_package| locked_package.contains_outpoint(outpoint)); + if let Some(package) = timelocked_covering_package { log_info!(logger, "Ignoring second claim for outpoint {}:{}, we already have one which we're waiting on a timelock at {} for.", outpoint.txid, outpoint.vout, package.package_locktime(cur_height)); false @@ -1480,4 +1481,92 @@ mod tests { assert_eq!(txs_broadcasted.len(), 1); assert_eq!(txs_broadcasted[0].lock_time.to_consensus_u32(), 2); } + + #[test] + fn test_duplicate_pending_claim_request_after_force_close_replay() { + let claim_height = 21; + let locktime = 42; + let mut nondust_htlcs = Vec::new(); + for i in 0..2 { + let preimage = PaymentPreimage([i + 1; 32]); + let hash = PaymentHash(Sha256::hash(&preimage.0[..]).to_byte_array()); + nondust_htlcs.push(HTLCOutputInCommitment { + offered: true, + amount_msat: 10000, + cltv_expiry: locktime, + payment_hash: hash, + transaction_output_index: Some(i as u32), + }); + } + + let mut tx_handler = new_test_tx_handler( + ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), + nondust_htlcs, + ); + let requests = build_offered_holder_htlc_requests(&tx_handler); + let destination_script = ScriptBuf::new(); + let broadcaster = TestBroadcaster::new(Network::Testnet); + let fee_estimator = TestFeeEstimator::new(253); + let fee_estimator = LowerBoundedFeeEstimator::new(&fee_estimator); + let logger = TestLogger::new(); + + // Simulate the force-close path registering the two holder HTLC claims as + // a single delayed package. + tx_handler.update_claims_view_from_requests( + requests.clone(), + claim_height, + claim_height, + &&broadcaster, + ConfirmationTarget::UrgentOnChainSweep, + &destination_script, + &fee_estimator, + &logger, + ); + assert_eq!( + tx_handler.locktimed_packages.get(&locktime).map(|packages| packages.len()), + Some(1), + ); + + // Replaying the same per-HTLC claim requests must match by outpoint + // coverage, otherwise each single-outpoint request would be added again. + tx_handler.update_claims_view_from_requests( + requests, + claim_height, + claim_height, + &&broadcaster, + ConfirmationTarget::UrgentOnChainSweep, + &destination_script, + &fee_estimator, + &logger, + ); + assert_eq!( + tx_handler.locktimed_packages.get(&locktime).map(|packages| packages.len()), + Some(1), + ); + + // At locktime, the delayed package should still yield one bump event + // covering both HTLCs. + tx_handler.update_claims_view_from_requests( + Vec::new(), + locktime, + locktime, + &&broadcaster, + ConfirmationTarget::UrgentOnChainSweep, + &destination_script, + &fee_estimator, + &logger, + ); + + let pending_events = tx_handler.get_and_clear_pending_claim_events(); + assert_eq!(pending_events.len(), 1); + assert_eq!(tx_handler.pending_claim_requests.len(), 1); + assert_eq!(tx_handler.claimable_outpoints.len(), 2); + match &pending_events[0].1 { + super::ClaimEvent::BumpHTLC { htlcs, tx_lock_time, .. } => { + assert_eq!(htlcs.len(), 2); + assert_eq!(tx_lock_time.to_consensus_u32(), locktime); + }, + _ => panic!("expected a single HTLC bump event"), + } + } } diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 4fd40df3a45..436bb01c907 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -2455,6 +2455,119 @@ fn test_restored_packages_retry() { do_test_restored_packages_retry(true); } +fn do_test_duplicate_delayed_holder_htlc_claims_after_claim_funds_replay(p2a_anchor: bool) { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut anchors_config = test_default_channel_config(); + anchors_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + anchors_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = p2a_anchor; + let node_chanmgrs = + create_node_chanmgrs(2, &node_cfgs, &[Some(anchors_config.clone()), Some(anchors_config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let coinbase_tx = provide_anchor_reserves(&nodes); + let (_, _, chan_id, funding_tx) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 50_000_000); + + // Seed two unresolved outbound HTLCs that will be aggregated into one + // delayed holder-commitment package after force close. + route_payment(&nodes[0], &[&nodes[1]], 10_000_000); + route_payment(&nodes[0], &[&nodes[1]], 11_000_000); + + // Add a third incoming HTLC which will later be claimed by preimage after + // the commitment transaction confirms, reproducing the replay path. + let (claim_route, claim_hash, claim_preimage, claim_secret) = + get_route_and_payment_hash!(nodes[1], nodes[0], 12_000_000); + nodes[1] + .node + .send_payment_with_route( + claim_route, + claim_hash, + RecipientOnionFields::secret_only(claim_secret, 12_000_000), + PaymentId(claim_hash.0), + ) + .unwrap(); + check_added_monitors(&nodes[1], 1); + let updates = get_htlc_update_msgs(&nodes[1], &nodes[0].node.get_our_node_id()); + nodes[0] + .node + .handle_update_add_htlc(nodes[1].node.get_our_node_id(), &updates.update_add_htlcs[0]); + do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false); + expect_and_process_pending_htlcs(&nodes[0], false); + expect_payment_claimable!(nodes[0], claim_hash, claim_secret, 12_000_000); + + // Force-close node 0 so its holder commitment hits chain and its HTLC + // claims are fed into OnchainTxHandler as delayed requests. + let message = "Channel force-closed".to_owned(); + nodes[0] + .node + .force_close_broadcasting_latest_txn( + &chan_id, + &nodes[1].node.get_our_node_id(), + message.clone(), + ) + .unwrap(); + check_added_monitors(&nodes[0], 1); + check_closed_broadcast(&nodes[0], 1, true); + let reason = ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message }; + check_closed_event(&nodes[0], 1, reason, &[nodes[1].node.get_our_node_id()], 1_000_000); + handle_bump_close_event(&nodes[0]); + + let (commitment_tx, anchor_tx) = { + let mut txn = nodes[0].tx_broadcaster.unique_txn_broadcast(); + assert_eq!(txn.len(), if p2a_anchor { 2 } else { 1 }); + let anchor_tx = p2a_anchor.then(|| txn.pop().unwrap()); + let commitment_tx = txn.pop().unwrap(); + check_spends!(commitment_tx, funding_tx); + if p2a_anchor { + check_spends!(anchor_tx.as_ref().unwrap(), commitment_tx, coinbase_tx); + } + (commitment_tx, anchor_tx) + }; + + let _ = mine_transaction(&nodes[0], &commitment_tx); + if p2a_anchor { + let _ = mine_transaction(&nodes[0], anchor_tx.as_ref().unwrap()); + } + + // Claim the incoming HTLC after the commitment is confirmed. This + // regenerates a single-outpoint claim request alongside the existing + // delayed package covering the two earlier HTLCs. + nodes[0].node.claim_funds(claim_preimage); + check_added_monitors(&nodes[0], 1); + expect_payment_claimed!(nodes[0], claim_hash, 12_000_000); + + // Once all holder HTLCs reach their timelock, we should see the original two-HTLC + // delayed package plus the replayed single-HTLC claim, not duplicates of + // the delayed package's outpoints. + connect_blocks(&nodes[0], TEST_FINAL_CLTV + 1); + + let mut htlc_event_sizes = nodes[0] + .chain_monitor + .chain_monitor + .get_and_clear_pending_events() + .into_iter() + .filter_map(|event| { + if let Event::BumpTransaction(BumpTransactionEvent::HTLCResolution { + htlc_descriptors, .. + }) = event + { + Some(htlc_descriptors.len()) + } else { + None + } + }) + .collect::>(); + htlc_event_sizes.sort_unstable(); + assert_eq!(htlc_event_sizes, vec![1, 2]); +} + +#[test] +fn test_duplicate_delayed_holder_htlc_claims_after_claim_funds_replay() { + do_test_duplicate_delayed_holder_htlc_claims_after_claim_funds_replay(false); + do_test_duplicate_delayed_holder_htlc_claims_after_claim_funds_replay(true); +} + fn do_test_monitor_rebroadcast_pending_claims(keyed_anchors: bool, p2a_anchor: bool) { // Test that we will retry broadcasting pending claims for a force-closed channel on every // `ChainMonitor::rebroadcast_pending_claims` call. From 0bb4d8782f8e53358bca3d8ef612abc623f22e5d Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 30 Apr 2026 14:23:24 +0200 Subject: [PATCH 08/17] lightning: ignore claims for pending spent outpoints When a transaction spends one outpoint from a delayed package, the split outpoint is tracked as a ContentiousOutpoint until the spend reaches anti-reorg finality. Reject replayed claim requests for those pending-spent outpoints so they are not added back before the spend reaches anti-reorg finality or reorgs out. Add an OnchainTxHandler regression that replays a holder claim during that pending-spent window and verifies reorg resurrection still works. --- lightning/src/chain/onchaintx.rs | 140 ++++++++++++++++++++++++++++++- 1 file changed, 137 insertions(+), 3 deletions(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 2cdad08de1e..1a5ec61b0c7 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -576,6 +576,16 @@ impl OnchainTxHandler { self.pending_claim_requests.len() != 0 } + fn is_outpoint_spend_waiting_threshold_conf(&self, outpoint: &BitcoinOutPoint) -> bool { + self.onchain_events_awaiting_threshold_conf.iter().any(|entry| { + if let OnchainEvent::ContentiousOutpoint { ref package } = entry.event { + package.contains_outpoint(outpoint) + } else { + false + } + }) + } + /// Lightning security model (i.e being able to redeem/timeout HTLC or penalize counterparty /// onchain) lays on the assumption of claim transactions getting confirmed before timelock /// expiration (CSV or CLTV following cases). In case of high-fee spikes, claim tx may get stuck @@ -802,7 +812,15 @@ impl OnchainTxHandler { // First drop any duplicate claims. requests.retain(|req| { let outpoint = req.outpoint(); - if self.claimable_outpoints.get(outpoint).is_some() { + if self.is_outpoint_spend_waiting_threshold_conf(outpoint) { + // This is a package-layer guard. ChannelMonitor filters regenerated + // HTLC claims using HTLC resolution state, while this keeps outpoints + // split from an existing package from being re-added during the reorg + // window. + log_info!(logger, "Ignoring claim for outpoint {}:{}, it is already spent by a transaction awaiting anti-reorg finality", + outpoint.txid, outpoint.vout); + false + } else if self.claimable_outpoints.get(outpoint).is_some() { log_info!(logger, "Ignoring second claim for outpoint {}:{}, already registered its claiming request", outpoint.txid, outpoint.vout); false @@ -1276,11 +1294,14 @@ impl OnchainTxHandler { #[cfg(test)] mod tests { - use bitcoin::hash_types::Txid; + use bitcoin::hash_types::{BlockHash, Txid}; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; + use bitcoin::locktime::absolute::LockTime; + use bitcoin::transaction::{OutPoint as BitcoinOutPoint, Version}; use bitcoin::Network; - use bitcoin::{key::Secp256k1, secp256k1::PublicKey, secp256k1::SecretKey, ScriptBuf}; + use bitcoin::{key::Secp256k1, secp256k1::PublicKey, secp256k1::SecretKey}; + use bitcoin::{Amount, ScriptBuf, Transaction, TxIn, TxOut}; use types::features::ChannelTypeFeatures; use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator}; @@ -1404,6 +1425,18 @@ mod tests { requests } + fn locked_outpoints( + tx_handler: &OnchainTxHandler, locktime: u32, + ) -> Vec { + tx_handler + .locktimed_packages + .get(&locktime) + .into_iter() + .flat_map(|packages| packages.iter()) + .flat_map(|package| package.outpoints().into_iter().map(|outpoint| *outpoint)) + .collect() + } + // Test that all claims with locktime equal to or less than the current height are broadcast // immediately while claims with locktime greater than the current height are only broadcast // once the locktime is reached. @@ -1569,4 +1602,105 @@ mod tests { _ => panic!("expected a single HTLC bump event"), } } + + #[test] + fn test_replayed_claim_ignored_for_pending_spent_outpoint() { + let claim_height = 21; + let spend_height = 22; + let locktime = 42; + let mut nondust_htlcs = Vec::new(); + for i in 0..2 { + let preimage = PaymentPreimage([i + 1; 32]); + let hash = PaymentHash(Sha256::hash(&preimage.0[..]).to_byte_array()); + nondust_htlcs.push(HTLCOutputInCommitment { + offered: true, + amount_msat: 10000, + cltv_expiry: locktime, + payment_hash: hash, + transaction_output_index: Some(i as u32), + }); + } + + let mut tx_handler = new_test_tx_handler( + ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), + nondust_htlcs, + ); + let requests = build_offered_holder_htlc_requests(&tx_handler); + let spent_outpoint = *requests[0].outpoint(); + let still_delayed_outpoint = *requests[1].outpoint(); + let destination_script = ScriptBuf::new(); + let broadcaster = TestBroadcaster::new(Network::Testnet); + let fee_estimator = TestFeeEstimator::new(253); + let fee_estimator = LowerBoundedFeeEstimator::new(&fee_estimator); + let logger = TestLogger::new(); + + // Register both holder HTLC claims as one delayed package before any + // individual outpoint spends are observed. + tx_handler.update_claims_view_from_requests( + requests.clone(), + claim_height, + claim_height, + &&broadcaster, + ConfirmationTarget::UrgentOnChainSweep, + &destination_script, + &fee_estimator, + &logger, + ); + assert_eq!(locked_outpoints(&tx_handler, locktime).len(), 2); + + // Spend one outpoint before the package reaches its timelock. The handler + // should split it into a ContentiousOutpoint until the spend reaches + // anti-reorg finality. + let spend_tx = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { previous_output: spent_outpoint, ..Default::default() }], + output: vec![TxOut { value: Amount::from_sat(1000), script_pubkey: ScriptBuf::new() }], + }; + tx_handler.update_claims_view_from_matched_txn( + &[&spend_tx], + spend_height, + BlockHash::all_zeros(), + spend_height, + &&broadcaster, + ConfirmationTarget::UrgentOnChainSweep, + &destination_script, + &fee_estimator, + &logger, + ); + let locked = locked_outpoints(&tx_handler, locktime); + assert_eq!(locked, vec![still_delayed_outpoint]); + + // Replaying both original claim requests during that window must not + // re-add the already-spent outpoint to the delayed package. + tx_handler.update_claims_view_from_requests( + requests, + spend_height, + spend_height, + &&broadcaster, + ConfirmationTarget::UrgentOnChainSweep, + &destination_script, + &fee_estimator, + &logger, + ); + let locked = locked_outpoints(&tx_handler, locktime); + assert_eq!(locked, vec![still_delayed_outpoint]); + assert!(tx_handler.pending_claim_requests.is_empty()); + assert!(tx_handler.claimable_outpoints.is_empty()); + + // If the spend reorgs out, the contentious outpoint is resurrected into + // the delayed package. + tx_handler.blocks_disconnected( + spend_height - 1, + &&broadcaster, + ConfirmationTarget::UrgentOnChainSweep, + &destination_script, + &fee_estimator, + &logger, + ); + let locked = locked_outpoints(&tx_handler, locktime); + assert_eq!(locked.len(), 2); + assert!(locked.contains(&spent_outpoint)); + assert!(locked.contains(&still_delayed_outpoint)); + } } From 3179a2e1f6a35d63fcf34823774d2f54bbd4e71b Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 6 May 2026 17:57:23 +0200 Subject: [PATCH 09/17] f: fold timelocked outpoint claim check Classify duplicate outpoint state in one helper. Preserve existing filter ordering and timelock logging. --- lightning/src/chain/onchaintx.rs | 73 +++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 1a5ec61b0c7..e4d23d77e88 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -91,6 +91,12 @@ enum OnchainEvent { ContentiousOutpoint { package: PackageTemplate }, } +enum OutpointClaimState { + WaitingThresholdConf, + ClaimingRequestRegistered, + WaitingTimelock(u32), +} + impl Writeable for OnchainEventEntry { fn write(&self, writer: &mut W) -> Result<(), io::Error> { write_tlv_fields!(writer, { @@ -576,14 +582,30 @@ impl OnchainTxHandler { self.pending_claim_requests.len() != 0 } - fn is_outpoint_spend_waiting_threshold_conf(&self, outpoint: &BitcoinOutPoint) -> bool { - self.onchain_events_awaiting_threshold_conf.iter().any(|entry| { + fn outpoint_claim_state( + &self, outpoint: &BitcoinOutPoint, cur_height: u32, + ) -> Option { + if self.onchain_events_awaiting_threshold_conf.iter().any(|entry| { if let OnchainEvent::ContentiousOutpoint { ref package } = entry.event { package.contains_outpoint(outpoint) } else { false } - }) + }) { + return Some(OutpointClaimState::WaitingThresholdConf); + } + + if self.claimable_outpoints.get(outpoint).is_some() { + return Some(OutpointClaimState::ClaimingRequestRegistered); + } + + self.locktimed_packages + .values() + .flat_map(|packages| packages.iter()) + .find(|locked_package| locked_package.contains_outpoint(outpoint)) + .map(|package| { + OutpointClaimState::WaitingTimelock(package.package_locktime(cur_height)) + }) } /// Lightning security model (i.e being able to redeem/timeout HTLC or penalize counterparty @@ -812,29 +834,30 @@ impl OnchainTxHandler { // First drop any duplicate claims. requests.retain(|req| { let outpoint = req.outpoint(); - if self.is_outpoint_spend_waiting_threshold_conf(outpoint) { - // This is a package-layer guard. ChannelMonitor filters regenerated - // HTLC claims using HTLC resolution state, while this keeps outpoints - // split from an existing package from being re-added during the reorg - // window. - log_info!(logger, "Ignoring claim for outpoint {}:{}, it is already spent by a transaction awaiting anti-reorg finality", - outpoint.txid, outpoint.vout); - false - } else if self.claimable_outpoints.get(outpoint).is_some() { - log_info!(logger, "Ignoring second claim for outpoint {}:{}, already registered its claiming request", - outpoint.txid, outpoint.vout); - false - } else { - let timelocked_covering_package = self.locktimed_packages.values() - .flat_map(|packages| packages.iter()) - .find(|locked_package| locked_package.contains_outpoint(outpoint)); - if let Some(package) = timelocked_covering_package { - log_info!(logger, "Ignoring second claim for outpoint {}:{}, we already have one which we're waiting on a timelock at {} for.", - outpoint.txid, outpoint.vout, package.package_locktime(cur_height)); - false - } else { - true + if let Some(claim_state) = self.outpoint_claim_state(outpoint, cur_height) { + match claim_state { + OutpointClaimState::WaitingThresholdConf => { + // This is a package-layer guard. ChannelMonitor filters regenerated + // HTLC claims using HTLC resolution state, while this keeps outpoints + // split from an existing package from being re-added during the reorg + // window. + log_info!(logger, "Ignoring claim for outpoint {}:{}, it is already spent by a transaction awaiting anti-reorg finality", + outpoint.txid, outpoint.vout); + false + }, + OutpointClaimState::ClaimingRequestRegistered => { + log_info!(logger, "Ignoring second claim for outpoint {}:{}, already registered its claiming request", + outpoint.txid, outpoint.vout); + false + }, + OutpointClaimState::WaitingTimelock(locktime) => { + log_info!(logger, "Ignoring second claim for outpoint {}:{}, we already have one which we're waiting on a timelock at {} for.", + outpoint.txid, outpoint.vout, locktime); + false + }, } + } else { + true } }); let mut requests = requests.into_iter() From 551ed617adba4bbf9d157b8b74063af54b4e9235 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 30 Apr 2026 14:29:33 +0200 Subject: [PATCH 10/17] lightning: skip resolved HTLC claim replays Filter regenerated HTLC claim requests once ChannelMonitor has persisted anti-reorg finality for the commitment HTLC output spend. This keeps replayed preimage updates from recreating claims after OnchainTxHandler has cleaned up its active retry state, relying on the monitor's persisted HTLC resolution state. --- lightning/src/chain/channelmonitor.rs | 25 +++++++- lightning/src/ln/monitor_tests.rs | 84 ++++++++++++++++++++++++++- 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 0e536a9a647..085219adcb0 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -4985,7 +4985,10 @@ impl ChannelMonitorImpl { .iter() .filter_map(|(htlc, _)| { if let Some(transaction_output_index) = htlc.transaction_output_index { - if htlc.offered && htlc.payment_hash == matching_payment_hash { + if htlc.offered + && htlc.payment_hash == matching_payment_hash + && !self.is_htlc_output_resolved_on_chain(htlc) + { let htlc_data = PackageSolvingData::CounterpartyOfferedHTLCOutput( CounterpartyOfferedHTLCOutput::build( per_commitment_point, @@ -5011,6 +5014,20 @@ impl ChannelMonitorImpl { .collect() } + fn is_htlc_output_resolved_on_chain(&self, htlc: &HTLCOutputInCommitment) -> bool { + if let Some(transaction_output_index) = htlc.transaction_output_index { + // Only suppress claims once the commitment HTLC output spend has + // reached anti-reorg finality. Any output created by that spend may + // still be CSV-delayed, but the original HTLC outpoint should not be + // re-claimed. + self.htlcs_resolved_on_chain.iter().any(|resolved_htlc| { + resolved_htlc.commitment_tx_output_idx == Some(transaction_output_index) + }) + } else { + false + } + } + /// Returns the HTLC claim requests and the counterparty output info. fn get_counterparty_output_claim_info( &self, funding_spent: &FundingScope, commitment_number: u64, commitment_txid: Txid, @@ -5058,6 +5075,9 @@ impl ChannelMonitorImpl { // per_commitment_data is corrupt or our commitment signing key leaked! return (claimable_outpoints, to_counterparty_output_info); } + if self.is_htlc_output_resolved_on_chain(htlc) { + continue; + } let preimage = if htlc.offered { if let Some((p, _)) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) @@ -5159,6 +5179,9 @@ impl ChannelMonitorImpl { let mut htlcs = Vec::with_capacity(holder_tx.nondust_htlcs().len()); debug_assert_eq!(holder_tx.nondust_htlcs().len(), holder_tx.counterparty_htlc_sigs.len()); for (htlc, counterparty_sig) in holder_tx.nondust_htlcs().iter().zip(holder_tx.counterparty_htlc_sigs.iter()) { + if self.is_htlc_output_resolved_on_chain(htlc) { + continue; + } assert!(htlc.transaction_output_index.is_some(), "Expected transaction output index for non-dust HTLC"); let preimage = if htlc.offered { diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 436bb01c907..111d1fbfd81 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -2458,12 +2458,15 @@ fn test_restored_packages_retry() { fn do_test_duplicate_delayed_holder_htlc_claims_after_claim_funds_replay(p2a_anchor: bool) { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let persister; + let new_chain_monitor; + let node_deserialized; let mut anchors_config = test_default_channel_config(); anchors_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; anchors_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = p2a_anchor; let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(anchors_config.clone()), Some(anchors_config)]); - let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let coinbase_tx = provide_anchor_reserves(&nodes); let (_, _, chan_id, funding_tx) = @@ -2542,11 +2545,14 @@ fn do_test_duplicate_delayed_holder_htlc_claims_after_claim_funds_replay(p2a_anc // the delayed package's outpoints. connect_blocks(&nodes[0], TEST_FINAL_CLTV + 1); - let mut htlc_event_sizes = nodes[0] + let events = nodes[0] .chain_monitor .chain_monitor .get_and_clear_pending_events() .into_iter() + .collect::>(); + let mut htlc_event_sizes = events + .iter() .filter_map(|event| { if let Event::BumpTransaction(BumpTransactionEvent::HTLCResolution { htlc_descriptors, .. @@ -2560,6 +2566,80 @@ fn do_test_duplicate_delayed_holder_htlc_claims_after_claim_funds_replay(p2a_anc .collect::>(); htlc_event_sizes.sort_unstable(); assert_eq!(htlc_event_sizes, vec![1, 2]); + + // Drive only the replayed single-HTLC event on-chain so we can replay the + // preimage once the spend is anti-reorg final, then again after reload. + for event in events { + if let Event::BumpTransaction(event) = event { + let is_single_htlc = if let BumpTransactionEvent::HTLCResolution { + ref htlc_descriptors, + .. + } = event + { + htlc_descriptors.len() == 1 + } else { + false + }; + if is_single_htlc { + nodes[0].bump_tx_handler.handle_event(&event); + break; + } + } + } + let mut htlc_txn = nodes[0].tx_broadcaster.unique_txn_broadcast(); + assert_eq!(htlc_txn.len(), 1); + let htlc_tx = htlc_txn.pop().unwrap(); + mine_transaction(&nodes[0], &htlc_tx); + connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1); + assert!(nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty()); + + // The spend has passed anti-reorg finality, but its CSV-delayed output is + // not yet spendable. Replaying the preimage in this window must not create + // a new conflicting claim for the already-spent commitment HTLC output. + get_monitor!(nodes[0], chan_id).provide_payment_preimage_unsafe_legacy( + &claim_hash, + &claim_preimage, + &node_cfgs[0].tx_broadcaster, + &LowerBoundedFeeEstimator::new(node_cfgs[0].fee_estimator), + &nodes[0].logger, + ); + assert!(nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty()); + let balances = nodes[0] + .chain_monitor + .chain_monitor + .get_monitor(chan_id) + .unwrap() + .get_claimable_balances(); + assert!(balances.iter().any(|balance| matches!( + balance, + Balance::ClaimableAwaitingConfirmations { + amount_satoshis: 12_000, + source: BalanceSource::Htlc, + .. + } + ))); + + connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - ANTI_REORG_DELAY); + let _ = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events(); + + // Reload before replaying the preimage so the regression covers persisted + // resolution state, not only in-memory filtering. + let serialized_channel_manager = nodes[0].node.encode(); + let serialized_monitor = get_monitor!(nodes[0], chan_id).encode(); + reload_node!( + nodes[0], &serialized_channel_manager, &[&serialized_monitor], persister, + new_chain_monitor, node_deserialized + ); + + // Replaying the preimage update must not regenerate a claim for the HTLC + // whose commitment output has anti-reorg persisted resolution state. + get_monitor!(nodes[0], chan_id).provide_payment_preimage_unsafe_legacy( + &claim_hash, &claim_preimage, &node_cfgs[0].tx_broadcaster, + &LowerBoundedFeeEstimator::new(node_cfgs[0].fee_estimator), &nodes[0].logger, + ); + assert!(nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty()); + expect_payment_claimed!(nodes[0], claim_hash, 12_000_000); + check_added_monitors(&nodes[0], 1); } #[test] From 1be1e8e6a3bd66bcf64262f8195edba0aeafa71a Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 6 May 2026 17:20:10 +0200 Subject: [PATCH 11/17] f: log resolved HTLC preimage losses Log when a replayed preimage claim is skipped because the HTLC output reached anti-reorg finality without that preimage. --- lightning/src/chain/channelmonitor.rs | 78 ++++++++++++++++++++++----- 1 file changed, 64 insertions(+), 14 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 085219adcb0..f1a7a253336 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -3813,7 +3813,7 @@ impl ChannelMonitorImpl { // First check if a counterparty commitment transaction has been broadcasted: macro_rules! claim_htlcs { ($commitment_number: expr, $txid: expr, $htlcs: expr) => { - let htlc_claim_reqs = self.get_counterparty_output_claims_for_preimage(*payment_preimage, funding_spent, $commitment_number, $txid, $htlcs, confirmed_spend_height); + let htlc_claim_reqs = self.get_counterparty_output_claims_for_preimage(*payment_preimage, funding_spent, $commitment_number, $txid, $htlcs, confirmed_spend_height, logger); let conf_target = self.closure_conf_target(); self.onchain_tx_handler.update_claims_view_from_requests( htlc_claim_reqs, self.best_block.height, self.best_block.height, broadcaster, @@ -3862,6 +3862,9 @@ impl ChannelMonitorImpl { None }; if let Some(holder_commitment_tx) = holder_commitment_tx { + self.log_holder_preimage_claim_after_htlc_resolved_on_chain( + logger, holder_commitment_tx, *payment_preimage, + ); // Assume that the broadcasted commitment transaction confirmed in the current best // block. Even if not, its a reasonable metric for the bump criteria on the HTLC // transactions. @@ -4965,11 +4968,11 @@ impl ChannelMonitorImpl { } } - fn get_counterparty_output_claims_for_preimage( + fn get_counterparty_output_claims_for_preimage( &self, preimage: PaymentPreimage, funding_spent: &FundingScope, commitment_number: u64, commitment_txid: Txid, per_commitment_option: Option<&Vec<(HTLCOutputInCommitment, Option>)>>, - confirmation_height: Option, + confirmation_height: Option, logger: &L, ) -> Vec { let per_commitment_claimable_data = match per_commitment_option { Some(outputs) => outputs, @@ -4985,10 +4988,17 @@ impl ChannelMonitorImpl { .iter() .filter_map(|(htlc, _)| { if let Some(transaction_output_index) = htlc.transaction_output_index { - if htlc.offered - && htlc.payment_hash == matching_payment_hash - && !self.is_htlc_output_resolved_on_chain(htlc) - { + if htlc.offered && htlc.payment_hash == matching_payment_hash { + if let Some(resolved_htlc) = self.htlc_output_resolution_on_chain(htlc) { + self.log_preimage_claim_after_htlc_resolved_on_chain( + logger, + commitment_txid, + htlc, + preimage, + resolved_htlc, + ); + return None; + } let htlc_data = PackageSolvingData::CounterpartyOfferedHTLCOutput( CounterpartyOfferedHTLCOutput::build( per_commitment_point, @@ -5014,17 +5024,57 @@ impl ChannelMonitorImpl { .collect() } - fn is_htlc_output_resolved_on_chain(&self, htlc: &HTLCOutputInCommitment) -> bool { - if let Some(transaction_output_index) = htlc.transaction_output_index { + fn htlc_output_resolution_on_chain( + &self, htlc: &HTLCOutputInCommitment, + ) -> Option<&IrrevocablyResolvedHTLC> { + htlc.transaction_output_index.and_then(|transaction_output_index| { // Only suppress claims once the commitment HTLC output spend has // reached anti-reorg finality. Any output created by that spend may // still be CSV-delayed, but the original HTLC outpoint should not be // re-claimed. - self.htlcs_resolved_on_chain.iter().any(|resolved_htlc| { + self.htlcs_resolved_on_chain.iter().find(|resolved_htlc| { resolved_htlc.commitment_tx_output_idx == Some(transaction_output_index) }) - } else { - false + }) + } + + fn log_preimage_claim_after_htlc_resolved_on_chain( + &self, logger: &L, commitment_txid: Txid, htlc: &HTLCOutputInCommitment, + preimage: PaymentPreimage, resolved_htlc: &IrrevocablyResolvedHTLC, + ) { + if resolved_htlc.payment_preimage == Some(preimage) { + return; + } + if let Some(transaction_output_index) = htlc.transaction_output_index { + let logger = WithContext::from(logger, None, None, Some(htlc.payment_hash)); + if let Some(resolving_txid) = resolved_htlc.resolving_txid.as_ref() { + log_error!(logger, "WE HAVE LIKELY LOST FUNDS: HTLC output {}:{} was irrevocably resolved on-chain by transaction {} without the payment preimage we now know; not replaying the claim", + commitment_txid, transaction_output_index, resolving_txid); + } else { + log_error!(logger, "WE HAVE LIKELY LOST FUNDS: HTLC output {}:{} was irrevocably resolved on-chain by an unknown transaction without the payment preimage we now know; not replaying the claim", + commitment_txid, transaction_output_index); + } + } + } + + fn log_holder_preimage_claim_after_htlc_resolved_on_chain( + &self, logger: &L, holder_tx: &HolderCommitmentTransaction, preimage: PaymentPreimage, + ) { + let matching_payment_hash = PaymentHash::from(preimage); + let tx = holder_tx.trust(); + for htlc in holder_tx.nondust_htlcs() { + if htlc.offered || htlc.payment_hash != matching_payment_hash { + continue; + } + if let Some(resolved_htlc) = self.htlc_output_resolution_on_chain(htlc) { + self.log_preimage_claim_after_htlc_resolved_on_chain( + logger, + tx.txid(), + htlc, + preimage, + resolved_htlc, + ); + } } } @@ -5075,7 +5125,7 @@ impl ChannelMonitorImpl { // per_commitment_data is corrupt or our commitment signing key leaked! return (claimable_outpoints, to_counterparty_output_info); } - if self.is_htlc_output_resolved_on_chain(htlc) { + if self.htlc_output_resolution_on_chain(htlc).is_some() { continue; } let preimage = if htlc.offered { @@ -5179,7 +5229,7 @@ impl ChannelMonitorImpl { let mut htlcs = Vec::with_capacity(holder_tx.nondust_htlcs().len()); debug_assert_eq!(holder_tx.nondust_htlcs().len(), holder_tx.counterparty_htlc_sigs.len()); for (htlc, counterparty_sig) in holder_tx.nondust_htlcs().iter().zip(holder_tx.counterparty_htlc_sigs.iter()) { - if self.is_htlc_output_resolved_on_chain(htlc) { + if self.htlc_output_resolution_on_chain(htlc).is_some() { continue; } assert!(htlc.transaction_output_index.is_some(), "Expected transaction output index for non-dust HTLC"); From 4be7d9782060ffd58038b58957fc63f58c36411a Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 30 Apr 2026 14:33:27 +0200 Subject: [PATCH 12/17] lightning: canonicalize htlc claim ids Hash HTLC claim outpoints in canonical order so the same logical HTLC set produces the same ClaimId regardless of descriptor order. Add a unit test covering reversed descriptor order. --- lightning/src/chain/mod.rs | 64 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/lightning/src/chain/mod.rs b/lightning/src/chain/mod.rs index 72006f78205..47901b80e16 100644 --- a/lightning/src/chain/mod.rs +++ b/lightning/src/chain/mod.rs @@ -563,10 +563,18 @@ pub struct ClaimId(pub [u8; 32]); impl ClaimId { pub(crate) fn from_htlcs(htlcs: &[HTLCDescriptor]) -> ClaimId { + let mut htlc_outpoints = htlcs + .iter() + .map(|htlc| { + (htlc.commitment_txid.to_byte_array(), htlc.htlc.transaction_output_index.unwrap()) + }) + .collect::>(); + htlc_outpoints.sort_unstable(); + let mut engine = Sha256::engine(); - for htlc in htlcs { - engine.input(&htlc.commitment_txid.to_byte_array()); - engine.input(&htlc.htlc.transaction_output_index.unwrap().to_be_bytes()); + for (commitment_txid, transaction_output_index) in htlc_outpoints { + engine.input(&commitment_txid); + engine.input(&transaction_output_index.to_be_bytes()); } ClaimId(Sha256::from_engine(engine).to_byte_array()) } @@ -581,8 +589,45 @@ impl ClaimId { #[cfg(test)] mod tests { use super::*; + use crate::ln::chan_utils::{ + ChannelTransactionParameters, HTLCOutputInCommitment, HolderCommitmentTransaction, + }; + use crate::sign::ChannelDerivationParameters; + use crate::types::payment::{PaymentHash, PaymentPreimage}; use bitcoin::hashes::Hash; + fn dummy_htlc_descriptor( + commitment_txid: Txid, transaction_output_index: u32, + ) -> HTLCDescriptor { + let channel_parameters = ChannelTransactionParameters::test_dummy(100_000); + let htlc = HTLCOutputInCommitment { + offered: true, + amount_msat: 1000, + cltv_expiry: 100, + payment_hash: PaymentHash::from(PaymentPreimage([1; 32])), + transaction_output_index: Some(transaction_output_index), + }; + let funding_outpoint = channel_parameters.funding_outpoint.unwrap(); + let commitment_tx = + HolderCommitmentTransaction::dummy(100_000, funding_outpoint, vec![htlc.clone()]); + let trusted_tx = commitment_tx.trust(); + + HTLCDescriptor { + channel_derivation_parameters: ChannelDerivationParameters { + value_satoshis: channel_parameters.channel_value_satoshis, + keys_id: [1; 32], + transaction_parameters: channel_parameters, + }, + commitment_txid, + per_commitment_number: trusted_tx.commitment_number(), + per_commitment_point: trusted_tx.per_commitment_point(), + feerate_per_kw: trusted_tx.negotiated_feerate_per_kw(), + htlc, + preimage: None, + counterparty_sig: commitment_tx.counterparty_htlc_sigs[0], + } + } + #[test] fn test_best_block() { let hash1 = BlockHash::from_slice(&[1; 32]).unwrap(); @@ -618,4 +663,17 @@ mod tests { let chain_c = BlockLocator::new(hash_other, 200); assert_eq!(chain_a.find_common_ancestor(&chain_c), None); } + + #[test] + fn test_htlc_claim_id_is_descriptor_order_independent() { + // Use opposite txid and vout ordering so the assertion would fail if + // ClaimId still hashed descriptors in caller-provided order. + let first = dummy_htlc_descriptor(Txid::from_slice(&[1; 32]).unwrap(), 2); + let second = dummy_htlc_descriptor(Txid::from_slice(&[2; 32]).unwrap(), 1); + + assert_eq!( + ClaimId::from_htlcs(&[first.clone(), second.clone()]), + ClaimId::from_htlcs(&[second, first]) + ); + } } From 55f52c403fc3c0c0fd40a8cb89f70c0454e17774 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 26 May 2026 14:52:37 +0200 Subject: [PATCH 13/17] Gate chanmon splice fuzz ops on cfg Restore cfg(splicing) to the fuzz check-cfg allow list and gate chanmon consistency splice opcodes on that cfg again. Without the cfg, those inputs stop before executing splice-specific operations. --- fuzz/Cargo.toml | 1 + fuzz/src/chanmon_consistency.rs | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index bf0d463f0fe..76b4968f043 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -43,5 +43,6 @@ check-cfg = [ "cfg(fuzzing)", "cfg(secp256k1_fuzz)", "cfg(hashes_fuzz)", + "cfg(splicing)", "cfg(chacha20_poly1305_fuzz)" ] diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index a0aa7bbe7ef..b67d306714e 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -3200,35 +3200,59 @@ pub fn do_test(data: &[u8], out: Out) { }, 0xa0 => { + if !cfg!(splicing) { + break 'fuzz_loop; + } let cp_node_id = harness.nodes[1].get_our_node_id(); harness.nodes[0].splice_in(&cp_node_id, &harness.chan_a_id()); }, 0xa1 => { + if !cfg!(splicing) { + break 'fuzz_loop; + } let cp_node_id = harness.nodes[0].get_our_node_id(); harness.nodes[1].splice_in(&cp_node_id, &harness.chan_a_id()); }, 0xa2 => { + if !cfg!(splicing) { + break 'fuzz_loop; + } let cp_node_id = harness.nodes[2].get_our_node_id(); harness.nodes[1].splice_in(&cp_node_id, &harness.chan_b_id()); }, 0xa3 => { + if !cfg!(splicing) { + break 'fuzz_loop; + } let cp_node_id = harness.nodes[1].get_our_node_id(); harness.nodes[2].splice_in(&cp_node_id, &harness.chan_b_id()); }, 0xa4 => { + if !cfg!(splicing) { + break 'fuzz_loop; + } let cp_node_id = harness.nodes[1].get_our_node_id(); harness.nodes[0].splice_out(&cp_node_id, &harness.chan_a_id()); }, 0xa5 => { + if !cfg!(splicing) { + break 'fuzz_loop; + } let cp_node_id = harness.nodes[0].get_our_node_id(); harness.nodes[1].splice_out(&cp_node_id, &harness.chan_a_id()); }, 0xa6 => { + if !cfg!(splicing) { + break 'fuzz_loop; + } let cp_node_id = harness.nodes[2].get_our_node_id(); harness.nodes[1].splice_out(&cp_node_id, &harness.chan_b_id()); }, 0xa7 => { + if !cfg!(splicing) { + break 'fuzz_loop; + } let cp_node_id = harness.nodes[1].get_our_node_id(); harness.nodes[2].splice_out(&cp_node_id, &harness.chan_b_id()); }, From 76586d0f57f24a5c8097d5719094f36186bd8c86 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 3 Jun 2026 16:59:18 +0200 Subject: [PATCH 14/17] fuzz: model chanmon mempool mining Route chanmon broadcasts through an explicit harness mempool. Relay, mining, wallet updates, and chain delivery share one path. This lets splice, anchor, and claim txs enter the mempool before mining. --- fuzz/src/chanmon_consistency.rs | 666 +++++++++++++++++++++++++------- 1 file changed, 534 insertions(+), 132 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index b67d306714e..dbb6d62d8e2 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -14,9 +14,10 @@ //! To test this we stand up a network of three nodes and read bytes from the fuzz input to denote //! actions such as sending payments, handling events, or changing monitor update return values on //! a per-node basis. This should allow it to find any cases where the ordering of actions results -//! in us getting out of sync with ourselves, and, assuming at least one of our recieve- or -//! send-side handling is correct, other peers. We consider it a failure if any action results in a -//! channel being force-closed. +//! in us getting out of sync with ourselves, and, assuming at least one of our receive- or +//! send-side handling is correct, other peers. The fuzzer also models transaction relay +//! through a harness mempool, making splice confirmation and block delivery closer to +//! normal node behavior. use bitcoin::amount::Amount; use bitcoin::constants::genesis_block; @@ -27,6 +28,7 @@ use bitcoin::script::{Builder, ScriptBuf}; use bitcoin::transaction::Version; use bitcoin::transaction::{Transaction, TxOut}; use bitcoin::FeeRate; +use bitcoin::OutPoint as BitcoinOutPoint; use bitcoin::block::Header; use bitcoin::hash_types::Txid; @@ -102,6 +104,22 @@ use std::sync::atomic; use std::sync::{Arc, Mutex}; const MAX_FEE: u32 = 10_000; +// The fuzz wallet needs enough confirmed inputs to build many splice +// transactions without accidentally exhausting wallet liquidity before the +// transaction-relay logic is what the test is really exercising. +const NUM_WALLET_UTXOS: u32 = 50; +// A mined transaction is considered deeply confirmed after this many blocks. +// This confirms the transaction in one block and then mines five empty depth +// blocks. +const DEFAULT_TX_CONFIRMATION_BLOCKS: u32 = 6; +// Single fuzz bytes can mine more than one block so a corpus entry does not +// need long runs of identical "mine one block" commands to reach CSV or CLTV +// boundaries. Mining is clipped below if unresolved HTLCs are near expiry. +const MINE_BLOCK_COUNTS: [u32; 8] = [1, 2, 3, 6, 12, 24, 48, 144]; +// Finish-time relay/mining is capped so cleanup can drive realistic +// asynchronous transaction work without letting a malformed state spin forever. +const MAX_FINISH_RELAY_MINE_ROUNDS: usize = 32; + struct FuzzEstimator { ret_val: atomic::AtomicU32, } @@ -183,19 +201,29 @@ impl BroadcasterInterface for TestBroadcaster { struct ChainState { blocks: Vec<(Header, Vec)>, confirmed_txids: HashSet, - /// Unconfirmed transactions (e.g., splice txs). Conflicting RBF candidates may coexist; - /// `confirm_pending_txs` determines which one confirms. + /// Unconfirmed transactions admitted by the harness mempool. Admission keeps + /// this vector in block order: every input is either confirmed already or + /// created by an earlier transaction in this vector. pending_txs: Vec<(Txid, Transaction)>, + /// Tracks unspent outputs created by confirmed transactions. Admission builds + /// a temporary package view from this set plus earlier mempool transactions, + /// which prevents phantom spends of outputs the harness never created. + utxos: HashSet, } impl ChainState { fn new() -> Self { + // Height zero is a dummy genesis block. All later heights are built by + // the harness and connected to ChannelManagers and ChannelMonitors + // through the same callback sequence LDK receives from a real chain + // source. let genesis_hash = genesis_block(Network::Bitcoin).block_hash(); let genesis_header = create_dummy_header(genesis_hash, 42); Self { blocks: vec![(genesis_header, Vec::new())], confirmed_txids: HashSet::new(), pending_txs: Vec::new(), + utxos: HashSet::new(), } } @@ -203,81 +231,277 @@ impl ChainState { (self.blocks.len() - 1) as u32 } - fn is_outpoint_spent(&self, outpoint: &bitcoin::OutPoint) -> bool { - self.blocks.iter().any(|(_, txs)| { - txs.iter().any(|tx| tx.input.iter().any(|input| input.previous_output == *outpoint)) - }) + // Identifies harness-created funding placeholders. We need to distinguish + // them from relayable transactions because they bypass normal mempool input + // validation during channel setup. + fn is_synthetic_funding_tx(tx: &Transaction) -> bool { + // Initial channel funding in this harness is represented by a no-input + // transaction. It is not a valid Bitcoin transaction, but it gives LDK a + // stable funding outpoint without making the harness model coin + // selection during channel setup. + !tx.is_coinbase() && tx.input.is_empty() + } + + // Checks whether a transaction spends unavailable or duplicate inputs. We + // need this to keep double-spends and unknown-prevout spends from producing + // impossible on-chain state. + fn has_invalid_inputs(tx: &Transaction, utxos: &HashSet) -> bool { + // The tiny UTXO set protects the chain model from two false positives: + // accepting a double-spend in the same block, and accepting a + // spend of an output the harness never created. + let mut spent_inputs = HashSet::new(); + for input in &tx.input { + if !spent_inputs.insert(input.previous_output) { + return true; + } + if !utxos.contains(&input.previous_output) { + return true; + } + } + false } - fn confirm_tx(&mut self, tx: Transaction) -> bool { - let txid = tx.compute_txid(); - if self.confirmed_txids.contains(&txid) { - return false; + // Applies a confirmed transaction to ChainState's UTXO set. We need this + // whenever setup mining or mempool mining makes new outputs available to + // later confirmed transactions. + fn apply_tx_to_utxos(&mut self, txid: Txid, tx: &Transaction) { + // Coinbase transactions have a null input, and synthetic funding + // transactions have no inputs, but neither consumes a modeled UTXO. + // Normal transactions consume their inputs before exposing outputs to + // later transactions. + let is_coinbase = tx.is_coinbase(); + if !is_coinbase { + for input in &tx.input { + self.utxos.remove(&input.previous_output); + } } - if tx.input.iter().any(|input| self.is_outpoint_spent(&input.previous_output)) { - return false; + for idx in 0..tx.output.len() { + self.utxos.insert(BitcoinOutPoint { txid, vout: idx as u32 }); } - self.confirmed_txids.insert(txid); + } + // Appends one block to the harness chain. We need a single primitive so + // setup mining, empty mining, and mempool mining share block construction. + fn mine_block(&mut self, txs: Vec) { let prev_hash = self.blocks.last().unwrap().0.block_hash(); let header = create_dummy_header(prev_hash, 42); - self.blocks.push((header, vec![tx])); + self.blocks.push((header, txs)); + } - for _ in 0..5 { - let prev_hash = self.blocks.last().unwrap().0.block_hash(); - let header = create_dummy_header(prev_hash, 42); - self.blocks.push((header, Vec::new())); + // Mines empty height-only blocks. We need this for confirmation depth and + // CSV/CLTV advancement without inventing transactions. + fn mine_empty_blocks(&mut self, count: u32) { + for _ in 0..count { + self.mine_block(Vec::new()); } - true } - /// Add a transaction to the pending pool (mempool). Multiple conflicting transactions (RBF - /// candidates) may coexist; `confirm_pending_txs` selects which one to confirm. - fn add_pending_tx(&mut self, tx: Transaction) { - self.pending_txs.push((tx.compute_txid(), tx)); + // Directly mines harness-generated setup transactions to a target depth. We + // need this bypass because wallet seeding and synthetic funding are not + // relayable mempool transactions. + fn mine_setup_tx_to_depth(&mut self, tx: Transaction, depth: u32) { + // Channel setup and wallet seeding need immediate confirmation. This + // bypasses the mempool and is only for harness-generated coinbase + // wallet transactions and synthetic no-input channel funding + // transactions. + assert!( + tx.is_coinbase() || Self::is_synthetic_funding_tx(&tx), + "direct setup mining is only for coinbase and synthetic funding transactions: {:?}", + tx, + ); + let txid = tx.compute_txid(); + assert!( + self.confirmed_txids.insert(txid), + "direct setup transaction was already confirmed: {:?}", + tx, + ); + self.apply_tx_to_utxos(txid, &tx); + + self.mine_block(vec![tx]); + self.mine_empty_blocks(depth.saturating_sub(1)); } - /// Confirm pending transactions in a single block, selecting deterministically among - /// conflicting RBF candidates. Sorting by txid ensures the winner is determined by fuzz input - /// content. Transactions that double-spend an already-confirmed outpoint are skipped. - fn confirm_pending_txs(&mut self) { - let mut txs = std::mem::take(&mut self.pending_txs); - txs.sort_by_key(|(txid, _)| *txid); + // Attempts to admit a broadcast transaction to the modeled mempool. We need + // admission to enforce locktime, input, and RBF rules so mining can later + // confirm the whole mempool without doing transaction selection. + fn admit_tx_to_mempool(&mut self, tx: Transaction) { + let txid = tx.compute_txid(); + // Enforce locktime at mempool admission. Once a transaction is admitted, + // mining can move the whole mempool into a block without rechecking + // whether each transaction has become height-ready. + let lock_time = tx.lock_time.to_consensus_u32(); + let locktime_enabled = + tx.input.iter().any(|input| input.sequence.enables_absolute_lock_time()); + + // Commitment transactions split the obscured commitment number across + // nSequence and nLockTime with fixed top bytes 0x80 and 0x20. The + // non-final sequence makes rust-bitcoin treat nLockTime as relevant, + // and the fixed 0x20 byte puts it above the 500M timestamp threshold + // even though it is not a fuzz-driven wall-clock lock. + let is_ldk_commitment_obscured_locktime = + tx.input.len() == 1 && tx.input[0].sequence.0 >> 24 == 0x80 && lock_time >> 24 == 0x20; + + // A height lock at or below the current tip is mempool-safe because + // mempools evaluate finality against the next block height. + let immature_absolute_locktime = + locktime_enabled && tx.lock_time.is_block_height() && self.tip_height() < lock_time; + assert!( + !immature_absolute_locktime, + "broadcast immature locktime transaction into chanmon harness mempool: {:?}", + tx, + ); + + // Timestamp locktimes depend on median-time-past, which this fuzzer does + // not model. Treat them as unsupported except for the commitment + // encoding identified above. + let unmodeled_time_locktime = locktime_enabled + && tx.lock_time.is_block_time() + && !is_ldk_commitment_obscured_locktime; + assert!( + !unmodeled_time_locktime, + "broadcast time-locked transaction into chanmon harness mempool: {:?}", + tx, + ); - let mut confirmed = Vec::new(); - let mut spent_outpoints = Vec::new(); - for (txid, tx) in txs { - if self.confirmed_txids.contains(&txid) { - continue; + // Coinbase transactions and the harness's no-input setup funding + // transactions are setup-only artifacts. They must be directly mined by + // mine_setup_tx_to_depth, never relayed through the normal mempool path. + assert!( + !tx.is_coinbase() && !Self::is_synthetic_funding_tx(&tx), + "setup-only transaction entered chanmon harness mempool: {:?}", + tx, + ); + + if self.confirmed_txids.contains(&txid) { + return; + } + if self.pending_txs.iter().any(|(pending_txid, _)| *pending_txid == txid) { + return; + } + + // A real node's mempool cannot contain two transactions spending the + // same input. This harness models direct RBF conflicts at admission: a + // new transaction can replace a conflicting mempool transaction only if + // that conflicting transaction itself signals RBF. The harness does not + // model fee-rate policy, so fuzz-controlled relay order chooses between + // otherwise valid RBF candidates. + let mut conflicting_pending_txids = HashSet::new(); + for (pending_txid, pending_tx) in &self.pending_txs { + let signals_rbf = pending_tx.input.iter().any(|input| input.sequence.is_rbf()); + let conflicts_with_new_tx = pending_tx.input.iter().any(|pending_input| { + tx.input.iter().any(|input| input.previous_output == pending_input.previous_output) + }); + if conflicts_with_new_tx { + if !signals_rbf { + return; + } + conflicting_pending_txids.insert(*pending_txid); } - if tx.input.iter().any(|input| { - self.is_outpoint_spent(&input.previous_output) - || spent_outpoints.contains(&input.previous_output) - }) { - continue; + } + if !conflicting_pending_txids.is_empty() { + let mut removed_outputs = HashSet::new(); + let mut retained_txs = Vec::new(); + for (pending_txid, pending_tx) in self.pending_txs.drain(..) { + let direct_conflict = conflicting_pending_txids.contains(&pending_txid); + let spends_removed_tx = pending_tx + .input + .iter() + .any(|input| removed_outputs.contains(&input.previous_output)); + if direct_conflict || spends_removed_tx { + // Remove direct conflicts and any children that would become + // orphans. Descendants do not make a conflict replaceable; + // they are only removed to keep the modeled mempool valid. + for idx in 0..pending_tx.output.len() { + removed_outputs + .insert(BitcoinOutPoint { txid: pending_txid, vout: idx as u32 }); + } + } else { + retained_txs.push((pending_txid, pending_tx)); + } } - self.confirmed_txids.insert(txid); - for input in &tx.input { - spent_outpoints.push(input.previous_output); + self.pending_txs = retained_txs; + } + + // Build the UTXO set this transaction would see if the current mempool + // confirmed as a package. Start from the confirmed chain, then apply + // each admitted mempool transaction in order so the new transaction can + // spend outputs from earlier mempool parents, but not from later + // transactions or from nowhere. Mining consumes pending_txs in this + // same order, so admission checks the exact order later block + // construction will use. + let mut available_utxos = self.utxos.clone(); + for (pending_txid, pending_tx) in &self.pending_txs { + // Mutate only the temporary package view. ChainState's real UTXO + // set changes later, when mining confirms the mempool. + let is_coinbase = pending_tx.is_coinbase(); + if !is_coinbase { + for input in &pending_tx.input { + available_utxos.remove(&input.previous_output); + } + } + for idx in 0..pending_tx.output.len() { + available_utxos.insert(BitcoinOutPoint { txid: *pending_txid, vout: idx as u32 }); } - confirmed.push(tx); } - - if confirmed.is_empty() { + if Self::has_invalid_inputs(&tx, &available_utxos) { return; } + self.pending_txs.push((txid, tx)); + } - let prev_hash = self.blocks.last().unwrap().0.block_hash(); - let header = create_dummy_header(prev_hash, 42); - self.blocks.push((header, confirmed)); - - for _ in 0..5 { - let prev_hash = self.blocks.last().unwrap().0.block_hash(); - let header = create_dummy_header(prev_hash, 42); - self.blocks.push((header, Vec::new())); + // Feeds broadcast transactions through modeled mempool admission. We need + // this on ChainState so propagation and confirmation share one owner for + // duplicate, locktime, input, and RBF rules. + fn relay_transactions(&mut self, txs: Vec) { + for tx in txs { + // Broadcast does not imply confirmation. Accepted transactions + // enter ChainState's pending pool, and a mining command later moves + // every admitted mempool transaction into the next block. + self.admit_tx_to_mempool(tx); } } + // Mines `count` blocks, confirming the current mempool in the first block. + // We need this on ChainState so block production is kept with the mempool + // state that decides which transactions are eligible to confirm. + fn mine_blocks(&mut self, count: u32) -> Vec { + assert!(count > 0, "mining zero blocks should not be requested"); + + let mempool_txs = std::mem::take(&mut self.pending_txs); + let confirmed_txs = if mempool_txs.is_empty() { + // A mining command should always advance height by at least one + // block. If the mempool was empty, that one block is empty. + self.mine_empty_blocks(1); + Vec::new() + } else { + let mut confirmed = Vec::new(); + for (txid, tx) in mempool_txs { + // Admission made this transaction block-ready. Mining deliberately + // does not reselect or drop transactions; it only consumes the + // mempool in order. + assert!( + !Self::has_invalid_inputs(&tx, &self.utxos), + "mempool transaction was no longer valid at mining time: {:?}", + tx, + ); + assert!( + self.confirmed_txids.insert(txid), + "mempool transaction was already confirmed at mining time: {:?}", + tx, + ); + self.apply_tx_to_utxos(txid, &tx); + confirmed.push(tx); + } + let confirmed_txs = confirmed.clone(); + self.mine_block(confirmed); + confirmed_txs + }; + // The first block either confirmed the mempool or was empty. Mine the + // remaining requested blocks as empty blocks. + self.mine_empty_blocks(count - 1); + confirmed_txs + } + fn block_at(&self, height: u32) -> &(Header, Vec) { &self.blocks[height as usize] } @@ -816,7 +1040,7 @@ struct HarnessNode<'a> { logger: Arc, broadcaster: Arc, fee_estimator: Arc, - wallet: TestWalletSource, + wallet: Arc, persistence_style: ChannelMonitorUpdateStatus, deferred: bool, serialized_manager: Vec, @@ -865,7 +1089,7 @@ impl<'a> HarnessNode<'a> { } fn new( - node_id: u8, wallet: TestWalletSource, fee_estimator: Arc, + node_id: u8, wallet: Arc, fee_estimator: Arc, broadcaster: Arc, persistence_style: ChannelMonitorUpdateStatus, deferred: bool, out: &Out, router: &'a FuzzRouter, chan_type: ChanType, ) -> Self { @@ -957,6 +1181,60 @@ impl<'a> HarnessNode<'a> { } } + // Connects this node from its tracked height to target_height, delivering + // each relevant chain callback to both ChainMonitor and ChannelManager. + fn connect_chain_range(&mut self, chain_state: &ChainState, target_height: u32) { + // Mining commands can advance the harness chain by more than one block. + // Transaction blocks must be connected explicitly so LDK learns about + // on-chain spends, while empty depth blocks still need best-block + // updates so CSV/CLTV-sensitive logic can run. This is the normal sync + // path, so both the raw ChainMonitor and the ChannelManager receive the + // callbacks and the node's tracked height advances to the target. + let mut height = self.height; + while height < target_height { + let mut next_height = height + 1; + while next_height <= target_height && chain_state.block_at(next_height).1.is_empty() { + next_height += 1; + } + if next_height > target_height { + // The rest of the range is empty. One best-block update to the + // final height is enough because LDK's Confirm API explicitly + // allows best_block_updated to skip intermediary blocks, and + // empty blocks have no transactions_confirmed calls whose chain + // order must be preserved. + height = target_height; + let (header, _) = chain_state.block_at(height); + self.monitor.best_block_updated(header, height); + self.node.best_block_updated(header, height); + break; + } + if next_height > height + 1 { + // Advance across the empty prefix before the next transaction + // block. Confirm::best_block_updated may skip intermediary + // blocks, so this compressed update still lets height-triggered + // LDK work run at the correct tip before the transaction + // confirmations are connected. + height = next_height - 1; + let (header, _) = chain_state.block_at(height); + self.monitor.best_block_updated(header, height); + self.node.best_block_updated(header, height); + } + height = next_height; + let (header, txn) = chain_state.block_at(height); + let txdata: Vec<_> = txn.iter().enumerate().map(|(i, tx)| (i + 1, tx)).collect(); + if !txdata.is_empty() { + // Transaction blocks need the explicit confirmation callback + // before the best-block update so watched spends are delivered + // in chain order before the node advances to that tip. + self.monitor.transactions_confirmed(header, &txdata, height); + self.node.transactions_confirmed(header, &txdata, height); + } + self.monitor.best_block_updated(header, height); + self.node.best_block_updated(header, height); + } + self.height = target_height; + } + fn sync_with_chain_state(&mut self, chain_state: &ChainState, num_blocks: Option) { let target_height = if let Some(num_blocks) = num_blocks { std::cmp::min(self.height + num_blocks, chain_state.tip_height()) @@ -964,15 +1242,7 @@ impl<'a> HarnessNode<'a> { chain_state.tip_height() }; - while self.height < target_height { - self.height += 1; - let (header, txn) = chain_state.block_at(self.height); - let txdata: Vec<_> = txn.iter().enumerate().map(|(i, tx)| (i + 1, tx)).collect(); - if !txdata.is_empty() { - self.node.transactions_confirmed(header, &txdata, self.height); - } - self.node.best_block_updated(header, self.height); - } + self.connect_chain_range(chain_state, target_height); } fn checkpoint_manager_persistence(&mut self) -> bool { @@ -1033,7 +1303,7 @@ impl<'a> HarnessNode<'a> { } fn splice_in(&self, counterparty_node_id: &PublicKey, channel_id: &ChannelId) { - let wallet = WalletSync::new(&self.wallet, Arc::clone(&self.logger)); + let wallet = WalletSync::new(Arc::clone(&self.wallet), Arc::clone(&self.logger)); match self.node.splice_channel(channel_id, counterparty_node_id) { Ok(funding_template) => { let feerate = @@ -1173,6 +1443,7 @@ impl<'a> HarnessNode<'a> { assert_eq!(chain_monitor.watch_channel(channel_id, mon), Ok(expected_status)); } self.node = manager.1; + self.height = self.node.current_best_block().height; self.monitor = chain_monitor; self.persister = persister; self.logger = logger; @@ -1940,7 +2211,6 @@ impl PaymentTracker { .get(&payment_hash) .expect("PaymentClaimable for unknown payment hash"); node.claim_funds(payment_preimage); - self.claimed_payment_hashes.insert(payment_hash); } } @@ -2008,7 +2278,8 @@ fn assert_test_invariants(nodes: &[HarnessNode<'_>; 3]) { assert_eq!(nodes[1].list_channels().len(), 6); assert_eq!(nodes[2].list_channels().len(), 3); - // All broadcasters should be empty. Broadcast transactions are handled explicitly. + // All broadcasters should be empty because broadcast transactions only enter + // the modeled mempool through explicit relay commands or finish cleanup. assert!(nodes[0].broadcaster.txn_broadcasted.borrow().is_empty()); assert!(nodes[1].broadcaster.txn_broadcasted.borrow().is_empty()); assert!(nodes[2].broadcaster.txn_broadcasted.borrow().is_empty()); @@ -2124,7 +2395,7 @@ fn make_channel( tx.clone(), ) .unwrap(); - chain_state.confirm_tx(tx); + chain_state.mine_setup_tx_to_depth(tx, DEFAULT_TX_CONFIRMATION_BLOCKS); } else { panic!("Wrong event type"); } @@ -2241,24 +2512,32 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { config_byte & 0b1000_0000 != 0, ]; - let wallet_a = TestWalletSource::new(SecretKey::from_slice(&[1; 32]).unwrap()); - let wallet_b = TestWalletSource::new(SecretKey::from_slice(&[2; 32]).unwrap()); - let wallet_c = TestWalletSource::new(SecretKey::from_slice(&[3; 32]).unwrap()); - let wallets = [&wallet_a, &wallet_b, &wallet_c]; - let coinbase_tx = bitcoin::Transaction { - version: bitcoin::transaction::Version::TWO, - lock_time: bitcoin::absolute::LockTime::ZERO, - input: vec![bitcoin::TxIn { ..Default::default() }], - output: wallets - .iter() - .map(|wallet| TxOut { - value: Amount::from_sat(100_000), - script_pubkey: wallet.get_change_script().unwrap(), - }) - .collect(), - }; - for (idx, wallet) in wallets.iter().enumerate() { - wallet.add_utxo(coinbase_tx.clone(), idx as u32); + let wallet_a = Arc::new(TestWalletSource::new(SecretKey::from_slice(&[1; 32]).unwrap())); + let wallet_b = Arc::new(TestWalletSource::new(SecretKey::from_slice(&[2; 32]).unwrap())); + let wallet_c = Arc::new(TestWalletSource::new(SecretKey::from_slice(&[3; 32]).unwrap())); + let wallets = [wallet_a.as_ref(), wallet_b.as_ref(), wallet_c.as_ref()]; + let mut chain_state = ChainState::new(); + for wallet in wallets { + // Seed each wallet with many confirmed outputs. Splice flows may need + // fresh inputs long after the channel setup phase, and exhausting the + // wallet would obscure the transaction-relay behavior under test. + let coinbase_tx = bitcoin::Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: bitcoin::absolute::LockTime::ZERO, + input: vec![bitcoin::TxIn { ..Default::default() }], + output: (0..NUM_WALLET_UTXOS) + .map(|_| TxOut { + value: Amount::from_sat(100_000), + script_pubkey: wallet.get_change_script().unwrap(), + }) + .collect(), + }; + for vout in 0..NUM_WALLET_UTXOS { + wallet.add_utxo(coinbase_tx.clone(), vout); + } + // Mine the wallet UTXOs into the same ChainState that later drives + // channel funding and splice transactions. + chain_state.mine_setup_tx_to_depth(coinbase_tx, DEFAULT_TX_CONFIRMATION_BLOCKS); } let fee_est_a = Arc::new(FuzzEstimator { ret_val: atomic::AtomicU32::new(253) }); @@ -2273,7 +2552,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { let mut nodes = [ HarnessNode::new( 0, - wallet_a, + Arc::clone(&wallet_a), Arc::clone(&fee_est_a), Arc::clone(&broadcast_a), persistence_styles[0], @@ -2284,7 +2563,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { ), HarnessNode::new( 1, - wallet_b, + Arc::clone(&wallet_b), Arc::clone(&fee_est_b), Arc::clone(&broadcast_b), persistence_styles[1], @@ -2295,7 +2574,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { ), HarnessNode::new( 2, - wallet_c, + Arc::clone(&wallet_c), Arc::clone(&fee_est_c), Arc::clone(&broadcast_c), persistence_styles[2], @@ -2305,8 +2584,6 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { chan_type, ), ]; - let mut chain_state = ChainState::new(); - // Connect peers first, then create channels. connect_peers(&nodes[0], &nodes[1]); connect_peers(&nodes[1], &nodes[2]); @@ -2327,8 +2604,9 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { make_channel(&mut nodes, 1, 2, 5, set_0reserve, false, &mut chain_state); make_channel(&mut nodes, 1, 2, 6, false, false, &mut chain_state); - // Wipe the transactions-broadcasted set to make sure we don't broadcast - // any transactions during normal operation after setup. + // Wipe setup-time broadcasts so normal operation starts with an empty + // relay queue. Later broadcasts only enter the mempool through relay + // commands or finish cleanup. nodes[0].broadcaster.txn_broadcasted.borrow_mut().clear(); nodes[1].broadcaster.txn_broadcasted.borrow_mut().clear(); nodes[2].broadcaster.txn_broadcasted.borrow_mut().clear(); @@ -2375,7 +2653,33 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { self.bc_link.first_channel_id() } - fn finish(&self) { + // Runs end-of-input cleanup by relaying and mining remaining broadcasts. + // Final invariants should not depend on the input ending with explicit relay + // and mining bytes. + fn finish(&mut self) { + for _ in 0..MAX_FINISH_RELAY_MINE_ROUNDS { + // Finish paths should not leave already-broadcast transactions + // stranded. Relay all broadcasts, mine them to normal depth, and + // repeat because confirmed transactions can trigger later broadcasts. + let mut txs = Vec::new(); + for node in &self.nodes { + txs.extend(node.broadcaster.txn_broadcasted.borrow_mut().drain(..)); + } + self.chain_state.relay_transactions(txs); + if self.chain_state.pending_txs.is_empty() { + assert_test_invariants(&self.nodes); + return; + } + assert!( + self.mine_blocks(DEFAULT_TX_CONFIRMATION_BLOCKS) > 0, + "finish cannot mine pending mempool transactions without crossing an unresolved HTLC timeout deadline" + ); + } + assert!( + !self.nodes.iter().any(|node| !node.broadcaster.txn_broadcasted.borrow().is_empty()) + && self.chain_state.pending_txs.is_empty(), + "finish tx mining loop failed to quiesce", + ); assert_test_invariants(&self.nodes); } @@ -2774,7 +3078,6 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { fn process_events(&mut self, node_idx: usize, fail: bool) -> bool { let nodes = &self.nodes; - let chain_state = &mut self.chain_state; let payments = &mut self.payments; // Multiple HTLCs can resolve for the same payment hash, so deduplicate // claim/fail handling per event batch. @@ -2801,7 +3104,11 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { | events::Event::ProbeFailed { payment_id, .. } => { payments.nodes[node_idx].mark_resolved_without_hash(payment_id); }, - events::Event::PaymentClaimed { .. } => {}, + events::Event::PaymentClaimed { payment_hash, .. } => { + if payments.payment_preimages.contains_key(&payment_hash) { + payments.claimed_payment_hashes.insert(payment_hash); + } + }, events::Event::PaymentPathSuccessful { .. } => {}, events::Event::PaymentPathFailed { .. } => {}, events::Event::PaymentForwarded { .. } if node_idx == 1 => {}, @@ -2818,13 +3125,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { .funding_transaction_signed(&channel_id, &counterparty_node_id, signed_tx) .unwrap(); }, - events::Event::SpliceNegotiated { new_funding_txo, .. } => { - let mut txs = nodes[node_idx].broadcaster.txn_broadcasted.borrow_mut(); - assert!(txs.len() >= 1); - let splice_tx = txs.remove(0); - assert_eq!(new_funding_txo.txid, splice_tx.compute_txid()); - chain_state.add_pending_tx(splice_tx); - }, + events::Event::SpliceNegotiated { .. } => {}, events::Event::SpliceNegotiationFailed { .. } => {}, events::Event::DiscardFunding { funding_info: @@ -2950,6 +3251,11 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { } fn settle_all(&mut self) { + let chain_state = &self.chain_state; + for node in &mut self.nodes { + node.sync_with_chain_state(chain_state, None); + } + // First, make sure peers are all connected to each other self.reconnect_ab(); self.reconnect_bc(); @@ -3024,6 +3330,107 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { } made_progress } + + // Relays one node's broadcasts into the modeled mempool. We need per-node + // relays so fuzz bytes can explore partial propagation before a block is + // mined. + fn relay_broadcasts_for_node(&mut self, node_idx: usize) { + // Move only one node's broadcasts into the harness mempool. Separate + // fuzz bytes for each node let corpus inputs model partial transaction + // propagation before mining. + let txs = self.nodes[node_idx] + .broadcaster + .txn_broadcasted + .borrow_mut() + .drain(..) + .collect::>(); + self.chain_state.relay_transactions(txs); + } + + fn earliest_pending_htlc_expiry(&self) -> Option { + let mut earliest_expiry: Option = None; + for node in &self.nodes { + for chan in node.list_channels() { + for htlc in &chan.pending_inbound_htlcs { + earliest_expiry = Some( + earliest_expiry + .map_or(htlc.cltv_expiry, |expiry| expiry.min(htlc.cltv_expiry)), + ); + } + for htlc in &chan.pending_outbound_htlcs { + earliest_expiry = Some( + earliest_expiry + .map_or(htlc.cltv_expiry, |expiry| expiry.min(htlc.cltv_expiry)), + ); + } + } + } + earliest_expiry + } + + fn safe_mine_block_count(&self, count: u32) -> u32 { + if let Some(expiry) = self.earliest_pending_htlc_expiry() { + let current_tip = self.chain_state.tip_height(); + // LDK may close to protect a pending HTLC before its raw CLTV + // expiry. Keep modeled mining outside that fail-back window so + // fuzzed block production does not force an on-chain timeout path. + let timeout_deadline = expiry.saturating_sub(channelmonitor::HTLC_FAIL_BACK_BUFFER); + assert!( + current_tip < timeout_deadline, + "pending HTLC with expiry {} and timeout deadline {} is already unsafe at tip {}", + expiry, + timeout_deadline, + current_tip + ); + // Stop before the deadline block itself, since connecting it is + // enough for ChannelMonitor timeout handling to run. + count.min(timeout_deadline - current_tip - 1) + } else { + count + } + } + + // Mines blocks through ChainState, then syncs wallets and nodes. We need the + // harness wrapper because block production is chain-only, while confirmed + // transactions still update wallet UTXOs and LDK chain listeners. + fn mine_blocks(&mut self, count: u32) -> u32 { + assert!(count > 0, "mining zero blocks should not be requested"); + + let count = self.safe_mine_block_count(count); + if count == 0 { + return 0; + } + let confirmed_txs = self.chain_state.mine_blocks(count); + let wallets = [ + self.nodes[0].wallet.as_ref(), + self.nodes[1].wallet.as_ref(), + self.nodes[2].wallet.as_ref(), + ]; + for tx in &confirmed_txs { + for wallet in wallets.iter().copied() { + let change_script = wallet.get_change_script().unwrap(); + for input in &tx.input { + // The test wallet is a simple UTXO source. When one of its + // outputs is spent by a confirmed transaction, remove it so + // later funding attempts cannot double-spend it. + wallet.remove_utxo(input.previous_output); + } + for (vout, output) in tx.output.iter().enumerate() { + if output.script_pubkey == change_script { + // Add wallet-owned outputs back to whichever test wallet + // owns the script. This lets splice flows recycle wallet + // change through later fuzz commands. + wallet.add_utxo(tx.clone(), vout as u32); + } + } + } + } + let chain_state = &self.chain_state; + for node in &mut self.nodes { + node.sync_with_chain_state(chain_state, None); + } + count + } } #[inline] @@ -3257,32 +3664,14 @@ pub fn do_test(data: &[u8], out: Out) { harness.nodes[2].splice_out(&cp_node_id, &harness.chan_b_id()); }, - // Sync node by 1 block to cover confirmation of a transaction. - 0xa8 => { - harness.chain_state.confirm_pending_txs(); - harness.nodes[0].sync_with_chain_state(&harness.chain_state, Some(1)); - }, - 0xa9 => { - harness.chain_state.confirm_pending_txs(); - harness.nodes[1].sync_with_chain_state(&harness.chain_state, Some(1)); - }, - 0xaa => { - harness.chain_state.confirm_pending_txs(); - harness.nodes[2].sync_with_chain_state(&harness.chain_state, Some(1)); - }, - // Sync node to chain tip to cover confirmation of a transaction post-reorg-risk. - 0xab => { - harness.chain_state.confirm_pending_txs(); - harness.nodes[0].sync_with_chain_state(&harness.chain_state, None); - }, - 0xac => { - harness.chain_state.confirm_pending_txs(); - harness.nodes[1].sync_with_chain_state(&harness.chain_state, None); - }, - 0xad => { - harness.chain_state.confirm_pending_txs(); - harness.nodes[2].sync_with_chain_state(&harness.chain_state, None); - }, + // Sync node by 1 block. + 0xa8 => harness.nodes[0].sync_with_chain_state(&harness.chain_state, Some(1)), + 0xa9 => harness.nodes[1].sync_with_chain_state(&harness.chain_state, Some(1)), + 0xaa => harness.nodes[2].sync_with_chain_state(&harness.chain_state, Some(1)), + // Sync node to chain tip. + 0xab => harness.nodes[0].sync_with_chain_state(&harness.chain_state, None), + 0xac => harness.nodes[1].sync_with_chain_state(&harness.chain_state, None), + 0xad => harness.nodes[2].sync_with_chain_state(&harness.chain_state, None), 0xb0 | 0xb1 | 0xb2 => { // Restart node A, picking among persisted and in-flight `ChannelMonitor` @@ -3407,6 +3796,19 @@ pub fn do_test(data: &[u8], out: Out) { .enable_op_for_all_signers(SignerOp::SignSpliceSharedInput); harness.nodes[2].signer_unblocked(None); }, + // Relay and mining commands share a contiguous range so mutations can + // move between "propagate transaction" and "advance chain state" + // without landing in unrelated signer or monitor-control commands. + 0xd5 => harness.relay_broadcasts_for_node(0), + 0xd6 => harness.relay_broadcasts_for_node(1), + 0xd7 => harness.relay_broadcasts_for_node(2), + 0xd8..=0xdf => { + // The table maps one byte range to several useful block counts, + // allowing both one-block timeout exploration and large jumps to + // CSV or CLTV boundaries. + let count = MINE_BLOCK_COUNTS[(v - 0xd8) as usize]; + harness.mine_blocks(count); + }, 0xf0 => harness.ab_link.complete_monitor_updates_for_node( 0, From 941b7e01e8d7f3048b27448a1baf2764000978b6 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 4 Jun 2026 12:17:35 +0200 Subject: [PATCH 15/17] fuzz: add chanmon holder signer fuzz ops Allow chanmon consistency inputs to block and later unblock holder-side signing operations. This lets focused force-close fuzzing reuse the signer-op machinery without carrying the larger mining and settlement model. --- fuzz/src/chanmon_consistency.rs | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index dbb6d62d8e2..8cd9796a65e 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -942,13 +942,16 @@ impl SignerProvider for KeyProvider { } } -// Since this fuzzer is only concerned with live-channel operations, we don't need to worry about -// any signer operations that come after a force close. -const SUPPORTED_SIGNER_OPS: [SignerOp; 4] = [ +// These signer operations can be blocked by fuzz bytes. The first four cover +// live-channel and splice signing, while the holder-side operations cover local +// on-chain claim signing after LDK has moved a channel to chain handling. +const SUPPORTED_SIGNER_OPS: [SignerOp; 6] = [ SignerOp::SignCounterpartyCommitment, SignerOp::GetPerCommitmentPoint, SignerOp::ReleaseCommitmentSecret, SignerOp::SignSpliceSharedInput, + SignerOp::SignHolderCommitment, + SignerOp::SignHolderHtlcTransaction, ]; impl KeyProvider { @@ -1294,6 +1297,18 @@ impl<'a> HarnessNode<'a> { self.node.timer_tick_occurred(); } + // Re-enables holder-side signer operations and asks the chain monitor to + // retry pending claim transactions. Holder signing becomes relevant after + // on-chain close handling starts. + fn enable_holder_signer_ops(&self) { + // Holder-side signing is requested when a node needs to build local + // on-chain claim transactions. Keep it separate from live-channel + // signing so fuzz inputs can choose when those requests unblock. + self.keys_manager.enable_op_for_all_signers(SignerOp::SignHolderCommitment); + self.keys_manager.enable_op_for_all_signers(SignerOp::SignHolderHtlcTransaction); + self.monitor.signer_unblocked(None); + } + fn current_feerate_sat_per_kw(&self) -> FeeRate { self.fee_estimator.feerate_sat_per_kw() } @@ -3265,9 +3280,14 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { self.nodes[1].keys_manager.enable_op_for_all_signers(op); self.nodes[2].keys_manager.enable_op_for_all_signers(op); } + // Live-channel signer work retries through the manager, while + // on-chain holder claims retry through the chain monitor. self.nodes[0].signer_unblocked(None); self.nodes[1].signer_unblocked(None); self.nodes[2].signer_unblocked(None); + self.nodes[0].monitor.signer_unblocked(None); + self.nodes[1].monitor.signer_unblocked(None); + self.nodes[2].monitor.signer_unblocked(None); self.process_all_events(); @@ -3809,6 +3829,13 @@ pub fn do_test(data: &[u8], out: Out) { let count = MINE_BLOCK_COUNTS[(v - 0xd8) as usize]; harness.mine_blocks(count); }, + // Keep holder signer unblocks near the signer op bytes while leaving + // the relay/mining range above contiguous. The helper re-enables both + // holder-side operations for every signer owned by the selected node, + // matching the existing key-manager-wide blocking model. + 0xe4 => harness.nodes[0].enable_holder_signer_ops(), + 0xe5 => harness.nodes[1].enable_holder_signer_ops(), + 0xe6 => harness.nodes[2].enable_holder_signer_ops(), 0xf0 => harness.ab_link.complete_monitor_updates_for_node( 0, From a24d2d02b5e58b546142fb25dd4d0bb3b12f3626 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 19 May 2026 14:54:30 +0200 Subject: [PATCH 16/17] fuzz: factor chanmon_consistency node loops Replace repeated per-node setup and event-processing calls with loops. Keep the existing assertions and early-continue behavior intact. --- fuzz/src/chanmon_consistency.rs | 52 +++++++++++++-------------------- 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 8cd9796a65e..5f156cce280 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -2295,9 +2295,9 @@ fn assert_test_invariants(nodes: &[HarnessNode<'_>; 3]) { // All broadcasters should be empty because broadcast transactions only enter // the modeled mempool through explicit relay commands or finish cleanup. - assert!(nodes[0].broadcaster.txn_broadcasted.borrow().is_empty()); - assert!(nodes[1].broadcaster.txn_broadcasted.borrow().is_empty()); - assert!(nodes[2].broadcaster.txn_broadcasted.borrow().is_empty()); + for node in nodes { + assert!(node.broadcaster.txn_broadcasted.borrow().is_empty()); + } } fn connect_peers(source: &ChanMan<'_>, dest: &ChanMan<'_>) { @@ -2622,14 +2622,14 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { // Wipe setup-time broadcasts so normal operation starts with an empty // relay queue. Later broadcasts only enter the mempool through relay // commands or finish cleanup. - nodes[0].broadcaster.txn_broadcasted.borrow_mut().clear(); - nodes[1].broadcaster.txn_broadcasted.borrow_mut().clear(); - nodes[2].broadcaster.txn_broadcasted.borrow_mut().clear(); + for node in &nodes { + node.broadcaster.txn_broadcasted.borrow_mut().clear(); + } // Sync all nodes to tip to lock the funding. - nodes[0].sync_with_chain_state(&chain_state, None); - nodes[1].sync_with_chain_state(&chain_state, None); - nodes[2].sync_with_chain_state(&chain_state, None); + for node in &mut nodes { + node.sync_with_chain_state(&chain_state, None); + } lock_fundings(&nodes); @@ -3169,7 +3169,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { fn process_all_events(&mut self) { let mut last_pass_no_updates = false; - for i in 0..std::usize::MAX { + 'settle: for i in 0..std::usize::MAX { if i == 100 { panic!( "It may take may iterations to settle the state, but it should not take forever" @@ -3180,30 +3180,18 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { made_progress |= self.ab_link.complete_all_monitor_updates(&self.nodes); made_progress |= self.bc_link.complete_all_monitor_updates(&self.nodes); // Then, make sure any current forwards make their way to their destination. - if self.process_msg_events(0, false, ProcessMessages::AllMessages) { - last_pass_no_updates = false; - continue; - } - if self.process_msg_events(1, false, ProcessMessages::AllMessages) { - last_pass_no_updates = false; - continue; - } - if self.process_msg_events(2, false, ProcessMessages::AllMessages) { - last_pass_no_updates = false; - continue; + for node_idx in 0..3 { + if self.process_msg_events(node_idx, false, ProcessMessages::AllMessages) { + last_pass_no_updates = false; + continue 'settle; + } } // ...making sure any payments are claimed. - if self.process_events(0, false) { - last_pass_no_updates = false; - continue; - } - if self.process_events(1, false) { - last_pass_no_updates = false; - continue; - } - if self.process_events(2, false) { - last_pass_no_updates = false; - continue; + for node_idx in 0..3 { + if self.process_events(node_idx, false) { + last_pass_no_updates = false; + continue 'settle; + } } if made_progress { last_pass_no_updates = false; From 172860c1b9fdfb9f11920bcc6c9c886bf9efcc30 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 4 Jun 2026 12:04:44 +0200 Subject: [PATCH 17/17] fuzz: cover chanmon force-close settlement Fold the mempool follow-up into the force-close fuzzing layer so this branch has one commit for settlement coverage. Keep relay and mining opcodes from the mempool model while adding explicit and timeout-driven close tracking, holder signer unblocks, and cleanup that drives on-chain claims. --- fuzz/src/chanmon_consistency.rs | 2228 +++++++++++++++++++++++++------ 1 file changed, 1810 insertions(+), 418 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 5f156cce280..77fedccf555 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -15,9 +15,9 @@ //! actions such as sending payments, handling events, or changing monitor update return values on //! a per-node basis. This should allow it to find any cases where the ordering of actions results //! in us getting out of sync with ourselves, and, assuming at least one of our receive- or -//! send-side handling is correct, other peers. The fuzzer also models transaction relay -//! through a harness mempool, making splice confirmation and block delivery closer to -//! normal node behavior. +//! send-side handling is correct, other peers. The fuzzer also models a small mempool and +//! exercises controlled force-closes, including user-initiated closes and timeout-driven closes, +//! through on-chain confirmation and cleanup. use bitcoin::amount::Amount; use bitcoin::constants::genesis_block; @@ -43,18 +43,18 @@ use lightning::chain; use lightning::chain::chaininterface::{ BroadcasterInterface, ConfirmationTarget, FeeEstimator, TransactionType, }; -use lightning::chain::channelmonitor::ChannelMonitor; +use lightning::chain::channelmonitor::{Balance, ChannelMonitor}; use lightning::chain::{ chainmonitor, channelmonitor, BlockLocator, ChannelMonitorUpdateStatus, Confirm, Watch, }; -use lightning::events; +use lightning::events::{self, EventsProvider}; use lightning::ln::channel::{ FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS, }; use lightning::ln::channel_state::ChannelDetails; use lightning::ln::channelmanager::{ ChainParameters, ChannelManager, ChannelManagerReadArgs, PaymentId, RecentPaymentDetails, - TrustedChannelFeatures, + TrustedChannelFeatures, MIN_CLTV_EXPIRY_DELTA, MIN_FINAL_CLTV_EXPIRY_DELTA, }; use lightning::ln::functional_test_utils::*; use lightning::ln::inbound_payment::ExpandedKey; @@ -62,6 +62,7 @@ use lightning::ln::msgs::{ self, BaseMessageHandler, ChannelMessageHandler, CommitmentUpdate, Init, MessageSendEvent, UpdateAddHTLC, }; +use lightning::ln::onion_utils::LocalHTLCFailureReason; use lightning::ln::outbound_payment::RecipientOnionFields; use lightning::ln::script::ShutdownScript; use lightning::ln::types::ChannelId; @@ -85,6 +86,8 @@ use lightning::util::test_channel_signer::{EnforcementState, SignerOp, TestChann use lightning::util::test_utils::TestWalletSource; use lightning::util::wallet_utils::{WalletSourceSync, WalletSync}; +use lightning::events::bump_transaction::sync::BumpTransactionEventHandlerSync; + use lightning_invoice::RawBolt11Invoice; use crate::utils::test_logger::{self, Output}; @@ -104,9 +107,10 @@ use std::sync::atomic; use std::sync::{Arc, Mutex}; const MAX_FEE: u32 = 10_000; -// The fuzz wallet needs enough confirmed inputs to build many splice -// transactions without accidentally exhausting wallet liquidity before the -// transaction-relay logic is what the test is really exercising. +// The fuzz wallet needs enough confirmed inputs to build many splice, anchor, +// and claim transactions without accidentally exhausting wallet liquidity +// before the on-chain close and claim behavior is what the test is really +// exercising. const NUM_WALLET_UTXOS: u32 = 50; // A mined transaction is considered deeply confirmed after this many blocks. // This confirms the transaction in one block and then mines five empty depth @@ -114,11 +118,21 @@ const NUM_WALLET_UTXOS: u32 = 50; const DEFAULT_TX_CONFIRMATION_BLOCKS: u32 = 6; // Single fuzz bytes can mine more than one block so a corpus entry does not // need long runs of identical "mine one block" commands to reach CSV or CLTV -// boundaries. Mining is clipped below if unresolved HTLCs are near expiry. +// boundaries. The early entries remain small so the fuzzer can still explore +// fine-grained height changes. const MINE_BLOCK_COUNTS: [u32; 8] = [1, 2, 3, 6, 12, 24, 48, 144]; -// Finish-time relay/mining is capped so cleanup can drive realistic -// asynchronous transaction work without letting a malformed state spin forever. -const MAX_FINISH_RELAY_MINE_ROUNDS: usize = 32; +// Progress loops are capped so `0xff` and cleanup paths can drive realistic +// asynchronous work without letting a malformed state spin forever. +const QUIESCENCE_ROUNDS: usize = 32; +// Force-close cleanup may require many height advances because claims can be +// chained through commitment, HTLC, anchor, and delayed-output transactions. +// This is intentionally large, but still finite, so a stuck cleanup becomes an +// assertion instead of an infinite fuzz run. +const FORCE_CLOSE_CLEANUP_ROUNDS: usize = 4096; +// Use LDK's minimum CLTV deltas to keep timeout-driven close scenarios reachable +// in short fuzz inputs while still respecting the library's configured limits. +const FUZZ_FINAL_CLTV_DELTA: u32 = MIN_FINAL_CLTV_EXPIRY_DELTA as u32; +const FUZZ_FORWARD_CLTV_DELTA: u32 = MIN_CLTV_EXPIRY_DELTA as u32; struct FuzzEstimator { ret_val: atomic::AtomicU32, @@ -334,10 +348,9 @@ impl ChainState { tx.input.iter().any(|input| input.sequence.enables_absolute_lock_time()); // Commitment transactions split the obscured commitment number across - // nSequence and nLockTime with fixed top bytes 0x80 and 0x20. The - // non-final sequence makes rust-bitcoin treat nLockTime as relevant, - // and the fixed 0x20 byte puts it above the 500M timestamp threshold - // even though it is not a fuzz-driven wall-clock lock. + // nSequence and nLockTime with fixed top bytes 0x80 and 0x20. That puts + // nLockTime above the 500M timestamp threshold even though it is not a + // fuzz-driven wall-clock lock. let is_ldk_commitment_obscured_locktime = tx.input.len() == 1 && tx.input[0].sequence.0 >> 24 == 0x80 && lock_time >> 24 == 0x20; @@ -451,14 +464,25 @@ impl ChainState { // Feeds broadcast transactions through modeled mempool admission. We need // this on ChainState so propagation and confirmation share one owner for - // duplicate, locktime, input, and RBF rules. - fn relay_transactions(&mut self, txs: Vec) { + // duplicate, locktime, input, and RBF rules. The return value reports + // whether any broadcasts were drained, even if admission later ignores a + // duplicate or invalid transaction. + fn relay_transactions(&mut self, txs: Vec) -> bool { + let found = !txs.is_empty(); for tx in txs { // Broadcast does not imply confirmation. Accepted transactions // enter ChainState's pending pool, and a mining command later moves // every admitted mempool transaction into the next block. self.admit_tx_to_mempool(tx); } + found + } + + // Reports whether the modeled mempool is non-empty. Fuzz mining bytes and + // cleanup loops use this to decide whether another mining pass can make + // progress. + fn has_pending_txs(&self) -> bool { + !self.pending_txs.is_empty() } // Mines `count` blocks, confirming the current mempool in the first block. @@ -507,6 +531,8 @@ impl ChainState { } } +// ChannelMonitor restart candidates are stored as serialized blobs, matching +// what a real persister would have durable at crash time. pub struct VecWriter(pub Vec); impl Writer for VecWriter { fn write_all(&mut self, buf: &[u8]) -> Result<(), ::lightning::io::Error> { @@ -515,6 +541,7 @@ impl Writer for VecWriter { } } +// Serializes a monitor snapshot for later reload selection. fn serialize_monitor(monitor: &ChannelMonitor) -> Vec { let mut ser = VecWriter(Vec::new()); monitor.write(&mut ser).unwrap(); @@ -810,6 +837,12 @@ type TestChainMonitor = chainmonitor::ChainMonitor< Arc, Arc, >; +type TestBumpTransactionEventHandler = BumpTransactionEventHandlerSync< + Arc, + Arc, Arc>>, + Arc, + Arc, +>; struct KeyProvider { node_secret: SecretKey, @@ -942,9 +975,11 @@ impl SignerProvider for KeyProvider { } } -// These signer operations can be blocked by fuzz bytes. The first four cover -// live-channel and splice signing, while the holder-side operations cover local -// on-chain claim signing after LDK has moved a channel to chain handling. +// These signer ops can be blocked by fuzz bytes. Normal fuzz bytes selectively +// unblock the live-channel and splice ops, while `settle_all` re-enables the +// whole set before final cleanup. The holder-side signing ops matter only after +// a force close, when monitors may need to build holder commitment or HTLC +// claim transactions. const SUPPORTED_SIGNER_OPS: [SignerOp; 6] = [ SignerOp::SignCounterpartyCommitment, SignerOp::GetPerCommitmentPoint, @@ -953,6 +988,25 @@ const SUPPORTED_SIGNER_OPS: [SignerOp; 6] = [ SignerOp::SignHolderCommitment, SignerOp::SignHolderHtlcTransaction, ]; +// LDK reports expected force-close paths through message text in a few places. +// The harness keeps these strings centralized so reviewers can audit exactly +// which peer errors are treated as control-flow artifacts of the test. +const HTLC_TIMEOUT_ERROR_PREFIX: &str = "Channel closed because HTLC(s) on the channel timed out"; +const ONCHAIN_TX_CONFIRMED_ERROR: &str = + "Channel closed because commitment or closing transaction was confirmed on chain."; +const CLOSED_CHANNEL_WRONG_NODE_ERROR_PREFIX: &str = + "Got a message for a channel from the wrong node! No such channel_id"; +const INVALID_REESTABLISH_FORCE_CLOSE_ERROR: &str = + "Peer sent an invalid channel_reestablish to force close in a non-standard way"; +const NEEDED_CHANNEL_REESTABLISH_ERROR: &str = "when we needed a channel_reestablish"; +// These bounds mirror the heights where LDK may decide it is no longer safe to +// keep a channel open. The harness uses them only when syncing a node to a new +// height, which is the point where timeout-driven closes become observable. +const HTLC_TIMEOUT_GRACE_BLOCKS: u32 = 3; +const HTLC_CLAIM_BUFFER_BLOCKS: u32 = 36; +// These distinct short strings let the harness recognize user force-close +// errors without accepting arbitrary SendErrorMessage actions. +const FORCE_CLOSE_ERROR_MESSAGES: [&str; 4] = ["]]]]]]]]]", "]]]]]]]]", "]]]]]]]", "]]]]]"]; impl KeyProvider { fn make_enforcement_state_cell( @@ -997,19 +1051,49 @@ type ChanMan<'a> = ChannelManager< >; #[inline] -fn assert_disconnect_action(action: &msgs::ErrorAction) -> (&msgs::WarningMessage, bool) { +// Verifies that an LDK HandleError is one of the harness-controlled paths. We +// need this because force-close coverage should not accept arbitrary protocol +// errors as expected behavior. +fn assert_expected_control_error_action( + action: &msgs::ErrorAction, channel_closes: Option<&HashMap>, +) { // Since sending/receiving messages may be delayed, `timer_tick_occurred` may cause a node to - // disconnect their counterparty if they're expecting a timely response. - if let msgs::ErrorAction::DisconnectPeerWithWarning { ref msg } = action { - let is_quiescent_msg = msg.data.contains("already sent splice_locked, cannot RBF"); - if !msg.data.contains("Disconnecting due to timeout awaiting response") && !is_quiescent_msg - { - panic!("Unexpected disconnect case: {}", msg.data); - } - (msg, is_quiescent_msg) - } else { - panic!("Expected disconnect, got: {:?}", action); - } + // disconnect their counterparty if they're expecting a timely response. User force-closes and + // on-chain HTLC timeouts send specific error strings through the peer. + let expected = match action { + msgs::ErrorAction::DisconnectPeerWithWarning { msg } => { + // Warning-only disconnects do not close channels. They are accepted + // when the fuzzer delays normal protocol progress far enough for a + // peer timeout, or when splice quiescence needs to be exited. + msg.data.contains("Disconnecting due to timeout awaiting response") + || msg.data.contains("already sent splice_locked, cannot RBF") + }, + msgs::ErrorAction::SendErrorMessage { msg } => { + // Error messages can close channels, so they are accepted only for + // channels that were pre-authorized, or for the narrow timeout and + // wrong-node messages LDK can emit while peers race with on-chain + // confirmation. + let expected_close = matches!( + channel_closes.and_then(|closes| closes.get(&msg.channel_id)), + Some(ChannelCloseState::Expected) + ); + msg.data.starts_with(HTLC_TIMEOUT_ERROR_PREFIX) + || msg.data.starts_with(CLOSED_CHANNEL_WRONG_NODE_ERROR_PREFIX) + || (expected_close + && (FORCE_CLOSE_ERROR_MESSAGES.contains(&msg.data.as_str()) + || msg.data == ONCHAIN_TX_CONFIRMED_ERROR + || msg.data == INVALID_REESTABLISH_FORCE_CLOSE_ERROR + || msg.data.contains(NEEDED_CHANNEL_REESTABLISH_ERROR))) + }, + _ => false, + }; + assert!(expected, "Expected harness control error, got: {:?}", action); +} + +// Recognizes splice-quiescence warning disconnects. We need to distinguish +// them from real close errors so the harness can exit quiescence and continue. +fn is_quiescent_disconnect_warning(msg: &msgs::WarningMessage) -> bool { + msg.data.contains("already sent splice_locked, cannot RBF") } #[derive(Clone, Copy, PartialEq)] @@ -1044,6 +1128,7 @@ struct HarnessNode<'a> { broadcaster: Arc, fee_estimator: Arc, wallet: Arc, + bump_tx_handler: TestBumpTransactionEventHandler, persistence_style: ChannelMonitorUpdateStatus, deferred: bool, serialized_manager: Vec, @@ -1079,6 +1164,10 @@ impl<'a> HarnessNode<'a> { keys_manager: &Arc, logger: Arc, persister: &Arc, deferred: bool, ) -> Arc { + // The monitor gets the same broadcaster and fee estimator as the + // ChannelManager. That is important after force-close because monitor + // events, not manager events, can produce the claim and bump + // transactions this harness later mines. Arc::new(chainmonitor::ChainMonitor::new( None, Arc::clone(broadcaster), @@ -1133,6 +1222,17 @@ impl<'a> HarnessNode<'a> { params, best_block_timestamp, ); + let wallet_sync = Arc::new(WalletSync::new(Arc::clone(&wallet), Arc::clone(&logger))); + // BumpTransaction events need a wallet-backed handler so anchor and + // claim transactions can be completed and rebroadcast into the harness + // mempool. Without this, the force-close path would observe monitor + // events but never turn some of them into mineable transactions. + let bump_tx_handler = BumpTransactionEventHandlerSync::new( + Arc::clone(&broadcaster), + wallet_sync, + Arc::clone(&keys_manager), + Arc::clone(&logger), + ); Self { node_id, node, @@ -1143,6 +1243,7 @@ impl<'a> HarnessNode<'a> { broadcaster, fee_estimator, wallet, + bump_tx_handler, persistence_style, deferred, serialized_manager: Vec::new(), @@ -1193,7 +1294,28 @@ impl<'a> HarnessNode<'a> { // updates so CSV/CLTV-sensitive logic can run. This is the normal sync // path, so both the raw ChainMonitor and the ChannelManager receive the // callbacks and the node's tracked height advances to the target. - let mut height = self.height; + let connect_manager = true; + self.connect_chain_range_inner(chain_state, self.height, target_height, connect_manager); + self.height = target_height; + } + + // Connects a block range to the monitor and, optionally, the manager. We + // need the optional manager path to catch raw monitors up after reload + // without replaying callbacks to an already-current manager. + fn connect_chain_range_inner( + &self, chain_state: &ChainState, start_height: u32, target_height: u32, + connect_manager: bool, + ) { + // The ChainMonitor always receives callbacks because monitors own + // on-chain claim state. The ChannelManager is skipped only for raw + // monitor catch-up after reload, where the manager is already at + // target_height. + // Empty stretches can be connected as best-block updates, but blocks + // with transactions need the explicit transactions_confirmed callback + // before best_block_updated. This keeps the selected LDK consumers' + // chain views in sync without sending one callback per empty block when + // the fuzzer mines long runs. + let mut height = start_height; while height < target_height { let mut next_height = height + 1; while next_height <= target_height && chain_state.block_at(next_height).1.is_empty() { @@ -1208,7 +1330,9 @@ impl<'a> HarnessNode<'a> { height = target_height; let (header, _) = chain_state.block_at(height); self.monitor.best_block_updated(header, height); - self.node.best_block_updated(header, height); + if connect_manager { + self.node.best_block_updated(header, height); + } break; } if next_height > height + 1 { @@ -1220,7 +1344,9 @@ impl<'a> HarnessNode<'a> { height = next_height - 1; let (header, _) = chain_state.block_at(height); self.monitor.best_block_updated(header, height); - self.node.best_block_updated(header, height); + if connect_manager { + self.node.best_block_updated(header, height); + } } height = next_height; let (header, txn) = chain_state.block_at(height); @@ -1230,12 +1356,15 @@ impl<'a> HarnessNode<'a> { // before the best-block update so watched spends are delivered // in chain order before the node advances to that tip. self.monitor.transactions_confirmed(header, &txdata, height); - self.node.transactions_confirmed(header, &txdata, height); + if connect_manager { + self.node.transactions_confirmed(header, &txdata, height); + } } self.monitor.best_block_updated(header, height); - self.node.best_block_updated(header, height); + if connect_manager { + self.node.best_block_updated(header, height); + } } - self.height = target_height; } fn sync_with_chain_state(&mut self, chain_state: &ChainState, num_blocks: Option) { @@ -1301,9 +1430,11 @@ impl<'a> HarnessNode<'a> { // retry pending claim transactions. Holder signing becomes relevant after // on-chain close handling starts. fn enable_holder_signer_ops(&self) { - // Holder-side signing is requested when a node needs to build local - // on-chain claim transactions. Keep it separate from live-channel - // signing so fuzz inputs can choose when those requests unblock. + // Counterparty signing operations are enough for live-channel message + // flow. Holder signing operations become relevant after a commitment + // transaction is on chain, when LDK may ask the signer for local claim + // material. Keep this as a separate fuzz action so inputs can exercise + // blocked signer state before and after a close. self.keys_manager.enable_op_for_all_signers(SignerOp::SignHolderCommitment); self.keys_manager.enable_op_for_all_signers(SignerOp::SignHolderHtlcTransaction); self.monitor.signer_unblocked(None); @@ -1390,6 +1521,22 @@ impl<'a> HarnessNode<'a> { } } + // Drains raw ChannelMonitor events for this node. We need this because + // monitor-generated BumpTransaction events do not flow through the manager + // event queue but still produce transactions the harness must mine. + fn process_monitor_pending_events(&self) { + // ChannelMonitor events are not exposed through the ChannelManager event + // queue. Draining them here lets the harness see BumpTransaction events + // produced by on-chain monitoring, then hand them to the synchronous + // bump handler so the resulting transactions can be relayed and mined. + self.monitor.process_pending_events(&|event: events::Event| { + if let events::Event::BumpTransaction(ref bump) = event { + self.bump_tx_handler.handle_event(bump); + } + Ok(()) + }); + } + fn reload( &mut self, use_old_mons: u8, out: &Out, router: &'a FuzzRouter, chan_type: ChanType, ) -> u64 { @@ -1498,6 +1645,8 @@ enum MppHopChannels { } struct EventQueues { + // The topology is A-B-C. A and C never talk directly, so messages moving + // away from B are split into the A-side and C-side queues. ab: Vec, ba: Vec, bc: Vec, @@ -1512,11 +1661,16 @@ impl EventQueues { fn take_for_node(&mut self, node_idx: usize) -> Vec { match node_idx { 0 => { + // Node A receives only messages queued on the A-B link. let mut events = Vec::new(); mem::swap(&mut events, &mut self.ab); events }, 1 => { + // Node B is the middle node, so it receives from both edge + // peers. Draining both queues together preserves the old + // "process node B" behavior while still allowing each link to be + // disconnected independently. let mut events = Vec::new(); mem::swap(&mut events, &mut self.ba); events.extend_from_slice(&self.bc[..]); @@ -1552,8 +1706,11 @@ impl EventQueues { fn route_from_middle<'a, I: IntoIterator>( &mut self, excess_events: I, expect_drop_node: Option, nodes: &[HarnessNode<'a>; 3], + channel_closes: Option<&HashMap>, ) { - // Push any events from Node B onto queues.ba and queues.bc. + // Push any events from Node B onto queues.ba and queues.bc. The helper + // exists because B can produce messages for either edge peer, while A + // and C only ever send messages toward B. let a_id = nodes[0].get_our_node_id(); let expect_drop_id = expect_drop_node.map(|id| nodes[id].get_our_node_id()); for event in excess_events { @@ -1583,7 +1740,11 @@ impl EventQueues { *node_id == a_id }, MessageSendEvent::HandleError { ref action, ref node_id } => { - assert_disconnect_action(action); + // Error events are routed like normal peer messages, but + // first prove that this is one of the harness-expected error + // paths. This avoids accepting arbitrary LDK HandleError + // output just because force-close coverage was enabled. + assert_expected_control_error_action(action, channel_closes); if Some(*node_id) == expect_drop_id { panic!( "peer_disconnected should drop msgs bound for the disconnected peer" @@ -1605,6 +1766,8 @@ impl EventQueues { } fn clear_link(&mut self, link: &PeerLink) { + // Disconnection drops in-flight messages in both directions for the + // affected link, matching the wire-level effect of peer_disconnected. match (link.node_a, link.node_b) { (0, 1) | (1, 0) => { self.ab.clear(); @@ -1618,7 +1781,16 @@ impl EventQueues { } } - fn drain_on_disconnect(&mut self, edge_node: usize, nodes: &[HarnessNode<'_>; 3]) { + // Drains messages generated by a peer disconnect. We need this to validate + // disconnect-time HandleError events before dropping in-flight link traffic. + fn drain_on_disconnect( + &mut self, edge_node: usize, nodes: &[HarnessNode<'_>; 3], + channel_closes: &HashMap, + ) { + // Disconnecting a peer can itself generate messages or HandleError + // events. Drain them before clearing the link so the harness checks + // their contents, while still ensuring no message remains queued for the + // disconnected edge peer. match edge_node { 0 => { for event in nodes[0].get_and_clear_pending_msg_events() { @@ -1632,12 +1804,17 @@ impl EventQueues { MessageSendEvent::BroadcastChannelUpdate { .. } => {}, MessageSendEvent::SendChannelUpdate { .. } => {}, MessageSendEvent::HandleError { ref action, .. } => { - assert_disconnect_action(action); + assert_expected_control_error_action(action, Some(channel_closes)); }, _ => panic!("Unhandled message event"), } } - self.route_from_middle(nodes[1].get_and_clear_pending_msg_events(), Some(0), nodes); + self.route_from_middle( + nodes[1].get_and_clear_pending_msg_events(), + Some(0), + nodes, + Some(channel_closes), + ); }, 2 => { for event in nodes[2].get_and_clear_pending_msg_events() { @@ -1651,12 +1828,17 @@ impl EventQueues { MessageSendEvent::BroadcastChannelUpdate { .. } => {}, MessageSendEvent::SendChannelUpdate { .. } => {}, MessageSendEvent::HandleError { ref action, .. } => { - assert_disconnect_action(action); + assert_expected_control_error_action(action, Some(channel_closes)); }, _ => panic!("Unhandled message event"), } } - self.route_from_middle(nodes[1].get_and_clear_pending_msg_events(), Some(2), nodes); + self.route_from_middle( + nodes[1].get_and_clear_pending_msg_events(), + Some(2), + nodes, + Some(channel_closes), + ); }, _ => panic!("unsupported disconnected edge"), } @@ -1706,7 +1888,69 @@ impl PeerLink { } } - fn disconnect(&mut self, nodes: &[HarnessNode<'_>; 3], queues: &mut EventQueues) { + // Asserts that untracked sibling channels on this link are still listed. We + // need this local invariant to catch an accidental force close on the wrong + // channel, while the payment tracker checks whether tracked closes were + // actually expected. + fn assert_no_unexpected_channel_closes( + &self, nodes: &[HarnessNode<'_>; 3], channel_closes: &HashMap, + ) { + // A tracked channel can disappear from list_channels after a close. Every + // untracked channel on the same peer link must remain open. This catches + // a force close on the wrong sibling channel, and lets the global tracker + // decide whether the tracked close itself was expected. + let node_a_channels = nodes[self.node_a].list_channels(); + let node_b_channels = nodes[self.node_b].list_channels(); + for channel_id in &self.channel_ids { + if channel_closes.contains_key(channel_id) { + continue; + } + assert!( + node_a_channels.iter().any(|chan| chan.channel_id == *channel_id), + "Node {} no longer lists channel {:?} without an intentional force-close", + self.node_a, + channel_id, + ); + assert!( + node_b_channels.iter().any(|chan| chan.channel_id == *channel_id), + "Node {} no longer lists channel {:?} without an intentional force-close", + self.node_b, + channel_id, + ); + } + } + + // Records channels from this link that have already disappeared. We need + // this after event or chain processing, where a close may be observable only + // through list_channels state. + fn record_disappeared_channels( + &self, nodes: &[HarnessNode<'_>; 3], + channel_closes: &mut HashMap, + ) { + // A topology-only disappearance is recorded as Unexpected first. A later + // ChannelClosed event or timeout pre-check can upgrade the same channel + // to Expected; otherwise the final close invariant fails. + let node_a_channels = nodes[self.node_a].list_channels(); + let node_b_channels = nodes[self.node_b].list_channels(); + for channel_id in &self.channel_ids { + if channel_closes.contains_key(channel_id) { + continue; + } + let node_a_has = node_a_channels.iter().any(|chan| chan.channel_id == *channel_id); + let node_b_has = node_b_channels.iter().any(|chan| chan.channel_id == *channel_id); + if !node_a_has || !node_b_has { + channel_closes.insert(*channel_id, ChannelCloseState::Unexpected); + } + } + } + + // Disconnects both peers on this link and clears its queued traffic. We + // need the wrapper to preserve the harness's per-link queue model while + // still checking disconnect-generated errors. + fn disconnect( + &mut self, nodes: &[HarnessNode<'_>; 3], queues: &mut EventQueues, + channel_closes: &HashMap, + ) { if self.disconnected { return; } @@ -1715,6 +1959,9 @@ impl PeerLink { nodes[self.node_a].peer_disconnected(node_b_id); nodes[self.node_b].peer_disconnected(node_a_id); self.disconnected = true; + // Only links involving B are supported. The edge node is the peer whose + // local messages can be drained directly, while B's newly generated + // messages may need routing to the other still-connected edge. let edge_node = if self.node_a == 1 { self.node_b } else if self.node_b == 1 { @@ -1722,7 +1969,7 @@ impl PeerLink { } else { panic!("unsupported link topology") }; - queues.drain_on_disconnect(edge_node, nodes); + queues.drain_on_disconnect(edge_node, nodes, channel_closes); queues.clear_link(self); } @@ -1749,6 +1996,7 @@ impl PeerLink { fn disconnect_for_reload( &mut self, restarted_node: usize, nodes: &[HarnessNode<'_>; 3], queues: &mut EventQueues, + channel_closes: &HashMap, ) { if self.disconnected { return; @@ -1761,10 +2009,14 @@ impl PeerLink { self.disconnected = true; if remaining_node == 1 { + // If the surviving peer is B, it may still have messages for the + // other edge node. Route those while asserting nothing is queued for + // the restarted node that just disconnected. queues.route_from_middle( nodes[1].get_and_clear_pending_msg_events(), Some(restarted_node), nodes, + Some(channel_closes), ); } else { nodes[remaining_node].get_and_clear_pending_msg_events(); @@ -1773,99 +2025,469 @@ impl PeerLink { } } +// A tracked path stores only the data the cleanup invariants need later: +// `(channel_id, amount_msat, short_channel_id)`. The channel id lets a close +// or dust threshold mark a path as no longer resolvable through the live +// channel graph. The SCID mirrors the route information in PaymentPathFailed +// events so the harness can match LDK's failure event back to the tracked path. +type PaymentPath = Vec<(ChannelId, u64, u64)>; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum SenderOutcome { + // The sender observed PaymentSent for this payment hash. + Sent, + // The sender observed PaymentFailed for this payment hash. + Failed, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum PaymentStatus { + // The payment was accepted by send_payment_with_route and still needs + // either success, failure, or an explanation why every path became blocked. + Pending, + // The sender-side payment id was resolved by a normal event. + Resolved, + // The sender ChannelManager was reloaded from a generation older than the + // send. After that reload, the payment may legitimately never produce a + // sender-side resolution event. + RolledBack, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum ChannelCloseState { + // The harness has a valid reason for this channel to be closing or closed: + // an explicit force-close command, or an HTLC timeout detected during + // height sync. Expected channels are treated as unavailable for new sends. + Expected, + // The channel disappeared before the harness knew a valid close reason. + // This lets topology-only observations be recorded and checked by the final + // close invariant. + Unexpected, +} + +#[derive(Debug)] struct PendingPayment { + source_idx: usize, payment_id: PaymentId, payment_hash: PaymentHash, + // The manager generation expected to persist this payment. If a reload uses + // an older manager snapshot, the payment is marked RolledBack so the cleanup + // invariant does not wait for an event from a payment the manager forgot. first_persisted_manager_generation: u64, + paths: Vec, + // A blocked path is one that cannot reasonably finish through normal + // payment events anymore because a channel in that path closed or because + // the on-chain output would be below dust. + blocked_paths: HashSet, + // A failed path is one LDK explicitly reported through PaymentPathFailed. + failed_paths: HashSet, + status: PaymentStatus, + // Sender and receiver events are intentionally tracked independently. A + // claimed on-chain HTLC may produce receiver-side completion before the + // sender-side PaymentSent or PaymentFailed has propagated back. + sender_outcome: Option, + receiver_claimed: bool, + // Set after the harness calls claim_funds. It stays set until the receiver + // sees PaymentClaimed, or until LDK rejects the local claim before it can be + // made safe on chain. + claim_funds_called: bool, } -struct NodePayments { - pending: Vec, - resolved: HashMap>, +struct PaymentTracker { + payment_ctr: u64, + // PaymentId is the stable handle used by sender-side events. + records_by_id: HashMap, + // PaymentHash is the stable handle used by receiver-side and HTLC events. + ids_by_hash: HashMap, + // Preimages are deterministic so fabricated fuzz inputs can replay the same + // high-level scenario without relying on randomness. + payment_preimages: HashMap, + // Absent channels are still considered live. Present channels are closing + // or already closed, with the value recording whether the close was + // expected. Keeping one state per channel avoids having to compare separate + // "observed" and "allowed" sets. + channel_closes: HashMap, } -impl NodePayments { +impl PaymentTracker { fn new() -> Self { - Self { pending: Vec::new(), resolved: new_hash_map() } + Self { + payment_ctr: 0, + records_by_id: new_hash_map(), + ids_by_hash: new_hash_map(), + payment_preimages: new_hash_map(), + channel_closes: new_hash_map(), + } } - fn add_pending( - &mut self, payment_id: PaymentId, payment_hash: PaymentHash, - first_persisted_manager_generation: u64, + // Checks that compact path accounting matches the full Route. We need this + // boundary assertion because later cleanup reasons about stored SCIDs rather + // than the original Route object. + fn assert_path_scids_match_route(payment_paths: &[PaymentPath], route: &Route) { + // The tracker is intentionally small and stores only the route data it + // needs later. Check it at the boundary where we still have the full + // Route, so later accounting bugs fail close to their source. + assert_eq!( + payment_paths.len(), + route.paths.len(), + "tracked payment path count differs from route path count", + ); + for (payment_path, route_path) in payment_paths.iter().zip(route.paths.iter()) { + assert_eq!( + payment_path.len(), + route_path.hops.len(), + "tracked payment hop count differs from route hop count", + ); + for ((_, _, scid), hop) in payment_path.iter().zip(route_path.hops.iter()) { + assert_eq!( + *scid, hop.short_channel_id, + "tracked payment path SCID differs from route hop SCID", + ); + } + } + } + + // Starts tracking a successfully accepted outbound payment. We need this to + // tie sender events, receiver events, route paths, and reload generation + // state to the same payment id/hash pair. + fn register_payment( + &mut self, source_idx: usize, payment_id: PaymentId, payment_hash: PaymentHash, + payment_paths: Vec, first_persisted_manager_generation: u64, ) { - self.pending.push(PendingPayment { + // Every generated PaymentHash maps to exactly one PaymentId. Keeping + // both directions lets receiver-side events update the same record that + // sender-side events later resolve. + assert!( + self.ids_by_hash.insert(payment_hash, payment_id).is_none(), + "duplicate payment_hash {:?}", + payment_hash + ); + let record = PendingPayment { + source_idx, payment_id, payment_hash, first_persisted_manager_generation, - }); + paths: payment_paths, + blocked_paths: HashSet::new(), + failed_paths: HashSet::new(), + status: PaymentStatus::Pending, + sender_outcome: None, + receiver_claimed: false, + claim_funds_called: false, + }; + assert!( + self.records_by_id.insert(payment_id, record).is_none(), + "duplicate payment_id {:?}", + payment_id + ); } - fn mark_sent(&mut self, sent_id: PaymentId, payment_hash: PaymentHash) { - let idx_opt = self.pending.iter().position(|pending| pending.payment_id == sent_id); - if let Some(idx) = idx_opt { - self.pending.remove(idx); - self.resolved.insert(sent_id, Some(payment_hash)); - } else { - assert!(self.resolved.contains_key(&sent_id)); + // Looks up payment accounting by hash. We need this because receive-side + // and HTLC events identify payments by PaymentHash rather than PaymentId. + fn record_for_hash(&self, hash: &PaymentHash) -> Option<&PendingPayment> { + let payment_id = self.ids_by_hash.get(hash)?; + self.records_by_id.get(payment_id) + } + + // Mutably looks up payment accounting by hash. We need this for receive-side + // event handling that updates the sender-indexed payment record. + fn record_for_hash_mut(&mut self, hash: &PaymentHash) -> Option<&mut PendingPayment> { + let payment_id = self.ids_by_hash.get(hash).copied()?; + self.records_by_id.get_mut(&payment_id) + } + + // Marks payments forgotten by a stale manager reload as rolled back. We need + // this so final cleanup does not wait for sender events the reloaded manager + // can no longer produce. + fn sync_pending_with_manager_generation( + &mut self, node_idx: usize, loaded_manager_generation: u64, + ) -> Vec { + // Reloading from a stale ChannelManager snapshot can roll back a payment + // that was sent after the persisted generation. Such a payment may still + // have monitor or peer artifacts, but the reloaded manager cannot be + // expected to emit its original sender-side completion event. + let mut rolled_back_payment_hashes = Vec::new(); + for record in self.records_by_id.values_mut() { + if record.source_idx == node_idx + && record.status == PaymentStatus::Pending + && record.first_persisted_manager_generation > loaded_manager_generation + { + record.status = PaymentStatus::RolledBack; + rolled_back_payment_hashes.push(record.payment_hash); + } } + rolled_back_payment_hashes + } + + // Reports whether any claim_funds call still needs receiver or sender-side + // accounting. We need this as a cleanup-loop predicate for claimed HTLCs + // that may resolve over multiple blocks and events. + fn has_unfinished_claims(&self) -> bool { + self.records_by_id.values().any(|record| { + record.claim_funds_called + && (!record.receiver_claimed + || (record.sender_outcome.is_none() + && !record.paths.is_empty() + && !record + .paths + .iter() + .enumerate() + .all(|(path_idx, _)| record.blocked_paths.contains(&path_idx)))) + }) } - fn mark_resolved_without_hash(&mut self, payment_id: PaymentId) { - let idx_opt = self.pending.iter().position(|pending| pending.payment_id == payment_id); - if let Some(idx) = idx_opt { - self.pending.remove(idx); - self.resolved.insert(payment_id, None); - } else if !self.resolved.contains_key(&payment_id) { - // Some resolutions can arrive immediately, before the send helper records - // the payment as pending. Track them so later duplicate events are accepted. - self.resolved.insert(payment_id, None); + // Reports whether payment state can still make cleanup progress. We need + // this to keep driving events and blocks while pending paths or claims are + // still meaningfully live. + fn has_live_payment_work(&self) -> bool { + // The cleanup loop uses this as the "is there still a payment reason to + // keep driving events and blocks?" predicate. Pending route paths and + // incomplete claim_funds calls both count as live work. + self.records_by_id.values().any(|record| { + record.status == PaymentStatus::Pending + && !(record.paths.len() > 0 + && record.paths.iter().enumerate().all(|(path_idx, _)| { + record.blocked_paths.contains(&path_idx) + || record.failed_paths.contains(&path_idx) + })) + }) || self.has_unfinished_claims() + } + + // Returns whether claim_funds has been called for a payment hash. We need + // this to validate HTLCHandlingFailed events against outstanding claims. + fn claim_funds_called(&self, hash: &PaymentHash) -> bool { + self.record_for_hash(hash).map(|record| record.claim_funds_called).unwrap_or(false) + } + + // Clears the outstanding claim_funds obligation for a payment. We need this + // when LDK rejects the local claim before PaymentClaimed, or when a stale + // manager reload means the reloaded node can no longer emit that event. + fn clear_claim(&mut self, hash: &PaymentHash) { + // PaymentClaimBuffer failures mean LDK refused to proceed with the + // claim because it was no longer safe to claim locally. Clear the + // claim_funds obligation instead of waiting forever for PaymentClaimed. + if let Some(record) = self.record_for_hash_mut(hash) { + record.claim_funds_called = false; } } - fn mark_successful_probe(&mut self, payment_id: PaymentId) { - let idx_opt = self.pending.iter().position(|pending| pending.payment_id == payment_id); - if let Some(idx) = idx_opt { - self.pending.remove(idx); - self.resolved.insert(payment_id, None); - } else { - assert!(self.resolved.contains_key(&payment_id)); + // Blocks tracked paths for one payment that used a closed channel. We need + // this for HTLC-timeout close events that identify the affected hash. + fn block_paths_containing_channel(&mut self, hash: &PaymentHash, channel_id: ChannelId) { + // HTLC-timeout close events include a payment hash. Only paths for that + // payment that use the closed channel should be considered blocked. + if let Some(record) = self.record_for_hash_mut(hash) { + for path_idx in 0..record.paths.len() { + let path_contains_channel = + record.paths[path_idx].iter().any(|(chan_id, _, _)| *chan_id == channel_id); + if path_contains_channel { + record.blocked_paths.insert(path_idx); + } + } } } - fn sync_pending_with_manager_generation( - &mut self, loaded_manager_generation: u64, - ) -> Vec { - let mut rolled_back_payment_hashes = Vec::new(); - let pending = mem::take(&mut self.pending); - for pending_payment in pending { - if pending_payment.first_persisted_manager_generation > loaded_manager_generation { - rolled_back_payment_hashes.push(pending_payment.payment_hash); - } else { - self.pending.push(pending_payment); + // Blocks dust-sized path parts through a closed channel. We need this + // because dust HTLCs cannot produce an on-chain claim event to resolve the + // payment path later. + fn block_dust_paths_containing_channel(&mut self, channel_id: ChannelId, dust_limit_msat: u64) { + // The harness no longer records dust at force-close time. Instead, + // whenever a channel closes, it marks any path part below the standard + // dust threshold as blocked. This applies uniformly to explicit closes + // and timeout-driven closes. + for record in self.records_by_id.values_mut() { + for path_idx in 0..record.paths.len() { + if record.blocked_paths.contains(&path_idx) { + continue; + } + let path_contains_dust_part = + record.paths[path_idx].iter().any(|(chan_id, part_amt, _)| { + *chan_id == channel_id && *part_amt < dust_limit_msat + }); + if path_contains_dust_part { + record.blocked_paths.insert(path_idx); + } } } - rolled_back_payment_hashes } -} -struct PaymentTracker { - nodes: [NodePayments; 3], - claimed_payment_hashes: HashSet, - payment_preimages: HashMap, - payment_ctr: u64, -} + // Marks a channel close as expected. We need this to make explicit closes + // and timeout-derived closes unavailable for new sends immediately, while + // still letting final invariants reject channels that disappeared first. + fn expect_channel_close(&mut self, channel_id: ChannelId) { + // If topology observation saw the channel disappear first, upgrade that + // `Unexpected` state once LDK later reports a valid close reason. + self.channel_closes.insert(channel_id, ChannelCloseState::Expected); + } -impl PaymentTracker { - fn new() -> Self { - Self { - nodes: [NodePayments::new(), NodePayments::new(), NodePayments::new()], - claimed_payment_hashes: HashSet::new(), - payment_preimages: new_hash_map(), - payment_ctr: 0, + // Allows a channel close only after checking the HTLC-timeout predicate. We + // need this guard so height sync cannot accidentally whitelist arbitrary + // force closes. + fn allow_htlc_timeout_channel_close( + &mut self, channel_id: ChannelId, outbound_timed_out: bool, inbound_timed_out: bool, + ) { + // Height sync is the only place where a non-explicit close becomes + // expected. Keep the timeout predicate attached to the allowance so a + // future caller cannot mark arbitrary channels as timeout-closable. + assert!( + outbound_timed_out || inbound_timed_out, + "HTLC-timeout close allowance for channel {:?} without a timed-out HTLC", + channel_id, + ); + self.expect_channel_close(channel_id); + } + + // Blocks remaining pending paths through channels already closing or closed. + // We need this final cleanup step to distinguish stranded paths from live, + // still-resolvable payment work. + fn block_unresolvable_closed_paths(&mut self) { + // After cleanup, any remaining pending payment path through a tracked + // closed channel is unresolvable. Mark it blocked before asserting no + // pending payments; the separate close invariant then checks that the + // tracked close was expected. + let channel_closes = &self.channel_closes; + for record in self.records_by_id.values_mut() { + if record.status != PaymentStatus::Pending { + continue; + } + for path_idx in 0..record.paths.len() { + let path_finished = record.blocked_paths.contains(&path_idx) + || record.failed_paths.contains(&path_idx); + let path_contains_closed_channel = record.paths[path_idx] + .iter() + .any(|(chan_id, _, _)| channel_closes.contains_key(chan_id)); + if path_finished || !path_contains_closed_channel { + continue; + } + record.blocked_paths.insert(path_idx); + } + } + } + + // Records a path-level failure event against a tracked payment. We need this + // for MPP accounting, where individual paths can fail before the whole + // payment resolves. + fn mark_path_failed( + &mut self, node_idx: usize, payment_id: Option, failed_path: &Path, + ) { + // PaymentPathFailed may omit a payment id for events not tracked by + // this harness. When it has one, match by SCID sequence, which is the + // common representation shared between the original Route and the event. + let Some(payment_id) = payment_id else { return }; + let Some(record) = self.records_by_id.get_mut(&payment_id) else { return }; + assert_eq!(record.source_idx, node_idx); + if record.status != PaymentStatus::Pending { + return; + } + if let Some((path_idx, _)) = record.paths.iter().enumerate().find(|(path_idx, path)| { + !record.blocked_paths.contains(path_idx) + && !record.failed_paths.contains(path_idx) + && path.len() == failed_path.hops.len() + && path + .iter() + .zip(failed_path.hops.iter()) + .all(|((_, _, scid), hop)| *scid == hop.short_channel_id) + }) { + record.failed_paths.insert(path_idx); } } - // Returns a bool indicating whether the payment failed. + // Marks receiver-side completion for a payment hash. We need this because + // PaymentClaimed is independent from sender-side PaymentSent/PaymentFailed. + fn mark_receiver_claimed(&mut self, hash: PaymentHash) { + if let Some(record) = self.record_for_hash_mut(&hash) { + record.receiver_claimed = true; + } + } + + // Asserts no live pending payments remain. We need this final invariant to + // ensure cleanup resolved every path or proved it cannot make progress + // because the path failed, closed, or was dust-blocked. + fn assert_no_pending(&self, context: &str) { + // A pending payment is acceptable only if every path has either failed + // or been blocked. The reason for each close-backed block is checked by + // the channel-close invariant, so this assertion can focus on payment + // liveness. + let mut pending_ids = [Vec::new(), Vec::new(), Vec::new()]; + for record in self.records_by_id.values().filter(|record| { + let all_paths_finished = !record.paths.is_empty() + && record.paths.iter().enumerate().all(|(path_idx, _)| { + record.blocked_paths.contains(&path_idx) + || record.failed_paths.contains(&path_idx) + }); + record.status == PaymentStatus::Pending && !all_paths_finished + }) { + pending_ids[record.source_idx].push(record.payment_id); + } + for (idx, ids) in pending_ids.iter().enumerate() { + assert!( + ids.is_empty(), + "Node {} has {} stuck pending payments {}: ids={:?}", + idx, + ids.len(), + context, + ids, + ); + } + } + + // Asserts claim_funds calls reached valid receiver and sender outcomes. We + // need this because on-chain claims can resolve receiver-side before the + // sender learns success or an expected blocked-path failure. + fn assert_claims_resolved(&self) { + // claim_funds is special: after the receiver claims, we also need to + // verify the sender side either learned success, learned failure only + // after all paths were blocked, or was otherwise blocked from receiving + // a meaningful resolution. + for record in self.records_by_id.values().filter(|record| record.claim_funds_called) { + assert!( + record.receiver_claimed, + "Payment {:?} was claimed with claim_funds but receiver never got PaymentClaimed", + record.payment_hash, + ); + let all_paths_blocked = !record.paths.is_empty() + && record + .paths + .iter() + .enumerate() + .all(|(path_idx, _)| record.blocked_paths.contains(&path_idx)); + match record.sender_outcome { + Some(SenderOutcome::Sent) => {}, + Some(SenderOutcome::Failed) => assert!( + all_paths_blocked, + "claimed payment {:?} failed sender-side without every path blocked: \ + blocked={:?}, failed={:?}, paths={:?}", + record.payment_hash, record.blocked_paths, record.failed_paths, record.paths, + ), + None => assert!( + all_paths_blocked, + "claimed payment {:?} never resolved sender-side: blocked={:?}, \ + failed={:?}, paths={:?}", + record.payment_hash, record.blocked_paths, record.failed_paths, record.paths, + ), + } + } + } + + // Verifies that every tracked channel close became expected. We need this + // because closes can surface later as events or topology changes after the + // timeout or force-close command that made them expected. + fn assert_no_unexpected_channel_closes(&self) { + // This is the global close invariant. A close is acceptable only when + // an explicit fuzz command expected it or height sync detected an HTLC + // timeout condition for that channel. + for (channel_id, state) in &self.channel_closes { + assert!( + *state == ChannelCloseState::Expected, + "Channel {:?} closed without an explicit force-close or HTLC timeout", + channel_id, + ); + } + } + + // Checks whether LDK kept outbound state for a send attempt. We need this + // because a payment can exhaust retries immediately while committed HTLCs + // still need later accounting. fn check_payment_send_events(source: &ChanMan, sent_payment_id: PaymentId) -> bool { for payment in source.list_recent_payments() { match payment { @@ -1874,10 +2496,13 @@ impl PaymentTracker { { return true; }, - RecentPaymentDetails::Abandoned { payment_id, .. } - if payment_id == sent_payment_id => + RecentPaymentDetails::Abandoned { payment_id, payment_hash, .. } + if payment_id == sent_payment_id + && Self::has_pending_outbound_htlc(source, payment_hash) => { - return false; + // Retries may already be exhausted even though committed HTLCs are still + // in flight and will later resolve with PaymentFailed. + return true; }, _ => {}, } @@ -1885,7 +2510,22 @@ impl PaymentTracker { return false; } + // Detects whether a payment hash still has committed outbound HTLCs. We + // need this to keep exhausted-retry payments tracked until those HTLCs + // resolve through messages or chain activity. + fn has_pending_outbound_htlc(source: &ChanMan, payment_hash: PaymentHash) -> bool { + source.list_channels().iter().any(|chan| { + chan.pending_outbound_htlcs.iter().any(|htlc| htlc.payment_hash == payment_hash) + }) + } + + // Creates deterministic payment material and registers the inbound hash + // with the receiver. We need deterministic material so hand-built fuzz + // scenarios can be replayed while still using an LDK-accepted secret. fn next_payment(&mut self, dest: &ChanMan) -> (PaymentSecret, PaymentHash, PaymentId) { + // Derive payment material from the counter so route construction is + // reproducible across fuzz runs. The receiver still creates the inbound + // payment, which gives us a real PaymentSecret accepted by LDK. self.payment_ctr += 1; let mut payment_preimage = PaymentPreimage([0; 32]); payment_preimage.0[0..8].copy_from_slice(&self.payment_ctr.to_be_bytes()); @@ -1899,10 +2539,19 @@ impl PaymentTracker { (secret, hash, id) } + // Sends a single-hop payment and records it only if LDK kept outbound state. + // We need this as the base payment path that later close and claim + // accounting can reason about. fn send( &mut self, nodes: &[HarnessNode<'_>; 3], source_idx: usize, dest_idx: usize, dest_chan_id: ChannelId, amt: u64, ) -> bool { + if self.channel_closes.contains_key(&dest_chan_id) { + // Do not intentionally send over a channel the harness already knows + // is closed. That would test route construction failure, not + // force-close cleanup. + return false; + } let source = &nodes[source_idx]; let dest = &nodes[dest_idx]; let (secret, hash, id) = self.next_payment(dest); @@ -1919,10 +2568,12 @@ impl PaymentTracker { }) .unwrap_or((0, 0, 0)); let route_params = RouteParameters::from_payment_params_and_value( - PaymentParameters::from_node_id(source.get_our_node_id(), TEST_FINAL_CLTV), + PaymentParameters::from_node_id(source.get_our_node_id(), FUZZ_FINAL_CLTV_DELTA), amt, ); let route = Route { + // Direct sends use one path and one hop. The hand-built route keeps + // the harness deterministic and lets PaymentPath mirror it exactly. paths: vec![Path { hops: vec![RouteHop { pubkey: dest.get_our_node_id(), @@ -1930,13 +2581,15 @@ impl PaymentTracker { short_channel_id: dest_scid, channel_features: dest.channel_features(), fee_msat: amt, - cltv_expiry_delta: 200, + cltv_expiry_delta: FUZZ_FINAL_CLTV_DELTA, maybe_announced_channel: true, }], blinded_tail: None, }], route_params: Some(route_params.clone()), }; + let payment_paths = vec![vec![(dest_chan_id, amt, dest_scid)]]; + Self::assert_path_scids_match_route(&payment_paths, &route); let onion = RecipientOnionFields::secret_only(secret, amt); let res = source.send_payment_with_route(route, hash, onion, id); let succeeded = match res { @@ -1951,19 +2604,32 @@ impl PaymentTracker { }, }; if succeeded { - self.nodes[source_idx].add_pending( + self.register_payment( + source_idx, id, hash, + payment_paths, source.next_manager_persistence_generation(), ); } succeeded } + // Sends a two-hop payment through the middle node. We need this to cover + // forwarded HTLC cleanup and to record which channel on each link carried + // the payment. fn send_hop( &mut self, nodes: &[HarnessNode<'_>; 3], source_idx: usize, middle_idx: usize, middle_chan_id: ChannelId, dest_idx: usize, dest_chan_id: ChannelId, amt: u64, ) { + if self.channel_closes.contains_key(&middle_chan_id) + || self.channel_closes.contains_key(&dest_chan_id) + { + // Two-hop sends are useful only when both channels are still live. + // Once either hop closes, later cleanup should account for the + // existing payment, not create a new one over the closed path. + return; + } let source = &nodes[source_idx]; let middle = &nodes[middle_idx]; let dest = &nodes[dest_idx]; @@ -1980,15 +2646,20 @@ impl PaymentTracker { ) }) .unwrap_or((0, 0, 0)); - let dest_scid = dest + let Some(dest_scid) = dest .list_channels() .iter() .find(|chan| chan.channel_id == dest_chan_id) .and_then(|chan| chan.short_channel_id) - .unwrap_or(0); + else { + // The destination channel can already have disappeared due to a + // close observed by the destination before the tracker recorded it. + // Treat that as a no-op send attempt. + return; + }; let first_hop_fee = 50_000; let route_params = RouteParameters::from_payment_params_and_value( - PaymentParameters::from_node_id(source.get_our_node_id(), TEST_FINAL_CLTV), + PaymentParameters::from_node_id(source.get_our_node_id(), FUZZ_FINAL_CLTV_DELTA), amt, ); let route = Route { @@ -2000,7 +2671,7 @@ impl PaymentTracker { short_channel_id: middle_scid, channel_features: middle.channel_features(), fee_msat: first_hop_fee, - cltv_expiry_delta: 100, + cltv_expiry_delta: FUZZ_FORWARD_CLTV_DELTA, maybe_announced_channel: true, }, RouteHop { @@ -2009,7 +2680,7 @@ impl PaymentTracker { short_channel_id: dest_scid, channel_features: dest.channel_features(), fee_msat: amt, - cltv_expiry_delta: 200, + cltv_expiry_delta: FUZZ_FINAL_CLTV_DELTA, maybe_announced_channel: true, }, ], @@ -2017,6 +2688,11 @@ impl PaymentTracker { }], route_params: Some(route_params.clone()), }; + let payment_paths = vec![vec![ + (middle_chan_id, amt + first_hop_fee, middle_scid), + (dest_chan_id, amt, dest_scid), + ]]; + Self::assert_path_scids_match_route(&payment_paths, &route); let onion = RecipientOnionFields::secret_only(secret, amt); let res = source.send_payment_with_route(route, hash, onion, id); let succeeded = match res { @@ -2032,72 +2708,96 @@ impl PaymentTracker { }, }; if succeeded { - self.nodes[source_idx].add_pending( + self.register_payment( + source_idx, id, hash, + payment_paths, source.next_manager_persistence_generation(), ); } } - fn send_noret( - &mut self, nodes: &[HarnessNode<'_>; 3], source_idx: usize, dest_idx: usize, - dest_chan_id: ChannelId, amt: u64, - ) { - self.send(nodes, source_idx, dest_idx, dest_chan_id, amt); - } - - // Direct MPP payment (no hop) + // Sends a direct MPP payment with no intermediate hop. We need this to + // cover payments split across sibling channels while tracking each part + // separately when one channel closes. fn send_mpp_direct( &mut self, nodes: &[HarnessNode<'_>; 3], source_idx: usize, dest_idx: usize, dest_chan_ids: &[ChannelId], amt: u64, ) { + // MPP sends filter out already-closed channels but keep using any live + // sibling channels. This preserves coverage where one part is blocked + // by a close while later fuzz bytes still try to use remaining channels. + let live_dest_chan_ids = { + dest_chan_ids + .iter() + .copied() + .filter(|chan_id| !self.channel_closes.contains_key(chan_id)) + .collect::>() + }; + if live_dest_chan_ids.is_empty() { + return; + } let source = &nodes[source_idx]; let dest = &nodes[dest_idx]; let (secret, hash, id) = self.next_payment(dest); - let num_paths = dest_chan_ids.len(); + let mut paths = Vec::new(); + let dest_chans = dest.list_channels(); + let dest_scids: Vec<_> = live_dest_chan_ids + .iter() + .filter_map(|chan_id| { + dest_chans + .iter() + .find(|chan| chan.channel_id == *chan_id) + .and_then(|chan| chan.short_channel_id) + .map(|scid| (*chan_id, scid)) + }) + .collect(); + let num_paths = dest_scids.len(); if num_paths == 0 { return; } - let amt_per_path = amt / num_paths as u64; - let mut paths = Vec::with_capacity(num_paths); - - let dest_chans = dest.list_channels(); - let dest_scids = dest_chan_ids.iter().map(|chan_id| { - dest_chans - .iter() - .find(|chan| chan.channel_id == *chan_id) - .and_then(|chan| chan.short_channel_id) - .unwrap() - }); - - for (i, dest_scid) in dest_scids.enumerate() { + for (i, (_, dest_scid)) in dest_scids.iter().enumerate() { let path_amt = if i == num_paths - 1 { amt - amt_per_path * (num_paths as u64 - 1) } else { amt_per_path }; - paths.push(Path { hops: vec![RouteHop { pubkey: dest.get_our_node_id(), node_features: dest.node_features(), - short_channel_id: dest_scid, + short_channel_id: *dest_scid, channel_features: dest.channel_features(), fee_msat: path_amt, - cltv_expiry_delta: 200, + cltv_expiry_delta: FUZZ_FINAL_CLTV_DELTA, maybe_announced_channel: true, }], blinded_tail: None, }); } - let route_params = RouteParameters::from_payment_params_and_value( - PaymentParameters::from_node_id(dest.get_our_node_id(), TEST_FINAL_CLTV), + PaymentParameters::from_node_id(dest.get_our_node_id(), FUZZ_FINAL_CLTV_DELTA), amt, ); let route = Route { paths, route_params: Some(route_params) }; + let payment_paths = dest_scids + .iter() + .enumerate() + .map(|(i, (chan_id, dest_scid))| { + // Match the same remainder handling used while building the + // Route so the accounting amount exactly equals the route amount + // for each part. + let path_amt = if i == num_paths - 1 { + amt - amt_per_path * (num_paths as u64 - 1) + } else { + amt_per_path + }; + vec![(*chan_id, path_amt, *dest_scid)] + }) + .collect::>(); + Self::assert_path_scids_match_route(&payment_paths, &route); let onion = RecipientOnionFields::secret_only(secret, amt); let res = source.send_payment_with_route(route, hash, onion, id); let succeeded = match res { @@ -2105,62 +2805,80 @@ impl PaymentTracker { Ok(()) => Self::check_payment_send_events(source, id), }; if succeeded { - self.nodes[source_idx].add_pending( + self.register_payment( + source_idx, id, hash, + payment_paths, source.next_manager_persistence_generation(), ); } } - // MPP payment via hop - splits payment across multiple channels on either or both hops + // Sends a two-hop MPP payment split across one or both links. We need this + // to cover path-level failure and close accounting when only some parts of a + // forwarded payment are stranded. fn send_mpp_hop( &mut self, nodes: &[HarnessNode<'_>; 3], source_idx: usize, middle_idx: usize, middle_chan_ids: &[ChannelId], dest_idx: usize, dest_chan_ids: &[ChannelId], amt: u64, ) { + // For two-hop MPP, the path count is the maximum number of live channels + // on either side. The shorter side is reused round-robin, creating paths + // that can be independently blocked by either the first or second hop. + let (live_middle_chan_ids, live_dest_chan_ids) = { + ( + middle_chan_ids + .iter() + .copied() + .filter(|chan_id| !self.channel_closes.contains_key(chan_id)) + .collect::>(), + dest_chan_ids + .iter() + .copied() + .filter(|chan_id| !self.channel_closes.contains_key(chan_id)) + .collect::>(), + ) + }; + if live_middle_chan_ids.is_empty() || live_dest_chan_ids.is_empty() { + return; + } let source = &nodes[source_idx]; let middle = &nodes[middle_idx]; let dest = &nodes[dest_idx]; let (secret, hash, id) = self.next_payment(dest); - // Create paths by pairing middle_scids with dest_scids. - let num_paths = middle_chan_ids.len().max(dest_chan_ids.len()); - if num_paths == 0 { - return; - } - - let first_hop_fee = 50_000; - let amt_per_path = amt / num_paths as u64; - let fee_per_path = first_hop_fee / num_paths as u64; - let mut paths = Vec::with_capacity(num_paths); - let middle_chans = middle.list_channels(); - let middle_scids: Vec<_> = middle_chan_ids + let middle_scids: Vec<_> = live_middle_chan_ids .iter() - .map(|chan_id| { + .filter_map(|chan_id| { middle_chans .iter() .find(|chan| chan.channel_id == *chan_id) .and_then(|chan| chan.short_channel_id) - .unwrap() + .map(|scid| (*chan_id, scid)) }) .collect(); - let dest_chans = dest.list_channels(); - let dest_scids: Vec<_> = dest_chan_ids + let dest_scids: Vec<_> = live_dest_chan_ids .iter() - .map(|chan_id| { + .filter_map(|chan_id| { dest_chans .iter() .find(|chan| chan.channel_id == *chan_id) .and_then(|chan| chan.short_channel_id) - .unwrap() + .map(|scid| (*chan_id, scid)) }) .collect(); - + let num_paths = middle_scids.len().max(dest_scids.len()); + if middle_scids.is_empty() || dest_scids.is_empty() { + return; + } + let first_hop_fee = 50_000; + let amt_per_path = amt / num_paths as u64; + let fee_per_path = first_hop_fee / num_paths as u64; + let mut paths = Vec::with_capacity(num_paths); for i in 0..num_paths { - let middle_scid = middle_scids[i % middle_scids.len()]; - let dest_scid = dest_scids[i % dest_scids.len()]; - + let (_, middle_scid) = middle_scids[i % middle_scids.len()]; + let (_, dest_scid) = dest_scids[i % dest_scids.len()]; let path_amt = if i == num_paths - 1 { amt - amt_per_path * (num_paths as u64 - 1) } else { @@ -2171,7 +2889,6 @@ impl PaymentTracker { } else { fee_per_path }; - paths.push(Path { hops: vec![ RouteHop { @@ -2180,7 +2897,7 @@ impl PaymentTracker { short_channel_id: middle_scid, channel_features: middle.channel_features(), fee_msat: path_fee, - cltv_expiry_delta: 100, + cltv_expiry_delta: FUZZ_FORWARD_CLTV_DELTA, maybe_announced_channel: true, }, RouteHop { @@ -2189,19 +2906,39 @@ impl PaymentTracker { short_channel_id: dest_scid, channel_features: dest.channel_features(), fee_msat: path_amt, - cltv_expiry_delta: 200, + cltv_expiry_delta: FUZZ_FINAL_CLTV_DELTA, maybe_announced_channel: true, }, ], blinded_tail: None, }); } - let route_params = RouteParameters::from_payment_params_and_value( - PaymentParameters::from_node_id(dest.get_our_node_id(), TEST_FINAL_CLTV), + PaymentParameters::from_node_id(dest.get_our_node_id(), FUZZ_FINAL_CLTV_DELTA), amt, ); let route = Route { paths, route_params: Some(route_params) }; + let payment_paths = (0..num_paths) + .map(|i| { + let (middle_chan_id, middle_scid) = middle_scids[i % middle_scids.len()]; + let (dest_chan_id, dest_scid) = dest_scids[i % dest_scids.len()]; + let path_amt = if i == num_paths - 1 { + amt - amt_per_path * (num_paths as u64 - 1) + } else { + amt_per_path + }; + let path_fee = if i == num_paths - 1 { + first_hop_fee - fee_per_path * (num_paths as u64 - 1) + } else { + fee_per_path + }; + vec![ + (middle_chan_id, path_amt + path_fee, middle_scid), + (dest_chan_id, path_amt, dest_scid), + ] + }) + .collect::>(); + Self::assert_path_scids_match_route(&payment_paths, &route); let onion = RecipientOnionFields::secret_only(secret, amt); let res = source.send_payment_with_route(route, hash, onion, id); let succeeded = match res { @@ -2209,48 +2946,96 @@ impl PaymentTracker { Ok(()) => Self::check_payment_send_events(source, id), }; if succeeded { - self.nodes[source_idx].add_pending( + self.register_payment( + source_idx, id, hash, + payment_paths, source.next_manager_persistence_generation(), ); } } + // Claims or fails a receive-side payment event. We need this helper to keep + // claim_funds bookkeeping tied to the deterministic preimage map. fn claim_payment(&mut self, node: &HarnessNode<'_>, payment_hash: PaymentHash, fail: bool) { if fail { + // Event-processing fuzz bytes can choose to fail claimable payments + // backward, preserving existing chanmon coverage where payments are + // rejected rather than claimed. node.fail_htlc_backwards(&payment_hash); } else { + // The preimage was generated by next_payment, so a missing entry + // means LDK produced a claimable event for a payment outside the + // harness accounting model. let payment_preimage = *self .payment_preimages .get(&payment_hash) .expect("PaymentClaimable for unknown payment hash"); node.claim_funds(payment_preimage); + self.record_for_hash_mut(&payment_hash) + .expect("PaymentClaimable for unknown payment record") + .claim_funds_called = true; } } - fn assert_all_resolved(&self) { - for (idx, node) in self.nodes.iter().enumerate() { - assert!( - node.pending.is_empty(), - "Node {} has {} stuck pending payments after settling all state", - idx, - node.pending.len() - ); + // Records sender-side payment success and clears the pending record. We need + // this outcome to cross-check receiver claims and blocked-path accounting. + fn mark_sent(&mut self, node_idx: usize, sent_id: PaymentId, payment_hash: PaymentHash) { + if let Some(record) = self.record_for_hash_mut(&payment_hash) { + assert_ne!(record.sender_outcome, Some(SenderOutcome::Failed)); + record.sender_outcome = Some(SenderOutcome::Sent); } + self.mark_resolved_payment(node_idx, sent_id, true); } - fn assert_claims_reported(&self) { - for hash in self.claimed_payment_hashes.iter() { - let found = self - .nodes - .iter() - .any(|node| node.resolved.values().any(|h| h.as_ref() == Some(hash))); - assert!( - found, - "Payment {:?} was claimed by receiver but sender never got PaymentSent", - hash - ); + // Records sender-side payment failure and clears the pending record. We need + // this to distinguish valid failures caused by blocked paths from failures + // that would contradict a receiver-side claim. + fn mark_failed( + &mut self, node_idx: usize, payment_id: PaymentId, payment_hash: Option, + ) { + // PaymentFailed may carry the hash, but older or probe-like paths can + // resolve by id only. Look up the hash from the record when possible so + // sender_outcome remains tied to the same receiver-side record. + let payment_hash = payment_hash + .or_else(|| self.records_by_id.get(&payment_id).map(|record| record.payment_hash)); + if let Some(payment_hash) = payment_hash { + if let Some(record) = self.record_for_hash_mut(&payment_hash) { + assert_ne!(record.sender_outcome, Some(SenderOutcome::Sent)); + record.sender_outcome = Some(SenderOutcome::Failed); + } + } + self.mark_resolved_payment(node_idx, payment_id, false); + } + + // Resolves an id-only payment event. We need this for failure paths where + // LDK does not include the payment hash but still clears sender state. + fn mark_resolved_without_hash(&mut self, node_idx: usize, payment_id: PaymentId) { + self.mark_resolved_payment(node_idx, payment_id, false); + } + + // Moves a tracked payment id out of Pending. We need one transition point so + // success, hash-bearing failure, and id-only failure all update the same + // accounting state. + fn mark_resolved_payment( + &mut self, node_idx: usize, payment_id: PaymentId, assert_already_resolved: bool, + ) { + // Some events can arrive before the send helper records the payment, + // especially when the payment is immediately rejected. In non-strict + // mode, ignore those unknown ids. In strict mode, PaymentSent for an + // unknown id would be a tracker bug. + let Some(record) = self.records_by_id.get_mut(&payment_id) else { + assert!(!assert_already_resolved); + return; + }; + assert_eq!(record.source_idx, node_idx); + if record.status == PaymentStatus::Pending { + record.status = PaymentStatus::Resolved; + } else if assert_already_resolved { + assert_eq!(record.status, PaymentStatus::Resolved); + } else { + record.status = PaymentStatus::Resolved; } } } @@ -2268,7 +3053,14 @@ struct Harness<'a, Out: Output + MaybeSend + MaybeSync> { fn build_node_config(chan_type: ChanType) -> UserConfig { let mut config = UserConfig::default(); + // Keep forwarding fees at zero so hand-written routes with simple amounts + // remain valid. The first-hop fee used in the route is explicit where it + // matters for payment accounting. config.channel_config.forwarding_fee_proportional_millionths = 0; + // Tight CLTV deltas make timeout-driven force closes reachable with short + // fuzz inputs. These are still LDK-supported minimums, not arbitrary + // test-only values. + config.channel_config.cltv_expiry_delta = MIN_CLTV_EXPIRY_DELTA; config.channel_handshake_config.announce_for_forwarding = true; config.reject_inbound_splices = false; match chan_type { @@ -2288,10 +3080,25 @@ fn build_node_config(chan_type: ChanType) -> UserConfig { config } -fn assert_test_invariants(nodes: &[HarnessNode<'_>; 3]) { - assert_eq!(nodes[0].list_channels().len(), 3); - assert_eq!(nodes[1].list_channels().len(), 6); - assert_eq!(nodes[2].list_channels().len(), 3); +// Checks topology and broadcast invariants that should hold after cleanup. We +// need this final guard to catch harness bugs that leave stray broadcasts or +// accidentally lose open channels while allowing tracked closed channels. +fn assert_test_invariants(nodes: &[HarnessNode<'_>; 3], has_closed_channels: bool) { + let channel_counts = [ + nodes[0].list_channels().len(), + nodes[1].list_channels().len(), + nodes[2].list_channels().len(), + ]; + if has_closed_channels { + // Once a close is tracked, some channels may legitimately be absent from + // list_channels. Still assert that no node ever gains channels, which + // would indicate the harness lost track of topology. + assert!(channel_counts[0] <= 3); + assert!(channel_counts[1] <= 6); + assert!(channel_counts[2] <= 3); + } else { + assert_eq!(channel_counts, [3, 6, 3]); + } // All broadcasters should be empty because broadcast transactions only enter // the modeled mempool through explicit relay commands or finish cleanup. @@ -2394,6 +3201,9 @@ fn make_channel( .. } = events.pop().unwrap() { + // Funding transactions are synthetic no-input transactions. The + // version number is varied per test channel solely to make txids and + // funding outpoints distinct. let tx = Transaction { version: Version(chan_id), lock_time: LockTime::ZERO, @@ -2410,6 +3220,9 @@ fn make_channel( tx.clone(), ) .unwrap(); + // Mine through the chain model instead of directly notifying nodes, + // so the setup path and later force-close path share the same block + // representation. chain_state.mine_setup_tx_to_depth(tx, DEFAULT_TX_CONFIRMATION_BLOCKS); } else { panic!("Wrong event type"); @@ -2533,9 +3346,10 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { let wallets = [wallet_a.as_ref(), wallet_b.as_ref(), wallet_c.as_ref()]; let mut chain_state = ChainState::new(); for wallet in wallets { - // Seed each wallet with many confirmed outputs. Splice flows may need - // fresh inputs long after the channel setup phase, and exhausting the - // wallet would obscure the transaction-relay behavior under test. + // Seed each wallet with many confirmed outputs. Anchor and splice + // flows may need fresh inputs long after the channel setup phase, and + // exhausting the wallet would obscure the force-close behavior under + // test. let coinbase_tx = bitcoin::Transaction { version: bitcoin::transaction::Version::TWO, lock_time: bitcoin::absolute::LockTime::ZERO, @@ -2551,7 +3365,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { wallet.add_utxo(coinbase_tx.clone(), vout); } // Mine the wallet UTXOs into the same ChainState that later drives - // channel funding and splice transactions. + // channel funding, splice transactions, and on-chain claims. chain_state.mine_setup_tx_to_depth(coinbase_tx, DEFAULT_TX_CONFIRMATION_BLOCKS); } @@ -2599,6 +3413,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { chan_type, ), ]; + // Connect peers first, then create channels. connect_peers(&nodes[0], &nodes[1]); connect_peers(&nodes[1], &nodes[2]); @@ -2623,11 +3438,14 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { // relay queue. Later broadcasts only enter the mempool through relay // commands or finish cleanup. for node in &nodes { + // Channel setup broadcasts synthetic funding transactions. Clear the + // broadcasters after setup because all setup funding is already + // represented in ChainState. node.broadcaster.txn_broadcasted.borrow_mut().clear(); } - - // Sync all nodes to tip to lock the funding. for node in &mut nodes { + // Sync every node to the funding depth before channel_ready exchange + // so all channels start from the same confirmed chain view. node.sync_with_chain_state(&chain_state, None); } @@ -2672,32 +3490,40 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { // Final invariants should not depend on the input ending with explicit relay // and mining bytes. fn finish(&mut self) { - for _ in 0..MAX_FINISH_RELAY_MINE_ROUNDS { - // Finish paths should not leave already-broadcast transactions - // stranded. Relay all broadcasts, mine them to normal depth, and - // repeat because confirmed transactions can trigger later broadcasts. - let mut txs = Vec::new(); - for node in &self.nodes { - txs.extend(node.broadcaster.txn_broadcasted.borrow_mut().drain(..)); - } - self.chain_state.relay_transactions(txs); - if self.chain_state.pending_txs.is_empty() { - assert_test_invariants(&self.nodes); - return; - } - assert!( - self.mine_blocks(DEFAULT_TX_CONFIRMATION_BLOCKS) > 0, - "finish cannot mine pending mempool transactions without crossing an unresolved HTLC timeout deadline" - ); - } - assert!( - !self.nodes.iter().any(|node| !node.broadcaster.txn_broadcasted.borrow().is_empty()) - && self.chain_state.pending_txs.is_empty(), - "finish tx mining loop failed to quiesce", - ); - assert_test_invariants(&self.nodes); - } - + // A fuzz input may stop immediately after a broadcast or relay. Before + // final invariants, mine those transactions to avoid reporting a false + // stuck state just because the byte stream ended. + self.mine_relayed_txs_until_quiet("finish"); + self.record_disappeared_channels(); + self.assert_no_unexpected_channel_closes(); + let has_closed_channels = !self.payments.channel_closes.is_empty(); + assert_test_invariants(&self.nodes, has_closed_channels); + } + + // Checks both peer links for unintended sibling-channel closes. We need this + // harness-level wrapper so final validation covers the whole A-B-C topology. + fn assert_no_unexpected_channel_closes(&self) { + self.ab_link + .assert_no_unexpected_channel_closes(&self.nodes, &self.payments.channel_closes); + self.bc_link + .assert_no_unexpected_channel_closes(&self.nodes, &self.payments.channel_closes); + } + + // Records closes that are visible through current topology state. We need + // this because some closes are observed as missing channels instead of, or + // before, explicit ChannelClosed accounting. + fn record_disappeared_channels(&mut self) { + // Run this from both finish and settle_all because a close can be + // observed through ChannelClosed events or simply through a channel + // disappearing from list_channels after chain/message processing. The + // tracker records topology-only disappearance as Unexpected until a valid + // close reason upgrades it. + self.ab_link.record_disappeared_channels(&self.nodes, &mut self.payments.channel_closes); + self.bc_link.record_disappeared_channels(&self.nodes, &mut self.payments.channel_closes); + } + + // Finds the peer link for a source/destination pair. Payment opcodes work in + // node indexes, while accounting needs the concrete channels on that link. fn link_between(&self, source_idx: usize, dest_idx: usize) -> &PeerLink { if self.ab_link.connects(source_idx, dest_idx) { &self.ab_link @@ -2708,25 +3534,25 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { } } + // Returns all channel ids on a peer link so MPP opcodes can spread parts + // across sibling channels. fn channel_ids_between(&self, source_idx: usize, dest_idx: usize) -> [ChannelId; 3] { self.link_between(source_idx, dest_idx).channel_ids().clone() } + // Returns the first channel on a peer link for simple single-path opcodes. fn first_channel_id_between(&self, source_idx: usize, dest_idx: usize) -> ChannelId { self.link_between(source_idx, dest_idx).first_channel_id() } - fn send_on_channel( - &mut self, source_idx: usize, dest_idx: usize, dest_chan_id: ChannelId, amt: u64, - ) -> bool { - self.payments.send(&self.nodes, source_idx, dest_idx, dest_chan_id, amt) - } - + // Sends a direct single-path payment over the first channel on the selected + // link. fn send(&mut self, source_idx: usize, dest_idx: usize, amt: u64) { let dest_chan_id = self.first_channel_id_between(source_idx, dest_idx); - self.payments.send_noret(&self.nodes, source_idx, dest_idx, dest_chan_id, amt); + self.payments.send(&self.nodes, source_idx, dest_idx, dest_chan_id, amt); } + // Sends a two-hop payment over the first channel on each selected link. fn send_hop(&mut self, source_idx: usize, middle_idx: usize, dest_idx: usize, amt: u64) { let middle_chan_id = self.first_channel_id_between(source_idx, middle_idx); let dest_chan_id = self.first_channel_id_between(middle_idx, dest_idx); @@ -2741,6 +3567,9 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { ); } + // Converts a fuzz MPP-direct selector into concrete channels. We need this + // wrapper to keep byte-level variants separate from PaymentTracker's route + // and path accounting. fn send_mpp_direct( &mut self, source_idx: usize, dest_idx: usize, channels: MppDirectChannels, amt: u64, ) { @@ -2769,6 +3598,8 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { } } + // Converts a fuzz MPP-hop selector into concrete first-hop and second-hop + // channel sets. fn send_mpp_hop( &mut self, source_idx: usize, middle_idx: usize, dest_idx: usize, channels: MppHopChannels, amt: u64, @@ -2925,7 +3756,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { fn process_msg_event( node_idx: usize, source_node_id: PublicKey, event: MessageSendEvent, corrupt_forward: bool, limit_events: ProcessMessages, nodes: &[HarnessNode<'_>; 3], - out: &Out, + channel_closes: &HashMap, out: &Out, ) -> Option { match event { MessageSendEvent::UpdateHTLCs { node_id, channel_id, updates } => { @@ -2948,6 +3779,13 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { None }, MessageSendEvent::SendChannelReestablish { ref node_id, ref msg } => { + if channel_closes.contains_key(&msg.channel_id) { + // Once a channel is tracked as closing or closed, a + // delayed reestablish for it is stale harness traffic. + // Dropping it avoids turning the same tracked close into + // a second unrelated error path. + return None; + } let dest_idx = log_peer_message(node_idx, node_id, nodes, out, "channel_reestablish"); nodes[dest_idx].handle_channel_reestablish(source_node_id, msg); @@ -3020,15 +3858,35 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { nodes[dest_idx].handle_splice_locked(source_node_id, msg); None }, - MessageSendEvent::HandleError { ref action, ref node_id, .. } => { - let (msg, is_quiescent) = assert_disconnect_action(action); - let dest_idx = log_peer_message(node_idx, node_id, nodes, out, "warning"); - if is_quiescent { - nodes[node_idx].node.exit_quiescence(node_id, &msg.channel_id).unwrap(); - nodes[dest_idx] - .node - .exit_quiescence(&source_node_id, &msg.channel_id) - .unwrap(); + MessageSendEvent::HandleError { ref action, ref node_id } => { + assert_expected_control_error_action(action, Some(channel_closes)); + match action { + msgs::ErrorAction::DisconnectPeerWithWarning { msg } => { + let dest_idx = + log_peer_message(node_idx, node_id, nodes, out, "warning"); + if is_quiescent_disconnect_warning(msg) { + // Splice quiescence warnings are recovery + // signals, not channel closes. Exit + // quiescence on both peers so later fuzz bytes + // can continue normal message flow. + nodes[node_idx] + .node + .exit_quiescence(node_id, &msg.channel_id) + .unwrap(); + nodes[dest_idx] + .node + .exit_quiescence(&source_node_id, &msg.channel_id) + .unwrap(); + } + }, + msgs::ErrorAction::SendErrorMessage { msg } => { + // SendErrorMessage is delivered to the peer so the + // counterparty observes the close or protocol error + // the same way it would over the wire. + let dest_idx = log_peer_message(node_idx, node_id, nodes, out, "error"); + nodes[dest_idx].handle_error(source_node_id, msg); + }, + _ => unreachable!(), } None }, @@ -3050,9 +3908,13 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { let nodes = &self.nodes; let out = &self.out; let queues = &mut self.queues; + let channel_closes = &self.payments.channel_closes; let mut events = queues.take_for_node(node_idx); let mut new_events = Vec::new(); if limit_events != ProcessMessages::OnePendingMessage { + // Calling get_and_clear_pending_msg_events can itself release HTLC + // holding cells. OnePendingMessage intentionally avoids that side + // effect, while the other modes include fresh events. new_events = nodes[node_idx].get_and_clear_pending_msg_events(); } let mut had_events = false; @@ -3068,6 +3930,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { corrupt_forward, limit_events, nodes, + channel_closes, out, ); if limit_events != ProcessMessages::AllMessages { @@ -3076,7 +3939,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { } if node_idx == 1 { let remaining = extra_ev.into_iter().chain(events_iter).collect::>(); - queues.route_from_middle(remaining, None, nodes); + queues.route_from_middle(remaining, None, nodes, Some(channel_closes)); } else if node_idx == 0 { if let Some(ev) = extra_ev { queues.push_for_node(0, ev); @@ -3091,6 +3954,9 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { had_events } + // Drains user-facing events from one node and updates harness accounting. + // We need this as the bridge from LDK events into payment, close, splice, + // and bump-transaction state tracked by the fuzzer. fn process_events(&mut self, node_idx: usize, fail: bool) -> bool { let nodes = &self.nodes; let payments = &mut self.payments; @@ -3107,27 +3973,62 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { } }, events::Event::PaymentSent { payment_id, payment_hash, .. } => { - payments.nodes[node_idx].mark_sent(payment_id.unwrap(), payment_hash); + // PaymentSent is the sender-side success signal. It may + // arrive after receiver-side PaymentClaimed if the HTLC was + // claimed on chain and then propagated back. + payments.mark_sent(node_idx, payment_id.unwrap(), payment_hash); }, // Even though we don't explicitly send probes, because probes are detected based on // hashing the payment hash+preimage, it is rather trivial for the fuzzer to build // payments that accidentally end up looking like probes. events::Event::ProbeSuccessful { payment_id, .. } => { - payments.nodes[node_idx].mark_successful_probe(payment_id); + payments.mark_resolved_without_hash(node_idx, payment_id); + }, + events::Event::PaymentFailed { payment_id, payment_hash, .. } => { + // PaymentFailed resolves the sender-side id. The hash is + // optional in the event, so the tracker fills it from the + // record when possible. + payments.mark_failed(node_idx, payment_id, payment_hash); }, - events::Event::PaymentFailed { payment_id, .. } - | events::Event::ProbeFailed { payment_id, .. } => { - payments.nodes[node_idx].mark_resolved_without_hash(payment_id); + events::Event::ProbeFailed { payment_id, .. } => { + payments.mark_resolved_without_hash(node_idx, payment_id); }, events::Event::PaymentClaimed { payment_hash, .. } => { - if payments.payment_preimages.contains_key(&payment_hash) { - payments.claimed_payment_hashes.insert(payment_hash); - } + // PaymentClaimed is the receiver-side confirmation that a + // previous claim_funds call completed. + payments.mark_receiver_claimed(payment_hash); }, events::Event::PaymentPathSuccessful { .. } => {}, - events::Event::PaymentPathFailed { .. } => {}, + events::Event::PaymentPathFailed { payment_id, path, .. } => { + // Path-level failure lets MPP accounting finish individual + // paths without requiring the entire payment to fail. + payments.mark_path_failed(node_idx, payment_id, &path); + }, events::Event::PaymentForwarded { .. } if node_idx == 1 => {}, events::Event::ChannelReady { .. } => {}, + events::Event::HTLCHandlingFailed { + failure_type: events::HTLCHandlingFailureType::Receive { payment_hash }, + failure_reason: + Some(events::HTLCHandlingFailureReason::Local { + reason: LocalHTLCFailureReason::PaymentClaimBuffer, + }), + .. + } if payments.claim_funds_called(&payment_hash) => { + // LDK rejected the receive-side claim because the claim + // buffer was hit. In that case no PaymentClaimed event is + // expected, so clear the outstanding claim obligation. + payments.clear_claim(&payment_hash); + }, + events::Event::HTLCHandlingFailed { + failure_type: events::HTLCHandlingFailureType::Receive { payment_hash }, + .. + } => { + assert!( + !payments.claim_funds_called(&payment_hash), + "Payment {:?} hit HTLCHandlingFailed::Receive after claim_funds", + payment_hash, + ); + }, events::Event::HTLCHandlingFailed { .. } => {}, events::Event::FundingTransactionReadyForSigning { channel_id, @@ -3140,13 +4041,76 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { .funding_transaction_signed(&channel_id, &counterparty_node_id, signed_tx) .unwrap(); }, - events::Event::SpliceNegotiated { .. } => {}, + events::Event::SpliceNegotiated { .. } => { + // A negotiated splice is not yet a mined splice, and the + // harness also keeps transaction propagation explicit. LDK + // broadcasts the new funding transaction through the test + // broadcaster; relay commands or finish cleanup decide when + // that broadcast reaches the modeled mempool, and mining + // commands decide when the mempool gets confirmed. + }, events::Event::SpliceNegotiationFailed { .. } => {}, - events::Event::DiscardFunding { - funding_info: - events::FundingInfo::Contribution { .. } | events::FundingInfo::Tx { .. }, - .. - } => {}, + events::Event::ChannelClosed { channel_id, reason, .. } => { + // Most closes must have been expected before LDK emits + // ChannelClosed. HTLC timeout closes are the exception: the + // timeout condition is observed while connecting the block, + // so the close event itself carries the evidence. + let expected_before_close = matches!( + payments.channel_closes.get(&channel_id), + Some(ChannelCloseState::Expected) + ); + let allowed_by_htlc_timeout = match &reason { + events::ClosureReason::HTLCsTimedOut { .. } => true, + events::ClosureReason::CounterpartyForceClosed { peer_msg } => { + peer_msg.0.starts_with(HTLC_TIMEOUT_ERROR_PREFIX) + }, + _ => false, + }; + if allowed_by_htlc_timeout { + payments.expect_channel_close(channel_id); + } + assert!( + expected_before_close || allowed_by_htlc_timeout, + "Channel {:?} closed without an explicit force-close or HTLC timeout: {:?}", + channel_id, + reason, + ); + // Dust-blocked HTLCs can never produce an on-chain claim, + // regardless of whether the close was explicit or caused by + // an HTLC timeout. Mark those paths blocked at close time so + // later pending-payment assertions do not wait for an event + // LDK cannot produce. + payments.block_dust_paths_containing_channel( + channel_id, + MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS as u64 * 1000, + ); + if let events::ClosureReason::HTLCsTimedOut { + payment_hash: Some(payment_hash), + } = reason + { + // Timeout closes identify the affected payment hash, so + // the tracker can block only the paths for that payment + // that actually used the closed channel. + payments.block_paths_containing_channel(&payment_hash, channel_id); + } + }, + events::Event::DiscardFunding { .. } => { + // Aborted funding or splice negotiation does not touch the + // chain model unless a transaction was actually relayed and + // mined. + }, + events::Event::SpendableOutputs { .. } => { + // This target tracks wallet UTXOs from confirmed + // wallet-owned transaction outputs. It does not sweep LDK + // spendable outputs. + }, + events::Event::BumpTransaction(bump) => { + // Let LDK's wallet-backed bump handler complete and + // rebroadcast anchor or claim transactions. Relay commands + // or finish cleanup decide when those broadcasts enter the + // modeled mempool. + nodes[node_idx].bump_tx_handler.handle_event(&bump); + }, _ => panic!("Unhandled event: {:?}", event), } } @@ -3167,103 +4131,83 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { self.process_events(node_idx, fail); } + // Drives event and message processing until the harness is quiet. We need + // this before settlement phases that assume no immediately deliverable work + // is still queued. fn process_all_events(&mut self) { - let mut last_pass_no_updates = false; - 'settle: for i in 0..std::usize::MAX { - if i == 100 { - panic!( - "It may take may iterations to settle the state, but it should not take forever" - ); - } - let mut made_progress = self.checkpoint_manager_persistences(); - // Next, make sure no monitor completion callbacks are pending. - made_progress |= self.ab_link.complete_all_monitor_updates(&self.nodes); - made_progress |= self.bc_link.complete_all_monitor_updates(&self.nodes); - // Then, make sure any current forwards make their way to their destination. - for node_idx in 0..3 { - if self.process_msg_events(node_idx, false, ProcessMessages::AllMessages) { - last_pass_no_updates = false; - continue 'settle; - } - } - // ...making sure any payments are claimed. - for node_idx in 0..3 { - if self.process_events(node_idx, false) { - last_pass_no_updates = false; - continue 'settle; - } - } - if made_progress { - last_pass_no_updates = false; - continue; - } - if last_pass_no_updates { - // In some cases, we may generate a message to send in - // `process_msg_events`, but block sending until - // `complete_all_monitor_updates` gets called on the next - // iteration. - // - // Thus, we only exit if we manage two iterations with no messages - // or events to process. - break; - } - last_pass_no_updates = true; - } - } - - fn disconnect_ab(&mut self) { - self.ab_link.disconnect(&self.nodes, &mut self.queues); - } - - fn disconnect_bc(&mut self) { - self.bc_link.disconnect(&self.nodes, &mut self.queues); - } - - fn reconnect_ab(&mut self) { - self.ab_link.reconnect(&self.nodes); - } - - fn reconnect_bc(&mut self) { - self.bc_link.reconnect(&self.nodes); + let settled = self.progress_until_quiet(100); + assert!(settled, "process_all_events exceeded settle budget"); } + // Restarts one node from a fuzz-selected persisted state. We need this to + // keep monitor replay coverage while updating payment records that the + // reloaded manager can no longer remember. fn restart_node(&mut self, node_idx: usize, v: u8, router: &'a FuzzRouter) { if !self.nodes[node_idx].deferred { self.nodes[node_idx].checkpoint_manager_persistence(); } match node_idx { 0 => { - self.ab_link.disconnect_for_reload(0, &self.nodes, &mut self.queues); + self.ab_link.disconnect_for_reload( + 0, + &self.nodes, + &mut self.queues, + &self.payments.channel_closes, + ); }, 1 => { - self.ab_link.disconnect_for_reload(1, &self.nodes, &mut self.queues); - self.bc_link.disconnect_for_reload(1, &self.nodes, &mut self.queues); + self.ab_link.disconnect_for_reload( + 1, + &self.nodes, + &mut self.queues, + &self.payments.channel_closes, + ); + self.bc_link.disconnect_for_reload( + 1, + &self.nodes, + &mut self.queues, + &self.payments.channel_closes, + ); }, 2 => { - self.bc_link.disconnect_for_reload(2, &self.nodes, &mut self.queues); + self.bc_link.disconnect_for_reload( + 2, + &self.nodes, + &mut self.queues, + &self.payments.channel_closes, + ); }, _ => panic!("invalid node index"), } let loaded_manager_generation = self.nodes[node_idx].reload(v, &self.out, router, self.chan_type); - let rolled_back_payment_hashes = self.payments.nodes[node_idx] - .sync_pending_with_manager_generation(loaded_manager_generation); + self.catch_up_raw_monitors_for_node(node_idx); + let rolled_back_payment_hashes = + self.payments.sync_pending_with_manager_generation(node_idx, loaded_manager_generation); for payment_hash in rolled_back_payment_hashes { - self.payments.claimed_payment_hashes.remove(&payment_hash); + // If a reload rolled back a manager past claim_funds, the harness + // must not require the reloaded manager to later report + // PaymentClaimed for that claim. + self.payments.clear_claim(&payment_hash); } } + // Drives the whole harness toward settlement and checks final invariants. We + // need this for the `0xff` command, where the test proves force-close and + // payment cleanup either complete or fail at the invariant that explains + // what remained stuck. fn settle_all(&mut self) { - let chain_state = &self.chain_state; - for node in &mut self.nodes { - node.sync_with_chain_state(chain_state, None); - } - - // First, make sure peers are all connected to each other + // First, make sure peers are all connected to each other. The cleanup + // invariants are about eventual resolution, not about preserving a + // fuzzer-selected temporary disconnection into the final settle step. self.reconnect_ab(); self.reconnect_bc(); for op in SUPPORTED_SIGNER_OPS { + // The fuzzer may have blocked any signer operation. Re-enable all of + // them before final cleanup so failures after this point indicate + // missing event, chain, or accounting progress instead of an + // intentionally blocked signer. self.nodes[0].keys_manager.enable_op_for_all_signers(op); self.nodes[1].keys_manager.enable_op_for_all_signers(op); self.nodes[2].keys_manager.enable_op_for_all_signers(op); @@ -3287,21 +4231,84 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { } self.process_all_events(); - // Verify no payments are stuck - all should have resolved - self.payments.assert_all_resolved(); - // Verify that every payment claimed by a receiver resulted in a - // PaymentSent event at the sender. - self.payments.assert_claims_reported(); + if !self.payments.channel_closes.is_empty() { + for _ in 0..FORCE_CLOSE_CLEANUP_ROUNDS { + // Force-close cleanup alternates between event/message progress + // and height progress. This mirrors real nodes, where blocks + // trigger monitor claims, those claims broadcast transactions, + // and later blocks confirm the resulting transactions. + self.flush_progress(QUIESCENCE_ROUNDS); + for node in self.nodes.iter() { + node.timer_tick_occurred(); + } + self.flush_progress(QUIESCENCE_ROUNDS); + let balances = self.claimable_balances(); + let has_pending_htlcs = self.has_pending_htlcs(); + // Payment work can be waiting in the tracker or still committed + // in live channel HTLC state. Chain work can be waiting in + // claimable balances, broadcasts, the mempool, monitor updates, + // or those same HTLCs. Keep the two signals separate so the loop + // mines only when there is both something to finish and some + // modeled mechanism that could still advance it. + let needs_payment_completion = + self.payments.has_live_payment_work() || has_pending_htlcs; + let has_cleanup_balances = !balances.is_empty(); + let can_drive_more_cleanup = + has_cleanup_balances || self.has_pending_work() || has_pending_htlcs; + let next_claimed_htlc_boundary = self.next_claimed_htlc_boundary(&balances); + // Mining one more block is safe only while the next block remains + // below the nearest claim boundary that still needs sender-side + // accounting. + let can_advance_without_claimed_expiry = next_claimed_htlc_boundary + .map_or(true, |boundary| { + self.chain_state.tip_height().saturating_add(1) < boundary + }); + if !needs_payment_completion || !can_drive_more_cleanup { + // Either all payment work is accounted for, or there is no + // remaining event, transaction, balance, or HTLC state that + // could make another cleanup round useful. + break; + } + if self.payments.has_unfinished_claims() && !can_advance_without_claimed_expiry { + // Stop before mining across a claimed HTLC expiry that + // still needs sender-side resolution. Crossing that boundary + // would turn a useful payment-accounting invariant into a + // race against this final cleanup driver. + break; + } + self.mine_blocks(1); + self.flush_progress(QUIESCENCE_ROUNDS); + } + } + + self.payments.block_unresolvable_closed_paths(); + self.payments.assert_no_pending("after settling all state"); + self.payments.assert_claims_resolved(); // All HTLCs should have been claimed or failed once we reach quiescence. for (idx, node) in self.nodes.iter().enumerate() { for chan in node.list_channels() { + if self.payments.channel_closes.contains_key(&chan.channel_id) { + continue; + } + let inbound_hashes = chan + .pending_inbound_htlcs + .iter() + .map(|htlc| htlc.payment_hash) + .collect::>(); + let outbound_hashes = chan + .pending_outbound_htlcs + .iter() + .map(|htlc| htlc.payment_hash) + .collect::>(); assert!( chan.pending_inbound_htlcs.is_empty() && chan.pending_outbound_htlcs.is_empty(), "Node {} channel {:?} has stuck HTLCs after settling all state: \ - {} inbound {:?}, {} outbound {:?}", + inbound_hashes={:?}, outbound_hashes={:?}, {} inbound {:?}, {} outbound {:?}", idx, chan.channel_id, + inbound_hashes, + outbound_hashes, chan.pending_inbound_htlcs.len(), chan.pending_inbound_htlcs, chan.pending_outbound_htlcs.len(), @@ -3310,22 +4317,44 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { } } - // Finally, make sure that at least one end of each channel can make a substantial payment. - let chan_ab_ids = self.ab_link.channel_ids().clone(); - let chan_bc_ids = self.bc_link.channel_ids().clone(); - for chan_id in chan_ab_ids { - assert!( - self.send_on_channel(0, 1, chan_id, 10_000_000) - || self.send_on_channel(1, 0, chan_id, 10_000_000) - ); + self.ab_link.complete_all_monitor_updates(&self.nodes); + self.bc_link.complete_all_monitor_updates(&self.nodes); + self.record_disappeared_channels(); + // Check both tracker-level and topology-level close invariants. The + // tracker verifies every observed close became expected; the peer links + // verify every non-closed sibling channel is still listed by both peers. + self.payments.assert_no_unexpected_channel_closes(); + self.assert_no_unexpected_channel_closes(); + + for chan_id in *self.ab_link.channel_ids() { + if self.payments.channel_closes.contains_key(&chan_id) { + continue; + } + // After all cleanup, each still-open channel should be able to send + // in at least one direction, unless neither side advertises any + // usable amount. Probe with the channel's current min/max instead of + // a fixed amount because force-close and fee updates can change the + // sendable range. + if self.probe_amount_for_direction(0, chan_id).is_some() { + assert!(self.can_send_after_settle(0, 1, chan_id)); + } else if self.probe_amount_for_direction(1, chan_id).is_some() { + assert!(self.can_send_after_settle(1, 0, chan_id)); + } } - for chan_id in chan_bc_ids { - assert!( - self.send_on_channel(1, 2, chan_id, 10_000_000) - || self.send_on_channel(2, 1, chan_id, 10_000_000) - ); + for chan_id in *self.bc_link.channel_ids() { + if self.payments.channel_closes.contains_key(&chan_id) { + continue; + } + if self.probe_amount_for_direction(1, chan_id).is_some() { + assert!(self.can_send_after_settle(1, 2, chan_id)); + } else if self.probe_amount_for_direction(2, chan_id).is_some() { + assert!(self.can_send_after_settle(2, 1, chan_id)); + } } + self.process_all_events(); + self.payments.assert_no_pending("after settle probes"); + self.nodes[0].record_last_htlc_clear_fee(); self.nodes[1].record_last_htlc_clear_fee(); self.nodes[2].record_last_htlc_clear_fee(); @@ -3355,59 +4384,24 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { self.chain_state.relay_transactions(txs); } - fn earliest_pending_htlc_expiry(&self) -> Option { - let mut earliest_expiry: Option = None; + // Relays every node's pending broadcasts into the modeled mempool. Cleanup + // uses this when it should not depend on which peer fuzz bytes propagate. + // The bool reports whether any broadcasts were drained, not whether they + // were all admitted. + fn relay_all_broadcasts(&mut self) -> bool { + let mut txs = Vec::new(); for node in &self.nodes { - for chan in node.list_channels() { - for htlc in &chan.pending_inbound_htlcs { - earliest_expiry = Some( - earliest_expiry - .map_or(htlc.cltv_expiry, |expiry| expiry.min(htlc.cltv_expiry)), - ); - } - for htlc in &chan.pending_outbound_htlcs { - earliest_expiry = Some( - earliest_expiry - .map_or(htlc.cltv_expiry, |expiry| expiry.min(htlc.cltv_expiry)), - ); - } - } - } - earliest_expiry - } - - fn safe_mine_block_count(&self, count: u32) -> u32 { - if let Some(expiry) = self.earliest_pending_htlc_expiry() { - let current_tip = self.chain_state.tip_height(); - // LDK may close to protect a pending HTLC before its raw CLTV - // expiry. Keep modeled mining outside that fail-back window so - // fuzzed block production does not force an on-chain timeout path. - let timeout_deadline = expiry.saturating_sub(channelmonitor::HTLC_FAIL_BACK_BUFFER); - assert!( - current_tip < timeout_deadline, - "pending HTLC with expiry {} and timeout deadline {} is already unsafe at tip {}", - expiry, - timeout_deadline, - current_tip - ); - // Stop before the deadline block itself, since connecting it is - // enough for ChannelMonitor timeout handling to run. - count.min(timeout_deadline - current_tip - 1) - } else { - count + txs.extend(node.broadcaster.txn_broadcasted.borrow_mut().drain(..)); } + self.chain_state.relay_transactions(txs) } // Mines blocks through ChainState, then syncs wallets and nodes. We need the // harness wrapper because block production is chain-only, while confirmed // transactions still update wallet UTXOs and LDK chain listeners. - fn mine_blocks(&mut self, count: u32) -> u32 { + fn mine_blocks(&mut self, count: u32) -> bool { assert!(count > 0, "mining zero blocks should not be requested"); - let count = self.safe_mine_block_count(count); - if count == 0 { - return 0; - } let confirmed_txs = self.chain_state.mine_blocks(count); let wallets = [ self.nodes[0].wallet.as_ref(), @@ -3420,24 +4414,416 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { for input in &tx.input { // The test wallet is a simple UTXO source. When one of its // outputs is spent by a confirmed transaction, remove it so - // later funding attempts cannot double-spend it. + // later funding or bump attempts cannot double-spend it. wallet.remove_utxo(input.previous_output); } for (vout, output) in tx.output.iter().enumerate() { if output.script_pubkey == change_script { // Add wallet-owned outputs back to whichever test wallet - // owns the script. This lets splice flows recycle wallet - // change through later fuzz commands. + // owns the script. This is what lets anchor and splice + // flows recycle wallet change through later fuzz + // commands. wallet.add_utxo(tx.clone(), vout as u32); } } } } - let chain_state = &self.chain_state; - for node in &mut self.nodes { - node.sync_with_chain_state(chain_state, None); + self.sync_all_nodes_with_chain_state(); + true + } + + // Repeatedly relays broadcasts and mines pending transactions to depth. We + // need this for finish and settle paths where confirmed claims may broadcast + // child transactions that also need confirmation. + fn mine_relayed_txs_until_quiet(&mut self, context: &str) { + for _ in 0..QUIESCENCE_ROUNDS { + // Finish and settle paths should not leave already-broadcast + // transactions stranded. Relay all broadcasts, mine them to normal + // depth, and repeat because confirmed claims can broadcast children. + self.relay_all_broadcasts(); + if !self.chain_state.has_pending_txs() { + return; + } + self.mine_blocks(DEFAULT_TX_CONFIRMATION_BLOCKS); + } + assert!( + !self.nodes.iter().any(|node| !node.broadcaster.txn_broadcasted.borrow().is_empty()) + && !self.chain_state.has_pending_txs(), + "{context} tx mining loop failed to quiesce", + ); + } + + // Collects claimable balances from every monitor. We need this to know when + // on-chain funds still require more blocks, transactions, or monitor work. + fn claimable_balances(&self) -> Vec { + // ChannelMonitor::get_claimable_balances needs references to currently + // open channels so it can classify balances relative to the live channel + // set. Closed channels are represented by monitors, while open channel + // refs prevent open-channel balances from being misclassified. + let open_channels = self.nodes[0] + .node + .list_channels() + .iter() + .chain(self.nodes[1].node.list_channels().iter()) + .chain(self.nodes[2].node.list_channels().iter()) + .cloned() + .collect::>(); + let open_refs: Vec<_> = open_channels.iter().collect(); + self.nodes.iter().flat_map(|node| node.monitor.get_claimable_balances(&open_refs)).collect() + } + + // Reports whether any monitor persistence completion is still pending. We + // need this because unresolved monitor updates can block claim generation or + // channel-manager progress even when no peer messages are queued. + fn has_pending_monitor_updates(&self) -> bool { + self.nodes.iter().any(|node| { + node.persister + .latest_monitors + .lock() + .unwrap() + .values() + .any(|state| !state.pending_monitor_completions.is_empty()) + }) + } + + // Finds the nearest HTLC claim or expiry boundary that still matters. We + // need this so settlement can advance blocks without skipping over a height + // where sender-side resolution may change. + fn next_claimed_htlc_boundary(&self, balances: &[Balance]) -> Option { + // When a claimed HTLC still needs sender-side resolution, advancing + // past its claim or expiry boundary can change the failure mode the + // harness observes. Return the nearest such height so settle_all can + // stop before crossing it. + balances + .iter() + .filter_map(|balance| { + let (height, payment_hash) = match balance { + Balance::ContentiousClaimable { timeout_height, payment_hash, .. } => { + (*timeout_height, payment_hash) + }, + Balance::MaybeTimeoutClaimableHTLC { + claimable_height, payment_hash, .. + } => (*claimable_height, payment_hash), + Balance::MaybePreimageClaimableHTLC { expiry_height, payment_hash, .. } => { + (*expiry_height, payment_hash) + }, + _ => return None, + }; + // Boundaries only matter for claims whose sender side has not + // resolved and whose paths are not all already blocked. + let claim_needs_sender_resolution = + self.payments + .record_for_hash(payment_hash) + .map(|record| { + record.claim_funds_called + && record.sender_outcome.is_none() + && !(record.paths.len() > 0 + && record.paths.iter().enumerate().all(|(path_idx, _)| { + record.blocked_paths.contains(&path_idx) + })) + }) + .unwrap_or(false); + claim_needs_sender_resolution.then_some(height) + }) + .min() + } + + // Checks whether any modeled subsystem still has work to do. We need this + // broader signal because on-chain cleanup may be pending in queues, + // broadcasts, mempool entries, monitors, or claimable balances. + fn has_pending_work(&self) -> bool { + // This is deliberately broader than "has queued messages". On-chain + // cleanup can be waiting on monitor updates, newly broadcast + // transactions, or claimable balances even when peer message queues are + // empty. + !self.queues.ab.is_empty() + || !self.queues.ba.is_empty() + || !self.queues.bc.is_empty() + || !self.queues.cb.is_empty() + || self.chain_state.has_pending_txs() + || self.nodes.iter().any(|node| !node.broadcaster.txn_broadcasted.borrow().is_empty()) + || self.has_pending_monitor_updates() + || !self.claimable_balances().is_empty() + } + + // Checks live channels for unresolved HTLCs. We need this final settlement + // signal to prove the force-close cleanup did not leave channel HTLC state + // stuck. + fn has_pending_htlcs(&self) -> bool { + self.nodes.iter().any(|node| { + node.list_channels().iter().any(|chan| { + !chan.pending_inbound_htlcs.is_empty() || !chan.pending_outbound_htlcs.is_empty() + }) + }) + } + + // Completes any deferred monitor updates across both peer links. We need + // this because fuzzed persistence delay should be released during cleanup so + // chain and message processing can continue. + fn complete_pending_monitor_updates(&self) -> bool { + let mut completed_monitor_update = false; + for id in self.ab_link.channel_ids() { + completed_monitor_update |= self.nodes[0].complete_all_monitor_updates(id); + completed_monitor_update |= self.nodes[1].complete_all_monitor_updates(id); + } + for id in self.bc_link.channel_ids() { + completed_monitor_update |= self.nodes[1].complete_all_monitor_updates(id); + completed_monitor_update |= self.nodes[2].complete_all_monitor_updates(id); + } + completed_monitor_update + } + + // Syncs every harness node to the current ChainState tip. We need this after + // mining so all managers and monitors observe the same confirmed chain. + fn sync_all_nodes_with_chain_state(&mut self) { + for idx in 0..self.nodes.len() { + self.sync_node_with_chain_state(idx, None); + } + } + + // Syncs one node toward the ChainState tip and pre-authorizes timeout closes. + // We need the pre-sync check because LDK may emit ChannelClosed while + // processing the connected blocks. + fn sync_node_with_chain_state(&mut self, node_idx: usize, num_blocks: Option) { + let target_height = if let Some(num_blocks) = num_blocks { + std::cmp::min( + self.nodes[node_idx].height.saturating_add(num_blocks), + self.chain_state.tip_height(), + ) + } else { + self.chain_state.tip_height() + }; + // Before delivering the height to LDK, decide whether reaching that + // height makes an HTLC-timeout close expected. This lets the later + // ChannelClosed event be checked against a concrete timeout predicate. + self.allow_htlc_timeout_closes_for_node(node_idx, target_height); + self.nodes[node_idx].sync_with_chain_state(&self.chain_state, num_blocks); + } + + // Marks channels whose HTLCs can force-close at the target height. We need + // this to accept timeout-driven closes without accepting arbitrary channel + // disappearance. + fn allow_htlc_timeout_closes_for_node(&mut self, node_idx: usize, target_height: u32) { + for chan in self.nodes[node_idx].list_channels() { + // LDK may force-close when either our outbound HTLC has aged past + // the timeout grace or our inbound HTLC is close enough to expiry + // that claiming with a known preimage is unsafe to delay. + let outbound_timed_out = chan.pending_outbound_htlcs.iter().any(|htlc| { + htlc.cltv_expiry.saturating_add(HTLC_TIMEOUT_GRACE_BLOCKS) <= target_height + }); + let inbound_timed_out = chan.pending_inbound_htlcs.iter().any(|htlc| { + htlc.cltv_expiry <= target_height.saturating_add(HTLC_CLAIM_BUFFER_BLOCKS) + && self.payments.payment_preimages.contains_key(&htlc.payment_hash) + }); + if outbound_timed_out || inbound_timed_out { + self.payments.allow_htlc_timeout_channel_close( + chan.channel_id, + outbound_timed_out, + inbound_timed_out, + ); + } + } + } + + // Drains monitor-generated events on every node. We need this after mining + // or completing persistence because monitors can enqueue claim handling work + // independently of peer messages. + fn process_monitor_pending_events(&self) { + for node in &self.nodes { + node.process_monitor_pending_events(); + } + } + + // Runs one granular cleanup pass and reports whether anything advanced. We + // need this to settle natural follow-up work without adding a large compound + // fuzz opcode. + fn progress_round(&mut self) -> bool { + let made_progress = self.checkpoint_manager_persistences(); + let completed_monitor_update = self.complete_pending_monitor_updates(); + let mut had_msg_or_ev = false; + // A progress round is deliberately granular: drain peer messages, drain + // user events, relay anything broadcast, and mine only if the mempool is + // non-empty. This keeps normal fuzz bytes meaningful while still letting + // settle_all drive on-chain cleanup to quiescence. + for node_idx in 0..3 { + if self.process_msg_events(node_idx, false, ProcessMessages::AllMessages) { + had_msg_or_ev = true; + } + } + for node_idx in 0..3 { + if self.process_events(node_idx, false) { + had_msg_or_ev = true; + } + } + let relayed_before_mining = self.relay_all_broadcasts(); + let mined_txs = self.chain_state.has_pending_txs(); + if mined_txs { + // If any transactions entered the mempool, mine them deeply enough + // for normal confirmation-sensitive logic to run in the same + // progress round. + self.mine_blocks(DEFAULT_TX_CONFIRMATION_BLOCKS); + } + self.process_monitor_pending_events(); + let relayed_after_mining = self.relay_all_broadcasts(); + made_progress + || completed_monitor_update + || relayed_before_mining + || relayed_after_mining + || mined_txs || had_msg_or_ev + } + + // Repeats progress rounds until two consecutive quiet passes or exhaustion. + // We need the second quiet pass because one drain can expose work that is + // only visible on the next pass. + fn progress_until_quiet(&mut self, max_iters: usize) -> bool { + let mut last_pass_no_updates = false; + for _ in 0..max_iters { + if self.progress_round() { + last_pass_no_updates = false; + continue; + } + if last_pass_no_updates { + return true; + } + last_pass_no_updates = true; } - count + false + } + + // Drains immediately processable work and asserts if that work keeps making + // progress past the iteration budget. Height-gated work may remain for the + // caller to handle by mining another block. + fn flush_progress(&mut self, max_iters: usize) { + let settled = self.progress_until_quiet(max_iters); + let pending_work = self.has_pending_work(); + assert!( + !pending_work || settled, + "flush_progress exhausted {max_iters} iterations without quiescing", + ); + } + + // Replays missed chain entries to raw monitors after reload. We need this + // because the manager can already be caught up while a reloaded monitor + // starts from an older best block. + fn catch_up_raw_monitors_for_node(&self, node_idx: usize) { + let node = &self.nodes[node_idx]; + let mut min_monitor_height = node.height; + for chan_id in node.monitor.list_monitors() { + if let Ok(mon) = node.monitor.get_monitor(chan_id) { + // Reload may install monitors older than the manager's current + // height. Connect from the oldest monitor height so every raw + // monitor sees the transactions it missed while the manager was + // already caught up. + min_monitor_height = + std::cmp::min(min_monitor_height, mon.current_best_block().height); + } + } + // The manager is already caught up to node.height. Only the raw + // monitors need these older callbacks. + let connect_manager = false; + node.connect_chain_range_inner( + &self.chain_state, + min_monitor_height, + node.height, + connect_manager, + ); + } + + // Disconnects the A-B peer link through the shared PeerLink path. We need a + // named wrapper so fuzz opcodes can target this edge directly. + fn disconnect_ab(&mut self) { + self.ab_link.disconnect(&self.nodes, &mut self.queues, &self.payments.channel_closes); + } + + // Disconnects the B-C peer link through the shared PeerLink path. We need a + // named wrapper so fuzz opcodes can target this edge directly. + fn disconnect_bc(&mut self) { + self.bc_link.disconnect(&self.nodes, &mut self.queues, &self.payments.channel_closes); + } + + // Reconnects the A-B peer link. We need this wrapper to keep topology + // commands symmetric with the disconnect opcodes. + fn reconnect_ab(&mut self) { + self.ab_link.reconnect(&self.nodes); + } + + // Reconnects the B-C peer link. We need this wrapper to keep topology + // commands symmetric with the disconnect opcodes. + fn reconnect_bc(&mut self) { + self.bc_link.reconnect(&self.nodes); + } + + // Executes an explicit force-close command for one channel. We need this to + // authorize exactly that close before calling into LDK so later invariants + // can reject unrelated channel loss. + fn force_close( + &mut self, closer_idx: usize, channel_id: ChannelId, counterparty_idx: usize, reason: &str, + ) { + // Explicit fuzz opcodes should authorize exactly the channel they + // close. The set-size check catches accidental future edits that mark a + // whole link or all channels as closeable. + let had_close_entry = self.payments.channel_closes.contains_key(&channel_id); + let close_count = self.payments.channel_closes.len(); + self.payments.expect_channel_close(channel_id); + assert_eq!( + self.payments.channel_closes.len(), + close_count + usize::from(!had_close_entry), + "explicit force-close expected more than one new channel to close", + ); + assert!( + matches!( + self.payments.channel_closes.get(&channel_id), + Some(ChannelCloseState::Expected) + ), + "explicit force-close did not expect its own channel to close", + ); + let _ = self.nodes[closer_idx].node.force_close_broadcasting_latest_txn( + &channel_id, + &self.nodes[counterparty_idx].get_our_node_id(), + reason.to_string(), + ); + } + + // Chooses an amount that should be sendable in one direction after cleanup. + // We need this because post-force-close fees and reserves can change a + // channel's valid send range. + fn probe_amount_for_direction( + &self, source_idx: usize, dest_chan_id: ChannelId, + ) -> Option { + self.nodes[source_idx] + .node + .list_usable_channels() + .iter() + .find(|chan| chan.channel_id == dest_chan_id) + .and_then(|chan| { + // After cleanup, the old fixed probe may sit outside the channel's + // current sendable range. Keep its target when possible. + let probe_amt = cmp::max( + cmp::min(10_000_000, chan.next_outbound_htlc_limit_msat), + chan.next_outbound_htlc_minimum_msat, + ); + if probe_amt == 0 || probe_amt > chan.next_outbound_htlc_limit_msat { + None + } else { + Some(probe_amt) + } + }) + } + + // Attempts a final probe payment over a surviving channel. We need this to + // prove open sibling channels remain usable after force-close settlement. + fn can_send_after_settle( + &mut self, source_idx: usize, dest_idx: usize, dest_chan_id: ChannelId, + ) -> bool { + if self.payments.channel_closes.contains_key(&dest_chan_id) { + // Closed channels are validated by close accounting, not by the + // post-settle sendability probe. + return false; + } + let Some(amt) = self.probe_amount_for_direction(source_idx, dest_chan_id) else { + return false; + }; + self.payments.send(&self.nodes, source_idx, dest_idx, dest_chan_id, amt) } } @@ -3673,13 +5059,13 @@ pub fn do_test(data: &[u8], out: Out) { }, // Sync node by 1 block. - 0xa8 => harness.nodes[0].sync_with_chain_state(&harness.chain_state, Some(1)), - 0xa9 => harness.nodes[1].sync_with_chain_state(&harness.chain_state, Some(1)), - 0xaa => harness.nodes[2].sync_with_chain_state(&harness.chain_state, Some(1)), + 0xa8 => harness.sync_node_with_chain_state(0, Some(1)), + 0xa9 => harness.sync_node_with_chain_state(1, Some(1)), + 0xaa => harness.sync_node_with_chain_state(2, Some(1)), // Sync node to chain tip. - 0xab => harness.nodes[0].sync_with_chain_state(&harness.chain_state, None), - 0xac => harness.nodes[1].sync_with_chain_state(&harness.chain_state, None), - 0xad => harness.nodes[2].sync_with_chain_state(&harness.chain_state, None), + 0xab => harness.sync_node_with_chain_state(0, None), + 0xac => harness.sync_node_with_chain_state(1, None), + 0xad => harness.sync_node_with_chain_state(2, None), 0xb0 | 0xb1 | 0xb2 => { // Restart node A, picking among persisted and in-flight `ChannelMonitor` @@ -3817,10 +5203,16 @@ pub fn do_test(data: &[u8], out: Out) { let count = MINE_BLOCK_COUNTS[(v - 0xd8) as usize]; harness.mine_blocks(count); }, - // Keep holder signer unblocks near the signer op bytes while leaving - // the relay/mining range above contiguous. The helper re-enables both - // holder-side operations for every signer owned by the selected node, - // matching the existing key-manager-wide blocking model. + // Explicit force closes cover both directions on both peer links. + // Each command records exactly one expected channel close before + // calling into LDK. + 0xe0 => harness.force_close(0, harness.chan_a_id(), 1, FORCE_CLOSE_ERROR_MESSAGES[0]), + 0xe1 => harness.force_close(1, harness.chan_b_id(), 2, FORCE_CLOSE_ERROR_MESSAGES[1]), + 0xe2 => harness.force_close(1, harness.chan_a_id(), 0, FORCE_CLOSE_ERROR_MESSAGES[2]), + 0xe3 => harness.force_close(2, harness.chan_b_id(), 1, FORCE_CLOSE_ERROR_MESSAGES[3]), + // Holder signer unblocks are separate from force-close commands so + // the fuzzer can explore transactions being delayed by signer policy + // after a commitment transaction is already on chain. 0xe4 => harness.nodes[0].enable_holder_signer_ops(), 0xe5 => harness.nodes[1].enable_holder_signer_ops(), 0xe6 => harness.nodes[2].enable_holder_signer_ops(),