test(lnbits): exercise FIFO eviction in `recordSeen` (follow-up to #50) #54

New issue

Open

opened 2026-06-13 22:03:04 +00:00 by padreug · 0 comments

padreug commented

2026-06-13 22:03:04 +00:00

Owner

Migrated from aiolabs/lamassu-next#54 — opened by @padreug on 2026-05-26.\n\n## Gap

packages/lnbits/src/client.ts ships a recordSeen(set, value, max) helper for FIFO eviction on the two dedup caches landed in #50 (seenEventIds, capped at 1000; seenPaymentHashes per-sub, capped at 500). The current 11-test suite exercises hit/miss behaviour but doesn't exercise the eviction itself — a typo (set.size >= max → set.size > max, or worse) would let the cache grow unboundedly and no test would fail.

Surfaced in the review of commit 8c4be01 by the bitspire session.

Fix shape

Two reasonable approaches:

Lower the constant temporarily in a test (e.g. set SEEN_EVENT_IDS_MAX = 3). Cheap, but requires exporting the constant or monkey-patching.
Expose the cap via a constructor option for test injection — e.g. new LnbitsClient({ ..., dedupCaps: { events: 3, hashes: 3 } }). Cleaner, no production change beyond opening the seam.

(2) is probably the right move; the cap shouldn't change between dev and prod, but a test seam keeps the test focused.

Test sketch

it('FIFO-evicts the oldest entry when seenEventIds exceeds the cap', () => {
  const client = new LnbitsClient({
    serverPubkey,
    relays: ['ws://test/'],
    dedupCaps: { events: 3 },
  })
  client.initialize(mockNostr, recipientId)

  // Inject 4 distinct legitimate events
  triggerEvent(buildPushEvent({ paymentHash: 'h1', createdAt: 1 }))
  triggerEvent(buildPushEvent({ paymentHash: 'h2', createdAt: 2 }))
  triggerEvent(buildPushEvent({ paymentHash: 'h3', createdAt: 3 }))
  triggerEvent(buildPushEvent({ paymentHash: 'h4', createdAt: 4 }))

  // Cache should hold {h2, h3, h4} — h1 evicted
  expect((client as any).seenEventIds.size).toBe(3)

  // Re-injecting h1's event should be allowed (not deduped)
  // and re-injecting h2's event should be deduped
  // ... etc
})

Same shape for the per-sub seenPaymentHashes cap.

Out of scope

Not asking to rip up #50. This is a test-only follow-up.
The current caps (1000 / 500) are conservative; this is purely about making the eviction path exercisable so a typo doesn't silently regress.

References

aiolabs/lamassu-next#50 — the dedup work this tests.
Identified during bitspire-session review of commit 8c4be01.
Estimated effort: 30min.

> _Migrated from [aiolabs/lamassu-next#54](https://git.atitlan.io/aiolabs/lamassu-next/issues/54) — opened by @padreug on 2026-05-26._\n\n## Gap `packages/lnbits/src/client.ts` ships a `recordSeen(set, value, max)` helper for FIFO eviction on the two dedup caches landed in #50 (`seenEventIds`, capped at 1000; `seenPaymentHashes` per-sub, capped at 500). The current 11-test suite exercises hit/miss behaviour but **doesn't exercise the eviction itself** — a typo (`set.size >= max` → `set.size > max`, or worse) would let the cache grow unboundedly and no test would fail. Surfaced in the review of commit `8c4be01` by the bitspire session. ## Fix shape Two reasonable approaches: 1. **Lower the constant temporarily** in a test (e.g. set `SEEN_EVENT_IDS_MAX = 3`). Cheap, but requires exporting the constant or monkey-patching. 2. **Expose the cap via a constructor option** for test injection — e.g. `new LnbitsClient({ ..., dedupCaps: { events: 3, hashes: 3 } })`. Cleaner, no production change beyond opening the seam. (2) is probably the right move; the cap shouldn't change between dev and prod, but a test seam keeps the test focused. ## Test sketch ```ts it('FIFO-evicts the oldest entry when seenEventIds exceeds the cap', () => { const client = new LnbitsClient({ serverPubkey, relays: ['ws://test/'], dedupCaps: { events: 3 }, }) client.initialize(mockNostr, recipientId) // Inject 4 distinct legitimate events triggerEvent(buildPushEvent({ paymentHash: 'h1', createdAt: 1 })) triggerEvent(buildPushEvent({ paymentHash: 'h2', createdAt: 2 })) triggerEvent(buildPushEvent({ paymentHash: 'h3', createdAt: 3 })) triggerEvent(buildPushEvent({ paymentHash: 'h4', createdAt: 4 })) // Cache should hold {h2, h3, h4} — h1 evicted expect((client as any).seenEventIds.size).toBe(3) // Re-injecting h1's event should be allowed (not deduped) // and re-injecting h2's event should be deduped // ... etc }) ``` Same shape for the per-sub `seenPaymentHashes` cap. ## Out of scope - Not asking to rip up #50. This is a test-only follow-up. - The current caps (1000 / 500) are conservative; this is purely about making the eviction path exercisable so a typo doesn't silently regress. ## References - `aiolabs/lamassu-next#50` — the dedup work this tests. - Identified during bitspire-session review of commit `8c4be01`. - Estimated effort: 30min.

padreug referenced this issue

2026-06-18 21:38:06 +00:00

track: consume LNbits sidecar bunker (replace VITE_ATM_PRIVATE_KEY with bunker:// URL) #52

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

aiolabs/bitspire#54

No description provided.

Rows
Columns

test(lnbits): exercise FIFO eviction in recordSeen (follow-up to #50) #54

Fix shape

Test sketch

Out of scope

References

test(lnbits): exercise FIFO eviction in `recordSeen` (follow-up to #50) #54