Commit 510ff78
committed
Reclaim forwarded-payment replay markers instead of leaking them
ForwardingStore::record_forward writes a permanent
ForwardedPaymentReplayMarker for every forwarded HTLC, used to guard
against LDK replaying an event whose side effects we already
recorded. Nothing removed these markers: the existing aggregation
pass reclaimed detail records but never touched the marker store, so
it grew without bound for the life of the node. This was worse in
the default Stats tracking mode, which never writes a detail record
for aggregation to key cleanup off, and the background loop used to
exit for good the first time it observed an empty details store
under that mode -- immediately, on a routing node's very first
startup.
Fixes:
- Add a forwarded_at_timestamp field to the marker (TLV required;
this store hasn't shipped in a release, so there's no on-disk
format to stay compatible with).
- Add an age-based sweep, prune_expired_replay_markers, that runs
every aggregation cycle independent of retention_secs, so Stats
mode reclaims markers too. A marker is only removed once it's past
a fixed one-bucket-wide age cutoff and has no corresponding detail
left -- age alone isn't sufficient, since the aggregation pass
defers an entire bucket whenever any one detail in it is still
missing its own marker.
- Skip the per-marker detail lookup when the details store is empty,
the common case in Stats mode.
- Remove the background loop's early-exit logic entirely rather than
patching it further: it could permanently stop reclaiming on a
fresh or idle node with nothing to reclaim yet. The loop now runs
unconditionally for the life of the node in every tracking mode.
- Pair marker removal with detail removal during aggregation itself,
not just the separate sweep, so Detailed mode reclaims almost all
of its markers immediately.
- Gate marker removal, both the paired kind and the sweep, behind
one bucket width of node uptime. On startup the background
processor may still need to replay an event whose marker we're
relying on to recognize the replay and skip double-counting it;
after a long outage that event's detail can already look old
enough to aggregate on the very first pass. The startup pass now
aggregates without touching any markers, leaving them for the
delayed sweep once it's safe.
- Log and skip a marker-removal failure instead of aborting the rest
of the pass with `?` -- a single failure no longer blocks cleanup
of every other payment in the same batch.
Covered by new tests for: markers leaking across both tracking
modes, the exact bucket-width age cutoff, a sibling-bucket
corruption case where age-only pruning would strand a still-present
detail, the background loop no longer exiting while markers remain,
the startup pass not sweeping or pairing-removing a marker too
early in both tracking modes, and a marker-removal failure not
aborting the rest of a pass. For every substantive change, the
corresponding piece was manually reverted to confirm the test fails
with the predicted symptom before restoring it.1 parent 43dcb60 commit 510ff78
1 file changed
Lines changed: 719 additions & 55 deletions
0 commit comments