Skip to content

Use CollectionLength for the persisted event queue length - #1145

Open
cryptokvc wants to merge 1 commit into
lightningdevkit:mainfrom
cryptokvc:patch-1
Open

cryptokvc wants to merge 1 commit into
lightningdevkit:mainfrom
cryptokvc:patch-1

Conversation

@cryptokvc

Copy link
Copy Markdown

Fixes #1138.

EventQueueSerWrapper wrote the queue length as u16, while still writing every event, so EventQueueDeserWrapper restored a truncated queue: 65,536 persisted events came back empty and 65,540 came back as 4.

This switches both wrappers to lightning::util::ser::CollectionLength, as suggested in the issue:

  • counts below 65,535 keep the existing two-byte encoding, so existing snapshots decode unchanged;
  • larger counts use the 0xffff marker followed by the remainder;
  • as noted in the issue, this assumes no existing snapshot holds exactly 65,535 events;
  • the reader bounds its preallocation to 1,024 entries, since the length comes from persisted data.

Tests (in event::tests):

  • event_queue_round_trips_around_u16_max: full round trip at 65,534, 65,535, 65,536 and 65,540 events. It fails on main (65,536 events decode as 0) and passes with this change;
  • event_queue_keeps_legacy_encoding_below_u16_max: a small queue encodes to the same bytes as the legacy u16 format and decodes from them.

cargo test --lib passes locally (203 tests); I didn't run the integration tests, which need bitcoind/electrs.

AI disclosure: written with AI assistance (Claude); I reviewed the change and ran the tests above.

🤖 Generated with Claude Code

EventQueueSerWrapper cast the queue length to u16, so a queue with
65,536 or more events was restored truncated on restart (65,536 events
came back empty). Write and read the count as CollectionLength, which
keeps the two-byte encoding below 65,535 and uses the 0xffff extension
above it, and bound the reader's preallocation.

Add round-trip tests at 65,534, 65,535, 65,536 and 65,540 events and a
test that small queues keep the legacy encoding.

Written with AI assistance (Claude); reviewed and tested locally.

Fixes lightningdevkit#1138.

Co-authored-by: Claude <noreply@anthropic.com>
@ldk-reviews-bot

ldk-reviews-bot commented Oct 9, 2026 •

Copy link
Copy Markdown

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot
ldk-reviews-bot requested a review from tnull October 9, 2026 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avoid truncating persisted event queues above u16::MAX

2 participants