Skip to content

feat(upgrade): add v1.3.0 upgrade handler (noop)#263

Merged
puneet2019 merged 10 commits into
mainfrom
feat/v1.3.0-authz-recovery
Jun 15, 2026
Merged

feat(upgrade): add v1.3.0 upgrade handler (noop)#263
puneet2019 merged 10 commits into
mainfrom
feat/v1.3.0-authz-recovery

Conversation

@puneet2019

@puneet2019 puneet2019 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Makes the v1.3.0 upgrade handler a noop (RunMigrations). After investigation, the authz-grant restoration originally proposed here is both unnecessary and unsafe, so it is dropped.

Why no restore is needed

The moca-iavl commit-time bug left authz fastnode-vs-merkle-tree drift. But the only authz grants moca's state machine reads are create-time gates, verified across staking / slashing / distribution / sp / virtualgroup:

consumer where gated
validator self-del → gov MsgDelegate MsgCreateValidator only blockHeight != 0 (genesis skips)
SP funding → gov MsgDeposit MsgCreateStorageProvider only blockHeight != 0

Nothing reads them after creation — Delegate, Redelegate, Undelegate, EditValidator, Unjail, Impeach, WithdrawReward, WithdrawCommission, slashing, SP Deposit top-ups, EditStorageProvider, UpdateSpStoragePrice/Status all skip the check. And a new validator/SP creator MsgGrants right before creating, so even creation needs no pre-existing grant. So the dropped grants need no restoration; existing validators/SPs are unaffected.

Why a handler can't safely touch them anyway

The only grant enumeration reachable from a handler is the store iterator, which reads the IAVL fastnode, not the tree. A 2-node test proved a purge/regrant keyed off it forks on a node whose fastnode is missing a tree-backed key (fastnode < tree): the key survives in that node's tree while others delete it → divergent apphash → halt.

The real v1.3.0 fixes (not a handler)

  • cosmos/iavl#1009 (in the v1.3.0 binary) — stops the prove=true panic on the phantom keys.
  • IAVL fastnode rebuild — state-sync the drifted nodes, or bump fastStorageVersionValue — rebuilds the fastnode from the canonical tree, covering both drift directions with zero consensus risk.

Change

Removes app/upgrades/v1_3_0_authz_recovery.go + test; the handler is an inline noop with a comment explaining the above.

…23,239

The moca-iavl commit-time bug (reformatted-root GetNode miss; cosmos/iavl
#1007/#1009) dropped authz grants from the merkle tree at mainnet block
17,123,239. The grants are still in live state (no-proof queries return them)
but are UNREACHABLE via merkle traversal, so prove=true queries panic and
state-synced nodes never receive them.

Audited all 34 substores on a block-synced mainnet sentry at version
18,271,389 with the new cmd/iavl_audit (per-leaf merkle Get with
skipFastStorageUpgrade=true). Result: authz is the ONLY damaged store
(4 unreachable leaves). evm (12,496 leaves), sp (2,447,617), staking,
bank, acc and every other store are fully reachable.

The v1.3.0 handler (previously a noop) now re-inserts the 4 grants from an
embedded, chain-id-keyed recovery.json (deterministic no-op on devnet/testnet).
SaveGrant is unconditional: the keeper's GetAuthorization reads the fastnode
cache which still holds these grants, so an if-absent check would wrongly skip
the merkle repair. Because the committed authz root currently excludes these
grants on every node, the same SaveGrant set yields the same new root
everywhere. Must run on the v1.3.0 binary (carries iavl#1009 so SaveGrant's
tree traversal does not hit the missing node).

Tested: handler re-inserts all 4 grants on moca_2288-1; no-op on moca_5151-1.
…-rc1

devnet (moca_5151-1) has no real merkle damage, but it was a no-op for
the recovery handler — so the recovery path never actually ran anywhere
but mainnet. Add 2 synthetic GenericAuthorization grants (no expiration)
to the devnet section of recovery.json so the v1.3.0 upgrade exercises
SaveGrant end-to-end on devnet before the mainnet run.

- recovery.json: devnet section now lists 2 dummy grants; note marks them
  explicitly SYNTHETIC (not dropped grants). Deterministic to re-insert
  (generic, nil expiration), so no fork risk.
- tests: add Devnet_ReinsertsGrants (mirrors the mainnet test); repoint
  OtherChain_NoOp to testnet (moca_222888-1), which stays empty.
- CHANGELOG: correct the #263 line (devnet is no longer a no-op).

mainnet (4 grants) and testnet (0) are unchanged.
Replaces the JSON re-insert approach. The moca-iavl commit-time bug left
authz fastnode-vs-tree drift; among the dropped grants are the
SelfDelAddress -> gov MsgDelegate grants that CheckStakeAuthorization
requires on every MsgCreateValidator, so on a clean node no validator
could be created.

The handler now:
  1. purges every authz grant (empty authz tree -> identical apphash on
     every node, no per-node-drift data to reconstruct);
  2. re-grants only what moca's custom handlers require, keyed off the
     canonical staking/sp stores so the write set is deterministic:
       - each validator SelfDelAddress -> gov : MsgDelegate (Generic)
       - each SP FundingAddress     -> gov : MsgDeposit  (Generic)
     SaveGrant's Set restores dropped grants into the tree and overwrites
     the phantom fastnode entry, healing those keys.

Bad/empty addresses are skipped (logged) so one malformed entry can't
halt the upgrade. Non-essential grants (user<->user, capped deposit
authorizations) are dropped; owners re-create them.

Note: this fixes the consensus tree. Residual phantom fastnode entries on
already-drifted nodes are node-local and need an IAVL fast-storage
rebuild (state-sync, or bump fastStorageVersionValue) -- not reachable
from a consensus handler.

Drops recovery.json (no static data needed).
Make the v1.3.0 reset fix the IAVL fastnode drift entirely within the
upgrade handler — no app.toml iavl-disable-fastnode, no state-sync, no
moca-iavl change.

A plain Delete only clears a key's fastnode entry when the key is in the
merkle tree (iavl Remove returns removed=true). A phantom is in fastnode
only, so Delete short-circuits and leaves it. The purge phase now sweeps
every key the authz store iterator yields (which includes drifted-node
phantoms) doing Set(key)+Delete(key): Set puts the key in the tree so the
following Delete removes it from BOTH the tree and the fastnode.

Result on every node: empty authz tree AND empty authz fastnode. The
final tree is empty regardless of how many keys each node swept, so the
apphash is identical; the fastnode is empty everywhere, so the drift is
gone. cmd/iavl_audit confirmed authz is the only drifted store, so this
is the complete fastnode fix.

Phase 2 (restore validator/sp -> gov grants) unchanged. Handler now takes
the authz StoreKey to reach the raw KVStore.
A 2-node test proved the in-handler purge is not consensus-safe. The only
key enumeration reachable from a handler is the store iterator, which
reads the IAVL fastnode index, not the merkle tree. On a node whose
fastnode is MISSING a tree-backed key (fastnode < tree), the iterator
never yields that key, so the purge skips it; the key survives in that
node's tree while other nodes delete it -> divergent apphash -> fork.
Reproduced directly: injecting a phantom on one validator (fastnode >
tree) and deleting a tree-backed fastnode entry on the other (fastnode <
tree), the post-upgrade block diverged (wrong Block.Header.AppHash) and
the chain halted.

So the handler no longer touches the authz store wholesale. It only
re-grants the grants moca's custom handlers require — validator
SelfDelAddress -> gov (MsgDelegate) and SP funding -> gov (MsgDeposit) —
keyed off the canonical staking/sp stores, which is identical on every
node and independent of the drifted authz fastnode. The restored grants
are Generic with no expiration, so SaveGrant touches no grant-queue state
and does not depend on reading the drifted store; its Set re-adds the
grant to the tree and overwrites any stale fastnode entry for that key.

The residual fastnode drift (non-essential phantoms / missing entries) is
a node-local concern fixed by an IAVL fastnode rebuild — state-sync or a
fastStorageVersionValue bump — which rebuilds the fastnode from the
canonical tree and covers both drift directions with no consensus risk.
@phenix3443

Copy link
Copy Markdown
Contributor

I found two blocking issues in the current upgrade handler:

  1. app/upgrades/v1_3_0_authz_recovery.go:98

    The handler recreates every recovered SP deposit grant as GenericAuthorization. That changes behavior, not just data shape. Existing SP flows grant DepositAuthorization, which enforces both the target SP address and the remaining max deposit amount. GenericAuthorization accepts any MsgDeposit of that type unconditionally, so after this upgrade the gov module would gain effectively unlimited deposit authority over every recovered SP funding account.

    Relevant references:

    • x/sp/types/authz.go (DepositAuthorization.Accept enforces SpAddress and MaxDeposit)
    • x/sp/client/cli/tx.go and e2e/core/basesuite.go (current flows grant DepositAuthorization, not generic authz)
    • moca-cosmos-sdk/x/authz/generic_authorization.go (GenericAuthorization.Accept always returns Accept: true)
  2. app/upgrades/v1_3_0_authz_recovery.go:71

    The recovery source is narrower than the PR description and narrower than the bug it claims to fix. The handler only reconstructs grants that can be derived from the current validator set and current SP set, so it cannot restore any dropped authz entry whose granter is no longer an active validator self-delegation address or a current SP funding address. Those grants remain permanently lost.

    If the real goal is only to restore the minimum grants required for MsgCreateValidator and SP deposit flows, the PR title/body/changelog should state that explicitly. If the goal is to recover the authz entries dropped by the IAVL bug, this implementation under-recovers.

I also ran go test ./app/upgrades/..., and it passes, but the current test only validates the new assumptions. It does not check preservation of DepositAuthorization semantics or recovery of dropped grants outside the current validator/SP-derived set.

Address review on #263:

1. Drop the SP deposit grant restoration. Re-granting it as
   GenericAuthorization broadened authority (unbounded deposit for the gov
   module over every SP funding account) vs the original scoped
   DepositAuthorization. It was also ineffective for the actual incident:
   the dropped mainnet SP-deposit grants' granters are not SP funding
   addresses, and mainnet currently has 0 SPs, so the loop is a no-op
   there while over-granting on chains that do have SPs.

2. Narrow + accurately scope the handler to the one grant the state
   machine requires: each validator SelfDelAddress -> gov : MsgDelegate
   (Generic, matching the standard create-validator flow), which
   CheckStakeAuthorization enforces on MsgCreateValidator. This is NOT a
   full recovery of every dropped authz entry — grants whose granter is
   not a current validator are not reconstructable from on-chain state
   and owners re-create them. PR title/body/changelog updated to say so.

Handler renamed V1_3_0RestoreGovGrants -> V1_3_0RestoreValidatorDelegateGrant;
SpKeeper dependency removed.
@puneet2019 puneet2019 changed the title feat(upgrade): v1.3.0 authz recovery for iavl merkle-drop at 17,123,239 feat(upgrade): v1.3.0 restore validator delegate grant required for create-validator Jun 12, 2026
puneet2019 added a commit that referenced this pull request Jun 12, 2026
….3.x)

Mirror of the #263 review fix on release/1.3.x: drop the SP deposit grant
restoration (re-granting it generically broadened authority vs the scoped
DepositAuthorization, and was a no-op for the real incident) and narrow
the handler to the one grant the state machine requires —
validator SelfDelAddress -> gov : MsgDelegate (Generic), enforced by
CheckStakeAuthorization on MsgCreateValidator.

Renamed V1_3_0RestoreGovGrants -> V1_3_0RestoreValidatorDelegateGrant;
SpKeeper dependency removed. Module path evmos/evmos/v12 (release/1.3.x).
…gates

Drop the authz-restore handler. Investigation showed the only authz
grants moca's state machine reads are create-time gates:
  - validator self-del -> gov (MsgDelegate) in MsgCreateValidator
  - SP funding -> gov (MsgDeposit) in MsgCreateStorageProvider
Both are gated on blockHeight != 0 (genesis skips them). Nothing reads
them after creation — delegate, redelegate, undelegate, edit-validator,
unjail, withdraw rewards/commission, slashing, SP deposit top-ups, edit,
price/status updates all skip the check (verified across staking,
slashing, distribution, sp, virtualgroup). A new validator/SP creator
re-grants right before creating, so even creation needs no pre-existing
grant. So the grants the moca-iavl bug dropped need no restoration.

Restoring/deleting them from a handler is also consensus-unsafe: the only
enumeration reachable from a handler is the store iterator, which reads
the IAVL fastnode not the tree, so it forks on a node whose fastnode is
missing a tree-backed key (proven in a 2-node test).

The real v1.3.0 fixes are not a handler: cosmos/iavl#1009 (in the binary)
stops the prove=true panic on the phantom keys, and an IAVL fastnode
rebuild (state-sync / fastStorageVersionValue bump) clears the drift,
covering both drift directions with no consensus risk.

Removes app/upgrades/v1_3_0_authz_recovery.go and its test.
puneet2019 added a commit that referenced this pull request Jun 12, 2026
…gates (release/1.3.x)

Mirror of the #263 decision on release/1.3.x. The only authz grants
moca's state machine reads are create-time gates (validator self-del ->
gov MsgDelegate in MsgCreateValidator, SP funding -> gov MsgDeposit in
MsgCreateStorageProvider); nothing consumes them after creation, and a
new creator re-grants before creating, so the grants the moca-iavl bug
dropped need no restoration. Restoring/deleting from a handler is also
consensus-unsafe (enumeration via the store iterator reads the fastnode,
not the tree, and forks on a node whose fastnode is missing a tree-backed
key).

Real fixes are not a handler: cosmos/iavl#1009 in the binary stops the
prove=true panic, and an IAVL fastnode rebuild (state-sync /
fastStorageVersionValue bump) clears the drift.

Removes app/upgrades/v1_3_0_authz_recovery.go and its test.
@puneet2019 puneet2019 changed the title feat(upgrade): v1.3.0 restore validator delegate grant required for create-validator feat(upgrade): add v1.3.0 upgrade handler (noop) Jun 12, 2026
main tracks upstream cosmos/iavl, so no moca-iavl pin here. Correct the framing:
the validator->gov StakeAuthorization / SP->gov DepositAuthorization grants that
can look 'missing' are consumed and auto-deleted by normal authz flow (not
dropped by the bug), so nothing needs restoring. The fastnode-phantom cleanup is
delivered by the v1.3.0 release on release/1.3.x via the moca-iavl
fastStorageVersionValue bump; the handler on main is a pure noop.
@puneet2019 puneet2019 merged commit 49fd252 into main Jun 15, 2026
26 of 27 checks passed
@puneet2019 puneet2019 deleted the feat/v1.3.0-authz-recovery branch June 15, 2026 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants