Skip to content

Queue async monitor update writes synchronously#4671

Open
tnull wants to merge 1 commit into
lightningdevkit:mainfrom
tnull:2026-06-async-monitor-update-ordering
Open

Queue async monitor update writes synchronously#4671
tnull wants to merge 1 commit into
lightningdevkit:mainfrom
tnull:2026-06-async-monitor-update-ordering

Conversation

@tnull

@tnull tnull commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Monitor update persistence relies on KVStore write calls being queued in call order. The incremental async update path deferred that write until executor polling, allowing async scheduling to reorder persistence operations relative to later updates or cleanup.

Add a regression test that builds an incremental update future without polling it and verifies that the write has already been queued.

Co-Authored-By: HAL 9000

This finding was discovered by Project Loupe

Monitor update persistence relies on KVStore write calls being queued in
call order. The incremental async update path deferred that write until
executor polling, allowing async scheduling to reorder persistence
operations relative to later updates or cleanup.

Add a regression test that builds an incremental update future without
polling it and verifies that the write has already been queued.

Co-Authored-By: HAL 9000

This finding was discovered by Project Loupe
@tnull tnull requested a review from TheBlueMatt June 10, 2026 11:37
@ldk-reviews-bot

ldk-reviews-bot commented Jun 10, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @TheBlueMatt 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-claude-review-bot

ldk-claude-review-bot commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

The fix is correct and consistent with the persist_new_channel pattern at line 1597; res_a is awaited at line 1633 with a valid 'static boxed future. The test correctly forces the incremental path via maximum_pending_updates = update_id + 1 and asserts synchronous queueing. I have no new issues to add beyond my prior review pass.

No issues found.

@tnull tnull self-assigned this Jun 11, 2026
@tnull tnull moved this to Goal: Merge in Weekly Goals Jun 11, 2026
@tnull tnull force-pushed the 2026-06-async-monitor-update-ordering branch from a3967b5 to 8ac52cf Compare June 11, 2026 16:43
@tnull

tnull commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Updated to line-wrap commit messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Goal: Merge

Development

Successfully merging this pull request may close these issues.

3 participants