fix: canonical validator ids and safe reconcile close-out#82
Merged
Conversation
Validator entity id now comes from MsgCreateValidator.validatorAddress (the canonical staking module key) instead of being derived from the tx signer pubkey. The derivation is kept as a cross-check that fails loudly on mismatch, in both the tx and genesis gentx paths. This removes the implicit id == operatorAddress coupling the reconciler and reward/commission FKs rely on, and fixes the latent 'Unsupported Signer' id corruption. Reconcile close-out (entity tracked locally but absent from the chain read) is now restricted to Unstaking -> Unstaked. An entity cannot leave the chain store without unbonding first, so a Staked row missing from the read is an impossible state: it is logged as an error and never wiped. This bounds the blast radius of a hypothetical empty/partial chain read to rows already unbonding. Verified on localnet: genesis and tx validator creation produce identical ids with zero cross-check failures, and a live app unstake transitioned Unstaking -> Unstaked within two blocks of the chain dropping it.
oten91
approved these changes
Jun 11, 2026
This was referenced Jun 16, 2026
jorgecuesta
added a commit
that referenced
this pull request
Jun 16, 2026
## Problem The genesis handler only creates validators from `genutil.gen_txs`. A migration/restart (state-export) genesis instead carries the live validator set under `app_state.staking.validators` with `gen_txs` empty, so those validators are **never indexed**. Observed on **beta** (`pocket-lego-testnet`): genesis had `staking.validators = 5`, `gen_txs = 0`, and the DB was missing all 5 (only the 3 post-genesis `MsgCreateValidator` tx validators were present). The ingested genesis was correct — the handler simply skipped that section (it reads `staking.params` but not `staking.validators`). Mainnet (`gen_txs = 1`, `staking.validators = 0`) is unaffected. ## Change `_handleGenesisGenTxs` now also processes `app_state.staking.validators` in the **same persistence pass** as the gen_tx validators. Keeping it in one handler is deliberate: splitting it into a parallel handler would race on `optimizedBulkCreate`'s `destroy`-by-`block_id` of the shared `MsgCreateValidator` model (both would wipe each other's rows at the genesis block). Per staking validator, a `MsgCreateValidator` is synthesized (mirroring how genesis apps synthesize a `MsgStakeApplication` so relations resolve), with: - `id` = `operator_address` (canonical staking key, consistent with #82) - `ed25519_id` from the consensus pubkey (`sha256(pubkey)[:20]`, hex) - `signer_id` = operator; `signer_pokt_prefix_id` = operator re-encoded to the `pokt` prefix (no signer pubkey exists in a state export) - `commission` converted from the genesis decimal (`"0.1"`) to the protobuf-scaled integer (`"100000000000000000"`) that the live path and `reconcileValidators` persist - `stake_status` mapped from the export's bond status - `transaction_id` = a genesis fake tx hash, index space continued past the gen_tx validators so hashes never collide If a genesis ever carries both sources, operators already created from a gen_tx are skipped to avoid a duplicate id. ## Verification - Replayed the handler's derivation against the real beta genesis: it reproduces the **exact** rows (`id`, `ed25519_id`, `signer_pokt_prefix_id`, `commission.rate`, `stake_status`, tx hash) that were confirmed earlier against the live cometbft validator set and the existing DB row shape. - `tsc` (changed files) and `eslint` clean. ## Self-review (Otto out on leave) Ran a high-effort self-review (parallel finders + verify). Findings addressed: - **Race** on the shared `MsgCreateValidator` `destroy`-by-`block_id` → unified into one handler/one write pass. - **Wrong comment** claiming the gen_tx path stores the integer commission form (it stores raw decimal) → corrected; the conversion matches the live/reconcile representation and existing rows. - **Message.json shape** → now stores a `MsgCreateValidator`-shaped payload matching the declared `typeUrl`, not the raw `staking.Validator` shape. - **Double ed25519 computation** → computed once in the synthesized msg; the Validator reads it back. - **`min_self_delegation` NaN** on an empty/missing field → guarded. - `securityContact` retained (camelCase) to match the live path's actual stored data; the schema's `securityContract` is a pre-existing never-populated typo, out of scope. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
Follow-up to #81, hardening two review findings with verified fixes.
Validator id from
validatorAddress(review point 3)Validator.idnow comes straight fromMsgCreateValidator.validatorAddress— the canonical staking module key thatreconcileValidators(keyed bycv.operatorAddress) and the reward/commission FKs already assume — instead of being re-derived from the tx signer pubkey. The derivation is kept as a cross-check that throws on mismatch, in both the tx handler and the genesis gentx path. Side effect: theUnsupported Signer: ...fallback can no longer leak into the entity id.Verified on localnet with a clean reindex:
MsgCreateValidatortx: row created withid == validator_address, status correctly mirrorsBOND_STATUS_UNBONDEDvalidator_rewardsFKsClose-out restricted to
Unstaking → Unstaked(review point 2)An entity cannot leave the chain store without unbonding first, and the unbonding-begin triggers already move rows to
Unstakingwhen that block is processed. So the reconcile close-out now only transitionsUnstaking → Unstaked; aStakedrow missing from the chain read is an impossible state and gets logged as an error instead of being wiped. This bounds the blast radius of a hypothetical empty/partial chain read to rows already unbonding.Note: the empty-read scenario itself cannot happen silently through this stack — cosmjs throws at every layer (HTTP ≥400, JSON-RPC error object, ABCI
code != 0), so an empty list requires a genuinely successful response. This change is defense in depth plus indexer-bug detection.Verified on localnet: live app unstake →
Unstakingon the begin event → chain dropped the app at unbonding end → row transitioned toUnstaked/stake 0 within two blocks, zero false-positive error logs.🤖 Generated with Claude Code