Skip to content

Commit cd0d19c

Browse files
wpaulinowpaulino
committed
Update HTLC script detection to check for anchor output variants
1 parent a447965 commit cd0d19c

File tree

5 files changed

+103
-48
lines changed

5 files changed

+103
-48
lines changed

‎lightning/src/chain/channelmonitor.rs

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ use bitcoin::secp256k1;
3737
use ln::{PaymentHash, PaymentPreimage};
3838
use ln::msgs::DecodeError;
3939
use ln::chan_utils;
40-
use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCType, ChannelTransactionParameters, HolderCommitmentTransaction};
40+
use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCClaim, ChannelTransactionParameters, HolderCommitmentTransaction};
4141
use ln::channelmanager::HTLCSource;
4242
use chain;
4343
use chain::{BestBlock, WatchedOutput};
@@ -3159,25 +3159,15 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
31593159
fn is_resolving_htlc_output<L: Deref>(&mut self, tx: &Transaction, height: u32, logger: &L) where L::Target: Logger {
31603160
'outer_loop: for input in &tx.input {
31613161
let mut payment_data = None;
3162-
let witness_items = input.witness.len();
3163-
let htlctype = input.witness.last().map(|w| w.len()).and_then(HTLCType::scriptlen_to_htlctype);
3164-
let prev_last_witness_len = input.witness.second_to_last().map(|w| w.len()).unwrap_or(0);
3165-
let revocation_sig_claim = (witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && prev_last_witness_len == 33)
3166-
|| (witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && prev_last_witness_len == 33);
3167-
let accepted_preimage_claim = witness_items == 5 && htlctype == Some(HTLCType::AcceptedHTLC)
3168-
&& input.witness.second_to_last().unwrap().len() == 32;
3169-
#[cfg(not(fuzzing))]
3170-
let accepted_timeout_claim = witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && !revocation_sig_claim;
3171-
let offered_preimage_claim = witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) &&
3172-
!revocation_sig_claim && input.witness.second_to_last().unwrap().len() == 32;
3173-
3174-
#[cfg(not(fuzzing))]
3175-
let offered_timeout_claim = witness_items == 5 && htlctype == Some(HTLCType::OfferedHTLC);
3162+
let htlc_claim = HTLCClaim::from_witness(&input.witness);
3163+
let revocation_sig_claim = htlc_claim == Some(HTLCClaim::Revocation);
3164+
let accepted_preimage_claim = htlc_claim == Some(HTLCClaim::AcceptedPreimage);
3165+
let accepted_timeout_claim = htlc_claim == Some(HTLCClaim::AcceptedTimeout);
3166+
let offered_preimage_claim = htlc_claim == Some(HTLCClaim::OfferedPreimage);
3167+
let offered_timeout_claim = htlc_claim == Some(HTLCClaim::OfferedTimeout);
31763168

31773169
let mut payment_preimage = PaymentPreimage([0; 32]);
3178-
if accepted_preimage_claim {
3179-
payment_preimage.0.copy_from_slice(input.witness.second_to_last().unwrap());
3180-
} else if offered_preimage_claim {
3170+
if offered_preimage_claim || accepted_preimage_claim {
31813171
payment_preimage.0.copy_from_slice(input.witness.second_to_last().unwrap());
31823172
}
31833173

‎lightning/src/ln/chan_utils.rs

Lines changed: 72 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ use chain;
4242
use util::crypto::sign;
4343

4444
pub(crate) const MAX_HTLCS: u16 = 483;
45+
pub(crate) const OFFERED_HTLC_SCRIPT_WEIGHT: usize = 133;
46+
pub(crate) const OFFERED_HTLC_SCRIPT_WEIGHT_ANCHORS: usize = 136;
47+
// The weight of `accepted_htlc_script` can vary in function of its CLTV argument value. We define a
48+
// range that encompasses both its non-anchors and anchors variants.
49+
pub(crate) const MIN_ACCEPTED_HTLC_SCRIPT_WEIGHT: usize = 136;
50+
pub(crate) const MAX_ACCEPTED_HTLC_SCRIPT_WEIGHT: usize = 143;
4551

4652
/// Gets the weight for an HTLC-Success transaction.
4753
#[inline]
@@ -60,18 +66,72 @@ pub fn htlc_timeout_tx_weight(opt_anchors: bool) -> u64 {
6066
}
6167

6268
#[derive(PartialEq)]
63-
pub(crate) enum HTLCType {
64-
AcceptedHTLC,
65-
OfferedHTLC
69+
pub(crate) enum HTLCClaim {
70+
OfferedTimeout,
71+
OfferedPreimage,
72+
AcceptedTimeout,
73+
AcceptedPreimage,
74+
Revocation,
6675
}
6776

68-
impl HTLCType {
69-
/// Check if a given tx witnessScript len matchs one of a pre-signed HTLC
70-
pub(crate) fn scriptlen_to_htlctype(witness_script_len: usize) -> Option<HTLCType> {
71-
if witness_script_len == 133 {
72-
Some(HTLCType::OfferedHTLC)
73-
} else if witness_script_len >= 136 && witness_script_len <= 139 {
74-
Some(HTLCType::AcceptedHTLC)
77+
impl HTLCClaim {
78+
/// Check if a given input witness attempts to claim a HTLC.
79+
pub(crate) fn from_witness(witness: &Witness) -> Option<Self> {
80+
debug_assert_eq!(OFFERED_HTLC_SCRIPT_WEIGHT_ANCHORS, MIN_ACCEPTED_HTLC_SCRIPT_WEIGHT);
81+
if witness.len() < 2 {
82+
return None;
83+
}
84+
let witness_script = witness.last().unwrap();
85+
let second_to_last = witness.second_to_last().unwrap();
86+
if witness_script.len() == OFFERED_HTLC_SCRIPT_WEIGHT {
87+
if witness.len() == 3 && second_to_last.len() == 33 {
88+
// <revocation sig> <revocationpubkey> <witness_script>
89+
Some(Self::Revocation)
90+
} else if witness.len() == 3 && second_to_last.len() == 32 {
91+
// <remotehtlcsig> <payment_preimage> <witness_script>
92+
Some(Self::OfferedPreimage)
93+
} else if witness.len() == 5 && second_to_last.len() == 0 {
94+
// 0 <remotehtlcsig> <localhtlcsig> <> <witness_script>
95+
Some(Self::OfferedTimeout)
96+
} else {
97+
None
98+
}
99+
} else if witness_script.len() == OFFERED_HTLC_SCRIPT_WEIGHT_ANCHORS {
100+
// It's possible for the weight of `offered_htlc_script` and `accepted_htlc_script` to
101+
// match so we check for both here.
102+
if witness.len() == 3 && second_to_last.len() == 33 {
103+
// <revocation sig> <revocationpubkey> <witness_script>
104+
Some(Self::Revocation)
105+
} else if witness.len() == 3 && second_to_last.len() == 32 {
106+
// <remotehtlcsig> <payment_preimage> <witness_script>
107+
Some(Self::OfferedPreimage)
108+
} else if witness.len() == 5 && second_to_last.len() == 0 {
109+
// 0 <remotehtlcsig> <localhtlcsig> <> <witness_script>
110+
Some(Self::OfferedTimeout)
111+
} else if witness.len() == 3 && second_to_last.len() == 0 {
112+
// <remotehtlcsig> <> <witness_script>
113+
Some(Self::AcceptedTimeout)
114+
} else if witness.len() == 5 && second_to_last.len() == 32 {
115+
// 0 <remotehtlcsig> <localhtlcsig> <payment_preimage> <witness_script>
116+
Some(Self::AcceptedPreimage)
117+
} else {
118+
None
119+
}
120+
} else if witness_script.len() > MIN_ACCEPTED_HTLC_SCRIPT_WEIGHT &&
121+
witness_script.len() <= MAX_ACCEPTED_HTLC_SCRIPT_WEIGHT {
122+
// Handle remaining range of ACCEPTED_HTLC_SCRIPT_WEIGHT.
123+
if witness.len() == 3 && second_to_last.len() == 33 {
124+
// <revocation sig> <revocationpubkey> <witness_script>
125+
Some(Self::Revocation)
126+
} else if witness.len() == 3 && second_to_last.len() == 0 {
127+
// <remotehtlcsig> <> <witness_script>
128+
Some(Self::AcceptedTimeout)
129+
} else if witness.len() == 5 && second_to_last.len() == 32 {
130+
// 0 <remotehtlcsig> <localhtlcsig> <payment_preimage> <witness_script>
131+
Some(Self::AcceptedPreimage)
132+
} else {
133+
None
134+
}
75135
} else {
76136
None
77137
}
@@ -285,7 +345,7 @@ pub fn derive_public_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, per_com
285345

286346
/// Derives a per-commitment-transaction revocation key from its constituent parts.
287347
///
288-
/// Only the cheating participant owns a valid witness to propagate a revoked
348+
/// Only the cheating participant owns a valid witness to propagate a revoked
289349
/// commitment transaction, thus per_commitment_secret always come from cheater
290350
/// and revocation_base_secret always come from punisher, which is the broadcaster
291351
/// of the transaction spending with this key knowledge.
@@ -320,7 +380,7 @@ pub fn derive_private_revocation_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1
320380
/// the public equivalend of derive_private_revocation_key - using only public keys to derive a
321381
/// public key instead of private keys.
322382
///
323-
/// Only the cheating participant owns a valid witness to propagate a revoked
383+
/// Only the cheating participant owns a valid witness to propagate a revoked
324384
/// commitment transaction, thus per_commitment_point always come from cheater
325385
/// and revocation_base_point always come from punisher, which is the broadcaster
326386
/// of the transaction spending with this key knowledge.

‎lightning/src/ln/functional_test_utils.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2118,7 +2118,6 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec<NodeC
21182118

21192119
// Note that the following only works for CLTV values up to 128
21202120
pub const ACCEPTED_HTLC_SCRIPT_WEIGHT: usize = 137; //Here we have a diff due to HTLC CLTV expiry being < 2^15 in test
2121-
pub const OFFERED_HTLC_SCRIPT_WEIGHT: usize = 133;
21222121

21232122
#[derive(PartialEq)]
21242123
pub enum HTLCType { NONE, TIMEOUT, SUCCESS }

‎lightning/src/ln/functional_tests.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONC
2323
use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, PAYMENT_EXPIRY_BLOCKS };
2424
use ln::channel::{Channel, ChannelError};
2525
use ln::{chan_utils, onion_utils};
26-
use ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment};
26+
use ln::chan_utils::{OFFERED_HTLC_SCRIPT_WEIGHT, htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment};
2727
use routing::gossip::{NetworkGraph, NetworkUpdate};
2828
use routing::router::{PaymentParameters, Route, RouteHop, RouteParameters, find_route, get_route};
2929
use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
@@ -9518,10 +9518,7 @@ fn test_invalid_funding_tx() {
95189518
// a panic as we'd try to extract a 32 byte preimage from a witness element without checking
95199519
// its length.
95209520
let mut wit_program: Vec<u8> = channelmonitor::deliberately_bogus_accepted_htlc_witness_program();
9521-
assert!(chan_utils::HTLCType::scriptlen_to_htlctype(wit_program.len()).unwrap() ==
9522-
chan_utils::HTLCType::AcceptedHTLC);
9523-
9524-
let wit_program_script: Script = wit_program.clone().into();
9521+
let wit_program_script: Script = wit_program.into();
95259522
for output in tx.output.iter_mut() {
95269523
// Make the confirmed funding transaction have a bogus script_pubkey
95279524
output.script_pubkey = Script::new_v0_p2wsh(&wit_program_script.wscript_hash());

‎lightning/src/util/macro_logger.rs

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use bitcoin::blockdata::transaction::Transaction;
1515
use bitcoin::secp256k1::PublicKey;
1616

1717
use routing::router::Route;
18-
use ln::chan_utils::HTLCType;
18+
use ln::chan_utils::HTLCClaim;
1919
use util::logger::DebugBytes;
2020

2121
pub(crate) struct DebugPubKey<'a>(pub &'a PublicKey);
@@ -90,25 +90,34 @@ pub(crate) struct DebugTx<'a>(pub &'a Transaction);
9090
impl<'a> core::fmt::Display for DebugTx<'a> {
9191
fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
9292
if self.0.input.len() >= 1 && self.0.input.iter().any(|i| !i.witness.is_empty()) {
93-
if self.0.input.len() == 1 && self.0.input[0].witness.last().unwrap().len() == 71 &&
94-
(self.0.input[0].sequence.0 >> 8*3) as u8 == 0x80 {
93+
let first_input = &self.0.input[0];
94+
let witness_script_len = first_input.witness.last().unwrap_or(&[]).len();
95+
if self.0.input.len() == 1 && witness_script_len == 71 &&
96+
(first_input.sequence.0 >> 8*3) as u8 == 0x80 {
9597
write!(f, "commitment tx ")?;
96-
} else if self.0.input.len() == 1 && self.0.input[0].witness.last().unwrap().len() == 71 {
98+
} else if self.0.input.len() == 1 && witness_script_len == 71 {
9799
write!(f, "closing tx ")?;
98-
} else if self.0.input.len() == 1 && HTLCType::scriptlen_to_htlctype(self.0.input[0].witness.last().unwrap().len()) == Some(HTLCType::OfferedHTLC) &&
99-
self.0.input[0].witness.len() == 5 {
100+
} else if self.0.input.len() == 1 && HTLCClaim::from_witness(&first_input.witness) == Some(HTLCClaim::OfferedTimeout) {
100101
write!(f, "HTLC-timeout tx ")?;
101-
} else if self.0.input.len() == 1 && HTLCType::scriptlen_to_htlctype(self.0.input[0].witness.last().unwrap().len()) == Some(HTLCType::AcceptedHTLC) &&
102-
self.0.input[0].witness.len() == 5 {
102+
} else if self.0.input.len() == 1 && HTLCClaim::from_witness(&first_input.witness) == Some(HTLCClaim::AcceptedPreimage) {
103103
write!(f, "HTLC-success tx ")?;
104104
} else {
105+
let mut num_preimage = 0;
106+
let mut num_timeout = 0;
107+
let mut num_revoked = 0;
105108
for inp in &self.0.input {
106-
if !inp.witness.is_empty() {
107-
if HTLCType::scriptlen_to_htlctype(inp.witness.last().unwrap().len()) == Some(HTLCType::OfferedHTLC) { write!(f, "preimage-")?; break }
108-
else if HTLCType::scriptlen_to_htlctype(inp.witness.last().unwrap().len()) == Some(HTLCType::AcceptedHTLC) { write!(f, "timeout-")?; break }
109+
let htlc_claim = HTLCClaim::from_witness(&inp.witness);
110+
match htlc_claim {
111+
Some(HTLCClaim::AcceptedPreimage)|Some(HTLCClaim::OfferedPreimage) => num_preimage += 1,
112+
Some(HTLCClaim::AcceptedTimeout)|Some(HTLCClaim::OfferedTimeout) => num_timeout += 1,
113+
Some(HTLCClaim::Revocation) => num_revoked += 1,
114+
None => continue,
109115
}
110116
}
111-
write!(f, "tx ")?;
117+
if num_preimage > 0 || num_timeout > 0 || num_revoked > 0 {
118+
write!(f, "HTLC claim tx ({} preimage, {} timeout, {} revoked)",
119+
num_preimage, num_timeout, num_revoked)?;
120+
}
112121
}
113122
} else {
114123
debug_assert!(false, "We should never generate unknown transaction types");

0 commit comments

Comments
 (0)