Merge pull request #447 from ariard/2020-01-fix-weight-computation

Bound incoming HTLC witnessScript to min/max limits

diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs
index 19a07aaa19c2ea2c30f5498aee3c32278f7cb0c2..263547204c415a963e2223c50da0a9841f2e61e1 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 3d8962b4fe6c2b35c9f82f9f3e2ea27c85dde9f3..d2a0cae9ba9120afb8e834931b3debb16ef3e647 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -368,12 +368,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 1753e16dcc7351a1b0f50a7245086bf08a58c295..70f11405d0cb8399beb1c540acbe84ff20955982 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;
@@ -2677,10 +2676,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) => {
@@ -2871,7 +2870,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, .. } => {
@@ -2911,7 +2911,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 3936460f37eae318463543da2d45c566409bb2f1..eae4d3aa0688b9c8f13d34eaaeef264522929b58 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -876,6 +876,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 394e40da95c25390cf516011201acb373d5e5eea..50af478a5cf1b768149e1e11f6e67453508eddc9 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::features::{ChannelFeatures, InitFeatures};

diff --git a/lightning/src/util/macro_logger.rs b/lightning/src/util/macro_logger.rs
index ef7fbd9bb6c299b556edd768c9794ad4c2a5961e..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;
 
@@ -93,15 +94,29 @@ macro_rules! log_route {
 pub(crate) struct DebugTx<'a>(pub &'a Transaction);
 impl<'a> std::fmt::Display for DebugTx<'a> {
        fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
-               if self.0.input.len() == 1 && self.0.input[0].witness.last().unwrap().len() == 71 && (self.0.input[0].sequence >> 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 { write!(f, "closing tx")?; }
-               else if self.0.input.len() == 1 && self.0.input[0].witness.last().unwrap().len() == 133 && 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) && self.0.input[0].witness.len() == 5 { write!(f, "HTLC-success tx")?; }
-               else {
-                       for inp in &self.0.input {
-                               if inp.witness.last().unwrap().len() == 133 { write!(f, "preimage tx")?; break }
-                               else if inp.witness.last().unwrap().len() == 138 { write!(f, "timeout tx")?; break }
+               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 >> 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 {
+                               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 {
+                               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 {
+                               write!(f, "HTLC-success tx")?;
+                       } else {
+                               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 }
+                                       }
+                               }
+                               write!(f, "tx")?;
                        }
+               } else {
+                       write!(f, "INVALID TRANSACTION")?;
                }
                Ok(())
        }