Skip to content

Commit df0bdce

Browse files
alecchendevalecchendev
committed
Fail HTLC backwards before upstream claims on-chain
Fail inbound HTLCs if they expire within a certain number of blocks from the current height. If we haven't seen the preimage for an HTLC by the time the previous hop's timeout expires, we've lost that HTLC, so we might as well fail it back instead of having our counterparty force-close the channel.
1 parent 5566e07 commit df0bdce

File tree

2 files changed

+197
-18
lines changed

2 files changed

+197
-18
lines changed

‎lightning/src/chain/channelmonitor.rs

+82-15
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ use crate::ln::{PaymentHash, PaymentPreimage};
3737
use crate::ln::msgs::DecodeError;
3838
use crate::ln::chan_utils;
3939
use crate::ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCClaim, ChannelTransactionParameters, HolderCommitmentTransaction};
40-
use crate::ln::channelmanager::{HTLCSource, SentHTLCId};
40+
use crate::ln::channelmanager::{HTLCSource, SentHTLCId, HTLCPreviousHopData};
4141
use crate::chain;
4242
use crate::chain::{BestBlock, WatchedOutput};
4343
use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator};
@@ -236,6 +236,15 @@ pub const ANTI_REORG_DELAY: u32 = 6;
236236
/// in a race condition between the user connecting a block (which would fail it) and the user
237237
/// providing us the preimage (which would claim it).
238238
pub(crate) const HTLC_FAIL_BACK_BUFFER: u32 = CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS;
239+
/// Number of blocks before an inbound HTLC expires at which we fail it backwards.
240+
///
241+
/// If we have forwarded an HTLC to a peer and they have not claimed it on or off-chain by the
242+
/// time the previous hop's HTLC timeout expires, we're probably going to lose the inbound HTLC.
243+
/// Instead of also losing the channel, we fail the HTLC backwards.
244+
///
245+
/// To give us some buffer in case we're slow to process blocks, we fail a few blocks before the
246+
/// timeout officially expires to ensure we fail back before our counterparty force closes.
247+
pub(crate) const TIMEOUT_FAIL_BACK_BUFFER: u32 = 3;
239248

240249
// TODO(devrandom) replace this with HolderCommitmentTransaction
241250
#[derive(Clone, PartialEq, Eq)]
@@ -3303,6 +3312,60 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
33033312
}
33043313
}
33053314

3315+
// Fail back HTLCs on backwards channels if they expire within `TIMEOUT_FAIL_BACK_BUFFER`
3316+
// blocks. If we haven't seen the preimage for an HTLC by the time the previous hop's
3317+
// timeout expires, we've lost that HTLC, so we might as well fail it back instead of having our
3318+
// counterparty force-close the channel.
3319+
let height = self.best_block.height();
3320+
macro_rules! fail_soon_to_expire_htlcs {
3321+
($htlcs: expr) => {{
3322+
for (htlc, source_opt) in $htlcs {
3323+
// Only check forwarded HTLCs' previous hops
3324+
let source = match source_opt {
3325+
Some(source) => source,
3326+
None => continue,
3327+
};
3328+
let cltv_expiry = match source {
3329+
HTLCSource::PreviousHopData(HTLCPreviousHopData { cltv_expiry: Some(cltv_expiry), .. }) => *cltv_expiry,
3330+
_ => continue,
3331+
};
3332+
if cltv_expiry <= height + TIMEOUT_FAIL_BACK_BUFFER {
3333+
let duplicate_event = self.pending_monitor_events.iter().any(
3334+
|update| if let &MonitorEvent::HTLCEvent(ref upd) = update {
3335+
upd.source == *source
3336+
} else { false });
3337+
if !duplicate_event {
3338+
log_debug!(logger, "Failing back HTLC {} upstream to preserve the \
3339+
channel as the forward HTLC hasn't resolved and our backward HTLC \
3340+
expires soon at {}", log_bytes!(htlc.payment_hash.0), cltv_expiry);
3341+
self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
3342+
source: source.clone(),
3343+
payment_preimage: None,
3344+
payment_hash: htlc.payment_hash,
3345+
htlc_value_satoshis: Some(htlc.amount_msat / 1000),
3346+
}));
3347+
}
3348+
}
3349+
}
3350+
}}
3351+
}
3352+
3353+
let current_holder_htlcs = self.current_holder_commitment_tx.htlc_outputs.iter()
3354+
.map(|&(ref a, _, ref b)| (a, b.as_ref()));
3355+
fail_soon_to_expire_htlcs!(current_holder_htlcs);
3356+
3357+
if let Some(ref txid) = self.current_counterparty_commitment_txid {
3358+
if let Some(ref htlc_outputs) = self.counterparty_claimable_outpoints.get(txid) {
3359+
fail_soon_to_expire_htlcs!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, (b.as_ref().clone()).map(|boxed| &**boxed))));
3360+
}
3361+
};
3362+
3363+
if let Some(ref txid) = self.prev_counterparty_commitment_txid {
3364+
if let Some(ref htlc_outputs) = self.counterparty_claimable_outpoints.get(txid) {
3365+
fail_soon_to_expire_htlcs!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, (b.as_ref().clone()).map(|boxed| &**boxed))));
3366+
}
3367+
};
3368+
33063369
// Find which on-chain events have reached their confirmation threshold.
33073370
let onchain_events_awaiting_threshold_conf =
33083371
self.onchain_events_awaiting_threshold_conf.drain(..).collect::<Vec<_>>();
@@ -3347,20 +3410,24 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
33473410
matured_htlcs.push(source.clone());
33483411
}
33493412

3350-
log_debug!(logger, "HTLC {} failure update in {} has got enough confirmations to be passed upstream",
3351-
log_bytes!(payment_hash.0), entry.txid);
3352-
self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
3353-
payment_hash,
3354-
payment_preimage: None,
3355-
source: source.clone(),
3356-
htlc_value_satoshis,
3357-
}));
3358-
self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC {
3359-
commitment_tx_output_idx,
3360-
resolving_txid: Some(entry.txid),
3361-
resolving_tx: entry.transaction,
3362-
payment_preimage: None,
3363-
});
3413+
let duplicate_event = self.pending_monitor_events.iter().any(
3414+
|update| if let &MonitorEvent::HTLCEvent(ref upd) = update { upd.source == *source } else { false });
3415+
if !duplicate_event {
3416+
log_debug!(logger, "HTLC {} failure update in {} has got enough confirmations to be passed upstream",
3417+
log_bytes!(payment_hash.0), entry.txid);
3418+
self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
3419+
payment_hash,
3420+
payment_preimage: None,
3421+
source: source.clone(),
3422+
htlc_value_satoshis,
3423+
}));
3424+
self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC {
3425+
commitment_tx_output_idx,
3426+
resolving_txid: Some(entry.txid),
3427+
resolving_tx: entry.transaction,
3428+
payment_preimage: None,
3429+
});
3430+
}
33643431
},
33653432
OnchainEvent::MaturingOutput { descriptor } => {
33663433
log_debug!(logger, "Descriptor {} has got enough confirmations to be passed upstream", log_spendable!(descriptor));

‎lightning/src/ln/functional_tests.rs

+115-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
use crate::chain;
1515
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
1616
use crate::chain::chaininterface::LowerBoundedFeeEstimator;
17-
use crate::chain::channelmonitor;
17+
use crate::chain::channelmonitor::{self, TIMEOUT_FAIL_BACK_BUFFER};
1818
use crate::chain::channelmonitor::{CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
1919
use crate::chain::transaction::OutPoint;
2020
use crate::sign::{ChannelSigner, EcdsaChannelSigner, EntropySource};
@@ -2211,6 +2211,118 @@ fn channel_reserve_in_flight_removes() {
22112211
claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_3);
22122212
}
22132213

2214+
enum PostFailBackAction {
2215+
TimeoutOnChain,
2216+
ClaimOnChain,
2217+
FailOffChain,
2218+
ClaimOffChain,
2219+
}
2220+
2221+
#[test]
2222+
fn test_fail_back_before_backwards_timeout() {
2223+
do_test_fail_back_before_backwards_timeout(PostFailBackAction::TimeoutOnChain);
2224+
do_test_fail_back_before_backwards_timeout(PostFailBackAction::ClaimOnChain);
2225+
do_test_fail_back_before_backwards_timeout(PostFailBackAction::FailOffChain);
2226+
do_test_fail_back_before_backwards_timeout(PostFailBackAction::ClaimOffChain);
2227+
}
2228+
2229+
fn do_test_fail_back_before_backwards_timeout(post_fail_back_action: PostFailBackAction) {
2230+
// Test that we fail an HTLC upstream if we are still waiting for confirmation downstream
2231+
// just before the upstream timeout expires
2232+
let chanmon_cfgs = create_chanmon_cfgs(3);
2233+
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
2234+
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
2235+
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
2236+
2237+
create_announced_chan_between_nodes(&nodes, 0, 1);
2238+
let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2);
2239+
2240+
connect_blocks(&nodes[0], 2*CHAN_CONFIRM_DEPTH + 1 - nodes[0].best_block_info().1);
2241+
connect_blocks(&nodes[1], 2*CHAN_CONFIRM_DEPTH + 1 - nodes[1].best_block_info().1);
2242+
connect_blocks(&nodes[2], 2*CHAN_CONFIRM_DEPTH + 1 - nodes[2].best_block_info().1);
2243+
2244+
let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 3_000_000);
2245+
2246+
// Force close downstream with timeout
2247+
nodes[1].node.force_close_broadcasting_latest_txn(&chan_2.2, &nodes[2].node.get_our_node_id()).unwrap();
2248+
check_added_monitors!(nodes[1], 1);
2249+
check_closed_broadcast!(nodes[1], true);
2250+
2251+
connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
2252+
let node_1_txn = test_txn_broadcast(&nodes[1], &chan_2, None, HTLCType::TIMEOUT);
2253+
check_closed_event(&nodes[1], 1, ClosureReason::HolderForceClosed, false);
2254+
2255+
// Nothing is confirmed for a while
2256+
connect_blocks(&nodes[1], MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - TIMEOUT_FAIL_BACK_BUFFER);
2257+
2258+
// Check that nodes[1] fails the HTLC upstream
2259+
expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], vec![HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: chan_2.2 }]);
2260+
check_added_monitors!(nodes[1], 1);
2261+
let events = nodes[1].node.get_and_clear_pending_msg_events();
2262+
assert_eq!(events.len(), 1);
2263+
let (update_fail, commitment_signed) = match events[0] {
2264+
MessageSendEvent::UpdateHTLCs { node_id: _, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, ref update_fee, ref commitment_signed } } => {
2265+
assert!(update_add_htlcs.is_empty());
2266+
assert!(update_fulfill_htlcs.is_empty());
2267+
assert_eq!(update_fail_htlcs.len(), 1);
2268+
assert!(update_fail_malformed_htlcs.is_empty());
2269+
assert!(update_fee.is_none());
2270+
(update_fail_htlcs[0].clone(), commitment_signed.clone())
2271+
},
2272+
_ => panic!("Unexpected event"),
2273+
};
2274+
2275+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &update_fail);
2276+
commitment_signed_dance!(nodes[0], nodes[1], commitment_signed, false);
2277+
expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().blamed_chan_closed(true));
2278+
2279+
// Make sure we handle possible duplicate fails or extra messages after failing back
2280+
match post_fail_back_action {
2281+
PostFailBackAction::TimeoutOnChain => {
2282+
// Confirm nodes[1]'s claim with timeout, make sure we don't fail upstream again
2283+
mine_transaction(&nodes[1], &node_1_txn[0]); // Commitment
2284+
mine_transaction(&nodes[1], &node_1_txn[1]); // HTLC timeout
2285+
},
2286+
PostFailBackAction::ClaimOnChain => {
2287+
nodes[2].node.claim_funds(payment_preimage);
2288+
expect_payment_claimed!(nodes[2], payment_hash, 3_000_000);
2289+
check_added_monitors!(nodes[2], 1);
2290+
get_htlc_update_msgs(&nodes[2], &nodes[1].node.get_our_node_id());
2291+
2292+
connect_blocks(&nodes[2], TEST_FINAL_CLTV - CLTV_CLAIM_BUFFER + 2);
2293+
let node_2_txn = test_txn_broadcast(&nodes[2], &chan_2, None, HTLCType::SUCCESS);
2294+
check_closed_broadcast!(nodes[2], true);
2295+
check_closed_event(&nodes[2], 1, ClosureReason::CommitmentTxConfirmed, false);
2296+
check_added_monitors!(nodes[2], 1);
2297+
2298+
mine_transaction(&nodes[1], &node_2_txn[0]); // Commitment
2299+
mine_transaction(&nodes[1], &node_2_txn[1]); // HTLC success
2300+
},
2301+
PostFailBackAction::FailOffChain => {
2302+
nodes[2].node.fail_htlc_backwards(&payment_hash);
2303+
expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[2], vec![HTLCDestination::FailedPayment { payment_hash: payment_hash.clone() }]);
2304+
check_added_monitors!(nodes[2], 1);
2305+
let commitment_update = get_htlc_update_msgs(&nodes[2], &nodes[1].node.get_our_node_id());
2306+
let update_fail = commitment_update.update_fail_htlcs[0].clone();
2307+
2308+
nodes[1].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &update_fail);
2309+
let err_msg = get_err_msg(&nodes[1], &nodes[2].node.get_our_node_id());
2310+
assert_eq!(err_msg.channel_id, chan_2.2);
2311+
},
2312+
PostFailBackAction::ClaimOffChain => {
2313+
nodes[2].node.claim_funds(payment_preimage);
2314+
expect_payment_claimed!(nodes[2], payment_hash, 3_000_000);
2315+
check_added_monitors!(nodes[2], 1);
2316+
let commitment_update = get_htlc_update_msgs(&nodes[2], &nodes[1].node.get_our_node_id());
2317+
let update_fulfill = commitment_update.update_fulfill_htlcs[0].clone();
2318+
2319+
nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &update_fulfill);
2320+
let err_msg = get_err_msg(&nodes[1], &nodes[2].node.get_our_node_id());
2321+
assert_eq!(err_msg.channel_id, chan_2.2);
2322+
},
2323+
};
2324+
}
2325+
22142326
#[test]
22152327
fn channel_monitor_network_test() {
22162328
// Simple test which builds a network of ChannelManagers, connects them to each other, and
@@ -2307,7 +2419,7 @@ fn channel_monitor_network_test() {
23072419
let node2_commitment_txid;
23082420
{
23092421
let node_txn = test_txn_broadcast(&nodes[2], &chan_3, None, HTLCType::NONE);
2310-
connect_blocks(&nodes[2], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + MIN_CLTV_EXPIRY_DELTA as u32 + 1);
2422+
connect_blocks(&nodes[2], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
23112423
test_txn_broadcast(&nodes[2], &chan_3, None, HTLCType::TIMEOUT);
23122424
node2_commitment_txid = node_txn[0].txid();
23132425

@@ -2992,7 +3104,7 @@ fn do_test_htlc_on_chain_timeout(connect_style: ConnectStyle) {
29923104
// Verify that B's ChannelManager is able to detect that HTLC is timeout by its own tx and react backward in consequence
29933105
mine_transaction(&nodes[1], &commitment_tx[0]);
29943106
check_closed_event(&nodes[1], 1, ClosureReason::CommitmentTxConfirmed, false);
2995-
connect_blocks(&nodes[1], 200 - nodes[2].best_block_info().1);
3107+
connect_blocks(&nodes[1], 100 - nodes[2].best_block_info().1);
29963108
let timeout_tx = {
29973109
let mut txn = nodes[1].tx_broadcaster.txn_broadcast();
29983110
if nodes[1].connect_style.borrow().skips_blocks() {

0 commit comments

Comments
 (0)