fix(consensus): seat genesis validators in AuthorityManager so a fresh chain bootstraps DPoS#827
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughA new public method Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/sentrix-core/tests/genesis_validator_bootstrap.rs`:
- Around line 41-62: The test assertions only verify the count of validators
rather than their exact identities, which would allow regressions where
incorrect validator addresses are seated or migrated. After the initial
assertion on bc.authority.active_validators().len(), add an additional assertion
that compares the actual validator addresses from
bc.authority.active_validators() against the expected validator addresses
declared in GENESIS_TOML. Similarly, after asserting
bc.stake_registry.active_set.len() equals 3, add an assertion that compares the
actual validator addresses in bc.stake_registry.active_set against the same
expected addresses from GENESIS_TOML to ensure the exact validator set was
migrated, not just any three addresses.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ec493043-9486-430c-b781-579b10005f18
📒 Files selected for processing (2)
crates/sentrix-core/src/blockchain.rscrates/sentrix-core/tests/genesis_validator_bootstrap.rs
…ng) (#828) Scheduled + manual-dispatch workflow that boots a fresh 3-validator testnet from genesis, produces blocks, and asserts the validators converge on the same block hash — covering real BFT + libp2p + block production, which the in-process cargo tests can't. Not a PR gate (schedule/dispatch only), so a flaky network run never blocks a merge. scripts/ci-testnet-convergence.sh is self-contained (generates keystores + genesis, inits + starts N nodes on isolated loopback ports, polls for liveness, then checks block-hash agreement). Depends on #827 to bootstrap DPoS from genesis — merge that first.
… bootstraps DPoS A fresh genesis-only chain booted with an empty authority set and never produced a block. initialize_genesis credited the premine and seated block 0 but nothing seated the [[genesis.validators]]. At the Voyager DPoS fork, activate_voyager migrates the AuthorityManager set into the stake registry; with an empty set it migrated nothing, leaving active_set empty, so consensus hit "BFT activation blocked: active_set is empty" and sat at height 0. (Found bootstrapping a fresh testnet from genesis; the live nets were bootstrapped through the Pioneer-era admin-add path, which masked it.) sentrix init now seats the genesis validators via a new Blockchain::seat_genesis_validators, after construction. It is kept out of the bare Blockchain::new / new_with_genesis constructor on purpose so that path stays a clean slate: many tests build a chain and add a single proposer of their own, and auto-seating there breaks them with NotYourTurn. AuthorityManager is not committed to the trie/state_root, so seating does not change the genesis hash, and only init runs it; running nodes restore the authority set from the persisted state blob, so existing chains are unaffected. Verified: a fresh 3-validator testnet boots from genesis, produces blocks (0 to 26), and all three converge on identical block hashes. Regression test asserts the genesis validators become the exact authority set and that activate_voyager migrates that exact set into the stake registry, and asserts the bare constructor stays validator-free.
6ff69b9 to
611c945
Compare
Problem
A fresh genesis-only chain boots with an empty authority set and never produces a block.
Blockchain::initialize_genesiscredited the premine balances and seated block 0 but never seated the[[genesis.validators]]anywhere. At the Voyager DPoS fork,activate_voyagermigrates theAuthorityManagervalidator set into the stake registry — with an emptyAuthorityManagerit migrates nothing, so the DPoSactive_setis empty and consensus hits:The chain then sits at height 0 forever. (Found while bootstrapping a fresh testnet from genesis — the existing testnet/mainnet were bootstrapped through the Pioneer-era admin-add path, which masked this.)
Fix
Seat the
[[genesis.validators]]in theAuthorityManagerduringinitialize_genesis(viaadd_validator_unchecked— genesis has no admin signature to authorise the add).activate_voyagerthen has a set to migrate into the stake registry at the fork, soactive_setis non-empty and consensus produces blocks.Why this is consensus-safe
AuthorityManageris not committed to the trie / state_root (onlyAccountDB+ stake-registry-derived keys are), so the genesis block hash is unchanged.stake_registryis still left empty at genesis — it's populated at the Voyager fork height, consistently across nodes — so the genesis state-root identity with pre-Genesis-TOML chains is preserved.sentrix init(a fresh data dir). Running nodes restoreauthorityfrom the persisted state blob (it's a serialized field, not rebuilt from genesis), so existing chains are unaffected.Verification
crates/sentrix-core/tests/genesis_validator_bootstrap.rs): asserts the genesis validators land in theAuthorityManagerat init, the stake registry stays empty pre-fork, andactivate_voyagermigrates all of them into the stake registry'sactive_set.Summary by CodeRabbit
Documentation
New Features
Tests
Chores