Skip to content

Commit 8baadf4

Browse files
committed
Don't double-claim MPP payments that are pending on multiple chans
On startup, we walk the preimages and payment HTLC sets on all our `ChannelMonitor`s, re-claiming all payments which we recently claimed. This ensures all HTLCs in any claimed payments are claimed across all channels. In doing so, we expect to see the same payment multiple times, after all it may have been received as multiple HTLCs across multiple channels. In such cases, there's no reason to redundantly claim the same set of HTLCs again and again. In the current code, doing so may lead to redundant `PaymentClaimed` events, and in a coming commit will instead cause an assertion failure.
1 parent 548d1ae commit 8baadf4

File tree

2 files changed

+11
-2
lines changed

2 files changed

+11
-2
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1166,7 +1166,7 @@ impl From<&MPPClaimHTLCSource> for HTLCClaimSource {
11661166
}
11671167
}
11681168

1169-
#[derive(Clone, Debug, PartialEq, Eq)]
1169+
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
11701170
/// The source of an HTLC which is being claimed as a part of an incoming payment. Each part is
11711171
/// tracked in [`PendingMPPClaim`] as well as in [`ChannelMonitor`]s, so that it can be converted
11721172
/// to an [`HTLCClaimSource`] for claim replays on startup.
@@ -14224,10 +14224,18 @@ where
1422414224
testing_dnssec_proof_offer_resolution_override: Mutex::new(new_hash_map()),
1422514225
};
1422614226

14227+
let mut processed_claims: HashSet<Vec<MPPClaimHTLCSource>> = new_hash_set();
1422714228
for (_, monitor) in args.channel_monitors.iter() {
1422814229
for (payment_hash, (payment_preimage, payment_claims)) in monitor.get_stored_preimages() {
1422914230
if !payment_claims.is_empty() {
1423014231
for payment_claim in payment_claims {
14232+
if processed_claims.contains(&payment_claim.mpp_parts) {
14233+
// We might get the same payment a few times from different channels
14234+
// that the MPP payment was received using. There's no point in trying
14235+
// to claim the same payment again and again, so we check if the HTLCs
14236+
// are the same and skip the payment here.
14237+
continue;
14238+
}
1423114239
if payment_claim.mpp_parts.is_empty() {
1423214240
return Err(DecodeError::InvalidValue);
1423314241
}
@@ -14282,6 +14290,7 @@ where
1428214290
(Some(MonitorUpdateCompletionAction::PaymentClaimed { payment_hash, pending_mpp_claim }), pending_claim_ptr)
1428314291
);
1428414292
}
14293+
processed_claims.insert(payment_claim.mpp_parts);
1428514294
}
1428614295
} else {
1428714296
let per_peer_state = channel_manager.per_peer_state.read().unwrap();

lightning/src/ln/reload_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,7 @@ fn do_test_partial_claim_before_restart(persist_both_monitors: bool) {
900900
if persist_both_monitors {
901901
if let Event::ChannelClosed { reason: ClosureReason::OutdatedChannelManager, .. } = events[2] { } else { panic!(); }
902902
if let Event::PaymentClaimed { amount_msat: 15_000_000, .. } = events[3] { } else { panic!(); }
903-
check_added_monitors(&nodes[3], 6);
903+
check_added_monitors(&nodes[3], 4);
904904
} else {
905905
if let Event::PaymentClaimed { amount_msat: 15_000_000, .. } = events[2] { } else { panic!(); }
906906
check_added_monitors(&nodes[3], 3);

0 commit comments

Comments
 (0)