From cd0d19c005ee4fa11de93a2bca621eda6b81ce95 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 25 Aug 2022 13:23:29 -0700 Subject: [PATCH] Update HTLC script detection to check for anchor output variants --- lightning/src/chain/channelmonitor.rs | 26 +++---- lightning/src/ln/chan_utils.rs | 84 +++++++++++++++++++---- lightning/src/ln/functional_test_utils.rs | 1 - lightning/src/ln/functional_tests.rs | 7 +- lightning/src/util/macro_logger.rs | 33 +++++---- 5 files changed, 103 insertions(+), 48 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 857495630..bfb6e1ab3 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -37,7 +37,7 @@ use bitcoin::secp256k1; use ln::{PaymentHash, PaymentPreimage}; use ln::msgs::DecodeError; use ln::chan_utils; -use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCType, ChannelTransactionParameters, HolderCommitmentTransaction}; +use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCClaim, ChannelTransactionParameters, HolderCommitmentTransaction}; use ln::channelmanager::HTLCSource; use chain; use chain::{BestBlock, WatchedOutput}; @@ -3159,25 +3159,15 @@ impl ChannelMonitorImpl { fn is_resolving_htlc_output(&mut self, tx: &Transaction, height: u32, logger: &L) where L::Target: Logger { 'outer_loop: for input in &tx.input { let mut payment_data = None; - let witness_items = input.witness.len(); - let htlctype = input.witness.last().map(|w| w.len()).and_then(HTLCType::scriptlen_to_htlctype); - let prev_last_witness_len = input.witness.second_to_last().map(|w| w.len()).unwrap_or(0); - let revocation_sig_claim = (witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && prev_last_witness_len == 33) - || (witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && prev_last_witness_len == 33); - let accepted_preimage_claim = witness_items == 5 && htlctype == Some(HTLCType::AcceptedHTLC) - && input.witness.second_to_last().unwrap().len() == 32; - #[cfg(not(fuzzing))] - let accepted_timeout_claim = witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && !revocation_sig_claim; - let offered_preimage_claim = witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && - !revocation_sig_claim && input.witness.second_to_last().unwrap().len() == 32; - - #[cfg(not(fuzzing))] - let offered_timeout_claim = witness_items == 5 && htlctype == Some(HTLCType::OfferedHTLC); + let htlc_claim = HTLCClaim::from_witness(&input.witness); + let revocation_sig_claim = htlc_claim == Some(HTLCClaim::Revocation); + let accepted_preimage_claim = htlc_claim == Some(HTLCClaim::AcceptedPreimage); + let accepted_timeout_claim = htlc_claim == Some(HTLCClaim::AcceptedTimeout); + let offered_preimage_claim = htlc_claim == Some(HTLCClaim::OfferedPreimage); + let offered_timeout_claim = htlc_claim == Some(HTLCClaim::OfferedTimeout); let mut payment_preimage = PaymentPreimage([0; 32]); - if accepted_preimage_claim { - payment_preimage.0.copy_from_slice(input.witness.second_to_last().unwrap()); - } else if offered_preimage_claim { + if offered_preimage_claim || accepted_preimage_claim { payment_preimage.0.copy_from_slice(input.witness.second_to_last().unwrap()); } diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 722693a43..df0378938 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -42,6 +42,12 @@ use chain; use util::crypto::sign; pub(crate) const MAX_HTLCS: u16 = 483; +pub(crate) const OFFERED_HTLC_SCRIPT_WEIGHT: usize = 133; +pub(crate) const OFFERED_HTLC_SCRIPT_WEIGHT_ANCHORS: usize = 136; +// The weight of `accepted_htlc_script` can vary in function of its CLTV argument value. We define a +// range that encompasses both its non-anchors and anchors variants. +pub(crate) const MIN_ACCEPTED_HTLC_SCRIPT_WEIGHT: usize = 136; +pub(crate) const MAX_ACCEPTED_HTLC_SCRIPT_WEIGHT: usize = 143; /// Gets the weight for an HTLC-Success transaction. #[inline] @@ -60,18 +66,72 @@ pub fn htlc_timeout_tx_weight(opt_anchors: bool) -> u64 { } #[derive(PartialEq)] -pub(crate) enum HTLCType { - AcceptedHTLC, - OfferedHTLC +pub(crate) enum HTLCClaim { + OfferedTimeout, + OfferedPreimage, + AcceptedTimeout, + AcceptedPreimage, + Revocation, } -impl HTLCType { - /// Check if a given tx witnessScript len matchs one of a pre-signed HTLC - pub(crate) fn scriptlen_to_htlctype(witness_script_len: usize) -> Option { - if witness_script_len == 133 { - Some(HTLCType::OfferedHTLC) - } else if witness_script_len >= 136 && witness_script_len <= 139 { - Some(HTLCType::AcceptedHTLC) +impl HTLCClaim { + /// Check if a given input witness attempts to claim a HTLC. + pub(crate) fn from_witness(witness: &Witness) -> Option { + debug_assert_eq!(OFFERED_HTLC_SCRIPT_WEIGHT_ANCHORS, MIN_ACCEPTED_HTLC_SCRIPT_WEIGHT); + if witness.len() < 2 { + return None; + } + let witness_script = witness.last().unwrap(); + let second_to_last = witness.second_to_last().unwrap(); + if witness_script.len() == OFFERED_HTLC_SCRIPT_WEIGHT { + if witness.len() == 3 && second_to_last.len() == 33 { + // + Some(Self::Revocation) + } else if witness.len() == 3 && second_to_last.len() == 32 { + // + Some(Self::OfferedPreimage) + } else if witness.len() == 5 && second_to_last.len() == 0 { + // 0 <> + Some(Self::OfferedTimeout) + } else { + None + } + } else if witness_script.len() == OFFERED_HTLC_SCRIPT_WEIGHT_ANCHORS { + // It's possible for the weight of `offered_htlc_script` and `accepted_htlc_script` to + // match so we check for both here. + if witness.len() == 3 && second_to_last.len() == 33 { + // + Some(Self::Revocation) + } else if witness.len() == 3 && second_to_last.len() == 32 { + // + Some(Self::OfferedPreimage) + } else if witness.len() == 5 && second_to_last.len() == 0 { + // 0 <> + Some(Self::OfferedTimeout) + } else if witness.len() == 3 && second_to_last.len() == 0 { + // <> + Some(Self::AcceptedTimeout) + } else if witness.len() == 5 && second_to_last.len() == 32 { + // 0 + Some(Self::AcceptedPreimage) + } else { + None + } + } else if witness_script.len() > MIN_ACCEPTED_HTLC_SCRIPT_WEIGHT && + witness_script.len() <= MAX_ACCEPTED_HTLC_SCRIPT_WEIGHT { + // Handle remaining range of ACCEPTED_HTLC_SCRIPT_WEIGHT. + if witness.len() == 3 && second_to_last.len() == 33 { + // + Some(Self::Revocation) + } else if witness.len() == 3 && second_to_last.len() == 0 { + // <> + Some(Self::AcceptedTimeout) + } else if witness.len() == 5 && second_to_last.len() == 32 { + // 0 + Some(Self::AcceptedPreimage) + } else { + None + } } else { None } @@ -285,7 +345,7 @@ pub fn derive_public_key(secp_ctx: &Secp256k1, per_com /// Derives a per-commitment-transaction revocation key from its constituent parts. /// -/// Only the cheating participant owns a valid witness to propagate a revoked +/// Only the cheating participant owns a valid witness to propagate a revoked /// commitment transaction, thus per_commitment_secret always come from cheater /// and revocation_base_secret always come from punisher, which is the broadcaster /// of the transaction spending with this key knowledge. @@ -320,7 +380,7 @@ pub fn derive_private_revocation_key(secp_ctx: &Secp256k1 /// the public equivalend of derive_private_revocation_key - using only public keys to derive a /// public key instead of private keys. /// -/// Only the cheating participant owns a valid witness to propagate a revoked +/// Only the cheating participant owns a valid witness to propagate a revoked /// commitment transaction, thus per_commitment_point always come from cheater /// and revocation_base_point always come from punisher, which is the broadcaster /// of the transaction spending with this key knowledge. diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 554e60ba7..ce04e5f77 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2118,7 +2118,6 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec = channelmonitor::deliberately_bogus_accepted_htlc_witness_program(); - assert!(chan_utils::HTLCType::scriptlen_to_htlctype(wit_program.len()).unwrap() == - chan_utils::HTLCType::AcceptedHTLC); - - let wit_program_script: Script = wit_program.clone().into(); + let wit_program_script: Script = wit_program.into(); for output in tx.output.iter_mut() { // Make the confirmed funding transaction have a bogus script_pubkey output.script_pubkey = Script::new_v0_p2wsh(&wit_program_script.wscript_hash()); diff --git a/lightning/src/util/macro_logger.rs b/lightning/src/util/macro_logger.rs index cd79a3f7b..01a0d3ff4 100644 --- a/lightning/src/util/macro_logger.rs +++ b/lightning/src/util/macro_logger.rs @@ -15,7 +15,7 @@ use bitcoin::blockdata::transaction::Transaction; use bitcoin::secp256k1::PublicKey; use routing::router::Route; -use ln::chan_utils::HTLCType; +use ln::chan_utils::HTLCClaim; use util::logger::DebugBytes; pub(crate) struct DebugPubKey<'a>(pub &'a PublicKey); @@ -90,25 +90,34 @@ pub(crate) struct DebugTx<'a>(pub &'a Transaction); impl<'a> core::fmt::Display for DebugTx<'a> { fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { if self.0.input.len() >= 1 && self.0.input.iter().any(|i| !i.witness.is_empty()) { - if self.0.input.len() == 1 && self.0.input[0].witness.last().unwrap().len() == 71 && - (self.0.input[0].sequence.0 >> 8*3) as u8 == 0x80 { + let first_input = &self.0.input[0]; + let witness_script_len = first_input.witness.last().unwrap_or(&[]).len(); + if self.0.input.len() == 1 && witness_script_len == 71 && + (first_input.sequence.0 >> 8*3) as u8 == 0x80 { write!(f, "commitment tx ")?; - } else if self.0.input.len() == 1 && self.0.input[0].witness.last().unwrap().len() == 71 { + } else if self.0.input.len() == 1 && witness_script_len == 71 { write!(f, "closing tx ")?; - } else if self.0.input.len() == 1 && HTLCType::scriptlen_to_htlctype(self.0.input[0].witness.last().unwrap().len()) == Some(HTLCType::OfferedHTLC) && - self.0.input[0].witness.len() == 5 { + } else if self.0.input.len() == 1 && HTLCClaim::from_witness(&first_input.witness) == Some(HTLCClaim::OfferedTimeout) { write!(f, "HTLC-timeout tx ")?; - } else if self.0.input.len() == 1 && HTLCType::scriptlen_to_htlctype(self.0.input[0].witness.last().unwrap().len()) == Some(HTLCType::AcceptedHTLC) && - self.0.input[0].witness.len() == 5 { + } else if self.0.input.len() == 1 && HTLCClaim::from_witness(&first_input.witness) == Some(HTLCClaim::AcceptedPreimage) { write!(f, "HTLC-success tx ")?; } else { + let mut num_preimage = 0; + let mut num_timeout = 0; + let mut num_revoked = 0; for inp in &self.0.input { - if !inp.witness.is_empty() { - if HTLCType::scriptlen_to_htlctype(inp.witness.last().unwrap().len()) == Some(HTLCType::OfferedHTLC) { write!(f, "preimage-")?; break } - else if HTLCType::scriptlen_to_htlctype(inp.witness.last().unwrap().len()) == Some(HTLCType::AcceptedHTLC) { write!(f, "timeout-")?; break } + let htlc_claim = HTLCClaim::from_witness(&inp.witness); + match htlc_claim { + Some(HTLCClaim::AcceptedPreimage)|Some(HTLCClaim::OfferedPreimage) => num_preimage += 1, + Some(HTLCClaim::AcceptedTimeout)|Some(HTLCClaim::OfferedTimeout) => num_timeout += 1, + Some(HTLCClaim::Revocation) => num_revoked += 1, + None => continue, } } - write!(f, "tx ")?; + if num_preimage > 0 || num_timeout > 0 || num_revoked > 0 { + write!(f, "HTLC claim tx ({} preimage, {} timeout, {} revoked)", + num_preimage, num_timeout, num_revoked)?; + } } } else { debug_assert!(false, "We should never generate unknown transaction types"); -- 2.39.5