Skip to content

Commit ba35014

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 6375ec5 commit ba35014

5 files changed

Lines changed: 322 additions & 78 deletions

File tree

‎lightning/src/chain/channelmonitor.rs‎

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

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

276329
/// If an output goes from claimable only by us to claimable by us or our counterparty within this
@@ -3938,7 +3991,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
39383991
// startup until it is explicitly acked.
39393992
self.push_monitor_event(MonitorEvent::HTLCEvent(HTLCUpdate {
39403993
payment_hash: htlc.payment_hash,
3941-
payment_preimage: Some(claim.preimage),
3994+
resolution: OutboundHTLCResolution::Claimed {
3995+
preimage: claim.preimage,
3996+
skimmed_fee_msat: claim.skimmed_fee_msat,
3997+
},
39423998
source: *source.clone(),
39433999
htlc_value_msat: Some(htlc.amount_msat),
39444000
user_channel_id: self.user_channel_id,
@@ -4704,7 +4760,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
47044760
&mut self.pending_monitor_events,
47054761
MonitorEvent::HTLCEvent(HTLCUpdate {
47064762
payment_hash,
4707-
payment_preimage: None,
4763+
resolution: OutboundHTLCResolution::on_chain_timeout(),
47084764
source: source.clone(),
47094765
htlc_value_msat: Some(amount_msat),
47104766
user_channel_id: self.user_channel_id,
@@ -6042,7 +6098,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
60426098
&payment_hash, entry.txid);
60436099
self.push_monitor_event(MonitorEvent::HTLCEvent(HTLCUpdate {
60446100
payment_hash,
6045-
payment_preimage: None,
6101+
resolution: OutboundHTLCResolution::on_chain_timeout(),
60466102
source,
60476103
htlc_value_msat: htlc_value_satoshis.map(|v| v * 1000),
60486104
user_channel_id: self.user_channel_id,
@@ -6152,7 +6208,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
61526208
expires soon at {}", log_bytes!(htlc.payment_hash.0), inbound_htlc_expiry);
61536209
push_monitor_event(&mut self.pending_monitor_events, MonitorEvent::HTLCEvent(HTLCUpdate {
61546210
source: source.clone(),
6155-
payment_preimage: None,
6211+
resolution: OutboundHTLCResolution::on_chain_timeout(),
61566212
payment_hash: htlc.payment_hash,
61576213
htlc_value_msat: Some(htlc.amount_msat),
61586214
user_channel_id: self.user_channel_id,
@@ -6570,7 +6626,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
65706626
self.counterparty_fulfilled_htlcs.insert(SentHTLCId::from_source(&source), payment_preimage);
65716627
push_monitor_event(&mut self.pending_monitor_events, MonitorEvent::HTLCEvent(HTLCUpdate {
65726628
source,
6573-
payment_preimage: Some(payment_preimage),
6629+
resolution: OutboundHTLCResolution::Claimed { preimage: payment_preimage, skimmed_fee_msat: None },
65746630
payment_hash,
65756631
htlc_value_msat: Some(amount_msat),
65766632
user_channel_id: self.user_channel_id,
@@ -6595,7 +6651,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
65956651
self.counterparty_fulfilled_htlcs.insert(SentHTLCId::from_source(&source), payment_preimage);
65966652
push_monitor_event(&mut self.pending_monitor_events, MonitorEvent::HTLCEvent(HTLCUpdate {
65976653
source,
6598-
payment_preimage: Some(payment_preimage),
6654+
resolution: OutboundHTLCResolution::Claimed { preimage: payment_preimage, skimmed_fee_msat: None },
65996655
payment_hash,
66006656
htlc_value_msat: Some(amount_msat),
66016657
user_channel_id: self.user_channel_id,

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 63 additions & 61 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};
@@ -14164,67 +14164,69 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1416414164
Some(channel_id),
1416514165
Some(htlc_update.payment_hash),
1416614166
);
14167-
if let Some(preimage) = htlc_update.payment_preimage {
14168-
log_trace!(
14169-
logger,
14170-
"Claiming HTLC with preimage {} from our monitor",
14171-
preimage
14172-
);
14173-
let from_onchain = self
14174-
.per_peer_state
14175-
.read()
14176-
.unwrap()
14177-
.get(&counterparty_node_id)
14178-
.map_or(true, |peer_state_mtx| {
14179-
!peer_state_mtx
14180-
.lock()
14181-
.unwrap()
14182-
.channel_by_id
14183-
.contains_key(&channel_id)
14167+
match htlc_update.resolution {
14168+
OutboundHTLCResolution::Claimed { preimage, skimmed_fee_msat } => {
14169+
log_trace!(
14170+
logger,
14171+
"Claiming HTLC with preimage {} from our monitor",
14172+
preimage
14173+
);
14174+
let from_onchain = self
14175+
.per_peer_state
14176+
.read()
14177+
.unwrap()
14178+
.get(&counterparty_node_id)
14179+
.map_or(true, |peer_state_mtx| {
14180+
!peer_state_mtx
14181+
.lock()
14182+
.unwrap()
14183+
.channel_by_id
14184+
.contains_key(&channel_id)
14185+
});
14186+
if self.persistent_monitor_events {
14187+
self.register_pending_forward_monitor_event_acks(
14188+
htlc_update.payment_hash,
14189+
htlc_update.source.previous_hop_data(),
14190+
monitor_event_source,
14191+
);
14192+
}
14193+
// Claim the funds from the previous hop, if there is one. In the future we can
14194+
// store attribution data in the `ChannelMonitor` and provide it here.
14195+
self.claim_funds_internal(
14196+
htlc_update.source,
14197+
preimage,
14198+
htlc_update.htlc_value_msat,
14199+
skimmed_fee_msat,
14200+
from_onchain,
14201+
counterparty_node_id,
14202+
funding_outpoint,
14203+
channel_id,
14204+
htlc_update.user_channel_id,
14205+
None,
14206+
None,
14207+
Some(event_id),
14208+
);
14209+
},
14210+
OutboundHTLCResolution::Failed { reason } => {
14211+
log_trace!(logger, "Failing HTLC from our monitor");
14212+
let failure_type = htlc_update
14213+
.source
14214+
.failure_type(counterparty_node_id, channel_id);
14215+
let completion_update = Some(PaymentCompleteUpdate {
14216+
counterparty_node_id,
14217+
channel_funding_outpoint: funding_outpoint,
14218+
channel_id,
14219+
htlc_id: SentHTLCId::from_source(&htlc_update.source),
1418414220
});
14185-
if self.persistent_monitor_events {
14186-
self.register_pending_forward_monitor_event_acks(
14187-
htlc_update.payment_hash,
14188-
htlc_update.source.previous_hop_data(),
14189-
monitor_event_source,
14221+
self.fail_htlc_backwards_internal(
14222+
&htlc_update.source,
14223+
&htlc_update.payment_hash,
14224+
&reason,
14225+
failure_type,
14226+
completion_update,
1419014227
);
14191-
}
14192-
// Claim the funds from the previous hop, if there is one. In the future we can
14193-
// store attribution data in the `ChannelMonitor` and provide it here.
14194-
self.claim_funds_internal(
14195-
htlc_update.source,
14196-
preimage,
14197-
htlc_update.htlc_value_msat,
14198-
None,
14199-
from_onchain,
14200-
counterparty_node_id,
14201-
funding_outpoint,
14202-
channel_id,
14203-
htlc_update.user_channel_id,
14204-
None,
14205-
None,
14206-
Some(event_id),
14207-
);
14208-
} else {
14209-
log_trace!(logger, "Failing HTLC from our monitor");
14210-
let failure_reason = LocalHTLCFailureReason::OnChainTimeout;
14211-
let failure_type =
14212-
htlc_update.source.failure_type(counterparty_node_id, channel_id);
14213-
let reason = HTLCFailReason::from_failure_code(failure_reason);
14214-
let completion_update = Some(PaymentCompleteUpdate {
14215-
counterparty_node_id,
14216-
channel_funding_outpoint: funding_outpoint,
14217-
channel_id,
14218-
htlc_id: SentHTLCId::from_source(&htlc_update.source),
14219-
});
14220-
self.fail_htlc_backwards_internal(
14221-
&htlc_update.source,
14222-
&htlc_update.payment_hash,
14223-
&reason,
14224-
failure_type,
14225-
completion_update,
14226-
);
14227-
self.chain_monitor.ack_monitor_event(monitor_event_source);
14228+
self.chain_monitor.ack_monitor_event(monitor_event_source);
14229+
},
1422814230
}
1422914231
},
1423014232
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)