Skip to content

Commit cfd2bd9

Browse files
Gather to-decode HTLC fwds from channels on manager read
We have an overarching goal of (mostly) getting rid of ChannelManager persistence and rebuilding the ChannelManager's state from existing ChannelMonitors, due to issues when the two structs are out-of-sync on restart. The main issue that can arise is channel force closure. Here we start this process by rebuilding ChannelManager::decode_update_add_htlcs from the Channels, which will soon be included in the ChannelMonitors as part of a different series of PRs. The newly built map is not yet used but will be in the next commit.
1 parent 1a75d90 commit cfd2bd9

File tree

2 files changed

+68
-3
lines changed

2 files changed

+68
-3
lines changed

lightning/src/ln/channel.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7778,6 +7778,20 @@ where
77787778
Ok(())
77797779
}
77807780

7781+
/// Useful for reconstructing the set of pending HTLCs when deserializing the `ChannelManager`.
7782+
pub(super) fn get_inbound_committed_update_adds(&self) -> Vec<msgs::UpdateAddHTLC> {
7783+
self.context
7784+
.pending_inbound_htlcs
7785+
.iter()
7786+
.filter_map(|htlc| match htlc.state {
7787+
InboundHTLCState::Committed { ref update_add_htlc_opt } => {
7788+
update_add_htlc_opt.clone()
7789+
},
7790+
_ => None,
7791+
})
7792+
.collect()
7793+
}
7794+
77817795
/// Marks an outbound HTLC which we have received update_fail/fulfill/malformed
77827796
#[inline]
77837797
fn mark_outbound_htlc_removed(

lightning/src/ln/channelmanager.rs

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17358,6 +17358,7 @@ where
1735817358
decode_update_add_htlcs_legacy.unwrap_or_else(|| new_hash_map());
1735917359
let mut pending_intercepted_htlcs_legacy =
1736017360
pending_intercepted_htlcs_legacy.unwrap_or_else(|| new_hash_map());
17361+
let mut decode_update_add_htlcs = new_hash_map();
1736117362
let peer_storage_dir: Vec<(PublicKey, Vec<u8>)> = peer_storage_dir.unwrap_or_else(Vec::new);
1736217363
if fake_scid_rand_bytes.is_none() {
1736317364
fake_scid_rand_bytes = Some(args.entropy_source.get_secure_random_bytes());
@@ -17669,6 +17670,21 @@ where
1766917670
let mut peer_state_lock = peer_state_mtx.lock().unwrap();
1767017671
let peer_state = &mut *peer_state_lock;
1767117672
is_channel_closed = !peer_state.channel_by_id.contains_key(channel_id);
17673+
if let Some(chan) = peer_state.channel_by_id.get(channel_id) {
17674+
if let Some(funded_chan) = chan.as_funded() {
17675+
let inbound_committed_update_adds =
17676+
funded_chan.get_inbound_committed_update_adds();
17677+
if !inbound_committed_update_adds.is_empty() {
17678+
// Reconstruct `ChannelManager::decode_update_add_htlcs` from the serialized
17679+
// `Channel`, as part of removing the requirement to regularly persist the
17680+
// `ChannelManager`.
17681+
decode_update_add_htlcs.insert(
17682+
funded_chan.context.outbound_scid_alias(),
17683+
inbound_committed_update_adds,
17684+
);
17685+
}
17686+
}
17687+
}
1767217688
}
1767317689

1767417690
if is_channel_closed {
@@ -17727,9 +17743,15 @@ where
1772717743
};
1772817744
// The ChannelMonitor is now responsible for this HTLC's
1772917745
// failure/success and will let us know what its outcome is. If we
17730-
// still have an entry for this HTLC in `forward_htlcs` or
17731-
// `pending_intercepted_htlcs`, we were apparently not persisted after
17732-
// the monitor was when forwarding the payment.
17746+
// still have an entry for this HTLC in `forward_htlcs`,
17747+
// `pending_intercepted_htlcs`, or `decode_update_add_htlcs`, we were apparently not
17748+
// persisted after the monitor was when forwarding the payment.
17749+
dedup_decode_update_add_htlcs(
17750+
&mut decode_update_add_htlcs,
17751+
&prev_hop_data,
17752+
"HTLC was forwarded to the closed channel",
17753+
&args.logger,
17754+
);
1773317755
dedup_decode_update_add_htlcs(
1773417756
&mut decode_update_add_htlcs_legacy,
1773517757
&prev_hop_data,
@@ -18220,6 +18242,35 @@ where
1822018242
}
1822118243
}
1822218244

18245+
// In the future, the set of pending HTLCs will be pulled from `Channel{Monitor}` data and
18246+
// placed in `ChannelManager::decode_update_add_htlcs` on read, to be handled on the next call
18247+
// to `process_pending_htlc_forwards`. This is part of a larger effort to remove the requirement
18248+
// of regularly persisting the `ChannelManager`. However, this new pipeline cannot currently
18249+
// handle inbound HTLC receives, some of which may be present in `failed_htlcs`, so continue
18250+
// handling HTLCs present in `failed_htlcs` during deserialization for now.
18251+
for (src, _, _, _, _, _) in failed_htlcs.iter() {
18252+
if let HTLCSource::PreviousHopData(prev_hop_data) = src {
18253+
dedup_decode_update_add_htlcs(
18254+
&mut decode_update_add_htlcs,
18255+
prev_hop_data,
18256+
"HTLC was failed backwards during manager read",
18257+
&args.logger,
18258+
);
18259+
}
18260+
}
18261+
18262+
// See above comment on `failed_htlcs`.
18263+
for htlcs in claimable_payments.values().map(|pmt| &pmt.htlcs) {
18264+
for prev_hop_data in htlcs.iter().map(|h| &h.prev_hop) {
18265+
dedup_decode_update_add_htlcs(
18266+
&mut decode_update_add_htlcs,
18267+
prev_hop_data,
18268+
"HTLC was already decoded and marked as a claimable payment",
18269+
&args.logger,
18270+
);
18271+
}
18272+
}
18273+
1822318274
let best_block = BestBlock::new(best_block_hash, best_block_height);
1822418275
let flow = OffersMessageFlow::new(
1822518276
chain_hash,

0 commit comments

Comments
 (0)