Repository navigation
EventQueue: load the persisted queue at start - #116
Open
hash-money wants to merge 1 commit into
Open
hash-money wants to merge 1 commit into
hash-money wants to merge 1 commit into
Conversation
`EventQueue::new` always began with an empty `VecDeque`, while every push and every `event_handled` persisted the queue under `orange_events`. The key was written and never read, so events still unhandled when the process exited were lost on the next run. ldk-node reads its own queue back at build time (`io::utils::read_event_queue`); mirror that: `new` reads the key, starts empty on `NotFound`, and fails the build on any other read error or on a queue that does not decode, as ldk-node does. The restart test fails on the previous code (the reloaded queue is empty). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
EventQueue::newalways starts with an emptyVecDeque, while every push and everyevent_handledpersist the queue underorange_events(EVENT_QUEUE_PERSISTENCE_KEY). The key is written and never read, so any event still unhandled when the process exits is lost on the next run. ldk-node reads its own queue back at build time (io::utils::read_event_queue; inbuilder.rsNotFoundis a fresh queue and any other error isBuildError::ReadFailed).This mirrors that:
newbecomesasyncand fallible, reads the key, starts empty onNotFound, and fails the build (InitFailure::IoError) on any other read error or on a queue that does not decode. The reader is the counterpart ofEventQueueSerWrapper(au16count, then the events), as in ldk-node. A store that errors on read now failsWallet::newhere rather than at ldk-node'sbuild_with_storeof the same store a few lines later (BuildError::ReadFailed), so nothing that built before fails now.Delivery becomes at-least-once across a restart, since an LDK event is persisted here before it is acknowledged to ldk-node. That is the delivery consumers of ldk-node's own queue already get: it is reloaded at build time and an event stays at its front until
event_handled; the new doc onEventQueue::newsays so.Seen in the field: after restarting a wallet whose consumer had stalled (#115), the only event that came back was the one ldk-node replayed from its own queue; nothing from this queue did.
Tests:
unhandled_events_survive_a_restart(fails onmaster: the reloaded queue is empty),an_undecodable_stored_queue_fails_the_buildandan_unreadable_store_fails_the_build.Checked:
cargo fmt --check, the clippy line fromci/check-lint.sh,RUSTDOCFLAGS=-D warnings cargo doc --no-deps,cargo test -p orange-sdk --lib,cargo check --features _test-utils --tests. The_test-utilsintegration suite needs bitcoind/electrs and was not run here.Touches the same file as #115 and conflicts with it textually (the constructor's return, the test module, the file tail); neither depends on the other, so whichever lands second gets rebased. Until this lands we carry it on the same fork pin as #115.