Skip to content

Commit 059e32a

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 09a0f24 commit 059e32a

File tree

2 files changed

+11
-2
lines changed

2 files changed

+11
-2
lines changed

lightning/src/ln/channelmanager.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -1167,7 +1167,7 @@ impl From<&MPPClaimHTLCSource> for HTLCClaimSource {
11671167
}
11681168
}
11691169

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

14164+
let mut processed_claims: HashSet<Vec<MPPClaimHTLCSource>> = new_hash_set();
1416414165
for (_, monitor) in args.channel_monitors.iter() {
1416514166
for (payment_hash, (payment_preimage, payment_claims)) in monitor.get_stored_preimages() {
1416614167
if !payment_claims.is_empty() {
1416714168
for payment_claim in payment_claims {
14169+
if processed_claims.contains(&payment_claim.mpp_parts) {
14170+
// We might get the same payment a few times from different channels
14171+
// that the MPP payment was received using. There's no point in trying
14172+
// to claim the same payment again and again, so we check if the HTLCs
14173+
// are the same and skip the payment here.
14174+
continue;
14175+
}
1416814176
if payment_claim.mpp_parts.is_empty() {
1416914177
return Err(DecodeError::InvalidValue);
1417014178
}
@@ -14219,6 +14227,7 @@ where
1421914227
(Some(MonitorUpdateCompletionAction::PaymentClaimed { payment_hash, pending_mpp_claim }), pending_claim_ptr)
1422014228
);
1422114229
}
14230+
processed_claims.insert(payment_claim.mpp_parts);
1422214231
}
1422314232
} else {
1422414233
let per_peer_state = channel_manager.per_peer_state.read().unwrap();

lightning/src/ln/reload_tests.rs

+1-1
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)