Update HTLC script detection to check for anchor output variants
authorWilmer Paulino <wilmer.paulino@gmail.com>
Thu, 25 Aug 2022 20:23:29 +0000 (13:23 -0700)
committerWilmer Paulino <wilmer.paulino@gmail.com>
Tue, 13 Sep 2022 17:58:32 +0000 (10:58 -0700)
lightning/src/chain/channelmonitor.rs
lightning/src/ln/chan_utils.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/util/macro_logger.rs

index 857495630259d542af26e96d7cd80a59e90f713e..bfb6e1ab3a8b72f363d948c539fd2ee212540bfa 100644 (file)
@@ -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<Signer: Sign> ChannelMonitorImpl<Signer> {
        fn is_resolving_htlc_output<L: Deref>(&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());
                        }
 
index 722693a430e57ebe8f5edae4c464ea335a1d5299..df0378938b42eeb0b1ff0fbd1413f6f140617a12 100644 (file)
@@ -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<HTLCType> {
-               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<Self> {
+               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 {
+                               // <revocation sig> <revocationpubkey> <witness_script>
+                               Some(Self::Revocation)
+                       } else if witness.len() == 3 && second_to_last.len() == 32 {
+                               // <remotehtlcsig> <payment_preimage> <witness_script>
+                               Some(Self::OfferedPreimage)
+                       } else if witness.len() == 5 && second_to_last.len() == 0 {
+                               // 0 <remotehtlcsig> <localhtlcsig> <> <witness_script>
+                               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 {
+                               // <revocation sig> <revocationpubkey> <witness_script>
+                               Some(Self::Revocation)
+                       } else if witness.len() == 3 && second_to_last.len() == 32 {
+                               // <remotehtlcsig> <payment_preimage> <witness_script>
+                               Some(Self::OfferedPreimage)
+                       } else if witness.len() == 5 && second_to_last.len() == 0 {
+                               // 0 <remotehtlcsig> <localhtlcsig> <> <witness_script>
+                               Some(Self::OfferedTimeout)
+                       } else if witness.len() == 3 && second_to_last.len() == 0 {
+                               // <remotehtlcsig> <> <witness_script>
+                               Some(Self::AcceptedTimeout)
+                       } else if witness.len() == 5 && second_to_last.len() == 32 {
+                               // 0 <remotehtlcsig> <localhtlcsig> <payment_preimage> <witness_script>
+                               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 {
+                               // <revocation sig> <revocationpubkey> <witness_script>
+                               Some(Self::Revocation)
+                       } else if witness.len() == 3 && second_to_last.len() == 0 {
+                               // <remotehtlcsig> <> <witness_script>
+                               Some(Self::AcceptedTimeout)
+                       } else if witness.len() == 5 && second_to_last.len() == 32 {
+                               // 0 <remotehtlcsig> <localhtlcsig> <payment_preimage> <witness_script>
+                               Some(Self::AcceptedPreimage)
+                       } else {
+                               None
+                       }
                } else {
                        None
                }
@@ -285,7 +345,7 @@ pub fn derive_public_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, 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<T: secp256k1::Signing>(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.
index 554e60ba7379117c6b1842f01416fbca11bc7718..ce04e5f7764a6601f5eb1c9e6d135fc74f0de349 100644 (file)
@@ -2118,7 +2118,6 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec<NodeC
 
 // Note that the following only works for CLTV values up to 128
 pub const ACCEPTED_HTLC_SCRIPT_WEIGHT: usize = 137; //Here we have a diff due to HTLC CLTV expiry being < 2^15 in test
-pub const OFFERED_HTLC_SCRIPT_WEIGHT: usize = 133;
 
 #[derive(PartialEq)]
 pub enum HTLCType { NONE, TIMEOUT, SUCCESS }
index fb9ce86234c90b91de765cfe6d714bc08b7fb128..2f2c6482cb873a823005e917a81280cb1e31c86a 100644 (file)
@@ -23,7 +23,7 @@ use ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONC
 use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, PAYMENT_EXPIRY_BLOCKS };
 use ln::channel::{Channel, ChannelError};
 use ln::{chan_utils, onion_utils};
-use ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment};
+use ln::chan_utils::{OFFERED_HTLC_SCRIPT_WEIGHT, htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment};
 use routing::gossip::{NetworkGraph, NetworkUpdate};
 use routing::router::{PaymentParameters, Route, RouteHop, RouteParameters, find_route, get_route};
 use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
@@ -9518,10 +9518,7 @@ fn test_invalid_funding_tx() {
        // a panic as we'd try to extract a 32 byte preimage from a witness element without checking
        // its length.
        let mut wit_program: Vec<u8> = 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());
index cd79a3f7bba8b3d500bf7036c1a202e4d6f2cd71..01a0d3ff45f192b4744a8d9ef20eb614bf3f169d 100644 (file)
@@ -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");