Skip to content

feat(restore): snapshot bootstrap + genesis-baked validators + consensus shard enforcement#825

Merged
tcsenpai merged 3 commits into
stabilisationfrom
chore/restore
May 20, 2026
Merged

feat(restore): snapshot bootstrap + genesis-baked validators + consensus shard enforcement#825
tcsenpai merged 3 commits into
stabilisationfrom
chore/restore

Conversation

@tcsenpai

@tcsenpai tcsenpai commented May 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Three coordinated epics that make a freshly-cloned repo bootable into a fully-restored Demos node with both forks pre-applied and a secure consensus shard. End-to-end pristine-machine launch is now ./run --docker --clean.

  • State-Restore-from-Snapshot (epic 15): commit a verified snapshot of historical state (13 880 gcr_main + 1 382 gcr_storageprogram) and teach the genesis builder to restore it under a single transaction. Pre-applies osDenomination + gasFeeSeparation migrations at block 0 (deterministic, consensus-free). Plan + runbook in forking/restore/.
  • Multi-node interop tests (epic 16): two-node side-by-side via the new testing/multinode/ overlay. Validated peer discovery, genesis-hash match, sync convergence, cold-companion fast-sync, plus five rejection edge cases (tampered genesis, tampered snapshot, pre-populated PG, missing snapshot, corrupted manifest).
  • Consensus shard enforcement (epic 17): getShard() now intersects online peers with the active baked validator set. Empty-validator fallback gated by DEMOS_REQUIRE_VALIDATORS=true for production. Closes the gap revealed by epic 16 P10 where any peer could mine.

What's in the commit

Snapshot pipeline

  • data/snapshot/{gcr_main,gcr_storageprogram,identity_commitments}.jsonl + manifest.json (≈20 MB committed; deterministic regeneration via bun snapshot:transform).
  • scripts/state-snapshot/{transform,verify-snapshot,dry-run-restore}.ts + package.json scripts.
  • src/libs/blockchain/genesis/{loadSnapshot,restoreSnapshot,verifySnapshot,seedValidators,applyForksAtGenesis}.ts.
  • src/libs/blockchain/chainGenesis.ts branches on loadSnapshot() availability; legacy fallback preserved byte-identical.

Genesis config

  • data/genesis.json: balances: [], fork activationHeight: null (block-N hook stays dormant; fork_state is the durable record), real treasuryAddress baked, validators[] with node2, node3, operator, and a test pubkey.

Operator workflow

  • scripts/docker-run --clean wipes named volumes (demos_pgdata, demos_node_data) before up. Equivalent to bare-metal ./run -b true.
  • scripts/gen-test-identity.ts + scripts/derive-pubkey.ts for multi-node setups.

Multi-node harness

  • testing/multinode/docker-compose.companion.yml + up-companion.sh overlay.

Consensus shard fix

  • src/libs/consensus/v2/routines/getShard.ts intersects online peers with GCR.getGCRValidatorsAtBlock(lastBlock). Empty set → fallback (default loud warning + legacy behavior) or fatal throw if DEMOS_REQUIRE_VALIDATORS=true.

Docs

  • forking/restore/PLAN.md + forking/restore/RUNBOOK.md (1843 words, 10 sections including §2.4 on validator-set enforcement).
  • Cross-link in forking/RUNBOOK_FORK_ACTIVATION.md flagging the production env var.

Quality gates

  • 103 pass / 9 skip / 0 fail across testing/state-snapshot/, testing/forks/restore/, testing/consensus/.
  • 9 skips are PG-gated tests that pass on a live Postgres; gate uses it.skipIf(!pgAvailable) so CI reports them as proper skips, not silent passes.
  • bun snapshot:verify exits 0 against committed snapshot.
  • Snapshot JSONL sha256s are deterministic on re-run.
  • No new TypeScript errors introduced. (Pre-existing l2ps-messaging + verify-release-gate errors unchanged.)
  • osDenomination and gasFeeSeparation migration code untouched.
  • Legacy non-snapshot genesis path preserved byte-identical.

Test plan

Reviewer checklist:

  • bun test testing/state-snapshot/ testing/forks/restore/ testing/consensus/ reports 103 pass / 9 skip / 0 fail.
  • bun snapshot:verify exits 0.
  • Re-running bun snapshot:transform against the same source produces byte-identical output (sha256 of every file in data/snapshot/ stable).
  • Fresh-machine smoke: git clone … && cd node && git checkout chore/restore && ./run --docker --clean -d then verify:
    • gcr_main row count = 13 880
    • gcr_storageprogram row count = 1 382
    • validators row count = 4
    • fork_state has both forks applied=true, applied_at_block=0
    • SELECT SUM(balance) FROM gcr_main = 15024999999998868586000000000 (post-fork OS)
    • blocks advances beyond 0 once consensus tick fires
  • Set DEMOS_REQUIRE_VALIDATORS=true in a test container with the validators table emptied; node refuses to operate with the documented error message.

Risks

  • Block-0 hash diverges from the previous testnet's genesis. This is intentional (chain wipe) and documented in the RUNBOOK.
  • Production environments must set DEMOS_REQUIRE_VALIDATORS=true; without it getShard falls back to the legacy permissive behavior with a loud warning. The RUNBOOK calls this out.
  • Snapshot data is ≈20 MB committed to repo. Acceptable trade for reproducibility; alternative is post-clone fetch from a CDN/IPFS (deferred).

Out of scope

  • Three-validator Byzantine consensus rehearsal — deferred until mnemonics for the other two baked validator pubkeys (0x24c664… node3, 0xc8bc58… node2) are available. Tracked as Mycelium task #156 (closed/deferred).
  • IPFS/CDN distribution of the snapshot.
  • TOCTOU between loadSnapshot verify and restoreSnapshot stream — adversary needs filesystem write between two sequential async ops with operator already trusted; documented but not closed (would need verify-then-buffer pipeline refactor).

Summary by CodeRabbit

  • New Features

    • Added snapshot-based state restoration for genesis block initialization with validator seeding and deterministic fork pre-application
    • New CLI tools for snapshot operations: verification, transformation, and dry-run restoration testing
  • Documentation

    • Added comprehensive operational runbooks and technical plans for snapshot restoration workflows
  • Tests

    • Extensive test coverage for snapshot restoration, fork activation, and validator seeding
  • Chores

    • Updated Docker Compose configuration and scripts for multi-node testing scenarios

Review Change Stack

…sus shard enforcement

Three-epic ship that lets a pristine machine boot a fully-restored Demos
node with all forks (osDenomination, gasFeeSeparation) pre-applied and a
secure consensus shard, by running `./run --docker --clean`.

## Epic 15 — State-Restore-from-Snapshot

- New `data/snapshot/` (committed) with 13880 gcr_main + 1382
  gcr_storageprogram rows extracted from a peer PG dump (node3.demos.sh,
  block 2285755), serialised as JSONL with a sha256 + counts + sums
  manifest. Source dump (62 MB gz, sha256 c4568141...) lives in the
  gitignored `.snapshot-restore/` for operators who regenerate.
- `scripts/state-snapshot/transform.ts` — deterministic pg_dump → JSONL
  converter; resets nonce=0 + assignedTxs=[] per operator policy.
- `scripts/state-snapshot/verify-snapshot.ts` + companion logic in
  `src/libs/blockchain/genesis/verifySnapshot.ts` — single-pass sha256 +
  row-count + balance/size-sum integrity gate.
- `src/libs/blockchain/genesis/loadSnapshot.ts` — streaming JSONL
  iterator + manifest accessor. Verifies before yielding.
- `src/libs/blockchain/genesis/restoreSnapshot.ts` — preflightEmpty
  + bulk insert under caller transaction. Fails loud on non-empty DB.
- `src/libs/blockchain/genesis/applyForksAtGenesis.ts` — pre-applies
  osDenomination + gasFeeSeparation at block 0 inside the same
  transaction as the restore. Removes consensus dependency on fork
  activation; solo nodes boot fully post-fork.
- `src/libs/blockchain/genesis/seedValidators.ts` — seeds the
  `validators` table from `data/genesis.json validators[]`.
- `src/libs/blockchain/chainGenesis.ts` — branches on
  `loadSnapshot().available`: snapshot path runs restore → seedValidators
  → applyForksAtGenesis in one transaction; legacy
  `genesisData.balances` path preserved for dev/devnet.
- `data/genesis.json` — `balances: []`, `forks.{osDenomination,
  gasFeeSeparation}.activationHeight: null` (block-N hook stays dormant;
  fork_state is the durable record), real treasuryAddress baked.
  Initial `validators[]` populated with node2, node3, the operator's
  pubkey, and a test pubkey.
- `scripts/docker-run --clean` — new flag wipes named volumes
  (`demos_pgdata`, `demos_node_data`) before `up`. Equivalent to the
  bare-metal `./run -b true`.
- `forking/restore/{PLAN,RUNBOOK}.md` — full epic plan + operator
  runbook with pinned manifest values.
- Cross-link in `forking/RUNBOOK_FORK_ACTIVATION.md`.
- 95 pass / 9 skip / 0 fail across snapshot + restore suites.

## Epic 16 — Multi-node interop + edge cases

- `scripts/gen-test-identity.ts` + `scripts/derive-pubkey.ts` —
  ed25519 mnemonic generator + verifier; mirrors the runtime
  `Identity.mnemonicToSeed` (sha3_512 of raw mnemonic) so the derived
  pubkey matches what the node will compute.
- `testing/multinode/docker-compose.companion.yml` + `up-companion.sh`
  — side-car overlay attaching a second node to the same docker
  network + shared postgres (separate db `demos_companion`), with
  cross-seeded peerlists.
- Exercised: peer discovery, genesis-hash match, block-by-block
  sync convergence, cold-companion fast-sync from running host,
  genesis-hash divergence rejection, snapshot tamper rejection,
  pre-populated PG rejection, missing-snapshot fallback to legacy
  path, manifest drift rejection, validator-quorum behavior.
- Revealed an underlying security gap (epic 17 below).

## Epic 17 — Consensus shard enforcement

- `src/libs/consensus/v2/routines/getShard.ts` — intersects the
  online-peer set with `GCR.getGCRValidatorsAtBlock(lastBlock)`. Only
  active baked validators participate in consensus. Empty-set
  fallback gated by `DEMOS_REQUIRE_VALIDATORS=true` (production)
  vs unset (dev/devnet warns and falls back to legacy behavior).
  Defensive null-filter on `Validators.address`.
- `testing/consensus/getShard.test.ts` — 8 unit tests covering full
  intersection, mixed validator/non-validator peers, both empty-set
  modes, exited-validator path, valid_at gating, determinism, NULL
  addresses.
- Empirically validated against the multi-node setup: non-validator
  companion has shard size 0, refuses to mine alone; host (validator)
  mines normally; production strict mode throws cleanly.
- `forking/restore/RUNBOOK.md` §2.4 documents the new env var and
  behavior table; `forking/RUNBOOK_FORK_ACTIVATION.md` cross-link
  notes production requirement.

## Quality gates

- 103 pass / 9 skip / 0 fail across all new suites
  (`testing/state-snapshot/`, `testing/forks/restore/`,
  `testing/consensus/`)
- `bun snapshot:verify` exits 0 against the committed snapshot
- snapshot JSONL sha256s deterministic on re-run
- No new TypeScript errors introduced (pre-existing l2ps-messaging +
  verify-release-gate errors unchanged)
- Migration code paths (`osDenomination`, `gasFeeSeparation`) untouched
- Legacy non-snapshot genesis path preserved byte-identical
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@tcsenpai has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 29 minutes and 1 second before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2e63a25d-9a61-445f-8303-9c9730cf51a9

📥 Commits

Reviewing files that changed from the base of the PR and between 5c1cfb3 and ab29bff.

📒 Files selected for processing (13)
  • forking/restore/PLAN.md
  • scripts/derive-pubkey.ts
  • scripts/gen-test-identity.ts
  • scripts/state-snapshot/transform.ts
  • src/libs/blockchain/chainGenesis.ts
  • src/libs/blockchain/genesis/restoreSnapshot.ts
  • src/libs/blockchain/genesis/seedValidators.ts
  • src/libs/blockchain/genesis/verifySnapshot.ts
  • src/libs/consensus/v2/routines/getShard.ts
  • testing/consensus/getShard.test.ts
  • testing/forks/restore/genesisRestore.test.ts
  • testing/forks/restore/restoreE2E_with_forks.test.ts
  • testing/multinode/up-companion.sh

Walkthrough

This PR implements a complete hard-fork chain bootstrap system that restores pre-fork state from a committed PostgreSQL snapshot, re-applies fork logic deterministically at genesis block 0, and seeds genesis-time validators. The snapshot includes three tables (gcr_main, gcr_storageprogram, identity_commitments), is transformed into deterministic JSONL with SHA-256 integrity manifest, verified on restore, and combined with validator seeding and fork pre-application in a single genesis transaction.

Changes

Snapshot Restore + Genesis Fork Pre-Apply

Layer / File(s) Summary
Snapshot Manifest & Verification
src/libs/blockchain/genesis/verifySnapshot.ts, scripts/state-snapshot/verify-snapshot.ts
Manifest schema with schemaVersion, per-file SHA-256/row counts/sums, and verifySnapshot routine that validates file existence, hash integrity, and row/sum invariants before allowing restore.
Snapshot Transformation (SQL→JSONL)
scripts/state-snapshot/transform.ts, testing/state-snapshot/transform.test.ts
Parses gzipped pg_dump column-inserts, tokenizes VALUES clauses with SQL escape handling, applies per-column coercion and deterministic ordering, enforces nonce=0/assignedTxs=[], drops test identity rows, outputs JSONL with SHA-256 manifest.
Snapshot Loading & Type Conversion
src/libs/blockchain/genesis/loadSnapshot.ts, testing/state-snapshot/verify-snapshot.test.ts
Async-iterable JSONL streaming with per-row field validation, type conversion (string→bigint, snake_case→camelCase), line-numbered error tracking, and integrity verification gate; returns unavailable sentinel for legacy fallback.
Snapshot Restoration
src/libs/blockchain/genesis/restoreSnapshot.ts, testing/forks/restore/genesisRestore.test.ts, testing/forks/restore/restoreE2E.test.ts
Bulk-inserts snapshot rows in batches with progress logging; enforces preflight empty-DB check and manifest row-count validation; runs within caller-owned transaction.
Genesis Validator Seeding
src/libs/blockchain/genesis/seedValidators.ts, testing/forks/restore/seedValidators.test.ts, testing/forks/restore/validatorE2E.test.ts
Validates seed shapes (address, status, url, bigint staked_amount); preflight empty-DB check; batch-inserts with NULL unstake fields; logs totals.
Fork Pre-Application at Genesis
src/libs/blockchain/genesis/applyForksAtGenesis.ts, testing/forks/restore/applyForksAtGenesis.test.ts, testing/forks/restore/forkAfterRestore.test.ts, testing/forks/restore/restoreE2E_with_forks.test.ts
Runs osDenomination and gasFeeSeparation migrations at block 0 within caller transaction; tracks applied flags; computes elapsed time; returns report.
Genesis Flow Integration
src/libs/blockchain/chainGenesis.ts
Conditionally branches on snapshot availability: when available, runs restore + optional validator seeding + fork pre-apply in single transaction; when unavailable, falls back to legacy users derivation from genesisData.balances.
Consensus Shard Filtering by Active Validators
src/libs/consensus/v2/routines/getShard.ts, testing/consensus/getShard.test.ts
Filters shard peer selection to active validators from DB; enforces availability via DEMOS_REQUIRE_VALIDATORS flag; falls back to all online peers with warning when validators absent.
Configuration & Data Updates
.gitignore, data/genesis.json, package.json, data/snapshot/manifest.json
Ignores .snapshot-restore/ and .test-identity/ while committing data/snapshot/; updates genesis.json with fork configs (osDenomination, gasFeeSeparation, treasury), empty balances, and validators list; adds npm scripts for snapshot workflow.
Supporting Utility Scripts
scripts/derive-pubkey.ts, scripts/gen-test-identity.ts, scripts/docker-run, scripts/state-snapshot/dry-run-restore.ts
Derives ed25519 public key from mnemonic; generates deterministic test identities; adds --clean flag for volume wiping; adds dry-run restore for operator validation.
Multinode Companion Setup
testing/multinode/docker-compose.companion.yml, testing/multinode/up-companion.sh
Docker overlay for companion node with separate database; shell script with test-identity validation and bidirectional peerlist pre-seeding.
Operational Documentation
forking/RUNBOOK_FORK_ACTIVATION.md, forking/restore/PLAN.md, forking/restore/RUNBOOK.md
Comprehensive runbooks covering snapshot restoration, genesis fork pre-apply, prerequisites, pre-flight checks, boot sequence, post-genesis verification, recovery paths, and safety notes including validator-set enforcement.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • kynesyslabs/node#817: This PR applies gasFeeSeparationMigration with treasury address at genesis, which was added in #817.
  • kynesyslabs/node#812: This PR runs osDenominationMigration at genesis block 0, which was implemented in #812.
  • kynesyslabs/node#517: Both PRs modify src/libs/consensus/v2/routines/getShard.ts for validator-based peer filtering.

Suggested labels

Review effort 5/5

Suggested reviewers

  • cwilvx

Poem

🐰 A snapshot frozen in time,
From blocks of old to genesis prime,
With validators seeded, forks applied,
The chain boots fresh with state in stride!
Verify, restore, and fork on through—
Hard-fork bootstrap, your dreams come true! 🎉

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/restore

@greptile-apps

greptile-apps Bot commented May 20, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces snapshot-based genesis bootstrapping (epic 15), a multi-node test harness (epic 16), and consensus shard validator enforcement (epic 17). Together, a fresh clone can be booted into a fully-restored Demos node with historical state, genesis-baked validators, and deterministic fork pre-application at block 0.

  • Snapshot pipeline: ~20 MB committed JSONL snapshot streamed via typed async-iterable loaders with single-pass sha256+row+sum integrity checks into gcr_main, gcr_storageprogram, and identity_commitments inside one atomic TypeORM transaction.
  • Validator enforcement: getShard() now intersects online peers with the active baked validator set (with a per-block cache), gated by DEMOS_REQUIRE_VALIDATORS=true for production fallback control.
  • Genesis fork pre-application: both osDenomination and gasFeeSeparation migrations run deterministically at block 0 inside the snapshot transaction, ahead of the consensus tick.

Confidence Score: 5/5

Safe to merge; the snapshot pipeline, genesis transaction, and shard enforcement are well-structured with clear rollback semantics and thorough test coverage.

All three epics follow consistent patterns: single-pass integrity checks before any DB write, batched inserts inside a single caller-owned transaction, and explicit preflight guards that produce actionable operator messages. The per-block validator cache in getShard correctly invalidates on block advance. No defects were found that affect current runtime behavior on the committed genesis.json config.

applyForksAtGenesis.ts and seedValidators.ts have minor validation gaps worth hardening before future fork additions.

Important Files Changed

Filename Overview
src/libs/blockchain/genesis/applyForksAtGenesis.ts New file orchestrating genesis-time fork pre-application; applies forks unconditionally when the forks block is present regardless of activationHeight, and passes treasuryAddress without runtime validation.
src/libs/blockchain/genesis/restoreSnapshot.ts New file; bulk-inserts snapshot rows via batched TypeORM inserts inside a caller-owned transaction with robust preflight emptiness check and clear partial-genesis error messages.
src/libs/blockchain/genesis/loadSnapshot.ts New file; cleanly separates directory resolution, integrity gating, and typed streaming. Single-pass sha256/row/sum checks prevent TOCTOU races.
src/libs/blockchain/genesis/verifySnapshot.ts New file; uses single-pass streaming for sha256 + row count + field sums, resolving the previously noted double-read/TOCTOU concern. Correct error ordering (sha256 first, then parse errors).
src/libs/blockchain/genesis/seedValidators.ts New file; validates address format, staked_amount positivity, and connection_url shape, but accepts any non-empty string as status without checking against known enum values.
src/libs/consensus/v2/routines/getShard.ts Adds per-block-keyed validator cache to address previous hot-path DB query concern; NULL-address filtering and DEMOS_REQUIRE_VALIDATORS fallback are well-guarded.
src/libs/blockchain/chainGenesis.ts Branches on snapshot availability; snapshot/seed/fork transaction commits before insertBlock (acknowledged in comments), creating a partial-genesis recovery scenario that requires manual wipe.
data/genesis.json Replaces legacy balance array with empty balances and bakes four validators with status="2"; activationHeight is null for both forks, disabling the block-N hook as intended.

Sequence Diagram

sequenceDiagram
    participant Op as Operator
    participant GG as generateGenesisBlock
    participant LS as loadSnapshot
    participant VS as verifySnapshot
    participant DB as PostgreSQL (transaction)
    participant RS as restoreSnapshot
    participant SV as seedValidators
    participant AF as applyForksAtGenesis
    participant IB as insertBlock(0)

    Op->>GG: boot (genesis.json present)
    GG->>LS: loadSnapshot()
    LS->>VS: verifySnapshot(snapshotDir)
    VS-->>LS: sha256 + row + sum checks pass
    LS-->>GG: SnapshotLoaderAvailable

    GG->>DB: "dataSource.transaction(em => ...)"
    DB->>RS: restoreSnapshot(em, loader)
    RS->>DB: preflightEmpty — COUNT all tables
    RS->>DB: bulkInsert gcr_main (500/batch)
    RS->>DB: bulkInsert gcr_storageprogram (100/batch)
    RS->>DB: bulkInsert identity_commitments
    RS-->>DB: RestoreReport

    DB->>SV: seedValidators(em, genesis.validators)
    SV->>DB: COUNT validators (must be 0)
    SV->>DB: INSERT validators[]
    SV-->>DB: inserted N

    DB->>AF: applyForksAtGenesis(em, genesis.forks)
    AF->>DB: runOsDenominationMigration(em, 0)
    AF->>DB: runGasFeeSeparationMigration(em, 0, treasuryAddr)
    AF-->>DB: ApplyForksReport

    DB-->>GG: transaction committed

    GG->>IB: insertBlock(block0) — own internal transaction
    IB-->>GG: block 0 written

    Note over DB,IB: Partial-genesis window: if insertBlock fails after snapshot tx commits, recovery requires manual wipe
Loading

Reviews (3): Last reviewed commit: "perf(consensus): cache validator-set que..." | Re-trigger Greptile

Comment thread src/libs/consensus/v2/routines/getShard.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

🧹 Nitpick comments (2)
testing/forks/restore/restoreE2E.test.ts (1)

362-364: ⚡ Quick win

Keep simulateRestore behavior aligned with production for identities.

The identity loop only increments a counter and never inserts into identity_commitments. That makes the harness less representative once fixtures include identity rows.

🤖 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 `@testing/forks/restore/restoreE2E.test.ts` around lines 362 - 364, The loop
over loader.streamIdentityCommitments() in simulateRestore only increments a
local identity counter and never persists rows into identity_commitments, making
tests diverge from production; inside that for-await loop (where
loader.streamIdentityCommitments() is iterated and identity++ occurs) call the
appropriate persistence method to insert each commitment into the test DB (e.g.,
use loader.insertIdentityCommitment(commitment) or the project’s equivalent such
as createIdentityCommitment/restoreIdentityCommitment), passing the streamed row
data (or derived commitment) so the identity_commitments table is actually
populated as in production. Ensure you use the exact loader method your codebase
exposes and keep the identity counter logic intact.
testing/state-snapshot/transform.test.ts (1)

233-308: ⚡ Quick win

Add a regression test for unterminated quoted value tokens.

Given parser hardening in parseValuesPayload, add a case like "'abc" to assert it throws. This protects against future regressions in malformed SQL handling.

🤖 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 `@testing/state-snapshot/transform.test.ts` around lines 233 - 308, Add a
regression test to ensure parseValuesPayload throws on unterminated quoted
tokens: create a new it-block that calls parseValuesPayload("'abc") (or similar
unterminated single-quote string) and expects it to throw (e.g., toThrow /
toThrowError matching "unterminated" or generic throw); reference the existing
parseValuesPayload tests in transform.test.ts and match the style used for other
error tests like the "throws on unparseable bareword" case.
🤖 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 `@data/genesis.json`:
- Around line 42-53: Replace null connection_url values in the genesis
validators array so the validator seeding logic receives strings: find entries
where validators[].connection_url is null (e.g., the two records shown) and
change them to a valid URL string or at minimum an empty string ("") consistent
with the validator seeding contract expectations; ensure the fields "address"
and "status" remain unchanged and validate the modified JSON parses successfully
before committing.

In `@forking/restore/PLAN.md`:
- Around line 3-5: Update the PLAN.md wording to reflect that backward
compatibility is preserved: change the "Target" or "Scope" section text that
currently reads “No backward compatibility required” to indicate that the PR
stack preserves the legacy genesis path and therefore retains backward
compatibility for the legacy bootstrap flow; update any other mentions of
compatibility in the same document (e.g., the "Scope" paragraph and surrounding
lines) so operators reading PLAN.md will understand both the fresh-chain PG
snapshot bootstrap and the preserved legacy genesis behavior.

In `@scripts/derive-pubkey.ts`:
- Around line 17-24: Check that the mnemonic read from mnemonicFile is non-empty
after trimming and throw or exit with a clear error if it's empty (validate the
variable mnemonic), and after calling await ucrypto.getIdentity("ed25519")
verify the returned identity is truthy before accessing identity.publicKey; if
the lookup fails, log or throw a descriptive error and stop execution instead of
dereferencing a possibly undefined identity. Ensure these validations are placed
around the existing mnemonic, ucrypto.generateAllIdentities(seedBytes), and
ucrypto.getIdentity("ed25519") calls.

In `@scripts/gen-test-identity.ts`:
- Around line 20-33: The current --name parsing and preflight check are unsafe:
validate the token at args[nameIdx+1] (used by NAME) to ensure it exists, does
not start with "-" and contains no path separators (use path.basename or reject
values with "/" or "\\") and sanitize/normalize it before building MNEMONIC_FILE
and PUBKEY_FILE; additionally, when fs.existsSync(MNEMONIC_FILE) is true do not
assume PUBKEY_FILE exists—check fs.existsSync(PUBKEY_FILE) before calling
fs.readFileSync and handle the partial-state case (either treat as needing
--force with a clear message or regenerate) so you never read a missing pubkey
file and never accept flag-like or path values as the name.

In `@scripts/state-snapshot/transform.ts`:
- Around line 362-365: The "bigint-string" branch currently uses Math.trunc for
value.kind === "number", which silently drops fractional parts; change this to
reject non-integer numbers instead: when handling the "bigint-string" case (the
switch branch labeled "bigint-string" that inspects value.kind), keep returning
value.value.toString() for value.kind === "bigint", but for value.kind ===
"number" first assert Number.isInteger(value.value) and if true return
value.value.toString(), otherwise throw a descriptive Error (e.g., cannot coerce
non-integer number to bigint-string) to avoid silent truncation.
- Around line 148-168: In parseValuesPayload, the single-quoted branch currently
pushes a string token even if no closing quote is found; change it to detect an
unterminated string (i.e., the loop finishes without encountering the closing
"'") and reject by throwing an error or returning a parse failure instead of
pushing to tokens; specifically update the logic around the temporary variable
out and the while loop over payload to check after the loop whether a closing
quote was seen and, if not, abort (throw or return) to prevent adding a partial
token to tokens.

In `@src/libs/blockchain/chainGenesis.ts`:
- Around line 142-160: The restore transaction currently commits before
insertBlock(...) runs, risking a partial-genesis state if insertBlock fails; fix
by performing the block-0 persistence inside the same dataSource.transaction
callback (or use the same transactional EntityManager passed to
restoreSnapshot/seedValidators/applyForksAtGenesis) so insertBlock uses the
transactional em and is inserted before the transaction completes; update any
other similar calls around the referenced code (lines near 211-212) to ensure
all genesis persistence (restoreSnapshot, seedValidators, applyForksAtGenesis,
and insertBlock) occur atomically within the single transaction.

In `@src/libs/blockchain/genesis/seedValidators.ts`:
- Around line 37-75: Add a guard in validateSeed to fail fast when
seed.connection_url is missing or malformed: verify seed.connection_url is a
non-empty string and then attempt to parse it with the URL constructor (wrap in
try/catch) and throw an Error with the same [GENESIS][VALIDATORS]
seed[${index}].connection_url message pattern on failure; place this check
alongside the other field validations in validateSeed so invalid connection_url
values are rejected before persistence.

In `@src/libs/blockchain/genesis/verifySnapshot.ts`:
- Around line 63-77: In parseManifest (in
src/libs/blockchain/genesis/verifySnapshot.ts) you must explicitly validate
manifest sum fields before calling BigInt: check that
parsed.manifest.balance_sum and parsed.manifest.size_bytes_sum exist and are
strings (or numbers if expected) and throw descriptive errors (e.g.,
`manifest.balance_sum missing` / `manifest.size_bytes_sum missing` or wrong
type) rather than relying on BigInt to fail; update the parsing logic around the
current BigInt conversions (lines referenced as 262–263) to validate types first
and only then convert to BigInt so verification failures are deterministic and
operator-friendly.

In `@testing/forks/restore/genesisRestore.test.ts`:
- Around line 245-248: The test cleanup truncates gcr_main, gcr_storageprogram,
identity_commitments but restoreSnapshot's preflight also checks blocks and
validators, causing order-dependent failures; update the teardown in
testing/forks/restore/genesisRestore.test.ts (the ds?.isInitialized branch that
runs ds.query) to include the blocks and validators tables in the TRUNCATE
statement so the test DB is fully aligned with restore preflight checks.

In `@testing/forks/restore/restoreE2E_with_forks.test.ts`:
- Around line 322-330: The block has contradictory notes about null
activationHeight vs test behavior; update the comment to a single canonical
statement: explain that applyForksAtGenesis checks activationHeight !== null &&
activationHeight !== undefined so null in genesis.json means "skip pre-apply",
and then state this test intentionally uses a non-null activationHeight (e.g. 1)
to verify migrations run (pre-P7 shape); remove the conflicting sentences and
keep only the clear mapping between activationHeight, applyForksAtGenesis, and
the purpose of this test.

In `@testing/multinode/up-companion.sh`:
- Line 26: Add an explicit file-existence check before reading
.test-identity/companion.pubkey: verify the file exists and is readable, and if
not print a clear operator message and exit non-zero; then read it into
COMPANION_PUBKEY only after the check. Update the up-companion.sh logic around
COMPANION_PUBKEY="$(cat .test-identity/companion.pubkey)" to perform the check
(e.g., test -f/. -r) and a descriptive error like "companion pubkey not found at
.test-identity/companion.pubkey; please run setup" before exiting.

---

Nitpick comments:
In `@testing/forks/restore/restoreE2E.test.ts`:
- Around line 362-364: The loop over loader.streamIdentityCommitments() in
simulateRestore only increments a local identity counter and never persists rows
into identity_commitments, making tests diverge from production; inside that
for-await loop (where loader.streamIdentityCommitments() is iterated and
identity++ occurs) call the appropriate persistence method to insert each
commitment into the test DB (e.g., use
loader.insertIdentityCommitment(commitment) or the project’s equivalent such as
createIdentityCommitment/restoreIdentityCommitment), passing the streamed row
data (or derived commitment) so the identity_commitments table is actually
populated as in production. Ensure you use the exact loader method your codebase
exposes and keep the identity counter logic intact.

In `@testing/state-snapshot/transform.test.ts`:
- Around line 233-308: Add a regression test to ensure parseValuesPayload throws
on unterminated quoted tokens: create a new it-block that calls
parseValuesPayload("'abc") (or similar unterminated single-quote string) and
expects it to throw (e.g., toThrow / toThrowError matching "unterminated" or
generic throw); reference the existing parseValuesPayload tests in
transform.test.ts and match the style used for other error tests like the
"throws on unparseable bareword" case.
🪄 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: 56d4ced2-a2c6-404a-a9b1-2e14374541f6

📥 Commits

Reviewing files that changed from the base of the PR and between 0af3d2e and 5c1cfb3.

📒 Files selected for processing (36)
  • .gitignore
  • data/genesis.json
  • data/snapshot/gcr_main.jsonl
  • data/snapshot/gcr_storageprogram.jsonl
  • data/snapshot/identity_commitments.jsonl
  • data/snapshot/manifest.json
  • forking/RUNBOOK_FORK_ACTIVATION.md
  • forking/restore/PLAN.md
  • forking/restore/RUNBOOK.md
  • package.json
  • scripts/derive-pubkey.ts
  • scripts/docker-run
  • scripts/gen-test-identity.ts
  • scripts/state-snapshot/dry-run-restore.ts
  • scripts/state-snapshot/transform.ts
  • scripts/state-snapshot/verify-snapshot.ts
  • src/libs/blockchain/chainGenesis.ts
  • src/libs/blockchain/genesis/applyForksAtGenesis.ts
  • src/libs/blockchain/genesis/loadSnapshot.ts
  • src/libs/blockchain/genesis/restoreSnapshot.ts
  • src/libs/blockchain/genesis/seedValidators.ts
  • src/libs/blockchain/genesis/verifySnapshot.ts
  • src/libs/consensus/v2/routines/getShard.ts
  • testing/consensus/getShard.test.ts
  • testing/forks/restore/applyForksAtGenesis.test.ts
  • testing/forks/restore/forkAfterRestore.test.ts
  • testing/forks/restore/genesisRestore.test.ts
  • testing/forks/restore/restoreE2E.test.ts
  • testing/forks/restore/restoreE2E_with_forks.test.ts
  • testing/forks/restore/seedValidators.test.ts
  • testing/forks/restore/validatorE2E.test.ts
  • testing/multinode/docker-compose.companion.yml
  • testing/multinode/up-companion.sh
  • testing/state-snapshot/determinism.test.ts
  • testing/state-snapshot/transform.test.ts
  • testing/state-snapshot/verify-snapshot.test.ts

Comment thread data/genesis.json
Comment thread forking/restore/PLAN.md Outdated
Comment thread scripts/derive-pubkey.ts
Comment thread scripts/gen-test-identity.ts
Comment thread scripts/state-snapshot/transform.ts
Comment thread src/libs/blockchain/genesis/seedValidators.ts
Comment thread src/libs/blockchain/genesis/verifySnapshot.ts
Comment thread testing/forks/restore/genesisRestore.test.ts Outdated
Comment thread testing/forks/restore/restoreE2E_with_forks.test.ts Outdated
Comment thread testing/multinode/up-companion.sh
Greptile findings (P2):
- verifySnapshot.ts: collapse sha256 + row-count into a single streaming
  pass; eliminate second readFile and misleading "reuse leftover" comment.
- chainGenesis.ts + restoreSnapshot.ts: keep block-0 insertBlock outside
  the restore transaction (insertBlock owns its own savepoint-per-tx
  flow + Merkle update + governance hooks; threading em through is
  invasive), but improve preflightEmpty error to distinguish
  "partial-genesis" (gcr_main populated, blocks empty) from
  "populated DB" so operators see exactly which recovery is needed.
- transform.ts processLines monolith: SKIPPED — the function intentionally
  consolidates the per-table streaming/coercion/sum scaffolding into one
  loop; splitting would re-duplicate it. Per-table behavior already
  isolated via *_KINDS + *_COLUMNS maps.

CodeRabbit findings:
- parseManifest: validate balance_sum (string) and size_bytes_sum
  (safe-integer) shape before downstream BigInt coercion.
- derive-pubkey.ts: fail loudly on empty mnemonic file + null identity.
- gen-test-identity.ts: --name requires a value and matches [a-zA-Z0-9._-]+.
- transform.ts parseValuesPayload: track `closed` flag on string-literal
  scanner; throw on unterminated quote.
- transform.ts coerceField (bigint-string from number): require
  Number.isSafeInteger; throw on non-integer instead of silent truncate.
- seedValidators.ts validateSeed: connection_url must be null or
  non-empty string; reject empty / non-string.
- genesisRestore.test.ts: truncate blocks + validators in addition to
  the three GCR tables (preflight checks all five).
- restoreE2E_with_forks.test.ts: collapse contradictory comment block
  into clear per-test descriptions.
- up-companion.sh: explicit existence check for .test-identity/companion.pubkey
  before reading.
- PLAN.md: clarify Target line so back-compat with legacy genesis path
  is explicit (matches implementation).
- data/genesis.json null connection_url: SKIPPED — false positive, the
  entity types `string | null` and seedValidators accepts null
  deliberately.

Test suite unchanged: 103 pass / 9 skip / 0 fail.
bun snapshot:verify still exits 0. JSONL sha256s stable.
@tcsenpai

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Greptile iter 1 P2: `GCR.getGCRValidatorsAtBlock(lastBlock)` ran on every
consensus tick. The validator set only changes via stake/unstake/exit
txs at block boundaries — caching keyed on `lastBlockNumber` collapses
N round-trips per block into one.

- Module-scoped `cachedBlock` / `cachedValidators` invalidated when
  `lastBlockNumber` advances.
- `__resetValidatorCache()` exported for tests so cases don't leak
  cached state.
- New test case (i) verifies single DB call across two ticks at same
  block, and re-query on block bump. 9/9 getShard tests pass.

Full suite still 104 pass / 9 skip / 0 fail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant