From 4b3afdd8d295330003250199bdf673b8a39acd3a Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 3 Jul 2019 11:15:12 -0400 Subject: [PATCH 01/10] Add log_trace on to_remote/to_local inclusion in commitment tx --- src/ln/channel.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ln/channel.rs b/src/ln/channel.rs index 7b32e16e1dd..698cb419daf 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -913,6 +913,7 @@ impl Channel { let value_to_b = if local { value_to_remote } else { value_to_self }; if value_to_a >= (dust_limit_satoshis as i64) { + log_trace!(self, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a); txouts.push((TxOut { script_pubkey: chan_utils::get_revokeable_redeemscript(&keys.revocation_key, if local { self.their_to_self_delay } else { BREAKDOWN_TIMEOUT }, @@ -922,6 +923,7 @@ impl Channel { } if value_to_b >= (dust_limit_satoshis as i64) { + log_trace!(self, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b); txouts.push((TxOut { script_pubkey: Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0) .push_slice(&Hash160::hash(&keys.b_payment_key.serialize())[..]) From 72c5423fd5eca2221641687df57ddf7fe505f756 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Thu, 30 May 2019 20:54:02 -0400 Subject: [PATCH 02/10] Track HTLC-failure trigger tx until anti-reorg delay reached Broadcasting a commitment tx means that we have to fail inbound HTLC in backward channel. Doing it prematurely would put us at risk in case of reorg. So we delay passing failure update upstream until solving tx mature to HTLC_FAIL_ANTI_ REORG_DELAY. Requirements differ if HTLC is a revoked/non-revoked dust/ non-revoked non-dust one. Add connect_blocks in test_utils to fix broken tests due to anti-reorg delay enforcement Remove anti-duplicate htlc update stuff in ManySimpleChannelMonitor --- src/ln/channelmonitor.rs | 105 ++++++++++++++++++++++---------- src/ln/functional_test_utils.rs | 10 +++ src/ln/functional_tests.rs | 13 +++- 3 files changed, 95 insertions(+), 33 deletions(-) diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 37fcf2db223..178760e1043 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -178,10 +178,6 @@ impl ChainListener for SimpleManyChannelMonit // In case of reorg we may have htlc outputs solved in a different way so // we prefer to keep claims but don't store duplicate updates for a given // (payment_hash, HTLCSource) pair. - // TODO: Note that we currently don't really use this as ChannelManager - // will fail/claim backwards after the first block. We really should delay - // a few blocks before failing backwards (but can claim backwards - // immediately) as long as we have a few blocks of headroom. let mut existing_claim = false; e.get_mut().retain(|htlc_data| { if htlc.0 == htlc_data.0 { @@ -306,7 +302,6 @@ pub(crate) const HTLC_FAIL_TIMEOUT_BLOCKS: u32 = 3; /// Number of blocks we wait on seeing a confirmed HTLC-Timeout or previous revoked commitment /// transaction before we fail corresponding inbound HTLCs. This prevents us from failing backwards /// and then getting a reorg resulting in us losing money. -//TODO: We currently don't actually use this...we should pub(crate) const HTLC_FAIL_ANTI_REORG_DELAY: u32 = 6; #[derive(Clone, PartialEq)] @@ -401,6 +396,8 @@ pub struct ChannelMonitor { destination_script: Script, + htlc_updated_waiting_threshold_conf: HashMap, PaymentHash)>>, + // We simply modify last_block_hash in Channel's block_connected so that serialization is // consistent but hopefully the users' copy handles block_connected in a consistent way. // (we do *not*, however, update them in insert_combine to ensure any local user copies keep @@ -462,7 +459,8 @@ impl PartialEq for ChannelMonitor { self.current_remote_commitment_number != other.current_remote_commitment_number || self.current_local_signed_commitment_tx != other.current_local_signed_commitment_tx || self.payment_preimages != other.payment_preimages || - self.destination_script != other.destination_script + self.destination_script != other.destination_script || + self.htlc_updated_waiting_threshold_conf != other.htlc_updated_waiting_threshold_conf { false } else { @@ -512,6 +510,8 @@ impl ChannelMonitor { payment_preimages: HashMap::new(), destination_script: destination_script, + htlc_updated_waiting_threshold_conf: HashMap::new(), + last_block_hash: Default::default(), secp_ctx: Secp256k1::new(), logger, @@ -1019,6 +1019,17 @@ impl ChannelMonitor { self.last_block_hash.write(writer)?; self.destination_script.write(writer)?; + writer.write_all(&byte_utils::be64_to_array(self.htlc_updated_waiting_threshold_conf.len() as u64))?; + for (ref target, ref updates) in self.htlc_updated_waiting_threshold_conf.iter() { + writer.write_all(&byte_utils::be32_to_array(**target))?; + writer.write_all(&byte_utils::be64_to_array(updates.len() as u64))?; + for ref update in updates.iter() { + update.0.write(writer)?; + update.1.write(writer)?; + update.2.write(writer)?; + } + } + Ok(()) } @@ -1082,13 +1093,12 @@ impl ChannelMonitor { /// HTLC-Success/HTLC-Timeout transactions. /// Return updates for HTLC pending in the channel and failed automatically by the broadcast of /// revoked remote commitment tx - fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32, fee_estimator: &FeeEstimator) -> (Vec, (Sha256dHash, Vec), Vec, Vec<(HTLCSource, Option, PaymentHash)>) { + fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32, fee_estimator: &FeeEstimator) -> (Vec, (Sha256dHash, Vec), Vec) { // 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 let mut txn_to_broadcast = Vec::new(); let mut watch_outputs = Vec::new(); let mut spendable_outputs = Vec::new(); - let mut htlc_updated = Vec::new(); let commitment_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers! let per_commitment_option = self.remote_claimable_outpoints.get(&commitment_txid); @@ -1097,7 +1107,7 @@ impl ChannelMonitor { ( $thing : expr ) => { match $thing { Ok(a) => a, - Err(_) => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated) + Err(_) => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs) } }; } @@ -1122,7 +1132,7 @@ impl ChannelMonitor { }; let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.their_delayed_payment_base_key.unwrap())); let a_htlc_key = match self.their_htlc_base_key { - None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated), + None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs), Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &their_htlc_base_key)), }; @@ -1204,7 +1214,7 @@ impl ChannelMonitor { if transaction_output_index as usize >= tx.output.len() || tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 || tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() { - return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); // Corrupted per_commitment_data, fuck this user + return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); // Corrupted per_commitment_data, fuck this user } let input = TxIn { previous_output: BitcoinOutPoint { @@ -1249,16 +1259,22 @@ impl ChannelMonitor { watch_outputs.append(&mut tx.output.clone()); self.remote_commitment_txn_on_chain.insert(commitment_txid, (commitment_number, tx.output.iter().map(|output| { output.script_pubkey.clone() }).collect())); - // TODO: We really should only fail backwards after our revocation claims have been - // confirmed, but we also need to do more other tracking of in-flight pre-confirm - // on-chain claims, so we can do that at the same time. macro_rules! check_htlc_fails { ($txid: expr, $commitment_tx: expr) => { if let Some(ref outpoints) = self.remote_claimable_outpoints.get($txid) { for &(ref htlc, ref source_option) in outpoints.iter() { if let &Some(ref source) = source_option { - log_trace!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of revoked remote commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx); - htlc_updated.push(((**source).clone(), None, htlc.payment_hash.clone())); + log_info!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of revoked remote commitment transaction, waiting for confirmation (at height {})", log_bytes!(htlc.payment_hash.0), $commitment_tx, height + HTLC_FAIL_ANTI_REORG_DELAY - 1); + match self.htlc_updated_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + hash_map::Entry::Occupied(mut entry) => { + let e = entry.get_mut(); + e.retain(|ref update| update.0 != **source); + e.push(((**source).clone(), None, htlc.payment_hash.clone())); + } + hash_map::Entry::Vacant(entry) => { + entry.insert(vec![((**source).clone(), None, htlc.payment_hash.clone())]); + } + } } } } @@ -1274,7 +1290,7 @@ impl ChannelMonitor { } // No need to check local commitment txn, symmetric HTLCSource must be present as per-htlc data on remote commitment tx } - if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); } // Nothing to be done...probably a false positive/local tx + if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); } // Nothing to be done...probably a false positive/local tx let outputs = vec!(TxOut { script_pubkey: self.destination_script.clone(), @@ -1289,7 +1305,7 @@ impl ChannelMonitor { let predicted_weight = spend_tx.get_weight() + Self::get_witnesses_weight(&input_descriptors[..]); if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid()) { - return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); + return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); } let mut values_drain = values.drain(..); @@ -1319,9 +1335,6 @@ impl ChannelMonitor { log_trace!(self, "Got broadcast of non-revoked remote commitment transaction {}", commitment_txid); - // TODO: We really should only fail backwards after our revocation claims have been - // confirmed, but we also need to do more other tracking of in-flight pre-confirm - // on-chain claims, so we can do that at the same time. macro_rules! check_htlc_fails { ($txid: expr, $commitment_tx: expr, $id: tt) => { if let Some(ref latest_outpoints) = self.remote_claimable_outpoints.get($txid) { @@ -1342,7 +1355,16 @@ impl ChannelMonitor { } } log_trace!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of remote commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx); - htlc_updated.push(((**source).clone(), None, htlc.payment_hash.clone())); + match self.htlc_updated_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + hash_map::Entry::Occupied(mut entry) => { + let e = entry.get_mut(); + e.retain(|ref update| update.0 != **source); + e.push(((**source).clone(), None, htlc.payment_hash.clone())); + } + hash_map::Entry::Vacant(entry) => { + entry.insert(vec![((**source).clone(), None, htlc.payment_hash.clone())]); + } + } } } } @@ -1375,7 +1397,7 @@ impl ChannelMonitor { }, }; let a_htlc_key = match self.their_htlc_base_key { - None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated), + None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs), Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &their_htlc_base_key)), }; @@ -1431,7 +1453,7 @@ impl ChannelMonitor { if transaction_output_index as usize >= tx.output.len() || tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 || tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() { - return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); // Corrupted per_commitment_data, fuck this user + return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); // Corrupted per_commitment_data, fuck this user } if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) { let input = TxIn { @@ -1499,7 +1521,7 @@ impl ChannelMonitor { } } - if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); } // Nothing to be done...probably a false positive/local tx + if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); } // Nothing to be done...probably a false positive/local tx let outputs = vec!(TxOut { script_pubkey: self.destination_script.clone(), @@ -1513,7 +1535,7 @@ impl ChannelMonitor { }; let predicted_weight = spend_tx.get_weight() + Self::get_witnesses_weight(&input_descriptors[..]); if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid()) { - return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); + return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); } let mut values_drain = values.drain(..); @@ -1534,7 +1556,7 @@ impl ChannelMonitor { } } - (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated) + (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs) } /// Attempts to claim a remote HTLC-Success/HTLC-Timeout's outputs using the revocation key @@ -1811,7 +1833,7 @@ impl ChannelMonitor { } }; if funding_txo.is_none() || (prevout.txid == funding_txo.as_ref().unwrap().0.txid && prevout.vout == funding_txo.as_ref().unwrap().0.index as u32) { - let (remote_txn, new_outputs, mut spendable_output, mut updated) = self.check_spend_remote_transaction(tx, height, fee_estimator); + let (remote_txn, new_outputs, mut spendable_output) = self.check_spend_remote_transaction(tx, height, fee_estimator); txn = remote_txn; spendable_outputs.append(&mut spendable_output); if !new_outputs.1.is_empty() { @@ -1830,9 +1852,6 @@ impl ChannelMonitor { spendable_outputs.push(spendable_output); } } - if updated.len() > 0 { - htlc_updated.append(&mut updated); - } } else { if let Some(&(commitment_number, _)) = self.remote_commitment_txn_on_chain.get(&prevout.txid) { let (tx, spendable_output) = self.check_spend_remote_htlc(tx, commitment_number, fee_estimator); @@ -1883,6 +1902,12 @@ impl ChannelMonitor { } } } + if let Some(updates) = self.htlc_updated_waiting_threshold_conf.remove(&height) { + for update in updates { + log_trace!(self, "HTLC {} failure update has get enough confirmation to be pass upstream", log_bytes!((update.2).0)); + htlc_updated.push(update); + } + } self.last_block_hash = block_hash.clone(); (watch_outputs, spendable_outputs, htlc_updated) } @@ -2283,6 +2308,21 @@ impl ReadableArgs> for (Sha256dHash, ChannelM let last_block_hash: Sha256dHash = Readable::read(reader)?; let destination_script = Readable::read(reader)?; + let waiting_threshold_conf_len: u64 = Readable::read(reader)?; + let mut htlc_updated_waiting_threshold_conf = HashMap::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128)); + for _ in 0..waiting_threshold_conf_len { + let height_target = Readable::read(reader)?; + let updates_len: u64 = Readable::read(reader)?; + let mut updates = Vec::with_capacity(cmp::min(updates_len as usize, MAX_ALLOC_SIZE / 128)); + for _ in 0..updates_len { + let htlc_source = Readable::read(reader)?; + let preimage = Readable::read(reader)?; + let hash = Readable::read(reader)?; + updates.push((htlc_source, preimage, hash)); + } + htlc_updated_waiting_threshold_conf.insert(height_target, updates); + } + Ok((last_block_hash.clone(), ChannelMonitor { commitment_transaction_number_obscure_factor, @@ -2306,6 +2346,9 @@ impl ReadableArgs> for (Sha256dHash, ChannelM payment_preimages, destination_script, + + htlc_updated_waiting_threshold_conf, + last_block_hash, secp_ctx, logger, diff --git a/src/ln/functional_test_utils.rs b/src/ln/functional_test_utils.rs index 9a24d503cee..b8f5e5835a3 100644 --- a/src/ln/functional_test_utils.rs +++ b/src/ln/functional_test_utils.rs @@ -20,6 +20,7 @@ use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::network::constants::Network; use bitcoin_hashes::sha256::Hash as Sha256; +use bitcoin_hashes::sha256d::Hash as Sha256d; use bitcoin_hashes::Hash; use secp256k1::Secp256k1; @@ -46,6 +47,15 @@ pub fn confirm_transaction(chain: &chaininterface::ChainWatchInterfaceUtil, tx: } } +pub fn connect_blocks(chain: &chaininterface::ChainWatchInterfaceUtil, depth: u32, height: u32, parent: bool, prev_blockhash: Sha256d) { + let mut header = BlockHeader { version: 0x2000000, prev_blockhash: if parent { prev_blockhash } else { Default::default() }, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + chain.block_connected_checked(&header, height + 1, &Vec::new(), &Vec::new()); + for i in 2..depth + 1 { + header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + chain.block_connected_checked(&header, height + i, &Vec::new(), &Vec::new()); + } +} + pub struct Node { pub chain_monitor: Arc, pub tx_broadcaster: Arc, diff --git a/src/ln/functional_tests.rs b/src/ln/functional_tests.rs index 00225f18bc9..6d1209d996e 100644 --- a/src/ln/functional_tests.rs +++ b/src/ln/functional_tests.rs @@ -8,7 +8,7 @@ use chain::keysinterface::{KeysInterface, SpendableOutputDescriptor}; use chain::keysinterface; use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC, BREAKDOWN_TIMEOUT}; use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,HTLCForwardInfo,RAACommitmentOrder, PaymentPreimage, PaymentHash}; -use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, ManyChannelMonitor}; +use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, ManyChannelMonitor, HTLC_FAIL_ANTI_REORG_DELAY}; use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT}; use ln::onion_utils; use ln::router::{Route, RouteHop}; @@ -1871,6 +1871,7 @@ fn claim_htlc_outputs_shared_tx() { let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); + connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); @@ -1938,6 +1939,7 @@ fn claim_htlc_outputs_single_tx() { let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200); nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200); + connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 200, true, header.bitcoin_hash()); let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); @@ -1949,7 +1951,7 @@ fn claim_htlc_outputs_single_tx() { } let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(node_txn.len(), 12); // ChannelManager : 2, ChannelMontitor: 8 (1 standard revoked output, 2 revocation htlc tx, 1 local commitment tx + 1 htlc timeout tx) * 2 (block-rescan) + assert_eq!(node_txn.len(), 22); // ChannelManager : 2, ChannelMontitor: 8 (1 standard revoked output, 2 revocation htlc tx, 1 local commitment tx + 1 htlc timeout tx) * 2 (block-rescan) + 5 * (1 local commitment tx + 1 htlc timeout tx) assert_eq!(node_txn[0], node_txn[7]); assert_eq!(node_txn[1], node_txn[8]); @@ -1959,6 +1961,10 @@ fn claim_htlc_outputs_single_tx() { assert_eq!(node_txn[3], node_txn[5]); //local commitment tx + htlc timeout tx broadcasted by ChannelManger assert_eq!(node_txn[4], node_txn[6]); + for i in 12..22 { + if i % 2 == 0 { assert_eq!(node_txn[3], node_txn[i]); } else { assert_eq!(node_txn[4], node_txn[i]); } + } + assert_eq!(node_txn[0].input.len(), 1); assert_eq!(node_txn[1].input.len(), 1); assert_eq!(node_txn[2].input.len(), 1); @@ -2293,6 +2299,7 @@ fn test_simple_commitment_revoked_fail_backward() { let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42}; nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); + connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); check_added_monitors!(nodes[1], 0); check_closed_broadcast!(nodes[1]); @@ -2445,6 +2452,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42}; nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); + connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), if deliver_bs_raa { 1 } else { 2 }); @@ -4106,6 +4114,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno } else { nodes[2].chain_monitor.block_connected_checked(&header, 1, &[&ds_prev_commitment_tx[0]], &[1; 1]); } + connect_blocks(&nodes[2].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); check_closed_broadcast!(nodes[2]); expect_pending_htlcs_forwardable!(nodes[2]); check_added_monitors!(nodes[2], 2); From 587af43eca7dc254bf0feb96d4732fd56cf6fcc3 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Mon, 4 Feb 2019 21:21:11 -0500 Subject: [PATCH 03/10] Implement block_disconnect for pruning of waiting-conf HTLC updates Modify ChainListener API by adding height field to block_disconnect --- fuzz/fuzz_targets/full_stack_target.rs | 4 ++-- src/chain/chaininterface.rs | 7 ++++--- src/ln/channelmanager.rs | 2 +- src/ln/channelmonitor.rs | 15 ++++++++++++++- src/ln/functional_tests.rs | 4 +++- 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/fuzz/fuzz_targets/full_stack_target.rs b/fuzz/fuzz_targets/full_stack_target.rs index 16bebfbd01b..8737f6c262b 100644 --- a/fuzz/fuzz_targets/full_stack_target.rs +++ b/fuzz/fuzz_targets/full_stack_target.rs @@ -209,8 +209,8 @@ impl<'a> MoneyLossDetector<'a> { if self.height > 0 && (self.max_height < 6 || self.height >= self.max_height - 6) { self.height -= 1; let header = BlockHeader { version: 0x20000000, prev_blockhash: self.header_hashes[self.height], merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - self.manager.block_disconnected(&header); - self.monitor.block_disconnected(&header); + self.manager.block_disconnected(&header, self.height as u32); + self.monitor.block_disconnected(&header, self.height as u32); let removal_height = self.height; self.txids_confirmed.retain(|_, height| { removal_height != *height diff --git a/src/chain/chaininterface.rs b/src/chain/chaininterface.rs index d1995b44221..c0330fb2963 100644 --- a/src/chain/chaininterface.rs +++ b/src/chain/chaininterface.rs @@ -78,7 +78,8 @@ pub trait ChainListener: Sync + Send { fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]); /// Notifies a listener that a block was disconnected. /// Unlike block_connected, this *must* never be called twice for the same disconnect event. - fn block_disconnected(&self, header: &BlockHeader); + /// Height must be the one of the block which was disconnected (not new height of the best chain) + fn block_disconnected(&self, header: &BlockHeader, disconnected_height: u32); } /// An enum that represents the speed at which we want a transaction to confirm used for feerate @@ -279,11 +280,11 @@ impl ChainWatchInterfaceUtil { } /// Notify listeners that a block was disconnected. - pub fn block_disconnected(&self, header: &BlockHeader) { + pub fn block_disconnected(&self, header: &BlockHeader, disconnected_height: u32) { let listeners = self.listeners.lock().unwrap().clone(); for listener in listeners.iter() { match listener.upgrade() { - Some(arc) => arc.block_disconnected(header), + Some(arc) => arc.block_disconnected(&header, disconnected_height), None => () } } diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index 3ffd080e915..201865d6738 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -2474,7 +2474,7 @@ impl ChainListener for ChannelManager { } /// We force-close the channel without letting our counterparty participate in the shutdown - fn block_disconnected(&self, header: &BlockHeader) { + fn block_disconnected(&self, header: &BlockHeader, _: u32) { let _ = self.total_consistency_lock.read().unwrap(); let mut failed_channels = Vec::new(); { diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 178760e1043..379ae31c3eb 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -201,7 +201,13 @@ impl ChainListener for SimpleManyChannelMonit pending_events.append(&mut new_events); } - fn block_disconnected(&self, _: &BlockHeader) { } + fn block_disconnected(&self, header: &BlockHeader, disconnected_height: u32) { + let block_hash = header.bitcoin_hash(); + let mut monitors = self.monitors.lock().unwrap(); + for monitor in monitors.values_mut() { + monitor.block_disconnected(disconnected_height, &block_hash); + } + } } impl SimpleManyChannelMonitor { @@ -1912,6 +1918,13 @@ impl ChannelMonitor { (watch_outputs, spendable_outputs, htlc_updated) } + fn block_disconnected(&mut self, height: u32, block_hash: &Sha256dHash) { + if let Some(_) = self.htlc_updated_waiting_threshold_conf.remove(&(height + HTLC_FAIL_ANTI_REORG_DELAY - 1)) { + //We discard htlc update there as failure-trigger tx (revoked commitment tx, non-revoked commitment tx, HTLC-timeout tx) has been disconnected + } + self.last_block_hash = block_hash.clone(); + } + pub(super) fn would_broadcast_at_height(&self, height: u32) -> bool { // We need to consider all HTLCs which are: // * in any unrevoked remote commitment transaction, as they could broadcast said diff --git a/src/ln/functional_tests.rs b/src/ln/functional_tests.rs index 6d1209d996e..ec3304b399a 100644 --- a/src/ln/functional_tests.rs +++ b/src/ln/functional_tests.rs @@ -2674,8 +2674,10 @@ fn test_unconf_chan() { header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; headers.push(header.clone()); } + let mut height = 99; while !headers.is_empty() { - nodes[0].node.block_disconnected(&headers.pop().unwrap()); + nodes[0].node.block_disconnected(&headers.pop().unwrap(), height); + height -= 1; } check_closed_broadcast!(nodes[0]); let channel_state = nodes[0].node.channel_state.lock().unwrap(); From a2b6a76e59abd1054b3c49b848fd72fb462ee67c Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 6 Feb 2019 20:02:38 -0500 Subject: [PATCH 04/10] Delay failure of non-dust HTLC-outputs until solving timeout tx matures Fix tests broken by introduced change --- fuzz/fuzz_targets/full_stack_target.rs | 9 +++++++-- src/ln/channelmonitor.rs | 16 +++++++++++++--- src/ln/functional_tests.rs | 2 ++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/fuzz/fuzz_targets/full_stack_target.rs b/fuzz/fuzz_targets/full_stack_target.rs index 8737f6c262b..f2b24a1a733 100644 --- a/fuzz/fuzz_targets/full_stack_target.rs +++ b/fuzz/fuzz_targets/full_stack_target.rs @@ -881,13 +881,18 @@ mod tests { // 02000000013f00000000000000000000000000000000000000000000000000000000000000000000000000000080020001000000000000220020e2000000000000000000000000000000000000000000000000000000000000006cc10000000000001600142e0000000000000000000000000000000000000005000020 - the commitment transaction for channel 3f00000000000000000000000000000000000000000000000000000000000000 // 00fd - A feerate request (returning min feerate, which our open_channel also uses) (gonna be ingested by FuzzEstimator) // 0c005e - connect a block with one transaction of len 94 - // 0200000001fb00000000000000000000000000000000000000000000000000000000000000000000000000000000014f00000000000000220020f60000000000000000000000000000000000000000000000000000000000000000000000 - the commitment transaction for channel 3d00000000000000000000000000000000000000000000000000000000000000 + // 0200000001fb00000000000000000000000000000000000000000000000000000000000000000000000000000000014f00000000000000220020f60000000000000000000000000000000000000000000000000000000000000000000000 - the funding transaction + // 0c0000 - connect a block with no transactions + // 0c0000 - connect a block with no transactions + // 0c0000 - connect a block with no transactions + // 0c0000 - connect a block with no transactions + // 0c0000 - connect a block with no transactions // // 07 - process the now-pending HTLC forward // - client now fails the HTLC backwards as it was unable to extract the payment preimage (CHECK 9 duplicate and CHECK 10) let logger = Arc::new(TrackingLogger { lines: Mutex::new(HashMap::new()) }); - super::do_test(&::hex::decode("00000000000000000000000000000000000000000000000000000000000000000000000001000300000000000000000000000000000000000000000000000000000000000000000300320003000000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000030012000603000000000000000000000000000000030016001000000000030000000000000000000000000000000300120141030000000000000000000000000000000300fe00207500000000000000000000000000000000000000000000000000000000000000ff4f00f805273c1b203bb5ebf8436bfde57b3be8c2f5e95d9491dbb181909679000000000000c35000000000000000000000000000000222ffffffffffffffff00000000000002220000000000000000000000fd000601e3030000000000000000000000000000000000000000000000000000000000000001030000000000000000000000000000000000000000000000000000000000000002030000000000000000000000000000000000000000000000000000000000000003030000000000000000000000000000000000000000000000000000000000000004030053030000000000000000000000000000000000000000000000000000000000000005030000000000000000000000000000000000000000000000000000000000000000010300000000000000000000000000000000fd00fd00fd0300120084030000000000000000000000000000000300940022ff4f00f805273c1b203bb5ebf8436bfde57b3be8c2f5e95d9491dbb1819096793d0000000000000000000000000000000000000000000000000000000000000000005c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001030000000000000000000000000000000c005e020000000100000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0150c3000000000000220020ae00000000000000000000000000000000000000000000000000000000000000000000000c00000c00000c00000c00000c00000c00000c00000c00000c00000c00000c00000c000003001200430300000000000000000000000000000003005300243d000000000000000000000000000000000000000000000000000000000000000301000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000001030132000300000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000003014200030200000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000300000000000000000000000000000003011200060100000000000000000000000000000003011600100000000001000000000000000000000000000000050103020000000000000000000000000000000000000000000000000000000000000000c3500003e800fd00fd00fd0301120110010000000000000000000000000000000301ff00210200000000000000020000000000000002000000000000000200000000000000000000000000001a00000000004c4b4000000000000003e800000000000003e80000000203f00005030000000000000000000000000000000000000000000000000000000000000100030000000000000000000000000000000000000000000000000000000000000200030000000000000000000000000000000000000000000000000000000000000300030000000000000000000000000000000000000000000000000000000000000400030000000000000000000000000000000000000000000000000000000000000500030000000000000000000000000000000301210000000000000000000000000000000000010000000000000000000000000000000a03011200620100000000000000000000000000000003017200233f00000000000000000000000000000000000000000000000000000000000000f6000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100010000000000000000000000000000000b03011200430100000000000000000000000000000003015300243f000000000000000000000000000000000000000000000000000000000000000301000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000003001205ac030000000000000000000000000000000300ff00803d0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003e80ff0000000000000000000000000000000000000000000000000000000000000000000121000300000000000000000000000000000000000000000000000000000000000005550000000e000001000000000000000003e8000000010000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300c1ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffef000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000fd03001200640300000000000000000000000000000003007400843d000000000000000000000000000000000000000000000000000000000000004d00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000300000000000000000000000000000003001200630300000000000000000000000000000003007300853d000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000030200000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000703011200640100000000000000000000000000000003017400843f00000000000000000000000000000000000000000000000000000000000000f700000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000100000000000000000000000000000003011200630100000000000000000000000000000003017300853f00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003020000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000030112004a0100000000000000000000000000000003015a00823f000000000000000000000000000000000000000000000000000000000000000000000000000000ff008888888888888888888888888888888888888888888888888888888888880100000000000000000000000000000003011200640100000000000000000000000000000003017400843f00000000000000000000000000000000000000000000000000000000000000fb00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000100000000000000000000000000000003011200630100000000000000000000000000000003017300853f0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000303000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000003001205ac030000000000000000000000000000000300ff00803d0000000000000000000000000000000000000000000000000000000000000000000000000000010000000000003e80ff0000000000000000000000000000000000000000000000000000000000000000000121000300000000000000000000000000000000000000000000000000000000000005550000000e000001000000000000000003e8000000010000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300c1ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffef000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000fd03001200630300000000000000000000000000000003007300853d0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000303000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000003001200640300000000000000000000000000000003007400843d00000000000000000000000000000000000000000000000000000000000000be00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000300000000000000000000000000000003001200630300000000000000000000000000000003007300853d000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000030400000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000703011200640100000000000000000000000000000003017400843f00000000000000000000000000000000000000000000000000000000000000fa00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000100000000000000000000000000000003011200630100000000000000000000000000000003017300853f00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003040000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000030112002c0100000000000000000000000000000003013c00833f00000000000000000000000000000000000000000000000000000000000000000000000000000100000100000000000000000000000000000003011200640100000000000000000000000000000003017400843f00000000000000000000000000000000000000000000000000000000000000fd00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000100000000000000000000000000000003011200630100000000000000000000000000000003017300853f000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000000000000000000000000000000000030500000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000703001200630300000000000000000000000000000003007300853d0000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000000305000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000003001200640300000000000000000000000000000003007400843d000000000000000000000000000000000000000000000000000000000000004f00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000300000000000000000000000000000003001205ac030000000000000000000000000000000300ff00803d00000000000000000000000000000000000000000000000000000000000000000000000000000200000000000b0838ff0000000000000000000000000000000000000000000000000000000000000000000121000300000000000000000000000000000000000000000000000000000000000005550000000e0000010000000000000003e800000000010000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300c1ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffef000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000fd03001200a4030000000000000000000000000000000300b400843d00000000000000000000000000000000000000000000000000000000000000070000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010001c8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f000000000000000300000000000000000000000000000003001200630300000000000000000000000000000003007300853d00000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000003060000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000070c007d02000000013f00000000000000000000000000000000000000000000000000000000000000000000000000000080020001000000000000220020e2000000000000000000000000000000000000000000000000000000000000006cc10000000000001600142e000000000000000000000000000000000000000500002000fd0c005e0200000001fb00000000000000000000000000000000000000000000000000000000000000000000000000000000014f00000000000000220020f6000000000000000000000000000000000000000000000000000000000000000000000007").unwrap(), &(Arc::clone(&logger) as Arc)); + super::do_test(&::hex::decode("00000000000000000000000000000000000000000000000000000000000000000000000001000300000000000000000000000000000000000000000000000000000000000000000300320003000000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000030012000603000000000000000000000000000000030016001000000000030000000000000000000000000000000300120141030000000000000000000000000000000300fe00207500000000000000000000000000000000000000000000000000000000000000ff4f00f805273c1b203bb5ebf8436bfde57b3be8c2f5e95d9491dbb181909679000000000000c35000000000000000000000000000000222ffffffffffffffff00000000000002220000000000000000000000fd000601e3030000000000000000000000000000000000000000000000000000000000000001030000000000000000000000000000000000000000000000000000000000000002030000000000000000000000000000000000000000000000000000000000000003030000000000000000000000000000000000000000000000000000000000000004030053030000000000000000000000000000000000000000000000000000000000000005030000000000000000000000000000000000000000000000000000000000000000010300000000000000000000000000000000fd00fd00fd0300120084030000000000000000000000000000000300940022ff4f00f805273c1b203bb5ebf8436bfde57b3be8c2f5e95d9491dbb1819096793d0000000000000000000000000000000000000000000000000000000000000000005c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001030000000000000000000000000000000c005e020000000100000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0150c3000000000000220020ae00000000000000000000000000000000000000000000000000000000000000000000000c00000c00000c00000c00000c00000c00000c00000c00000c00000c00000c00000c000003001200430300000000000000000000000000000003005300243d000000000000000000000000000000000000000000000000000000000000000301000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000001030132000300000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000003014200030200000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000300000000000000000000000000000003011200060100000000000000000000000000000003011600100000000001000000000000000000000000000000050103020000000000000000000000000000000000000000000000000000000000000000c3500003e800fd00fd00fd0301120110010000000000000000000000000000000301ff00210200000000000000020000000000000002000000000000000200000000000000000000000000001a00000000004c4b4000000000000003e800000000000003e80000000203f00005030000000000000000000000000000000000000000000000000000000000000100030000000000000000000000000000000000000000000000000000000000000200030000000000000000000000000000000000000000000000000000000000000300030000000000000000000000000000000000000000000000000000000000000400030000000000000000000000000000000000000000000000000000000000000500030000000000000000000000000000000301210000000000000000000000000000000000010000000000000000000000000000000a03011200620100000000000000000000000000000003017200233f00000000000000000000000000000000000000000000000000000000000000f6000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100010000000000000000000000000000000b03011200430100000000000000000000000000000003015300243f000000000000000000000000000000000000000000000000000000000000000301000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000003001205ac030000000000000000000000000000000300ff00803d0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003e80ff0000000000000000000000000000000000000000000000000000000000000000000121000300000000000000000000000000000000000000000000000000000000000005550000000e000001000000000000000003e8000000010000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300c1ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffef000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000fd03001200640300000000000000000000000000000003007400843d000000000000000000000000000000000000000000000000000000000000004d00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000300000000000000000000000000000003001200630300000000000000000000000000000003007300853d000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000030200000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000703011200640100000000000000000000000000000003017400843f00000000000000000000000000000000000000000000000000000000000000f700000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000100000000000000000000000000000003011200630100000000000000000000000000000003017300853f00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003020000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000030112004a0100000000000000000000000000000003015a00823f000000000000000000000000000000000000000000000000000000000000000000000000000000ff008888888888888888888888888888888888888888888888888888888888880100000000000000000000000000000003011200640100000000000000000000000000000003017400843f00000000000000000000000000000000000000000000000000000000000000fb00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000100000000000000000000000000000003011200630100000000000000000000000000000003017300853f0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000303000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000003001205ac030000000000000000000000000000000300ff00803d0000000000000000000000000000000000000000000000000000000000000000000000000000010000000000003e80ff0000000000000000000000000000000000000000000000000000000000000000000121000300000000000000000000000000000000000000000000000000000000000005550000000e000001000000000000000003e8000000010000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300c1ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffef000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000fd03001200630300000000000000000000000000000003007300853d0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000303000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000003001200640300000000000000000000000000000003007400843d00000000000000000000000000000000000000000000000000000000000000be00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000300000000000000000000000000000003001200630300000000000000000000000000000003007300853d000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000030400000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000703011200640100000000000000000000000000000003017400843f00000000000000000000000000000000000000000000000000000000000000fa00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000100000000000000000000000000000003011200630100000000000000000000000000000003017300853f00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003040000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000030112002c0100000000000000000000000000000003013c00833f00000000000000000000000000000000000000000000000000000000000000000000000000000100000100000000000000000000000000000003011200640100000000000000000000000000000003017400843f00000000000000000000000000000000000000000000000000000000000000fd00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000100000000000000000000000000000003011200630100000000000000000000000000000003017300853f000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000000000000000000000000000000000030500000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000703001200630300000000000000000000000000000003007300853d0000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000000305000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000003001200640300000000000000000000000000000003007400843d000000000000000000000000000000000000000000000000000000000000004f00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000300000000000000000000000000000003001205ac030000000000000000000000000000000300ff00803d00000000000000000000000000000000000000000000000000000000000000000000000000000200000000000b0838ff0000000000000000000000000000000000000000000000000000000000000000000121000300000000000000000000000000000000000000000000000000000000000005550000000e0000010000000000000003e800000000010000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0300c1ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffef000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000fd03001200a4030000000000000000000000000000000300b400843d00000000000000000000000000000000000000000000000000000000000000070000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010001c8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f000000000000000300000000000000000000000000000003001200630300000000000000000000000000000003007300853d00000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000003060000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000070c007d02000000013f00000000000000000000000000000000000000000000000000000000000000000000000000000080020001000000000000220020e2000000000000000000000000000000000000000000000000000000000000006cc10000000000001600142e000000000000000000000000000000000000000500002000fd0c005e0200000001fb00000000000000000000000000000000000000000000000000000000000000000000000000000000014f00000000000000220020f600000000000000000000000000000000000000000000000000000000000000000000000c00000c00000c00000c00000c000007").unwrap(), &(Arc::clone(&logger) as Arc)); let log_entries = logger.lines.lock().unwrap(); assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendAcceptChannel event in peer_handler for node 030000000000000000000000000000000000000000000000000000000000000000 for channel ff4f00f805273c1b203bb5ebf8436bfde57b3be8c2f5e95d9491dbb181909679".to_string())), Some(&1)); // 1 diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 379ae31c3eb..59dffa6e90b 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -1876,7 +1876,7 @@ impl ChannelMonitor { // While all commitment/HTLC-Success/HTLC-Timeout transactions have one input, HTLCs // can also be resolved in a few other ways which can have more than one output. Thus, // we call is_resolving_htlc_output here outside of the tx.input.len() == 1 check. - let mut updated = self.is_resolving_htlc_output(tx); + let mut updated = self.is_resolving_htlc_output(tx, height); if updated.len() > 0 { htlc_updated.append(&mut updated); } @@ -1994,7 +1994,7 @@ impl ChannelMonitor { /// Check if any transaction broadcasted is resolving HTLC output by a success or timeout on a local /// or remote commitment tx, if so send back the source, preimage if found and payment_hash of resolved HTLC - fn is_resolving_htlc_output(&mut self, tx: &Transaction) -> Vec<(HTLCSource, Option, PaymentHash)> { + fn is_resolving_htlc_output(&mut self, tx: &Transaction, height: u32) -> Vec<(HTLCSource, Option, PaymentHash)> { let mut htlc_updated = Vec::new(); 'outer_loop: for input in &tx.input { @@ -2101,7 +2101,17 @@ impl ChannelMonitor { payment_preimage.0.copy_from_slice(&input.witness[1]); htlc_updated.push((source, Some(payment_preimage), payment_hash)); } else { - htlc_updated.push((source, None, payment_hash)); + log_info!(self, "Failing HTLC with payment_hash {} timeout by a spend tx, waiting for confirmation (at height{})", log_bytes!(payment_hash.0), height + HTLC_FAIL_ANTI_REORG_DELAY - 1); + match self.htlc_updated_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + hash_map::Entry::Occupied(mut entry) => { + let e = entry.get_mut(); + e.retain(|ref update| update.0 != source); + e.push((source, None, payment_hash.clone())); + } + hash_map::Entry::Vacant(entry) => { + entry.insert(vec![(source, None, payment_hash)]); + } + } } } } diff --git a/src/ln/functional_tests.rs b/src/ln/functional_tests.rs index ec3304b399a..4b58e3b29e6 100644 --- a/src/ln/functional_tests.rs +++ b/src/ln/functional_tests.rs @@ -2241,6 +2241,7 @@ fn test_htlc_on_chain_timeout() { } nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![timeout_tx]}, 1); + connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); check_added_monitors!(nodes[1], 0); check_closed_broadcast!(nodes[1]); @@ -3898,6 +3899,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() { check_spends!(htlc_success_txn[1], commitment_txn[0].clone()); nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![htlc_timeout_tx] }, 200); + connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 200, true, header.bitcoin_hash()); expect_pending_htlcs_forwardable!(nodes[1]); let htlc_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); assert!(htlc_updates.update_add_htlcs.is_empty()); From 273f2fc14ad4ce4dc9f09559289afdf2e17c1b92 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 6 Feb 2019 20:28:55 -0500 Subject: [PATCH 05/10] Fail back dust HTLC of local commitment tx after enough confirmations Add test_failure_delay_htlc_local_commitment and test_no_failure_dust_htlc_local_commitment Move some bits of check_spend_remote as we need to fail dust HTLCs which can be spread on both prev/lastest local commitment tx --- src/ln/channelmonitor.rs | 74 +++++++++++++++---- src/ln/functional_tests.rs | 142 ++++++++++++++++++++++++++++++++++++- 2 files changed, 202 insertions(+), 14 deletions(-) diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 59dffa6e90b..bb6b3632321 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -1738,42 +1738,90 @@ impl ChannelMonitor { /// Attempts to claim any claimable HTLCs in a commitment transaction which was not (yet) /// revoked using data in local_claimable_outpoints. /// Should not be used if check_spend_revoked_transaction succeeds. - fn check_spend_local_transaction(&self, tx: &Transaction, _height: u32) -> (Vec, Vec, (Sha256dHash, Vec)) { + fn check_spend_local_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec, Vec, (Sha256dHash, Vec)) { let commitment_txid = tx.txid(); - // TODO: If we find a match here we need to fail back HTLCs that weren't included in the - // broadcast commitment transaction, either because they didn't meet dust or because they - // weren't yet included in our commitment transaction(s). + let mut local_txn = Vec::new(); + let mut spendable_outputs = Vec::new(); + let mut watch_outputs = Vec::new(); + + macro_rules! wait_threshold_conf { + ($height: expr, $source: expr, $update: expr, $commitment_tx: expr, $payment_hash: expr) => { + log_info!(self, "Failing HTLC with payment_hash {} from {} local commitment tx due to broadcast of transaction, waiting confirmation (at height{})", log_bytes!($payment_hash.0), $commitment_tx, height + HTLC_FAIL_ANTI_REORG_DELAY - 1); + match self.htlc_updated_waiting_threshold_conf.entry($height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + hash_map::Entry::Occupied(mut entry) => { + let e = entry.get_mut(); + e.retain(|ref update| update.0 != $source); + e.push(($source, $update, $payment_hash)); + } + hash_map::Entry::Vacant(entry) => { + entry.insert(vec![($source, $update, $payment_hash)]); + } + } + } + } + + macro_rules! append_onchain_update { + ($updates: expr) => { + local_txn.append(&mut $updates.0); + spendable_outputs.append(&mut $updates.1); + watch_outputs.append(&mut $updates.2); + } + } + + // HTLCs set may differ between last and previous local commitment txn, in case of one them hitting chain, ensure we cancel all HTLCs backward + let mut is_local_tx = false; + if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx { if local_tx.txid == commitment_txid { + is_local_tx = true; log_trace!(self, "Got latest local commitment tx broadcast, searching for available HTLCs to claim"); match self.key_storage { Storage::Local { ref delayed_payment_base_key, ref latest_per_commitment_point, .. } => { - let (local_txn, spendable_outputs, watch_outputs) = self.broadcast_by_local_state(local_tx, latest_per_commitment_point, &Some(*delayed_payment_base_key)); - return (local_txn, spendable_outputs, (commitment_txid, watch_outputs)); + append_onchain_update!(self.broadcast_by_local_state(local_tx, latest_per_commitment_point, &Some(*delayed_payment_base_key))); }, Storage::Watchtower { .. } => { - let (local_txn, spendable_outputs, watch_outputs) = self.broadcast_by_local_state(local_tx, &None, &None); - return (local_txn, spendable_outputs, (commitment_txid, watch_outputs)); + append_onchain_update!(self.broadcast_by_local_state(local_tx, &None, &None)); } } } } if let &Some(ref local_tx) = &self.prev_local_signed_commitment_tx { if local_tx.txid == commitment_txid { + is_local_tx = true; log_trace!(self, "Got previous local commitment tx broadcast, searching for available HTLCs to claim"); match self.key_storage { Storage::Local { ref delayed_payment_base_key, ref prev_latest_per_commitment_point, .. } => { - let (local_txn, spendable_outputs, watch_outputs) = self.broadcast_by_local_state(local_tx, prev_latest_per_commitment_point, &Some(*delayed_payment_base_key)); - return (local_txn, spendable_outputs, (commitment_txid, watch_outputs)); + append_onchain_update!(self.broadcast_by_local_state(local_tx, prev_latest_per_commitment_point, &Some(*delayed_payment_base_key))); }, Storage::Watchtower { .. } => { - let (local_txn, spendable_outputs, watch_outputs) = self.broadcast_by_local_state(local_tx, &None, &None); - return (local_txn, spendable_outputs, (commitment_txid, watch_outputs)); + append_onchain_update!(self.broadcast_by_local_state(local_tx, &None, &None)); } } } } - (Vec::new(), Vec::new(), (commitment_txid, Vec::new())) + + macro_rules! fail_dust_htlcs_after_threshold_conf { + ($local_tx: expr) => { + for &(ref htlc, _, ref source) in &$local_tx.htlc_outputs { + if htlc.transaction_output_index.is_none() { + if let &Some(ref source) = source { + wait_threshold_conf!(height, source.clone(), None, "lastest", htlc.payment_hash.clone()); + } + } + } + } + } + + if is_local_tx { + if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx { + fail_dust_htlcs_after_threshold_conf!(local_tx); + } + if let &Some(ref local_tx) = &self.prev_local_signed_commitment_tx { + fail_dust_htlcs_after_threshold_conf!(local_tx); + } + } + + (local_txn, spendable_outputs, (commitment_txid, watch_outputs)) } /// Generate a spendable output event when closing_transaction get registered onchain. diff --git a/src/ln/functional_tests.rs b/src/ln/functional_tests.rs index 4b58e3b29e6..0b8ce98da8e 100644 --- a/src/ln/functional_tests.rs +++ b/src/ln/functional_tests.rs @@ -27,7 +27,7 @@ use bitcoin::util::bip143; use bitcoin::util::address::Address; use bitcoin::util::bip32::{ChildNumber, ExtendedPubKey, ExtendedPrivKey}; use bitcoin::blockdata::block::{Block, BlockHeader}; -use bitcoin::blockdata::transaction::{Transaction, TxOut, TxIn, SigHashType}; +use bitcoin::blockdata::transaction::{Transaction, TxOut, TxIn, SigHashType, OutPoint as BitcoinOutPoint}; use bitcoin::blockdata::script::{Builder, Script}; use bitcoin::blockdata::opcodes; use bitcoin::blockdata::constants::genesis_block; @@ -5513,3 +5513,143 @@ fn test_update_fulfill_htlc_bolt2_after_malformed_htlc_message_must_forward_upda check_added_monitors!(nodes[1], 1); } + +fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) { + // Dust-HTLC failure updates must be delayed until failure-trigger tx (in this case local commitment) reach HTLC_FAIL_ANTI_REORG_DELAY + // We can have at most two valid local commitment tx, so both cases must be covered, and both txs must be checked to get them all as + // HTLC could have been removed from lastest local commitment tx but still valid until we get remote RAA + + let nodes = create_network(2); + let chan =create_announced_chan_between_nodes(&nodes, 0, 1); + + let bs_dust_limit = nodes[1].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().our_dust_limit_satoshis; + + // We route 2 dust-HTLCs between A and B + let (_, payment_hash_1) = route_payment(&nodes[0], &[&nodes[1]], bs_dust_limit*1000); + let (_, payment_hash_2) = route_payment(&nodes[0], &[&nodes[1]], bs_dust_limit*1000); + route_payment(&nodes[0], &[&nodes[1]], 1000000); + + // Cache one local commitment tx as previous + let as_prev_commitment_tx = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().last_local_commitment_txn.clone(); + + // Fail one HTLC to prune it in the will-be-latest-local commitment tx + assert!(nodes[1].node.fail_htlc_backwards(&payment_hash_2)); + check_added_monitors!(nodes[1], 0); + expect_pending_htlcs_forwardable!(nodes[1]); + check_added_monitors!(nodes[1], 1); + + let remove = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &remove.update_fail_htlcs[0]).unwrap(); + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &remove.commitment_signed).unwrap(); + check_added_monitors!(nodes[0], 1); + + // Cache one local commitment tx as lastest + let as_last_commitment_tx = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().last_local_commitment_txn.clone(); + + let events = nodes[0].node.get_and_clear_pending_msg_events(); + match events[0] { + MessageSendEvent::SendRevokeAndACK { node_id, .. } => { + assert_eq!(node_id, nodes[1].node.get_our_node_id()); + }, + _ => panic!("Unexpected event"), + } + match events[1] { + MessageSendEvent::UpdateHTLCs { node_id, .. } => { + assert_eq!(node_id, nodes[1].node.get_our_node_id()); + }, + _ => panic!("Unexpected event"), + } + + assert_ne!(as_prev_commitment_tx, as_last_commitment_tx); + // Fail the 2 dust-HTLCs, move their failure in maturation buffer (htlc_updated_waiting_threshold_conf) + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + if announce_latest { + nodes[0].chain_monitor.block_connected_checked(&header, 1, &[&as_last_commitment_tx[0]], &[1; 1]); + } else { + nodes[0].chain_monitor.block_connected_checked(&header, 1, &[&as_prev_commitment_tx[0]], &[1; 1]); + } + + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match events[0] { + MessageSendEvent::BroadcastChannelUpdate { .. } => {}, + _ => panic!("Unexpected event"), + } + + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); + let events = nodes[0].node.get_and_clear_pending_events(); + // Only 2 PaymentFailed events should show up, over-dust HTLC has to be failed by timeout tx + assert_eq!(events.len(), 2); + let mut first_failed = false; + for event in events { + match event { + Event::PaymentFailed { payment_hash, .. } => { + if payment_hash == payment_hash_1 { + assert!(!first_failed); + first_failed = true; + } else { + assert_eq!(payment_hash, payment_hash_2); + } + } + _ => panic!("Unexpected event"), + } + } +} + +#[test] +fn test_failure_delay_dust_htlc_local_commitment() { + do_test_failure_delay_dust_htlc_local_commitment(true); + do_test_failure_delay_dust_htlc_local_commitment(false); +} + +#[test] +fn test_no_failure_dust_htlc_local_commitment() { + // Transaction filters for failing back dust htlc based on local commitment txn infos has been + // prone to error, we test here that a dummy transaction don't fail them. + + let nodes = create_network(2); + let chan = create_announced_chan_between_nodes(&nodes, 0, 1); + + // Rebalance a bit + send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000); + + let as_dust_limit = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().our_dust_limit_satoshis; + let bs_dust_limit = nodes[1].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().our_dust_limit_satoshis; + + // We route 2 dust-HTLCs between A and B + let (preimage_1, _) = route_payment(&nodes[0], &[&nodes[1]], bs_dust_limit*1000); + let (preimage_2, _) = route_payment(&nodes[1], &[&nodes[0]], as_dust_limit*1000); + + // Build a dummy invalid transaction trying to spend a commitment tx + let input = TxIn { + previous_output: BitcoinOutPoint { txid: chan.3.txid(), vout: 0 }, + script_sig: Script::new(), + sequence: 0, + witness: Vec::new(), + }; + + let outp = TxOut { + script_pubkey: Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script(), + value: 10000, + }; + + let dummy_tx = Transaction { + version: 2, + lock_time: 0, + input: vec![input], + output: vec![outp] + }; + + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + nodes[0].chan_monitor.simple_monitor.block_connected(&header, 1, &[&dummy_tx], &[1;1]); + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + assert_eq!(nodes[0].node.get_and_clear_pending_msg_events().len(), 0); + // We broadcast a few more block to check everything is all right + connect_blocks(&nodes[0].chain_monitor, 20, 1, true, header.bitcoin_hash()); + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + assert_eq!(nodes[0].node.get_and_clear_pending_msg_events().len(), 0); + + claim_payment(&nodes[0], &vec!(&nodes[1])[..], preimage_1); + claim_payment(&nodes[1], &vec!(&nodes[0])[..], preimage_2); +} From c5ee3608786b9f9da317747b4eb37916d8fd8343 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Fri, 8 Feb 2019 21:43:56 -0500 Subject: [PATCH 06/10] Add block_disconnecting tests to cancel HTLC failure updates Add test_sweep_outbound_htlc_failure_update --- src/ln/functional_test_utils.rs | 19 ++++- src/ln/functional_tests.rs | 129 ++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 2 deletions(-) diff --git a/src/ln/functional_test_utils.rs b/src/ln/functional_test_utils.rs index b8f5e5835a3..15c372ddeb5 100644 --- a/src/ln/functional_test_utils.rs +++ b/src/ln/functional_test_utils.rs @@ -15,7 +15,7 @@ use util::logger::Logger; use util::config::UserConfig; use bitcoin::util::hash::BitcoinHash; -use bitcoin::blockdata::block::BlockHeader; +use bitcoin::blockdata::block::{BlockHeader, Block}; use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::network::constants::Network; @@ -47,13 +47,28 @@ pub fn confirm_transaction(chain: &chaininterface::ChainWatchInterfaceUtil, tx: } } -pub fn connect_blocks(chain: &chaininterface::ChainWatchInterfaceUtil, depth: u32, height: u32, parent: bool, prev_blockhash: Sha256d) { +pub fn connect_blocks(chain: &chaininterface::ChainWatchInterfaceUtil, depth: u32, height: u32, parent: bool, prev_blockhash: Sha256d) -> Sha256d { let mut header = BlockHeader { version: 0x2000000, prev_blockhash: if parent { prev_blockhash } else { Default::default() }, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; chain.block_connected_checked(&header, height + 1, &Vec::new(), &Vec::new()); for i in 2..depth + 1 { header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; chain.block_connected_checked(&header, height + i, &Vec::new(), &Vec::new()); } + header.bitcoin_hash() +} + +pub fn disconnect_blocks(chain: &chaininterface::ChainWatchInterfaceUtil, depth: u32, height: u32, parent: bool, prev_blockhash: Sha256d) { + let mut header = BlockHeader { version: 0x2000000, prev_blockhash: if parent { prev_blockhash } else { Default::default() }, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + let mut blocks = Vec::new(); + for _ in 0..depth { + blocks.push(Block { header, txdata: Vec::new() }); + header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + } + let mut height = height; + for block in blocks.pop() { + chain.block_disconnected(&block.header, height); + height -= 1; + } } pub struct Node { diff --git a/src/ln/functional_tests.rs b/src/ln/functional_tests.rs index 0b8ce98da8e..2706627a05e 100644 --- a/src/ln/functional_tests.rs +++ b/src/ln/functional_tests.rs @@ -5653,3 +5653,132 @@ fn test_no_failure_dust_htlc_local_commitment() { claim_payment(&nodes[0], &vec!(&nodes[1])[..], preimage_1); claim_payment(&nodes[1], &vec!(&nodes[0])[..], preimage_2); } + +fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { + // Outbound HTLC-failure updates must be cancelled if we get a reorg before we reach HTLC_FAIL_ANTI_REORG_DELAY. + // Broadcast of revoked remote commitment tx, trigger failure-update of dust/non-dust HTLCs + // Broadcast of remote commitment tx, trigger failure-update of dust-HTLCs + // Broadcast of timeout tx on remote commitment tx, trigger failure-udate of non-dust HTLCs + // Broadcast of local commitment tx, trigger failure-update of dust-HTLCs + // Broadcast of HTLC-timeout tx on local commitment tx, trigger failure-update of non-dust HTLCs + + let nodes = create_network(3); + let chan = create_announced_chan_between_nodes(&nodes, 0, 1); + + let bs_dust_limit = nodes[1].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().our_dust_limit_satoshis; + + let (payment_preimage_1, dust_hash) = route_payment(&nodes[0], &[&nodes[1]], bs_dust_limit*1000); + let (payment_preimage_2, non_dust_hash) = route_payment(&nodes[0], &[&nodes[1]], 1000000); + + let as_commitment_tx = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().last_local_commitment_txn.clone(); + let bs_commitment_tx = nodes[1].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().last_local_commitment_txn.clone(); + + // We revoked bs_commitment_tx + if revoked { + let (payment_preimage_3, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000); + claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage_3); + } + + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + let mut timeout_tx = Vec::new(); + if local { + // We fail dust-HTLC 1 by broadcast of local commitment tx + nodes[0].chain_monitor.block_connected_checked(&header, 1, &[&as_commitment_tx[0]], &[1; 1]); + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match events[0] { + MessageSendEvent::BroadcastChannelUpdate { .. } => {}, + _ => panic!("Unexpected event"), + } + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].clone()); + let parent_hash = connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 2, true, header.bitcoin_hash()); + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash, .. } => { + assert_eq!(payment_hash, dust_hash); + }, + _ => panic!("Unexpected event"), + } + assert_eq!(timeout_tx[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); + // We fail non-dust-HTLC 2 by broadcast of local HTLC-timeout tx on local commitment tx + let header_2 = BlockHeader { version: 0x20000000, prev_blockhash: parent_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + nodes[0].chain_monitor.block_connected_checked(&header_2, 7, &[&timeout_tx[0]], &[1; 1]); + let header_3 = BlockHeader { version: 0x20000000, prev_blockhash: header_2.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 8, true, header_3.bitcoin_hash()); + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash, .. } => { + assert_eq!(payment_hash, non_dust_hash); + }, + _ => panic!("Unexpected event"), + } + } else { + // We fail dust-HTLC 1 by broadcast of remote commitment tx. If revoked, fail also non-dust HTLC + nodes[0].chain_monitor.block_connected_checked(&header, 1, &[&bs_commitment_tx[0]], &[1; 1]); + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match events[0] { + MessageSendEvent::BroadcastChannelUpdate { .. } => {}, + _ => panic!("Unexpected event"), + } + timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].clone()); + let parent_hash = connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 2, true, header.bitcoin_hash()); + let header_2 = BlockHeader { version: 0x20000000, prev_blockhash: parent_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + if !revoked { + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash, .. } => { + assert_eq!(payment_hash, dust_hash); + }, + _ => panic!("Unexpected event"), + } + assert_eq!(timeout_tx[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); + // We fail non-dust-HTLC 2 by broadcast of local timeout tx on remote commitment tx + nodes[0].chain_monitor.block_connected_checked(&header_2, 7, &[&timeout_tx[0]], &[1; 1]); + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + let header_3 = BlockHeader { version: 0x20000000, prev_blockhash: header_2.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 8, true, header_3.bitcoin_hash()); + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash, .. } => { + assert_eq!(payment_hash, non_dust_hash); + }, + _ => panic!("Unexpected event"), + } + } else { + // If revoked, both dust & non-dust HTLCs should have been failed after HTLC_FAIL_ANTI_REORG_DELAY confs of revoked + // commitment tx + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2); + let first; + match events[0] { + Event::PaymentFailed { payment_hash, .. } => { + if payment_hash == dust_hash { first = true; } + else { first = false; } + }, + _ => panic!("Unexpected event"), + } + match events[1] { + Event::PaymentFailed { payment_hash, .. } => { + if first { assert_eq!(payment_hash, non_dust_hash); } + else { assert_eq!(payment_hash, dust_hash); } + }, + _ => panic!("Unexpected event"), + } + } + } +} + +#[test] +fn test_sweep_outbound_htlc_failure_update() { + do_test_sweep_outbound_htlc_failure_update(false, true); + do_test_sweep_outbound_htlc_failure_update(false, false); + do_test_sweep_outbound_htlc_failure_update(true, false); +} From 041b04c31870f5ae054665ea23d0425195b55e6d Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Sat, 30 Mar 2019 21:56:51 -0400 Subject: [PATCH 07/10] Move htlc_updated_waiting_threshold_conf to an OnchainEvent model We need also to track claim tx until their maturation to know when we may safely remove them from could-be-bumped-txn buffer --- src/ln/channelmonitor.rs | 167 ++++++++++++++++++++++++++++----------- src/util/ser.rs | 20 +++++ 2 files changed, 142 insertions(+), 45 deletions(-) diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index bb6b3632321..2ddb2690ad8 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -352,6 +352,23 @@ enum InputDescriptors { RevokedOutput, // either a revoked to_local output on commitment tx, a revoked HTLC-Timeout output or a revoked HTLC-Success output } +/// Upon discovering of some classes of onchain tx by ChannelMonitor, we may have to take actions on it +/// once they mature to enough confirmations (HTLC_FAIL_ANTI_REORG_DELAY) +#[derive(Clone, PartialEq)] +enum OnchainEvent { + /// Outpoint under claim process by our own tx, once this one get enough confirmations, we remove it from + /// bump-txn candidate buffer. + Claim { + outpoint: BitcoinOutPoint, + }, + /// HTLC output getting solved by a timeout, at maturation we pass upstream payment source information to solve + /// inbound HTLC in backward channel. Note, in case of preimage, we pass info to upstream without delay as we can + /// only win from it, so it's never an OnchainEvent + HTLCUpdate { + htlc_update: (HTLCSource, PaymentHash), + }, +} + const SERIALIZATION_VERSION: u8 = 1; const MIN_SERIALIZATION_VERSION: u8 = 1; @@ -402,7 +419,10 @@ pub struct ChannelMonitor { destination_script: Script, - htlc_updated_waiting_threshold_conf: HashMap, PaymentHash)>>, + // Used to track onchain events, i.e transactions parts of channels confirmed on chain, on which + // we have to take actions once they reach enough confs. Key is a block height timer, i.e we enforce + // actions when we receive a block with given height. Actions depend on OnchainEvent type. + onchain_events_waiting_threshold_conf: HashMap>, // We simply modify last_block_hash in Channel's block_connected so that serialization is // consistent but hopefully the users' copy handles block_connected in a consistent way. @@ -466,7 +486,7 @@ impl PartialEq for ChannelMonitor { self.current_local_signed_commitment_tx != other.current_local_signed_commitment_tx || self.payment_preimages != other.payment_preimages || self.destination_script != other.destination_script || - self.htlc_updated_waiting_threshold_conf != other.htlc_updated_waiting_threshold_conf + self.onchain_events_waiting_threshold_conf != other.onchain_events_waiting_threshold_conf { false } else { @@ -516,7 +536,7 @@ impl ChannelMonitor { payment_preimages: HashMap::new(), destination_script: destination_script, - htlc_updated_waiting_threshold_conf: HashMap::new(), + onchain_events_waiting_threshold_conf: HashMap::new(), last_block_hash: Default::default(), secp_ctx: Secp256k1::new(), @@ -1025,14 +1045,22 @@ impl ChannelMonitor { self.last_block_hash.write(writer)?; self.destination_script.write(writer)?; - writer.write_all(&byte_utils::be64_to_array(self.htlc_updated_waiting_threshold_conf.len() as u64))?; - for (ref target, ref updates) in self.htlc_updated_waiting_threshold_conf.iter() { + writer.write_all(&byte_utils::be64_to_array(self.onchain_events_waiting_threshold_conf.len() as u64))?; + for (ref target, ref events) in self.onchain_events_waiting_threshold_conf.iter() { writer.write_all(&byte_utils::be32_to_array(**target))?; - writer.write_all(&byte_utils::be64_to_array(updates.len() as u64))?; - for ref update in updates.iter() { - update.0.write(writer)?; - update.1.write(writer)?; - update.2.write(writer)?; + writer.write_all(&byte_utils::be64_to_array(events.len() as u64))?; + for ev in events.iter() { + match *ev { + OnchainEvent::Claim { ref outpoint } => { + writer.write_all(&[0; 1])?; + outpoint.write(writer)?; + }, + OnchainEvent::HTLCUpdate { ref htlc_update } => { + writer.write_all(&[1; 1])?; + htlc_update.0.write(writer)?; + htlc_update.1.write(writer)?; + } + } } } @@ -1271,14 +1299,21 @@ impl ChannelMonitor { for &(ref htlc, ref source_option) in outpoints.iter() { if let &Some(ref source) = source_option { log_info!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of revoked remote commitment transaction, waiting for confirmation (at height {})", log_bytes!(htlc.payment_hash.0), $commitment_tx, height + HTLC_FAIL_ANTI_REORG_DELAY - 1); - match self.htlc_updated_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + match self.onchain_events_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { hash_map::Entry::Occupied(mut entry) => { let e = entry.get_mut(); - e.retain(|ref update| update.0 != **source); - e.push(((**source).clone(), None, htlc.payment_hash.clone())); + e.retain(|ref event| { + match **event { + OnchainEvent::HTLCUpdate { ref htlc_update } => { + return htlc_update.0 != **source + }, + _ => return true + } + }); + e.push(OnchainEvent::HTLCUpdate { htlc_update: ((**source).clone(), htlc.payment_hash.clone())}); } hash_map::Entry::Vacant(entry) => { - entry.insert(vec![((**source).clone(), None, htlc.payment_hash.clone())]); + entry.insert(vec![OnchainEvent::HTLCUpdate { htlc_update: ((**source).clone(), htlc.payment_hash.clone())}]); } } } @@ -1361,14 +1396,21 @@ impl ChannelMonitor { } } log_trace!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of remote commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx); - match self.htlc_updated_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + match self.onchain_events_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { hash_map::Entry::Occupied(mut entry) => { let e = entry.get_mut(); - e.retain(|ref update| update.0 != **source); - e.push(((**source).clone(), None, htlc.payment_hash.clone())); + e.retain(|ref event| { + match **event { + OnchainEvent::HTLCUpdate { ref htlc_update } => { + return htlc_update.0 != **source + }, + _ => return true + } + }); + e.push(OnchainEvent::HTLCUpdate { htlc_update: ((**source).clone(), htlc.payment_hash.clone())}); } hash_map::Entry::Vacant(entry) => { - entry.insert(vec![((**source).clone(), None, htlc.payment_hash.clone())]); + entry.insert(vec![OnchainEvent::HTLCUpdate { htlc_update: ((**source).clone(), htlc.payment_hash.clone())}]); } } } @@ -1745,16 +1787,23 @@ impl ChannelMonitor { let mut watch_outputs = Vec::new(); macro_rules! wait_threshold_conf { - ($height: expr, $source: expr, $update: expr, $commitment_tx: expr, $payment_hash: expr) => { - log_info!(self, "Failing HTLC with payment_hash {} from {} local commitment tx due to broadcast of transaction, waiting confirmation (at height{})", log_bytes!($payment_hash.0), $commitment_tx, height + HTLC_FAIL_ANTI_REORG_DELAY - 1); - match self.htlc_updated_waiting_threshold_conf.entry($height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + ($height: expr, $source: expr, $commitment_tx: expr, $payment_hash: expr) => { + log_trace!(self, "Failing HTLC with payment_hash {} from {} local commitment tx due to broadcast of transaction, waiting confirmation (at height{})", log_bytes!($payment_hash.0), $commitment_tx, height + HTLC_FAIL_ANTI_REORG_DELAY - 1); + match self.onchain_events_waiting_threshold_conf.entry($height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { hash_map::Entry::Occupied(mut entry) => { let e = entry.get_mut(); - e.retain(|ref update| update.0 != $source); - e.push(($source, $update, $payment_hash)); + e.retain(|ref event| { + match **event { + OnchainEvent::HTLCUpdate { ref htlc_update } => { + return htlc_update.0 != $source + }, + _ => return true + } + }); + e.push(OnchainEvent::HTLCUpdate { htlc_update: ($source, $payment_hash)}); } hash_map::Entry::Vacant(entry) => { - entry.insert(vec![($source, $update, $payment_hash)]); + entry.insert(vec![OnchainEvent::HTLCUpdate { htlc_update: ($source, $payment_hash)}]); } } } @@ -1805,7 +1854,7 @@ impl ChannelMonitor { for &(ref htlc, _, ref source) in &$local_tx.htlc_outputs { if htlc.transaction_output_index.is_none() { if let &Some(ref source) = source { - wait_threshold_conf!(height, source.clone(), None, "lastest", htlc.payment_hash.clone()); + wait_threshold_conf!(height, source.clone(), "lastest", htlc.payment_hash.clone()); } } } @@ -1956,10 +2005,16 @@ impl ChannelMonitor { } } } - if let Some(updates) = self.htlc_updated_waiting_threshold_conf.remove(&height) { - for update in updates { - log_trace!(self, "HTLC {} failure update has get enough confirmation to be pass upstream", log_bytes!((update.2).0)); - htlc_updated.push(update); + if let Some(events) = self.onchain_events_waiting_threshold_conf.remove(&height) { + for ev in events { + match ev { + OnchainEvent::Claim { outpoint: _ } => { + }, + OnchainEvent::HTLCUpdate { htlc_update } => { + log_trace!(self, "HTLC {} failure update has got enough confirmations to be passed upstream", log_bytes!((htlc_update.1).0)); + htlc_updated.push((htlc_update.0, None, htlc_update.1)); + }, + } } } self.last_block_hash = block_hash.clone(); @@ -1967,8 +2022,10 @@ impl ChannelMonitor { } fn block_disconnected(&mut self, height: u32, block_hash: &Sha256dHash) { - if let Some(_) = self.htlc_updated_waiting_threshold_conf.remove(&(height + HTLC_FAIL_ANTI_REORG_DELAY - 1)) { - //We discard htlc update there as failure-trigger tx (revoked commitment tx, non-revoked commitment tx, HTLC-timeout tx) has been disconnected + if let Some(_) = self.onchain_events_waiting_threshold_conf.remove(&(height + HTLC_FAIL_ANTI_REORG_DELAY - 1)) { + //We may discard: + //- htlc update there as failure-trigger tx (revoked commitment tx, non-revoked commitment tx, HTLC-timeout tx) has been disconnected + //- our claim tx on a commitment tx output } self.last_block_hash = block_hash.clone(); } @@ -2150,14 +2207,21 @@ impl ChannelMonitor { htlc_updated.push((source, Some(payment_preimage), payment_hash)); } else { log_info!(self, "Failing HTLC with payment_hash {} timeout by a spend tx, waiting for confirmation (at height{})", log_bytes!(payment_hash.0), height + HTLC_FAIL_ANTI_REORG_DELAY - 1); - match self.htlc_updated_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + match self.onchain_events_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { hash_map::Entry::Occupied(mut entry) => { let e = entry.get_mut(); - e.retain(|ref update| update.0 != source); - e.push((source, None, payment_hash.clone())); + e.retain(|ref event| { + match **event { + OnchainEvent::HTLCUpdate { ref htlc_update } => { + return htlc_update.0 != source + }, + _ => return true + } + }); + e.push(OnchainEvent::HTLCUpdate { htlc_update: (source, payment_hash)}); } hash_map::Entry::Vacant(entry) => { - entry.insert(vec![(source, None, payment_hash)]); + entry.insert(vec![OnchainEvent::HTLCUpdate { htlc_update: (source, payment_hash)}]); } } } @@ -2380,18 +2444,31 @@ impl ReadableArgs> for (Sha256dHash, ChannelM let destination_script = Readable::read(reader)?; let waiting_threshold_conf_len: u64 = Readable::read(reader)?; - let mut htlc_updated_waiting_threshold_conf = HashMap::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128)); + let mut onchain_events_waiting_threshold_conf = HashMap::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128)); for _ in 0..waiting_threshold_conf_len { let height_target = Readable::read(reader)?; - let updates_len: u64 = Readable::read(reader)?; - let mut updates = Vec::with_capacity(cmp::min(updates_len as usize, MAX_ALLOC_SIZE / 128)); - for _ in 0..updates_len { - let htlc_source = Readable::read(reader)?; - let preimage = Readable::read(reader)?; - let hash = Readable::read(reader)?; - updates.push((htlc_source, preimage, hash)); + let events_len: u64 = Readable::read(reader)?; + let mut events = Vec::with_capacity(cmp::min(events_len as usize, MAX_ALLOC_SIZE / 128)); + for _ in 0..events_len { + let ev = match >::read(reader)? { + 0 => { + let outpoint = Readable::read(reader)?; + OnchainEvent::Claim { + outpoint + } + }, + 1 => { + let htlc_source = Readable::read(reader)?; + let hash = Readable::read(reader)?; + OnchainEvent::HTLCUpdate { + htlc_update: (htlc_source, hash) + } + }, + _ => return Err(DecodeError::InvalidValue), + }; + events.push(ev); } - htlc_updated_waiting_threshold_conf.insert(height_target, updates); + onchain_events_waiting_threshold_conf.insert(height_target, events); } Ok((last_block_hash.clone(), ChannelMonitor { @@ -2418,7 +2495,7 @@ impl ReadableArgs> for (Sha256dHash, ChannelM destination_script, - htlc_updated_waiting_threshold_conf, + onchain_events_waiting_threshold_conf, last_block_hash, secp_ctx, diff --git a/src/util/ser.rs b/src/util/ser.rs index 1b10a393090..a2ef16b5e24 100644 --- a/src/util/ser.rs +++ b/src/util/ser.rs @@ -9,6 +9,7 @@ use std::hash::Hash; use secp256k1::Signature; use secp256k1::key::{PublicKey, SecretKey}; use bitcoin::blockdata::script::Script; +use bitcoin::blockdata::transaction::OutPoint; use bitcoin_hashes::sha256d::Hash as Sha256dHash; use std::marker::Sized; use ln::msgs::DecodeError; @@ -422,3 +423,22 @@ impl Readable for Option } } } + +impl Writeable for OutPoint { + fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { + self.txid.write(w)?; + self.vout.write(w)?; + Ok(()) + } +} + +impl Readable for OutPoint { + fn read(r: &mut R) -> Result { + let txid = Readable::read(r)?; + let vout = Readable::read(r)?; + Ok(OutPoint { + txid, + vout, + }) + } +} From 963f002056d86365447dc1ce244251a449df5b6d Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Thu, 18 Jul 2019 18:50:03 -0400 Subject: [PATCH 08/10] Add more comments about timelock assumptions and security model Rename HTLC_FAIL_ANTI_REORG_DELAY to ANTI_REORG_DELAY because we are going to rely on it also to remove bump candidates outpoint from tracker after claim get enough depth. Rename HTLC_FAIL_TIMEOUT_BLOCKS to LATENCY_GRACE_PERIOD_BLOCKS because it's carrying more meaningfully that we are doing a favor to our peer instead of ruthlessly enforcing the contract. CLTV_EXPIRY_DELTA should be > to LATENCY_GRACE_PERIOD_BLOCKS + +CLTV_CLAIM_BUFFER + ANTI_REORG_DELAY + LATENCY_GRACE_PERIOD_BLOCKS When we reached height + LATENCY_GRACE_PERIOD_BLOCKS and we have pending unsolved outbound HTLC, we fail onchain with our local commitment tx. At this point we expect to get in chain in a worst-case delay of CLTV_CLAIM_BUFFER. When our HTLC-timeout is confirmed with ANTI_REORG_DELAY we may safely fail backward the corresponding inbound output. --- src/ln/channelmanager.rs | 23 +++++++++-------- src/ln/channelmonitor.rs | 52 +++++++++++++++++++++++--------------- src/ln/functional_tests.rs | 42 +++++++++++++++--------------- 3 files changed, 65 insertions(+), 52 deletions(-) diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index 201865d6738..64a9f2ebb8e 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -28,7 +28,7 @@ use secp256k1; use chain::chaininterface::{BroadcasterInterface,ChainListener,ChainWatchInterface,FeeEstimator}; use chain::transaction::OutPoint; use ln::channel::{Channel, ChannelError}; -use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, HTLC_FAIL_ANTI_REORG_DELAY}; +use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY}; use ln::router::Route; use ln::msgs; use ln::onion_utils; @@ -351,20 +351,21 @@ pub struct ChannelManager { const CLTV_EXPIRY_DELTA: u16 = 6 * 12; //TODO? pub(super) const CLTV_FAR_FAR_AWAY: u32 = 6 * 24 * 7; //TODO? -// Check that our CLTV_EXPIRY is at least CLTV_CLAIM_BUFFER + 2*HTLC_FAIL_TIMEOUT_BLOCKS + -// HTLC_FAIL_ANTI_REORG_DELAY, ie that if the next-hop peer fails the HTLC within -// HTLC_FAIL_TIMEOUT_BLOCKS then we'll still have HTLC_FAIL_TIMEOUT_BLOCKS left to fail it -// backwards ourselves before hitting the CLTV_CLAIM_BUFFER point and failing the channel -// on-chain to time out the HTLC. +// Check that our CLTV_EXPIRY is at least CLTV_CLAIM_BUFFER + ANTI_REORG_DELAY + LATENCY_GRACE_PERIOD_BLOCKS, +// ie that if the next-hop peer fails the HTLC within +// LATENCY_GRACE_PERIOD_BLOCKS then we'll still have CLTV_CLAIM_BUFFER left to timeout it onchain, +// then waiting ANTI_REORG_DELAY to be reorg-safe on the outbound HLTC and +// failing the corresponding htlc backward, and us now seeing the last block of ANTI_REORG_DELAY before +// LATENCY_GRACE_PERIOD_BLOCKS. #[deny(const_err)] #[allow(dead_code)] -const CHECK_CLTV_EXPIRY_SANITY: u32 = CLTV_EXPIRY_DELTA as u32 - 2*HTLC_FAIL_TIMEOUT_BLOCKS - CLTV_CLAIM_BUFFER - HTLC_FAIL_ANTI_REORG_DELAY; +const CHECK_CLTV_EXPIRY_SANITY: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - CLTV_CLAIM_BUFFER - ANTI_REORG_DELAY - LATENCY_GRACE_PERIOD_BLOCKS; // Check for ability of an attacker to make us fail on-chain by delaying inbound claim. See // ChannelMontior::would_broadcast_at_height for a description of why this is needed. #[deny(const_err)] #[allow(dead_code)] -const CHECK_CLTV_EXPIRY_SANITY_2: u32 = CLTV_EXPIRY_DELTA as u32 - HTLC_FAIL_TIMEOUT_BLOCKS - 2*CLTV_CLAIM_BUFFER; +const CHECK_CLTV_EXPIRY_SANITY_2: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER; macro_rules! secp_call { ( $res: expr, $err: expr ) => { @@ -820,7 +821,7 @@ impl ChannelManager { let pending_forward_info = if next_hop_data.hmac == [0; 32] { // OUR PAYMENT! // final_expiry_too_soon - if (msg.cltv_expiry as u64) < self.latest_block_height.load(Ordering::Acquire) as u64 + (CLTV_CLAIM_BUFFER + HTLC_FAIL_TIMEOUT_BLOCKS) as u64 { + if (msg.cltv_expiry as u64) < self.latest_block_height.load(Ordering::Acquire) as u64 + (CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS) as u64 { return_err!("The final CLTV expiry is too soon to handle", 17, &[0;0]); } // final_incorrect_htlc_amount @@ -912,8 +913,8 @@ impl ChannelManager { break Some(("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, Some(self.get_channel_update(chan).unwrap()))); } let cur_height = self.latest_block_height.load(Ordering::Acquire) as u32 + 1; - // We want to have at least HTLC_FAIL_TIMEOUT_BLOCKS to fail prior to going on chain CLAIM_BUFFER blocks before expiration - if msg.cltv_expiry <= cur_height + CLTV_CLAIM_BUFFER + HTLC_FAIL_TIMEOUT_BLOCKS as u32 { // expiry_too_soon + // We want to have at least LATENCY_GRACE_PERIOD_BLOCKS to fail prior to going on chain CLAIM_BUFFER blocks before expiration + if msg.cltv_expiry <= cur_height + CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS as u32 { // expiry_too_soon break Some(("CLTV expiry is too close", 0x1000 | 14, Some(self.get_channel_update(chan).unwrap()))); } if msg.cltv_expiry > cur_height + CLTV_FAR_FAR_AWAY as u32 { // expiry_too_far diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 2ddb2690ad8..6e1b212ea24 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -303,12 +303,24 @@ const CLTV_SHARED_CLAIM_BUFFER: u32 = 12; pub(crate) const CLTV_CLAIM_BUFFER: u32 = 6; /// Number of blocks by which point we expect our counterparty to have seen new blocks on the /// network and done a full update_fail_htlc/commitment_signed dance (+ we've updated all our -/// copies of ChannelMonitors, including watchtowers). -pub(crate) const HTLC_FAIL_TIMEOUT_BLOCKS: u32 = 3; -/// Number of blocks we wait on seeing a confirmed HTLC-Timeout or previous revoked commitment -/// transaction before we fail corresponding inbound HTLCs. This prevents us from failing backwards -/// and then getting a reorg resulting in us losing money. -pub(crate) const HTLC_FAIL_ANTI_REORG_DELAY: u32 = 6; +/// copies of ChannelMonitors, including watchtowers). We could enforce the contract by failing +/// at CLTV expiration height but giving a grace period to our peer may be profitable for us if he +/// can provide an over-late preimage. Nevertheless, grace period has to be accounted in our +/// CLTV_EXPIRY_DELTA to be secure. Following this policy we may decrease the rate of channel failures +/// due to expiration but increase the cost of funds being locked longuer in case of failure. +/// This delay also cover a low-power peer being slow to process blocks and so being behind us on +/// accurate block height. +/// In case of onchain failure to be pass backward we may see the last block of ANTI_REORG_DELAY +/// with at worst this delay, so we are not only using this value as a mercy for them but also +/// us as a safeguard to delay with enough time. +pub(crate) const LATENCY_GRACE_PERIOD_BLOCKS: u32 = 3; +/// Number of blocks we wait on seeing a HTLC output being solved before we fail corresponding inbound +/// HTLCs. This prevents us from failing backwards and then getting a reorg resulting in us losing money. +/// We use also this delay to be sure we can remove our in-flight claim txn from bump candidates buffer. +/// It may cause spurrious generation of bumped claim txn but that's allright given the outpoint is already +/// solved by a previous claim tx. What we want to avoid is reorg evicting our claim tx and us not +/// keeping bumping another claim tx to solve the outpoint. +pub(crate) const ANTI_REORG_DELAY: u32 = 6; #[derive(Clone, PartialEq)] enum Storage { @@ -353,7 +365,7 @@ enum InputDescriptors { } /// Upon discovering of some classes of onchain tx by ChannelMonitor, we may have to take actions on it -/// once they mature to enough confirmations (HTLC_FAIL_ANTI_REORG_DELAY) +/// once they mature to enough confirmations (ANTI_REORG_DELAY) #[derive(Clone, PartialEq)] enum OnchainEvent { /// Outpoint under claim process by our own tx, once this one get enough confirmations, we remove it from @@ -1298,8 +1310,8 @@ impl ChannelMonitor { if let Some(ref outpoints) = self.remote_claimable_outpoints.get($txid) { for &(ref htlc, ref source_option) in outpoints.iter() { if let &Some(ref source) = source_option { - log_info!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of revoked remote commitment transaction, waiting for confirmation (at height {})", log_bytes!(htlc.payment_hash.0), $commitment_tx, height + HTLC_FAIL_ANTI_REORG_DELAY - 1); - match self.onchain_events_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + log_info!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of revoked remote commitment transaction, waiting for confirmation (at height {})", log_bytes!(htlc.payment_hash.0), $commitment_tx, height + ANTI_REORG_DELAY - 1); + match self.onchain_events_waiting_threshold_conf.entry(height + ANTI_REORG_DELAY - 1) { hash_map::Entry::Occupied(mut entry) => { let e = entry.get_mut(); e.retain(|ref event| { @@ -1396,7 +1408,7 @@ impl ChannelMonitor { } } log_trace!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of remote commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx); - match self.onchain_events_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + match self.onchain_events_waiting_threshold_conf.entry(height + ANTI_REORG_DELAY - 1) { hash_map::Entry::Occupied(mut entry) => { let e = entry.get_mut(); e.retain(|ref event| { @@ -1788,8 +1800,8 @@ impl ChannelMonitor { macro_rules! wait_threshold_conf { ($height: expr, $source: expr, $commitment_tx: expr, $payment_hash: expr) => { - log_trace!(self, "Failing HTLC with payment_hash {} from {} local commitment tx due to broadcast of transaction, waiting confirmation (at height{})", log_bytes!($payment_hash.0), $commitment_tx, height + HTLC_FAIL_ANTI_REORG_DELAY - 1); - match self.onchain_events_waiting_threshold_conf.entry($height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + log_trace!(self, "Failing HTLC with payment_hash {} from {} local commitment tx due to broadcast of transaction, waiting confirmation (at height{})", log_bytes!($payment_hash.0), $commitment_tx, height + ANTI_REORG_DELAY - 1); + match self.onchain_events_waiting_threshold_conf.entry($height + ANTI_REORG_DELAY - 1) { hash_map::Entry::Occupied(mut entry) => { let e = entry.get_mut(); e.retain(|ref event| { @@ -2022,7 +2034,7 @@ impl ChannelMonitor { } fn block_disconnected(&mut self, height: u32, block_hash: &Sha256dHash) { - if let Some(_) = self.onchain_events_waiting_threshold_conf.remove(&(height + HTLC_FAIL_ANTI_REORG_DELAY - 1)) { + if let Some(_) = self.onchain_events_waiting_threshold_conf.remove(&(height + ANTI_REORG_DELAY - 1)) { //We may discard: //- htlc update there as failure-trigger tx (revoked commitment tx, non-revoked commitment tx, HTLC-timeout tx) has been disconnected //- our claim tx on a commitment tx output @@ -2059,16 +2071,16 @@ impl ChannelMonitor { // from us until we've reached the point where we go on-chain with the // corresponding inbound HTLC, we must ensure that outbound HTLCs go on chain at // least CLTV_CLAIM_BUFFER blocks prior to the inbound HTLC. - // aka outbound_cltv + HTLC_FAIL_TIMEOUT_BLOCKS == height - CLTV_CLAIM_BUFFER + // aka outbound_cltv + LATENCY_GRACE_PERIOD_BLOCKS == height - CLTV_CLAIM_BUFFER // inbound_cltv == height + CLTV_CLAIM_BUFFER - // outbound_cltv + HTLC_FAIL_TIMEOUT_BLOCKS + CLTV_CLAIM_BUFFER <= inbound_cltv - CLTV_CLAIM_BUFFER - // HTLC_FAIL_TIMEOUT_BLOCKS + 2*CLTV_CLAIM_BUFFER <= inbound_cltv - outbound_cltv + // outbound_cltv + LATENCY_GRACE_PERIOD_BLOCKS + CLTV_CLAIM_BUFFER <= inbound_cltv - CLTV_CLAIM_BUFFER + // LATENCY_GRACE_PERIOD_BLOCKS + 2*CLTV_CLAIM_BUFFER <= inbound_cltv - outbound_cltv // CLTV_EXPIRY_DELTA <= inbound_cltv - outbound_cltv (by check in ChannelManager::decode_update_add_htlc_onion) - // HTLC_FAIL_TIMEOUT_BLOCKS + 2*CLTV_CLAIM_BUFFER <= CLTV_EXPIRY_DELTA + // LATENCY_GRACE_PERIOD_BLOCKS + 2*CLTV_CLAIM_BUFFER <= CLTV_EXPIRY_DELTA // The final, above, condition is checked for statically in channelmanager // with CHECK_CLTV_EXPIRY_SANITY_2. let htlc_outbound = $local_tx == htlc.offered; - if ( htlc_outbound && htlc.cltv_expiry + HTLC_FAIL_TIMEOUT_BLOCKS <= height) || + if ( htlc_outbound && htlc.cltv_expiry + LATENCY_GRACE_PERIOD_BLOCKS <= height) || (!htlc_outbound && htlc.cltv_expiry <= height + CLTV_CLAIM_BUFFER && self.payment_preimages.contains_key(&htlc.payment_hash)) { log_info!(self, "Force-closing channel due to {} HTLC timeout, HTLC expiry is {}", if htlc_outbound { "outbound" } else { "inbound "}, htlc.cltv_expiry); return true; @@ -2206,8 +2218,8 @@ impl ChannelMonitor { payment_preimage.0.copy_from_slice(&input.witness[1]); htlc_updated.push((source, Some(payment_preimage), payment_hash)); } else { - log_info!(self, "Failing HTLC with payment_hash {} timeout by a spend tx, waiting for confirmation (at height{})", log_bytes!(payment_hash.0), height + HTLC_FAIL_ANTI_REORG_DELAY - 1); - match self.onchain_events_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) { + log_info!(self, "Failing HTLC with payment_hash {} timeout by a spend tx, waiting for confirmation (at height{})", log_bytes!(payment_hash.0), height + ANTI_REORG_DELAY - 1); + match self.onchain_events_waiting_threshold_conf.entry(height + ANTI_REORG_DELAY - 1) { hash_map::Entry::Occupied(mut entry) => { let e = entry.get_mut(); e.retain(|ref event| { diff --git a/src/ln/functional_tests.rs b/src/ln/functional_tests.rs index 2706627a05e..084b6bc5c61 100644 --- a/src/ln/functional_tests.rs +++ b/src/ln/functional_tests.rs @@ -8,7 +8,7 @@ use chain::keysinterface::{KeysInterface, SpendableOutputDescriptor}; use chain::keysinterface; use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC, BREAKDOWN_TIMEOUT}; use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,HTLCForwardInfo,RAACommitmentOrder, PaymentPreimage, PaymentHash}; -use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, ManyChannelMonitor, HTLC_FAIL_ANTI_REORG_DELAY}; +use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ManyChannelMonitor, ANTI_REORG_DELAY}; use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT}; use ln::onion_utils; use ln::router::{Route, RouteHop}; @@ -1695,7 +1695,7 @@ fn channel_monitor_network_test() { { let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[3].chain_monitor.block_connected_checked(&header, 2, &Vec::new()[..], &[0; 0]); - for i in 3..TEST_FINAL_CLTV + 2 + HTLC_FAIL_TIMEOUT_BLOCKS + 1 { + for i in 3..TEST_FINAL_CLTV + 2 + LATENCY_GRACE_PERIOD_BLOCKS + 1 { header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[3].chain_monitor.block_connected_checked(&header, i, &Vec::new()[..], &[0; 0]); } @@ -1871,7 +1871,7 @@ fn claim_htlc_outputs_shared_tx() { let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); - connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); @@ -1939,7 +1939,7 @@ fn claim_htlc_outputs_single_tx() { let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200); nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200); - connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 200, true, header.bitcoin_hash()); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 200, true, header.bitcoin_hash()); let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); @@ -2241,7 +2241,7 @@ fn test_htlc_on_chain_timeout() { } nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![timeout_tx]}, 1); - connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); check_added_monitors!(nodes[1], 0); check_closed_broadcast!(nodes[1]); @@ -2300,7 +2300,7 @@ fn test_simple_commitment_revoked_fail_backward() { let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42}; nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); - connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); check_added_monitors!(nodes[1], 0); check_closed_broadcast!(nodes[1]); @@ -2453,7 +2453,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42}; nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); - connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), if deliver_bs_raa { 1 } else { 2 }); @@ -3899,7 +3899,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() { check_spends!(htlc_success_txn[1], commitment_txn[0].clone()); nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![htlc_timeout_tx] }, 200); - connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 200, true, header.bitcoin_hash()); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 200, true, header.bitcoin_hash()); expect_pending_htlcs_forwardable!(nodes[1]); let htlc_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); assert!(htlc_updates.update_add_htlcs.is_empty()); @@ -4118,7 +4118,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno } else { nodes[2].chain_monitor.block_connected_checked(&header, 1, &[&ds_prev_commitment_tx[0]], &[1; 1]); } - connect_blocks(&nodes[2].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); + connect_blocks(&nodes[2].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); check_closed_broadcast!(nodes[2]); expect_pending_htlcs_forwardable!(nodes[2]); check_added_monitors!(nodes[2], 2); @@ -4348,7 +4348,7 @@ fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) { // to "time out" the HTLC. let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - for i in 1..TEST_FINAL_CLTV + HTLC_FAIL_TIMEOUT_BLOCKS + CHAN_CONFIRM_DEPTH + 1 { + for i in 1..TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + CHAN_CONFIRM_DEPTH + 1 { nodes[0].chain_monitor.block_connected_checked(&header, i, &Vec::new(), &Vec::new()); header.prev_blockhash = header.bitcoin_hash(); } @@ -4387,7 +4387,7 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no } let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - for i in 1..TEST_FINAL_CLTV + HTLC_FAIL_TIMEOUT_BLOCKS + CHAN_CONFIRM_DEPTH + 1 { + for i in 1..TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + CHAN_CONFIRM_DEPTH + 1 { nodes[0].chain_monitor.block_connected_checked(&header, i, &Vec::new(), &Vec::new()); header.prev_blockhash = header.bitcoin_hash(); } @@ -4791,7 +4791,7 @@ fn test_onion_failure() { }, || {}, true, Some(UPDATE|13), Some(msgs::HTLCFailChannelUpdate::ChannelClosed { short_channel_id: channels[0].0.contents.short_channel_id, is_permanent: true})); run_onion_failure_test("expiry_too_soon", 0, &nodes, &route, &payment_hash, |msg| { - let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - HTLC_FAIL_TIMEOUT_BLOCKS + 1; + let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - LATENCY_GRACE_PERIOD_BLOCKS + 1; let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[1].chain_monitor.block_connected_checked(&header, height, &Vec::new()[..], &[0; 0]); }, ||{}, true, Some(UPDATE|14), Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage{msg: ChannelUpdate::dummy()})); @@ -4801,7 +4801,7 @@ fn test_onion_failure() { }, false, Some(PERM|15), None); run_onion_failure_test("final_expiry_too_soon", 1, &nodes, &route, &payment_hash, |msg| { - let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - HTLC_FAIL_TIMEOUT_BLOCKS + 1; + let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - LATENCY_GRACE_PERIOD_BLOCKS + 1; let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[2].chain_monitor.block_connected_checked(&header, height, &Vec::new()[..], &[0; 0]); }, || {}, true, Some(17), None); @@ -5515,7 +5515,7 @@ fn test_update_fulfill_htlc_bolt2_after_malformed_htlc_message_must_forward_upda } fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) { - // Dust-HTLC failure updates must be delayed until failure-trigger tx (in this case local commitment) reach HTLC_FAIL_ANTI_REORG_DELAY + // Dust-HTLC failure updates must be delayed until failure-trigger tx (in this case local commitment) reach ANTI_REORG_DELAY // We can have at most two valid local commitment tx, so both cases must be covered, and both txs must be checked to get them all as // HTLC could have been removed from lastest local commitment tx but still valid until we get remote RAA @@ -5577,7 +5577,7 @@ fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) { } assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); - connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); + connect_blocks(&nodes[0].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); let events = nodes[0].node.get_and_clear_pending_events(); // Only 2 PaymentFailed events should show up, over-dust HTLC has to be failed by timeout tx assert_eq!(events.len(), 2); @@ -5655,7 +5655,7 @@ fn test_no_failure_dust_htlc_local_commitment() { } fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { - // Outbound HTLC-failure updates must be cancelled if we get a reorg before we reach HTLC_FAIL_ANTI_REORG_DELAY. + // Outbound HTLC-failure updates must be cancelled if we get a reorg before we reach ANTI_REORG_DELAY. // Broadcast of revoked remote commitment tx, trigger failure-update of dust/non-dust HTLCs // Broadcast of remote commitment tx, trigger failure-update of dust-HTLCs // Broadcast of timeout tx on remote commitment tx, trigger failure-udate of non-dust HTLCs @@ -5692,7 +5692,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { } assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].clone()); - let parent_hash = connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 2, true, header.bitcoin_hash()); + let parent_hash = connect_blocks(&nodes[0].chain_monitor, ANTI_REORG_DELAY - 1, 2, true, header.bitcoin_hash()); let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { @@ -5707,7 +5707,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); nodes[0].chain_monitor.block_connected_checked(&header_2, 7, &[&timeout_tx[0]], &[1; 1]); let header_3 = BlockHeader { version: 0x20000000, prev_blockhash: header_2.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 8, true, header_3.bitcoin_hash()); + connect_blocks(&nodes[0].chain_monitor, ANTI_REORG_DELAY - 1, 8, true, header_3.bitcoin_hash()); let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { @@ -5727,7 +5727,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { _ => panic!("Unexpected event"), } timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].clone()); - let parent_hash = connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 2, true, header.bitcoin_hash()); + let parent_hash = connect_blocks(&nodes[0].chain_monitor, ANTI_REORG_DELAY - 1, 2, true, header.bitcoin_hash()); let header_2 = BlockHeader { version: 0x20000000, prev_blockhash: parent_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; if !revoked { let events = nodes[0].node.get_and_clear_pending_events(); @@ -5743,7 +5743,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { nodes[0].chain_monitor.block_connected_checked(&header_2, 7, &[&timeout_tx[0]], &[1; 1]); assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); let header_3 = BlockHeader { version: 0x20000000, prev_blockhash: header_2.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 8, true, header_3.bitcoin_hash()); + connect_blocks(&nodes[0].chain_monitor, ANTI_REORG_DELAY - 1, 8, true, header_3.bitcoin_hash()); let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { @@ -5753,7 +5753,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { _ => panic!("Unexpected event"), } } else { - // If revoked, both dust & non-dust HTLCs should have been failed after HTLC_FAIL_ANTI_REORG_DELAY confs of revoked + // If revoked, both dust & non-dust HTLCs should have been failed after ANTI_REORG_DELAY confs of revoked // commitment tx let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); From 81cea88d38c58595a12182a63abe7af090a374b5 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Sat, 30 Mar 2019 22:12:55 -0400 Subject: [PATCH 09/10] Add in-flight claim-tx tracking When we generate a justice tx, a htlc tx on remote commitment or a htlc tx on local commitment we track them until first conf. --- src/ln/channelmonitor.rs | 333 +++++++++++++++++++++++++++++++-------- 1 file changed, 271 insertions(+), 62 deletions(-) diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 6e1b212ea24..f6d741afa10 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -364,6 +364,32 @@ enum InputDescriptors { RevokedOutput, // either a revoked to_local output on commitment tx, a revoked HTLC-Timeout output or a revoked HTLC-Success output } +/// When ChannelMonitor discovers an onchain outpoint being a step of a channel and that it needs +/// to generate a tx to push channel state forward, we cache outpoint-solving tx material to build +/// a new bumped one in case of lenghty confirmation delay +#[derive(Clone, PartialEq)] +enum TxMaterial { + Revoked { + script: Script, + pubkey: Option, + key: SecretKey, + is_htlc: bool, + amount: u64, + }, + RemoteHTLC { + script: Script, + key: SecretKey, + preimage: Option, + amount: u64, + }, + LocalHTLC { + script: Script, + sigs: (Signature, Signature), + preimage: Option, + amount: u64, + } +} + /// 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) #[derive(Clone, PartialEq)] @@ -431,6 +457,12 @@ pub struct ChannelMonitor { destination_script: Script, + // Used to track outpoint in the process of being claimed by our transactions. We need to scan all transactions + // for inputs spending this. If height timer (u32) is expired and claim tx hasn't reached enough confirmations + // before, use TxMaterial to regenerate a new claim tx with a satoshis-per-1000-weight-units higher than last + // one (u64). + our_claim_txn_waiting_first_conf: HashMap, + // Used to track onchain events, i.e transactions parts of channels confirmed on chain, on which // we have to take actions once they reach enough confs. Key is a block height timer, i.e we enforce // actions when we receive a block with given height. Actions depend on OnchainEvent type. @@ -447,13 +479,16 @@ pub struct ChannelMonitor { } macro_rules! subtract_high_prio_fee { - ($self: ident, $fee_estimator: expr, $value: expr, $predicted_weight: expr, $spent_txid: expr) => { + ($self: ident, $fee_estimator: expr, $value: expr, $predicted_weight: expr, $spent_txid: expr, $used_feerate: expr) => { { - let mut fee = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * $predicted_weight / 1000; + $used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority); + let mut fee = $used_feerate * $predicted_weight / 1000; if $value <= fee { - fee = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) * $predicted_weight / 1000; + $used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal); + fee = $used_feerate * $predicted_weight / 1000; if $value <= fee { - fee = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) * $predicted_weight / 1000; + $used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); + fee = $used_feerate * $predicted_weight / 1000; if $value <= fee { log_error!($self, "Failed to generate an on-chain punishment tx spending {} as even low priority fee ({} sat) was more than the entire claim balance ({} sat)", $spent_txid, fee, $value); @@ -498,6 +533,7 @@ impl PartialEq for ChannelMonitor { self.current_local_signed_commitment_tx != other.current_local_signed_commitment_tx || self.payment_preimages != other.payment_preimages || self.destination_script != other.destination_script || + self.our_claim_txn_waiting_first_conf != other.our_claim_txn_waiting_first_conf || self.onchain_events_waiting_threshold_conf != other.onchain_events_waiting_threshold_conf { false @@ -548,6 +584,8 @@ impl ChannelMonitor { payment_preimages: HashMap::new(), destination_script: destination_script, + our_claim_txn_waiting_first_conf: HashMap::new(), + onchain_events_waiting_threshold_conf: HashMap::new(), last_block_hash: Default::default(), @@ -1057,6 +1095,42 @@ impl ChannelMonitor { self.last_block_hash.write(writer)?; self.destination_script.write(writer)?; + writer.write_all(&byte_utils::be64_to_array(self.our_claim_txn_waiting_first_conf.len() as u64))?; + for (ref outpoint, claim_tx_data) in self.our_claim_txn_waiting_first_conf.iter() { + outpoint.write(writer)?; + writer.write_all(&byte_utils::be32_to_array(claim_tx_data.0))?; + match claim_tx_data.1 { + TxMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref amount} => { + writer.write_all(&[0; 1])?; + script.write(writer)?; + pubkey.write(writer)?; + writer.write_all(&key[..])?; + if *is_htlc { + writer.write_all(&[0; 1])?; + } else { + writer.write_all(&[1; 1])?; + } + writer.write_all(&byte_utils::be64_to_array(*amount))?; + }, + TxMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref amount } => { + writer.write_all(&[1; 1])?; + script.write(writer)?; + key.write(writer)?; + preimage.write(writer)?; + writer.write_all(&byte_utils::be64_to_array(*amount))?; + }, + TxMaterial::LocalHTLC { ref script, ref sigs, ref preimage, ref amount } => { + writer.write_all(&[2; 1])?; + script.write(writer)?; + sigs.0.write(writer)?; + sigs.1.write(writer)?; + preimage.write(writer)?; + writer.write_all(&byte_utils::be64_to_array(*amount))?; + } + } + writer.write_all(&byte_utils::be64_to_array(claim_tx_data.2))?; + } + writer.write_all(&byte_utils::be64_to_array(self.onchain_events_waiting_threshold_conf.len() as u64))?; for (ref target, ref events) in self.onchain_events_waiting_threshold_conf.iter() { writer.write_all(&byte_utils::be32_to_array(**target))?; @@ -1193,10 +1267,9 @@ impl ChannelMonitor { } else { None }; let mut total_value = 0; - let mut values = Vec::new(); let mut inputs = Vec::new(); - let mut htlc_idxs = Vec::new(); - let mut input_descriptors = Vec::new(); + let mut inputs_info = Vec::new(); + let mut inputs_desc = Vec::new(); for (idx, outp) in tx.output.iter().enumerate() { if outp.script_pubkey == revokeable_p2wsh { @@ -1209,10 +1282,9 @@ impl ChannelMonitor { sequence: 0xfffffffd, witness: Vec::new(), }); - htlc_idxs.push(None); - values.push(outp.value); + inputs_desc.push(InputDescriptors::RevokedOutput); + inputs_info.push((None, outp.value)); total_value += outp.value; - input_descriptors.push(InputDescriptors::RevokedOutput); } else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() { spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH { outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 }, @@ -1225,7 +1297,7 @@ impl ChannelMonitor { macro_rules! sign_input { ($sighash_parts: expr, $input: expr, $htlc_idx: expr, $amount: expr) => { { - let (sig, redeemscript) = match self.key_storage { + let (sig, redeemscript, revocation_key) = match self.key_storage { Storage::Local { ref revocation_base_key, .. } => { let redeemscript = if $htlc_idx.is_none() { revokeable_redeemscript.clone() } else { let htlc = &per_commitment_option.unwrap()[$htlc_idx.unwrap()].0; @@ -1233,7 +1305,7 @@ impl ChannelMonitor { }; let sighash = hash_to_message!(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]); let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key)); - (self.secp_ctx.sign(&sighash, &revocation_key), redeemscript) + (self.secp_ctx.sign(&sighash, &revocation_key), redeemscript, revocation_key) }, Storage::Watchtower { .. } => { unimplemented!(); @@ -1246,7 +1318,8 @@ impl ChannelMonitor { } else { $input.witness.push(revocation_pubkey.serialize().to_vec()); } - $input.witness.push(redeemscript.into_bytes()); + $input.witness.push(redeemscript.clone().into_bytes()); + (redeemscript, revocation_key) } } } @@ -1273,10 +1346,9 @@ impl ChannelMonitor { }; if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER { inputs.push(input); - htlc_idxs.push(Some(idx)); - values.push(tx.output[transaction_output_index as usize].value); - total_value += htlc.amount_msat / 1000; - input_descriptors.push(if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }); + inputs_desc.push(if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }); + inputs_info.push((Some(idx), tx.output[transaction_output_index as usize].value)); + total_value += tx.output[transaction_output_index as usize].value; } else { let mut single_htlc_tx = Transaction { version: 2, @@ -1288,10 +1360,15 @@ impl ChannelMonitor { }), }; let predicted_weight = single_htlc_tx.get_weight() + Self::get_witnesses_weight(&[if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }]); - if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, tx.txid()) { + let mut used_feerate; + if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) { let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx); - sign_input!(sighash_parts, single_htlc_tx.input[0], Some(idx), htlc.amount_msat / 1000); + let (redeemscript, revocation_key) = sign_input!(sighash_parts, single_htlc_tx.input[0], Some(idx), htlc.amount_msat / 1000); assert!(predicted_weight >= single_htlc_tx.get_weight()); + match self.our_claim_txn_waiting_first_conf.entry(single_htlc_tx.input[0].previous_output.clone()) { + hash_map::Entry::Occupied(_) => {}, + hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::Revoked { script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: true, amount: htlc.amount_msat / 1000 }, used_feerate)); } + } txn_to_broadcast.push(single_htlc_tx); } } @@ -1355,18 +1432,22 @@ impl ChannelMonitor { input: inputs, output: outputs, }; - let predicted_weight = spend_tx.get_weight() + Self::get_witnesses_weight(&input_descriptors[..]); - if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid()) { + let predicted_weight = spend_tx.get_weight() + Self::get_witnesses_weight(&inputs_desc[..]); + + let mut used_feerate; + if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); } - let mut values_drain = values.drain(..); let sighash_parts = bip143::SighashComponents::new(&spend_tx); - for (input, htlc_idx) in spend_tx.input.iter_mut().zip(htlc_idxs.iter()) { - let value = values_drain.next().unwrap(); - sign_input!(sighash_parts, input, htlc_idx, value); + for (input, info) in spend_tx.input.iter_mut().zip(inputs_info.iter()) { + let (redeemscript, revocation_key) = sign_input!(sighash_parts, input, info.0, info.1); + match self.our_claim_txn_waiting_first_conf.entry(input.previous_output.clone()) { + hash_map::Entry::Occupied(_) => {}, + hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::Revoked { script: redeemscript, pubkey: if info.0.is_some() { Some(revocation_pubkey) } else { None }, key: revocation_key, is_htlc: if info.0.is_some() { true } else { false }, amount: info.1 }, used_feerate)); } + } } assert!(predicted_weight >= spend_tx.get_weight()); @@ -1480,20 +1561,20 @@ impl ChannelMonitor { } let mut total_value = 0; - let mut values = Vec::new(); let mut inputs = Vec::new(); - let mut input_descriptors = Vec::new(); + let mut inputs_desc = Vec::new(); + let mut inputs_info = Vec::new(); macro_rules! sign_input { ($sighash_parts: expr, $input: expr, $amount: expr, $preimage: expr) => { { - let (sig, redeemscript) = match self.key_storage { + let (sig, redeemscript, htlc_key) = match self.key_storage { Storage::Local { ref htlc_base_key, .. } => { let htlc = &per_commitment_option.unwrap()[$input.sequence as usize].0; let redeemscript = chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey); let sighash = hash_to_message!(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]); let htlc_key = ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, revocation_point, &htlc_base_key)); - (self.secp_ctx.sign(&sighash, &htlc_key), redeemscript) + (self.secp_ctx.sign(&sighash, &htlc_key), redeemscript, htlc_key) }, Storage::Watchtower { .. } => { unimplemented!(); @@ -1502,7 +1583,8 @@ impl ChannelMonitor { $input.witness.push(sig.serialize_der().to_vec()); $input.witness[0].push(SigHashType::All as u8); $input.witness.push($preimage); - $input.witness.push(redeemscript.into_bytes()); + $input.witness.push(redeemscript.clone().into_bytes()); + (redeemscript, htlc_key) } } } @@ -1527,9 +1609,9 @@ impl ChannelMonitor { }; if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER { inputs.push(input); - values.push((tx.output[transaction_output_index as usize].value, payment_preimage)); - total_value += htlc.amount_msat / 1000; - input_descriptors.push(if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC }); + inputs_desc.push(if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC }); + inputs_info.push((payment_preimage, tx.output[transaction_output_index as usize].value)); + total_value += tx.output[transaction_output_index as usize].value; } else { let mut single_htlc_tx = Transaction { version: 2, @@ -1541,14 +1623,19 @@ impl ChannelMonitor { }), }; let predicted_weight = single_htlc_tx.get_weight() + Self::get_witnesses_weight(&[if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC }]); - if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, tx.txid()) { + let mut used_feerate; + if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) { let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx); - sign_input!(sighash_parts, single_htlc_tx.input[0], htlc.amount_msat / 1000, payment_preimage.0.to_vec()); + let (redeemscript, htlc_key) = sign_input!(sighash_parts, single_htlc_tx.input[0], htlc.amount_msat / 1000, payment_preimage.0.to_vec()); assert!(predicted_weight >= single_htlc_tx.get_weight()); spendable_outputs.push(SpendableOutputDescriptor::StaticOutput { outpoint: BitcoinOutPoint { txid: single_htlc_tx.txid(), vout: 0 }, output: single_htlc_tx.output[0].clone(), }); + match self.our_claim_txn_waiting_first_conf.entry(single_htlc_tx.input[0].previous_output.clone()) { + hash_map::Entry::Occupied(_) => {}, + hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::RemoteHTLC { script: redeemscript, key: htlc_key, preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000 }, used_feerate)); } + } txn_to_broadcast.push(single_htlc_tx); } } @@ -1574,8 +1661,18 @@ impl ChannelMonitor { value: htlc.amount_msat / 1000, }), }; - let sighash_parts = bip143::SighashComponents::new(&timeout_tx); - sign_input!(sighash_parts, timeout_tx.input[0], htlc.amount_msat / 1000, vec![0]); + let predicted_weight = timeout_tx.get_weight() + Self::get_witnesses_weight(&[InputDescriptors::ReceivedHTLC]); + let mut used_feerate; + if subtract_high_prio_fee!(self, fee_estimator, timeout_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) { + let sighash_parts = bip143::SighashComponents::new(&timeout_tx); + let (redeemscript, htlc_key) = sign_input!(sighash_parts, timeout_tx.input[0], htlc.amount_msat / 1000, vec![0]); + assert!(predicted_weight >= timeout_tx.get_weight()); + //TODO: track SpendableOutputDescriptor + match self.our_claim_txn_waiting_first_conf.entry(timeout_tx.input[0].previous_output.clone()) { + hash_map::Entry::Occupied(_) => {}, + hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::RemoteHTLC { script : redeemscript, key: htlc_key, preimage: None, amount: htlc.amount_msat / 1000 }, used_feerate)); } + } + } txn_to_broadcast.push(timeout_tx); } } @@ -1593,19 +1690,23 @@ impl ChannelMonitor { input: inputs, output: outputs, }; - let predicted_weight = spend_tx.get_weight() + Self::get_witnesses_weight(&input_descriptors[..]); - if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid()) { + + let mut predicted_weight = spend_tx.get_weight() + Self::get_witnesses_weight(&inputs_desc[..]); + + let mut used_feerate; + if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); } - let mut values_drain = values.drain(..); let sighash_parts = bip143::SighashComponents::new(&spend_tx); - for input in spend_tx.input.iter_mut() { - let value = values_drain.next().unwrap(); - sign_input!(sighash_parts, input, value.0, (value.1).0.to_vec()); + for (input, info) in spend_tx.input.iter_mut().zip(inputs_info.iter()) { + let (redeemscript, htlc_key) = sign_input!(sighash_parts, input, info.1, (info.0).0.to_vec()); + match self.our_claim_txn_waiting_first_conf.entry(input.previous_output.clone()) { + hash_map::Entry::Occupied(_) => {}, + hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::RemoteHTLC { script: redeemscript, key: htlc_key, preimage: Some(*(info.0)), amount: info.1}, used_feerate)); } + } } - assert!(predicted_weight >= spend_tx.get_weight()); spendable_outputs.push(SpendableOutputDescriptor::StaticOutput { outpoint: BitcoinOutPoint { txid: spend_tx.txid(), vout: 0 }, @@ -1620,7 +1721,7 @@ impl ChannelMonitor { } /// Attempts to claim a remote HTLC-Success/HTLC-Timeout's outputs using the revocation key - fn check_spend_remote_htlc(&self, tx: &Transaction, commitment_number: u64, fee_estimator: &FeeEstimator) -> (Option, Option) { + fn check_spend_remote_htlc(&mut self, tx: &Transaction, commitment_number: u64, height: u32, fee_estimator: &FeeEstimator) -> (Option, Option) { if tx.input.len() != 1 || tx.output.len() != 1 { return (None, None) } @@ -1682,17 +1783,18 @@ impl ChannelMonitor { output: outputs, }; let predicted_weight = spend_tx.get_weight() + Self::get_witnesses_weight(&[InputDescriptors::RevokedOutput]); - if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid()) { + let mut used_feerate; + if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) { return (None, None); } let sighash_parts = bip143::SighashComponents::new(&spend_tx); - let sig = match self.key_storage { + let (sig, revocation_key) = match self.key_storage { Storage::Local { ref revocation_base_key, .. } => { let sighash = hash_to_message!(&sighash_parts.sighash_all(&spend_tx.input[0], &redeemscript, amount)[..]); let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key)); - self.secp_ctx.sign(&sighash, &revocation_key) + (self.secp_ctx.sign(&sighash, &revocation_key), revocation_key) } Storage::Watchtower { .. } => { unimplemented!(); @@ -1701,19 +1803,24 @@ impl ChannelMonitor { spend_tx.input[0].witness.push(sig.serialize_der().to_vec()); spend_tx.input[0].witness[0].push(SigHashType::All as u8); spend_tx.input[0].witness.push(vec!(1)); - spend_tx.input[0].witness.push(redeemscript.into_bytes()); + spend_tx.input[0].witness.push(redeemscript.clone().into_bytes()); assert!(predicted_weight >= spend_tx.get_weight()); let outpoint = BitcoinOutPoint { txid: spend_tx.txid(), vout: 0 }; let output = spend_tx.output[0].clone(); + match self.our_claim_txn_waiting_first_conf.entry(spend_tx.input[0].previous_output.clone()) { + hash_map::Entry::Occupied(_) => {}, + hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::Revoked { script: redeemscript, pubkey: None, key: revocation_key, is_htlc: false, amount: tx.output[0].value }, used_feerate)); } + } (Some(spend_tx), Some(SpendableOutputDescriptor::StaticOutput { outpoint, output })) } else { (None, None) } } - fn broadcast_by_local_state(&self, local_tx: &LocalSignedTx, per_commitment_point: &Option, delayed_payment_base_key: &Option) -> (Vec, Vec, Vec) { + fn broadcast_by_local_state(&self, local_tx: &LocalSignedTx, per_commitment_point: &Option, delayed_payment_base_key: &Option, height: u32) -> (Vec, Vec, Vec, Vec<(BitcoinOutPoint, (u32, TxMaterial, u64))>) { let mut res = Vec::with_capacity(local_tx.htlc_outputs.len()); let mut spendable_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); let mut watch_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); + let mut pending_claims = Vec::with_capacity(local_tx.htlc_outputs.len()); macro_rules! add_dynamic_output { ($father_tx: expr, $vout: expr) => { @@ -1758,9 +1865,11 @@ impl ChannelMonitor { htlc_timeout_tx.input[0].witness[2].push(SigHashType::All as u8); htlc_timeout_tx.input[0].witness.push(Vec::new()); - htlc_timeout_tx.input[0].witness.push(chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key).into_bytes()); + let htlc_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key); + htlc_timeout_tx.input[0].witness.push(htlc_script.clone().into_bytes()); add_dynamic_output!(htlc_timeout_tx, 0); + pending_claims.push((htlc_timeout_tx.input[0].previous_output.clone(), (height + 3, TxMaterial::LocalHTLC { script: htlc_script, sigs: (*their_sig, *our_sig), preimage: None, amount: htlc.amount_msat / 1000}, 0))); res.push(htlc_timeout_tx); } else { if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) { @@ -1775,9 +1884,11 @@ impl ChannelMonitor { htlc_success_tx.input[0].witness[2].push(SigHashType::All as u8); htlc_success_tx.input[0].witness.push(payment_preimage.0.to_vec()); - htlc_success_tx.input[0].witness.push(chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key).into_bytes()); + let htlc_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key); + htlc_success_tx.input[0].witness.push(htlc_script.clone().into_bytes()); add_dynamic_output!(htlc_success_tx, 0); + pending_claims.push((htlc_success_tx.input[0].previous_output.clone(), (height + 3, TxMaterial::LocalHTLC { script: htlc_script, sigs: (*their_sig, *our_sig), preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000}, 0))); res.push(htlc_success_tx); } } @@ -1786,7 +1897,7 @@ impl ChannelMonitor { } } - (res, spendable_outputs, watch_outputs) + (res, spendable_outputs, watch_outputs, pending_claims) } /// Attempts to claim any claimable HTLCs in a commitment transaction which was not (yet) @@ -1826,6 +1937,12 @@ impl ChannelMonitor { local_txn.append(&mut $updates.0); spendable_outputs.append(&mut $updates.1); watch_outputs.append(&mut $updates.2); + for claim in $updates.3 { + match self.our_claim_txn_waiting_first_conf.entry(claim.0) { + hash_map::Entry::Occupied(_) => {}, + hash_map::Entry::Vacant(entry) => { entry.insert(claim.1); } + } + } } } @@ -1838,10 +1955,10 @@ impl ChannelMonitor { log_trace!(self, "Got latest local commitment tx broadcast, searching for available HTLCs to claim"); match self.key_storage { Storage::Local { ref delayed_payment_base_key, ref latest_per_commitment_point, .. } => { - append_onchain_update!(self.broadcast_by_local_state(local_tx, latest_per_commitment_point, &Some(*delayed_payment_base_key))); + append_onchain_update!(self.broadcast_by_local_state(local_tx, latest_per_commitment_point, &Some(*delayed_payment_base_key), height)); }, Storage::Watchtower { .. } => { - append_onchain_update!(self.broadcast_by_local_state(local_tx, &None, &None)); + append_onchain_update!(self.broadcast_by_local_state(local_tx, &None, &None, height)); } } } @@ -1852,10 +1969,10 @@ impl ChannelMonitor { log_trace!(self, "Got previous local commitment tx broadcast, searching for available HTLCs to claim"); match self.key_storage { Storage::Local { ref delayed_payment_base_key, ref prev_latest_per_commitment_point, .. } => { - append_onchain_update!(self.broadcast_by_local_state(local_tx, prev_latest_per_commitment_point, &Some(*delayed_payment_base_key))); + append_onchain_update!(self.broadcast_by_local_state(local_tx, prev_latest_per_commitment_point, &Some(*delayed_payment_base_key), height)); }, Storage::Watchtower { .. } => { - append_onchain_update!(self.broadcast_by_local_state(local_tx, &None, &None)); + append_onchain_update!(self.broadcast_by_local_state(local_tx, &None, &None, height)); } } } @@ -1917,7 +2034,9 @@ impl ChannelMonitor { let mut res = vec![local_tx.tx.clone()]; match self.key_storage { Storage::Local { ref delayed_payment_base_key, ref prev_latest_per_commitment_point, .. } => { - res.append(&mut self.broadcast_by_local_state(local_tx, prev_latest_per_commitment_point, &Some(*delayed_payment_base_key)).0); + res.append(&mut self.broadcast_by_local_state(local_tx, prev_latest_per_commitment_point, &Some(*delayed_payment_base_key), 0).0); + // We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do. + // The data will be re-generated and tracked in check_spend_local_transaction if we get a confirmation. }, _ => panic!("Can only broadcast by local channelmonitor"), }; @@ -1969,7 +2088,7 @@ impl ChannelMonitor { } } else { if let Some(&(commitment_number, _)) = self.remote_commitment_txn_on_chain.get(&prevout.txid) { - let (tx, spendable_output) = self.check_spend_remote_htlc(tx, commitment_number, fee_estimator); + let (tx, spendable_output) = self.check_spend_remote_htlc(tx, commitment_number, height, fee_estimator); if let Some(tx) = tx { txn.push(tx); } @@ -1989,14 +2108,37 @@ impl ChannelMonitor { if updated.len() > 0 { htlc_updated.append(&mut updated); } + for inp in &tx.input { + if self.our_claim_txn_waiting_first_conf.contains_key(&inp.previous_output) { + match self.onchain_events_waiting_threshold_conf.entry(height + ANTI_REORG_DELAY - 1) { + hash_map::Entry::Occupied(mut entry) => { + let e = entry.get_mut(); + e.retain(|ref event| { + match **event { + OnchainEvent::Claim { outpoint } => { + return outpoint != inp.previous_output + }, + _ => return true + } + }); + e.push(OnchainEvent::Claim { outpoint: inp.previous_output.clone()}); + } + hash_map::Entry::Vacant(entry) => { + entry.insert(vec![OnchainEvent::Claim { outpoint: inp.previous_output.clone()}]); + } + } + } + } } + let mut pending_claims = Vec::new(); if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx { if self.would_broadcast_at_height(height) { broadcaster.broadcast_transaction(&cur_local_tx.tx); match self.key_storage { Storage::Local { ref delayed_payment_base_key, ref latest_per_commitment_point, .. } => { - let (txs, mut spendable_output, new_outputs) = self.broadcast_by_local_state(&cur_local_tx, latest_per_commitment_point, &Some(*delayed_payment_base_key)); + let (txs, mut spendable_output, new_outputs, mut pending_txn) = self.broadcast_by_local_state(&cur_local_tx, latest_per_commitment_point, &Some(*delayed_payment_base_key), height); spendable_outputs.append(&mut spendable_output); + pending_claims.append(&mut pending_txn); if !new_outputs.is_empty() { watch_outputs.push((cur_local_tx.txid.clone(), new_outputs)); } @@ -2005,8 +2147,9 @@ impl ChannelMonitor { } }, Storage::Watchtower { .. } => { - let (txs, mut spendable_output, new_outputs) = self.broadcast_by_local_state(&cur_local_tx, &None, &None); + let (txs, mut spendable_output, new_outputs, mut pending_txn) = self.broadcast_by_local_state(&cur_local_tx, &None, &None, height); spendable_outputs.append(&mut spendable_output); + pending_claims.append(&mut pending_txn); if !new_outputs.is_empty() { watch_outputs.push((cur_local_tx.txid.clone(), new_outputs)); } @@ -2017,10 +2160,17 @@ impl ChannelMonitor { } } } + for claim in pending_claims { + match self.our_claim_txn_waiting_first_conf.entry(claim.0) { + hash_map::Entry::Occupied(_) => {}, + hash_map::Entry::Vacant(entry) => { entry.insert(claim.1); } + } + } if let Some(events) = self.onchain_events_waiting_threshold_conf.remove(&height) { for ev in events { match ev { - OnchainEvent::Claim { outpoint: _ } => { + OnchainEvent::Claim { outpoint } => { + self.our_claim_txn_waiting_first_conf.remove(&outpoint); }, OnchainEvent::HTLCUpdate { htlc_update } => { log_trace!(self, "HTLC {} failure update has got enough confirmations to be passed upstream", log_bytes!((htlc_update.1).0)); @@ -2029,6 +2179,7 @@ impl ChannelMonitor { } } } + //TODO: iter on buffered TxMaterial in our_claim_txn_waiting_first_conf, if block timer is expired generate a bumped claim tx (RBF or CPFP accordingly) self.last_block_hash = block_hash.clone(); (watch_outputs, spendable_outputs, htlc_updated) } @@ -2039,6 +2190,7 @@ impl ChannelMonitor { //- htlc update there as failure-trigger tx (revoked commitment tx, non-revoked commitment tx, HTLC-timeout tx) has been disconnected //- our claim tx on a commitment tx output } + self.our_claim_txn_waiting_first_conf.retain(|_, ref mut v| if v.0 == height + 3 { false } else { true }); self.last_block_hash = block_hash.clone(); } @@ -2455,6 +2607,61 @@ impl ReadableArgs> for (Sha256dHash, ChannelM let last_block_hash: Sha256dHash = Readable::read(reader)?; let destination_script = Readable::read(reader)?; + let our_claim_txn_waiting_first_conf_len: u64 = Readable::read(reader)?; + let mut our_claim_txn_waiting_first_conf = HashMap::with_capacity(cmp::min(our_claim_txn_waiting_first_conf_len as usize, MAX_ALLOC_SIZE / 128)); + for _ in 0..our_claim_txn_waiting_first_conf_len { + let outpoint = Readable::read(reader)?; + let height_target = Readable::read(reader)?; + let tx_material = match >::read(reader)? { + 0 => { + let script = Readable::read(reader)?; + let pubkey = Readable::read(reader)?; + let key = Readable::read(reader)?; + let is_htlc = match >::read(reader)? { + 0 => true, + 1 => false, + _ => return Err(DecodeError::InvalidValue), + }; + let amount = Readable::read(reader)?; + TxMaterial::Revoked { + script, + pubkey, + key, + is_htlc, + amount + } + }, + 1 => { + let script = Readable::read(reader)?; + let key = Readable::read(reader)?; + let preimage = Readable::read(reader)?; + let amount = Readable::read(reader)?; + TxMaterial::RemoteHTLC { + script, + key, + preimage, + amount + } + }, + 2 => { + let script = Readable::read(reader)?; + let their_sig = Readable::read(reader)?; + let our_sig = Readable::read(reader)?; + let preimage = Readable::read(reader)?; + let amount = Readable::read(reader)?; + TxMaterial::LocalHTLC { + script, + sigs: (their_sig, our_sig), + preimage, + amount + } + } + _ => return Err(DecodeError::InvalidValue), + }; + let last_fee = Readable::read(reader)?; + our_claim_txn_waiting_first_conf.insert(outpoint, (height_target, tx_material, last_fee)); + } + let waiting_threshold_conf_len: u64 = Readable::read(reader)?; let mut onchain_events_waiting_threshold_conf = HashMap::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128)); for _ in 0..waiting_threshold_conf_len { @@ -2507,6 +2714,8 @@ impl ReadableArgs> for (Sha256dHash, ChannelM destination_script, + our_claim_txn_waiting_first_conf, + onchain_events_waiting_threshold_conf, last_block_hash, From 757bcc2951b6750d39ef1d841593be1e7f1c57cd Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Thu, 18 Jul 2019 17:27:48 -0400 Subject: [PATCH 10/10] Implement dynamic height timer for bump candidates txn We must adapt our delay between two bumps of claim txn in respect to the timelock encumbering the targeted outpoint. If HTLC or revoked output is near to expire, we should try to get our claim in every block. If it's reasonably in the future, we may give us more latency to bump --- src/ln/channelmonitor.rs | 55 ++++++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index f6d741afa10..5dddd1e1557 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -460,8 +460,10 @@ pub struct ChannelMonitor { // Used to track outpoint in the process of being claimed by our transactions. We need to scan all transactions // for inputs spending this. If height timer (u32) is expired and claim tx hasn't reached enough confirmations // before, use TxMaterial to regenerate a new claim tx with a satoshis-per-1000-weight-units higher than last - // one (u64). - our_claim_txn_waiting_first_conf: HashMap, + // one (u64), if timelock expiration (u32) is near, decrease height timer, the in-between bumps delay. + // Last field cached (u32) is height of outpoint confirmation, which is needed to flush this tracker + // in case of reorgs, given block timer are scaled on timer expiration we can't deduce from it original height. + our_claim_txn_waiting_first_conf: HashMap, // Used to track onchain events, i.e transactions parts of channels confirmed on chain, on which // we have to take actions once they reach enough confs. Key is a block height timer, i.e we enforce @@ -624,6 +626,15 @@ impl ChannelMonitor { tx_weight } + fn get_height_timer(current_height: u32, timelock_expiration: u32) -> u32 { + if timelock_expiration <= current_height || timelock_expiration - current_height <= 3 { + return current_height + 1 + } else if timelock_expiration - current_height <= 15 { + return current_height + 3 + } + current_height + 15 + } + #[inline] fn place_secret(idx: u64) -> u8 { for i in 0..48 { @@ -1129,6 +1140,8 @@ impl ChannelMonitor { } } writer.write_all(&byte_utils::be64_to_array(claim_tx_data.2))?; + writer.write_all(&byte_utils::be32_to_array(claim_tx_data.3))?; + writer.write_all(&byte_utils::be32_to_array(claim_tx_data.4))?; } writer.write_all(&byte_utils::be64_to_array(self.onchain_events_waiting_threshold_conf.len() as u64))?; @@ -1283,7 +1296,7 @@ impl ChannelMonitor { witness: Vec::new(), }); inputs_desc.push(InputDescriptors::RevokedOutput); - inputs_info.push((None, outp.value)); + inputs_info.push((None, outp.value, self.our_to_self_delay as u32)); total_value += outp.value; } else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() { spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH { @@ -1347,7 +1360,7 @@ impl ChannelMonitor { if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER { inputs.push(input); inputs_desc.push(if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }); - inputs_info.push((Some(idx), tx.output[transaction_output_index as usize].value)); + inputs_info.push((Some(idx), tx.output[transaction_output_index as usize].value, htlc.cltv_expiry)); total_value += tx.output[transaction_output_index as usize].value; } else { let mut single_htlc_tx = Transaction { @@ -1360,6 +1373,7 @@ impl ChannelMonitor { }), }; let predicted_weight = single_htlc_tx.get_weight() + Self::get_witnesses_weight(&[if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }]); + let height_timer = Self::get_height_timer(height, htlc.cltv_expiry); let mut used_feerate; if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) { let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx); @@ -1367,7 +1381,7 @@ impl ChannelMonitor { assert!(predicted_weight >= single_htlc_tx.get_weight()); match self.our_claim_txn_waiting_first_conf.entry(single_htlc_tx.input[0].previous_output.clone()) { hash_map::Entry::Occupied(_) => {}, - hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::Revoked { script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: true, amount: htlc.amount_msat / 1000 }, used_feerate)); } + hash_map::Entry::Vacant(entry) => { entry.insert((height_timer, TxMaterial::Revoked { script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: true, amount: htlc.amount_msat / 1000 }, used_feerate, htlc.cltv_expiry, height)); } } txn_to_broadcast.push(single_htlc_tx); } @@ -1444,9 +1458,10 @@ impl ChannelMonitor { for (input, info) in spend_tx.input.iter_mut().zip(inputs_info.iter()) { let (redeemscript, revocation_key) = sign_input!(sighash_parts, input, info.0, info.1); + let height_timer = Self::get_height_timer(height, info.2); match self.our_claim_txn_waiting_first_conf.entry(input.previous_output.clone()) { hash_map::Entry::Occupied(_) => {}, - hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::Revoked { script: redeemscript, pubkey: if info.0.is_some() { Some(revocation_pubkey) } else { None }, key: revocation_key, is_htlc: if info.0.is_some() { true } else { false }, amount: info.1 }, used_feerate)); } + hash_map::Entry::Vacant(entry) => { entry.insert((height_timer, TxMaterial::Revoked { script: redeemscript, pubkey: if info.0.is_some() { Some(revocation_pubkey) } else { None }, key: revocation_key, is_htlc: if info.0.is_some() { true } else { false }, amount: info.1 }, used_feerate, if !info.0.is_some() { height + info.2 } else { info.2 }, height)); } } } assert!(predicted_weight >= spend_tx.get_weight()); @@ -1610,7 +1625,7 @@ impl ChannelMonitor { if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER { inputs.push(input); inputs_desc.push(if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC }); - inputs_info.push((payment_preimage, tx.output[transaction_output_index as usize].value)); + inputs_info.push((payment_preimage, tx.output[transaction_output_index as usize].value, htlc.cltv_expiry)); total_value += tx.output[transaction_output_index as usize].value; } else { let mut single_htlc_tx = Transaction { @@ -1623,6 +1638,7 @@ impl ChannelMonitor { }), }; let predicted_weight = single_htlc_tx.get_weight() + Self::get_witnesses_weight(&[if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC }]); + let height_timer = Self::get_height_timer(height, htlc.cltv_expiry); let mut used_feerate; if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) { let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx); @@ -1634,7 +1650,7 @@ impl ChannelMonitor { }); match self.our_claim_txn_waiting_first_conf.entry(single_htlc_tx.input[0].previous_output.clone()) { hash_map::Entry::Occupied(_) => {}, - hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::RemoteHTLC { script: redeemscript, key: htlc_key, preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000 }, used_feerate)); } + hash_map::Entry::Vacant(entry) => { entry.insert((height_timer, TxMaterial::RemoteHTLC { script: redeemscript, key: htlc_key, preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000 }, used_feerate, htlc.cltv_expiry, height)); } } txn_to_broadcast.push(single_htlc_tx); } @@ -1662,6 +1678,7 @@ impl ChannelMonitor { }), }; let predicted_weight = timeout_tx.get_weight() + Self::get_witnesses_weight(&[InputDescriptors::ReceivedHTLC]); + let height_timer = Self::get_height_timer(height, htlc.cltv_expiry); let mut used_feerate; if subtract_high_prio_fee!(self, fee_estimator, timeout_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) { let sighash_parts = bip143::SighashComponents::new(&timeout_tx); @@ -1670,7 +1687,7 @@ impl ChannelMonitor { //TODO: track SpendableOutputDescriptor match self.our_claim_txn_waiting_first_conf.entry(timeout_tx.input[0].previous_output.clone()) { hash_map::Entry::Occupied(_) => {}, - hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::RemoteHTLC { script : redeemscript, key: htlc_key, preimage: None, amount: htlc.amount_msat / 1000 }, used_feerate)); } + hash_map::Entry::Vacant(entry) => { entry.insert((height_timer, TxMaterial::RemoteHTLC { script : redeemscript, key: htlc_key, preimage: None, amount: htlc.amount_msat / 1000 }, used_feerate, htlc.cltv_expiry, height)); } } } txn_to_broadcast.push(timeout_tx); @@ -1702,9 +1719,10 @@ impl ChannelMonitor { for (input, info) in spend_tx.input.iter_mut().zip(inputs_info.iter()) { let (redeemscript, htlc_key) = sign_input!(sighash_parts, input, info.1, (info.0).0.to_vec()); + let height_timer = Self::get_height_timer(height, info.2); match self.our_claim_txn_waiting_first_conf.entry(input.previous_output.clone()) { hash_map::Entry::Occupied(_) => {}, - hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::RemoteHTLC { script: redeemscript, key: htlc_key, preimage: Some(*(info.0)), amount: info.1}, used_feerate)); } + hash_map::Entry::Vacant(entry) => { entry.insert((height_timer, TxMaterial::RemoteHTLC { script: redeemscript, key: htlc_key, preimage: Some(*(info.0)), amount: info.1}, used_feerate, info.2, height)); } } } assert!(predicted_weight >= spend_tx.get_weight()); @@ -1808,15 +1826,16 @@ impl ChannelMonitor { assert!(predicted_weight >= spend_tx.get_weight()); let outpoint = BitcoinOutPoint { txid: spend_tx.txid(), vout: 0 }; let output = spend_tx.output[0].clone(); + let height_timer = Self::get_height_timer(height, self.their_to_self_delay.unwrap() as u32); // We can safely unwrap given we are past channel opening match self.our_claim_txn_waiting_first_conf.entry(spend_tx.input[0].previous_output.clone()) { hash_map::Entry::Occupied(_) => {}, - hash_map::Entry::Vacant(entry) => { entry.insert((height + 3, TxMaterial::Revoked { script: redeemscript, pubkey: None, key: revocation_key, is_htlc: false, amount: tx.output[0].value }, used_feerate)); } + hash_map::Entry::Vacant(entry) => { entry.insert((height_timer, TxMaterial::Revoked { script: redeemscript, pubkey: None, key: revocation_key, is_htlc: false, amount: tx.output[0].value }, used_feerate, height + self.our_to_self_delay as u32, height)); } } (Some(spend_tx), Some(SpendableOutputDescriptor::StaticOutput { outpoint, output })) } else { (None, None) } } - fn broadcast_by_local_state(&self, local_tx: &LocalSignedTx, per_commitment_point: &Option, delayed_payment_base_key: &Option, height: u32) -> (Vec, Vec, Vec, Vec<(BitcoinOutPoint, (u32, TxMaterial, u64))>) { + fn broadcast_by_local_state(&self, local_tx: &LocalSignedTx, per_commitment_point: &Option, delayed_payment_base_key: &Option, height: u32) -> (Vec, Vec, Vec, Vec<(BitcoinOutPoint, (u32, TxMaterial, u64, u32, u32))>) { let mut res = Vec::with_capacity(local_tx.htlc_outputs.len()); let mut spendable_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); let mut watch_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); @@ -1869,7 +1888,8 @@ impl ChannelMonitor { htlc_timeout_tx.input[0].witness.push(htlc_script.clone().into_bytes()); add_dynamic_output!(htlc_timeout_tx, 0); - pending_claims.push((htlc_timeout_tx.input[0].previous_output.clone(), (height + 3, TxMaterial::LocalHTLC { script: htlc_script, sigs: (*their_sig, *our_sig), preimage: None, amount: htlc.amount_msat / 1000}, 0))); + let height_timer = Self::get_height_timer(height, htlc.cltv_expiry); + pending_claims.push((htlc_timeout_tx.input[0].previous_output.clone(), (height_timer, TxMaterial::LocalHTLC { script: htlc_script, sigs: (*their_sig, *our_sig), preimage: None, amount: htlc.amount_msat / 1000}, 0, htlc.cltv_expiry, height))); res.push(htlc_timeout_tx); } else { if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) { @@ -1888,7 +1908,8 @@ impl ChannelMonitor { htlc_success_tx.input[0].witness.push(htlc_script.clone().into_bytes()); add_dynamic_output!(htlc_success_tx, 0); - pending_claims.push((htlc_success_tx.input[0].previous_output.clone(), (height + 3, TxMaterial::LocalHTLC { script: htlc_script, sigs: (*their_sig, *our_sig), preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000}, 0))); + let height_timer = Self::get_height_timer(height, htlc.cltv_expiry); + pending_claims.push((htlc_success_tx.input[0].previous_output.clone(), (height_timer, TxMaterial::LocalHTLC { script: htlc_script, sigs: (*their_sig, *our_sig), preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000}, 0, htlc.cltv_expiry, height))); res.push(htlc_success_tx); } } @@ -2190,7 +2211,7 @@ impl ChannelMonitor { //- htlc update there as failure-trigger tx (revoked commitment tx, non-revoked commitment tx, HTLC-timeout tx) has been disconnected //- our claim tx on a commitment tx output } - self.our_claim_txn_waiting_first_conf.retain(|_, ref mut v| if v.0 == height + 3 { false } else { true }); + self.our_claim_txn_waiting_first_conf.retain(|_, ref mut v| if v.3 == height { false } else { true }); self.last_block_hash = block_hash.clone(); } @@ -2659,7 +2680,9 @@ impl ReadableArgs> for (Sha256dHash, ChannelM _ => return Err(DecodeError::InvalidValue), }; let last_fee = Readable::read(reader)?; - our_claim_txn_waiting_first_conf.insert(outpoint, (height_target, tx_material, last_fee)); + let timelock_expiration = Readable::read(reader)?; + let height = Readable::read(reader)?; + our_claim_txn_waiting_first_conf.insert(outpoint, (height_target, tx_material, last_fee, timelock_expiration, height)); } let waiting_threshold_conf_len: u64 = Readable::read(reader)?;