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..77fedccf555 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 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; @@ -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; @@ -41,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; @@ -60,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; @@ -83,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}; @@ -102,6 +107,33 @@ 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, 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 +// 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. 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]; +// 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, } @@ -183,19 +215,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,79 +245,285 @@ 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)) - }) - } - - fn confirm_tx(&mut self, tx: Transaction) -> bool { - let txid = tx.compute_txid(); - if self.confirmed_txids.contains(&txid) { - return false; + // 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; + } } - if tx.input.iter().any(|input| self.is_outpoint_spent(&input.previous_output)) { - return false; + 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); + } + } + 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. 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; + + // 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; - } - if tx.input.iter().any(|input| { - self.is_outpoint_spent(&input.previous_output) - || spent_outpoints.contains(&input.previous_output) - }) { - 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); } - self.confirmed_txids.insert(txid); - for input in &tx.input { - spent_outpoints.push(input.previous_output); + } + 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)); + } } - confirmed.push(tx); + self.pending_txs = retained_txs; } - if confirmed.is_empty() { + // 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 }); + } + } + if Self::has_invalid_inputs(&tx, &available_utxos) { return; } + self.pending_txs.push((txid, tx)); + } + + // 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. 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 + } - let prev_hash = self.blocks.last().unwrap().0.block_hash(); - let header = create_dummy_header(prev_hash, 42); - self.blocks.push((header, confirmed)); + // 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() + } - 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 `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) { @@ -283,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> { @@ -291,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(); @@ -586,6 +837,12 @@ type TestChainMonitor = chainmonitor::ChainMonitor< Arc, Arc, >; +type TestBumpTransactionEventHandler = BumpTransactionEventHandlerSync< + Arc, + Arc, Arc>>, + Arc, + Arc, +>; struct KeyProvider { node_secret: SecretKey, @@ -718,14 +975,38 @@ 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 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, SignerOp::ReleaseCommitmentSecret, SignerOp::SignSpliceSharedInput, + 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( @@ -770,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)] @@ -816,7 +1127,8 @@ struct HarnessNode<'a> { logger: Arc, broadcaster: Arc, fee_estimator: Arc, - wallet: TestWalletSource, + wallet: Arc, + bump_tx_handler: TestBumpTransactionEventHandler, persistence_style: ChannelMonitorUpdateStatus, deferred: bool, serialized_manager: Vec, @@ -852,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), @@ -865,7 +1181,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 { @@ -906,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, @@ -916,6 +1243,7 @@ impl<'a> HarnessNode<'a> { broadcaster, fee_estimator, wallet, + bump_tx_handler, persistence_style, deferred, serialized_manager: Vec::new(), @@ -957,6 +1285,88 @@ 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 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() { + 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); + if connect_manager { + 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); + if connect_manager { + 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); + if connect_manager { + self.node.transactions_confirmed(header, &txdata, height); + } + } + self.monitor.best_block_updated(header, height); + if connect_manager { + self.node.best_block_updated(header, 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 +1374,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 { @@ -1024,6 +1426,20 @@ 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) { + // 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); + } + fn current_feerate_sat_per_kw(&self) -> FeeRate { self.fee_estimator.feerate_sat_per_kw() } @@ -1033,7 +1449,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 = @@ -1105,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 { @@ -1173,6 +1605,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; @@ -1212,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, @@ -1226,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[..]); @@ -1266,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 { @@ -1297,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" @@ -1319,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(); @@ -1332,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() { @@ -1346,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() { @@ -1365,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"), } @@ -1420,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; } @@ -1429,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 { @@ -1436,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); } @@ -1463,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; @@ -1475,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(); @@ -1487,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(), + } + } + + // 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", + ); + } + } } - fn add_pending( - &mut self, payment_id: PaymentId, payment_hash: PaymentHash, - first_persisted_manager_generation: u64, + // 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); + } + } + + // 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, + ); } } - // Returns a bool indicating whether the payment failed. + // 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 { @@ -1588,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; }, _ => {}, } @@ -1599,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()); @@ -1613,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); @@ -1633,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(), @@ -1644,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 { @@ -1665,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]; @@ -1694,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 { @@ -1714,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 { @@ -1723,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, }, ], @@ -1731,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 { @@ -1746,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(); - if num_paths == 0 { - return; - } - - let amt_per_path = amt / num_paths as u64; - let mut paths = Vec::with_capacity(num_paths); - + let mut paths = Vec::new(); 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() { + 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; + 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 { @@ -1819,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 { @@ -1885,7 +2889,6 @@ impl PaymentTracker { } else { fee_per_path }; - paths.push(Path { hops: vec![ RouteHop { @@ -1894,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 { @@ -1903,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 { @@ -1923,49 +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.claimed_payment_hashes.insert(payment_hash); + 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; } } } @@ -1983,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 { @@ -2003,15 +3080,31 @@ 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. Broadcast transactions are handled explicitly. - 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()); + // All broadcasters should be empty because broadcast transactions only enter + // the modeled mempool through explicit relay commands or finish cleanup. + for node in nodes { + assert!(node.broadcaster.txn_broadcasted.borrow().is_empty()); + } } fn connect_peers(source: &ChanMan<'_>, dest: &ChanMan<'_>) { @@ -2108,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, @@ -2124,7 +3220,10 @@ fn make_channel( tx.clone(), ) .unwrap(); - chain_state.confirm_tx(tx); + // 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"); } @@ -2241,24 +3340,33 @@ 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. 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, + 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, splice transactions, and on-chain claims. + 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 +3381,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 +3392,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 +3403,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,7 +3413,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]); @@ -2327,16 +3434,20 @@ 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. - nodes[0].broadcaster.txn_broadcasted.borrow_mut().clear(); - nodes[1].broadcaster.txn_broadcasted.borrow_mut().clear(); - nodes[2].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); + // 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. + 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(); + } + 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); + } lock_fundings(&nodes); @@ -2375,10 +3486,44 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { self.bc_link.first_channel_id() } - fn finish(&self) { - assert_test_invariants(&self.nodes); - } - + // 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) { + // 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 @@ -2389,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); @@ -2422,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, ) { @@ -2450,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, @@ -2606,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 } => { @@ -2629,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); @@ -2701,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 }, @@ -2731,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; @@ -2749,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 { @@ -2757,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); @@ -2772,9 +3954,11 @@ 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 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. @@ -2789,23 +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::ProbeFailed { payment_id, .. } => { + payments.mark_resolved_without_hash(node_idx, payment_id); }, - events::Event::PaymentFailed { payment_id, .. } - | events::Event::ProbeFailed { payment_id, .. } => { - payments.nodes[node_idx].mark_resolved_without_hash(payment_id); + events::Event::PaymentClaimed { payment_hash, .. } => { + // PaymentClaimed is the receiver-side confirmation that a + // previous claim_funds call completed. + payments.mark_receiver_claimed(payment_hash); }, - events::Event::PaymentClaimed { .. } => {}, 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, @@ -2818,19 +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 { 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 { .. } => { + // 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), } } @@ -2851,117 +4131,95 @@ 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; - 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. - 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; - } - // ...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; - } - 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) { - // 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); } + // 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(); @@ -2973,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(), @@ -2996,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(); @@ -3024,6 +4367,464 @@ 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); + } + + // 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 { + 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) -> bool { + assert!(count > 0, "mining zero blocks should not be requested"); + + 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 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 is what lets anchor and splice + // flows recycle wallet change through later fuzz + // commands. + wallet.add_utxo(tx.clone(), vout as u32); + } + } + } + } + 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; + } + 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) + } } #[inline] @@ -3200,65 +5001,71 @@ 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()); }, - // 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.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.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` @@ -3383,6 +5190,32 @@ 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); + }, + // 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(), 0xf0 => harness.ab_link.complete_monitor_updates_for_node( 0, diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a2412bbaf5e..f1a7a253336 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}; @@ -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,13 @@ 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. + 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); }, _ => {}, @@ -517,7 +517,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 +534,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 +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, - /// 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. 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 @@ -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,14 +1341,15 @@ 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, - /// 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` @@ -2763,11 +2766,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 +2891,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 +3349,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); @@ -3811,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, @@ -3860,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. @@ -3879,7 +3884,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 +3892,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 +3931,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, ); @@ -4513,7 +4518,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 +4554,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); @@ -4806,11 +4811,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 +4855,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 +4884,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), @@ -4963,12 +4968,12 @@ 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, - ) -> Vec { + confirmation_height: Option, logger: &L, + ) -> Vec { let per_commitment_claimable_data = match per_commitment_option { Some(outputs) => outputs, None => return Vec::new(), @@ -4984,6 +4989,16 @@ impl ChannelMonitorImpl { .filter_map(|(htlc, _)| { if let Some(transaction_output_index) = htlc.transaction_output_index { 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, @@ -4993,7 +5008,7 @@ impl ChannelMonitorImpl { confirmation_height, ), ); - Some(PackageTemplate::build_package( + Some(ClaimRequest::new( commitment_txid, transaction_output_index, htlc_data, @@ -5009,13 +5024,67 @@ impl ChannelMonitorImpl { .collect() } - /// Returns the HTLC claim package templates and the counterparty output info + 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().find(|resolved_htlc| { + resolved_htlc.commitment_tx_output_idx == Some(transaction_output_index) + }) + }) + } + + 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, + ); + } + } + } + + /// 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; @@ -5056,6 +5125,9 @@ impl ChannelMonitorImpl { // per_commitment_data is corrupt or our commitment signing key leaked! return (claimable_outpoints, to_counterparty_output_info); } + if self.htlc_output_resolution_on_chain(htlc).is_some() { + continue; + } let preimage = if htlc.offered { if let Some((p, _)) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) @@ -5086,7 +5158,7 @@ impl ChannelMonitorImpl { ), ) }; - let counterparty_package = PackageTemplate::build_package( + let counterparty_package = ClaimRequest::new( commitment_txid, transaction_output_index, counterparty_htlc_outp, @@ -5104,7 +5176,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 +5207,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, ); @@ -5157,6 +5229,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.htlc_output_resolution_on_chain(htlc).is_some() { + continue; + } assert!(htlc.transaction_output_index.is_some(), "Expected transaction output index for non-dust HTLC"); let preimage = if htlc.offered { @@ -5187,13 +5262,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 +5288,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 +5324,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 @@ -5724,9 +5800,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); } } @@ -5759,7 +5835,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, @@ -6204,6 +6280,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; @@ -6290,18 +6367,25 @@ 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 { 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). - on_to_local_output_csv: if accepted_preimage_claim && !outbound_htlc { - Some(self.on_holder_tx_csv) } else { None }, + // 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, }, }); continue 'outer_loop; @@ -6402,7 +6486,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); } } @@ -6454,6 +6538,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] 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]) + ); + } } diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 3eb6d64f3a2..e4d23d77e88 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::{ @@ -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,6 +582,32 @@ impl OnchainTxHandler { self.pending_claim_requests.len() != 0 } + 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 /// 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 @@ -791,7 +823,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 +833,36 @@ 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; + let outpoint = req.outpoint(); + 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 + }, } - } - if all_outpoints_claiming { - log_info!(logger, "Ignoring second claim for outpoint {}:{}, already registered its claiming request", - req.outpoints()[0].txid, req.outpoints()[0].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()); - 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)); - false - } else { - true - } + 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() { @@ -1282,15 +1317,18 @@ 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}; - 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, @@ -1305,12 +1343,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(), @@ -1347,9 +1382,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, @@ -1360,66 +1392,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()) { - requests.push(PackageTemplate::build_package( + 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(), @@ -1429,11 +1440,63 @@ mod tests { preimage: None, counterparty_sig: *counterparty_sig, }, - 0 + 0, )), 0, )); } + 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. + #[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, @@ -1474,4 +1537,193 @@ 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"), + } + } + + #[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)); + } } 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)> { diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index f52f093917b..111d1fbfd81 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 @@ -2388,6 +2455,199 @@ 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 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 mut 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 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, .. + }) = event + { + Some(htlc_descriptors.len()) + } else { + None + } + }) + .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] +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.