Skip to content

Commit 80946b8

Browse files
committed
Maintain order of yielded claim events
Since the claim events are stored internally within a HashMap, they will be yielded in a random order once dispatched. Claim events may be invalidated if a conflicting claim has confirmed on-chain and we need to generate a new claim event; the randomized order could result in the new claim event being handled prior to the previous. To maintain the order in which the claim events are generated, we track them in a Vec instead and ensure only one instance of a PackageId only ever exists within it. This would have certain performance implications, but since we're bounded by the total number of HTLCs in a commitment anyway, we're comfortable with taking the cost.
1 parent 0ab997b commit 80946b8

File tree

1 file changed

+25
-13
lines changed

1 file changed

+25
-13
lines changed

lightning/src/chain/onchaintx.rs

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,10 @@ pub struct OnchainTxHandler<ChannelSigner: WriteableEcdsaChannelSigner> {
252252
pub(crate) pending_claim_requests: HashMap<PackageID, PackageTemplate>,
253253
#[cfg(not(test))]
254254
pending_claim_requests: HashMap<PackageID, PackageTemplate>,
255+
256+
// Used to track external events that need to be forwarded to the `ChainMonitor`.
255257
#[cfg(anchors)]
256-
pending_claim_events: HashMap<PackageID, ClaimEvent>,
258+
pending_claim_events: Vec<(PackageID, ClaimEvent)>,
257259

258260
// Used to link outpoints claimed in a connected block to a pending claim request. The keys
259261
// represent the outpoints that our `ChannelMonitor` has detected we have keys/scripts to claim.
@@ -430,7 +432,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
430432
pending_claim_requests,
431433
onchain_events_awaiting_threshold_conf,
432434
#[cfg(anchors)]
433-
pending_claim_events: HashMap::new(),
435+
pending_claim_events: Vec::new(),
434436
secp_ctx,
435437
})
436438
}
@@ -451,8 +453,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
451453
locktimed_packages: BTreeMap::new(),
452454
onchain_events_awaiting_threshold_conf: Vec::new(),
453455
#[cfg(anchors)]
454-
pending_claim_events: HashMap::new(),
455-
456+
pending_claim_events: Vec::new(),
456457
secp_ctx,
457458
}
458459
}
@@ -467,9 +468,9 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
467468

468469
#[cfg(anchors)]
469470
pub(crate) fn get_and_clear_pending_claim_events(&mut self) -> Vec<ClaimEvent> {
470-
let mut ret = HashMap::new();
471-
swap(&mut ret, &mut self.pending_claim_events);
472-
ret.into_iter().map(|(_, event)| event).collect::<Vec<_>>()
471+
let mut events = Vec::new();
472+
swap(&mut events, &mut self.pending_claim_events);
473+
events.into_iter().map(|(_, event)| event).collect()
473474
}
474475

475476
/// Lightning security model (i.e being able to redeem/timeout HTLC or penalize counterparty
@@ -713,7 +714,8 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
713714
package_id
714715
},
715716
};
716-
self.pending_claim_events.insert(package_id, claim_event);
717+
debug_assert_eq!(self.pending_claim_events.iter().filter(|entry| entry.0 == package_id).count(), 0);
718+
self.pending_claim_events.push((package_id, claim_event));
717719
package_id
718720
},
719721
};
@@ -833,9 +835,9 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
833835
log_debug!(logger, "Removing claim tracking for {} due to maturation of claim package {}.",
834836
outpoint, log_bytes!(package_id));
835837
self.claimable_outpoints.remove(outpoint);
836-
#[cfg(anchors)]
837-
self.pending_claim_events.remove(&package_id);
838838
}
839+
#[cfg(anchors)]
840+
self.pending_claim_events.retain(|(id, _)| *id != package_id);
839841
}
840842
},
841843
OnchainEvent::ContentiousOutpoint { package } => {
@@ -870,7 +872,12 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
870872
#[cfg(anchors)]
871873
OnchainClaim::Event(claim_event) => {
872874
log_info!(logger, "Yielding RBF-bumped onchain event to spend inputs {:?}", request.outpoints());
873-
self.pending_claim_events.insert(*package_id, claim_event);
875+
if let Some((existing_claim_idx, _)) = self.pending_claim_events.iter().enumerate()
876+
.find(|(_, entry)| entry.0 == *package_id)
877+
{
878+
self.pending_claim_events.remove(existing_claim_idx);
879+
}
880+
self.pending_claim_events.push((*package_id, claim_event));
874881
},
875882
}
876883
if let Some(request) = self.pending_claim_requests.get_mut(package_id) {
@@ -934,7 +941,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
934941
self.onchain_events_awaiting_threshold_conf.push(entry);
935942
}
936943
}
937-
for ((_package_id, _), request) in bump_candidates.iter_mut() {
944+
for ((_package_id, _), ref mut request) in bump_candidates.iter_mut() {
938945
if let Some((new_timer, new_feerate, bump_claim)) = self.generate_claim(height, &request, fee_estimator, &&*logger) {
939946
request.set_timer(new_timer);
940947
request.set_feerate(new_feerate);
@@ -946,7 +953,12 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
946953
#[cfg(anchors)]
947954
OnchainClaim::Event(claim_event) => {
948955
log_info!(logger, "Yielding onchain event after reorg to spend inputs {:?}", request.outpoints());
949-
self.pending_claim_events.insert(_package_id, claim_event);
956+
if let Some((existing_claim_idx, _)) = self.pending_claim_events.iter().enumerate()
957+
.find(|(_, entry)| entry.0 == *_package_id)
958+
{
959+
self.pending_claim_events.remove(existing_claim_idx);
960+
}
961+
self.pending_claim_events.push((*_package_id, claim_event));
950962
},
951963
}
952964
}

0 commit comments

Comments
 (0)