chain: wire NeutrinoClient to actor-based rescan pipeline#1205
Draft
Roasbeef wants to merge 6 commits into
Draft
chain: wire NeutrinoClient to actor-based rescan pipeline#1205Roasbeef wants to merge 6 commits into
Roasbeef wants to merge 6 commits into
Conversation
In this commit, we integrate the new actor+FSM rescan pipeline from the neutrino/rescan sub-package into btcwallet's NeutrinoClient. The integration is gated behind a UseActorRescan bool on NeutrinoClient, allowing a flag-flip rollout without removing the existing code path. When UseActorRescan is true, NotifyReceived and Rescan delegate to the RescanActor instead of the channel-based rescan. The actor's outbox events are translated to the existing notification callbacks (onFilteredBlockConnected, onBlockConnected, onBlockDisconnected) which feed the same enqueueNotification FIFO. This means the wallet's handleChainNotifications and all downstream processing remain completely unchanged. A new NotifyReceivedWithHeight method is added that accepts an optional rewind height (*uint32). When a new address is imported, the caller can specify the address creation height so the actor rewinds and re-scans from that point. The existing NotifyReceived delegates to this with a nil rewind height for backward compatibility. The callback wiring is centralized in buildActorCallbacks() to avoid duplication between the NotifyReceived and Rescan code paths. Helper methods newRescanChainSource(), startRescanActor(), and stopRescanActor() further reduce repetition. The OnTxConfirmed callback uses an interface assertion to call the new ChainService.MarkAsConfirmed method, ensuring confirmed transactions are removed from the broadcaster's rebroadcast pool.
Pull in lnd/fn/v2 v2.0.9 which is required by the neutrino rescan module's actor-based pipeline. The new lnd/actor v0.0.5 and btcsuite/btclog/v2 packages are transitive dependencies of the neutrino rescan actor. Also bump the Go toolchain to 1.25.5.
This commit improves the NeutrinoClient in several ways to prepare for the actor-based rescan path running behind a feature flag alongside the existing legacy rescanner. The constructor is split into NewNeutrinoClient (production path that wraps *neutrino.ChainService) and NewNeutrinoClientWithChainSource (accepts an arbitrary neutrino.ChainSource, primarily for tests). Both now accept a useActorRescan bool and wire up the newActorChainSource closure so that tests can supply a mock ChainSource while still exercising the real actor runtime and mailbox plumbing. Stop() is fixed to properly tear down active rescans. Previously it only closed the quit channel and flipped the started flag, leaking the rescan goroutine and actor. Now it snapshots the rescan, rescanQuit, and rescanActor references under the lock, resets the scanning state, then releases the lock before calling WaitForShutdown and actor.Stop so callbacks can drain without deadlocking. Lock discipline in notifyReceivedActor and rescanActorRescan is tightened: clientMtx is now released before performing blocking operations (chain source queries, actor construction, header lookups) and only re-acquired to install the new actor and flip state flags. The actor is started after the lock is released so its first mailbox read cannot race with the state installation. The rescanMtx acquisition in Rescan and NotifyReceivedWithHeight is hoisted above the UseActorRescan branch so both the legacy and actor-backed paths are serialized through the same external mutex, keeping wallet-side semantics aligned while the actor path runs behind a flag. Finally, newRescanChainSource is changed to return an error and to use the newActorChainSource hook when set, falling back to a type assertion on CS only when the hook is nil. The startRescanActor helper is inlined at both call sites since actor construction and Start are now separated to avoid holding the lock during startup.
…ckends Every existing NeutrinoClient test now runs in a table-driven loop over rescanModes (legacy, actor) so we get the same coverage for both rescan implementations from a single test body. A mockActorChainSource is added to mocks_test.go that implements neutrino.ChainSource with deterministic in-memory headers and heights. newMockNeutrinoClient is extended to accept a useActorRescan bool; when true it wires the mock chain source into the client's newActorChainSource hook so the real actor runtime runs against the mock backend. Helper functions testRescanAddr and testRescanStartHash are added to generate valid addresses and start hashes appropriate for each mode (the actor path needs a real genesis header from the mock chain, while the legacy path uses an empty header hash). The concurrent Rescan+NotifyReceived+NotifyBlocks stress test now collects errors via channels rather than calling require inside goroutines, preventing panics from escaping the test goroutine. A new TestNeutrinoClientStopWithActiveRescan test verifies that both backends can stop cleanly after the client has started an active notification pipeline, checking that started, rescanActor, and rescan are properly reset after shutdown.
Introduce a new UseActorRescan config option that selects the experimental actor/FSM-based SPV rescan implementation instead of the legacy rescanner goroutine. The flag requires --usespv to be set and errors out at config validation time if used without it. The flag is passed through to chain.NewNeutrinoClient in the RPC connect loop so the neutrino client knows which backend to use. Config tests exercise the validation (actor without SPV fails, actor with SPV succeeds, SPV tuning knobs are preserved).
Add wallet-level integration tests that exercise the full Wallet through a NeutrinoClient backed by mock chain services, running each test against both the legacy and actor rescan modes. The test harness (newWalletWithNeutrinoBackend) wires a real Wallet with a real NeutrinoClient but substitutes mock implementations for NeutrinoChainService and neutrino.ChainSource, giving end-to-end coverage of the wallet-to-chain-client interface without requiring an actual neutrino ChainService or network. Tests cover: address generation, rescan initiation, public/private key import with and without rescan, taproot script import, transaction publishing (verifying the mock service receives the broadcast), and txToOutputs coin selection with dry-run and real-commit paths including change address validation.
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
1 participant
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.
In this PR, we integrate the new actor+FSM rescan pipeline from
neutrino/rescan(see lightninglabs/neutrino#340) into btcwallet'sNeutrinoClient. The integration is gated behind aUseActorRescanboolon
NeutrinoClient, allowing a flag-flip rollout without removing theexisting channel-based code path.
When
UseActorRescanis true,NotifyReceivedandRescandelegate tothe
RescanActorinstead of the oldrescannerinterface. The actor'soutbox events are translated to the existing notification callbacks
(
onFilteredBlockConnected,onBlockConnected,onBlockDisconnected)which feed the same
enqueueNotificationFIFO. This means the wallet'shandleChainNotificationsand all downstream processing remain completelyunchanged.
We also add a new
NotifyReceivedWithHeightmethod that accepts an optionalrewind height (
*uint32). When a new address is imported, the caller (e.g.,lnd) can specify the address creation height so the actor rewinds and
re-scans from that point, eliminating the race condition where blocks
between the address import and the filter update are missed. The existing
NotifyReceiveddelegates to this with a nil rewind height for backwardcompatibility.
The callback wiring is centralized in
buildActorCallbacks()to avoidduplication between the
NotifyReceivedandRescancode paths. Helpermethods
newRescanChainSource(),startRescanActor(), andstopRescanActor()further reduce repetition. TheOnTxConfirmedcallbackuses an interface assertion to call
ChainService.MarkAsConfirmed, ensuringconfirmed transactions are removed from the broadcaster's rebroadcast pool.
Depends on lightninglabs/neutrino#340.