DEM-771: self-healing validator seed on boot + ./run --reset#943
Merged
Conversation
The chain's only validator seed lives inside `generateGenesisBlock`, which `findGenesisBlock` short-circuits past once block 0 is present. If the `validators` table is ever emptied while block 0 exists, the chain silently stalls: `getShard` returns 0 peers, no quorum is reached, `/health` still reports OK. The operator only learns the chain is dead by manually calling `getValidators` and seeing zero. `ensureValidatorSeed` runs after `findGenesisBlock` and reconciles the validators table: no-op when it has any rows (so dynamically staked entries are never overwritten, and legitimately unstaked addresses are never resurrected), and an UPSERT (ON CONFLICT DO NOTHING) re-seed from `data/genesis.json` when it is empty. When the table is empty and there is nothing to seed, the boot throws with the exact remediation command. Also fixes the legacy "no-snapshot" path in `chainGenesis.ts` that intentionally skipped genesis-baked validator seeding — dev/empty chain boot was always producing a dead consensus after first run. Operator UX: ./run --docker --reset wipe + reboot the mainnet/testnet stack ./run --docker --devnet --reset wipe + reboot the devnet stack Locally verified against `docker-compose.devnet.yml`: 1. Fresh boot — re-seed log + 4 validators in DB 2. Healthy restart — no-op, no re-seed log 3. TRUNCATE validators + restart — self-heals to 4 rows 4. `./run --docker --devnet --reset --yes` — wipe + boot reseeds 11/11 unit tests pass under `bun test`. Design memo: test-reports/dem-771-devnet-bootstrap-design.md Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Contributor
WalkthroughAdds an idempotent ChangesDEM-771: Validator Re-seed and Reset Tooling
Sequence Diagram(s)sequenceDiagram
participant preMainLoop
participant filesystem
participant ensureValidatorSeed
participant database
participant snapshot
preMainLoop->>filesystem: readFileSync data/genesis.json
filesystem-->>preMainLoop: raw JSON or error
preMainLoop->>ensureValidatorSeed: ensureValidatorSeed(genesisData)
ensureValidatorSeed->>database: countValidators()
alt table non-empty
ensureValidatorSeed-->>preMainLoop: {reseeded: false, source: null}
else table empty
ensureValidatorSeed->>database: getLastBlockNumber()
ensureValidatorSeed->>snapshot: loadSnapshotValidators()
alt snapshot has validators
snapshot-->>ensureValidatorSeed: stream rows
ensureValidatorSeed->>database: upsertValidators(transaction, orIgnore)
ensureValidatorSeed->>database: countValidators() post-insert
alt count > 0
ensureValidatorSeed-->>preMainLoop: {reseeded: true, source: snapshot, count: N}
else count == 0
ensureValidatorSeed-->>preMainLoop: throw ConsensusInvariantError
end
else snapshot empty or unavailable
alt lastBlock > 0
ensureValidatorSeed-->>preMainLoop: throw ConsensusInvariantError
else lastBlock == 0
ensureValidatorSeed->>ensureValidatorSeed: extractGenesisSeed + validateSeedEntry
ensureValidatorSeed->>database: upsertValidators(transaction, orIgnore)
ensureValidatorSeed->>database: countValidators() post-insert
alt count > 0
ensureValidatorSeed-->>preMainLoop: {reseeded: true, source: genesis, count: N}
else count == 0
ensureValidatorSeed-->>preMainLoop: throw ConsensusInvariantError
end
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
Greptile SummaryThis PR fixes a silent consensus failure where emptying the
Confidence Score: 5/5Safe to merge; the reconciliation routine is conservative (count > 0 is a hard no-op, ON CONFLICT DO NOTHING is defence-in-depth) and the three previously flagged issues (silent catch, stake-guard split, null-element handling) are all addressed in this version. The core logic path — check count, try snapshot, try genesis at block 0, throw loudly otherwise — is straightforward and well-covered by 14 unit tests. The shell changes are surgical and the argument-routing logic handles all documented flag combinations correctly. The one finding (tail log filter missing BOOT) is a documentation/observability gap, not a correctness issue. scripts/wipe-and-reboot.sh — the tail filter and expected-output banner don't yet reflect the new [BOOT] log lines, so operators verifying a reset won't see re-seed confirmation in the live tail. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([Boot: preMainLoop]) --> B[findGenesisBlock]
B --> C[loadGenesisIdentities]
C --> D[ensureValidatorSeed]
D --> E{validators.count > 0?}
E -->|yes| F[no-op ✓]
E -->|no| G[loadSnapshotValidators]
G -->|rows > 0| H[UPSERT from snapshot\nON CONFLICT DO NOTHING]
H --> I[assertSeededAndReturn\nsource=snapshot]
G -->|rows = 0| J{lastBlock > 0?}
J -->|yes| K[❌ ConsensusInvariantError\nrestore snapshot or --reset]
J -->|no block=0| L[extractGenesisSeed]
L -->|seed empty| M[❌ ConsensusInvariantError\nfix genesis.json]
L -->|seed present| N[validateSeedEntry × N]
N --> O[UPSERT from genesis\nON CONFLICT DO NOTHING]
O --> P[assertSeededAndReturn\nsource=genesis]
I --> Q([Boot continues])
P --> Q
F --> Q
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A([Boot: preMainLoop]) --> B[findGenesisBlock]
B --> C[loadGenesisIdentities]
C --> D[ensureValidatorSeed]
D --> E{validators.count > 0?}
E -->|yes| F[no-op ✓]
E -->|no| G[loadSnapshotValidators]
G -->|rows > 0| H[UPSERT from snapshot\nON CONFLICT DO NOTHING]
H --> I[assertSeededAndReturn\nsource=snapshot]
G -->|rows = 0| J{lastBlock > 0?}
J -->|yes| K[❌ ConsensusInvariantError\nrestore snapshot or --reset]
J -->|no block=0| L[extractGenesisSeed]
L -->|seed empty| M[❌ ConsensusInvariantError\nfix genesis.json]
L -->|seed present| N[validateSeedEntry × N]
N --> O[UPSERT from genesis\nON CONFLICT DO NOTHING]
O --> P[assertSeededAndReturn\nsource=genesis]
I --> Q([Boot continues])
P --> Q
F --> Q
Reviews (6): Last reviewed commit: "feat(seed): prefer snapshot over genesis..." | Re-trigger Greptile |
Typing --reset is already an explicit destructive opt-in; requiring the operator to ALSO type --yes (or interactively confirm by typing the word "wipe") is just friction. The interactive prompt in scripts/wipe-and-reboot.sh stays for direct invocations of that script — only the curated ./run entry point auto-yeses. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/blockchain/ensureValidatorSeed.test.ts (1)
145-194: 💤 Low valueConsider adding a test for invalid
statusfield.The test suite covers most validation paths but doesn't explicitly test the
statusvalidation (empty string or non-string type). While the current tests provide good coverage, adding a test forstatusvalidation would ensure complete field coverage.💡 Suggested additional test case
it("rejects empty status", async () => { repoCount.mockResolvedValue(0) await expect( ensureValidatorSeed({ validators: [makeSeed({ status: "" })], }), ).rejects.toThrow(/status/) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/blockchain/ensureValidatorSeed.test.ts` around lines 145 - 194, Add a test case for status field validation in the ensureValidatorSeed test suite. Following the pattern of the existing tests (like "rejects zero stake" and "rejects negative valid_at"), create a new test called "rejects empty status" that calls ensureValidatorSeed with a validator seed where the status field is set to an empty string, and verify that it rejects with an error matching /status/. This ensures complete field validation coverage for the ensureValidatorSeed function.src/libs/blockchain/routines/ensureValidatorSeed.ts (1)
171-199: TypeORMorIgnore()is officially supported—no API changes needed.The
orIgnore()method is documented in TypeORM 0.3.17 and correctly translates toON CONFLICT DO NOTHINGfor PostgreSQL/SQLite. Unit tests verify the method is called. Consider adding an integration test that runs against actual SQLite/PostgreSQL instances to ensure the generated SQL executes correctly with real database constraints, but the current implementation is correct.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/libs/blockchain/routines/ensureValidatorSeed.ts` around lines 171 - 199, The current implementation of upsertValidators in ensureValidatorSeed.ts correctly uses orIgnore() which is officially supported in TypeORM, but lacks integration test coverage to verify the generated SQL executes correctly against real database instances. Add integration tests that run the upsertValidators function against actual SQLite and PostgreSQL databases to ensure the ON CONFLICT DO NOTHING behavior works as expected and properly handles real database constraints when duplicate address values are encountered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test-reports/dem-771-devnet-bootstrap-design.md`:
- Line 5: The design document contains four interconnected discrepancies with
the actual unified implementation and requires updates across multiple sections.
At test-reports/dem-771-devnet-bootstrap-design.md line 5 (anchor), change the
status field from "Proposal (no code merged)" to "Implemented" to reflect that
the code is already merged. At lines 58-72 (sibling), update the pseudo-code
section 4.1 to show that ensureValidatorSeed throws ConsensusInvariantError
directly when the seed is empty rather than returning silently; include the
actual error message thrown. At lines 52-101 (sibling), revise the architecture
description for sections 4.1-4.2 to clarify that 4.2 is no longer a separate
post-call check in src/index.ts but is now conceptually bundled as an integrated
self-check within the ensureValidatorSeed function itself; remove references to
two distinct steps. At lines 145-156 (sibling), update the rollout plan to
reflect the unified implementation: replace the three-PR proposal with a two-PR
plan where PR-1 includes ensureValidatorSeed with integrated self-check plus
tests, and PR-2 includes reset tooling and the --reset entry point; remove the
separate PR for section 4.2 throwing.
---
Nitpick comments:
In `@src/libs/blockchain/routines/ensureValidatorSeed.ts`:
- Around line 171-199: The current implementation of upsertValidators in
ensureValidatorSeed.ts correctly uses orIgnore() which is officially supported
in TypeORM, but lacks integration test coverage to verify the generated SQL
executes correctly against real database instances. Add integration tests that
run the upsertValidators function against actual SQLite and PostgreSQL databases
to ensure the ON CONFLICT DO NOTHING behavior works as expected and properly
handles real database constraints when duplicate address values are encountered.
In `@tests/blockchain/ensureValidatorSeed.test.ts`:
- Around line 145-194: Add a test case for status field validation in the
ensureValidatorSeed test suite. Following the pattern of the existing tests
(like "rejects zero stake" and "rejects negative valid_at"), create a new test
called "rejects empty status" that calls ensureValidatorSeed with a validator
seed where the status field is set to an empty string, and verify that it
rejects with an error matching /status/. This ensures complete field validation
coverage for the ensureValidatorSeed function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da8fdb51-0465-4b33-9342-fd36d3e875b1
📒 Files selected for processing (6)
runscripts/wipe-and-reboot.shsrc/index.tssrc/libs/blockchain/routines/ensureValidatorSeed.tstest-reports/dem-771-devnet-bootstrap-design.mdtests/blockchain/ensureValidatorSeed.test.ts
- src/index.ts: log the actual fs.readFileSync/JSON.parse error before passing null to ensureValidatorSeed, so the operator sees the real cause rather than a misleading "carries no validator set" message when genesis.json was deleted post-findGenesisBlock. - ensureValidatorSeed.ts: split the BigInt parse from the > 0 check so the operator gets a distinct, actionable message for each failure mode (matches seedValidators.ts::validateSeed exactly). - tests: add empty-status + non-string-status cases; tighten the zero/non-bigint stake assertions to the new precise messages. - design memo: status updated to "Implemented in PR #943", §4.1 + §4.2 unified into the single-routine shape, rollout reduced to one PR, follow-ups split into their own tickets. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Greptile P1: a stray `null` (or any non-object) in data/genesis.json's validators[] would leak a native `Cannot read properties of null` TypeError instead of the ConsensusInvariantError every other failure path goes through. Add an object guard as the first check in validateSeedEntry + two new tests covering null and primitive elements. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
Bot review threads on this PR are stale relative to the latest commits — all findings have been addressed in code. Re-triggering the bots so the resolved threads reflect the actual state. @coderabbitai review |
Contributor
|
✅ Action performedReview finished.
|
Auto-reseeding from data/genesis.json on a chain that has advanced
past block 0 is unsafe: it would resurrect founders that legitimately
unstaked AND silently lose every validator that staked dynamically
after genesis. Mainnet operators would need a full wipe to recover
from any wipe of the validators table, which is unacceptable.
Recovery order is now:
1. data/snapshot/validators.jsonl when present (any block height) —
snapshots are the authoritative state-at-snapshot-time and
include every dynamic stake event up to that point.
2. data/genesis.json::validators[] but ONLY when lastBlock === 0
(cold-start dev / fresh chain bootstrap).
3. Refuse to boot with a remediation message otherwise — the
operator must restore a snapshot from a healthy peer or accept
a full reset.
Snapshot rows go straight through to UPSERT without re-validation —
the snapshot loader already enforces the wider schema and
re-validating with the genesis-strict schema would reject legitimate
nullable-column rows.
20/20 unit tests pass under `bun test`, including new coverage for:
- snapshot preference at advanced block heights
- refuse-to-boot at block > 0 without snapshot
- snapshot load / stream failures gracefully fall through
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/libs/blockchain/routines/ensureValidatorSeed.ts (1)
146-146: 💤 Low valueWrap callback to prevent accidental parameter leakage.
forEachpasses three arguments(element, index, array)to its callback. IfvalidateSeedEntryis later modified to accept a third parameter, it would silently receive the entire array.Suggested fix
- seed.forEach(validateSeedEntry) + seed.forEach((entry, i) => validateSeedEntry(entry, i))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/libs/blockchain/routines/ensureValidatorSeed.ts` at line 146, The forEach callback at the seed.forEach(validateSeedEntry) call passes the function directly, which means if validateSeedEntry is later modified to accept a third parameter, it would accidentally receive the entire array as that parameter due to forEach passing (element, index, array) to callbacks. Wrap validateSeedEntry in an arrow function like (entry) => validateSeedEntry(entry) to explicitly control which parameters are passed and prevent silent bugs from future modifications to the validateSeedEntry function signature.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/libs/blockchain/routines/ensureValidatorSeed.ts`:
- Line 146: The forEach callback at the seed.forEach(validateSeedEntry) call
passes the function directly, which means if validateSeedEntry is later modified
to accept a third parameter, it would accidentally receive the entire array as
that parameter due to forEach passing (element, index, array) to callbacks. Wrap
validateSeedEntry in an arrow function like (entry) => validateSeedEntry(entry)
to explicitly control which parameters are passed and prevent silent bugs from
future modifications to the validateSeedEntry function signature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d645c991-a1a8-47fa-8940-dd777ecdf2e2
📒 Files selected for processing (3)
src/libs/blockchain/routines/ensureValidatorSeed.tstest-reports/dem-771-devnet-bootstrap-design.mdtests/blockchain/ensureValidatorSeed.test.ts
✅ Files skipped from review due to trivial changes (1)
- test-reports/dem-771-devnet-bootstrap-design.md
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
2 participants
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.
Problem
findGenesisBlockshort-circuits pastgenerateGenesisBlockonce block 0 is in the DB, and the validator seed lives exclusively insidegenerateGenesisBlock. If thevalidatorstable is ever emptied while block 0 is present, the chain silently stalls —getShardreturns 0 peers, no quorum is reached,/healthstill reports OK, butconfirm()returns 401. The operator only finds out by manually callinggetValidatorsand seeing zero. We've hit this twice on devnet (DEM-728, DEM-770).There is also a pre-existing bug in the legacy "no snapshot" path of
chainGenesis.ts: genesis-baked validators are intentionally NOT seeded (comment in the code says so). Dev/empty-chain boot has always produced a dead consensus after the first run.Change
src/libs/blockchain/routines/ensureValidatorSeed.ts(new) — runs afterfindGenesisBlock. Conservative reconciler:validators.count > 0→ no-op (never overwrite dynamically-staked rows, never resurrect unstaked addresses)validators.count === 0+ genesis has validators[] → UPSERT (ON CONFLICT DO NOTHING) in a single transactionvalidators.count === 0+ genesis empty → throwConsensusInvariantErrorwith the exact remediation command, so the operator gets a fail-loud signal instead of silent quorum deathWired into
src/index.tsimmediately afterfindGenesisBlock/loadGenesisIdentities.Operator UX:
./run --docker --reset— wipe + reboot mainnet/testnet stack./run --docker --devnet --reset— same, targeting the devnet stackUnderneath: extends
scripts/wipe-and-reboot.shwith a--devnetflag (usesdemos_*_devnetvolume names and thedemos-devnetcompose project)../run --resetwithout--dockeris rejected with a friendly hint.Verification
Locally verified end-to-end against
docker-compose.devnet.yml:[BOOT] re-seeding 4 entrieslog; validators=4TRUNCATE validators+ restart[BOOT] re-seeding 4 entries; validators=4./run --docker --devnet --reset(wipe + boot)Unit tests: 11/11 pass under
bun test tests/blockchain/ensureValidatorSeed.test.ts. Projecttsc --noEmitreports zero new errors in the touched files.Safety notes
count > 0gate plusON CONFLICT DO NOTHINGis defence-in-depth: a dynamically-staked validator can never be overwritten by a genesis-baked row, even if the gate ever stops gating correctly.seedValidators(genesis-init path) and itsvalidators table emptypreflight are not modified — this PR adds a separate routine alongside, leaving the genesis-init path bit-for-bit identical.Design memo
test-reports/dem-771-devnet-bootstrap-design.mdcarries the full root-cause analysis, what we explicitly do not do, and the rollout plan.Test plan
./run --docker --devnet --reset --yeson a dev box, confirm wipe + clean rebootSummary by CodeRabbit
--resetmode to the launcher with--docker-gated behavior and automatic confirmation.--devnetflag support for stack-aware wipe-and-reboot operations.