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

Bound incoming HTLC witnessScript to min/max limits

Author: TheBlueMatt

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.
 

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.

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 };

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

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};

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(())
 	}