ln/refactor: move on chain timeout check into claimable htlc
What changed, and why it matters
This commit is a simple internal code cleanup. It moves an existing check that decides when an in-flight payment has waited too long on the blockchain into a reusable helper method. No behavior changes; it just prepares the code for future reuse with trampoline multi-part payments.
No security action needed. Treat as routine refactoring.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors channelmanager.rs by extracting the on-chain HTLC timeout comparison (height >= cltv_expiry - HTLC_FAIL_BACK_BUFFER) into a new MppPart::check_onchain_timeout method. The existing call site is updated to use this helper. The logic, constants, and failure handling remain identical.
Changed components
lightning/src/ln/channelmanager.rsInspect captured patch +15 / −8
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 26eb42d..cd1eb39 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -535,6 +535,14 @@ struct MppPart {
total_value_received: Option<u64>,
}
+impl MppPart {
+ /// Returns a boolean indicating whether the HTLC has timed out on chain, accounting for a buffer
+ /// that gives us time to resolve it.
+ fn check_onchain_timeout(&self, height: u32) -> bool {
+ height >= self.cltv_expiry - HTLC_FAIL_BACK_BUFFER
+ }
+}
+
impl PartialOrd for MppPart {
fn partial_cmp(&self, other: &MppPart) -> Option<cmp::Ordering> {
Some(self.cmp(other))
@@ -16214,14 +16222,15 @@ impl<
}
if let Some(height) = height_opt {
+ // If height is approaching the number of blocks we think it takes us to get our
+ // commitment transaction confirmed before the HTLC expires, plus the number of blocks
+ // we generally consider it to take to do a commitment update, just give up on it and
+ // fail the HTLC.
self.claimable_payments.lock().unwrap().claimable_payments.retain(
|payment_hash, payment| {
payment.htlcs.retain(|htlc| {
- // If height is approaching the number of blocks we think it takes us to get
- // our commitment transaction confirmed before the HTLC expires, plus the
- // number of blocks we generally consider it to take to do a commitment update,
- // just give up on it and fail the HTLC.
- if height >= htlc.mpp_part.cltv_expiry - HTLC_FAIL_BACK_BUFFER {
+ let htlc_timed_out = htlc.mpp_part.check_onchain_timeout(height);
+ if htlc_timed_out {
let reason = LocalHTLCFailureReason::PaymentClaimBuffer;
timed_out_htlcs.push((
HTLCSource::PreviousHopData(htlc.mpp_part.prev_hop.clone()),
@@ -16234,10 +16243,8 @@ impl<
payment_hash: payment_hash.clone(),
},
));
- false
- } else {
- true
}
+ !htlc_timed_out
});
!payment.htlcs.is_empty() // Only retain this entry if htlcs has at least one entry.
},
Why this scored 15/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.