Skip to content

Commit dc37d4d

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 8a418ba commit dc37d4d

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
@@ -1169,7 +1169,7 @@ impl From<&MPPClaimHTLCSource> for HTLCClaimSource {
11691169
}
11701170
}
11711171

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

14245+
let mut processed_claims: HashSet<Vec<MPPClaimHTLCSource>> = new_hash_set();
1424514246
for (_, monitor) in args.channel_monitors.iter() {
1424614247
for (payment_hash, (payment_preimage, payment_claims)) in monitor.get_stored_preimages() {
1424714248
if !payment_claims.is_empty() {
1424814249
for payment_claim in payment_claims {
14250+
if processed_claims.contains(&payment_claim.mpp_parts) {
14251+
// We might get the same payment a few times from different channels
14252+
// that the MPP payment was received using. There's no point in trying
14253+
// to claim the same payment again and again, so we check if the HTLCs
14254+
// are the same and skip the payment here.
14255+
continue;
14256+
}
1424914257
if payment_claim.mpp_parts.is_empty() {
1425014258
return Err(DecodeError::InvalidValue);
1425114259
}
@@ -14300,6 +14308,7 @@ where
1430014308
(Some(MonitorUpdateCompletionAction::PaymentClaimed { payment_hash, pending_mpp_claim }), pending_claim_ptr)
1430114309
);
1430214310
}
14311+
processed_claims.insert(payment_claim.mpp_parts);
1430314312
}
1430414313
} else {
1430514314
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)