From fbc7885a97ce3807e390ee5427908d7ac069042a Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Tue, 14 Jan 2020 13:47:01 -0500 Subject: [PATCH] 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. --- lightning/src/ln/chan_utils.rs | 19 +++++++++++++++++++ lightning/src/ln/channel.rs | 6 ------ lightning/src/ln/channelmonitor.rs | 16 ++++++++-------- lightning/src/ln/functional_test_utils.rs | 3 +++ lightning/src/ln/functional_tests.rs | 2 +- lightning/src/util/macro_logger.rs | 10 +++++----- 6 files changed, 36 insertions(+), 20 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 747e622f..a0cd3048 100644 --- 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 { + 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 bba65e52..030254fa 100644 --- 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 b9527f59..94d13a81 100644 --- 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 f3b3302e..6ca2dfd7 100644 --- 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]) -> 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 af656320..c11f0334 100644 --- 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 dba63663..e3a431ed 100644 --- 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")?; -- 2.30.2