Extract method to dedup pre-decode update_add
What the developer wrote
Authored by Valentine Wallace
Extract method to dedup pre-decode update_add
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.
We'll use this new util when reconstructing the
ChannelManager::decode_update_add_htlcs map from Channel data in upcoming
commits. While the Channel data is not included in the monitors yet, it will be
in future work.
What changed, and why it matters
This commit is a simple internal code cleanup: it pulls out a small block of duplicate logic into a helper function. The behavior of the program is unchanged; no security issue is introduced or fixed.
No action required; this is a non-functional refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change extracts the logic that removes already-forwarded HTLCs from decode_update_add_htlcs into a new private function dedup_decode_update_add_htlcs. The existing call site is replaced with a call to this helper, using the same matching criteria and adding a structured logger context. This is a pure refactor in preparation for future commits that will reuse the helper when reconstructing ChannelManager state from ChannelMonitor data.
Changed components
lightning/src/ln/channelmanager.rsInspect captured patch +37 / −13
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index aa78710..12cfe59 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -16819,6 +16819,38 @@ where
}
}
+// If the HTLC corresponding to `prev_hop_data` is present in `decode_update_add_htlcs`, remove it
+// from the map as it is already being stored and processed elsewhere.
+fn dedup_decode_update_add_htlcs<L: Deref>(
+ decode_update_add_htlcs: &mut HashMap<u64, Vec<msgs::UpdateAddHTLC>>,
+ prev_hop_data: &HTLCPreviousHopData, removal_reason: &'static str, logger: &L,
+) where
+ L::Target: Logger,
+{
+ decode_update_add_htlcs.retain(|src_outb_alias, update_add_htlcs| {
+ update_add_htlcs.retain(|update_add| {
+ let matches = *src_outb_alias == prev_hop_data.prev_outbound_scid_alias
+ && update_add.htlc_id == prev_hop_data.htlc_id;
+ if matches {
+ let logger = WithContext::from(
+ logger,
+ prev_hop_data.counterparty_node_id,
+ Some(update_add.channel_id),
+ Some(update_add.payment_hash),
+ );
+ log_info!(
+ logger,
+ "Removing pending to-decode HTLC with id {}: {}",
+ update_add.htlc_id,
+ removal_reason
+ );
+ }
+ !matches
+ });
+ !update_add_htlcs.is_empty()
+ });
+}
+
// Implement ReadableArgs for an Arc'd ChannelManager to make it a bit easier to work with the
// SipmleArcChannelManager type:
impl<
@@ -17686,19 +17718,11 @@ where
// still have an entry for this HTLC in `forward_htlcs` or
// `pending_intercepted_htlcs`, we were apparently not persisted after
// the monitor was when forwarding the payment.
- decode_update_add_htlcs.retain(
- |src_outb_alias, update_add_htlcs| {
- update_add_htlcs.retain(|update_add_htlc| {
- let matches = *src_outb_alias
- == prev_hop_data.prev_outbound_scid_alias
- && update_add_htlc.htlc_id == prev_hop_data.htlc_id;
- if matches {
- log_info!(logger, "Removing pending to-decode HTLC as it was forwarded to the closed channel");
- }
- !matches
- });
- !update_add_htlcs.is_empty()
- },
+ dedup_decode_update_add_htlcs(
+ &mut decode_update_add_htlcs,
+ &prev_hop_data,
+ "HTLC was forwarded to the closed channel",
+ &args.logger,
);
forward_htlcs.retain(|_, forwards| {
forwards.retain(|forward| {
Why this scored 14/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.