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.

Author: Antoine Riard

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(super) enum HTLCType {
+	AcceptedHTLC,
+	OfferedHTLC,
+	NoneHTLC,
+}
+
+impl HTLCType {
+	pub(super) fn weight_to_htlctype(witness_script_len: usize) ->  HTLCType {
+		if witness_script_len == 133 {
+			HTLCType::OfferedHTLC
+		} else if witness_script_len >= 136 && witness_script_len <= 139 {
+			HTLCType::AcceptedHTLC
+		} else {
+			HTLCType::NoneHTLC
+		}
+	}
+}
+
 // Various functions for key derivation and transaction creation for use within channels. Primarily
 // used in Channel and ChannelMonitor.
 

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.

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::weight_to_htlctype(input.witness[2].len()) == HTLCType::OfferedHTLC && input.witness[1].len() == 33)
+				|| (input.witness.len() == 3 && HTLCType::weight_to_htlctype(input.witness[2].len()) == HTLCType::AcceptedHTLC && input.witness[1].len() == 33);
+			let accepted_preimage_claim = input.witness.len() == 5 && HTLCType::weight_to_htlctype(input.witness[4].len()) == HTLCType::AcceptedHTLC;
+			let offered_preimage_claim = input.witness.len() == 3 && HTLCType::weight_to_htlctype(input.witness[2].len()) == 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::weight_to_htlctype(script.len()) == HTLCType::OfferedHTLC { &[InputDescriptors::RevokedOfferedHTLC] } else if HTLCType::weight_to_htlctype(script.len()) == 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::weight_to_htlctype(script.len()) == HTLCType::OfferedHTLC { "offered" } else if HTLCType::weight_to_htlctype(script.len()) == 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

@@ -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

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;