Skip to content

Commit 98fc5dc

Browse files
author
Antoine Riard
committed
Track HTLC-failure trigger tx until anti-reorg delay reached
Broadcasting a commitment tx means that we have to fail inbound HTLC in backward channel. Doing it prematurely would put us at risk in case of reorg and outbound HTLCs solved differently (i.e by a preimage). So we delay passing failure update upstream until solving tx mature to HTLC_FAIL_ANTI_ REORG_DELAY. Requirements differ if HTLC is a revoked/non-revoked dust/ non-revoked non-dust one. Add connect_blocks in test_utils to fix broken tests due to anti-reorg delay enforcement
1 parent eafdfc7 commit 98fc5dc

File tree

3 files changed

+136
-21
lines changed

3 files changed

+136
-21
lines changed

‎src/ln/channelmonitor.rs

Lines changed: 115 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,9 @@ pub struct ChannelMonitor {
385385

386386
destination_script: Script,
387387

388+
htlc_updated_waiting_outpoint_solving: HashMap<BitcoinOutPoint, (HTLCSource, Option<PaymentPreimage>, PaymentHash)>,
389+
htlc_updated_waiting_threshold_conf: HashMap<u32, Vec<(Option<BitcoinOutPoint>, (HTLCSource, Option<PaymentPreimage>, PaymentHash))>>,
390+
388391
// We simply modify last_block_hash in Channel's block_connected so that serialization is
389392
// consistent but hopefully the users' copy handles block_connected in a consistent way.
390393
// (we do *not*, however, update them in insert_combine to ensure any local user copies keep
@@ -414,7 +417,9 @@ impl PartialEq for ChannelMonitor {
414417
self.current_remote_commitment_number != other.current_remote_commitment_number ||
415418
self.current_local_signed_commitment_tx != other.current_local_signed_commitment_tx ||
416419
self.payment_preimages != other.payment_preimages ||
417-
self.destination_script != other.destination_script
420+
self.destination_script != other.destination_script ||
421+
self.htlc_updated_waiting_outpoint_solving != other.htlc_updated_waiting_outpoint_solving ||
422+
self.htlc_updated_waiting_threshold_conf != other.htlc_updated_waiting_threshold_conf
418423
{
419424
false
420425
} else {
@@ -464,6 +469,9 @@ impl ChannelMonitor {
464469
payment_preimages: HashMap::new(),
465470
destination_script: destination_script,
466471

472+
htlc_updated_waiting_outpoint_solving: HashMap::new(),
473+
htlc_updated_waiting_threshold_conf: HashMap::new(),
474+
467475
last_block_hash: Default::default(),
468476
secp_ctx: Secp256k1::new(),
469477
logger,
@@ -951,6 +959,26 @@ impl ChannelMonitor {
951959
self.last_block_hash.write(writer)?;
952960
self.destination_script.write(writer)?;
953961

962+
writer.write_all(&byte_utils::be64_to_array(self.htlc_updated_waiting_outpoint_solving.len() as u64))?;
963+
for (ref outpoint, ref update) in self.htlc_updated_waiting_outpoint_solving.iter() {
964+
outpoint.write(writer)?;
965+
update.0.write(writer)?;
966+
update.1.write(writer)?;
967+
update.2.write(writer)?;
968+
}
969+
970+
writer.write_all(&byte_utils::be64_to_array(self.htlc_updated_waiting_threshold_conf.len() as u64))?;
971+
for (ref target, ref updates) in self.htlc_updated_waiting_threshold_conf.iter() {
972+
writer.write_all(&byte_utils::be32_to_array(**target))?;
973+
writer.write_all(&byte_utils::be64_to_array(updates.len() as u64))?;
974+
for &(ref outpoint, ref update) in updates.iter() {
975+
outpoint.write(writer)?;
976+
update.0.write(writer)?;
977+
update.1.write(writer)?;
978+
update.2.write(writer)?;
979+
}
980+
}
981+
954982
Ok(())
955983
}
956984

@@ -1014,13 +1042,12 @@ impl ChannelMonitor {
10141042
/// HTLC-Success/HTLC-Timeout transactions.
10151043
/// Return updates for HTLC pending in the channel and failed automatically by the broadcast of
10161044
/// revoked remote commitment tx
1017-
fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec<Transaction>, (Sha256dHash, Vec<TxOut>), Vec<SpendableOutputDescriptor>, Vec<(HTLCSource, Option<PaymentPreimage>, PaymentHash)>) {
1045+
fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec<Transaction>, (Sha256dHash, Vec<TxOut>), Vec<SpendableOutputDescriptor>) {
10181046
// Most secp and related errors trying to create keys means we have no hope of constructing
10191047
// a spend transaction...so we return no transactions to broadcast
10201048
let mut txn_to_broadcast = Vec::new();
10211049
let mut watch_outputs = Vec::new();
10221050
let mut spendable_outputs = Vec::new();
1023-
let mut htlc_updated = Vec::new();
10241051

10251052
let commitment_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
10261053
let per_commitment_option = self.remote_claimable_outpoints.get(&commitment_txid);
@@ -1029,7 +1056,7 @@ impl ChannelMonitor {
10291056
( $thing : expr ) => {
10301057
match $thing {
10311058
Ok(a) => a,
1032-
Err(_) => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated)
1059+
Err(_) => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs)
10331060
}
10341061
};
10351062
}
@@ -1054,7 +1081,7 @@ impl ChannelMonitor {
10541081
};
10551082
let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.their_delayed_payment_base_key.unwrap()));
10561083
let a_htlc_key = match self.their_htlc_base_key {
1057-
None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated),
1084+
None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs),
10581085
Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &their_htlc_base_key)),
10591086
};
10601087

@@ -1134,7 +1161,7 @@ impl ChannelMonitor {
11341161
if transaction_output_index as usize >= tx.output.len() ||
11351162
tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 ||
11361163
tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
1137-
return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); // Corrupted per_commitment_data, fuck this user
1164+
return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); // Corrupted per_commitment_data, fuck this user
11381165
}
11391166
let input = TxIn {
11401167
previous_output: BitcoinOutPoint {
@@ -1182,8 +1209,15 @@ impl ChannelMonitor {
11821209
if let Some(ref outpoints) = self.remote_claimable_outpoints.get(&$txid) {
11831210
for &(ref htlc, ref source_option) in outpoints.iter() {
11841211
if let &Some(ref source) = source_option {
1185-
log_trace!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of revoked remote commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx);
1186-
htlc_updated.push(((**source).clone(), None, htlc.payment_hash.clone()));
1212+
log_info!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of revoked remote commitment transaction, waiting confirmation until {} height", log_bytes!(htlc.payment_hash.0), $commitment_tx, height + HTLC_FAIL_ANTI_REORG_DELAY - 1);
1213+
match self.htlc_updated_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) {
1214+
hash_map::Entry::Occupied(mut entry) => {
1215+
entry.get_mut().push((None, ((**source).clone(), None, htlc.payment_hash.clone())));
1216+
}
1217+
hash_map::Entry::Vacant(entry) => {
1218+
entry.insert(vec![(None, ((**source).clone(), None, htlc.payment_hash.clone()))]);
1219+
}
1220+
}
11871221
}
11881222
}
11891223
}
@@ -1199,7 +1233,7 @@ impl ChannelMonitor {
11991233
}
12001234
// No need to check local commitment txn, symmetric HTLCSource must be present as per-htlc data on remote commitment tx
12011235
}
1202-
if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); } // Nothing to be done...probably a false positive/local tx
1236+
if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); } // Nothing to be done...probably a false positive/local tx
12031237

12041238
let outputs = vec!(TxOut {
12051239
script_pubkey: self.destination_script.clone(),
@@ -1257,11 +1291,19 @@ impl ChannelMonitor {
12571291
// need to here.
12581292
for &(ref broadcast_htlc, ref broadcast_source) in per_commitment_data.iter() {
12591293
if broadcast_htlc.transaction_output_index.is_some() && Some(source) == broadcast_source.as_ref() {
1294+
self.htlc_updated_waiting_outpoint_solving.insert(BitcoinOutPoint { txid: commitment_txid.clone(), vout: broadcast_htlc.transaction_output_index.unwrap() }, ((**source).clone(), None, htlc.payment_hash.clone()));
12601295
continue $id;
12611296
}
12621297
}
12631298
log_trace!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of remote commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx);
1264-
htlc_updated.push(((**source).clone(), None, htlc.payment_hash.clone()));
1299+
match self.htlc_updated_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) {
1300+
hash_map::Entry::Occupied(mut entry) => {
1301+
entry.get_mut().push((None, ((**source).clone(), None, htlc.payment_hash.clone())));
1302+
}
1303+
hash_map::Entry::Vacant(entry) => {
1304+
entry.insert(vec![(None, ((**source).clone(), None, htlc.payment_hash.clone()))]);
1305+
}
1306+
}
12651307
}
12661308
}
12671309
}
@@ -1294,7 +1336,7 @@ impl ChannelMonitor {
12941336
},
12951337
};
12961338
let a_htlc_key = match self.their_htlc_base_key {
1297-
None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated),
1339+
None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs),
12981340
Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &their_htlc_base_key)),
12991341
};
13001342

@@ -1349,7 +1391,7 @@ impl ChannelMonitor {
13491391
if transaction_output_index as usize >= tx.output.len() ||
13501392
tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 ||
13511393
tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
1352-
return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); // Corrupted per_commitment_data, fuck this user
1394+
return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); // Corrupted per_commitment_data, fuck this user
13531395
}
13541396
if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
13551397
let input = TxIn {
@@ -1412,7 +1454,7 @@ impl ChannelMonitor {
14121454
}
14131455
}
14141456

1415-
if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); } // Nothing to be done...probably a false positive/local tx
1457+
if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); } // Nothing to be done...probably a false positive/local tx
14161458

14171459
let outputs = vec!(TxOut {
14181460
script_pubkey: self.destination_script.clone(),
@@ -1442,7 +1484,7 @@ impl ChannelMonitor {
14421484
}
14431485
}
14441486

1445-
(txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated)
1487+
(txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs)
14461488
}
14471489

14481490
/// Attempts to claim a remote HTLC-Success/HTLC-Timeout's outputs using the revocation key
@@ -1697,6 +1739,7 @@ impl ChannelMonitor {
16971739
let mut watch_outputs = Vec::new();
16981740
let mut spendable_outputs = Vec::new();
16991741
let mut htlc_updated = Vec::new();
1742+
//log_info!(self, "Height at {}", height);
17001743
for tx in txn_matched {
17011744
if tx.input.len() == 1 {
17021745
// Assuming our keys were not leaked (in which case we're screwed no matter what),
@@ -1714,7 +1757,7 @@ impl ChannelMonitor {
17141757
}
17151758
};
17161759
if funding_txo.is_none() || (prevout.txid == funding_txo.as_ref().unwrap().0.txid && prevout.vout == funding_txo.as_ref().unwrap().0.index as u32) {
1717-
let (remote_txn, new_outputs, mut spendable_output, mut updated) = self.check_spend_remote_transaction(tx, height);
1760+
let (remote_txn, new_outputs, mut spendable_output) = self.check_spend_remote_transaction(tx, height);
17181761
txn = remote_txn;
17191762
spendable_outputs.append(&mut spendable_output);
17201763
if !new_outputs.1.is_empty() {
@@ -1733,9 +1776,6 @@ impl ChannelMonitor {
17331776
spendable_outputs.push(spendable_output);
17341777
}
17351778
}
1736-
if updated.len() > 0 {
1737-
htlc_updated.append(&mut updated);
1738-
}
17391779
} else {
17401780
if let Some(&(commitment_number, _)) = self.remote_commitment_txn_on_chain.get(&prevout.txid) {
17411781
let (tx, spendable_output) = self.check_spend_remote_htlc(tx, commitment_number);
@@ -1758,6 +1798,22 @@ impl ChannelMonitor {
17581798
if updated.len() > 0 {
17591799
htlc_updated.append(&mut updated);
17601800
}
1801+
for inp in tx.input.iter() {
1802+
if let Some(htlc_update) = self.htlc_updated_waiting_outpoint_solving.remove(&inp.previous_output) {
1803+
if inp.witness.len() == 5 && inp.witness[4].len() == ACCEPTED_HTLC_SCRIPT_WEIGHT && inp.witness.len() == 3 && inp.witness[2].len() == OFFERED_HTLC_SCRIPT_WEIGHT {
1804+
htlc_updated.append(&mut vec![htlc_update]);
1805+
} else {
1806+
match self.htlc_updated_waiting_threshold_conf.entry(height + HTLC_FAIL_ANTI_REORG_DELAY - 1) {
1807+
hash_map::Entry::Occupied(mut entry) => {
1808+
entry.get_mut().push((Some(inp.previous_output.clone()), htlc_update));
1809+
}
1810+
hash_map::Entry::Vacant(entry) => {
1811+
entry.insert(vec![(Some(inp.previous_output.clone()), htlc_update)]);
1812+
}
1813+
}
1814+
}
1815+
}
1816+
}
17611817
}
17621818
if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
17631819
if self.would_broadcast_at_height(height) {
@@ -1786,6 +1842,15 @@ impl ChannelMonitor {
17861842
}
17871843
}
17881844
}
1845+
if let Some(updates) = self.htlc_updated_waiting_threshold_conf.remove(&height) {
1846+
for (outpoint, update) in updates {
1847+
if let Some(oup) = outpoint {
1848+
self.htlc_updated_waiting_outpoint_solving.remove(&oup);
1849+
}
1850+
log_info!(self, "HTLC {} failure update has get enough confirmation to be pass upstream", log_bytes!((update.2).0));
1851+
htlc_updated.push(update);
1852+
}
1853+
}
17891854
self.last_block_hash = block_hash.clone();
17901855
(watch_outputs, spendable_outputs, htlc_updated)
17911856
}
@@ -2159,6 +2224,34 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
21592224
let last_block_hash: Sha256dHash = Readable::read(reader)?;
21602225
let destination_script = Readable::read(reader)?;
21612226

2227+
let waiting_outpoint_solving_len: u64 = Readable::read(reader)?;
2228+
let mut htlc_updated_waiting_outpoint_solving = HashMap::with_capacity(cmp::min(waiting_outpoint_solving_len as usize, MAX_ALLOC_SIZE / 128));
2229+
for _ in 0..waiting_outpoint_solving_len {
2230+
let outpoint = Readable::read(reader)?;
2231+
let htlc_source = Readable::read(reader)?;
2232+
let preimage = Readable::read(reader)?;
2233+
let hash = Readable::read(reader)?;
2234+
if let Some(_) = htlc_updated_waiting_outpoint_solving.insert(outpoint, (htlc_source, preimage, hash)) {
2235+
return Err(DecodeError::InvalidValue);
2236+
}
2237+
}
2238+
2239+
let waiting_threshold_conf_len: u64 = Readable::read(reader)?;
2240+
let mut htlc_updated_waiting_threshold_conf = HashMap::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128));
2241+
for _ in 0..waiting_threshold_conf_len {
2242+
let height_target = Readable::read(reader)?;
2243+
let updates_len: u64 = Readable::read(reader)?;
2244+
let mut updates = Vec::with_capacity(cmp::min(updates_len as usize, MAX_ALLOC_SIZE / 128));
2245+
for _ in 0..updates_len {
2246+
let outpoint = Readable::read(reader)?;
2247+
let htlc_source = Readable::read(reader)?;
2248+
let preimage = Readable::read(reader)?;
2249+
let hash = Readable::read(reader)?;
2250+
updates.push((outpoint, (htlc_source, preimage, hash)));
2251+
}
2252+
htlc_updated_waiting_threshold_conf.insert(height_target, updates);
2253+
}
2254+
21622255
Ok((last_block_hash.clone(), ChannelMonitor {
21632256
commitment_transaction_number_obscure_factor,
21642257

@@ -2182,6 +2275,10 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
21822275
payment_preimages,
21832276

21842277
destination_script,
2278+
2279+
htlc_updated_waiting_outpoint_solving,
2280+
htlc_updated_waiting_threshold_conf,
2281+
21852282
last_block_hash,
21862283
secp_ctx,
21872284
logger,

‎src/ln/functional_test_utils.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use util::errors::APIError;
1414
use util::logger::Logger;
1515
use util::config::UserConfig;
1616

17-
use bitcoin::util::hash::BitcoinHash;
17+
use bitcoin::util::hash::{BitcoinHash, Sha256dHash};
1818
use bitcoin::blockdata::block::BlockHeader;
1919
use bitcoin::blockdata::transaction::{Transaction, TxOut};
2020
use bitcoin::network::constants::Network;
@@ -46,6 +46,15 @@ pub fn confirm_transaction(chain: &chaininterface::ChainWatchInterfaceUtil, tx:
4646
}
4747
}
4848

49+
pub fn connect_blocks(chain: &chaininterface::ChainWatchInterfaceUtil, depth: u32, height: u32, parent: bool, prev_blockhash: Sha256dHash) {
50+
let mut header = BlockHeader { version: 0x2000000, prev_blockhash: if parent { prev_blockhash } else { Default::default() }, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
51+
chain.block_connected_checked(&header, height + 1, &Vec::new(), &Vec::new());
52+
for i in 2..depth {
53+
header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
54+
chain.block_connected_checked(&header, height + i, &Vec::new(), &Vec::new());
55+
}
56+
}
57+
4958
pub struct Node {
5059
pub chain_monitor: Arc<chaininterface::ChainWatchInterfaceUtil>,
5160
pub tx_broadcaster: Arc<test_utils::TestBroadcaster>,

0 commit comments

Comments
 (0)