diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index d8291571884..16fe53cdf87 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -15,9 +15,9 @@ //! actions such as sending payments, handling events, or changing monitor update return values on //! a per-node basis. This should allow it to find any cases where the ordering of actions results //! in us getting out of sync with ourselves, and, assuming at least one of our receive- or -//! send-side handling is correct, other peers. We consider it a failure if any action results in -//! a channel being force-closed. The fuzzer also models transaction relay through a harness -//! mempool, making transaction confirmation and block delivery closer to normal node behavior. +//! send-side handling is correct, other peers. The fuzzer also models a small mempool and +//! exercises controlled force-closes, including user-initiated closes and timeout-driven closes, +//! through on-chain confirmation and cleanup. use bitcoin::amount::Amount; use bitcoin::constants::genesis_block; @@ -43,18 +43,20 @@ use lightning::chain; use lightning::chain::chaininterface::{ BroadcasterInterface, ConfirmationTarget, FeeEstimator, TransactionType, }; -use lightning::chain::channelmonitor::{ChannelMonitor, ANTI_REORG_DELAY}; +use lightning::chain::channelmonitor::{ + Balance, ChannelMonitor, ANTI_REORG_DELAY, HTLC_FAIL_BACK_BUFFER, +}; 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; @@ -85,6 +87,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}; @@ -105,16 +109,27 @@ use std::sync::{Arc, Mutex}; const MAX_FEE: u32 = 10_000; const MAX_SETTLE_ITERATIONS: usize = 256; -// Each wallet is seeded with enough confirmed UTXOs that repeated splice -// transactions don't run out of inputs mid-run. +// The fuzz wallet needs enough confirmed inputs for splice, anchor, and claim +// transactions so failures come from on-chain close handling rather than from +// the harness running out of wallet inputs. const NUM_WALLET_UTXOS: u32 = 50; // A single fuzz byte can mine more than one block so a corpus entry does not // need long runs of identical "mine one block" commands to reach CSV or CLTV -// boundaries. Mining commands are capped in `safe_mine_block_count` if -// unresolved HTLCs are near expiry. +// boundaries. The early entries remain small so the fuzzer can still explore +// fine-grained height changes. const MINE_BLOCK_COUNTS: [u32; 8] = [1, 2, 3, 6, 12, 24, 48, 144]; -// Finish-time relay/mining rounds are capped so cleanup cannot spin forever. -const MAX_FINISH_RELAY_MINE_ROUNDS: usize = 32; +// Progress loops are capped so `0xff` and cleanup paths can drive realistic +// asynchronous work without letting a malformed state spin forever. +const QUIESCENCE_ROUNDS: usize = 32; +// Force-close cleanup may require many height advances because claims can be +// chained through commitment, HTLC, anchor, and delayed-output transactions. +// This is intentionally large, but still finite, so a stuck cleanup becomes an +// assertion instead of an infinite fuzz run. +const FORCE_CLOSE_CLEANUP_ROUNDS: usize = 4096; +// Use LDK's minimum CLTV deltas to keep timeout-driven close scenarios reachable +// in short fuzz inputs while still respecting the library's configured limits. +const FUZZ_FINAL_CLTV_DELTA: u32 = MIN_FINAL_CLTV_EXPIRY_DELTA as u32; +const FUZZ_FORWARD_CLTV_DELTA: u32 = MIN_CLTV_EXPIRY_DELTA as u32; struct FuzzEstimator { ret_val: atomic::AtomicU32, @@ -227,6 +242,10 @@ impl ChainState { self.utxos.contains(outpoint) } + fn is_confirmed_txid(&self, txid: &Txid) -> bool { + self.confirmed_txids.contains(txid) + } + fn confirmed_output(&self, outpoint: &BitcoinOutPoint) -> Option<&TxOut> { if !self.confirmed_txids.contains(&outpoint.txid) { return None; @@ -316,6 +335,11 @@ impl ChainState { let locktime_enabled = tx.input.iter().any(|input| input.sequence.enables_absolute_lock_time()); + // Commitment transactions split the obscured commitment number across + // nSequence and nLockTime with fixed top bytes 0x80 and 0x20. The + // non-final sequence makes nLockTime relevant, and the 0x20 top byte + // puts the value 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; @@ -402,10 +426,22 @@ impl ChainState { self.pending_txs.push((txid, tx)); } - fn relay_transactions(&mut self, txs: Vec) { + // Feeds broadcast transactions through modeled mempool admission. Returns + // 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 { self.admit_tx_to_mempool(tx); } + found + } + + // Reports whether the modeled mempool is non-empty. Fuzz mining bytes and + // cleanup loops use this to decide whether another mining pass can make + // progress. + fn has_pending_txs(&self) -> bool { + !self.pending_txs.is_empty() } // Mines `count` blocks, confirming the current mempool in the first block. @@ -748,6 +784,12 @@ type TestChainMonitor = chainmonitor::ChainMonitor< Arc, Arc, >; +type TestBumpTransactionEventHandler = BumpTransactionEventHandlerSync< + Arc, + Arc, Arc>>, + Arc, + Arc, +>; struct KeyProvider { node_secret: SecretKey, @@ -880,14 +922,40 @@ 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] = [ +// Signer ops that fuzz bytes may block. The holder-side ops matter after a +// force-close, when monitors build holder commitment or HTLC claim +// transactions; `settle_all` re-enables everything so final failures are not +// caused by intentionally blocked signing. +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"; +// Bounds for accepting a non-explicit close as an HTLC-timeout close, mirroring +// the monitor's force-close conditions: an outbound HTLC closes the channel once +// `LATENCY_GRACE_PERIOD_BLOCKS` past expiry, an inbound HTLC with a known +// preimage once expiry is within `CLTV_CLAIM_BUFFER` blocks. Both LDK constants +// are private, so derive the claim buffer from the public fail-back buffer to +// catch drift. +const HTLC_TIMEOUT_GRACE_BLOCKS: u32 = 3; +const HTLC_CLAIM_BUFFER_BLOCKS: u32 = HTLC_FAIL_BACK_BUFFER - HTLC_TIMEOUT_GRACE_BLOCKS; +// 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( @@ -931,20 +999,11 @@ type ChanMan<'a> = ChannelManager< Arc, >; -#[inline] -fn assert_disconnect_action(action: &msgs::ErrorAction) -> (&msgs::WarningMessage, bool) { - // 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); - } +// Recognizes splice-quiescence warning disconnects, which are recoverable and +// must be distinguished from real close errors. +fn is_quiescent_disconnect_warning(msg: &msgs::WarningMessage) -> bool { + msg.data.contains("already sent splice_locked, cannot RBF") + || msg.data.contains("contribution no longer valid at quiescence") } #[derive(Clone, Copy, PartialEq)] @@ -980,6 +1039,7 @@ struct HarnessNode<'a> { fee_estimator: Arc, wallet: Arc, wallet_sync: WalletSync, Arc>, + bump_tx_handler: TestBumpTransactionEventHandler, persistence_style: ChannelMonitorUpdateStatus, deferred: bool, serialized_manager: Vec, @@ -1014,6 +1074,8 @@ impl<'a> HarnessNode<'a> { keys_manager: &Arc, logger: Arc, persister: &Arc, deferred: bool, ) -> Arc { + // The monitor shares the node's broadcaster so post-close claim and + // bump transactions reach the same queue that relay commands drain. Arc::new(chainmonitor::ChainMonitor::new( None, Arc::clone(broadcaster), @@ -1069,6 +1131,15 @@ impl<'a> HarnessNode<'a> { params, best_block_timestamp, ); + let bump_wallet_sync = Arc::new(WalletSync::new(Arc::clone(&wallet), Arc::clone(&logger))); + // Wallet-backed handler that turns BumpTransaction events into complete, + // broadcast anchor and HTLC claim transactions. + let bump_tx_handler = BumpTransactionEventHandlerSync::new( + Arc::clone(&broadcaster), + bump_wallet_sync, + Arc::clone(&keys_manager), + Arc::clone(&logger), + ); Self { node_id, node, @@ -1080,6 +1151,7 @@ impl<'a> HarnessNode<'a> { fee_estimator, wallet, wallet_sync, + bump_tx_handler, persistence_style, deferred, serialized_manager: Vec::new(), @@ -1242,6 +1314,15 @@ impl<'a> HarnessNode<'a> { self.node.timer_tick_occurred(); } + // Re-enables holder claim signing and asks the chain monitor to retry + // pending claim transactions. Different on-chain claim paths use + // SignHolderCommitment or SignHolderHtlcTransaction for force-closed channels. + fn enable_holder_signer_ops(&self) { + 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() } @@ -1269,6 +1350,7 @@ impl<'a> HarnessNode<'a> { ); } }, + Err(APIError::ChannelUnavailable { .. }) => {}, Err(e) => { assert!( matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), @@ -1288,8 +1370,12 @@ impl<'a> HarnessNode<'a> { .list_channels() .iter() .find(|chan| chan.channel_id == *channel_id) - .map(|chan| chan.outbound_capacity_msat) - .unwrap(); + .map(|chan| chan.outbound_capacity_msat); + let Some(outbound_capacity_msat) = outbound_capacity_msat else { + // The fuzzer can target a channel after a close removed it from + // list_channels; treat that as a stale splice command. + return; + }; if outbound_capacity_msat < 20_000_000 { return; } @@ -1312,6 +1398,7 @@ impl<'a> HarnessNode<'a> { ); } }, + Err(APIError::ChannelUnavailable { .. }) => {}, Err(e) => { assert!( matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), @@ -1322,6 +1409,18 @@ impl<'a> HarnessNode<'a> { } } + // Drains raw ChannelMonitor events. 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) { + 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 { @@ -1483,8 +1582,8 @@ impl EventQueues { fn route_from_middle<'a, I: IntoIterator>( &mut self, excess_events: I, expect_drop_node: Option, nodes: &[HarnessNode<'a>; 3], + close_tracker: &ChannelCloseTracker, ) { - // Push any events from Node B onto queues.ba and queues.bc. 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 { @@ -1514,7 +1613,9 @@ impl EventQueues { *node_id == a_id }, MessageSendEvent::HandleError { ref action, ref node_id } => { - assert_disconnect_action(action); + // Validate before routing so arbitrary HandleError output is + // not accepted as expected force-close behavior. + close_tracker.assert_expected_control_error_action(action); if Some(*node_id) == expect_drop_id { panic!( "peer_disconnected should drop msgs bound for the disconnected peer" @@ -1549,7 +1650,12 @@ impl EventQueues { } } - fn drain_on_disconnect(&mut self, edge_node: usize, nodes: &[HarnessNode<'_>; 3]) { + // Drains messages generated by a peer disconnect, validating HandleError + // events before the link queues are cleared. + fn drain_on_disconnect( + &mut self, edge_node: usize, nodes: &[HarnessNode<'_>; 3], + close_tracker: &ChannelCloseTracker, + ) { match edge_node { 0 => { for event in nodes[0].get_and_clear_pending_msg_events() { @@ -1563,12 +1669,17 @@ impl EventQueues { MessageSendEvent::BroadcastChannelUpdate { .. } => {}, MessageSendEvent::SendChannelUpdate { .. } => {}, MessageSendEvent::HandleError { ref action, .. } => { - assert_disconnect_action(action); + close_tracker.assert_expected_control_error_action(action); }, _ => 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, + close_tracker, + ); }, 2 => { for event in nodes[2].get_and_clear_pending_msg_events() { @@ -1582,12 +1693,17 @@ impl EventQueues { MessageSendEvent::BroadcastChannelUpdate { .. } => {}, MessageSendEvent::SendChannelUpdate { .. } => {}, MessageSendEvent::HandleError { ref action, .. } => { - assert_disconnect_action(action); + close_tracker.assert_expected_control_error_action(action); }, _ => 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, + close_tracker, + ); }, _ => panic!("unsupported disconnected edge"), } @@ -1637,7 +1753,55 @@ impl PeerLink { } } - fn disconnect(&mut self, nodes: &[HarnessNode<'_>; 3], queues: &mut EventQueues) { + // Asserts every untracked channel on this link is still listed by both + // peers, catching a force close that hit the wrong sibling channel. + fn assert_no_unexpected_channel_closes( + &self, nodes: &[HarnessNode<'_>; 3], close_tracker: &ChannelCloseTracker, + ) { + 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 close_tracker.is_closed_or_closing(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 disappeared from list_channels + // without a tracked close. + fn record_disappeared_channels( + &self, nodes: &[HarnessNode<'_>; 3], close_tracker: &mut ChannelCloseTracker, + ) { + 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 close_tracker.is_closed_or_closing(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 { + close_tracker.record_unexpected_channel_close(*channel_id); + } + } + } + + fn disconnect( + &mut self, nodes: &[HarnessNode<'_>; 3], queues: &mut EventQueues, + close_tracker: &ChannelCloseTracker, + ) { if self.disconnected { return; } @@ -1646,6 +1810,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 { @@ -1653,7 +1820,7 @@ impl PeerLink { } else { panic!("unsupported link topology") }; - queues.drain_on_disconnect(edge_node, nodes); + queues.drain_on_disconnect(edge_node, nodes, close_tracker); queues.clear_link(self); } @@ -1680,6 +1847,7 @@ impl PeerLink { fn disconnect_for_reload( &mut self, restarted_node: usize, nodes: &[HarnessNode<'_>; 3], queues: &mut EventQueues, + close_tracker: &ChannelCloseTracker, ) { if self.disconnected { return; @@ -1692,10 +1860,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, + close_tracker, ); } else { nodes[remaining_node].get_and_clear_pending_msg_events(); @@ -1704,99 +1876,477 @@ 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 { + Sent, + Failed, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum PaymentStatus { + Pending, + 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, +} + +struct ChannelCloseTracker { + // 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. + closed_channels: HashMap, +} + +impl ChannelCloseTracker { + fn new() -> Self { + Self { closed_channels: new_hash_map() } + } + + fn is_closed_or_closing(&self, channel_id: &ChannelId) -> bool { + self.closed_channels.contains_key(channel_id) + } + + fn is_expected(&self, channel_id: &ChannelId) -> bool { + matches!(self.closed_channels.get(channel_id), Some(ChannelCloseState::Expected)) + } + + fn has_closed_channels(&self) -> bool { + !self.closed_channels.is_empty() + } + + fn expect_channel_close(&mut self, channel_id: ChannelId) { + // Upgrades a topology-observed Unexpected entry once a valid close + // reason is known. + self.closed_channels.insert(channel_id, ChannelCloseState::Expected); + } + + // Records a close observed only through topology. It stays Unexpected, and + // fails the final close invariant, unless a ChannelClosed event or timeout + // pre-check later upgrades it. + fn record_unexpected_channel_close(&mut self, channel_id: ChannelId) { + self.closed_channels.entry(channel_id).or_insert(ChannelCloseState::Unexpected); + } + + fn record_channel_closed_event( + &mut self, channel_id: ChannelId, reason: &events::ClosureReason, + ) { + // Most closes must have been expected before LDK emits ChannelClosed. + // HTLC timeout closes are the exception: the timeout condition can be + // observed while connecting the block, so the close event itself carries + // the evidence. + let expected_before_close = self.is_expected(&channel_id); + 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 { + self.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, + ); + } + + // Asserts an LDK HandleError action is one of the error paths the harness + // can produce intentionally. + fn assert_expected_control_error_action(&self, action: &msgs::ErrorAction) { + 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. + msg.data.starts_with(HTLC_TIMEOUT_ERROR_PREFIX) + || msg.data.starts_with(CLOSED_CHANNEL_WRONG_NODE_ERROR_PREFIX) + || (self.is_expected(&msg.channel_id) + && (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); + } + + // Verifies every tracked close was authorized by an explicit force-close + // command or an HTLC-timeout condition. + fn assert_no_unexpected_channel_closes(&self) { + for (channel_id, state) in &self.closed_channels { + assert!( + *state == ChannelCloseState::Expected, + "Channel {:?} closed without an explicit force-close or HTLC timeout", + channel_id, + ); + } + } +} + +#[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, +} + +impl PendingPayment { + // Payments are registered with at least one path, asserted in + // register_payment, so these do not need an emptiness check. + fn all_paths_blocked(&self) -> bool { + (0..self.paths.len()).all(|path_idx| self.blocked_paths.contains(&path_idx)) + } + + fn all_paths_finished(&self) -> bool { + (0..self.paths.len()).all(|path_idx| { + self.blocked_paths.contains(&path_idx) || self.failed_paths.contains(&path_idx) + }) + } } -struct NodePayments { - pending: Vec, - resolved: HashMap>, +struct PaymentTracker { + payment_ctr: u64, + records_by_id: HashMap, + 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, } -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(), + } } - fn add_pending( - &mut self, payment_id: PaymentId, payment_hash: PaymentHash, - first_persisted_manager_generation: u64, + // Starts tracking a successfully accepted outbound payment under both its + // id and hash. + 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 { + assert!(!payment_paths.is_empty(), "tracked payment must have at least one path"); + 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)); + 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) + } + + 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. + 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 } - 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 any claim_funds call still needs receiver or sender-side + // accounting. + fn has_unfinished_claims(&self) -> bool { + self.records_by_id.values().any(|record| { + let still_needs_receiver_event = + !record.receiver_claimed && !record.all_paths_finished(); + record.claim_funds_called + && (still_needs_receiver_event + || (record.sender_outcome.is_none() && !record.all_paths_blocked())) + }) + } + + // Reports whether payment state can still make cleanup progress. + fn has_live_payment_work(&self) -> bool { + self.records_by_id + .values() + .any(|record| record.status == PaymentStatus::Pending && !record.all_paths_finished()) + || self.has_unfinished_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 claim_funds obligation 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) { + 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, for + // HTLC-timeout close events that identify the affected hash. + fn block_paths_containing_channel(&mut self, hash: &PaymentHash, channel_id: ChannelId) { + 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. Dust HTLCs have no + // commitment output, so once the channel closes they can never resolve + // through an on-chain claim. + fn block_dust_paths_containing_channel(&mut self, channel_id: ChannelId, dust_limit_msat: u64) { + 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, -} + // Blocks tracked paths through closing channels before final accounting. + // Pending payments use this to stop waiting for routes that can no longer + // resolve normally. Claimed payments use the same marker to allow a missing + // PaymentClaimed only when every route path became unresolvable. + fn block_unresolvable_closed_paths(&mut self, close_tracker: &ChannelCloseTracker) { + for record in self.records_by_id.values_mut() { + 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, _, _)| close_tracker.is_closed_or_closing(chan_id)); + if path_finished || !path_contains_closed_channel { + continue; + } + record.blocked_paths.insert(path_idx); + } + } + } -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, + // Stops final cleanup from waiting on send attempts that were accepted into + // outbound payment tracking but never committed an HTLC to any live channel. + // There is no peer or on-chain HTLC left for the harness to drive, so these + // do not belong in the force-close settlement invariant. + fn clear_uncommitted_pending_sends(&mut self, nodes: &[HarnessNode<'_>; 3]) { + for record in self.records_by_id.values_mut() { + if record.status != PaymentStatus::Pending + || record.sender_outcome.is_some() + || record.receiver_claimed + || record.claim_funds_called + || Self::has_pending_outbound_htlc( + &nodes[record.source_idx].node, + record.payment_hash, + ) { + continue; + } + record.status = PaymentStatus::Resolved; + } + } + + // Records a path-level failure event against a tracked payment, for MPP + // accounting where individual paths can fail before the 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); + } + } + + 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: every path resolved, failed, or + // was blocked by a close or dust. + fn assert_no_pending(&self, context: &str) { + let mut pending_ids = [Vec::new(), Vec::new(), Vec::new()]; + for record in self.records_by_id.values().filter(|record| { + record.status == PaymentStatus::Pending && !record.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 outcomes: either the receiver saw + // PaymentClaimed, or every route path became terminal before the receiver + // event was needed. "Terminal" includes explicit PaymentPathFailed events as + // well as blocked paths because an on-chain timeout can fail dust or expired + // HTLCs back without a receiver-side PaymentClaimed. + // + // Sender-side PaymentFailed is accepted after at least one blocked path, or + // after every path has failed or blocked. For MPP, one blocked path is + // enough to make the whole payment fail even when the remaining paths have + // not yet failed back through normal path-failure events. + fn assert_claims_resolved(&self) { + for record in self.records_by_id.values().filter(|record| record.claim_funds_called) { + let all_paths_blocked = record.all_paths_blocked(); + let all_paths_finished = record.all_paths_finished(); + assert!( + record.receiver_claimed || all_paths_finished, + "Payment {:?} was claimed with claim_funds but receiver never got PaymentClaimed", + record.payment_hash, + ); + match record.sender_outcome { + Some(SenderOutcome::Sent) => {}, + Some(SenderOutcome::Failed) => assert!( + !record.blocked_paths.is_empty() || all_paths_finished, + "claimed payment {:?} failed sender-side before any path was blocked \ + or all paths had finished: \ + 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, + ), + } } } - // Returns a bool indicating whether the payment failed. + // Checks whether LDK kept outbound state for a send attempt. 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 { @@ -1805,10 +2355,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; }, _ => {}, } @@ -1816,6 +2369,20 @@ impl PaymentTracker { return false; } + // Detects whether a payment hash still has outbound HTLCs that left the + // holding cell, to keep exhausted-retry payments tracked until those HTLCs + // resolve. + 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 && htlc.htlc_id.is_some()) + }) + } + + // Creates counter-derived payment material so hand-built fuzz inputs replay + // the same scenario. The receiver registers the hash, which yields a real + // PaymentSecret accepted by LDK. fn next_payment(&mut self, dest: &ChanMan) -> (PaymentSecret, PaymentHash, PaymentId) { self.payment_ctr += 1; let mut payment_preimage = PaymentPreimage([0; 32]); @@ -1830,10 +2397,16 @@ impl PaymentTracker { (secret, hash, id) } + // Sends a single-hop payment and records it only if LDK kept outbound state. fn send( &mut self, nodes: &[HarnessNode<'_>; 3], source_idx: usize, dest_idx: usize, - dest_chan_id: ChannelId, amt: u64, + dest_chan_id: ChannelId, amt: u64, close_tracker: &ChannelCloseTracker, ) -> bool { + if close_tracker.is_closed_or_closing(&dest_chan_id) { + // Sending over a known-closing channel 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); @@ -1850,7 +2423,7 @@ 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 { @@ -1861,13 +2434,14 @@ 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)]]; let onion = RecipientOnionFields::secret_only(secret, amt); let res = source.send_payment_with_route(route, hash, onion, id); let succeeded = match res { @@ -1882,19 +2456,31 @@ 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 to cover forwarded HTLC + // cleanup. 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, + close_tracker: &ChannelCloseTracker, ) { + if close_tracker.is_closed_or_closing(&middle_chan_id) + || close_tracker.is_closed_or_closing(&dest_chan_id) + { + // Once either hop closes, 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]; @@ -1911,15 +2497,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 close tracker records + // the topology change. 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 { @@ -1931,7 +2522,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 { @@ -1940,7 +2531,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, }, ], @@ -1948,6 +2539,10 @@ 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), + ]]; let onion = RecipientOnionFields::secret_only(secret, amt); let res = source.send_payment_with_route(route, hash, onion, id); let succeeded = match res { @@ -1963,69 +2558,77 @@ 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 split across the destination channels still + // considered live, so coverage of partially-closed MPP destinations remains. fn send_mpp_direct( &mut self, nodes: &[HarnessNode<'_>; 3], source_idx: usize, dest_idx: usize, - dest_chan_ids: &[ChannelId], amt: u64, + dest_chan_ids: &[ChannelId], amt: u64, close_tracker: &ChannelCloseTracker, ) { let source = &nodes[source_idx]; let dest = &nodes[dest_idx]; - let (secret, hash, id) = self.next_payment(dest); - let num_paths = dest_chan_ids.len(); + let dest_chans = dest.list_channels(); + let dest_scids: Vec<_> = dest_chan_ids + .iter() + .filter(|chan_id| !close_tracker.is_closed_or_closing(chan_id)) + .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 (secret, hash, id) = self.next_payment(dest); let amt_per_path = amt / num_paths as u64; - let mut paths = Vec::with_capacity(num_paths); - - let dest_chans = dest.list_channels(); - let dest_scids = dest_chan_ids.iter().map(|chan_id| { - dest_chans - .iter() - .find(|chan| chan.channel_id == *chan_id) - .and_then(|chan| chan.short_channel_id) - .unwrap() - }); - - for (i, dest_scid) in dest_scids.enumerate() { - 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, - channel_features: dest.channel_features(), - fee_msat: path_amt, - cltv_expiry_delta: 200, - maybe_announced_channel: true, - }], - blinded_tail: None, - }); - } - + // Build the tracked paths first and derive the Route from them so the + // two cannot drift apart. + let payment_paths: Vec = dest_scids + .iter() + .enumerate() + .map(|(i, (chan_id, dest_scid))| { + // The last path receives the rounding remainder. + 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(); + let paths = payment_paths + .iter() + .map(|payment_path| { + let (_, path_amt, dest_scid) = payment_path[0]; + Path { + hops: vec![RouteHop { + pubkey: dest.get_our_node_id(), + node_features: dest.node_features(), + short_channel_id: dest_scid, + channel_features: dest.channel_features(), + fee_msat: path_amt, + cltv_expiry_delta: FUZZ_FINAL_CLTV_DELTA, + maybe_announced_channel: true, + }], + blinded_tail: None, + } + }) + .collect(); 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) }; @@ -2036,100 +2639,114 @@ 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 the live channels on one or both + // links. The path count is the larger live channel count; the shorter side + // is reused round-robin so paths can be blocked independently by either hop. 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, + close_tracker: &ChannelCloseTracker, ) { 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 .iter() - .map(|chan_id| { + .filter(|chan_id| !close_tracker.is_closed_or_closing(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 .iter() - .map(|chan_id| { + .filter(|chan_id| !close_tracker.is_closed_or_closing(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(); - - 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 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 - }; - - paths.push(Path { - hops: vec![ - RouteHop { - pubkey: middle.get_our_node_id(), - node_features: middle.node_features(), - short_channel_id: middle_scid, - channel_features: middle.channel_features(), - fee_msat: path_fee, - cltv_expiry_delta: 100, - maybe_announced_channel: true, - }, - RouteHop { - pubkey: dest.get_our_node_id(), - node_features: dest.node_features(), - short_channel_id: dest_scid, - channel_features: dest.channel_features(), - fee_msat: path_amt, - cltv_expiry_delta: 200, - maybe_announced_channel: true, - }, - ], - blinded_tail: None, - }); + if middle_scids.is_empty() || dest_scids.is_empty() { + return; } - + let num_paths = middle_scids.len().max(dest_scids.len()); + let (secret, hash, id) = self.next_payment(dest); + 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; + // Build the tracked paths first and derive the Route from them so the + // two cannot drift apart. + let payment_paths: Vec = (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()]; + // The last path receives the rounding remainders. + 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(); + let paths = payment_paths + .iter() + .map(|payment_path| { + let (_, middle_amt, middle_scid) = payment_path[0]; + let (_, path_amt, dest_scid) = payment_path[1]; + Path { + hops: vec![ + RouteHop { + pubkey: middle.get_our_node_id(), + node_features: middle.node_features(), + short_channel_id: middle_scid, + channel_features: middle.channel_features(), + fee_msat: middle_amt - path_amt, + cltv_expiry_delta: FUZZ_FORWARD_CLTV_DELTA, + maybe_announced_channel: true, + }, + RouteHop { + pubkey: dest.get_our_node_id(), + node_features: dest.node_features(), + short_channel_id: dest_scid, + channel_features: dest.channel_features(), + fee_msat: path_amt, + cltv_expiry_delta: FUZZ_FINAL_CLTV_DELTA, + maybe_announced_channel: true, + }, + ], + blinded_tail: None, + } + }) + .collect(); 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) }; @@ -2140,50 +2757,87 @@ 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. fn claim_payment(&mut self, node: &HarnessNode<'_>, payment_hash: PaymentHash, fail: bool) { if fail { + // Fuzz bytes can choose to fail claimable payments backward, + // preserving 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 resolves the pending record. + 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 resolves the pending record. + 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); + } + + 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. With assert_already_resolved, + // a duplicate resolution must be Resolved rather than RolledBack. + 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. Ignore those + // unknown ids, except for PaymentSent where 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 assert_already_resolved && record.status != PaymentStatus::Pending { + assert_eq!(record.status, PaymentStatus::Resolved); + } + record.status = PaymentStatus::Resolved; } } @@ -2196,11 +2850,16 @@ struct Harness<'a, Out: Output + MaybeSend + MaybeSync> { bc_link: PeerLink, queues: EventQueues, payments: PaymentTracker, + close_tracker: ChannelCloseTracker, } fn build_node_config(chan_type: ChanType) -> UserConfig { let mut config = UserConfig::default(); 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 { @@ -2220,15 +2879,56 @@ 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); +fn assert_no_stale_splice_negotiation( + node: &HarnessNode<'_>, channel_id: &ChannelId, counterparty_node_id: &PublicKey, context: &str, +) { + let Some(channel) = node.list_channels().into_iter().find(|channel| { + channel.channel_id == *channel_id && channel.counterparty.node_id == *counterparty_node_id + }) else { + return; + }; + let Some(details) = channel.splice_details else { return }; + + assert!( + details.negotiation.is_none(), + "{} left active splice negotiation behind: {:?}", + context, + details + ); + assert!( + details.queued_contribution.is_some() + || !details.candidates.is_empty() + || details.confirmed_candidate.is_some() + || details.received_splice_locked_txid.is_some(), + "{} left empty splice details behind: {:?}", + context, + details + ); +} - // 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()); +// Checks topology and broadcast invariants that should hold after cleanup. +fn assert_test_invariants(nodes: &[HarnessNode<'_>; 3], has_closed_channels: bool) { + let channel_counts = [ + nodes[0].list_channels().len(), + nodes[1].list_channels().len(), + nodes[2].list_channels().len(), + ]; + if has_closed_channels { + // Once a close is tracked, some channels may legitimately be absent from + // list_channels. Still assert that no node ever gains channels, which + // would indicate the harness lost track of topology. + assert!(channel_counts[0] <= 3); + assert!(channel_counts[1] <= 6); + assert!(channel_counts[2] <= 3); + } else { + assert_eq!(channel_counts, [3, 6, 3]); + } + + // All broadcasters should be empty because broadcast transactions only enter + // the modeled mempool through explicit relay commands or finish cleanup. + for node in nodes { + assert!(node.broadcaster.txn_broadcasted.borrow().is_empty()); + } } fn connect_peers(source: &ChanMan<'_>, dest: &ChanMan<'_>) { @@ -2325,6 +3025,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, @@ -2341,6 +3044,9 @@ fn make_channel( tx.clone(), ) .unwrap(); + // Mine through the chain model instead of directly notifying nodes, + // so the setup path and later force-close path share the same block + // representation. chain_state.mine_setup_tx_to_depth(tx, ANTI_REORG_DELAY); } else { panic!("Wrong event type"); @@ -2464,6 +3170,9 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { let wallets = [wallet_a.as_ref(), wallet_b.as_ref(), wallet_c.as_ref()]; let mut chain_state = ChainState::new(); for wallet in wallets { + // Seed each wallet generously; anchor and splice flows need fresh + // inputs long after setup, and an exhausted wallet would obscure the + // close behavior under test. let coinbase_tx = bitcoin::Transaction { version: bitcoin::transaction::Version::TWO, lock_time: bitcoin::absolute::LockTime::ZERO, @@ -2525,6 +3234,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { chan_type, ), ]; + // Connect peers first, then create channels. connect_peers(&nodes[0], &nodes[1]); connect_peers(&nodes[1], &nodes[2]); @@ -2545,16 +3255,16 @@ 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; the setup funding is already represented in ChainState. + for node in &nodes { + 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); @@ -2582,6 +3292,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { bc_link: PeerLink::new(1, 2, chan_bc_ids), queues: EventQueues::new(), payments: PaymentTracker::new(), + close_tracker: ChannelCloseTracker::new(), } } @@ -2597,31 +3308,24 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { // Final invariants should not depend on the input ending with explicit relay // and mining bytes. fn finish(&mut self) { - for _ in 0..MAX_FINISH_RELAY_MINE_ROUNDS { - let mut txs = Vec::new(); - for node in &self.nodes { - txs.extend(node.broadcaster.txn_broadcasted.borrow_mut().drain(..)); - } - self.chain_state.relay_transactions(txs); - if self.chain_state.pending_txs.is_empty() { - assert_test_invariants(&self.nodes); - return; - } - if self.mine_blocks(ANTI_REORG_DELAY) == 0 { - // The input ended with pending mempool transactions but no safe - // block left before an HTLC fail-back window. Leave them - // unconfirmed rather than forcing finish cleanup to advance - // the chain past that boundary. - assert_test_invariants(&self.nodes); - return; - } - } - assert!( - !self.nodes.iter().any(|node| !node.broadcaster.txn_broadcasted.borrow().is_empty()) - && self.chain_state.pending_txs.is_empty(), - "finish tx mining loop failed to quiesce", - ); - assert_test_invariants(&self.nodes); + self.mine_relayed_txs_until_quiet("finish"); + self.record_disappeared_channels(); + self.assert_no_unexpected_channel_closes(); + let has_closed_channels = self.close_tracker.has_closed_channels(); + assert_test_invariants(&self.nodes, has_closed_channels); + } + + // Checks both peer links for unintended sibling-channel closes. + fn assert_no_unexpected_channel_closes(&self) { + self.ab_link.assert_no_unexpected_channel_closes(&self.nodes, &self.close_tracker); + self.bc_link.assert_no_unexpected_channel_closes(&self.nodes, &self.close_tracker); + } + + // Records closes visible only as channels missing from list_channels, which + // can happen before, or instead of, ChannelClosed event processing. + fn record_disappeared_channels(&mut self) { + self.ab_link.record_disappeared_channels(&self.nodes, &mut self.close_tracker); + self.bc_link.record_disappeared_channels(&self.nodes, &mut self.close_tracker); } fn link_between(&self, source_idx: usize, dest_idx: usize) -> &PeerLink { @@ -2642,15 +3346,16 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { 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) - } - 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, + &self.close_tracker, + ); } fn send_hop(&mut self, source_idx: usize, middle_idx: usize, dest_idx: usize, amt: u64) { @@ -2664,6 +3369,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { dest_idx, dest_chan_id, amt, + &self.close_tracker, ); } @@ -2679,6 +3385,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { dest_idx, &dest_chan_ids, amt, + &self.close_tracker, ); }, MppDirectChannels::RepeatedFirst => { @@ -2690,6 +3397,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { dest_idx, &dest_chan_ids, amt, + &self.close_tracker, ); }, } @@ -2714,6 +3422,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { dest_idx, &dest_chan_ids, amt, + &self.close_tracker, ); }, MppHopChannels::BothHops => { @@ -2725,6 +3434,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { dest_idx, &dest_chan_ids, amt, + &self.close_tracker, ); }, MppHopChannels::SecondHop => { @@ -2737,6 +3447,7 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { dest_idx, &dest_chan_ids, amt, + &self.close_tracker, ); }, } @@ -2851,7 +3562,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, + chain_state: &ChainState, close_tracker: &ChannelCloseTracker, out: &Out, ) -> Option { match event { MessageSendEvent::UpdateHTLCs { node_id, channel_id, updates } => { @@ -2874,6 +3585,12 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { None }, MessageSendEvent::SendChannelReestablish { ref node_id, ref msg } => { + if close_tracker.is_closed_or_closing(&msg.channel_id) { + // A delayed reestablish for a closing channel is stale + // traffic that would only manufacture a second error path + // for the same close. + 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); @@ -2914,6 +3631,12 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { MessageSendEvent::SendTxAbort { ref node_id, ref msg } => { let dest_idx = log_peer_message(node_idx, node_id, nodes, out, "tx_abort"); nodes[dest_idx].handle_tx_abort(source_node_id, msg); + assert_no_stale_splice_negotiation( + &nodes[dest_idx], + &msg.channel_id, + &source_node_id, + "tx_abort receive", + ); None }, MessageSendEvent::SendTxInitRbf { ref node_id, ref msg } => { @@ -2943,18 +3666,40 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { }, MessageSendEvent::SendSpliceLocked { ref node_id, ref msg } => { let dest_idx = log_peer_message(node_idx, node_id, nodes, out, "splice_locked"); + assert!( + chain_state.is_confirmed_txid(&msg.splice_txid), + "splice_locked referenced unconfirmed txid {}", + msg.splice_txid + ); 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 } => { + close_tracker.assert_expected_control_error_action(action); + 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 closes; exit quiescence on both peers. + 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 } => { + // Deliver the error so the counterparty observes the + // close 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 }, @@ -2974,8 +3719,10 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { } let nodes = &self.nodes; + let chain_state = &self.chain_state; let out = &self.out; let queues = &mut self.queues; + let close_tracker = &self.close_tracker; let mut events = queues.take_for_node(node_idx); let mut new_events = Vec::new(); if limit_events != ProcessMessages::OnePendingMessage { @@ -2994,6 +3741,8 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { corrupt_forward, limit_events, nodes, + chain_state, + close_tracker, out, ); if limit_events != ProcessMessages::AllMessages { @@ -3002,7 +3751,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, close_tracker); } else if node_idx == 0 { if let Some(ev) = extra_ev { queues.push_for_node(0, ev); @@ -3017,10 +3766,12 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { had_events } + // Drains user-facing events from one node and updates harness accounting. fn process_events(&mut self, node_idx: usize, fail: bool) -> bool { let nodes = &self.nodes; let payments = &mut self.payments; let chain_state = &self.chain_state; + let close_tracker = &mut self.close_tracker; // Multiple HTLCs can resolve for the same payment hash, so deduplicate // claim/fail handling per event batch. let mut claim_set = new_hash_map(); @@ -3028,29 +3779,60 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { let mut had_events = !events.is_empty(); for event in events.drain(..) { match event { - events::Event::PaymentClaimable { payment_hash, .. } => { + events::Event::PaymentClaimable { payment_hash, claim_deadline, .. } => { + if claim_deadline + .map_or(false, |deadline| nodes[node_idx].manager_height() >= deadline) + { + // Past the claim_deadline, claim_funds is outside its API + // contract; let LDK's timeout fail-back resolve the payment. + continue; + } if claim_set.insert(payment_hash.0, ()).is_none() { payments.claim_payment(&nodes[node_idx], payment_hash, fail); } }, events::Event::PaymentSent { payment_id, payment_hash, .. } => { - payments.nodes[node_idx].mark_sent(payment_id.unwrap(), payment_hash); + 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, .. } => { + 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, .. } => { + payments.mark_receiver_claimed(payment_hash); }, - events::Event::PaymentClaimed { .. } => {}, events::Event::PaymentPathSuccessful { .. } => {}, - events::Event::PaymentPathFailed { .. } => {}, + events::Event::PaymentPathFailed { payment_id, path, .. } => { + payments.mark_path_failed(node_idx, payment_id, &path); + }, events::Event::PaymentForwarded { .. } if node_idx == 1 => {}, - events::Event::ChannelReady { .. } => {}, + events::Event::ChannelReady { funding_txo, .. } => { + if let Some(funding_txo) = funding_txo { + assert!( + chain_state.is_confirmed_txid(&funding_txo.txid), + "ChannelReady referenced unconfirmed funding txid {}", + funding_txo.txid + ); + } + }, + 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, @@ -3083,6 +3865,13 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { Ok(()) => {}, Err(APIError::APIMisuseError { ref err }) if err.contains("does not have a pending splice negotiation") => {}, + Err(APIError::ChannelUnavailable { .. }) + if close_tracker.is_closed_or_closing(&channel_id) => + { + // The signing event was queued while the channel + // existed, but a close can be processed before the + // application handles the event. + }, Err(e) => panic!("{e:?}"), } } else { @@ -3100,17 +3889,47 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { // A queued signing event can be invalidated by a later `tx_abort` // before the application handles it. }, + Err(APIError::ChannelUnavailable { .. }) + if close_tracker.is_closed_or_closing(&channel_id) => + { + // A close can make a queued signing event stale before + // the application has a chance to provide signatures. + }, Err(e) => panic!("{e:?}"), } } }, - events::Event::SpliceNegotiated { .. } => {}, + events::Event::SpliceNegotiated { .. } => { + // LDK already broadcast the new funding transaction; relay and + // mining commands decide when it confirms. + }, events::Event::SpliceNegotiationFailed { .. } => {}, - events::Event::DiscardFunding { - funding_info: - events::FundingInfo::Contribution { .. } | events::FundingInfo::Tx { .. }, - .. - } => {}, + events::Event::ChannelClosed { channel_id, reason, .. } => { + close_tracker.record_channel_closed_event(channel_id, &reason); + // Dust HTLCs on the closed channel can never produce an + // on-chain claim, so block their paths up front. + 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 + { + // The timeout close names the affected payment, so only + // its paths through the closed channel are blocked. + payments.block_paths_containing_channel(&payment_hash, channel_id); + } + }, + events::Event::DiscardFunding { .. } => {}, + 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) => { + nodes[node_idx].bump_tx_handler.handle_event(&bump); + }, _ => panic!("Unhandled event: {:?}", event), } } @@ -3131,76 +3950,10 @@ 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. fn process_all_events(&mut self) { - let mut last_pass_no_updates = false; - for i in 0..std::usize::MAX { - if i == MAX_SETTLE_ITERATIONS { - panic!( - "It may take many 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(MAX_SETTLE_ITERATIONS); + assert!(settled, "process_all_events exceeded settle budget"); } // Finds the earliest loaded monitor height for a node. Startup sync uses it @@ -3217,20 +3970,42 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { min_monitor_height } + // Restarts one node from a fuzz-selected persisted state, updating payment + // records 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.close_tracker, + ); }, 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.close_tracker, + ); + self.bc_link.disconnect_for_reload( + 1, + &self.nodes, + &mut self.queues, + &self.close_tracker, + ); }, 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.close_tracker, + ); }, _ => panic!("invalid node index"), } @@ -3251,31 +4026,39 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { "reloaded node {} must sync to the harness tip before normal operation resumes", node_idx ); - let rolled_back_payment_hashes = self.payments.nodes[node_idx] - .sync_pending_with_manager_generation(loaded_manager_generation); + 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. fn settle_all(&mut self) { - let chain_state = &self.chain_state; - for node in &mut self.nodes { - node.sync_with_chain_state(chain_state, None); - } - - // First, make sure peers are all connected to each other + // The cleanup invariants are about eventual resolution, so undo any + // fuzzer-selected disconnections first. self.reconnect_ab(); self.reconnect_bc(); for op in SUPPORTED_SIGNER_OPS { + // Re-enable all signer ops so failures after this point indicate + // missing event, chain, or accounting progress rather than 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(); @@ -3287,21 +4070,85 @@ 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.close_tracker.has_closed_channels() { + 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); + // This only gates the explicit height advance below; progress + // rounds can still mine to confirm transactions already in the + // mempool. + 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.close_tracker); + self.payments.clear_uncommitted_pending_sends(&self.nodes); + 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.close_tracker.is_closed_or_closing(&chan.channel_id) { + continue; + } + let inbound_hashes = chan + .pending_inbound_htlcs + .iter() + .map(|htlc| htlc.payment_hash) + .collect::>(); + let outbound_hashes = chan + .pending_outbound_htlcs + .iter() + .map(|htlc| htlc.payment_hash) + .collect::>(); assert!( chan.pending_inbound_htlcs.is_empty() && chan.pending_outbound_htlcs.is_empty(), "Node {} channel {:?} has stuck HTLCs after settling all state: \ - {} inbound {:?}, {} outbound {:?}", + inbound_hashes={:?}, outbound_hashes={:?}, {} inbound {:?}, {} outbound {:?}", idx, chan.channel_id, + inbound_hashes, + outbound_hashes, chan.pending_inbound_htlcs.len(), chan.pending_inbound_htlcs, chan.pending_outbound_htlcs.len(), @@ -3310,22 +4157,35 @@ 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) - ); - } - 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) - ); + self.ab_link.complete_all_monitor_updates(&self.nodes); + self.bc_link.complete_all_monitor_updates(&self.nodes); + self.record_disappeared_channels(); + // Check both close-tracker and topology-level invariants. The close + // tracker verifies every observed close became expected; the peer links + // verify every non-closed sibling channel is still listed by both peers. + self.close_tracker.assert_no_unexpected_channel_closes(); + self.assert_no_unexpected_channel_closes(); + + // Every still-open channel must be able to send in at least one + // direction; closed channels are validated by close accounting instead. + for (node_a, node_b, channel_ids) in + [(0, 1, *self.ab_link.channel_ids()), (1, 2, *self.bc_link.channel_ids())] + { + for chan_id in channel_ids { + if self.close_tracker.is_closed_or_closing(&chan_id) { + continue; + } + self.assert_sendable_after_settle(node_a, node_b, chan_id); + } } + self.process_all_events(); + // Sendability probes can be accepted into a holding cell behind monitor + // progress. If they never committed an HTLC, they do not need cleanup + // accounting any more than the main fuzz-driven sends above. + self.payments.clear_uncommitted_pending_sends(&self.nodes); + 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(); @@ -3351,58 +4211,23 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { self.chain_state.relay_transactions(txs); } - fn earliest_pending_htlc_expiry(&self) -> Option { - let mut earliest_expiry: Option = None; + // Relays every node's pending broadcasts into the modeled mempool. Cleanup + // uses this when it should not depend on which peer fuzz bytes propagate. + // The bool reports whether any broadcasts were drained, not whether they + // were all admitted. + fn relay_all_broadcasts(&mut self) -> bool { + let mut txs = Vec::new(); for node in &self.nodes { - for chan in node.list_channels() { - for htlc in &chan.pending_inbound_htlcs { - earliest_expiry = Some( - earliest_expiry - .map_or(htlc.cltv_expiry, |expiry| expiry.min(htlc.cltv_expiry)), - ); - } - for htlc in &chan.pending_outbound_htlcs { - earliest_expiry = Some( - earliest_expiry - .map_or(htlc.cltv_expiry, |expiry| expiry.min(htlc.cltv_expiry)), - ); - } - } + txs.extend(node.broadcaster.txn_broadcasted.borrow_mut().drain(..)); } - earliest_expiry + self.chain_state.relay_transactions(txs) } - fn safe_mine_block_count(&self, count: u32) -> u32 { - if let Some(expiry) = self.earliest_pending_htlc_expiry() { - let current_tip = self.chain_state.tip_height(); - // LDK may close to protect a pending HTLC before its raw CLTV - // expiry. Keep mining outside that fail-back window so fuzzed block - // production does not force an on-chain timeout path. - let timeout_deadline = expiry.saturating_sub(channelmonitor::HTLC_FAIL_BACK_BUFFER); - assert!( - current_tip < timeout_deadline, - "pending HTLC with expiry {} and timeout deadline {} is already unsafe at tip {}", - expiry, - timeout_deadline, - current_tip - ); - // Stop before the deadline block itself, since connecting it is - // enough for ChannelMonitor timeout handling to run. - count.min(timeout_deadline - current_tip - 1) - } else { - count - } - } - - // Mines blocks through ChainState, then applies confirmed transactions to - // the wallets and syncs node chain listeners. - fn mine_blocks(&mut self, count: u32) -> u32 { + // Mines blocks through ChainState, applies confirmed transactions to the + // wallets, and syncs node chain listeners. + fn mine_blocks(&mut self, count: u32) { assert!(count > 0, "mining zero blocks should not be requested"); - let count = self.safe_mine_block_count(count); - if count == 0 { - return 0; - } let confirmed_txs = self.chain_state.mine_blocks(count); let wallets = [ self.nodes[0].wallet.as_ref(), @@ -3415,24 +4240,327 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> { for input in &tx.input { // The test wallet is a simple UTXO source. When one of its // outputs is spent by a confirmed transaction, remove it so - // later funding attempts cannot double-spend it. + // later funding or bump attempts cannot double-spend it. wallet.remove_utxo(input.previous_output); } for (vout, output) in tx.output.iter().enumerate() { if output.script_pubkey == change_script { - // Add outputs to whichever test wallet owns the script. - // This lets splice flows recycle wallet change through - // later fuzz commands. + // Confirmed wallet-owned outputs become new wallet + // inputs, letting anchor and splice flows reuse change + // through later fuzz commands. wallet.add_utxo(tx.clone(), vout as u32); } } } } - let chain_state = &self.chain_state; - for node in &mut self.nodes { - node.sync_with_chain_state(chain_state, None); + self.sync_all_nodes_with_chain_state(); + } + + // Repeatedly relays broadcasts and mines pending transactions to depth, 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 { + self.relay_all_broadcasts(); + if !self.chain_state.has_pending_txs() { + return; + } + self.mine_blocks(ANTI_REORG_DELAY); + } + 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 the monitors of closed channels, to know + // when on-chain funds still require more blocks, transactions, or monitor + // work. + fn claimable_balances(&self) -> Vec { + // get_claimable_balances skips monitors whose channel appears in the + // ignored list, so passing every open channel returns balances only for + // closed channels. + 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; these + // can block claim generation 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 for a claim that still + // needs sender-side resolution. Crossing such a boundary can change the + // failure mode the harness observes, so settlement stops mining before it. + fn next_claimed_htlc_boundary(&self, balances: &[Balance]) -> Option { + 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.all_paths_blocked() + }) + .unwrap_or(false); + claim_needs_sender_resolution.then_some(height) + }) + .min() + } + + // Checks whether any modeled subsystem still has work to do: queued + // messages, broadcasts, mempool entries, monitor updates, or claimable + // balances. + fn has_pending_work(&self) -> bool { + !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. + 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 so fuzzed persistence delay does + // not block cleanup. + 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); } - count + completed_monitor_update + } + + // Syncs every harness node to the current ChainState tip. + 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, first pre-authorizing the + // timeout closes LDK may perform while connecting the 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].manager_height().saturating_add(num_blocks), + self.chain_state.tip_height(), + ) + } else { + self.chain_state.tip_height() + }; + 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 allow LDK to force-close at the target height, + // mirroring the monitor's timeout conditions: an outbound HTLC past expiry + // by the grace period, or an inbound HTLC near expiry whose preimage the + // node may have learned through a claim. The monitor only knows a preimage + // after claim_funds, so checking the claim keeps this allowance close to + // what LDK can actually do. + fn allow_htlc_timeout_closes_for_node(&mut self, node_idx: usize, target_height: u32) { + for chan in self.nodes[node_idx].list_channels() { + 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.claim_funds_called(&htlc.payment_hash) + }); + if outbound_timed_out || inbound_timed_out { + self.close_tracker.expect_channel_close(chan.channel_id); + } + } + } + + // Drains monitor-generated events on every node; monitors can enqueue claim + // work independently of peer messages. + fn process_monitor_pending_events(&self) { + for node in &self.nodes { + node.process_monitor_pending_events(); + } + } + + // Runs one cleanup pass and reports whether anything advanced. + 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; + 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 { + // Mine mempool transactions deeply enough for confirmation-sensitive + // logic to run in the same progress round. + self.mine_blocks(ANTI_REORG_DELAY); + } + 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, since 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", + ); + } + + fn disconnect_ab(&mut self) { + self.ab_link.disconnect(&self.nodes, &mut self.queues, &self.close_tracker); + } + + fn disconnect_bc(&mut self) { + self.bc_link.disconnect(&self.nodes, &mut self.queues, &self.close_tracker); + } + + fn reconnect_ab(&mut self) { + self.ab_link.reconnect(&self.nodes); + } + + fn reconnect_bc(&mut self) { + self.bc_link.reconnect(&self.nodes); + } + + // Force-closes one channel, recording the expected close before calling + // into LDK so the invariants reject any other channel loss. + fn force_close( + &mut self, closer_idx: usize, channel_id: ChannelId, counterparty_idx: usize, reason: &str, + ) { + self.close_tracker.expect_channel_close(channel_id); + 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 a post-settle probe amount for one direction, clamping the + // historical 10_000_000 msat probe into the channel's current sendable + // range after fees, reserves, or force-close cleanup changed it. + 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| { + 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) + } + }) + } + + // Asserts a surviving channel can still route a payment in at least one + // direction after settlement. + fn assert_sendable_after_settle(&mut self, node_a: usize, node_b: usize, chan_id: ChannelId) { + let sent = + [(node_a, node_b), (node_b, node_a)].into_iter().any(|(source_idx, dest_idx)| { + let Some(amt) = self.probe_amount_for_direction(source_idx, chan_id) else { + return false; + }; + self.payments.send( + &self.nodes, + source_idx, + dest_idx, + chan_id, + amt, + &self.close_tracker, + ) + }); + assert!(sent, "channel {:?} cannot send in either direction after settling", chan_id); } } @@ -3644,13 +4772,13 @@ pub fn do_test(data: &[u8], out: Out) { }, // Sync node by 1 block. - 0xa8 => harness.nodes[0].sync_with_chain_state(&harness.chain_state, Some(1)), - 0xa9 => harness.nodes[1].sync_with_chain_state(&harness.chain_state, Some(1)), - 0xaa => harness.nodes[2].sync_with_chain_state(&harness.chain_state, Some(1)), + 0xa8 => harness.sync_node_with_chain_state(0, Some(1)), + 0xa9 => harness.sync_node_with_chain_state(1, Some(1)), + 0xaa => harness.sync_node_with_chain_state(2, Some(1)), // Sync node to chain tip. - 0xab => harness.nodes[0].sync_with_chain_state(&harness.chain_state, None), - 0xac => harness.nodes[1].sync_with_chain_state(&harness.chain_state, None), - 0xad => harness.nodes[2].sync_with_chain_state(&harness.chain_state, None), + 0xab => harness.sync_node_with_chain_state(0, None), + 0xac => harness.sync_node_with_chain_state(1, None), + 0xad => harness.sync_node_with_chain_state(2, None), 0xb0 | 0xb1 | 0xb2 => { // Restart node A, picking among persisted and in-flight `ChannelMonitor` @@ -3775,6 +4903,12 @@ pub fn do_test(data: &[u8], out: Out) { .enable_op_for_all_signers(SignerOp::SignSpliceSharedInput); harness.nodes[2].signer_unblocked(None); }, + // The harness toggles signer availability at node granularity, not + // per channel, so each byte re-enables both holder claim ops and + // asks that node's monitors to retry. + 0xd3 => harness.nodes[0].enable_holder_signer_ops(), + 0xd4 => harness.nodes[1].enable_holder_signer_ops(), + 0xd5 => harness.nodes[2].enable_holder_signer_ops(), 0xd6 => harness.relay_broadcasts_for_node(0), 0xd7 => harness.relay_broadcasts_for_node(1), 0xd8 => harness.relay_broadcasts_for_node(2), @@ -3782,6 +4916,13 @@ pub fn do_test(data: &[u8], out: Out) { let count = MINE_BLOCK_COUNTS[(v - 0xd9) 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. + 0xe1 => harness.force_close(0, harness.chan_a_id(), 1, FORCE_CLOSE_ERROR_MESSAGES[0]), + 0xe2 => harness.force_close(1, harness.chan_b_id(), 2, FORCE_CLOSE_ERROR_MESSAGES[1]), + 0xe3 => harness.force_close(1, harness.chan_a_id(), 0, FORCE_CLOSE_ERROR_MESSAGES[2]), + 0xe4 => harness.force_close(2, harness.chan_b_id(), 1, FORCE_CLOSE_ERROR_MESSAGES[3]), 0xf0 => harness.ab_link.complete_monitor_updates_for_node( 0, diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 2295ae3d7ff..aa3d274dac2 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -257,6 +257,7 @@ pub fn do_test(data: &[u8], out: Out) { pending_inbound_htlcs: Vec::new(), pending_outbound_htlcs: Vec::new(), current_dust_exposure_msat: None, + splice_details: None, }); } Some(&$first_hops_vec[..]) diff --git a/lightning-tests/src/upgrade_downgrade_tests.rs b/lightning-tests/src/upgrade_downgrade_tests.rs index 136ae919a97..dbb5372db0c 100644 --- a/lightning-tests/src/upgrade_downgrade_tests.rs +++ b/lightning-tests/src/upgrade_downgrade_tests.rs @@ -11,6 +11,7 @@ //! LDK. use lightning_0_2::commitment_signed_dance as commitment_signed_dance_0_2; +use lightning_0_2::events::bump_transaction::sync::WalletSourceSync as WalletSourceSync_0_2; use lightning_0_2::events::Event as Event_0_2; use lightning_0_2::get_monitor as get_monitor_0_2; use lightning_0_2::ln::channelmanager::PaymentId as PaymentId_0_2; @@ -60,6 +61,7 @@ use lightning::ln::splicing_tests::*; use lightning::ln::types::ChannelId; use lightning::onion_message::packet::Packet; use lightning::sign::OutputSpender; +use lightning::util::errors::APIError; use lightning::util::ser::{MaybeReadable, Writeable}; use lightning::util::wallet_utils::WalletSourceSync; @@ -819,3 +821,284 @@ fn test_onion_message_intercepted_scid_downgrade_to_0_2() { let result = ::read(&mut reader); assert!(result.is_err(), "LDK 0.2 should fail to decode a ShortChannelId variant"); } + +fn downgrade_setup_single_splice() -> (Vec, Vec, Vec, Vec) { + // Build a current node with a single pending (negotiated, not yet locked) splice that node 0 + // funded (so node 0 is contributory, node 1 is a non-contributory acceptor). Return both + // nodes' serialized ChannelManager + ChannelMonitor. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + let contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, contribution); + mine_transaction(&nodes[0], &splice_tx); + mine_transaction(&nodes[1], &splice_tx); + + let node_0_ser = nodes[0].node.encode(); + let node_1_ser = nodes[1].node.encode(); + let mon_0_ser = get_monitor!(nodes[0], channel_id).encode(); + let mon_1_ser = get_monitor!(nodes[1], channel_id).encode(); + (node_0_ser, node_1_ser, mon_0_ser, mon_1_ser) +} + +#[test] +fn downgrade_single_splice_loads_on_0_2() { + // A current node with a single pending splice serializes in a form LDK 0.2 can still read, + // whether or not we funded it: only odd TLVs are written (the even RBF gate is omitted for a + // single round), so 0.2 skips the contribution it can't track and loads the channel. RBF is + // the only state that blocks downgrade (see downgrade_rbf_refused_by_0_2). + let (node_0_ser, node_1_ser, mon_0_ser, mon_1_ser) = downgrade_setup_single_splice(); + + let mut chanmon_cfgs = lightning_0_2_utils::create_chanmon_cfgs(2); + chanmon_cfgs[0].keys_manager.disable_all_state_policy_checks = true; + chanmon_cfgs[1].keys_manager.disable_all_state_policy_checks = true; + let node_cfgs = lightning_0_2_utils::create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = lightning_0_2_utils::create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = lightning_0_2_utils::create_network(2, &node_cfgs, &node_chanmgrs); + let mut config = lightning_0_2_utils::test_default_channel_config(); + // The current side uses the anchors channel type by default; 0.2 only accepts a channel whose + // type it advertises support for, so enable anchors here too (otherwise the read is refused on + // the channel type, before the splice serialization is ever exercised). + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + + // Node 0 (contributory initiator): the contribution lives in an odd TLV that 0.2 skips. + let mgr_0 = lightning_0_2_utils::_reload_node( + &nodes[0], + config.clone(), + &node_0_ser, + &[&mon_0_ser[..]], + ); + assert_eq!(mgr_0.list_channels().len(), 1); + // Node 1 (non-contributory acceptor): nothing 0.2 can't represent. + let mgr_1 = + lightning_0_2_utils::_reload_node(&nodes[1], config, &node_1_ser, &[&mon_1_ser[..]]); + assert_eq!(mgr_1.list_channels().len(), 1); +} + +#[test] +#[should_panic(expected = "UnknownRequiredFeature")] +fn downgrade_rbf_refused_by_0_2() { + // RBF (more than one negotiation round) is the one splice state LDK 0.2 cannot operate. Current + // writes the even RBF-gate TLV for it, which 0.2 rejects as an unknown even (required) field, + // so reading the ChannelManager fails rather than silently mishandling the extra candidate. + let (node_0_ser, mon_0_ser); + { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + let contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let (first_splice_tx, new_funding_script) = + splice_channel(&nodes[0], &nodes[1], channel_id, contribution); + + // RBF the splice, producing a second negotiated candidate. + provide_utxo_reserves(&nodes, 2, added_value * 2); + let rbf_feerate = bitcoin::FeeRate::from_sat_per_kwu(1000); + let rbf_contribution = + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); + complete_rbf_handshake(&nodes[0], &nodes[1]); + complete_interactive_funding_negotiation( + &nodes[0], + &nodes[1], + channel_id, + rbf_contribution, + new_funding_script, + ); + let _ = sign_interactive_funding_tx( + &nodes[0], + &nodes[1], + false, + Some(first_splice_tx.compute_txid()), + ); + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + + node_0_ser = nodes[0].node.encode(); + mon_0_ser = get_monitor!(nodes[0], channel_id).encode(); + } + + let mut chanmon_cfgs = lightning_0_2_utils::create_chanmon_cfgs(2); + chanmon_cfgs[0].keys_manager.disable_all_state_policy_checks = true; + chanmon_cfgs[1].keys_manager.disable_all_state_policy_checks = true; + let node_cfgs = lightning_0_2_utils::create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = lightning_0_2_utils::create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = lightning_0_2_utils::create_network(2, &node_cfgs, &node_chanmgrs); + let mut config = lightning_0_2_utils::test_default_channel_config(); + // Match the anchors channel type used on the current side, so the manager read reaches (and + // fails on) the even RBF-gate TLV rather than refusing the channel type itself. + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + // _reload_node unwraps the manager read, which fails on the even RBF-gate TLV → panic. + let _ = lightning_0_2_utils::_reload_node(&nodes[0], config, &node_0_ser, &[&mon_0_ser[..]]); +} + +#[test] +fn upgrade_single_splice_from_0_2() { + // A pending single splice written by LDK 0.2 — which never tracked our contribution — is read + // by current: the candidate comes back via the TLV-3 fallback with `contribution: None`. + let (node_0_ser, node_1_ser, mon_0_ser, mon_1_ser, chan_id_bytes); + { + let chanmon_cfgs = lightning_0_2_utils::create_chanmon_cfgs(2); + let node_cfgs = lightning_0_2_utils::create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = lightning_0_2_utils::create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = lightning_0_2_utils::create_network(2, &node_cfgs, &node_chanmgrs); + let channel_id = lightning_0_2_utils::create_announced_chan_between_nodes_with_value( + &nodes, 0, 1, 100_000, 0, + ) + .2; + chan_id_bytes = channel_id.0; + + let contribution = lightning_0_2::ln::funding::SpliceContribution::SpliceOut { + outputs: vec![bitcoin::TxOut { + value: bitcoin::Amount::from_sat(1_000), + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }], + }; + // 0.2 drives the splice through tx_signatures, leaving one negotiated (unlocked) candidate. + let _ = lightning_0_2::ln::splicing_tests::splice_channel( + &nodes[0], + &nodes[1], + channel_id, + contribution, + ); + + node_0_ser = nodes[0].node.encode(); + node_1_ser = nodes[1].node.encode(); + mon_0_ser = get_monitor_0_2!(nodes[0], channel_id).encode(); + mon_1_ser = get_monitor_0_2!(nodes[1], channel_id).encode(); + } + + let mut chanmon_cfgs = create_chanmon_cfgs(2); + chanmon_cfgs[0].keys_manager.disable_all_state_policy_checks = true; + chanmon_cfgs[1].keys_manager.disable_all_state_policy_checks = true; + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let (persister_a, persister_b, chain_mon_a, chain_mon_b); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let (node_a, node_b); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let config = test_default_channel_config(); + reload_node!( + nodes[0], + config.clone(), + &node_0_ser, + &[&mon_0_ser[..]], + persister_a, + chain_mon_a, + node_a + ); + reload_node!( + nodes[1], + config, + &node_1_ser, + &[&mon_1_ser[..]], + persister_b, + chain_mon_b, + node_b + ); + + // Current reads the 0.2 splice: one negotiated candidate, no contribution recorded. + let channel_id = ChannelId(chan_id_bytes); + for node in nodes.iter() { + let channels = node.node.list_channels(); + let details = channels.iter().find(|c| c.channel_id == channel_id).unwrap(); + let splice = details.splice_details.as_ref().expect("pending splice"); + assert_eq!(splice.candidates.len(), 1); + assert_eq!(splice.candidates[0].contribution, None); + } + + // The splice cannot be RBF'd: 0.2 persisted no feerate, so splice_channel refuses it. + let node_id_1 = nodes[1].node.get_our_node_id(); + let err = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap_err(); + let expected = format!( + "Channel {} has a pending splice from a prior LDK version and cannot be spliced again", + channel_id, + ); + match err { + APIError::APIMisuseError { err } => assert_eq!(err, expected), + _ => panic!("unexpected error: {:?}", err), + } +} + +#[test] +fn splice_inherited_across_0_2_cannot_be_rbfed() { + // Negotiate a contributory splice on current, downgrade to LDK 0.2, then upgrade back. LDK 0.2 + // persists neither our contribution nor the splice feerate and does not retain the odd TLVs that + // carry them, so the splice returns to current without either. `splice_channel` needs the prior + // feerate to derive the RBF floor, so it refuses such a channel with a clean error rather than + // operating on incomplete state (which would otherwise trip a debug assertion that a pending + // splice always has a known feerate). + let chan_id_bytes; + let (v3_mgr, v3_mon); + { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0); + chan_id_bytes = channel_id.0; + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + let contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, contribution); + mine_transaction(&nodes[0], &splice_tx); + mine_transaction(&nodes[1], &splice_tx); + + v3_mgr = nodes[0].node.encode(); + v3_mon = get_monitor!(nodes[0], channel_id).encode(); + } + + // Downgrade node 0 to LDK 0.2 and re-serialize there, stripping the contribution and feerate. + let (v2_mgr, v2_mon); + { + let mut chanmon_cfgs = lightning_0_2_utils::create_chanmon_cfgs(2); + chanmon_cfgs[0].keys_manager.disable_all_state_policy_checks = true; + chanmon_cfgs[1].keys_manager.disable_all_state_policy_checks = true; + let node_cfgs = lightning_0_2_utils::create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = lightning_0_2_utils::create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = lightning_0_2_utils::create_network(2, &node_cfgs, &node_chanmgrs); + let mut config = lightning_0_2_utils::test_default_channel_config(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + let mgr = lightning_0_2_utils::_reload_node(&nodes[0], config, &v3_mgr, &[&v3_mon[..]]); + assert_eq!(mgr.list_channels().len(), 1); + let v2_channel_id = lightning_0_2::ln::types::ChannelId(chan_id_bytes); + v2_mgr = mgr.encode(); + v2_mon = get_monitor_0_2!(nodes[0], v2_channel_id).encode(); + } + + // Upgrade back to current and attempt to RBF the inherited splice. + let mut chanmon_cfgs = create_chanmon_cfgs(2); + chanmon_cfgs[0].keys_manager.disable_all_state_policy_checks = true; + chanmon_cfgs[1].keys_manager.disable_all_state_policy_checks = true; + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let (persister, chain_mon, new_node); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let config = test_default_channel_config(); + reload_node!(nodes[0], config, &v2_mgr, &[&v2_mon[..]], persister, chain_mon, new_node); + + let channel_id = ChannelId(chan_id_bytes); + let node_id_1 = nodes[1].node.get_our_node_id(); + let err = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap_err(); + let expected = format!( + "Channel {} has a pending splice from a prior LDK version and cannot be spliced again", + channel_id, + ); + match err { + APIError::APIMisuseError { err } => assert_eq!(err, expected), + _ => panic!("unexpected error: {:?}", err), + } +} diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a2412bbaf5e..2b13c1e42e2 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,34 @@ 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 { + // The delayed output created by the HTLC spend sits at the same + // index as the input spending the commitment HTLC output. This + // holds pre-anchors, where the spend has a single input and + // output, as well as post-anchors, where the counterparty + // signature commits to the pairing via + // SIGHASH_SINGLE | ANYONECANPAY. + let input_idx = tx.input.iter() + .position(|inp| inp.previous_output == input.previous_output) + .expect("input is one of tx.input") as u16; + debug_assert!( + self.has_delayed_maturing_output_for_tx(txid, input_idx, 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 +6495,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 +6547,21 @@ impl ChannelMonitorImpl { spendable_outputs } + #[cfg(debug_assertions)] + fn has_delayed_maturing_output_for_tx(&self, txid: Txid, output_index: u16, 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.outpoint.index == output_index && 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 75a4e1977d5..1fdd81cda58 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,212 @@ 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.clone(), + 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)); + + // Replaying the original claim requests after the reorg must not + // duplicate the resurrected outpoints in the delayed package either. + 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.len(), 2); + assert!(locked.contains(&spent_outpoint)); + assert!(locked.contains(&still_delayed_outpoint)); + assert!(tx_handler.pending_claim_requests.is_empty()); + assert!(tx_handler.claimable_outpoints.is_empty()); + } } 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/events/mod.rs b/lightning/src/events/mod.rs index ec0ad6ccd9b..2e56d35c887 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1647,8 +1647,12 @@ pub enum Event { /// [`ChainMonitor::get_claimable_balances`]: crate::chain::chainmonitor::ChainMonitor::get_claimable_balances last_local_balance_msat: Option, }, - /// Used to indicate that a splice for the given `channel_id` has been negotiated and its - /// funding transaction has been broadcast. + /// Used to indicate that a splice for the given `channel_id` has been negotiated, its + /// funding transaction has been broadcast, and local inputs or outputs were contributed to + /// it. + /// + /// This event is not emitted if the counterparty negotiated a splice without using a local + /// contribution. /// /// The splice is then considered pending until both parties have seen enough confirmations to /// consider the funding locked. Once this occurs, an [`Event::ChannelReady`] will be emitted. @@ -1679,9 +1683,9 @@ pub enum Event { }, /// Used to indicate that a splice negotiation round for the given `channel_id` has failed. /// - /// Each splice attempt (initial or RBF) resolves to either [`Event::SpliceNegotiated`] on - /// success or this event on failure. Prior successfully negotiated splice transactions are - /// unaffected. + /// Each splice attempt (initial or RBF) resolves to this event on failure. On success, + /// [`Event::SpliceNegotiated`] is emitted if the negotiated transaction includes local + /// inputs or outputs. Prior successfully negotiated splice transactions are unaffected. /// /// Any UTXOs contributed to the failed round that are not committed to a prior negotiated /// splice transaction will be returned via a preceding [`Event::DiscardFunding`]. diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index f36c19748f0..f60e63a87e9 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -1853,7 +1853,7 @@ fn test_async_splice_initial_commit_sig() { acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures); let _ = get_event!(initiator, Event::SpliceNegotiated); - let _ = get_event!(acceptor, Event::SpliceNegotiated); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); } #[test] @@ -1945,7 +1945,7 @@ fn test_async_splice_initial_commit_sig_waits_for_monitor_before_tx_signatures() acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures); let _ = get_event!(initiator, Event::SpliceNegotiated); - let _ = get_event!(acceptor, Event::SpliceNegotiated); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); } #[test] @@ -2022,5 +2022,5 @@ fn test_async_splice_shared_input_signature_released_on_unblock() { acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures); let _ = get_event!(initiator, Event::SpliceNegotiated); - let _ = get_event!(acceptor, Event::SpliceNegotiated); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ab9c964e5cb..4a23cf8309d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -47,8 +47,9 @@ use crate::ln::chan_utils::{ EMPTY_SCRIPT_SIG_WEIGHT, FUNDING_TRANSACTION_WITNESS_WEIGHT, }; use crate::ln::channel_state::{ - ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, - OutboundHTLCDetails, OutboundHTLCStateDetails, + ChannelShutdownState, ConfirmedSpliceCandidate, CounterpartyForwardingInfo, InboundHTLCDetails, + InboundHTLCStateDetails, OutboundHTLCDetails, OutboundHTLCStateDetails, SpliceCandidateDetails, + SpliceDetails, SpliceNegotiationDetails, SpliceNegotiationStatus, }; use crate::ln::channelmanager::{ self, BlindedFailure, ChannelReadyOrder, FundingConfirmedMessage, HTLCFailureMsg, @@ -84,7 +85,7 @@ use crate::util::config::{ use crate::util::errors::APIError; use crate::util::logger::{Level as LoggerLevel, Logger, Record, WithContext}; use crate::util::scid_utils::{block_from_scid, scid_from_parts}; -use crate::util::ser::{Readable, ReadableArgs, RequiredWrapper, Writeable, Writer}; +use crate::util::ser::{Iterable, Readable, ReadableArgs, RequiredWrapper, Writeable, Writer}; use crate::util::wallet_utils::{ConfirmedUtxo, Input}; use crate::{impl_readable_for_vec, impl_writeable_for_vec}; @@ -2964,9 +2965,20 @@ impl FundingScope { struct PendingFunding { funding_negotiation: Option, + /// Our contribution to the funding negotiation round currently in progress, if we are + /// contributing to it. Set when the round starts, moved into the [`NegotiatedCandidate`] + /// when negotiation completes, and dropped in + /// [`FundedChannel::reset_pending_splice_state`] if the round is abandoned. + /// + /// When the counterparty initiates an RBF and a prior round included our contribution, this + /// is set to that contribution adjusted to the new feerate (or the RBF is rejected if the + /// adjustment fails, in which case no round starts). This ensures a splice we contributed to + /// never loses our contribution in subsequent rounds. + negotiation_contribution: Option, + /// Funding candidates that have been negotiated but have not reached enough confirmations /// by both counterparties to have exchanged `splice_locked` and be promoted. - negotiated_candidates: Vec, + negotiated_candidates: Vec, /// The funding txid used in the `splice_locked` sent to the counterparty. sent_funding_txid: Option, @@ -2977,21 +2989,26 @@ struct PendingFunding { /// The feerate used in the last successfully negotiated funding transaction. /// Used for validating the minimum feerate increase rule on RBF attempts. last_funding_feerate_sat_per_1000_weight: Option, +} - /// The funding contributions from splice/RBF rounds where we contributed. - /// - /// A new entry is appended when we contribute to a negotiation round (either as initiator - /// or acceptor). Rounds where we don't contribute (e.g., counterparty-only splice) do not - /// add an entry. Once non-empty, every subsequent round appends: when the counterparty - /// initiates an RBF, the last entry is adjusted to the new feerate and appended as a new - /// entry (or the RBF is rejected if the adjustment fails, in which case no round starts). - /// - /// If the round aborts, the last entry is popped in - /// [`FundedChannel::reset_pending_splice_state`], restoring the prior round's contribution - /// as the most recent entry. - contributions: Vec, +/// A funding candidate that has been negotiated, together with our contribution, if any, to the +/// negotiation round that produced it. +#[derive(Debug)] +struct NegotiatedCandidate { + funding: FundingScope, + + /// Our contribution to the negotiation round that produced this candidate, or `None` if only + /// the counterparty contributed. Once a candidate includes our contribution, every later + /// candidate does as well: RBF rounds carry the contribution forward (possibly adjusted to a + /// new feerate) rather than dropping it, preserving the splice intention. + contribution: Option, } +impl_ser_tlv_based!(NegotiatedCandidate, { + (1, funding, required), + (3, contribution, option), +}); + #[derive(Debug)] enum FundingNegotiation { AwaitingAck { @@ -3050,20 +3067,47 @@ impl Writeable for PendingFundingWriteable<'_> { Some(FundingNegotiation::AwaitingSignatures { .. }) ) ); - let contributions_len = if self.reset_funding_negotiation - && self.pending_funding.funding_negotiation.is_some() - { - self.pending_funding.contributions.len().saturating_sub(1) - } else { - self.pending_funding.contributions.len() - }; + // The in-flight round's contribution is only written if its negotiation survives + // serialization round trips. It goes in an odd TLV that LDK 0.2 skips (0.2 never tracked + // contributions), so a single in-flight splice we contributed to stays loadable there. + let negotiation_contribution = funding_negotiation + .is_some() + .then(|| self.pending_funding.negotiation_contribution.as_ref()) + .flatten(); + let candidates = &self.pending_funding.negotiated_candidates; + debug_assert!( + candidates + .iter() + .skip_while(|candidate| candidate.contribution.is_none()) + .all(|candidate| candidate.contribution.is_some()), + "contributions must form a suffix of the negotiated candidates", + ); + // TLV 3 exposes only the first candidate's funding: the single-splice view LDK 0.2 + // understands. The authoritative candidate list — each funding bundled with its + // contribution — goes in the odd TLV 11, which current reads and 0.2 skips. A single + // non-contributory splice is fully captured by TLV 3 alone, so the bundle is then omitted. + // When a single splice does carry a contribution, 0.2 skips it (and operates the splice + // without it), so it need not block 0.2 from loading. + // + // The even TLV 14 is the only thing that makes 0.2 refuse, and it's written exactly when + // there is more than one negotiation round (RBF) — the one thing 0.2 cannot operate. The + // odd contribution fields are safe despite being load-bearing for RBF: this gate makes 0.2 + // refuse the whole channel in that case, so no reader ever skips them when they matter. + let first_funding = Iterable(candidates.iter().take(1).map(|candidate| &candidate.funding)); + let any_contribution = candidates.iter().any(|candidate| candidate.contribution.is_some()); + let negotiated_candidates = + (candidates.len() > 1 || any_contribution).then(|| Iterable(candidates.iter())); + let is_rbf = candidates.len() + usize::from(funding_negotiation.is_some()) > 1; + let rbf_gate = is_rbf.then_some(()); write_tlv_fields!(writer, { (1, funding_negotiation, upgradable_option), - (3, self.pending_funding.negotiated_candidates, required_vec), + (3, first_funding, required), (5, self.pending_funding.sent_funding_txid, option), (7, self.pending_funding.received_funding_txid, option), - (8, self.pending_funding.last_funding_feerate_sat_per_1000_weight, option), - (10, self.pending_funding.contributions[..contributions_len], optional_vec), + (9, self.pending_funding.last_funding_feerate_sat_per_1000_weight, option), + (11, negotiated_candidates, option), + (13, negotiation_contribution, option), + (14, rbf_gate, option), }); Ok(()) } @@ -3071,14 +3115,57 @@ impl Writeable for PendingFundingWriteable<'_> { impl Readable for PendingFunding { fn read(reader: &mut R) -> Result { - Ok(_decode_and_build!(reader, Self, { + let mut funding_negotiation = None; + let mut legacy_negotiated_candidates: Option> = None; + let mut sent_funding_txid = None; + let mut received_funding_txid = None; + let mut last_funding_feerate_sat_per_1000_weight = None; + let mut negotiated_candidates: Option> = None; + let mut negotiation_contribution: Option = None; + let mut rbf_gate: Option<()> = None; + + read_tlv_fields!(reader, { (1, funding_negotiation, upgradable_option), - (3, negotiated_candidates, required_vec), + (3, legacy_negotiated_candidates, optional_vec), (5, sent_funding_txid, option), (7, received_funding_txid, option), - (8, last_funding_feerate_sat_per_1000_weight, option), - (10, contributions, optional_vec), - })) + (9, last_funding_feerate_sat_per_1000_weight, option), + (11, negotiated_candidates, optional_vec), + (13, negotiation_contribution, option), + (14, rbf_gate, option), + }); + + // TLV 11 (the candidate list, each funding bundled with its contribution) is authoritative + // when present. It is omitted for a single non-contributory splice (TLV 3 holds its + // funding) and for data written by LDK 0.2 (which only ever wrote TLV 3 and tracked no + // contributions); in both cases the candidates carry no contribution. + let negotiated_candidates = negotiated_candidates.unwrap_or_else(|| { + legacy_negotiated_candidates + .unwrap_or_default() + .into_iter() + .map(|funding| NegotiatedCandidate { funding, contribution: None }) + .collect() + }); + // An in-flight contribution is only meaningful while its negotiation round is alive; + // rounds not surviving serialization round trips have their events handled separately. + if funding_negotiation.is_none() { + negotiation_contribution = None; + } + // TLV 14 only exists to make pre-RBF readers (LDK 0.2) refuse a channel they couldn't + // operate; current reconstructs RBF state from the candidate list, so it's informational. + debug_assert_eq!( + rbf_gate.is_some(), + negotiated_candidates.len() + usize::from(funding_negotiation.is_some()) > 1, + ); + + Ok(PendingFunding { + funding_negotiation, + negotiation_contribution, + negotiated_candidates, + sent_funding_txid, + received_funding_txid, + last_funding_feerate_sat_per_1000_weight, + }) } } @@ -3228,22 +3315,109 @@ impl PendingFunding { feerate_sat_per_kw >= min_feerate } + /// All stored contributions: those of the negotiated candidates followed by the in-flight + /// negotiation round's, if any. + fn contributions(&self) -> impl Iterator + '_ { + self.negotiated_candidates + .iter() + .filter_map(|candidate| candidate.contribution.as_ref()) + .chain(self.negotiation_contribution.as_ref()) + } + fn contributed_inputs(&self) -> impl Iterator + '_ { - self.contributions.iter().flat_map(|c| c.contributed_inputs()) + self.contributions().flat_map(|c| c.contributed_inputs()) } fn contributed_outputs(&self) -> impl Iterator + '_ { - self.contributions.iter().flat_map(|c| c.contributed_outputs()) + self.contributions().flat_map(|c| c.contributed_outputs()) } fn prior_contributed_inputs(&self) -> impl Iterator + '_ { - let len = self.contributions.len(); - self.contributions[..len.saturating_sub(1)].iter().flat_map(|c| c.contributed_inputs()) + self.negotiated_candidates + .iter() + .filter_map(|candidate| candidate.contribution.as_ref()) + .flat_map(|c| c.contributed_inputs()) } fn prior_contributed_outputs(&self) -> impl Iterator + '_ { - let len = self.contributions.len(); - self.contributions[..len.saturating_sub(1)].iter().flat_map(|c| c.contributed_outputs()) + self.negotiated_candidates + .iter() + .filter_map(|candidate| candidate.contribution.as_ref()) + .flat_map(|c| c.contributed_outputs()) + } + + /// Our most recent contribution across rounds, including any round still under negotiation. + fn latest_contribution(&self) -> Option<&FundingContribution> { + self.negotiation_contribution.as_ref().or_else(|| { + self.negotiated_candidates.last().and_then(|candidate| candidate.contribution.as_ref()) + }) + } + + fn to_details( + &self, context: &ChannelContext, best_block_height: u32, + queued_contribution: Option<&FundingContribution>, + ) -> SpliceDetails { + let negotiation = self.funding_negotiation.as_ref().map(|negotiation| { + let status = match negotiation { + FundingNegotiation::AwaitingAck { .. } => SpliceNegotiationStatus::AwaitingAck, + FundingNegotiation::ConstructingTransaction { .. } => { + SpliceNegotiationStatus::ConstructingTransaction + }, + FundingNegotiation::AwaitingSignatures { .. } => { + SpliceNegotiationStatus::AwaitingSignatures + }, + }; + SpliceNegotiationDetails { + status, + is_initiator: negotiation.is_initiator(), + funding_feerate_sat_per_1000_weight: negotiation + .funding_feerate_sat_per_1000_weight(), + new_channel_value_satoshis: negotiation + .as_funding() + .map(|funding| funding.get_value_satoshis()), + txid: negotiation.as_funding().and_then(|funding| funding.get_funding_txid()), + contribution: self.negotiation_contribution.clone(), + } + }); + let candidates = self + .negotiated_candidates + .iter() + .map(|candidate| SpliceCandidateDetails { + txid: candidate + .funding + .get_funding_txid() + .expect("negotiated candidates should have a funding txid"), + new_channel_value_satoshis: candidate.funding.get_value_satoshis(), + contribution: candidate.contribution.clone(), + }) + .collect(); + // At most one candidate can confirm, as they all double-spend the same input. + let confirmed_candidate = self.negotiated_candidates.iter().find_map(|candidate| { + let confirmations = candidate.funding.get_funding_tx_confirmations(best_block_height); + (confirmations > 0).then(|| { + let txid = candidate + .funding + .get_funding_txid() + .expect("negotiated candidates should have a funding txid"); + ConfirmedSpliceCandidate { + txid, + confirmations, + confirmations_required: context + .minimum_depth(&candidate.funding) + .expect("set for a ready channel"), + // The `splice_locked` we sent always refers to the confirmed candidate, as it + // is cleared if that candidate is ever unconfirmed by a reorg. + splice_locked_sent: self.sent_funding_txid == Some(txid), + } + }) + }); + SpliceDetails { + negotiation, + queued_contribution: queued_contribution.cloned(), + candidates, + confirmed_candidate, + received_splice_locked_txid: self.received_funding_txid, + } } fn check_get_splice_locked( @@ -3251,7 +3425,7 @@ impl PendingFunding { ) -> Option { debug_assert!(confirmed_funding_index < self.negotiated_candidates.len()); - let funding = &self.negotiated_candidates[confirmed_funding_index]; + let funding = &self.negotiated_candidates[confirmed_funding_index].funding; if !context.check_funding_meets_minimum_depth(funding, height) { return None; } @@ -7185,6 +7359,9 @@ pub struct SpliceFundingNegotiated { /// The outpoint of the channel's splice funding transaction. pub funding_txo: bitcoin::OutPoint, + /// Whether the holder contributed local inputs or outputs to the negotiated splice. + pub has_local_contribution: bool, + /// The features that this channel will operate with. pub channel_type: ChannelTypeFeatures, @@ -7224,8 +7401,7 @@ impl SpliceFundingFailed { } macro_rules! splice_funding_failed_for { - ($self: expr, $is_initiator: expr, $contribution: expr, - $contributed_inputs: ident, $contributed_outputs: ident) => {{ + ($self: expr, $contribution: expr, $contributed_inputs: ident, $contributed_outputs: ident) => {{ let contribution = $contribution; let existing_inputs = $self.pending_splice.as_ref().into_iter().flat_map(|ps| ps.$contributed_inputs()); @@ -7234,17 +7410,16 @@ macro_rules! splice_funding_failed_for { let filtered = contribution.clone().into_unique_contributions(existing_inputs, existing_outputs); match filtered { - None if !$is_initiator => None, - None => Some(SpliceFundingFailed { + None => SpliceFundingFailed { contributed_inputs: vec![], contributed_outputs: vec![], contribution: Some(contribution), - }), - Some((contributed_inputs, contributed_outputs)) => Some(SpliceFundingFailed { + }, + Some((contributed_inputs, contributed_outputs)) => SpliceFundingFailed { contributed_inputs, contributed_outputs, contribution: Some(contribution), - }), + }, } }}; } @@ -7275,16 +7450,10 @@ where /// Builds a [`SpliceFundingFailed`] from a contribution, filtering out inputs/outputs /// that are still committed to a prior splice round. fn splice_funding_failed_for(&self, contribution: FundingContribution) -> SpliceFundingFailed { - // The contribution was never pushed to `contributions`, so `contributed_inputs()` and - // `contributed_outputs()` return only prior rounds' entries for filtering. - splice_funding_failed_for!( - self, - true, - contribution, - contributed_inputs, - contributed_outputs - ) - .expect("is_initiator is true so this always returns Some") + // The contribution was never stored in the pending splice state, so + // `contributed_inputs()` and `contributed_outputs()` return only prior rounds' entries + // for filtering. + splice_funding_failed_for!(self, contribution, contributed_inputs, contributed_outputs) } fn abandon_quiescent_action(&mut self) -> Option { @@ -7326,12 +7495,15 @@ where }) } - fn pending_funding(&self) -> &[FundingScope] { - if let Some(pending_splice) = &self.pending_splice { - pending_splice.negotiated_candidates.as_slice() - } else { - &[] - } + fn negotiated_candidates(&self) -> &[NegotiatedCandidate] { + self.pending_splice + .as_ref() + .map(|pending_splice| pending_splice.negotiated_candidates.as_slice()) + .unwrap_or(&[]) + } + + fn pending_funding(&self) -> impl ExactSizeIterator + '_ { + self.negotiated_candidates().iter().map(|candidate| &candidate.funding) } fn funding_and_pending_funding_iter_mut(&mut self) -> impl Iterator { @@ -7340,10 +7512,40 @@ where .as_mut() .map(|pending_splice| pending_splice.negotiated_candidates.as_mut_slice()) .unwrap_or(&mut []) - .iter_mut(), + .iter_mut() + .map(|candidate| &mut candidate.funding), ) } + /// Returns details about any pending splice attempts for inclusion in + /// [`crate::ln::channel_state::ChannelDetails`]. + pub fn pending_splice_details(&self, best_block_height: u32) -> Option { + // A contribution committed via `funding_contributed` sits in `quiescent_action` until + // quiescence is reached and it becomes a negotiation; surface it as the queued contribution. + let queued_contribution = match &self.quiescent_action { + Some(QuiescentAction::Splice { contribution, .. }) => Some(contribution), + #[cfg(any(test, fuzzing, feature = "_test_utils"))] + Some(QuiescentAction::DoNothing) => None, + None => None, + }; + self.pending_splice + .as_ref() + .map(|pending_splice| { + pending_splice.to_details(&self.context, best_block_height, queued_contribution) + }) + // No `PendingFunding` yet (e.g. a first splice still awaiting quiescence), but a queued + // contribution is still worth surfacing on its own. + .or_else(|| { + queued_contribution.map(|contribution| SpliceDetails { + negotiation: None, + queued_contribution: Some(contribution.clone()), + candidates: Vec::new(), + confirmed_candidate: None, + received_splice_locked_txid: None, + }) + }) + } + fn has_pending_splice_awaiting_signatures(&self) -> bool { self.pending_splice .as_ref() @@ -7426,12 +7628,8 @@ where pending_splice.funding_negotiation.is_some(), "reset_pending_splice_state requires an active funding negotiation" ); - let is_initiator = pending_splice - .funding_negotiation - .take() - .map(|negotiation| negotiation.is_initiator()) - .unwrap_or(false); - let contribution = pending_splice.contributions.pop(); + pending_splice.funding_negotiation.take(); + let contribution = pending_splice.negotiation_contribution.take(); if let Some(ref contribution) = contribution { debug_assert!( pending_splice @@ -7442,19 +7640,13 @@ where ); } - // After pop, `contributed_inputs()` / `contributed_outputs()` return only prior - // rounds for filtering. - let splice_funding_failed = contribution.and_then(|contribution| { - splice_funding_failed_for!( - self, - is_initiator, - contribution, - contributed_inputs, - contributed_outputs - ) + // With the in-flight contribution taken, `contributed_inputs()` / + // `contributed_outputs()` return only prior rounds' entries for filtering. + let splice_funding_failed = contribution.map(|contribution| { + splice_funding_failed_for!(self, contribution, contributed_inputs, contributed_outputs) }); - if self.pending_funding().is_empty() { + if self.negotiated_candidates().is_empty() { self.pending_splice.take(); } @@ -7476,19 +7668,13 @@ where pending_splice.funding_negotiation.is_some(), "maybe_splice_funding_failed requires an active funding negotiation" ); - let is_initiator = pending_splice - .funding_negotiation - .as_ref() - .map(|negotiation| negotiation.is_initiator()) - .unwrap_or(false); - let contribution = pending_splice.contributions.last().cloned()?; - splice_funding_failed_for!( + let contribution = pending_splice.negotiation_contribution.clone()?; + Some(splice_funding_failed_for!( self, - is_initiator, contribution, prior_contributed_inputs, prior_contributed_outputs - ) + )) } #[rustfmt::skip] @@ -8114,7 +8300,7 @@ where } core::iter::once(&self.funding) - .chain(self.pending_funding().iter()) + .chain(self.pending_funding()) .try_for_each(|funding| self.context.validate_update_add_htlc(funding, msg, fee_estimator))?; // Now update local state: @@ -8546,7 +8732,7 @@ where let funding_contribution = self .pending_splice .as_ref() - .and_then(|pending_splice| pending_splice.contributions.last()) + .and_then(|pending_splice| pending_splice.negotiation_contribution.as_ref()) .cloned(); log_info!( @@ -8609,7 +8795,7 @@ where ) -> Result, ChannelError> { self.commitment_signed_check_state()?; - if !self.pending_funding().is_empty() { + if !self.negotiated_candidates().is_empty() { return Err(ChannelError::close( "Got a single commitment_signed message when expecting a batch".to_owned(), )); @@ -8677,7 +8863,7 @@ where // pending splice transaction has confirmed since receiving the batch. let mut commitment_txs = Vec::with_capacity(self.pending_funding().len() + 1); let mut htlc_data = None; - for funding in core::iter::once(&self.funding).chain(self.pending_funding().iter()) { + for funding in core::iter::once(&self.funding).chain(self.pending_funding()) { let funding_txid = funding.get_funding_txid().expect("Funding txid must be known for pending scope"); let msg = messages.get(&funding_txid).ok_or_else(|| { @@ -9548,11 +9734,29 @@ where funding.get_funding_txo().expect("funding outpoint should be set"); let channel_type = funding.get_channel_type().clone(); let funding_redeem_script = funding.get_funding_redeemscript(); + let has_local_contribution = self + .context + .interactive_tx_signing_session + .as_ref() + .map(|signing_session| signing_session.has_local_contribution()) + .unwrap_or(false); - pending_splice.negotiated_candidates.push(funding); + let contribution = pending_splice.negotiation_contribution.take(); + debug_assert!( + contribution.is_some() + || pending_splice + .negotiated_candidates + .iter() + .all(|candidate| candidate.contribution.is_none()), + "a round following one we contributed to must carry our contribution", + ); + pending_splice + .negotiated_candidates + .push(NegotiatedCandidate { funding, contribution }); let splice_negotiated = SpliceFundingNegotiated { funding_txo: funding_txo.into_bitcoin_outpoint(), + has_local_contribution, channel_type, funding_redeem_script, }; @@ -9572,29 +9776,21 @@ where ); } - let contrib_offset = pending_splice - .negotiated_candidates - .len() - .saturating_sub(pending_splice.contributions.len()); let candidates = pending_splice .negotiated_candidates .iter() - .enumerate() - .map(|(i, funding)| { - let txid = funding + .map(|candidate| { + let txid = candidate + .funding .get_funding_txid() .expect("negotiated candidates should have a funding txid"); - let contribution = i - .checked_sub(contrib_offset) - .and_then(|j| pending_splice.contributions.get(j)) - .cloned(); FundingCandidate { txid, channels: vec![ChannelFunding { counterparty_node_id: self.context.counterparty_node_id, channel_id: self.context.channel_id, purpose: FundingPurpose::Splice, - contribution, + contribution: candidate.contribution.clone(), }], } }) @@ -9755,7 +9951,7 @@ where debug_assert!(!self.funding.get_channel_type().supports_anchor_zero_fee_commitments()); let can_send_update_fee = core::iter::once(&self.funding) - .chain(self.pending_funding().iter()) + .chain(self.pending_funding()) .all(|funding| self.context.can_send_update_fee(funding, feerate_per_kw, fee_estimator, logger)); if !can_send_update_fee { return None; @@ -10106,7 +10302,7 @@ where } core::iter::once(&self.funding) - .chain(self.pending_funding().iter()) + .chain(self.pending_funding()) .try_for_each(|funding| FundedChannel::::check_remote_fee(funding.get_channel_type(), fee_estimator, msg.feerate_per_kw, Some(self.context.feerate_per_kw), logger))?; self.context.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced)); @@ -10818,7 +11014,6 @@ where // for this `txid`. let inferred_splice_locked = msg.my_current_funding_locked.as_ref().and_then(|funding_locked| { self.pending_funding() - .iter() .find(|funding| funding.get_funding_txid() == Some(funding_locked.txid)) .and_then(|_| { self.pending_splice.as_ref().and_then(|pending_splice| { @@ -11615,7 +11810,7 @@ where ); core::iter::once(&self.funding) - .chain(self.pending_funding().iter()) + .chain(self.pending_funding()) .try_for_each(|funding| self.context.can_accept_incoming_htlc(funding, dust_exposure_limiting_feerate, &logger)) } @@ -11933,6 +12128,7 @@ where let funding = pending_splice .negotiated_candidates .iter_mut() + .map(|candidate| &mut candidate.funding) .find(|funding| funding.get_funding_txid() == Some(splice_txid)) .unwrap(); @@ -11947,9 +12143,12 @@ where .funding_transaction .as_ref() .expect("Promoted splice funding should have a funding transaction"); - let contributions = core::mem::take(&mut pending_splice.contributions); - contributions + let candidates = core::mem::take(&mut pending_splice.negotiated_candidates); + let negotiation_contribution = pending_splice.negotiation_contribution.take(); + candidates .into_iter() + .filter_map(|candidate| candidate.contribution) + .chain(negotiation_contribution) .filter_map(|contribution| { contribution.into_unique_contributions( promoted_tx.input.iter().map(|i| i.previous_output), @@ -12038,7 +12237,9 @@ where let mut confirmed_funding_index = None; let mut funding_already_confirmed = false; - for (index, funding) in pending_splice.negotiated_candidates.iter_mut().enumerate() { + let candidates = + pending_splice.negotiated_candidates.iter_mut().map(|candidate| &mut candidate.funding); + for (index, funding) in candidates.enumerate() { if self.context.check_for_funding_tx_confirmed( funding, block_hash, height, index_in_block, &mut confirmed_tx, logger, )? { @@ -12198,7 +12399,8 @@ where if let Some(pending_splice) = &mut self.pending_splice { let mut confirmed_funding_index = None; - for (index, funding) in pending_splice.negotiated_candidates.iter().enumerate() { + let candidates = pending_splice.negotiated_candidates.iter().map(|candidate| &candidate.funding); + for (index, funding) in candidates.enumerate() { if funding.funding_tx_confirmation_height != 0 { if confirmed_funding_index.is_some() { let err_reason = "splice tx of another pending funding already confirmed"; @@ -12210,7 +12412,8 @@ where } if let Some(confirmed_funding_index) = confirmed_funding_index { - let funding = &mut pending_splice.negotiated_candidates[confirmed_funding_index]; + let funding = + &mut pending_splice.negotiated_candidates[confirmed_funding_index].funding; // Check if the splice funding transaction was unconfirmed if funding.get_funding_tx_confirmations(height) == 0 { @@ -12266,7 +12469,7 @@ where pub fn get_relevant_txids(&self) -> impl Iterator)> + '_ { core::iter::once(&self.funding) - .chain(self.pending_funding().iter()) + .chain(self.pending_funding()) .map(|funding| { ( funding.get_funding_txid(), @@ -12710,21 +12913,23 @@ where .as_ref() .map(|n| n.funding_feerate_sat_per_1000_weight()) }); - debug_assert!( - prev_feerate.is_some(), - "pending_splice should have last_funding_feerate or funding_negotiation", - ); - let min_rbf_feerate = prev_feerate.map(min_rbf_feerate); + let prev_feerate = match prev_feerate { + Some(prev_feerate) => prev_feerate, + None => { + // The feerate and our contribution are only persisted by LDK 0.3+, so their + // absence means this splice was last written by an older version (negotiated + // there, or round-tripped 0.3 -> 0.2 -> 0.3) and cannot be RBF'd. + return Err(APIError::APIMisuseError { + err: format!( + "Channel {} has a pending splice from a prior LDK version and cannot be spliced again", + self.context.channel_id(), + ), + }); + }, + }; + let min_rbf_feerate = Some(min_rbf_feerate(prev_feerate)); let prior = if pending_splice.last_funding_feerate_sat_per_1000_weight.is_some() { - if let Some(prior) = self - .pending_splice - .as_ref() - .and_then(|pending_splice| pending_splice.contributions.last()) - { - Some(prior.clone()) - } else { - None - } + pending_splice.latest_contribution().cloned() } else { None }; @@ -12968,6 +13173,18 @@ where contribution }; + // A queued splice never coexists with a negotiation we initiated: we return early above if + // one is already in flight, and a queued action is cleared the moment it becomes our + // negotiation at quiescence. It may coexist with a counterparty-initiated negotiation (e.g. + // queuing our own contribution while accepting their splice), so we only rule out our own. + debug_assert!( + self.pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) + .map_or(true, |funding_negotiation| !funding_negotiation.is_initiator()), + "A queued splice must not coexist with a funding negotiation we initiated", + ); + self.propose_quiescence(logger, QuiescentAction::Splice { contribution, locktime }) } @@ -13014,11 +13231,11 @@ where FundingNegotiation::AwaitingAck { context, new_holder_funding_key: funding_pubkey }; self.pending_splice = Some(PendingFunding { funding_negotiation: Some(funding_negotiation), + negotiation_contribution: None, negotiated_candidates: vec![], sent_funding_txid: None, received_funding_txid: None, last_funding_feerate_sat_per_1000_weight: None, - contributions: vec![], }); msgs::SpliceInit { @@ -13040,6 +13257,7 @@ where .negotiated_candidates .first() .unwrap() + .funding .get_holder_pubkeys() .funding_pubkey; @@ -13151,11 +13369,13 @@ where /// Checks during handling splice_init pub fn validate_splice_init(&self, msg: &msgs::SpliceInit) -> Result<(), ChannelError> { - if self.holder_commitment_point.current_point().is_none() { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} commitment point needs to be advanced once before spliced", - self.context.channel_id(), - ))); + // - If it has received shutdown: + // MUST send a warning and close the connection or send an error + // and fail the channel. + if !self.context.is_live() { + return Err(ChannelError::WarnAndDisconnect( + "Splicing requested on a channel that is not live".to_owned(), + )); } if !self.context.channel_state.is_quiescent() { @@ -13170,15 +13390,6 @@ where ))); } - // - If it has received shutdown: - // MUST send a warning and close the connection or send an error - // and fail the channel. - if !self.context.is_live() { - return Err(ChannelError::WarnAndDisconnect( - "Splicing requested on a channel that is not live".to_owned(), - )); - } - let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis); if their_funding_contribution == SignedAmount::ZERO { return Err(ChannelError::WarnAndDisconnect(format!( @@ -13187,6 +13398,12 @@ where ))); } + if self.holder_commitment_point.current_point().is_none() { + return Err(ChannelError::Abort(AbortReason::InternalError( + "Commitment point needs to be advanced once before spliced".into(), + ))); + } + Ok(()) } @@ -13203,13 +13420,10 @@ where counterparty_funding_pubkey, our_new_holder_keys, min_funding_satoshis, - ) - .map_err(|e| format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e))?; + )?; let (post_splice_holder_balance, post_splice_counterparty_balance) = - self.get_holder_counterparty_balances_floor_incl_fee(&candidate_scope).map_err( - |e| format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e), - )?; + self.get_holder_counterparty_balances_floor_incl_fee(&candidate_scope)?; let holder_selected_channel_reserve = Amount::from_sat(candidate_scope.holder_selected_channel_reserve_satoshis); @@ -13219,25 +13433,23 @@ where // We allow parties to draw from their previous reserve, as long as they satisfy their v2 reserve if our_funding_contribution != SignedAmount::ZERO { - post_splice_holder_balance.checked_sub(counterparty_selected_channel_reserve) - .ok_or(format!( - "Channel {} cannot be {}; our post-splice channel balance {} is smaller than their selected v2 reserve {}", - self.context.channel_id(), - if our_funding_contribution.is_positive() { "spliced in" } else { "spliced out" }, - post_splice_holder_balance, - counterparty_selected_channel_reserve, - ))?; + post_splice_holder_balance.checked_sub(counterparty_selected_channel_reserve).ok_or( + format!( + "Our post-splice channel balance {} is smaller than their selected v2 reserve {}", + post_splice_holder_balance, + counterparty_selected_channel_reserve, + ), + )?; } if their_funding_contribution != SignedAmount::ZERO { - post_splice_counterparty_balance.checked_sub(holder_selected_channel_reserve) - .ok_or(format!( - "Channel {} cannot be {}; their post-splice channel balance {} is smaller than our selected v2 reserve {}", - self.context.channel_id(), - if their_funding_contribution.is_positive() { "spliced in" } else { "spliced out" }, - post_splice_counterparty_balance, - holder_selected_channel_reserve, - ))?; + post_splice_counterparty_balance.checked_sub(holder_selected_channel_reserve).ok_or( + format!( + "Their post-splice channel balance {} is smaller than our selected v2 reserve {}", + post_splice_counterparty_balance, + holder_selected_channel_reserve, + ), + )?; } #[cfg(debug_assertions)] @@ -13348,7 +13560,11 @@ where holder_pubkeys, min_funding_satoshis, ) - .map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?; + .map_err(|e| { + self.quiescent_negotiation_err(ChannelError::Abort( + AbortReason::InvalidContribution(e), + )) + })?; // Adjust for the feerate and clone so we can store it for future RBF re-use. let (adjusted_contribution, our_funding_inputs, our_funding_outputs) = @@ -13388,11 +13604,11 @@ where ); self.pending_splice = Some(PendingFunding { funding_negotiation: Some(funding_negotiation), + negotiation_contribution: adjusted_contribution, negotiated_candidates: Vec::new(), received_funding_txid: None, sent_funding_txid: None, last_funding_feerate_sat_per_1000_weight: None, - contributions: adjusted_contribution.into_iter().collect(), }); Ok(msgs::SpliceAck { @@ -13407,57 +13623,56 @@ where fn validate_tx_init_rbf( &self, msg: &msgs::TxInitRbf, fee_estimator: &LowerBoundedFeeEstimator, ) -> Result<(ChannelPublicKeys, PublicKey), ChannelError> { - if self.holder_commitment_point.current_point().is_none() { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} commitment point needs to be advanced once before RBF", - self.context.channel_id(), - ))); + if !self.context.is_live() { + return Err(ChannelError::WarnAndDisconnect( + "RBF requested on a channel that is not live".to_owned(), + )); } - if !self.context.channel_state.is_quiescent() { return Err(ChannelError::WarnAndDisconnect("Quiescence needed for RBF".to_owned())); } - self.is_rbf_compatible().map_err(|msg| ChannelError::WarnAndDisconnect(msg))?; + if self.holder_commitment_point.current_point().is_none() { + return Err(ChannelError::Abort(AbortReason::InternalError( + "Commitment point needs to be advanced once before RBF".into(), + ))); + } - let pending_splice = match &self.pending_splice { - Some(pending_splice) => pending_splice, - None => { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} has no pending splice to RBF", - self.context.channel_id(), - ))); - }, - }; + self.is_rbf_compatible() + .map_err(|msg| ChannelError::Abort(AbortReason::RbfUnavailable(msg)))?; + + let (pending_splice, last_candidate) = self + .pending_splice + .as_ref() + .filter(|pending_splice| !pending_splice.negotiated_candidates.is_empty()) + .map(|pending_splice| { + ( + pending_splice, + pending_splice.negotiated_candidates.last().expect("checked above"), + ) + }) + .ok_or_else(|| { + ChannelError::Abort(AbortReason::RbfUnavailable( + "No pending splice available to RBF".into(), + )) + })?; if pending_splice.funding_negotiation.is_some() { return Err(ChannelError::Abort(AbortReason::NegotiationInProgress)); } if pending_splice.received_funding_txid.is_some() { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} counterparty already sent splice_locked, cannot RBF", - self.context.channel_id(), + return Err(ChannelError::Abort(AbortReason::RbfUnavailable( + "Already received splice_locked".into(), ))); } if pending_splice.sent_funding_txid.is_some() { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} already sent splice_locked, cannot RBF", - self.context.channel_id(), + return Err(ChannelError::Abort(AbortReason::RbfUnavailable( + "Already sent splice_locked".into(), ))); } - let last_candidate = match pending_splice.negotiated_candidates.last() { - Some(candidate) => candidate, - None => { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} has no negotiated splice candidates to RBF", - self.context.channel_id(), - ))); - }, - }; - let prev_feerate = pending_splice.last_funding_feerate_sat_per_1000_weight.unwrap_or_else(|| { fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::UrgentOnChainSweep) @@ -13475,8 +13690,8 @@ where // Reuse funding pubkeys from the last negotiated candidate since all RBF candidates // for the same splice share the same funding output script. Ok(( - last_candidate.get_holder_pubkeys().clone(), - *last_candidate.counterparty_funding_pubkey(), + last_candidate.funding.get_holder_pubkeys().clone(), + *last_candidate.funding.counterparty_funding_pubkey(), )) } @@ -13500,7 +13715,7 @@ where } else if let Some(prior) = self .pending_splice .as_ref() - .and_then(|pending_splice| pending_splice.contributions.last()) + .and_then(|pending_splice| pending_splice.latest_contribution()) { let net_value = holder_balance .ok_or_else(|| ChannelError::Abort(AbortReason::InsufficientRbfFeerate)) @@ -13531,7 +13746,11 @@ where holder_pubkeys, min_funding_satoshis, ) - .map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?; + .map_err(|e| { + self.quiescent_negotiation_err(ChannelError::Abort( + AbortReason::InvalidContribution(e), + )) + })?; // Consume the appropriate contribution source. let (our_funding_inputs, our_funding_outputs) = if queued_net_value.is_some() { @@ -13543,16 +13762,14 @@ where self.pending_splice .as_mut() .expect("pending_splice is Some") - .contributions - .push(adjusted_contribution.clone()); + .negotiation_contribution = Some(adjusted_contribution.clone()); adjusted_contribution.into_tx_parts() } else if prior_net_value.is_some() { let prior_contribution = self .pending_splice .as_ref() .expect("pending_splice is Some") - .contributions - .last() + .latest_contribution() .expect("prior_net_value was Some") .clone(); let adjusted_contribution = prior_contribution @@ -13561,8 +13778,7 @@ where self.pending_splice .as_mut() .expect("pending_splice is Some") - .contributions - .push(adjusted_contribution.clone()); + .negotiation_contribution = Some(adjusted_contribution.clone()); adjusted_contribution.into_tx_parts() } else { Default::default() @@ -13618,10 +13834,12 @@ where }; let last_candidate = pending_splice.negotiated_candidates.last().ok_or_else(|| { - ChannelError::WarnAndDisconnect("No negotiated splice candidates for RBF".to_owned()) + ChannelError::Abort(AbortReason::RbfUnavailable( + "No pending splice available to RBF".into(), + )) })?; - let holder_pubkeys = last_candidate.get_holder_pubkeys().clone(); - let counterparty_funding_pubkey = *last_candidate.counterparty_funding_pubkey(); + let holder_pubkeys = last_candidate.funding.get_holder_pubkeys().clone(); + let counterparty_funding_pubkey = *last_candidate.funding.counterparty_funding_pubkey(); let new_funding = self .validate_splice_contributions( @@ -13631,7 +13849,7 @@ where holder_pubkeys, min_funding_satoshis, ) - .map_err(|e| ChannelError::WarnAndDisconnect(e))?; + .map_err(|e| ChannelError::Abort(AbortReason::InvalidContribution(e)))?; Ok(new_funding) } @@ -13708,8 +13926,6 @@ where fn validate_splice_ack( &self, msg: &msgs::SpliceAck, min_funding_satoshis: u64, ) -> Result { - // TODO(splicing): Add check that we are the splice (quiescence) initiator - let pending_splice = self .pending_splice .as_ref() @@ -13732,7 +13948,7 @@ where new_keys, min_funding_satoshis, ) - .map_err(|e| ChannelError::WarnAndDisconnect(e))?; + .map_err(|e| ChannelError::Abort(AbortReason::InvalidContribution(e)))?; Ok(new_funding) } @@ -13886,7 +14102,7 @@ where if !pending_splice .negotiated_candidates .iter() - .any(|funding| funding.get_funding_txid() == Some(msg.splice_txid)) + .any(|candidate| candidate.funding.get_funding_txid() == Some(msg.splice_txid)) { let err = "unknown splice funding txid"; return Err(ChannelError::close(err.to_string())); @@ -14084,7 +14300,7 @@ where &self, fee_estimator: &LowerBoundedFeeEstimator, ) -> Result { let init = self.context.get_available_balances_for_scope(&self.funding, fee_estimator)?; - self.pending_funding().iter().try_fold(init, |acc, funding| { + self.pending_funding().try_fold(init, |acc, funding| { let e = self.context.get_available_balances_for_scope(funding, fee_estimator)?; Ok(AvailableBalances { inbound_capacity_msat: acc.inbound_capacity_msat.min(e.inbound_capacity_msat), @@ -14146,7 +14362,7 @@ where } self.context.resend_order = RAACommitmentOrder::RevokeAndACKFirst; - let update = if self.pending_funding().is_empty() { + let update = if self.negotiated_candidates().is_empty() { let (htlcs_ref, counterparty_commitment_tx) = self.build_commitment_no_state_update(&self.funding, logger); let htlc_outputs = htlcs_ref @@ -14177,7 +14393,7 @@ where } else { let mut htlc_data = None; let commitment_txs = core::iter::once(&self.funding) - .chain(self.pending_funding().iter()) + .chain(self.pending_funding()) .map(|funding| { let (htlcs_ref, counterparty_commitment_tx) = self.build_commitment_no_state_update(funding, logger); @@ -14239,7 +14455,7 @@ where &self, logger: &L, ) -> Result, ChannelError> { core::iter::once(&self.funding) - .chain(self.pending_funding().iter()) + .chain(self.pending_funding()) .map(|funding| self.send_commitment_no_state_update_for_funding(funding, logger)) .collect::, ChannelError>>() } @@ -14710,14 +14926,14 @@ where } let tx_init_rbf = self.send_tx_init_rbf(context); self.pending_splice.as_mut().unwrap() - .contributions.push(prior_contribution); + .negotiation_contribution = Some(prior_contribution); return Ok(Some(StfuResponse::TxInitRbf(tx_init_rbf))); } let splice_init = self.send_splice_init(context); debug_assert!(self.pending_splice.is_some()); self.pending_splice.as_mut().unwrap() - .contributions.push(prior_contribution); + .negotiation_contribution = Some(prior_contribution); return Ok(Some(StfuResponse::SpliceInit(splice_init))); }, #[cfg(any(test, fuzzing, feature = "_test_utils"))] @@ -16380,7 +16596,7 @@ impl Writeable for FundedChannel { // resumed on reestablishment, but keep any already-negotiated candidates. let reset_funding_negotiation = self.should_reset_pending_splice_state(true); let should_persist_pending_splice = - !reset_funding_negotiation || !self.pending_funding().is_empty(); + !reset_funding_negotiation || !self.negotiated_candidates().is_empty(); let pending_splice = should_persist_pending_splice .then(|| ()) .and_then(|_| self.pending_splice.as_ref()) diff --git a/lightning/src/ln/channel_state.rs b/lightning/src/ln/channel_state.rs index 6e5d633e920..d69d0ce901a 100644 --- a/lightning/src/ln/channel_state.rs +++ b/lightning/src/ln/channel_state.rs @@ -12,10 +12,12 @@ use alloc::vec::Vec; use bitcoin::secp256k1::PublicKey; +use bitcoin::Txid; use crate::chain::chaininterface::{FeeEstimator, LowerBoundedFeeEstimator}; use crate::chain::transaction::OutPoint; use crate::ln::channel::Channel; +use crate::ln::funding::FundingContribution; use crate::ln::types::ChannelId; use crate::sign::SignerProvider; use crate::types::features::{ChannelTypeFeatures, InitFeatures}; @@ -494,6 +496,14 @@ pub struct ChannelDetails { /// /// [`ChannelConfig::max_dust_htlc_exposure`]: crate::util::config::ChannelConfig::max_dust_htlc_exposure pub current_dust_exposure_msat: Option, + /// Details of any pending splice attempts on this channel. + /// + /// This includes a contribution we have committed but not yet begun negotiating, any splice + /// currently under negotiation with our counterparty, and any negotiated splice or RBF attempts + /// waiting on sufficient on-chain confirmations. This will be `None` if no splice is pending. + /// + /// This field will be `None` for objects serialized with LDK versions prior to 0.3. + pub splice_details: Option, } impl ChannelDetails { @@ -619,6 +629,9 @@ impl ChannelDetails { pending_inbound_htlcs: context.get_pending_inbound_htlc_details(funding), pending_outbound_htlcs: context.get_pending_outbound_htlc_details(funding), current_dust_exposure_msat: Some(balance.dust_exposure_msat), + splice_details: channel + .as_funded() + .and_then(|chan| chan.pending_splice_details(best_block_height)), } } } @@ -661,11 +674,180 @@ impl_ser_tlv_based!(ChannelDetails, { (45, pending_outbound_htlcs, optional_vec), (47, funding_redeem_script, option), (49, current_dust_exposure_msat, option), + (51, splice_details, option), (_unused, user_channel_id, (static_value, _user_channel_id_low.unwrap_or(0) as u128 | ((_user_channel_id_high.unwrap_or(0) as u128) << 64) )), }); +/// Details of pending splice attempts on a channel, as returned in +/// [`ChannelDetails::splice_details`]. +/// +/// This includes a contribution we have committed but not yet begun negotiating, any splice +/// currently under negotiation with the counterparty, and any negotiated splice or RBF attempts +/// waiting on sufficient on-chain confirmations. +#[derive(Clone, Debug, PartialEq)] +pub struct SpliceDetails { + /// A splice or RBF attempt currently being negotiated with the counterparty, if any. + /// + /// Note that a negotiation which has not yet reached + /// [`SpliceNegotiationStatus::AwaitingSignatures`] does not survive a restart, so this only + /// reflects in-memory negotiation state. + pub negotiation: Option, + /// Our contribution to a splice we have committed to via + /// [`ChannelManager::funding_contributed`] but which has not yet begun negotiating, as it is + /// awaiting quiescence with the counterparty, if any. + /// + /// Whether we end up the initiator of the resulting negotiation is not settled until quiescence + /// is reached (the counterparty may have its own splice in flight, as in + /// [`negotiation`]), so this only reflects that a contribution is queued. Like the in-flight + /// negotiation states, it does not survive a restart. + /// + /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed + /// [`negotiation`]: Self::negotiation + pub queued_contribution: Option, + /// Negotiated splice transactions that have not yet reached sufficient confirmations by both + /// counterparties to have exchanged `splice_locked`, in order: the original negotiation + /// followed by any RBF replacements. + /// + /// More than one entry indicates the use of RBF; at most one of these candidates will + /// ultimately confirm. + pub candidates: Vec, + /// The negotiated candidate that has confirmed on-chain, if any, along with its confirmation + /// progress. + /// + /// At most one candidate can confirm, as the candidates all double-spend the same input, so + /// this identifies the single confirming candidate rather than tracking confirmations on each. + pub confirmed_candidate: Option, + /// The txid announced in the `splice_locked` received from the counterparty, i.e., the + /// candidate that they consider to have sufficient confirmations. + /// + /// Unlike the `splice_locked` we sent (see [`ConfirmedSpliceCandidate::splice_locked_sent`]), + /// this need not match [`confirmed_candidate`]: during a reorg, our counterparty may observe a + /// different candidate confirm. + /// + /// [`confirmed_candidate`]: Self::confirmed_candidate + pub received_splice_locked_txid: Option, +} + +impl_ser_tlv_based!(SpliceDetails, { + (1, negotiation, option), + (3, queued_contribution, option), + (5, candidates, required_vec), + (7, confirmed_candidate, option), + (9, received_splice_locked_txid, option), +}); + +/// Details of a splice or RBF attempt currently being negotiated with the counterparty. +#[derive(Clone, Debug, PartialEq)] +pub struct SpliceNegotiationDetails { + /// How far the negotiation has progressed. + pub status: SpliceNegotiationStatus, + /// Whether we initiated the negotiation. + pub is_initiator: bool, + /// The feerate of the splice transaction under negotiation, denominated in satoshi per 1000 + /// weight units. + pub funding_feerate_sat_per_1000_weight: u32, + /// The value, in satoshis, of the channel once the splice transaction under negotiation + /// confirms and is promoted. + /// + /// This will be `None` while [`SpliceNegotiationStatus::AwaitingAck`], as the value is not + /// known until both counterparties' contributions have been exchanged. + pub new_channel_value_satoshis: Option, + /// The txid of the splice transaction under negotiation. + /// + /// This will be `None` until [`SpliceNegotiationStatus::AwaitingSignatures`], as the txid is + /// not known until the transaction has been fully constructed. + pub txid: Option, + /// Our contribution to the splice under negotiation, or `None` if we are not contributing. + /// + /// Note that for a counterparty-initiated RBF attempt, this is the prior round's contribution + /// adjusted to the new feerate. + pub contribution: Option, +} + +impl_ser_tlv_based!(SpliceNegotiationDetails, { + (1, status, required), + (3, is_initiator, required), + (5, funding_feerate_sat_per_1000_weight, required), + (7, new_channel_value_satoshis, option), + (9, txid, option), + (11, contribution, option), +}); + +/// The status of a splice or RBF negotiation in progress with the counterparty. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SpliceNegotiationStatus { + /// We sent `splice_init` or `tx_init_rbf` and are awaiting the counterparty's acknowledgement. + AwaitingAck, + /// The splice transaction is being interactively constructed. + ConstructingTransaction, + /// The splice transaction has been negotiated and is awaiting signatures from both + /// counterparties. + AwaitingSignatures, +} + +impl_ser_tlv_based_enum!(SpliceNegotiationStatus, + (0, AwaitingAck) => {}, + (2, ConstructingTransaction) => {}, + (4, AwaitingSignatures) => {}, +); + +/// Details of a negotiated splice transaction that has not yet reached sufficient confirmations +/// by both counterparties to have exchanged `splice_locked`. +#[derive(Clone, Debug, PartialEq)] +pub struct SpliceCandidateDetails { + /// The txid of the splice transaction. + pub txid: Txid, + /// The value, in satoshis, of the channel once this candidate confirms and is promoted. + pub new_channel_value_satoshis: u64, + /// Our contribution to this candidate, or `None` if we did not contribute. + /// + /// Once a candidate includes our contribution, every later candidate does as well: RBF + /// attempts carry the contribution forward (possibly adjusted to a new feerate) rather than + /// dropping it, preserving the splice intention. + /// + /// Note that [`FundingContribution::feerate`] is the feerate used when selecting the + /// contribution's inputs, which is not necessarily the exact feerate of the negotiated + /// transaction. + pub contribution: Option, +} + +impl_ser_tlv_based!(SpliceCandidateDetails, { + (1, txid, required), + (3, new_channel_value_satoshis, required), + (5, contribution, option), +}); + +/// The confirmation progress of the negotiated splice candidate that has confirmed on-chain, as +/// exposed in [`SpliceDetails::confirmed_candidate`]. +/// +/// At most one candidate can confirm, as the candidates all double-spend the same input, so this +/// identifies the single confirming candidate by its txid. +#[derive(Clone, Debug, PartialEq)] +pub struct ConfirmedSpliceCandidate { + /// The txid of the candidate that has confirmed on-chain. This matches the [`txid`] of one of + /// the [`SpliceDetails::candidates`]. + /// + /// [`txid`]: SpliceCandidateDetails::txid + pub txid: Txid, + /// The current number of confirmations of the candidate's transaction. + pub confirmations: u32, + /// The number of confirmations required before `splice_locked` can be sent for the candidate. + pub confirmations_required: u32, + /// Whether we have sent `splice_locked` for this candidate, i.e., we consider it to have + /// sufficient confirmations. The `splice_locked` we sent always refers to this confirmed + /// candidate, so it is tracked here rather than as a separate txid. + pub splice_locked_sent: bool, +} + +impl_ser_tlv_based!(ConfirmedSpliceCandidate, { + (1, txid, required), + (3, confirmations, required), + (5, confirmations_required, required), + (7, splice_locked_sent, required), +}); + #[derive(Clone, Copy, Debug, PartialEq, Eq)] /// Further information on the details of the channel shutdown. /// Upon channels being forced closed (i.e. commitment transaction confirmation detected @@ -718,7 +900,10 @@ mod tests { }, }; - use super::{ChannelCounterparty, ChannelDetails, ChannelShutdownState}; + use super::{ + ChannelCounterparty, ChannelDetails, ChannelShutdownState, ConfirmedSpliceCandidate, + SpliceCandidateDetails, SpliceDetails, SpliceNegotiationDetails, SpliceNegotiationStatus, + }; #[test] fn test_channel_details_serialization() { @@ -783,6 +968,29 @@ mod tests { is_dust: false, }], current_dust_exposure_msat: Some(150_000), + splice_details: Some(SpliceDetails { + negotiation: Some(SpliceNegotiationDetails { + status: SpliceNegotiationStatus::ConstructingTransaction, + is_initiator: true, + funding_feerate_sat_per_1000_weight: 1000, + new_channel_value_satoshis: Some(70_000), + txid: None, + contribution: None, + }), + queued_contribution: None, + candidates: vec![SpliceCandidateDetails { + txid: bitcoin::Txid::from_slice(&[7; 32]).unwrap(), + new_channel_value_satoshis: 60_000, + contribution: None, + }], + confirmed_candidate: Some(ConfirmedSpliceCandidate { + txid: bitcoin::Txid::from_slice(&[7; 32]).unwrap(), + confirmations: 3, + confirmations_required: 6, + splice_locked_sent: false, + }), + received_splice_locked_txid: Some(bitcoin::Txid::from_slice(&[7; 32]).unwrap()), + }), }; let mut buffer = Vec::new(); channel_details.write(&mut buffer).unwrap(); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 49392264709..0fb13938f99 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1147,7 +1147,7 @@ impl MsgHandleErrInternal { fn from_chan_no_close(err: ChannelError, channel_id: ChannelId) -> Self { let tx_abort = match &err { - &ChannelError::Abort(reason) => Some(reason.into_tx_abort_msg(channel_id)), + ChannelError::Abort(reason) => Some(reason.clone().into_tx_abort_msg(channel_id)), _ => None, }; let err = match err { @@ -6781,8 +6781,9 @@ impl< /// /// Calling this method will commence the process of creating a new funding transaction for the /// channel. Once the funding transaction has been constructed, an [`Event::SpliceNegotiated`] - /// will be emitted. At this point, any inputs contributed to the splice can only be re-spent - /// if an [`Event::DiscardFunding`] is seen. + /// will be emitted if the negotiated transaction includes local inputs or outputs. At this + /// point, any inputs contributed to the splice can only be re-spent if an + /// [`Event::DiscardFunding`] is seen. /// /// If any failures occur while negotiating the funding transaction, an /// [`Event::SpliceNegotiationFailed`] will be emitted. Any contributed inputs no longer used @@ -7005,18 +7006,20 @@ impl< ); } if let Some(splice_negotiated) = splice_negotiated { - self.pending_events.lock().unwrap().push_back(( - events::Event::SpliceNegotiated { - channel_id: *channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: chan.context().get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated - .funding_redeem_script, - }, - None, - )); + if splice_negotiated.has_local_contribution { + self.pending_events.lock().unwrap().push_back(( + events::Event::SpliceNegotiated { + channel_id: *channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id: chan.context().get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated + .funding_redeem_script, + }, + None, + )); + } } if chan.context().is_connected() { @@ -11199,17 +11202,19 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ .as_mut() .and_then(|v| v.splice_negotiated.take()) { - pending_events.push_back(( - events::Event::SpliceNegotiated { - channel_id: channel.context.channel_id(), - counterparty_node_id, - user_channel_id: channel.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated.funding_redeem_script, - }, - None, - )); + if splice_negotiated.has_local_contribution { + pending_events.push_back(( + events::Event::SpliceNegotiated { + channel_id: channel.context.channel_id(), + counterparty_node_id, + user_channel_id: channel.context.get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated.funding_redeem_script, + }, + None, + )); + } } } @@ -12299,18 +12304,20 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ // which also terminates quiescence. let needs_holding_cell_release = splice_negotiated.is_some(); if let Some(splice_negotiated) = splice_negotiated { - self.pending_events.lock().unwrap().push_back(( - events::Event::SpliceNegotiated { - channel_id: msg.channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: chan.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated - .funding_redeem_script, - }, - None, - )); + if splice_negotiated.has_local_contribution { + self.pending_events.lock().unwrap().push_back(( + events::Event::SpliceNegotiated { + channel_id: msg.channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id: chan.context.get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated + .funding_redeem_script, + }, + None, + )); + } } let holding_cell_res = if needs_holding_cell_release { self.check_free_peer_holding_cells(peer_state) @@ -14161,17 +14168,20 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ .and_then(|funding_tx_signed| funding_tx_signed.splice_negotiated.take()) { *needs_holding_cell_release = true; - self.pending_events.lock().unwrap().push_back(( - events::Event::SpliceNegotiated { - channel_id, - counterparty_node_id: node_id, - user_channel_id: funded_chan.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated.funding_redeem_script, - }, - None, - )); + if splice_negotiated.has_local_contribution { + self.pending_events.lock().unwrap().push_back(( + events::Event::SpliceNegotiated { + channel_id, + counterparty_node_id: node_id, + user_channel_id: funded_chan.context.get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated + .funding_redeem_script, + }, + None, + )); + } } if let Some(broadcast_tx) = msgs.signed_closing_tx { log_info!(logger, "Broadcasting closing tx {}", log_tx!(broadcast_tx)); diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index dfb702a2657..6769e2de3e5 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -91,7 +91,7 @@ impl SerialIdExt for SerialId { } } -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub(crate) enum AbortReason { InvalidStateTransition, UnexpectedCounterpartyMessage, @@ -142,6 +142,10 @@ pub(crate) enum AbortReason { /// /// [`ChannelManager::cancel_funding_contributed`]: crate::ln::channelmanager::ChannelManager::cancel_funding_contributed ManualIntervention, + /// The contribution is not valid given the current balances of the channel. + InvalidContribution(String), + /// A RBF is not available at this time. + RbfUnavailable(String), /// Internal error InternalError(&'static str), } @@ -209,6 +213,12 @@ impl Display for AbortReason { f.write_str("The initiator's feerate exceeds our maximum") }, AbortReason::ManualIntervention => f.write_str("Manually aborted funding negotiation"), + AbortReason::InvalidContribution(text) => { + f.write_fmt(format_args!("Invalid contribution: {}", text)) + }, + AbortReason::RbfUnavailable(text) => { + f.write_fmt(format_args!("Rejecting RBF attempt: {}", text)) + }, AbortReason::InternalError(text) => { f.write_fmt(format_args!("Internal error: {}", text)) }, 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. diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index f480c4e9bc0..4d7e5173261 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -22,6 +22,7 @@ use crate::ln::channel::{ DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_CHANNEL_VALUE_SATOSHIS, }; +use crate::ln::channel_state::{SpliceDetails, SpliceNegotiationStatus}; use crate::ln::channelmanager::{provided_init_features, PaymentId, BREAKDOWN_TIMEOUT}; use crate::ln::functional_test_utils::*; use crate::ln::funding::{FundingContribution, FundingContributionError, FundingTemplate}; @@ -174,23 +175,21 @@ fn config_with_min_funding_satoshis(min_funding_satoshis: u64) -> UserConfig { } #[cfg(test)] -fn assert_min_funding_error<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, min_funding_satoshis: u64) { - let msg_events = node.node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1, "{msg_events:?}"); - match &msg_events[0] { - MessageSendEvent::HandleError { - action: msgs::ErrorAction::DisconnectPeerWithWarning { msg }, - .. - } => { - assert!( - msg.data - .contains(&format!("configured min_funding_satoshis {min_funding_satoshis}")), - "unexpected warning: {}", - msg.data - ); - }, - _ => panic!("Expected HandleError with warning, got {:?}", msg_events[0]), - } +fn assert_min_funding_error<'a, 'b, 'c>( + node: &Node<'a, 'b, 'c>, recipient: PublicKey, min_funding_satoshis: u64, +) { + let msg = get_event_msg!(node, MessageSendEvent::SendTxAbort, recipient); + let data = tx_abort_data(&msg); + assert!( + data.contains(&format!("configured min_funding_satoshis {min_funding_satoshis}")), + "unexpected tx_abort: {}", + data + ); +} + +#[cfg(test)] +fn tx_abort_data(msg: &msgs::TxAbort) -> String { + String::from_utf8(msg.data.clone()).expect("tx_abort data should be valid UTF-8") } pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>( @@ -702,7 +701,6 @@ pub fn splice_channel<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, funding_contribution: FundingContribution, ) -> (Transaction, ScriptBuf) { - let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); let new_funding_script = complete_splice_handshake(initiator, acceptor); @@ -718,7 +716,7 @@ pub fn splice_channel<'a, 'b, 'c, 'd>( assert!(splice_locked.is_none()); expect_splice_pending_event(initiator, &node_id_acceptor); - expect_splice_pending_event(acceptor, &node_id_initiator); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); (splice_tx, new_funding_script) } @@ -1374,7 +1372,7 @@ fn test_min_funding_satoshis_rejects_splice_init_with_negative_counterparty_cont let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); assert!(splice_init.funding_contribution_satoshis < 0); nodes[1].node.handle_splice_init(node_id_0, &splice_init); - assert_min_funding_error(&nodes[1], min_funding_satoshis); + assert_min_funding_error(&nodes[1], node_id_0, min_funding_satoshis); } #[test] @@ -1472,7 +1470,7 @@ fn test_min_funding_satoshis_rejects_splice_ack_with_negative_counterparty_contr let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); assert!(splice_ack.funding_contribution_satoshis < 0); nodes[0].node.handle_splice_ack(node_id_1, &splice_ack); - assert_min_funding_error(&nodes[0], min_funding_satoshis); + assert_min_funding_error(&nodes[0], node_id_1, min_funding_satoshis); } #[test] @@ -1514,7 +1512,7 @@ fn test_min_funding_satoshis_rejects_tx_init_rbf_with_negative_counterparty_cont let tx_init_rbf = get_event_msg!(nodes[0], MessageSendEvent::SendTxInitRbf, node_id_1); assert!(tx_init_rbf.funding_output_contribution.unwrap() < 0); nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); - assert_min_funding_error(&nodes[1], min_funding_satoshis); + assert_min_funding_error(&nodes[1], node_id_0, min_funding_satoshis); } #[test] @@ -1571,7 +1569,7 @@ fn test_min_funding_satoshis_rejects_tx_ack_rbf_with_negative_counterparty_contr let tx_ack_rbf = get_event_msg!(nodes[1], MessageSendEvent::SendTxAckRbf, node_id_0); assert!(tx_ack_rbf.funding_output_contribution.unwrap() < 0); nodes[0].node.handle_tx_ack_rbf(node_id_1, &tx_ack_rbf); - assert_min_funding_error(&nodes[0], min_funding_satoshis); + assert_min_funding_error(&nodes[0], node_id_1, min_funding_satoshis); } #[test] @@ -1749,7 +1747,7 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_1_id); - expect_splice_pending_event(&nodes[1], &node_0_id); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Now that the splice is pending, another splice may be initiated. assert!(nodes[0].node.splice_channel(&channel_id, &node_1_id).is_ok()); @@ -2023,7 +2021,7 @@ fn do_test_splice_tiebreak( assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); mine_transaction(&nodes[0], &tx); mine_transaction(&nodes[1], &tx); @@ -2070,7 +2068,7 @@ fn do_test_splice_tiebreak( assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[1], &node_id_0); - expect_splice_pending_event(&nodes[0], &node_id_1); + assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); mine_transaction(&nodes[1], &new_splice_tx); mine_transaction(&nodes[0], &new_splice_tx); @@ -2536,7 +2534,7 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { reconnect_nodes!(|reconnect_args: &mut ReconnectArgs| { reconnect_args.send_interactive_tx_sigs = (false, true); }); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Reestablish the channel again to make sure node 0 doesn't retransmit `tx_signatures` // unnecessarily as it was delivered in the previous reestablishment. @@ -2930,7 +2928,7 @@ fn test_splice_reestablish_waits_for_holder_tx_signatures_before_commitment_sign nodes[1].node.handle_tx_signatures(node_id_0, &initiator_tx_signatures); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } #[test] @@ -3034,7 +3032,7 @@ fn test_splice_reestablish_sends_commitment_signed_before_tx_signatures() { nodes[1].node.handle_tx_signatures(node_id_0, &initiator_tx_signatures); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } #[test] @@ -4001,6 +3999,21 @@ fn acceptor_can_cancel_queued_funding_contributed_during_counterparty_splice() { .unwrap(); assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty()); + // The acceptor is mid-negotiation on the counterparty's splice and has its own contribution + // queued behind it; both surface at once. + let details = acceptor + .node + .list_channels() + .into_iter() + .find(|channel| channel.channel_id == channel_id) + .unwrap() + .splice_details + .unwrap(); + assert_eq!(details.queued_contribution, Some(queued_contribution.clone())); + let negotiation = details.negotiation.unwrap(); + assert_eq!(negotiation.status, SpliceNegotiationStatus::ConstructingTransaction); + assert!(!negotiation.is_initiator); + acceptor.node.cancel_funding_contributed(&channel_id, &node_id_initiator).unwrap(); let reason = NegotiationFailureReason::LocallyCanceled; expect_splice_failed_events(acceptor, &channel_id, queued_contribution, reason); @@ -4023,7 +4036,7 @@ fn acceptor_can_cancel_queued_funding_contributed_during_counterparty_splice() { let (splice_tx, splice_locked) = sign_interactive_funding_tx(initiator, acceptor, false, None); assert!(splice_locked.is_none()); expect_splice_pending_event(initiator, &node_id_acceptor); - expect_splice_pending_event(acceptor, &node_id_initiator); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); mine_transaction(initiator, &splice_tx); mine_transaction(acceptor, &splice_tx); @@ -4422,7 +4435,7 @@ fn free_holding_cell_on_tx_signatures_quiescence_exit() { } expect_splice_pending_event(initiator, &node_id_acceptor); - expect_splice_pending_event(acceptor, &node_id_initiator); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); } #[test] @@ -4902,7 +4915,7 @@ fn test_splice_buffer_commitment_signed_until_funding_tx_signed() { } expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Both nodes should broadcast the splice transaction. let splice_tx = { @@ -5144,7 +5157,7 @@ fn do_splice_waits_for_initial_commitment_monitor_update_before_releasing_tx_sig expect_splice_pending_event(&nodes[0], &node_id_1); if !complete_update_while_disconnected { - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } } @@ -5808,13 +5821,14 @@ fn do_test_splice_pending_htlcs(config: UserConfig) { splice_init.funding_contribution_satoshis -= 1; acceptor.node.handle_splice_init(node_id_initiator, &splice_init); - let msg = get_warning_msg(acceptor, &node_id_initiator); + let msg = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); assert_eq!(msg.channel_id, channel_id); let cannot_be_spliced_out = format!( - "Channel {} cannot be spliced out; their post-splice channel balance {} is smaller than our selected v2 reserve {}", - channel_id, post_splice_reserve - Amount::ONE_SAT, post_splice_reserve + "Their post-splice channel balance {} is smaller than our selected v2 reserve {}", + post_splice_reserve - Amount::ONE_SAT, + post_splice_reserve ); - assert_eq!(msg.data, cannot_be_spliced_out); + assert_eq!(tx_abort_data(&msg), format!("Invalid contribution: {cannot_be_spliced_out}")); acceptor.node.peer_disconnected(node_id_initiator); initiator.node.peer_disconnected(node_id_acceptor); @@ -6067,7 +6081,7 @@ fn test_splice_rbf_acceptor_basic() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let initial_channel_value_sat = 100_000; @@ -6116,7 +6130,7 @@ fn test_splice_rbf_acceptor_basic() { assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Step 11: Mine, lock, and verify DiscardFunding for the replaced splice candidate. let result = lock_rbf_splice_after_blocks( @@ -6148,7 +6162,7 @@ fn test_splice_rbf_discard_unique_contribution() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let initial_channel_value_sat = 100_000; @@ -6217,7 +6231,7 @@ fn test_splice_rbf_discard_unique_contribution() { assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); let result = lock_rbf_splice_after_blocks( &nodes[0], @@ -6249,7 +6263,7 @@ fn test_splice_rbf_at_high_feerate() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let initial_channel_value_sat = 100_000; @@ -6284,7 +6298,7 @@ fn test_splice_rbf_at_high_feerate() { ); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Step 3: RBF again using the template's min_rbf_feerate. The counterparty must accept it. provide_utxo_reserves(&nodes, 2, added_value * 2); @@ -6305,7 +6319,7 @@ fn test_splice_rbf_at_high_feerate() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(rbf_tx_1.compute_txid())); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } #[test] @@ -6506,7 +6520,7 @@ fn test_splice_rbf_insufficient_feerate_high() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(splice_tx.compute_txid())); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // prev=1000: flat increment gives 1000+25=1025, 25/24 rule gives 1000*25/24=1041. // Feerate 1025 satisfies the flat increment but not 25/24 — rejected. @@ -6583,22 +6597,11 @@ fn test_splice_rbf_no_pending_splice() { nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); - let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - match &msg_events[0] { - MessageSendEvent::HandleError { action, .. } => { - assert_eq!( - *action, - msgs::ErrorAction::DisconnectPeerWithWarning { - msg: msgs::WarningMessage { - channel_id, - data: format!("Channel {} has no pending splice to RBF", channel_id), - }, - } - ); - }, - _ => panic!("Expected HandleError, got {:?}", msg_events[0]), - } + let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); + assert_eq!( + tx_abort_data(&tx_abort), + "Rejecting RBF attempt: No pending splice available to RBF" + ); } #[test] @@ -6696,25 +6699,8 @@ fn test_splice_rbf_after_splice_locked() { nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); - let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - match &msg_events[0] { - MessageSendEvent::HandleError { action, .. } => { - assert_eq!( - *action, - msgs::ErrorAction::DisconnectPeerWithWarning { - msg: msgs::WarningMessage { - channel_id, - data: format!( - "Channel {} counterparty already sent splice_locked, cannot RBF", - channel_id, - ), - }, - } - ); - }, - _ => panic!("Expected HandleError, got {:?}", msg_events[0]), - } + let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); + assert_eq!(tx_abort_data(&tx_abort), "Rejecting RBF attempt: Already received splice_locked"); } #[test] @@ -6897,22 +6883,11 @@ fn test_splice_rbf_zeroconf_rejected() { nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); - let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - match &msg_events[0] { - MessageSendEvent::HandleError { action, .. } => { - assert_eq!( - *action, - msgs::ErrorAction::DisconnectPeerWithWarning { - msg: msgs::WarningMessage { - channel_id, - data: format!("Channel {} has option_zeroconf, cannot RBF", channel_id,), - }, - } - ); - }, - _ => panic!("Expected HandleError, got {:?}", msg_events[0]), - } + let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); + assert_eq!( + tx_abort_data(&tx_abort), + format!("Rejecting RBF attempt: Channel {} has option_zeroconf, cannot RBF", channel_id) + ); } #[test] @@ -7218,7 +7193,7 @@ pub fn do_test_splice_rbf_tiebreak( assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Mine, lock, and verify DiscardFunding for the replaced splice candidate. // Node 1's QuiescentAction was preserved, so after splice_locked it re-initiates @@ -7281,7 +7256,7 @@ pub fn do_test_splice_rbf_tiebreak( assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[1], &node_id_0); - expect_splice_pending_event(&nodes[0], &node_id_1); + assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); // Mine and lock. mine_transaction(&nodes[1], &new_splice_tx); @@ -7780,7 +7755,7 @@ fn test_splice_rbf_sequential() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let initial_channel_value_sat = 100_000; @@ -7818,7 +7793,7 @@ fn test_splice_rbf_sequential() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(splice_tx_0.compute_txid())); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // --- Round 2: RBF #2 at feerate 303. --- provide_utxo_reserves(&nodes, 2, added_value * 2); @@ -7839,7 +7814,7 @@ fn test_splice_rbf_sequential() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(splice_tx_1.compute_txid())); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // --- Mine and lock the final RBF, verifying DiscardFunding for both replaced candidates. --- let splice_tx_0_txid = splice_tx_0.compute_txid(); @@ -7865,7 +7840,7 @@ fn test_splice_rbf_amends_prior_net_positive_contribution_request() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let (_, _, channel_id, _) = @@ -7908,7 +7883,7 @@ fn test_splice_rbf_amends_prior_net_positive_contribution_request() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(replaced_txid)); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); tx }; @@ -7997,7 +7972,7 @@ fn test_splice_rbf_amends_prior_net_negative_contribution_request() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let (_, _, channel_id, _) = @@ -8042,7 +8017,7 @@ fn test_splice_rbf_amends_prior_net_negative_contribution_request() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(replaced_txid)); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); tx }; @@ -8228,23 +8203,13 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { // The initiator re-used the same UTXOs as round 0. Since those UTXOs are still committed // to round 0's splice, they are filtered and no DiscardFunding is emitted. - let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 1, "{events:?}"); - match &events[0] { - Event::SpliceNegotiationFailed { channel_id: cid, reason, contribution, .. } => { - assert_eq!(*cid, channel_id); - assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); - assert!(contribution.is_some()); - }, - other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), - } + let _ = get_event!(&nodes[0], Event::SpliceNegotiationFailed); // The acceptor re-contributed the same UTXOs as round 0 (via prior contribution // adjustment). Since those UTXOs are still committed to round 0's splice, they are - // filtered and no DiscardFunding is emitted. With all inputs/outputs filtered, no events - // are emitted for the acceptor. - let events = nodes[1].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 0, "{events:?}"); + // filtered and no DiscardFunding is emitted. The contribution still fails and needs a + // SpliceNegotiationFailed event so the wallet can resume funding. + let _ = get_event!(&nodes[1], Event::SpliceNegotiationFailed); // Reconnect. let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); @@ -9088,7 +9053,7 @@ fn test_splice_rbf_rejects_low_feerate_after_several_attempts() { ); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); prev_feerate = feerate; prev_splice_tx = rbf_tx; } @@ -9120,7 +9085,7 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let initial_channel_value_sat = 100_000; @@ -9163,7 +9128,7 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { ); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); prev_feerate = feerate; prev_splice_tx = rbf_tx; } @@ -9233,10 +9198,10 @@ fn test_no_disconnect_after_splice_completes() { let (_, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], false, None); assert!(splice_locked.is_none()); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Fire enough ticks to trigger a disconnect if the timer wasn't properly cleared. for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS { @@ -9728,40 +9693,35 @@ fn do_test_0reserve_splice_counterparty_validation( get_event_msg!(acceptor, MessageSendEvent::SendSpliceAck, node_id_initiator); } else { acceptor.node.handle_splice_init(node_id_initiator, &splice_init); - let msg_events = acceptor.node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - if let MessageSendEvent::HandleError { action, .. } = &msg_events[0] { - assert!(matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })); - } else { - panic!("Expected MessageSendEvent::HandleError"); - } + let msg = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); + assert_eq!(msg.channel_id, channel_id); let cannot_splice_out = if u64::try_from(funding_contribution_sat.abs()).unwrap() > initiator_value_to_self_sat { // They obviously can't afford their contribution, so we fail before even // querying `TxBuilder` format!( - "Got non-closing error: Channel {channel_id} cannot be spliced; \ - Their contribution candidate {funding_contribution_sat}sat \ + "Their contribution candidate {funding_contribution_sat}sat \ is greater than their total balance in the channel {initiator_value_to_self_sat}sat" ) } else if post_channel_value_sat < MIN_CHANNEL_VALUE_SATOSHIS { // We require all spliced channels to have a value of at least 1000 satoshis after the splice format!( - "Got non-closing error: Channel {channel_id} cannot be spliced; \ - Spliced channel value must be at least {MIN_CHANNEL_VALUE_SATOSHIS} satoshis. \ + "Spliced channel value must be at least {MIN_CHANNEL_VALUE_SATOSHIS} satoshis. \ It would be {post_channel_value_sat}" ) } else { // Last but not least, `TxBuilder` decides whether all parties can afford // HTLCs, anchors, and transaction fees while retaining at least one // output on the commitments - format!( - "Got non-closing error: Channel {channel_id} cannot \ - be spliced; Balance exhausted on local commitment" - ) + "Balance exhausted on local commitment".to_string() }; - acceptor.logger.assert_log("lightning::ln::channelmanager", cannot_splice_out, 1); + assert_eq!(tx_abort_data(&msg), format!("Invalid contribution: {cannot_splice_out}")); + acceptor.logger.assert_log( + "lightning::ln::channelmanager", + format!("Got non-closing error: Invalid contribution: {cannot_splice_out}"), + 1, + ); } channel_type @@ -10002,18 +9962,12 @@ fn do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( // balance, we previously would not complain. splice_init.funding_contribution_satoshis = funding_contribution_sat; acceptor.node.handle_splice_init(node_id_initiator, &splice_init); - let msg_events = acceptor.node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - if let MessageSendEvent::HandleError { action, .. } = &msg_events[0] { - assert!(matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })); - } else { - panic!("Expected MessageSendEvent::HandleError"); - } + let msg = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); + assert_eq!(msg.channel_id, channel_id); let post_splice_channel_value_sat = node_0_balance_leftover_amount.to_sat(); let cannot_splice_out = if matches!(acceptor_balance, AcceptorBalance::NoBalance) { format!( - "Got non-closing error: Channel {channel_id} cannot \ - be spliced; The post-splice channel value {post_splice_channel_value_sat} \ + "The post-splice channel value {post_splice_channel_value_sat} \ is smaller than their dust limit {high_dust_limit_satoshis}" ) } else { @@ -10026,13 +9980,17 @@ fn do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( high_dust_limit_satoshis ); format!( - "Got non-closing error: Channel {channel_id} cannot \ - be spliced out; their post-splice channel balance \ + "Their post-splice channel balance \ {node_0_balance_leftover_amount} is smaller than our selected v2 reserve \ {v2_channel_reserve}" ) }; - acceptor.logger.assert_log("lightning::ln::channelmanager", cannot_splice_out, 1); + assert_eq!(tx_abort_data(&msg), format!("Invalid contribution: {cannot_splice_out}")); + acceptor.logger.assert_log( + "lightning::ln::channelmanager", + format!("Got non-closing error: Invalid contribution: {cannot_splice_out}"), + 1, + ); } } @@ -10248,3 +10206,356 @@ fn test_splice_out_maximum_includes_pending_claimed_inbound_htlc() { assert!(nodes[1].node.splice_channel(&channel_id, &node_id_0).is_ok()); } + +#[test] +fn test_channel_details_pending_splice() { + // Test that `ChannelDetails::splice_details` reflects pending splice state throughout + // negotiation, signing, RBF, restarts, and locking. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let (persister_0, persister_1); + let (chain_monitor_0, chain_monitor_1); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let (node_0, node_1); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let splice_details = |node: &Node<'_, '_, '_>| { + node.node + .list_channels() + .iter() + .find(|channel| channel.channel_id == channel_id) + .unwrap() + .splice_details + .clone() + }; + + // No splice is pending yet. + assert_eq!(splice_details(&nodes[0]), None); + assert_eq!(splice_details(&nodes[1]), None); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Contributing funds queues the contribution but does not start the negotiation; that begins + // once the channel becomes quiescent and splice_init is sent. Until then it surfaces as a + // queued contribution with no negotiation. + let contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + assert_eq!( + splice_details(&nodes[0]), + Some(SpliceDetails { + negotiation: None, + queued_contribution: Some(contribution.clone()), + candidates: vec![], + confirmed_candidate: None, + received_splice_locked_txid: None, + }), + ); + assert_eq!(splice_details(&nodes[1]), None); + + let new_channel_value_sat = + (initial_channel_value_sat as i64 + contribution.net_value().to_sat()) as u64; + + let stfu_init = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + nodes[1].node.handle_stfu(node_id_0, &stfu_init); + let stfu_ack = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu_ack); + + // Once quiescent, the initiator sends splice_init and awaits the counterparty's splice_ack. + // The new channel value is not yet known as it depends on the counterparty's contribution. + let details = splice_details(&nodes[0]).unwrap(); + let negotiation = details.negotiation.unwrap(); + assert_eq!(negotiation.status, SpliceNegotiationStatus::AwaitingAck); + assert!(negotiation.is_initiator); + assert_eq!(negotiation.funding_feerate_sat_per_1000_weight, FEERATE_FLOOR_SATS_PER_KW); + assert_eq!(negotiation.new_channel_value_satoshis, None); + assert_eq!(negotiation.txid, None); + assert_eq!(negotiation.contribution, Some(contribution.clone())); + assert!(details.candidates.is_empty()); + assert_eq!(splice_details(&nodes[1]), None); + + let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); + nodes[1].node.handle_splice_init(node_id_0, &splice_init); + + // The acceptor starts constructing the transaction upon receiving splice_init, at which + // point both contributions are known. + let details = splice_details(&nodes[1]).unwrap(); + let negotiation = details.negotiation.unwrap(); + assert_eq!(negotiation.status, SpliceNegotiationStatus::ConstructingTransaction); + assert!(!negotiation.is_initiator); + assert_eq!(negotiation.funding_feerate_sat_per_1000_weight, FEERATE_FLOOR_SATS_PER_KW); + assert_eq!(negotiation.new_channel_value_satoshis, Some(new_channel_value_sat)); + assert_eq!(negotiation.txid, None); + assert_eq!(negotiation.contribution, None); + assert!(details.candidates.is_empty()); + + let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); + nodes[0].node.handle_splice_ack(node_id_1, &splice_ack); + + let negotiation = splice_details(&nodes[0]).unwrap().negotiation.unwrap(); + assert_eq!(negotiation.status, SpliceNegotiationStatus::ConstructingTransaction); + assert!(negotiation.is_initiator); + assert_eq!(negotiation.new_channel_value_satoshis, Some(new_channel_value_sat)); + assert_eq!(negotiation.txid, None); + + let new_funding_script = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + + complete_interactive_funding_negotiation( + &nodes[0], + &nodes[1], + channel_id, + contribution.clone(), + new_funding_script.clone(), + ); + + // Once construction completes, the negotiation awaits signatures and the txid is known. + let negotiation_0 = splice_details(&nodes[0]).unwrap().negotiation.unwrap(); + let negotiation_1 = splice_details(&nodes[1]).unwrap().negotiation.unwrap(); + assert_eq!(negotiation_0.status, SpliceNegotiationStatus::AwaitingSignatures); + assert_eq!(negotiation_1.status, SpliceNegotiationStatus::AwaitingSignatures); + assert!(negotiation_0.txid.is_some()); + assert_eq!(negotiation_0.txid, negotiation_1.txid); + assert_eq!(negotiation_0.new_channel_value_satoshis, Some(new_channel_value_sat)); + assert_eq!(negotiation_0.contribution, Some(contribution.clone())); + assert_eq!(negotiation_1.contribution, None); + + let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], false, None); + assert!(splice_locked.is_none()); + assert_eq!(negotiation_0.txid, Some(splice_tx.compute_txid())); + + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + + // With signatures exchanged, the negotiated splice is a candidate awaiting confirmations. + let details = splice_details(&nodes[0]).unwrap(); + assert_eq!(details.negotiation, None); + assert_eq!(details.candidates.len(), 1); + assert_eq!(details.candidates[0].txid, splice_tx.compute_txid()); + assert_eq!(details.candidates[0].new_channel_value_satoshis, new_channel_value_sat); + assert_eq!(details.candidates[0].contribution, Some(contribution.clone())); + assert_eq!(details.confirmed_candidate, None); + assert_eq!(details.received_splice_locked_txid, None); + + // The acceptor did not contribute to the splice. + let details = splice_details(&nodes[1]).unwrap(); + assert_eq!(details.negotiation, None); + assert_eq!(details.candidates.len(), 1); + assert_eq!(details.candidates[0].txid, splice_tx.compute_txid()); + assert_eq!(details.candidates[0].contribution, None); + + // Initiate an RBF attempt at a higher feerate. + provide_utxo_reserves(&nodes, 2, added_value * 2); + let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; + let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); + let rbf_contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); + + // The RBF contribution is queued behind the still-pending original candidate until quiescence + // is re-reached; until then it surfaces as a queued contribution with no negotiation. + let details = splice_details(&nodes[0]).unwrap(); + assert_eq!(details.queued_contribution, Some(rbf_contribution.clone())); + assert_eq!(details.negotiation, None); + assert_eq!(details.candidates.len(), 1); + assert_eq!(details.candidates[0].txid, splice_tx.compute_txid()); + + complete_rbf_handshake(&nodes[0], &nodes[1]); + + // The RBF negotiation is reported alongside the still-pending original candidate. + let details = splice_details(&nodes[0]).unwrap(); + let negotiation = details.negotiation.unwrap(); + assert_eq!(negotiation.status, SpliceNegotiationStatus::ConstructingTransaction); + assert_eq!(negotiation.funding_feerate_sat_per_1000_weight, rbf_feerate_sat_per_kwu as u32); + assert_eq!(negotiation.contribution, Some(rbf_contribution.clone())); + assert_eq!(details.candidates.len(), 1); + assert_eq!(details.candidates[0].txid, splice_tx.compute_txid()); + assert_eq!(details.candidates[0].contribution, Some(contribution.clone())); + + let rbf_channel_value_sat = + (initial_channel_value_sat as i64 + rbf_contribution.net_value().to_sat()) as u64; + + complete_interactive_funding_negotiation( + &nodes[0], + &nodes[1], + channel_id, + rbf_contribution.clone(), + new_funding_script, + ); + let (rbf_tx, splice_locked) = + sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(splice_tx.compute_txid())); + assert!(splice_locked.is_none()); + + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + + // Both the original splice and its RBF replacement are candidates, in negotiation order. + let details = splice_details(&nodes[0]).unwrap(); + assert_eq!(details.negotiation, None); + assert_eq!(details.candidates.len(), 2); + assert_eq!(details.candidates[0].txid, splice_tx.compute_txid()); + assert_eq!(details.candidates[0].contribution, Some(contribution.clone())); + assert_eq!(details.candidates[1].txid, rbf_tx.compute_txid()); + assert_eq!(details.candidates[1].new_channel_value_satoshis, rbf_channel_value_sat); + assert_eq!(details.candidates[1].contribution, Some(rbf_contribution.clone())); + + let details = splice_details(&nodes[1]).unwrap(); + assert_eq!(details.candidates.len(), 2); + assert_eq!(details.candidates[1].contribution, None); + + // Pending splice state, including per-candidate contributions, survives a restart. + let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); + reload_node!( + nodes[0], + &nodes[0].node.encode(), + &[&encoded_monitor_0], + persister_0, + chain_monitor_0, + node_0 + ); + let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode(); + reload_node!( + nodes[1], + &nodes[1].node.encode(), + &[&encoded_monitor_1], + persister_1, + chain_monitor_1, + node_1 + ); + + let details = splice_details(&nodes[0]).unwrap(); + assert_eq!(details.negotiation, None); + assert_eq!(details.candidates.len(), 2); + assert_eq!(details.candidates[0].txid, splice_tx.compute_txid()); + assert_eq!(details.candidates[0].contribution, Some(contribution)); + assert_eq!(details.candidates[1].txid, rbf_tx.compute_txid()); + assert_eq!(details.candidates[1].contribution, Some(rbf_contribution)); + + let details = splice_details(&nodes[1]).unwrap(); + assert_eq!(details.candidates.len(), 2); + assert!(details.candidates.iter().all(|candidate| candidate.contribution.is_none())); + + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_announcement_sigs = (true, true); + reconnect_nodes(reconnect_args); + + // Mine the RBF transaction; only its candidate confirms, identified by its index. + mine_transaction(&nodes[0], &rbf_tx); + mine_transaction(&nodes[1], &rbf_tx); + + let details = splice_details(&nodes[0]).unwrap(); + let confirmed = details.confirmed_candidate.unwrap(); + assert_eq!(confirmed.txid, rbf_tx.compute_txid()); + assert_eq!(confirmed.confirmations, 1); + assert_eq!(confirmed.confirmations_required, 6); + + connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1); + connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1); + + // Once sufficiently confirmed, the splice_locked we sent is reflected in the details until + // the counterparty's splice_locked is received and the splice is promoted. + let splice_locked = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceLocked, node_id_1); + let details = splice_details(&nodes[0]).unwrap(); + assert_eq!(details.received_splice_locked_txid, None); + let confirmed = details.confirmed_candidate.unwrap(); + assert_eq!(confirmed.txid, rbf_tx.compute_txid()); + assert!(confirmed.splice_locked_sent); + assert_eq!(confirmed.confirmations, ANTI_REORG_DELAY); + + lock_splice(&nodes[0], &nodes[1], &splice_locked, false, &[splice_tx.compute_txid()]); + + // The splice is no longer pending once promoted. + assert_eq!(splice_details(&nodes[0]), None); + assert_eq!(splice_details(&nodes[1]), None); +} + +#[test] +fn test_channel_details_first_contribution_on_rbf() { + // When the counterparty's splice did not include a contribution from us and our first + // contribution comes in an RBF round we initiate, the in-flight contribution must not be + // attributed to the negotiated counterparty-only candidate. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Splice initiated by node 1; node 0 does not contribute. + let contribution = do_initiate_splice_in(&nodes[1], &nodes[0], channel_id, added_value); + let (splice_tx, _) = splice_channel(&nodes[1], &nodes[0], channel_id, contribution); + + // Node 0 initiates an RBF, contributing for the first time. + let rbf_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let rbf_contribution = funding_template + .without_prior_contribution(rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(added_value) + .unwrap() + .build() + .unwrap(); + nodes[0] + .node + .funding_contributed(&channel_id, &node_id_1, rbf_contribution.clone(), None) + .unwrap(); + complete_rbf_handshake(&nodes[0], &nodes[1]); + let _ = get_event_msg!(nodes[0], MessageSendEvent::SendTxAddInput, node_id_1); + + // While the RBF is being negotiated, node 0's contribution belongs to the negotiation, not + // to the negotiated counterparty-only candidate. + let channels = nodes[0].node.list_channels(); + let details = channels[0].splice_details.as_ref().unwrap(); + assert_eq!(details.negotiation.as_ref().unwrap().contribution, Some(rbf_contribution.clone())); + assert_eq!(details.candidates.len(), 1); + assert_eq!(details.candidates[0].txid, splice_tx.compute_txid()); + assert_eq!(details.candidates[0].contribution, None); + + // Node 1 adjusted its prior contribution for the RBF round; the negotiated candidate keeps + // its original contribution. + let channels = nodes[1].node.list_channels(); + let details = channels[0].splice_details.as_ref().unwrap(); + assert!(details.negotiation.as_ref().unwrap().contribution.is_some()); + assert!(details.candidates[0].contribution.is_some()); + + // Abort the negotiation via disconnect. + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); + + expect_splice_failed_events( + &nodes[0], + &channel_id, + rbf_contribution, + NegotiationFailureReason::PeerDisconnected, + ); + // Node 1 did not initiate the RBF round and its contribution to it (the prior round's + // contribution adjusted to the new feerate) has no inputs or outputs unique from the prior + // round, so no events are emitted. + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); + + // After the reset, the contribution alignment is restored on both nodes. + let channels = nodes[0].node.list_channels(); + let details = channels[0].splice_details.as_ref().unwrap(); + assert_eq!(details.negotiation, None); + assert_eq!(details.candidates[0].contribution, None); + let channels = nodes[1].node.list_channels(); + let details = channels[0].splice_details.as_ref().unwrap(); + assert_eq!(details.negotiation, None); + assert!(details.candidates[0].contribution.is_some()); +} diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index b9b7a93877a..59804545381 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -4249,6 +4249,7 @@ mod tests { pending_inbound_htlcs: Vec::new(), pending_outbound_htlcs: Vec::new(), current_dust_exposure_msat: None, + splice_details: None, } } @@ -9813,6 +9814,7 @@ pub(crate) mod bench_utils { pending_inbound_htlcs: Vec::new(), pending_outbound_htlcs: Vec::new(), current_dust_exposure_msat: None, + splice_details: None, } } diff --git a/pending_changelog/4687-pending-splice-details.txt b/pending_changelog/4687-pending-splice-details.txt new file mode 100644 index 00000000000..789d05c3a0a --- /dev/null +++ b/pending_changelog/4687-pending-splice-details.txt @@ -0,0 +1,18 @@ +# API Updates + + * `ChannelDetails` now has a `splice_details` field + (`Option`) reporting any pending splice attempts on a channel: + a contribution committed via `ChannelManager::funding_contributed` but not yet + negotiating, the in-flight negotiation (`SpliceNegotiationDetails`, with + progress given by `SpliceNegotiationStatus`), the negotiated candidates + awaiting confirmation (`SpliceCandidateDetails`), the confirmed candidate's + progress (`ConfirmedSpliceCandidate`), and the txid of any `splice_locked` + received from the counterparty. + +# Backwards Compatibility + + * A channel with a pending splice negotiated before upgrading from a prior LDK + version (e.g. 0.2) cannot be RBF'ed: `ChannelManager::splice_channel` + now returns an `APIError::APIMisuseError`, as the prior feerate and our + contribution needed to derive the RBF feerate floor are not persisted by + older versions. diff --git a/possiblyrandom/src/lib.rs b/possiblyrandom/src/lib.rs index 6ddbc6de1a2..f27788d03fa 100644 --- a/possiblyrandom/src/lib.rs +++ b/possiblyrandom/src/lib.rs @@ -20,19 +20,13 @@ #![no_std] -#[cfg(any( - feature = "getrandom", - not(any(target_os = "unknown", target_os = "none")) -))] +#[cfg(any(feature = "getrandom", not(any(target_os = "unknown", target_os = "none"))))] extern crate getrandom; /// Possibly fills `dest` with random data. May fill it with zeros. #[inline] pub fn getpossiblyrandom(dest: &mut [u8]) { dest.fill(0); - #[cfg(any( - feature = "getrandom", - not(any(target_os = "unknown", target_os = "none")) - ))] + #[cfg(any(feature = "getrandom", not(any(target_os = "unknown", target_os = "none"))))] let _ = getrandom::getrandom(dest); }