Skip to content

Commit ced5650

Browse files
valentinewallacevalentinewallace
committed
MonitorEvents: HTLC failure reason and skimmed fee
We're moving towards having the ChannelMonitor be responsible for resolving HTLCs, via MonitorEvents, so we need to start surfacing more information in monitor events. Here we start persisting/surfacing a specific HTLC failure reason and the skimmed fee in MonitorEvent::HTLCEvents, which is useful when generating and handling monitor events for off-chain claims/fails. The skimmed fee for forwards is already put to use in this commit for generating correct PaymentForwarded events. The failure reason will be used in upcoming commits. We add the failure reason now to not have to change what we're serializing to disk later.
1 parent 1f04e15 commit ced5650

5 files changed

Lines changed: 316 additions & 72 deletions

File tree

‎lightning/src/chain/channelmonitor.rs‎

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ use crate::ln::channel_keys::{
5656
};
5757
use crate::ln::channelmanager::{HTLCSource, PaymentClaimDetails, SentHTLCId};
5858
use crate::ln::msgs::DecodeError;
59+
use crate::ln::onion_utils::{HTLCFailReason, LocalHTLCFailureReason};
5960
use crate::ln::types::ChannelId;
6061
use crate::sign::{
6162
ecdsa::EcdsaChannelSigner, ChannelDerivationParameters, DelayedPaymentOutputDescriptor,
@@ -252,24 +253,76 @@ impl_writeable_tlv_based_enum_upgradable_legacy!(MonitorEvent,
252253
// 6 was `UpdateFailed` until LDK 0.0.117
253254
);
254255

256+
/// The resolution of an outbound HTLC — either claimed with a preimage or failed back. Used in
257+
/// [`MonitorEvent::HTLCEvent`]s to provide resolution data to the `ChannelManager`.
258+
#[derive(Clone, PartialEq, Eq, Debug)]
259+
pub(crate) enum OutboundHTLCResolution {
260+
Claimed {
261+
preimage: PaymentPreimage,
262+
skimmed_fee_msat: Option<u64>,
263+
},
264+
/// The failure resolution may come from on-chain in the [`ChannelMonitor`] or off-chain from the
265+
/// `ChannelManager`, such as from an outbound edge to provide the manager with a resolution for
266+
/// the inbound edge.
267+
Failed {
268+
reason: HTLCFailReason,
269+
},
270+
}
271+
272+
impl OutboundHTLCResolution {
273+
/// Returns an on-chain timeout failure resolution.
274+
pub fn on_chain_timeout() -> Self {
275+
OutboundHTLCResolution::Failed {
276+
reason: HTLCFailReason::from_failure_code(LocalHTLCFailureReason::OnChainTimeout),
277+
}
278+
}
279+
}
280+
281+
impl_writeable_tlv_based_enum!(OutboundHTLCResolution,
282+
(1, Claimed) => {
283+
(1, preimage, required),
284+
(3, skimmed_fee_msat, option),
285+
},
286+
(3, Failed) => {
287+
(1, reason, required),
288+
},
289+
);
290+
255291
/// Simple structure sent back by `chain::Watch` when an HTLC from a forward channel is detected on
256292
/// chain. Used to update the corresponding HTLC in the backward channel. Failing to pass the
257293
/// preimage claim backward will lead to loss of funds.
258294
#[derive(Clone, PartialEq, Eq)]
259295
pub struct HTLCUpdate {
260296
pub(crate) payment_hash: PaymentHash,
261-
pub(crate) payment_preimage: Option<PaymentPreimage>,
262297
pub(crate) source: HTLCSource,
263298
pub(crate) htlc_value_msat: Option<u64>,
264299
pub(crate) user_channel_id: Option<u128>,
300+
pub(crate) resolution: OutboundHTLCResolution,
265301
}
266302
impl_writeable_tlv_based!(HTLCUpdate, {
267303
(0, payment_hash, required),
268304
(1, htlc_value_satoshis, (legacy, u64, |_| Ok(()), |us: &HTLCUpdate| us.htlc_value_msat.map(|v| v / 1000))),
269305
(2, source, required),
270-
(4, payment_preimage, option),
306+
(4, _legacy_payment_preimage, (legacy, Option<PaymentPreimage>,
307+
|v: Option<&Option<PaymentPreimage>>| {
308+
// Only use the legacy preimage if the new `resolution` TLV (9) wasn't read,
309+
// otherwise we'd clobber it (losing e.g. skimmed_fee_msat).
310+
if resolution.0.is_none() {
311+
if let Some(Some(preimage)) = v {
312+
resolution = crate::util::ser::RequiredWrapper(Some(
313+
OutboundHTLCResolution::Claimed { preimage: *preimage, skimmed_fee_msat: None }
314+
));
315+
}
316+
}
317+
Ok(())
318+
},
319+
|us: &HTLCUpdate| match &us.resolution {
320+
OutboundHTLCResolution::Claimed { preimage, .. } => Some(Some(*preimage)),
321+
_ => None,
322+
})),
271323
(5, user_channel_id, option),
272324
(7, htlc_value_msat, (default_value, htlc_value_satoshis.map(|v: u64| v * 1000))), // Added in 0.4
325+
(9, resolution, (default_value, OutboundHTLCResolution::on_chain_timeout())),
273326
});
274327

275328
/// If an output goes from claimable only by us to claimable by us or our counterparty within this
@@ -3910,7 +3963,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
39103963
// startup until it is explicitly acked.
39113964
self.push_monitor_event(MonitorEvent::HTLCEvent(HTLCUpdate {
39123965
payment_hash: htlc.payment_hash,
3913-
payment_preimage: Some(claim.preimage),
3966+
resolution: OutboundHTLCResolution::Claimed {
3967+
preimage: claim.preimage,
3968+
skimmed_fee_msat: claim.skimmed_fee_msat,
3969+
},
39143970
source: *source.clone(),
39153971
htlc_value_msat: Some(htlc.amount_msat),
39163972
user_channel_id: self.user_channel_id,
@@ -4647,7 +4703,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
46474703
&mut self.pending_monitor_events,
46484704
MonitorEvent::HTLCEvent(HTLCUpdate {
46494705
payment_hash,
4650-
payment_preimage: None,
4706+
resolution: OutboundHTLCResolution::on_chain_timeout(),
46514707
source: source.clone(),
46524708
htlc_value_msat: Some(amount_msat),
46534709
user_channel_id: self.user_channel_id,
@@ -5985,7 +6041,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
59856041
&payment_hash, entry.txid);
59866042
self.push_monitor_event(MonitorEvent::HTLCEvent(HTLCUpdate {
59876043
payment_hash,
5988-
payment_preimage: None,
6044+
resolution: OutboundHTLCResolution::on_chain_timeout(),
59896045
source,
59906046
htlc_value_msat: htlc_value_satoshis.map(|v| v * 1000),
59916047
user_channel_id: self.user_channel_id,
@@ -6096,7 +6152,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
60966152
expires soon at {}", log_bytes!(htlc.payment_hash.0), inbound_htlc_expiry);
60976153
push_monitor_event(&mut self.pending_monitor_events, MonitorEvent::HTLCEvent(HTLCUpdate {
60986154
source: source.clone(),
6099-
payment_preimage: None,
6155+
resolution: OutboundHTLCResolution::on_chain_timeout(),
61006156
payment_hash: htlc.payment_hash,
61016157
htlc_value_msat: Some(htlc.amount_msat),
61026158
user_channel_id: self.user_channel_id,
@@ -6514,7 +6570,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
65146570
self.counterparty_fulfilled_htlcs.insert(SentHTLCId::from_source(&source), payment_preimage);
65156571
push_monitor_event(&mut self.pending_monitor_events, MonitorEvent::HTLCEvent(HTLCUpdate {
65166572
source,
6517-
payment_preimage: Some(payment_preimage),
6573+
resolution: OutboundHTLCResolution::Claimed { preimage: payment_preimage, skimmed_fee_msat: None },
65186574
payment_hash,
65196575
htlc_value_msat: Some(amount_msat),
65206576
user_channel_id: self.user_channel_id,
@@ -6539,7 +6595,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
65396595
self.counterparty_fulfilled_htlcs.insert(SentHTLCId::from_source(&source), payment_preimage);
65406596
push_monitor_event(&mut self.pending_monitor_events, MonitorEvent::HTLCEvent(HTLCUpdate {
65416597
source,
6542-
payment_preimage: Some(payment_preimage),
6598+
resolution: OutboundHTLCResolution::Claimed { preimage: payment_preimage, skimmed_fee_msat: None },
65436599
payment_hash,
65446600
htlc_value_msat: Some(amount_msat),
65456601
user_channel_id: self.user_channel_id,

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 57 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ use crate::chain::chaininterface::{
4545
use crate::chain::chainmonitor::MonitorEventSource;
4646
use crate::chain::channelmonitor::{
4747
ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, MonitorEvent,
48-
WithChannelMonitor, ANTI_REORG_DELAY, CLTV_CLAIM_BUFFER, HTLC_FAIL_BACK_BUFFER,
49-
LATENCY_GRACE_PERIOD_BLOCKS, MAX_BLOCKS_FOR_CONF,
48+
OutboundHTLCResolution, WithChannelMonitor, ANTI_REORG_DELAY, CLTV_CLAIM_BUFFER,
49+
HTLC_FAIL_BACK_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, MAX_BLOCKS_FOR_CONF,
5050
};
5151
use crate::chain::transaction::{OutPoint, TransactionData};
5252
use crate::chain::{BlockLocator, ChannelMonitorUpdateStatus, Confirm, Watch};
@@ -13936,60 +13936,62 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1393613936
Some(channel_id),
1393713937
Some(htlc_update.payment_hash),
1393813938
);
13939-
if let Some(preimage) = htlc_update.payment_preimage {
13940-
log_trace!(
13941-
logger,
13942-
"Claiming HTLC with preimage {} from our monitor",
13943-
preimage
13944-
);
13945-
let from_onchain = self
13946-
.per_peer_state
13947-
.read()
13948-
.unwrap()
13949-
.get(&counterparty_node_id)
13950-
.map_or(true, |peer_state_mtx| {
13951-
!peer_state_mtx
13952-
.lock()
13953-
.unwrap()
13954-
.channel_by_id
13955-
.contains_key(&channel_id)
13939+
match htlc_update.resolution {
13940+
OutboundHTLCResolution::Claimed { preimage, skimmed_fee_msat } => {
13941+
log_trace!(
13942+
logger,
13943+
"Claiming HTLC with preimage {} from our monitor",
13944+
preimage
13945+
);
13946+
let from_onchain = self
13947+
.per_peer_state
13948+
.read()
13949+
.unwrap()
13950+
.get(&counterparty_node_id)
13951+
.map_or(true, |peer_state_mtx| {
13952+
!peer_state_mtx
13953+
.lock()
13954+
.unwrap()
13955+
.channel_by_id
13956+
.contains_key(&channel_id)
13957+
});
13958+
// Claim the funds from the previous hop, if there is one. In the future we can
13959+
// store attribution data in the `ChannelMonitor` and provide it here.
13960+
self.claim_funds_internal(
13961+
htlc_update.source,
13962+
preimage,
13963+
htlc_update.htlc_value_msat,
13964+
skimmed_fee_msat,
13965+
from_onchain,
13966+
counterparty_node_id,
13967+
funding_outpoint,
13968+
channel_id,
13969+
htlc_update.user_channel_id,
13970+
None,
13971+
None,
13972+
Some(event_id),
13973+
);
13974+
},
13975+
OutboundHTLCResolution::Failed { reason } => {
13976+
log_trace!(logger, "Failing HTLC from our monitor");
13977+
let failure_type = htlc_update
13978+
.source
13979+
.failure_type(counterparty_node_id, channel_id);
13980+
let completion_update = Some(PaymentCompleteUpdate {
13981+
counterparty_node_id,
13982+
channel_funding_outpoint: funding_outpoint,
13983+
channel_id,
13984+
htlc_id: SentHTLCId::from_source(&htlc_update.source),
1395613985
});
13957-
// Claim the funds from the previous hop, if there is one. In the future we can
13958-
// store attribution data in the `ChannelMonitor` and provide it here.
13959-
self.claim_funds_internal(
13960-
htlc_update.source,
13961-
preimage,
13962-
htlc_update.htlc_value_msat,
13963-
None,
13964-
from_onchain,
13965-
counterparty_node_id,
13966-
funding_outpoint,
13967-
channel_id,
13968-
htlc_update.user_channel_id,
13969-
None,
13970-
None,
13971-
Some(event_id),
13972-
);
13973-
} else {
13974-
log_trace!(logger, "Failing HTLC from our monitor");
13975-
let failure_reason = LocalHTLCFailureReason::OnChainTimeout;
13976-
let failure_type =
13977-
htlc_update.source.failure_type(counterparty_node_id, channel_id);
13978-
let reason = HTLCFailReason::from_failure_code(failure_reason);
13979-
let completion_update = Some(PaymentCompleteUpdate {
13980-
counterparty_node_id,
13981-
channel_funding_outpoint: funding_outpoint,
13982-
channel_id,
13983-
htlc_id: SentHTLCId::from_source(&htlc_update.source),
13984-
});
13985-
self.fail_htlc_backwards_internal(
13986-
&htlc_update.source,
13987-
&htlc_update.payment_hash,
13988-
&reason,
13989-
failure_type,
13990-
completion_update,
13991-
);
13992-
self.chain_monitor.ack_monitor_event(monitor_event_source);
13986+
self.fail_htlc_backwards_internal(
13987+
&htlc_update.source,
13988+
&htlc_update.payment_hash,
13989+
&reason,
13990+
failure_type,
13991+
completion_update,
13992+
);
13993+
self.chain_monitor.ack_monitor_event(monitor_event_source);
13994+
},
1399313995
}
1399413996
},
1399513997
MonitorEvent::HolderForceClosed(_)

‎lightning/src/ln/onion_utils.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1910,11 +1910,11 @@ impl From<&HTLCFailReason> for HTLCHandlingFailureReason {
19101910
}
19111911

19121912
#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
1913-
#[cfg_attr(test, derive(PartialEq))]
1914-
pub(super) struct HTLCFailReason(HTLCFailReasonRepr);
1913+
#[derive(PartialEq, Eq)]
1914+
pub(crate) struct HTLCFailReason(HTLCFailReasonRepr);
19151915

19161916
#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
1917-
#[cfg_attr(test, derive(PartialEq))]
1917+
#[derive(PartialEq, Eq)]
19181918
enum HTLCFailReasonRepr {
19191919
LightningError { err: msgs::OnionErrorPacket, hold_time: Option<u32> },
19201920
Reason { data: Vec<u8>, failure_reason: LocalHTLCFailureReason },
@@ -2073,7 +2073,7 @@ impl HTLCFailReason {
20732073
Self(HTLCFailReasonRepr::Reason { data, failure_reason })
20742074
}
20752075

2076-
pub(super) fn from_failure_code(failure_reason: LocalHTLCFailureReason) -> Self {
2076+
pub(crate) fn from_failure_code(failure_reason: LocalHTLCFailureReason) -> Self {
20772077
Self::reason(failure_reason, Vec::new())
20782078
}
20792079

‎lightning/src/ln/payment_tests.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -966,7 +966,9 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
966966
// Drive the commitment signed dance manually so we can account for the extra monitor
967967
// update when persistent monitor events are enabled.
968968
let persistent = nodes[1].node.test_persistent_monitor_events_enabled();
969-
nodes[1].node.handle_commitment_signed_batch_test(node_c_id, &htlc_fulfill.commitment_signed);
969+
nodes[1]
970+
.node
971+
.handle_commitment_signed_batch_test(node_c_id, &htlc_fulfill.commitment_signed);
970972
check_added_monitors(&nodes[1], 1);
971973
let (extra_msg, cs_raa, htlcs) =
972974
do_main_commitment_signed_dance(&nodes[1], &nodes[2], false);

0 commit comments

Comments
 (0)