diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 63f730781a6..6aee5e7e67a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -377,7 +377,7 @@ pub(super) struct Channel { /// The hash of the block in which the funding transaction was included. funding_tx_confirmed_in: Option, - funding_tx_confirmation_height: u64, + funding_tx_confirmation_height: u32, short_channel_id: Option, counterparty_dust_limit_satoshis: u64, @@ -3591,7 +3591,7 @@ impl Channel { } } } - self.funding_tx_confirmation_height = height as u64; + self.funding_tx_confirmation_height = height; self.funding_tx_confirmed_in = Some(*block_hash); self.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) { Ok(scid) => Some(scid), @@ -3678,6 +3678,32 @@ impl Channel { Ok((None, timed_out_htlcs)) } + /// Indicates the funding transaction is no longer confirmed in the main chain. This may + /// force-close the channel, but may also indicate a harmless reorganization of a block or two + /// before the channel has reached funding_locked and we can just wait for more blocks. + pub fn funding_transaction_unconfirmed(&mut self) -> Result<(), msgs::ErrorMessage> { + if self.funding_tx_confirmation_height != 0 { + // We handle the funding disconnection by calling update_best_block with a height one + // below where our funding was connected, implying a reorg back to conf_height - 1. + let reorg_height = self.funding_tx_confirmation_height - 1; + // We use the time field to bump the current time we set on channel updates if its + // larger. If we don't know that time has moved forward, we can just set it to the last + // time we saw and it will be ignored. + let best_time = self.update_time_counter; + match self.update_best_block(reorg_height, best_time) { + Ok((funding_locked, timed_out_htlcs)) => { + assert!(funding_locked.is_none(), "We can't generate a funding with 0 confirmations?"); + assert!(timed_out_htlcs.is_empty(), "We can't have accepted HTLCs with a timeout before our funding confirmation?"); + Ok(()) + }, + Err(e) => Err(e) + } + } else { + // We never learned about the funding confirmation anyway, just ignore + Ok(()) + } + } + // Methods to get unprompted messages to send to the remote end (or where we already returned // something in the handler for the message that prompted this message): diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d2ea7dfea79..f9e00bc5d03 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -28,7 +28,7 @@ use bitcoin::hashes::hmac::{Hmac, HmacEngine}; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::sha256d::Hash as Sha256dHash; use bitcoin::hashes::cmp::fixed_time_eq; -use bitcoin::hash_types::BlockHash; +use bitcoin::hash_types::{BlockHash, Txid}; use bitcoin::secp256k1::key::{SecretKey,PublicKey}; use bitcoin::secp256k1::Secp256k1; @@ -851,6 +851,11 @@ impl ChannelMana } } + /// Gets the current configuration applied to all new channels, as + pub fn get_current_default_configuration(&self) -> &UserConfig { + &self.default_configuration + } + /// Creates a new outbound channel to the given remote node and with the given value. /// /// user_id will be provided back as user_channel_id in FundingGenerationReady events to allow @@ -3353,7 +3358,7 @@ where "Blocks must be disconnected in chain-order - the disconnected block must have the correct height"); *self.last_block_hash.write().unwrap() = header.prev_blockhash; - self.do_chain_event(new_height, |channel| channel.update_best_block(new_height, header.time)); + self.do_chain_event(Some(new_height), |channel| channel.update_best_block(new_height, header.time)); } } @@ -3364,8 +3369,11 @@ impl ChannelMana F::Target: FeeEstimator, L::Target: Logger, { + /// Calls a function which handles an on-chain event (blocks dis/connected, transactions + /// un/confirmed, etc) on each channel, handling any resulting errors or messages generated by + /// the function. fn do_chain_event) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage>> - (&self, height: u32, f: FN) { + (&self, height_opt: Option, f: FN) { // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called // during initialization prior to the chain_monitor being fully configured in some cases. // See the docs for `ChannelManagerReadArgs` for more. @@ -3424,24 +3432,26 @@ impl ChannelMana true }); - channel_state.claimable_htlcs.retain(|&(ref payment_hash, _), htlcs| { - htlcs.retain(|htlc| { - // If height is approaching the number of blocks we think it takes us to get - // our commitment transaction confirmed before the HTLC expires, plus the - // number of blocks we generally consider it to take to do a commitment update, - // just give up on it and fail the HTLC. - if height >= htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER { - let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec(); - htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(height)); - timed_out_htlcs.push((HTLCSource::PreviousHopData(htlc.prev_hop.clone()), payment_hash.clone(), HTLCFailReason::Reason { - failure_code: 0x4000 | 15, - data: htlc_msat_height_data - })); - false - } else { true } + if let Some(height) = height_opt { + channel_state.claimable_htlcs.retain(|&(ref payment_hash, _), htlcs| { + htlcs.retain(|htlc| { + // If height is approaching the number of blocks we think it takes us to get + // our commitment transaction confirmed before the HTLC expires, plus the + // number of blocks we generally consider it to take to do a commitment update, + // just give up on it and fail the HTLC. + if height >= htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER { + let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec(); + htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(height)); + timed_out_htlcs.push((HTLCSource::PreviousHopData(htlc.prev_hop.clone()), payment_hash.clone(), HTLCFailReason::Reason { + failure_code: 0x4000 | 15, + data: htlc_msat_height_data + })); + false + } else { true } + }); + !htlcs.is_empty() // Only retain this entry if htlcs has at least one entry. }); - !htlcs.is_empty() // Only retain this entry if htlcs has at least one entry. - }); + } } self.handle_init_event_channel_failures(failed_channels); @@ -3477,7 +3487,7 @@ impl ChannelMana log_trace!(self.logger, "{} transactions included in block {} at height {} provided", txdata.len(), block_hash, height); let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier); - self.do_chain_event(height, |channel| channel.transactions_confirmed(&block_hash, height, txdata, &self.logger).map(|a| (a, Vec::new()))); + self.do_chain_event(Some(height), |channel| channel.transactions_confirmed(&block_hash, height, txdata, &self.logger).map(|a| (a, Vec::new()))); } /// Updates channel state with the current best blockchain tip. You should attempt to call this @@ -3506,7 +3516,7 @@ impl ChannelMana self.latest_block_height.store(height as usize, Ordering::Release); *self.last_block_hash.write().unwrap() = block_hash; - self.do_chain_event(height, |channel| channel.update_best_block(height, header.time)); + self.do_chain_event(Some(height), |channel| channel.update_best_block(height, header.time)); loop { // Update last_node_announcement_serial to be the max of its current value and the @@ -3522,6 +3532,61 @@ impl ChannelMana } } + /// Gets the set of txids which should be monitored for their confirmation state. + /// + /// If you're providing information about reorganizations via [`transaction_unconfirmed`], this + /// is the set of transactions which you may need to call [`transaction_unconfirmed`] for. + /// + /// This may be useful to poll to determine the set of transactions which must be registered + /// with an Electrum server or for which an Electrum server needs to be polled to determine + /// transaction confirmation state. + /// + /// This may update after any [`transactions_confirmed`] or [`block_connected`] call. + /// + /// Note that this is NOT the set of transactions which must be included in calls to + /// [`transactions_confirmed`] if they are confirmed, but a small subset of it. + /// + /// [`transactions_confirmed`]: Self::transactions_confirmed + /// [`transaction_unconfirmed`]: Self::transaction_unconfirmed + /// [`block_connected`]: chain::Listen::block_connected + pub fn get_relevant_txids(&self) -> Vec { + let channel_state = self.channel_state.lock().unwrap(); + let mut res = Vec::with_capacity(channel_state.short_to_id.len()); + for chan in channel_state.by_id.values() { + if let Some(funding_txo) = chan.get_funding_txo() { + res.push(funding_txo.txid); + } + } + res + } + + /// Marks a transaction as having been reorganized out of the blockchain. + /// + /// If a transaction is included in [`get_relevant_txids`], and is no longer in the main branch + /// of the blockchain, this function should be called to indicate that the transaction should + /// be considered reorganized out. + /// + /// Once this is called, the given transaction will no longer appear on [`get_relevant_txids`], + /// though this may be called repeatedly for a given transaction without issue. + /// + /// Note that if the transaction is confirmed on the main chain in a different block (indicated + /// via a call to [`transactions_confirmed`]), it may re-appear in [`get_relevant_txids`], thus + /// be very wary of race-conditions wherein the final state of a transaction indicated via + /// these APIs is not the same as its state on the blockchain. + /// + /// [`transactions_confirmed`]: Self::transactions_confirmed + /// [`get_relevant_txids`]: Self::get_relevant_txids + pub fn transaction_unconfirmed(&self, txid: &Txid) { + let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier); + self.do_chain_event(None, |channel| { + if let Some(funding_txo) = channel.get_funding_txo() { + if funding_txo.txid == *txid { + channel.funding_transaction_unconfirmed().map(|_| (None, Vec::new())) + } else { Ok((None, Vec::new())) } + } else { Ok((None, Vec::new())) } + }); + } + /// Blocks until ChannelManager needs to be persisted or a timeout is reached. It returns a bool /// indicating whether persistence is necessary. Only one listener on /// `await_persistable_update` or `await_persistable_update_timeout` is guaranteed to be woken diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index da3e19c04e0..e3f5003aaf6 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -62,7 +62,7 @@ pub fn mine_transaction<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transac pub fn confirm_transaction_at<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transaction, conf_height: u32) { let first_connect_height = node.best_block_info().1 + 1; assert!(first_connect_height <= conf_height); - if conf_height - first_connect_height >= 1 { + if conf_height > first_connect_height { connect_blocks(node, conf_height - first_connect_height); } let mut block = Block { @@ -77,6 +77,7 @@ pub fn confirm_transaction_at<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &T } /// The possible ways we may notify a ChannelManager of a new block +#[derive(Clone, Copy, PartialEq)] pub enum ConnectStyle { /// Calls update_best_block first, detecting transactions in the block only after receiving the /// header and height information. @@ -281,7 +282,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> { let mut w = test_utils::TestVecWriter(Vec::new()); self.node.write(&mut w).unwrap(); <(BlockHash, ChannelManager)>::read(&mut ::std::io::Cursor::new(w.0), ChannelManagerReadArgs { - default_config: UserConfig::default(), + default_config: *self.node.get_current_default_configuration(), keys_manager: self.keys_manager, fee_estimator: &test_utils::TestFeeEstimator { sat_per_kw: 253 }, chain_monitor: self.chain_monitor, @@ -1413,7 +1414,7 @@ pub fn check_preimage_claim<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, prev_txn: &Vec< res } -pub fn get_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec>, a: usize, b: usize) { +pub fn handle_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec>, a: usize, b: usize, needs_err_handle: bool, expected_error: &str) { let events_1 = nodes[a].node.get_and_clear_pending_msg_events(); assert_eq!(events_1.len(), 2); let as_update = match events_1[0] { @@ -1425,25 +1426,30 @@ pub fn get_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec { assert_eq!(node_id, nodes[b].node.get_our_node_id()); - assert_eq!(msg.data, "Commitment or closing transaction was confirmed on chain."); + assert_eq!(msg.data, expected_error); + if needs_err_handle { + nodes[b].node.handle_error(&nodes[a].node.get_our_node_id(), msg); + } }, _ => panic!("Unexpected event"), } let events_2 = nodes[b].node.get_and_clear_pending_msg_events(); - assert_eq!(events_2.len(), 2); + assert_eq!(events_2.len(), if needs_err_handle { 1 } else { 2 }); let bs_update = match events_2[0] { MessageSendEvent::BroadcastChannelUpdate { ref msg } => { msg.clone() }, _ => panic!("Unexpected event"), }; - match events_2[1] { - MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => { - assert_eq!(node_id, nodes[a].node.get_our_node_id()); - assert_eq!(msg.data, "Commitment or closing transaction was confirmed on chain."); - }, - _ => panic!("Unexpected event"), + if !needs_err_handle { + match events_2[1] { + MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => { + assert_eq!(node_id, nodes[a].node.get_our_node_id()); + assert_eq!(msg.data, expected_error); + }, + _ => panic!("Unexpected event"), + } } for node in nodes { @@ -1452,6 +1458,10 @@ pub fn get_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec(nodes: &Vec>, a: usize, b: usize) { + handle_announce_close_broadcast_events(nodes, a, b, false, "Commitment or closing transaction was confirmed on chain."); +} + #[cfg(test)] macro_rules! get_channel_value_stat { ($node: expr, $channel_id: expr) => {{ diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index c9573a67c13..d5a3d042f29 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -14,7 +14,6 @@ use chain::Watch; use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs}; use ln::features::InitFeatures; use ln::msgs::{ChannelMessageHandler, ErrorAction, HTLCFailChannelUpdate}; -use util::config::UserConfig; use util::enforcing_trait_impls::EnforcingSigner; use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider}; use util::test_utils; @@ -184,7 +183,7 @@ fn test_onchain_htlc_timeout_delay_remote_commitment() { do_test_onchain_htlc_reorg(false, false); } -fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, connect_style: ConnectStyle) { +fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_unconfirmed: bool, connect_style: ConnectStyle) { // After creating a chan between nodes, we disconnect all blocks previously seen to force a // channel close on nodes[0] side. We also use this to provide very basic testing of logic // around freeing background events which store monitor updates during block_[dis]connected. @@ -197,7 +196,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, connect_styl let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); *nodes[0].connect_style.borrow_mut() = connect_style; - let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2; + let chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); let channel_state = nodes[0].node.channel_state.lock().unwrap(); assert_eq!(channel_state.by_id.len(), 1); @@ -205,8 +204,19 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, connect_styl mem::drop(channel_state); if !reorg_after_reload { - disconnect_all_blocks(&nodes[0]); - check_closed_broadcast!(nodes[0], true); + if use_funding_unconfirmed { + let relevant_txids = nodes[0].node.get_relevant_txids(); + assert_eq!(&relevant_txids[..], &[chan.3.txid()]); + nodes[0].node.transaction_unconfirmed(&relevant_txids[0]); + } else { + disconnect_all_blocks(&nodes[0]); + } + if connect_style == ConnectStyle::FullBlockViaListen && !use_funding_unconfirmed { + handle_announce_close_broadcast_events(&nodes, 0, 1, true, "Funding transaction was un-confirmed. Locked at 6 confs, now have 2 confs."); + } else { + handle_announce_close_broadcast_events(&nodes, 0, 1, true, "Funding transaction was un-confirmed. Locked at 6 confs, now have 0 confs."); + } + check_added_monitors!(nodes[1], 1); { let channel_state = nodes[0].node.channel_state.lock().unwrap(); assert_eq!(channel_state.by_id.len(), 0); @@ -233,14 +243,13 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, connect_styl assert!(chan_0_monitor_read.is_empty()); let mut nodes_0_read = &nodes_0_serialized[..]; - let config = UserConfig::default(); nodes_0_deserialized = { let mut channel_monitors = HashMap::new(); channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor); <(BlockHash, ChannelManager)>::read( &mut nodes_0_read, ChannelManagerReadArgs { - default_config: config, + default_config: *nodes[0].node.get_current_default_configuration(), keys_manager, fee_estimator: node_cfgs[0].fee_estimator, chain_monitor: nodes[0].chain_monitor, @@ -251,14 +260,31 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, connect_styl }; nodes[0].node = &nodes_0_deserialized; assert!(nodes_0_read.is_empty()); + if !reorg_after_reload { + // If the channel is already closed when we reload the node, we'll broadcast a closing + // transaction via the ChannelMonitor which is missing a corresponding channel. + assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1); + nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clear(); + } nodes[0].chain_monitor.watch_channel(chan_0_monitor.get_funding_txo().0.clone(), chan_0_monitor).unwrap(); check_added_monitors!(nodes[0], 1); } if reorg_after_reload { - disconnect_all_blocks(&nodes[0]); - check_closed_broadcast!(nodes[0], true); + if use_funding_unconfirmed { + let relevant_txids = nodes[0].node.get_relevant_txids(); + assert_eq!(&relevant_txids[..], &[chan.3.txid()]); + nodes[0].node.transaction_unconfirmed(&relevant_txids[0]); + } else { + disconnect_all_blocks(&nodes[0]); + } + if connect_style == ConnectStyle::FullBlockViaListen && !use_funding_unconfirmed { + handle_announce_close_broadcast_events(&nodes, 0, 1, true, "Funding transaction was un-confirmed. Locked at 6 confs, now have 2 confs."); + } else { + handle_announce_close_broadcast_events(&nodes, 0, 1, true, "Funding transaction was un-confirmed. Locked at 6 confs, now have 0 confs."); + } + check_added_monitors!(nodes[1], 1); { let channel_state = nodes[0].node.channel_state.lock().unwrap(); assert_eq!(channel_state.by_id.len(), 0); @@ -267,25 +293,44 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, connect_styl } // With expect_channel_force_closed set the TestChainMonitor will enforce that the next update // is a ChannelForcClosed on the right channel with should_broadcast set. - *nodes[0].chain_monitor.expect_channel_force_closed.lock().unwrap() = Some((chan_id, true)); + *nodes[0].chain_monitor.expect_channel_force_closed.lock().unwrap() = Some((chan.2, true)); nodes[0].node.test_process_background_events(); // Required to free the pending background monitor update check_added_monitors!(nodes[0], 1); + assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1); + nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clear(); + + // Now check that we can create a new channel + create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); + send_payment(&nodes[0], &[&nodes[1]], 8000000, 8_000_000); } #[test] fn test_unconf_chan() { - do_test_unconf_chan(true, true, ConnectStyle::BestBlockFirstSkippingBlocks); - do_test_unconf_chan(false, true, ConnectStyle::BestBlockFirstSkippingBlocks); - do_test_unconf_chan(true, false, ConnectStyle::BestBlockFirstSkippingBlocks); - do_test_unconf_chan(false, false, ConnectStyle::BestBlockFirstSkippingBlocks); + do_test_unconf_chan(true, true, false, ConnectStyle::BestBlockFirstSkippingBlocks); + do_test_unconf_chan(false, true, false, ConnectStyle::BestBlockFirstSkippingBlocks); + do_test_unconf_chan(true, false, false, ConnectStyle::BestBlockFirstSkippingBlocks); + do_test_unconf_chan(false, false, false, ConnectStyle::BestBlockFirstSkippingBlocks); } #[test] fn test_unconf_chan_via_listen() { - do_test_unconf_chan(true, true, ConnectStyle::FullBlockViaListen); - do_test_unconf_chan(false, true, ConnectStyle::FullBlockViaListen); - do_test_unconf_chan(true, false, ConnectStyle::FullBlockViaListen); - do_test_unconf_chan(false, false, ConnectStyle::FullBlockViaListen); + do_test_unconf_chan(true, true, false, ConnectStyle::FullBlockViaListen); + do_test_unconf_chan(false, true, false, ConnectStyle::FullBlockViaListen); + do_test_unconf_chan(true, false, false, ConnectStyle::FullBlockViaListen); + do_test_unconf_chan(false, false, false, ConnectStyle::FullBlockViaListen); +} + +#[test] +fn test_unconf_chan_via_funding_unconfirmed() { + do_test_unconf_chan(true, true, true, ConnectStyle::BestBlockFirstSkippingBlocks); + do_test_unconf_chan(false, true, true, ConnectStyle::BestBlockFirstSkippingBlocks); + do_test_unconf_chan(true, false, true, ConnectStyle::BestBlockFirstSkippingBlocks); + do_test_unconf_chan(false, false, true, ConnectStyle::BestBlockFirstSkippingBlocks); + + do_test_unconf_chan(true, true, true, ConnectStyle::FullBlockViaListen); + do_test_unconf_chan(false, true, true, ConnectStyle::FullBlockViaListen); + do_test_unconf_chan(true, false, true, ConnectStyle::FullBlockViaListen); + do_test_unconf_chan(false, false, true, ConnectStyle::FullBlockViaListen); } #[test]