Bound incoming HTLC witnessScript to min/max limits

Fix a crash where previously we weren't able to detect any accepted
HTLC if its witness-encoded cltv expiry was different from expected
ACCEPTED_HTLC_SCRIPT_WEIGHT. This should work for any cltv expiry
included between 0 and 16777216 on mainnet, testnet and regtest.

diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs
index 747e622f075f0bc86c9072d7e86b4df3e0c55101..a0cd3048f7e2dff07d8ba85fbfae20bd32782df5 100644 (file)
--- a/lightning/src/ln/chan_utils.rs
+++ b/lightning/src/ln/chan_utils.rs
@@ -25,6 +25,25 @@ use secp256k1;
 pub(super) const HTLC_SUCCESS_TX_WEIGHT: u64 = 703;
 pub(super) const HTLC_TIMEOUT_TX_WEIGHT: u64 = 663;
 
+#[derive(PartialEq)]
+pub(crate) enum HTLCType {
+       AcceptedHTLC,
+       OfferedHTLC
+}
+
+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)
+               } else {
+                       None
+               }
+       }
+}
+
 // Various functions for key derivation and transaction creation for use within channels. Primarily
 // used in Channel and ChannelMonitor.
 

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index bba65e52963114dad5b099d064a09405776cad61..030254fa0db95ce34e53ce6ce0da45e2324ee453 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -367,12 +367,6 @@ const B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT: u64 = 104; // prevout: 40, nSequence:
 /// it's 2^24.
 pub const MAX_FUNDING_SATOSHIS: u64 = (1 << 24);
 
-#[cfg(test)]
-pub const ACCEPTED_HTLC_SCRIPT_WEIGHT: usize = 138; //Here we have a diff due to HTLC CLTV expiry being < 2^15 in test
-#[cfg(not(test))]
-pub const ACCEPTED_HTLC_SCRIPT_WEIGHT: usize = 139;
-pub const OFFERED_HTLC_SCRIPT_WEIGHT: usize = 133;
-
 /// Used to return a simple Error back to ChannelManager. Will get converted to a
 /// msgs::ErrorAction::SendErrorMessage or msgs::ErrorAction::IgnoreError as appropriate with our
 /// channel_id in ChannelManager.

diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs
index b9527f599b246552056cd5b98a6866c0fb4514f8..94d13a8117ae4ac91aa532c3e1b1fb80e1ed4bad 100644 (file)
--- a/lightning/src/ln/channelmonitor.rs
+++ b/lightning/src/ln/channelmonitor.rs
@@ -31,9 +31,8 @@ use secp256k1;
 
 use ln::msgs::DecodeError;
 use ln::chan_utils;
-use ln::chan_utils::{HTLCOutputInCommitment, LocalCommitmentTransaction};
+use ln::chan_utils::{HTLCOutputInCommitment, LocalCommitmentTransaction, HTLCType};
 use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
-use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT};
 use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface, FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT};
 use chain::transaction::OutPoint;
 use chain::keysinterface::SpendableOutputDescriptor;
@@ -2676,10 +2675,10 @@ impl ChannelMonitor {
 
                'outer_loop: for input in &tx.input {
                        let mut payment_data = None;
-                       let revocation_sig_claim = (input.witness.len() == 3 && input.witness[2].len() == OFFERED_HTLC_SCRIPT_WEIGHT && input.witness[1].len() == 33)
-                               || (input.witness.len() == 3 && input.witness[2].len() == ACCEPTED_HTLC_SCRIPT_WEIGHT && input.witness[1].len() == 33);
-                       let accepted_preimage_claim = input.witness.len() == 5 && input.witness[4].len() == ACCEPTED_HTLC_SCRIPT_WEIGHT;
-                       let offered_preimage_claim = input.witness.len() == 3 && input.witness[2].len() == OFFERED_HTLC_SCRIPT_WEIGHT;
+                       let revocation_sig_claim = (input.witness.len() == 3 && HTLCType::scriptlen_to_htlctype(input.witness[2].len()) == Some(HTLCType::OfferedHTLC) && input.witness[1].len() == 33)
+                               || (input.witness.len() == 3 && HTLCType::scriptlen_to_htlctype(input.witness[2].len()) == Some(HTLCType::AcceptedHTLC) && input.witness[1].len() == 33);
+                       let accepted_preimage_claim = input.witness.len() == 5 && HTLCType::scriptlen_to_htlctype(input.witness[4].len()) == Some(HTLCType::AcceptedHTLC);
+                       let offered_preimage_claim = input.witness.len() == 3 && HTLCType::scriptlen_to_htlctype(input.witness[2].len()) == Some(HTLCType::OfferedHTLC);
 
                        macro_rules! log_claim {
                                ($tx_info: expr, $local_tx: expr, $htlc: expr, $source_avail: expr) => {
@@ -2870,7 +2869,8 @@ impl ChannelMonitor {
                for per_outp_material in cached_claim_datas.per_input_material.values() {
                        match per_outp_material {
                                &InputMaterial::Revoked { ref script, ref is_htlc, ref amount, .. } => {
-                                       inputs_witnesses_weight += Self::get_witnesses_weight(if !is_htlc { &[InputDescriptors::RevokedOutput] } else if script.len() == OFFERED_HTLC_SCRIPT_WEIGHT { &[InputDescriptors::RevokedOfferedHTLC] } else if script.len() == ACCEPTED_HTLC_SCRIPT_WEIGHT { &[InputDescriptors::RevokedReceivedHTLC] } else { &[] });
+                                       log_trace!(self, "Is HLTC ? {}", is_htlc);
+                                       inputs_witnesses_weight += Self::get_witnesses_weight(if !is_htlc { &[InputDescriptors::RevokedOutput] } else if HTLCType::scriptlen_to_htlctype(script.len()) == Some(HTLCType::OfferedHTLC) { &[InputDescriptors::RevokedOfferedHTLC] } else if HTLCType::scriptlen_to_htlctype(script.len()) == Some(HTLCType::AcceptedHTLC) { &[InputDescriptors::RevokedReceivedHTLC] } else { unreachable!() });
                                        amt += *amount;
                                },
                                &InputMaterial::RemoteHTLC { ref preimage, ref amount, .. } => {
@@ -2910,7 +2910,7 @@ impl ChannelMonitor {
                                                bumped_tx.input[i].witness.push(vec!(1));
                                        }
                                        bumped_tx.input[i].witness.push(script.clone().into_bytes());
-                                       log_trace!(self, "Going to broadcast bumped Penalty Transaction {} claiming revoked {} output {} from {} with new feerate {}", bumped_tx.txid(), if !is_htlc { "to_local" } else if script.len() == OFFERED_HTLC_SCRIPT_WEIGHT { "offered" } else if script.len() == ACCEPTED_HTLC_SCRIPT_WEIGHT { "received" } else { "" }, outp.vout, outp.txid, new_feerate);
+                                       log_trace!(self, "Going to broadcast bumped Penalty Transaction {} claiming revoked {} output {} from {} with new feerate {}", bumped_tx.txid(), if !is_htlc { "to_local" } else if HTLCType::scriptlen_to_htlctype(script.len()) == Some(HTLCType::OfferedHTLC) { "offered" } else if HTLCType::scriptlen_to_htlctype(script.len()) == Some(HTLCType::AcceptedHTLC) { "received" } else { "" }, outp.vout, outp.txid, new_feerate);
                                },
                                &InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref amount, ref locktime } => {
                                        if !preimage.is_some() { bumped_tx.lock_time = *locktime };

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index f3b3302eae39658853e7e11089905ca9c9f91ad6..6ca2dfd753a6667e8ce1b68d77873abaf05764e4 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -875,6 +875,9 @@ pub fn create_network(node_count: usize, node_config: &[Option<UserConfig>]) ->
        nodes
 }
 
+pub const ACCEPTED_HTLC_SCRIPT_WEIGHT: usize = 138; //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 }
 /// Tests that the given node has broadcast transactions for the given Channel

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index af656320e37c081da706861ef82acee8c9b4d2e2..c11f03348c0dab6dff3ff424b116db6448298a51 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -8,7 +8,7 @@ use chain::keysinterface::{KeysInterface, SpendableOutputDescriptor};
 use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC};
 use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,HTLCForwardInfo,RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT};
 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, Channel, ChannelError};
+use ln::channel::{Channel, ChannelError};
 use ln::onion_utils;
 use ln::router::{Route, RouteHop};
 use ln::msgs;

diff --git a/lightning/src/util/macro_logger.rs b/lightning/src/util/macro_logger.rs
index dba63663471d48e9181ed25d1e460c4c304b0153..e3a431ed54f8614d988c8ad2654d7389d9ed1e59 100644 (file)
--- a/lightning/src/util/macro_logger.rs
+++ b/lightning/src/util/macro_logger.rs
@@ -5,6 +5,7 @@ use bitcoin::blockdata::transaction::Transaction;
 use secp256k1::key::PublicKey;
 
 use ln::router::Route;
+use ln::chan_utils::HTLCType;
 
 use std;
 
@@ -99,18 +100,17 @@ impl<'a> std::fmt::Display for DebugTx<'a> {
                                write!(f, "commitment tx")?;
                        } else if self.0.input.len() == 1 && self.0.input[0].witness.last().unwrap().len() == 71 {
                                write!(f, "closing tx")?;
-                       } else if self.0.input.len() == 1 && self.0.input[0].witness.last().unwrap().len() == 133 &&
+                       } 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 {
                                write!(f, "HTLC-timeout tx")?;
-                       } else if self.0.input.len() == 1 &&
-                                       (self.0.input[0].witness.last().unwrap().len() == 138 || self.0.input[0].witness.last().unwrap().len() == 139) &&
+                       } 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 {
                                write!(f, "HTLC-success tx")?;
                        } else {
                                for inp in &self.0.input {
                                        if !inp.witness.is_empty() {
-                                               if inp.witness.last().unwrap().len() == 133 { write!(f, "preimage-")?; break }
-                                               else if inp.witness.last().unwrap().len() == 138 { write!(f, "timeout-")?; break }
+                                               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 }
                                        }
                                }
                                write!(f, "tx")?;