fix(satoshi-wallet): recover from runtime Core wallet unloads - #1294
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The wallet-info route suppresses RPC -18, preventing recovery and the documented 503 response.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds runtime recovery when Bitcoin Core unloads the Satoshi watch-only wallet.
Changes:
- Introduces shared setup/retry coordination and
-18detection. - Returns 503 during recovery and monitors idle unloads via metrics.
- Adds unit/integration coverage and documentation.
| File | Description |
|---|---|
wallet-recovery.ts |
Coordinates readiness, retries, and fatal failures. |
wallet-api.ts |
Integrates runtime recovery into API and metrics. |
runtime-recovery.test.ts |
Tests coordinator behavior. |
runtime-recovery.integration.mjs |
Tests built-service recovery with RPC fixtures. |
package.json |
Adds the recovery integration command. |
README.md |
Documents recovery behavior and testing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
macterra
enabled auto-merge (squash)
September 25, 2026 18:31
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.
After Bitcoin Core unloaded the configured watch-only wallet, Satoshi wallet requests could fail indefinitely because successful startup permanently disabled setup retries. Core RPC error
-18now clears wallet readiness and schedules the existing 30-second setup retry. Requests return 503 for that condition; the existing 60-second metrics collector also detects idle unloads.Startup, manual setup, and runtime recovery share one in-flight setup attempt. Recovery reuses descriptor validation, stops on descriptor mismatch or missing SQLite support, and clears the retry timer on success. Failed sends, anchors, and fee bumps are returned to the caller without automatic replay. Runtime unload detection applies only to the Bitcoin Core backend.
Updated #1288 and the service documentation to reflect this scope. Added fast coordinator tests and a separate built-service integration command that uses the actual Bitcoin RPC client against isolated HTTP fixtures with real 30/60-second timers.
Validation:
npm run test:recovery --prefix services/mediators/satoshi-walletpassed: successful startup, concurrent request failures with HTTP 500 / RPC -18 envelopes, HTTP 503 responses, automatic reload, idle metrics recovery, unrelated RPC errors retaining 500, descriptor refusal, and explicit manual repair. Uses no live wallet or node.Closes #1288.