Skip to content

Commit 0e890f3

Browse files
TheBlueMattTheBlueMatt
committed
(XXX: Handle monitor restoration return) Inform ChannelManager when fulfilled HTLCs are finalized
When an HTLC has been failed, we track it up until the point there exists no broadcastable commitment transaction which has the HTLC present, at which point Channel returns the HTLCSource back to the ChannelManager, which fails the HTLC backwards appropriately. When an HTLC is fulfilled, however, we fulfill on the backwards path immediately. This is great for claiming upstream HTLCs, but when we want to track pending payments, we need to ensure we can check with ChannelMonitor data to rebuild pending payments. In order to do so, we need an event similar to the HTLC failure event, but for fulfills instead. This commit does so, informing the ChannelManager via a new return element where appropriate of the HTLCSource corresponding to the failed HTLC.
1 parent da8d4d9 commit 0e890f3

File tree

2 files changed

+41
-14
lines changed

2 files changed

+41
-14
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,7 @@ pub(super) struct RAAUpdates {
345345
pub commitment_update: Option<msgs::CommitmentUpdate>,
346346
pub to_forward_htlcs: Vec<(PendingHTLCInfo, u64)>,
347347
pub failed_htlcs: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
348+
pub finalized_claim_htlcs: Vec<HTLCSource>,
348349
pub monitor_update: ChannelMonitorUpdate,
349350
pub holding_cell_failed_htlcs: Vec<(HTLCSource, PaymentHash)>,
350351
}
@@ -416,6 +417,7 @@ pub(super) struct Channel<Signer: Sign> {
416417
monitor_pending_commitment_signed: bool,
417418
monitor_pending_forwards: Vec<(PendingHTLCInfo, u64)>,
418419
monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
420+
monitor_pending_finalized_fulfills: Vec<HTLCSource>,
419421

420422
// pending_update_fee is filled when sending and receiving update_fee.
421423
//
@@ -702,6 +704,7 @@ impl<Signer: Sign> Channel<Signer> {
702704
monitor_pending_commitment_signed: false,
703705
monitor_pending_forwards: Vec::new(),
704706
monitor_pending_failures: Vec::new(),
707+
monitor_pending_finalized_fulfills: Vec::new(),
705708

706709
#[cfg(debug_assertions)]
707710
holder_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
@@ -965,6 +968,7 @@ impl<Signer: Sign> Channel<Signer> {
965968
monitor_pending_commitment_signed: false,
966969
monitor_pending_forwards: Vec::new(),
967970
monitor_pending_failures: Vec::new(),
971+
monitor_pending_finalized_fulfills: Vec::new(),
968972

969973
#[cfg(debug_assertions)]
970974
holder_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
@@ -2778,6 +2782,7 @@ impl<Signer: Sign> Channel<Signer> {
27782782
log_trace!(logger, "Updating HTLCs on receipt of RAA in channel {}...", log_bytes!(self.channel_id()));
27792783
let mut to_forward_infos = Vec::new();
27802784
let mut revoked_htlcs = Vec::new();
2785+
let mut finalized_claim_htlcs = Vec::new();
27812786
let mut update_fail_htlcs = Vec::new();
27822787
let mut update_fail_malformed_htlcs = Vec::new();
27832788
let mut require_commitment = false;
@@ -2804,6 +2809,7 @@ impl<Signer: Sign> Channel<Signer> {
28042809
if let Some(reason) = fail_reason.clone() { // We really want take() here, but, again, non-mut ref :(
28052810
revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
28062811
} else {
2812+
finalized_claim_htlcs.push(htlc.source.clone());
28072813
// They fulfilled, so we sent them money
28082814
value_to_self_msat_diff -= htlc.amount_msat as i64;
28092815
}
@@ -2900,9 +2906,10 @@ impl<Signer: Sign> Channel<Signer> {
29002906
}
29012907
self.monitor_pending_forwards.append(&mut to_forward_infos);
29022908
self.monitor_pending_failures.append(&mut revoked_htlcs);
2909+
self.monitor_pending_finalized_fulfills.append(&mut finalized_claim_htlcs);
29032910
log_debug!(logger, "Received a valid revoke_and_ack for channel {} but awaiting a monitor update resolution to reply.", log_bytes!(self.channel_id()));
29042911
return Ok(RAAUpdates {
2905-
commitment_update: None,
2912+
commitment_update: None, finalized_claim_htlcs: Vec::new(),
29062913
to_forward_htlcs: Vec::new(), failed_htlcs: Vec::new(),
29072914
monitor_update,
29082915
holding_cell_failed_htlcs: Vec::new()
@@ -2927,6 +2934,7 @@ impl<Signer: Sign> Channel<Signer> {
29272934

29282935
Ok(RAAUpdates {
29292936
commitment_update: Some(commitment_update),
2937+
finalized_claim_htlcs,
29302938
to_forward_htlcs: to_forward_infos,
29312939
failed_htlcs: revoked_htlcs,
29322940
monitor_update,
@@ -2953,13 +2961,15 @@ impl<Signer: Sign> Channel<Signer> {
29532961
update_fee: None,
29542962
commitment_signed
29552963
}),
2964+
finalized_claim_htlcs,
29562965
to_forward_htlcs: to_forward_infos, failed_htlcs: revoked_htlcs,
29572966
monitor_update, holding_cell_failed_htlcs: htlcs_to_fail
29582967
})
29592968
} else {
29602969
log_debug!(logger, "Received a valid revoke_and_ack for channel {} with no reply necessary.", log_bytes!(self.channel_id()));
29612970
Ok(RAAUpdates {
29622971
commitment_update: None,
2972+
finalized_claim_htlcs,
29632973
to_forward_htlcs: to_forward_infos, failed_htlcs: revoked_htlcs,
29642974
monitor_update, holding_cell_failed_htlcs: htlcs_to_fail
29652975
})
@@ -3077,11 +3087,16 @@ impl<Signer: Sign> Channel<Signer> {
30773087
/// which failed. The messages which were generated from that call which generated the
30783088
/// monitor update failure must *not* have been sent to the remote end, and must instead
30793089
/// have been dropped. They will be regenerated when monitor_updating_restored is called.
3080-
pub fn monitor_update_failed(&mut self, resend_raa: bool, resend_commitment: bool, mut pending_forwards: Vec<(PendingHTLCInfo, u64)>, mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>) {
3090+
pub fn monitor_update_failed(&mut self, resend_raa: bool, resend_commitment: bool,
3091+
mut pending_forwards: Vec<(PendingHTLCInfo, u64)>,
3092+
mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
3093+
mut pending_finalized_claims: Vec<HTLCSource>
3094+
) {
30813095
self.monitor_pending_revoke_and_ack |= resend_raa;
30823096
self.monitor_pending_commitment_signed |= resend_commitment;
30833097
self.monitor_pending_forwards.append(&mut pending_forwards);
30843098
self.monitor_pending_failures.append(&mut pending_fails);
3099+
self.monitor_pending_finalized_fulfills.append(&mut pending_finalized_claims);
30853100
self.channel_state |= ChannelState::MonitorUpdateFailed as u32;
30863101
}
30873102

@@ -3115,6 +3130,8 @@ impl<Signer: Sign> Channel<Signer> {
31153130
mem::swap(&mut forwards, &mut self.monitor_pending_forwards);
31163131
let mut failures = Vec::new();
31173132
mem::swap(&mut failures, &mut self.monitor_pending_failures);
3133+
let mut finalized_claims = Vec::new(); // XXX return this
3134+
mem::swap(&mut finalized_claims, &mut self.monitor_pending_finalized_fulfills);
31183135

31193136
if self.channel_state & (ChannelState::PeerDisconnected as u32) != 0 {
31203137
self.monitor_pending_revoke_and_ack = false;
@@ -5196,6 +5213,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
51965213
(5, self.config, required),
51975214
(7, self.shutdown_scriptpubkey, option),
51985215
(9, self.target_closing_feerate_sats_per_kw, option),
5216+
(11, self.monitor_pending_finalized_fulfills, vec_type),
51995217
});
52005218

52015219
Ok(())
@@ -5429,13 +5447,15 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
54295447

54305448
let mut announcement_sigs = None;
54315449
let mut target_closing_feerate_sats_per_kw = None;
5450+
let mut monitor_pending_finalized_fulfills = Some(Vec::new());
54325451
read_tlv_fields!(reader, {
54335452
(0, announcement_sigs, option),
54345453
(1, minimum_depth, option),
54355454
(3, counterparty_selected_channel_reserve_satoshis, option),
54365455
(5, config, option), // Note that if none is provided we will *not* overwrite the existing one.
54375456
(7, shutdown_scriptpubkey, option),
54385457
(9, target_closing_feerate_sats_per_kw, option),
5458+
(11, monitor_pending_finalized_fulfills, vec_type),
54395459
});
54405460

54415461
let mut secp_ctx = Secp256k1::new();
@@ -5471,6 +5491,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
54715491
monitor_pending_commitment_signed,
54725492
monitor_pending_forwards,
54735493
monitor_pending_failures,
5494+
monitor_pending_finalized_fulfills: monitor_pending_finalized_fulfills.unwrap(),
54745495

54755496
pending_update_fee,
54765497
holding_cell_update_fee,

‎lightning/src/ln/channelmanager.rs

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -997,7 +997,7 @@ macro_rules! handle_monitor_err {
997997
($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr) => {
998998
handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, Vec::new(), Vec::new())
999999
};
1000-
($self: ident, $err: expr, $short_to_id: expr, $chan: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr, $chan_id: expr) => {
1000+
($self: ident, $err: expr, $short_to_id: expr, $chan: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr, $failed_finalized_fulfills: expr, $chan_id: expr) => {
10011001
match $err {
10021002
ChannelMonitorUpdateErr::PermanentFailure => {
10031003
log_error!($self.logger, "Closing channel {} due to monitor update ChannelMonitorUpdateErr::PermanentFailure", log_bytes!($chan_id[..]));
@@ -1018,7 +1018,7 @@ macro_rules! handle_monitor_err {
10181018
(res, true)
10191019
},
10201020
ChannelMonitorUpdateErr::TemporaryFailure => {
1021-
log_info!($self.logger, "Disabling channel {} due to monitor update TemporaryFailure. On restore will send {} and process {} forwards and {} fails",
1021+
log_info!($self.logger, "Disabling channel {} due to monitor update TemporaryFailure. On restore will send {} and process {} forwards, {} fails, and {} fulfill finalizations",
10221022
log_bytes!($chan_id[..]),
10231023
if $resend_commitment && $resend_raa {
10241024
match $action_type {
@@ -1029,25 +1029,29 @@ macro_rules! handle_monitor_err {
10291029
else if $resend_raa { "RAA" }
10301030
else { "nothing" },
10311031
(&$failed_forwards as &Vec<(PendingHTLCInfo, u64)>).len(),
1032-
(&$failed_fails as &Vec<(HTLCSource, PaymentHash, HTLCFailReason)>).len());
1032+
(&$failed_fails as &Vec<(HTLCSource, PaymentHash, HTLCFailReason)>).len(),
1033+
(&$failed_finalized_fulfills as &Vec<HTLCSource>).len());
10331034
if !$resend_commitment {
10341035
debug_assert!($action_type == RAACommitmentOrder::RevokeAndACKFirst || !$resend_raa);
10351036
}
10361037
if !$resend_raa {
10371038
debug_assert!($action_type == RAACommitmentOrder::CommitmentFirst || !$resend_commitment);
10381039
}
1039-
$chan.monitor_update_failed($resend_raa, $resend_commitment, $failed_forwards, $failed_fails);
1040+
$chan.monitor_update_failed($resend_raa, $resend_commitment, $failed_forwards, $failed_fails, $failed_finalized_fulfills);
10401041
(Err(MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore("Failed to update ChannelMonitor".to_owned()), *$chan_id)), false)
10411042
},
10421043
}
10431044
};
1044-
($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr) => { {
1045-
let (res, drop) = handle_monitor_err!($self, $err, $channel_state.short_to_id, $entry.get_mut(), $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, $entry.key());
1045+
($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr, $failed_finalized_fulfills: expr) => { {
1046+
let (res, drop) = handle_monitor_err!($self, $err, $channel_state.short_to_id, $entry.get_mut(), $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, $failed_finalized_fulfills, $entry.key());
10461047
if drop {
10471048
$entry.remove_entry();
10481049
}
10491050
res
10501051
} };
1052+
($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr) => {
1053+
handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, Vec::new());
1054+
}
10511055
}
10521056

10531057
macro_rules! return_monitor_err {
@@ -1410,7 +1414,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
14101414
if let Some(monitor_update) = monitor_update {
14111415
if let Err(e) = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), monitor_update) {
14121416
let (result, is_permanent) =
1413-
handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, false, false, Vec::new(), Vec::new(), chan_entry.key());
1417+
handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, false, false, Vec::new(), Vec::new(), Vec::new(), chan_entry.key());
14141418
if is_permanent {
14151419
remove_channel!(channel_state, chan_entry);
14161420
break result;
@@ -2820,7 +2824,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
28202824
let ret_err = match res {
28212825
Ok(Some((update_fee, commitment_signed, monitor_update))) => {
28222826
if let Err(e) = self.chain_monitor.update_channel(chan.get_funding_txo().unwrap(), monitor_update) {
2823-
let (res, drop) = handle_monitor_err!(self, e, short_to_id, chan, RAACommitmentOrder::CommitmentFirst, false, true, Vec::new(), Vec::new(), chan_id);
2827+
let (res, drop) = handle_monitor_err!(self, e, short_to_id, chan, RAACommitmentOrder::CommitmentFirst, false, true, Vec::new(), Vec::new(), Vec::new(), chan_id);
28242828
if drop { retain_channel = false; }
28252829
res
28262830
} else {
@@ -3512,7 +3516,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
35123516
// hasn't persisted to disk yet - we can't lose money on a transaction that we haven't
35133517
// accepted payment from yet. We do, however, need to wait to send our funding_locked
35143518
// until we have persisted our monitor.
3515-
chan.monitor_update_failed(false, false, Vec::new(), Vec::new());
3519+
chan.monitor_update_failed(false, false, Vec::new(), Vec::new(), Vec::new());
35163520
},
35173521
}
35183522
}
@@ -3630,7 +3634,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
36303634
if let Some(monitor_update) = monitor_update {
36313635
if let Err(e) = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), monitor_update) {
36323636
let (result, is_permanent) =
3633-
handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, false, false, Vec::new(), Vec::new(), chan_entry.key());
3637+
handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, false, false, Vec::new(), Vec::new(), Vec::new(), chan_entry.key());
36343638
if is_permanent {
36353639
remove_channel!(channel_state, chan_entry);
36363640
break result;
@@ -3926,12 +3930,14 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
39263930
assert!(raa_updates.commitment_update.is_none());
39273931
assert!(raa_updates.to_forward_htlcs.is_empty());
39283932
assert!(raa_updates.failed_htlcs.is_empty());
3933+
assert!(raa_updates.finalized_claim_htlcs.is_empty());
39293934
break Err(MsgHandleErrInternal::ignore_no_close("Previous monitor update failure prevented responses to RAA".to_owned()));
39303935
} else {
39313936
if let Err(e) = handle_monitor_err!(self, e, channel_state, chan,
39323937
RAACommitmentOrder::CommitmentFirst, false,
39333938
raa_updates.commitment_update.is_some(),
3934-
raa_updates.to_forward_htlcs, raa_updates.failed_htlcs) {
3939+
raa_updates.to_forward_htlcs, raa_updates.failed_htlcs,
3940+
raa_updates.finalized_claim_htlcs) {
39353941
break Err(e);
39363942
} else { unreachable!(); }
39373943
}
@@ -4168,7 +4174,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
41684174
if let Some((commitment_update, monitor_update)) = commitment_opt {
41694175
if let Err(e) = self.chain_monitor.update_channel(chan.get_funding_txo().unwrap(), monitor_update) {
41704176
has_monitor_update = true;
4171-
let (res, close_channel) = handle_monitor_err!(self, e, short_to_id, chan, RAACommitmentOrder::CommitmentFirst, false, true, Vec::new(), Vec::new(), channel_id);
4177+
let (res, close_channel) = handle_monitor_err!(self, e, short_to_id, chan, RAACommitmentOrder::CommitmentFirst, false, true, Vec::new(), Vec::new(), Vec::new(), channel_id);
41724178
handle_errors.push((chan.get_counterparty_node_id(), res));
41734179
if close_channel { return false; }
41744180
} else {

0 commit comments

Comments
 (0)