Skip to content

Commit 9104d6f

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 9104d6f

File tree

3 files changed

+101
-19
lines changed

3 files changed

+101
-19
lines changed

‎src/ln/channelmonitor.rs

Lines changed: 89 additions & 17 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: Vec<(u32, 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: Vec::new(),
474+
467475
last_block_hash: Default::default(),
468476
secp_ctx: Secp256k1::new(),
469477
logger,
@@ -951,6 +959,23 @@ 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 conf, ref outpoint, ref update) in self.htlc_updated_waiting_threshold_conf.iter() {
972+
writer.write_all(&byte_utils::be32_to_array(*conf))?;
973+
outpoint.write(writer)?;
974+
(*update).0.write(writer)?;
975+
update.1.write(writer)?;
976+
update.2.write(writer)?;
977+
}
978+
954979
Ok(())
955980
}
956981

@@ -1014,13 +1039,12 @@ impl ChannelMonitor {
10141039
/// HTLC-Success/HTLC-Timeout transactions.
10151040
/// Return updates for HTLC pending in the channel and failed automatically by the broadcast of
10161041
/// 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)>) {
1042+
fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec<Transaction>, (Sha256dHash, Vec<TxOut>), Vec<SpendableOutputDescriptor>) {
10181043
// Most secp and related errors trying to create keys means we have no hope of constructing
10191044
// a spend transaction...so we return no transactions to broadcast
10201045
let mut txn_to_broadcast = Vec::new();
10211046
let mut watch_outputs = Vec::new();
10221047
let mut spendable_outputs = Vec::new();
1023-
let mut htlc_updated = Vec::new();
10241048

10251049
let commitment_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
10261050
let per_commitment_option = self.remote_claimable_outpoints.get(&commitment_txid);
@@ -1029,7 +1053,7 @@ impl ChannelMonitor {
10291053
( $thing : expr ) => {
10301054
match $thing {
10311055
Ok(a) => a,
1032-
Err(_) => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated)
1056+
Err(_) => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs)
10331057
}
10341058
};
10351059
}
@@ -1054,7 +1078,7 @@ impl ChannelMonitor {
10541078
};
10551079
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()));
10561080
let a_htlc_key = match self.their_htlc_base_key {
1057-
None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated),
1081+
None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs),
10581082
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)),
10591083
};
10601084

@@ -1134,7 +1158,7 @@ impl ChannelMonitor {
11341158
if transaction_output_index as usize >= tx.output.len() ||
11351159
tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 ||
11361160
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
1161+
return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); // Corrupted per_commitment_data, fuck this user
11381162
}
11391163
let input = TxIn {
11401164
previous_output: BitcoinOutPoint {
@@ -1183,7 +1207,7 @@ impl ChannelMonitor {
11831207
for &(ref htlc, ref source_option) in outpoints.iter() {
11841208
if let &Some(ref source) = source_option {
11851209
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()));
1210+
self.htlc_updated_waiting_threshold_conf.push((1, None, ((**source).clone(), None, htlc.payment_hash.clone())));
11871211
}
11881212
}
11891213
}
@@ -1199,7 +1223,7 @@ impl ChannelMonitor {
11991223
}
12001224
// No need to check local commitment txn, symmetric HTLCSource must be present as per-htlc data on remote commitment tx
12011225
}
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
1226+
if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); } // Nothing to be done...probably a false positive/local tx
12031227

12041228
let outputs = vec!(TxOut {
12051229
script_pubkey: self.destination_script.clone(),
@@ -1257,11 +1281,12 @@ impl ChannelMonitor {
12571281
// need to here.
12581282
for &(ref broadcast_htlc, ref broadcast_source) in per_commitment_data.iter() {
12591283
if broadcast_htlc.transaction_output_index.is_some() && Some(source) == broadcast_source.as_ref() {
1284+
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()));
12601285
continue $id;
12611286
}
12621287
}
12631288
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()));
1289+
self.htlc_updated_waiting_threshold_conf.push((1, None, ((**source).clone(), None, htlc.payment_hash.clone())));
12651290
}
12661291
}
12671292
}
@@ -1294,7 +1319,7 @@ impl ChannelMonitor {
12941319
},
12951320
};
12961321
let a_htlc_key = match self.their_htlc_base_key {
1297-
None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated),
1322+
None => return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs),
12981323
Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &their_htlc_base_key)),
12991324
};
13001325

@@ -1349,7 +1374,7 @@ impl ChannelMonitor {
13491374
if transaction_output_index as usize >= tx.output.len() ||
13501375
tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 ||
13511376
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
1377+
return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); // Corrupted per_commitment_data, fuck this user
13531378
}
13541379
if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
13551380
let input = TxIn {
@@ -1412,7 +1437,7 @@ impl ChannelMonitor {
14121437
}
14131438
}
14141439

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
1440+
if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); } // Nothing to be done...probably a false positive/local tx
14161441

14171442
let outputs = vec!(TxOut {
14181443
script_pubkey: self.destination_script.clone(),
@@ -1442,7 +1467,7 @@ impl ChannelMonitor {
14421467
}
14431468
}
14441469

1445-
(txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated)
1470+
(txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs)
14461471
}
14471472

14481473
/// Attempts to claim a remote HTLC-Success/HTLC-Timeout's outputs using the revocation key
@@ -1714,7 +1739,7 @@ impl ChannelMonitor {
17141739
}
17151740
};
17161741
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);
1742+
let (remote_txn, new_outputs, mut spendable_output) = self.check_spend_remote_transaction(tx, height);
17181743
txn = remote_txn;
17191744
spendable_outputs.append(&mut spendable_output);
17201745
if !new_outputs.1.is_empty() {
@@ -1733,9 +1758,6 @@ impl ChannelMonitor {
17331758
spendable_outputs.push(spendable_output);
17341759
}
17351760
}
1736-
if updated.len() > 0 {
1737-
htlc_updated.append(&mut updated);
1738-
}
17391761
} else {
17401762
if let Some(&(commitment_number, _)) = self.remote_commitment_txn_on_chain.get(&prevout.txid) {
17411763
let (tx, spendable_output) = self.check_spend_remote_htlc(tx, commitment_number);
@@ -1758,6 +1780,15 @@ impl ChannelMonitor {
17581780
if updated.len() > 0 {
17591781
htlc_updated.append(&mut updated);
17601782
}
1783+
for inp in tx.input.iter() {
1784+
if let Some(htlc_update) = self.htlc_updated_waiting_outpoint_solving.remove(&inp.previous_output) {
1785+
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 {
1786+
htlc_updated.append(&mut vec![htlc_update]);
1787+
} else {
1788+
self.htlc_updated_waiting_threshold_conf.push((1, Some(inp.previous_output.clone()), htlc_update));
1789+
}
1790+
}
1791+
}
17611792
}
17621793
if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
17631794
if self.would_broadcast_at_height(height) {
@@ -1786,6 +1817,20 @@ impl ChannelMonitor {
17861817
}
17871818
}
17881819
}
1820+
let mut outpoint_solved = Vec::new();
1821+
for &mut(ref mut confs, ref outpoint, ref update) in self.htlc_updated_waiting_threshold_conf.iter_mut() {
1822+
*confs += 1;
1823+
//TODO switch to target insread of number of conf
1824+
if *confs == HTLC_FAIL_ANTI_REORG_DELAY {
1825+
if let Some(outpoint) = *outpoint {
1826+
outpoint_solved.push(outpoint.clone());
1827+
}
1828+
htlc_updated.push(update.clone());
1829+
}
1830+
}
1831+
for outp in outpoint_solved {
1832+
self.htlc_updated_waiting_outpoint_solving.remove(&outp);
1833+
}
17891834
self.last_block_hash = block_hash.clone();
17901835
(watch_outputs, spendable_outputs, htlc_updated)
17911836
}
@@ -2159,6 +2204,29 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
21592204
let last_block_hash: Sha256dHash = Readable::read(reader)?;
21602205
let destination_script = Readable::read(reader)?;
21612206

2207+
let waiting_outpoint_solving_len: u64 = Readable::read(reader)?;
2208+
let mut htlc_updated_waiting_outpoint_solving = HashMap::with_capacity(cmp::min(waiting_outpoint_solving_len as usize, MAX_ALLOC_SIZE / 128));
2209+
for _ in 0..waiting_outpoint_solving_len {
2210+
let outpoint = Readable::read(reader)?;
2211+
let htlc_source = Readable::read(reader)?;
2212+
let preimage = Readable::read(reader)?;
2213+
let hash = Readable::read(reader)?;
2214+
if let Some(_) = htlc_updated_waiting_outpoint_solving.insert(outpoint, (htlc_source, preimage, hash)) {
2215+
return Err(DecodeError::InvalidValue);
2216+
}
2217+
}
2218+
2219+
let waiting_threshold_conf_len: u64 = Readable::read(reader)?;
2220+
let mut htlc_updated_waiting_threshold_conf = Vec::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128));
2221+
for _ in 0..waiting_threshold_conf_len {
2222+
let conf = Readable::read(reader)?;
2223+
let outpoint = Readable::read(reader)?;
2224+
let htlc_source = Readable::read(reader)?;
2225+
let preimage = Readable::read(reader)?;
2226+
let hash = Readable::read(reader)?;
2227+
htlc_updated_waiting_threshold_conf.push((conf, outpoint, (htlc_source, preimage, hash)));
2228+
}
2229+
21622230
Ok((last_block_hash.clone(), ChannelMonitor {
21632231
commitment_transaction_number_obscure_factor,
21642232

@@ -2182,6 +2250,10 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
21822250
payment_preimages,
21832251

21842252
destination_script,
2253+
2254+
htlc_updated_waiting_outpoint_solving,
2255+
htlc_updated_waiting_threshold_conf,
2256+
21852257
last_block_hash,
21862258
secp_ctx,
21872259
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, &Vec::new(), &Vec::new());
52+
for i in 0..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>,

‎src/ln/functional_tests.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use chain::keysinterface::{KeysInterface, SpendableOutputDescriptor};
88
use chain::keysinterface;
99
use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC, BREAKDOWN_TIMEOUT};
1010
use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,HTLCForwardInfo,RAACommitmentOrder, PaymentPreimage, PaymentHash};
11-
use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, ManyChannelMonitor};
11+
use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, ManyChannelMonitor, HTLC_FAIL_ANTI_REORG_DELAY};
1212
use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT};
1313
use ln::onion_utils;
1414
use ln::router::{Route, RouteHop};
@@ -1723,6 +1723,7 @@ fn claim_htlc_outputs_shared_tx() {
17231723
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
17241724
nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
17251725
nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
1726+
connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY, 1, true, header.bitcoin_hash());
17261727

17271728
let events = nodes[1].node.get_and_clear_pending_events();
17281729
assert_eq!(events.len(), 1);

0 commit comments

Comments
 (0)