fix(genesis): overlay genesisData.balances on snapshot rows#858
Conversation
…or top-ups actually land When data/snapshot/ is present, restoreSnapshot owns every gcr_main row and the legacy genesisData.balances array was silently dropped (per chainGenesis.ts:131-138 historical comment). An operator who edited data/genesis.json to add an address would see block-0 hash change (extra.genesisData carries the raw JSON) but the address balance would remain at whatever the snapshot row said — or 0 once ensureGCRForUser later materialised the row. Concretely: address 0xd17624...d98a was listed in data/genesis.json with a non-zero balance, the chain accepted the resulting block-0 hash, but gcr_main reported balance "0". Three independent identities, all funded in genesis on paper, all 0 on chain. The disk diverged from what the hash committed to. Fix: new mergeGenesisBalances() runs after restoreSnapshot inside the same transaction. genesisData.balances wins on conflict — operator intent is "the snapshot is yesterday's state, the genesis.balances overlay is today's top-up". Only the balance column is overwritten; identities, points, referralInfo, flags from the snapshot row are preserved. Missing pubkeys get a fresh row with the same defaults HandleGCR.createAccount would assign, so the row is indistinguishable from one materialised organically. Consensus impact: none. Block-0 hash is computed from serializeBlockContent whose extra.genesisData already includes the balances array. Every honest node parsing the same genesis.json now derives the same gcr_main state, which is what the hash already committed to. The fix closes the gap between hash and on-disk state, it does not change the hash. Validation: balances are parsed to bigint OS up front and rejected if negative, fractional, NaN, or wrong-shaped — silent BigInt(0) on a typo would re-introduce the same class of "operator wrote it but the chain ignored it" bug. Tests: 18 unit cases covering no-op paths, validation rejects, coercion (string|number|bigint), dedup last-wins, UPDATE preserves snapshot identity columns, INSERT writes defaults, mixed batch counts.
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? |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Disabled knowledge base sources:
WalkthroughThis PR adds a new genesis balance overlay feature that applies committed genesis balance data to snapshot-restored accounts. It introduces a new ChangesGenesis balance overlay during snapshot restore
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 long-standing silent bug where
Confidence Score: 4/5The change is safe to merge — it closes a real on-disk/hash divergence at genesis and runs inside an existing transaction, so partial failures roll back cleanly. The logic is well-scoped, the validation path fails loudly on malformed input, and the row-level merge correctly preserves snapshot-owned columns. The row-by-row SELECT+SAVE loop introduces no correctness problems but does not scale for large balances arrays. The result.total JSDoc is mildly misleading about deduplication. Both are quality observations rather than behavioral defects. src/libs/blockchain/genesis/mergeGenesisBalances.ts — specifically the N+1 loop at lines 207–226 if the balances list ever grows beyond a handful of founder addresses. Important Files Changed
Sequence DiagramsequenceDiagram
participant G as generateGenesisBlock
participant TX as dataSource.transaction
participant RS as restoreSnapshot
participant SV as seedValidators
participant MGB as mergeGenesisBalances
participant AF as applyForksAtGenesis
participant DB as gcr_main (DB)
G->>TX: begin transaction
TX->>RS: restoreSnapshot(em, snapshot)
RS->>DB: bulk-insert snapshot rows
TX->>SV: seedValidators(em, validators)
SV->>DB: insert validator rows
TX->>MGB: mergeGenesisBalances(em, genesisData.balances)
loop for each parsed pubkey
MGB->>DB: findOne(pubkey)
alt row exists (snapshot row)
MGB->>DB: "save(existing, balance=new)"
else row missing
MGB->>DB: save(fresh row with defaults)
end
end
TX->>AF: applyForksAtGenesis(em, forks)
AF->>DB: apply fork migrations
TX-->>G: commit (or rollback on any throw)
Reviews (1): Last reviewed commit: "fix(genesis): overlay genesisData.balanc..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (const [pubkey, balance] of parsed) { | ||
| const existing = await repo.findOne({ where: { pubkey } }) | ||
| if (existing) { | ||
| // Only the balance column is overwritten; everything else | ||
| // (identities, points, referralInfo, flagged, …) belongs to | ||
| // the snapshot row and we have no business stomping it. | ||
| existing.balance = balance | ||
| existing.updatedAt = new Date() | ||
| await repo.save(existing) | ||
| result.updated++ | ||
| } else { | ||
| const fresh = repo.create({ | ||
| pubkey, | ||
| balance, | ||
| ...defaultEmptyAccountFields(pubkey), | ||
| }) | ||
| await repo.save(fresh) | ||
| result.inserted++ | ||
| } | ||
| } |
There was a problem hiding this comment.
The main loop issues a repo.findOne plus a repo.save for every distinct pubkey, sequentially. For a genesisData.balances that carries tens or hundreds of entries this results in that many individual SELECT + UPDATE/INSERT round-trips inside the transaction. A bulk approach — one SELECT ... WHERE pubkey IN (...) to split entries into existing/missing sets, then an INSERT ... ON CONFLICT DO UPDATE or a batched save([...]) for each set — would reduce this to two queries regardless of input size. At genesis time this is a one-shot operation so correctness is unaffected, but the current implementation does not scale if the balances list grows.
| /** Total entries processed from genesisData.balances */ | ||
| total: number |
There was a problem hiding this comment.
total field is assigned parsed.size (unique pubkeys after deduplication) but its JSDoc says "Total entries processed from genesisData.balances" and the preceding log line reports the raw balances.length. If a balances array has duplicate pubkeys, total will be less than the number of array items, which is surprising for a caller reading both the log and the result. Consider updating the JSDoc to clarify that it counts distinct pubkeys.
| /** Total entries processed from genesisData.balances */ | |
| total: number | |
| /** Distinct pubkeys processed from genesisData.balances (duplicates are last-wins deduped) */ | |
| total: number |
1 participant
Summary
When
data/snapshot/is present,restoreSnapshotowns everygcr_mainrow and the legacygenesisData.balancesarray was silently dropped (perchainGenesis.ts:131-138historical comment). An operator who editeddata/genesis.jsonto add an address would see block-0 hash change (extra.genesisDatacarries the raw JSON) but the address balance would stay at whatever the snapshot row said — or0onceensureGCRForUserlater materialised the row.Repro
0xd17624…d98alisted indata/genesis.json.balanceswith non-zero balancegetAddressInforeportsbalance: "0"Fix
mergeGenesisBalances()runs afterrestoreSnapshotinside the same transaction.genesisData.balanceswins on conflict — operator intent is "snapshot is yesterday's state, genesis.balances is today's top-up". Only thebalancecolumn is overwritten;identities,points,referralInfo, flags are preserved. Missing pubkeys get a fresh row with the same defaultsHandleGCR.createAccountwould assign.Consensus impact
None. Block-0 hash is computed from
serializeBlockContentwhoseextra.genesisDataalready includes thebalancesarray. Every honest node parsing the samegenesis.jsonnow derives the samegcr_mainstate, which is what the hash already committed to. The fix closes the gap between hash and on-disk state; it does not change the hash.Validation
Balances parsed to
bigintOS up front; rejects negative / fractional / NaN / wrong-shaped entries. SilentBigInt(0)on a typo would re-introduce the same class of "operator wrote it but chain ignored it" bug.Test plan
testing/genesis/mergeGenesisBalances.test.ts:total/updated/inserted)data_*PG volume, boot withdata/snapshot/+data/genesis.json.balancescontaining extra addresses, verifygetAddressInforeturns the genesis-declared balance for those addresses.Summary by CodeRabbit
Bug Fixes
Tests