Conversation
`isGenesisReady()` flips true as soon as the genesis wallet syncs, which happens well before `runInitialSplit()` seeds the pool. Readiness only knew about `degraded`, so for that whole window `/ready` returned 200 and `/fund` was served against an empty pool. That is worse than a confusing POOL_EXHAUSTED. A request in the window triggers a background refill that reserves from the same candidate set the bootstrap's own split is drawing on; losing that race fails the bootstrap into `degraded`, which never self-heals because `bootPromise` is memoized. So a client doing the correct thing — polling /ready, then funding — could permanently poison the service. Readiness now takes the lifecycle phase instead of a `startupDegraded` boolean, and requires `ready`. The lossy boolean was the root of it: a phase collapsed to "is it degraded" cannot express "not finished". The new `funding_initializing` reason covers `idle` and `initializing`, and `/fund` refuses the same phases so the two can never disagree. The funds query now runs only at phase `ready`, where its answer can change the verdict. Behavioural note for consumers: the 503 window is now much longer. Genesis sync plus the block-reward height lock are waited out inside the bootstrap, so a cold private network can sit at `funding_initializing` for tens of minutes where `/ready` previously flipped 200 within seconds of sync. That is the honest answer, but healthcheck retry budgets sized against the old behaviour will need raising — README says so explicitly. Closes #25 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both waits are near-immediate on a stack that is actually working: genesis sync against a reachable fullnode returns in seconds, and a block-reward height lock clears in seconds once blocks are being produced. A minute leaves headroom for both. A longer ceiling buys nothing. It cannot rescue a fullnode that is unreachable or a network that has stopped producing blocks — it only holds /ready and /fund at a transient `funding_initializing` while a developer waits for a verdict that is not coming. Reaching `degraded` promptly is the signal that something needs debugging. This also bounds the readiness window the previous commit introduced: the bootstrap now settles within a few minutes at worst instead of tens, so healthcheck budgets stay reasonable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 58 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
tuliomir
changed the title
There was a problem hiding this comment.
Pull request overview
This PR fixes a readiness/funding race where the service could report /ready before the funding bootstrap finished seeding the UTXO pool, allowing /fund to run early and potentially push the bootstrap into a permanently degraded state. It does so by gating readiness and /fund on the funding subsystem’s lifecycle StartupPhase (must be ready), adding an explicit funding_initializing readiness reason for the pre-seed window, and tightening related bootstrap timeouts with updated documentation and tests.
Changes:
- Make readiness depend on the full startup lifecycle phase (not just a degraded boolean), introducing
funding_initializinguntil phaseready. - Refuse
/fundunless the startup phase isready(and keepdegradedexplicitly non-ready). - Tighten default genesis sync / reward-unlock timeouts and update docs + tests accordingly.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/startup.ts | Exports StartupPhase so readiness and tests can key off the lifecycle phase. |
| src/routes.ts | Gates /ready and /fund on StartupPhase, adds funding_initializing, and avoids wallet-funds queries before bootstrap completion. |
| src/genesis.service.ts | Reduces default reward-unlock wait timeout to fail faster into degraded when blocks aren’t progressing. |
| src/config.ts | Reduces default GENESIS_SYNC_TIMEOUT_MS to 60s (bounded 1s–600s) to surface misconfigurations faster. |
| README.md | Documents the new readiness semantics (funding_initializing until bootstrap completes) and updated operational guidance. |
| CLAUDE.md | Updates architecture notes to match the new readiness/phase gating. |
| .env.example | Updates documented default for GENESIS_SYNC_TIMEOUT_MS and guidance for overrides. |
| tests/src/routes-readiness.test.ts | Adds coverage ensuring funding_initializing short-circuits without querying wallet funds. |
| tests/src/readiness.test.ts | Updates pure readiness tests for phase-based gating and adds initializing/idle cases. |
| tests/src/fund-endpoint.test.ts | Adds assertions that /fund returns 503 while initializing or idle. |
| tests/src/fund-endpoint-error-codes.test.ts | Ensures tests pin startup state to ready so domain error mapping is exercised. |
| tests/src/config.test.ts | Updates default timeout expectation to 60s. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (startupPhase !== "ready") { | ||
| return jsonErrorFromService( | ||
| new ServiceNotReadyError("funding subsystem is still initializing"), | ||
| ); | ||
| } |
/readyreturned200and/fundwas served during the window between the genesis wallet syncing and the initial split seeding the UTXO pool. This shipped in v0.0.2.isGenesisReady()flips true as soon as the wallet syncs, which happens well beforerunInitialSplit()completes. Readiness only knew aboutdegraded, so for that whole window the service advertised itself as able to fund an empty pool.Why this is worse than a confusing POOL_EXHAUSTED
A
/fundrequest in that window triggers a background refill that reserves from the same candidate set the bootstrap's own split is drawing on. Losing that race fails the bootstrap intodegraded, which never self-heals becausebootPromiseis memoized (#16).So a client doing exactly the right thing — polling
/ready, then funding — could permanently poison the service for its whole lifetime.How it works
computeReadinesstakes the lifecycle phase instead of astartupDegradedboolean and requiresready. The lossy boolean was the root cause: a phase collapsed to "is it degraded" cannot express "not finished". A newfunding_initializingreason coversidleandinitializing, andhandleFundrefuses the same phases so the two endpoints can never disagree. The wallet funds query now runs only at phaseready, where its answer can change the verdict.StartupPhaseso readiness can consume it losslessly.GENESIS_SYNC_TIMEOUT_MSare capped at a minute each. Both are near-immediate on a working stack, and a longer ceiling cannot rescue an unreachable fullnode or a network that has stopped producing blocks — it only delays thedegradedverdict a developer needs in order to debug.phase === "ready"provably implies a seeded pool:setStartupState("ready")is the last statement of the bootstrap, afterrunInitialSplit(), which returns only when the pool holds at least one test UTXO and otherwise throws intodegraded.Behavioral change
The
503window is longer than before, because it is now honest./readypreviously flipped200within seconds of genesis sync; it now waits for the bootstrap to finish seeding. With both timeouts capped at a minute the bootstrap settles within a few minutes at worst, but a healthcheck retry budget sized against the old behaviour may need raising. The README states the expectation explicitly./fundreturns503 SERVICE_NOT_READY(retryable) during the window rather than accepting work it cannot serve.Acceptance criteria
/readyreports503 funding_initializingwhile the bootstrap isidleorinitializing, and/fundrefuses the same phases.degradedstill reportsfunding_degraded— the transient reason must not shadow a state that never self-heals.ready.bun run checkpasses.Closes #25
Stack created with GitHub Stacks CLI • Give Feedback 💬