Repository navigation
Conversation
gnoland start reads genesis through the streaming loader, which logged the top-level initial_height as an unknown field and dropped it, so a hardfork genesis started at height 1. The loader now reads it. applyStreamingAppState had no check that app_state.initial_height agrees with it. Both loaders now call validateInitialHeight. The streaming one panics on a mismatch or an unreadable height, since the handshake ignores a returned refusal and would boot a chain with no accounts until gnolang#5995 lands.
Collaborator
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
This branch has not been deployed
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.
Overview
TLDR
A genesis file can ask for a chain to start above height 1, and
gnoland startignores that and starts at 1, logging one warning. This makes the node start where the file says, and stop at boot when the file's two copies of that height disagree. A chain whose file sets no height, mainnet included, sees no change.Two copies of the height
A genesis carries its initial height twice.
GenesisDoc.InitialHeightsits at the top of the file, and consensus and the block store follow it: the first block takes this height, and 0 or 1 both mean an ordinary start at 1.GnoGenesisState.InitialHeightsits insideapp_state, so tooling reading the app state finds it there.gnogenesis fork generatewrites both to the same value. Nothing ties them, so a hand edit can make them differ.Two genesis loaders
Integration tests and
gnodevbuild aGnoGenesisStatein memory, andapplyInMemoryAppStateapplies it.gnoland startcannot decode a genesis of several hundred megabytes into memory, soLoadStreamingGenesisDocwalks the file once, copies balances and transactions to an on-disk cache, and hands the app a*GenesisStateRefforapplyStreamingAppStateto read back.The walk decodes top-level fields by name in
decodeTopLevelField, and any name it does not list is logged asignoring unknown top-level genesis fieldand skipped. The two apply functions are separate code, so a guard written for one has to be added to the other by hand.Why the new check panics
An app refuses a genesis by setting
ResponseInitChain.Error. The handshake inReplayBlocksreads only the Go error fromInitChainSync, so a returned refusal is logged and the node boots on whateverInitChainhad already written. That is whyenforceStrictReplaypanics, and the check inapplyStreamingAppStatepanics the same way.applyInMemoryAppStatestill returns its mismatch, so a mismatch there still boots. gnolang/gno#5995 proposes that the handshake read the field, after which both panics could become returns.Before and after
Before: a genesis with both heights at 1000 starts at height 1 through
gnoland start. With 1000 at the top and 7 inapp_state, it also starts at 1, and nothing compares them.After: both heights at 1000 commit the first block at 1000, with no block 1. With 1000 and 7 the node exits on
panic: InitialHeight mismatch: GnoGenesisState.InitialHeight=7, GenesisDoc.InitialHeight=1000.Words used here
validateInitialHeightapp_stateheight is set and differs from the top-level one.applyInMemoryAppStateandapplyStreamingAppStatecall it.enforceStrictReplayReplayBlocksInitChainwhen the app's height is 0.4/10, medium
Problem
A hardfork genesis that sets
"initial_height": "1000"starts its chain at height 1 whengnoland startreads it, and the only trace isignoring unknown top-level genesis field.gnoland startreads genesis through the streaming loader, whosedecodeTopLevelFielddecodes the fields it names, andinitial_heightis not one, so the height never reaches the handshake. #5684 added the loader after #5511 added the field.The same path does not compare that height with
app_state.initial_height, where the in-memory loader does.Fix
gnoland startnow begins the chain at the file's height and stops at boot when the two copies differ, and on a node booted that way/genesisserves the height too.The streaming check panics rather than returns, since the handshake ignores a returned refusal: with a returned error, a genesis with 1000 at the top and 7 in
app_statebooted a chain at height 1000 with no accounts. Both apply functions share the comparison throughvalidateInitialHeight.This follows #5995, which makes the handshake read a refusal and leaves this path alone, and either can land first. Their production code merges cleanly, and whichever lands second merges two test files,
app_test.goandnode_initial_height_test.go.A chain whose file sets no height sees no change: mainnet's published genesis sets neither copy, and
misc/deploymentsholds no such file. On a file that sets one:app_stateheight panics on a first boot.Two tests used this field as their example of an unknown one and no longer do, and
TestServeGenesis_SemanticEquivalenceWithSource, which CI skips without a fixture and which passes with one carrying the field, no longer strips it.A real
gnoland starton a genesis with both heights at 1000 commits its first block at 1000, holds no block 1 and carries on across a restart, and a node restarted onto this binary frommasterkeeps its height and app hash.Not fixed
Design alternatives
enforceStrictReplaydoes. Return the error instead once the handshake readsResponseInitChain.Error.How it was reviewed
Single-reader review, first: one reader checks the whole diff, its findings are fixed, and a fresh reader starts again until one finds nothing.
Read the code in this order
🔴 hot is the riskiest code, 🟠 warm other code, 🔵 cold a test. The tier ranks the file's risk, not how much of it changed.
gno.land/pkg/gnoland/app.goReads the
app_stateheight and stops the boot when it differs from the top-level one.If wrong: a genesis whose heights differ boots on no state, or a valid hardfork genesis refuses to boot.
gno.land/pkg/gnoland/genesis_state_ref.goFills
doc.InitialHeighton the first load and on each one after it.If wrong: a hardfork genesis boots at height 1, or panics once the check above compares against 0.
gno.land/pkg/gnoland/node_initial_height_test.goBoots a node from a genesis file read by the streaming loader and checks which heights the store holds.
If wrong: a dropped height shows up only on a real hardfork boot.