Skip to content

Commit e207dd6

Browse files
committed
refactor(spv): single source for the pending-islock expiry check + log
The `is_expired` test and its "Dropping InstantLock for txid ... after awaiting quorum data" warning were duplicated between `validate_pending` (the re-validation loop) and `expire_pending` (the advancement-independent sweep), so a future change to PENDING_TTL or to the message would have to be made in both places. Fold both into `PendingInstantLock::is_expired_logged`, next to the `is_expired` predicate and the `first_seen` field it reads. Callers keep their own bookkeeping — `validate_pending` still counts the drop as invalid and skips re-verification, `expire_pending` still filters the queue and updates progress — so behaviour is unchanged. Addresses the remaining CodeRabbit nitpick on #895.
1 parent a69e57d commit e207dd6

1 file changed

Lines changed: 23 additions & 18 deletions

File tree

dash-spv/src/sync/instantsend/manager.rs

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,27 @@ impl PendingInstantLock {
6969
fn is_expired(&self, now: Instant) -> bool {
7070
now.saturating_duration_since(self.first_seen) > PENDING_TTL
7171
}
72+
73+
/// [`is_expired`](Self::is_expired), logging the drop when it returns true.
74+
///
75+
/// Both expiry paths — the advancement-independent sweep in
76+
/// [`expire_pending`](InstantSendManager::expire_pending) and the
77+
/// re-validation loop in
78+
/// [`validate_pending`](InstantSendManager::validate_pending) — report a
79+
/// dropped lock identically, so the check and its warning live here and
80+
/// cannot drift apart when the TTL or the message changes. Callers remain
81+
/// responsible for their own bookkeeping (progress counters, removal).
82+
fn is_expired_logged(&self, now: Instant) -> bool {
83+
let expired = self.is_expired(now);
84+
if expired {
85+
tracing::warn!(
86+
"Dropping InstantLock for txid {} after awaiting quorum data for over {}s",
87+
self.instant_lock.txid,
88+
PENDING_TTL.as_secs()
89+
);
90+
}
91+
expired
92+
}
7293
}
7394

7495
/// InstantSend manager.
@@ -262,13 +283,7 @@ impl InstantSendManager {
262283
let txid = pending_lock.instant_lock.txid;
263284

264285
// Drop locks that have been awaiting quorum data for too long.
265-
let expired = pending_lock.is_expired(now);
266-
if expired {
267-
tracing::warn!(
268-
"Dropping InstantLock for txid {} after awaiting quorum data for over {}s",
269-
txid,
270-
PENDING_TTL.as_secs()
271-
);
286+
if pending_lock.is_expired_logged(now) {
272287
self.progress.add_invalid(1);
273288
continue;
274289
}
@@ -334,17 +349,7 @@ impl InstantSendManager {
334349
/// see [`PendingInstantLock::is_expired`].
335350
pub(super) fn expire_pending(&mut self, now: Instant) -> usize {
336351
let before = self.pending_instantlocks.len();
337-
self.pending_instantlocks.retain(|pending| {
338-
let expired = pending.is_expired(now);
339-
if expired {
340-
tracing::warn!(
341-
"Dropping InstantLock for txid {} after awaiting quorum data for over {}s",
342-
pending.instant_lock.txid,
343-
PENDING_TTL.as_secs()
344-
);
345-
}
346-
!expired
347-
});
352+
self.pending_instantlocks.retain(|pending| !pending.is_expired_logged(now));
348353
let expired = before - self.pending_instantlocks.len();
349354
if expired > 0 {
350355
self.progress.add_invalid(expired as u32);

0 commit comments

Comments
 (0)