gaia panic in local test: cannot delete latest saved version upon upgrade

[yaruwangway]

Summary of Bug

Gaia upgrade from v6 to a binary built from theta-prepare branch in local test according to this tutorial.

panic upon upgrade height:

panic: cannot delete latest saved version (8)

goroutine 1 [running]:
github.com/cosmos/cosmos-sdk/store/rootmulti.(*Store).pruneStores(0x140010bd080)
	/Users/yaruwang/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.45.1/store/rootmulti/store.go:430 +0x210
github.com/cosmos/cosmos-sdk/store/rootmulti.(*Store).Commit(0x140010bd080)
	/Users/yaruwang/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.45.1/store/rootmulti/store.go:404 +0x184
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).Commit(0x1400122dba0)
	/Users/yaruwang/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.45.1/baseapp/abci.go:308 +0x210
github.com/tendermint/tendermint/abci/client.(*localClient).CommitSync(0x14000244ea0)
	/Users/yaruwang/go/pkg/mod/github.com/tendermint/tendermint@v0.34.14/abci/client/local_client.go:264 +0xec
github.com/tendermint/tendermint/proxy.(*appConnConsensus).CommitSync(0x1400126ede0)
	/Users/yaruwang/go/pkg/mod/github.com/tendermint/tendermint@v0.34.14/proxy/app_conn.go:93 +0x34
github.com/tendermint/tendermint/state.(*BlockExecutor).Commit(0x14000d612d0, {{{0xb, 0x0}, {0x14000d5cd28, 0x8}}, {0x14000d5cd80, 0xe}, 0x89dbae, 0x89dc1c, {{0x140010e2c20, ...}, ...}, ...}, ...)
	/Users/yaruwang/go/pkg/mod/github.com/tendermint/tendermint@v0.34.14/state/execution.go:228 +0x1b8
github.com/tendermint/tendermint/state.(*BlockExecutor).ApplyBlock(0x14000d612d0, {{{0xb, 0x0}, {0x14000d5cd28, 0x8}}, {0x14000d5cd80, 0xe}, 0x89dbae, 0x89dc1c, {{0x140010e2c20, ...}, ...}, ...}, ...)
	/Users/yaruwang/go/pkg/mod/github.com/tendermint/tendermint@v0.34.14/state/execution.go:180 +0x598
github.com/tendermint/tendermint/consensus.(*Handshaker).replayBlock(0x14000fc7dc0, {{{0xb, 0x0}, {0x14000d5cd28, 0x8}}, {0x14000d5cd80, 0xe}, 0x89dbae, 0x89dc1b, {{0x14000f60320, ...}, ...}, ...}, ...)
	/Users/yaruwang/go/pkg/mod/github.com/tendermint/tendermint@v0.34.14/consensus/replay.go:503 +0x1a8
github.com/tendermint/tendermint/consensus.(*Handshaker).ReplayBlocks(0x14000f85dc0, {{{0xb, 0x0}, {0x14000d5cd28, 0x8}}, {0x14000d5cd80, 0xe}, 0x89dbae, 0x89dc1b, {{0x14000f60320, ...}, ...}, ...}, ...)
	/Users/yaruwang/go/pkg/mod/github.com/tendermint/tendermint@v0.34.14/consensus/replay.go:416 +0x6d0
github.com/tendermint/tendermint/consensus.(*Handshaker).Handshake(0x14000fc7dc0, {0x1028a61e0, 0x14000036a90})
	/Users/yaruwang/go/pkg/mod/github.com/tendermint/tendermint@v0.34.14/consensus/replay.go:268 +0x400
github.com/tendermint/tendermint/node.doHandshake({0x10289ec90, 0x140005a0770}, {{{0xb, 0x0}, {0x14000d5cd28, 0x8}}, {0x14000d5cd80, 0xe}, 0x89dbae, 0x89dc1b, ...}, ...)
	/Users/yaruwang/go/pkg/mod/github.com/tendermint/tendermint@v0.34.14/node/node.go:325 +0x114
github.com/tendermint/tendermint/node.NewNode(0x14001117cc0, {0x10285b378, 0x14001273540}, 0x140005a0940, {0x10283dbc0, 0x14000f669a8}, 0x140005a09d0, 0x102817868, 0x140005a0ae0, {0x10287ddc0, ...}, ...)
	/Users/yaruwang/go/pkg/mod/github.com/tendermint/tendermint@v0.34.14/node/node.go:733 +0x510
github.com/cosmos/cosmos-sdk/server.startInProcess(0x1400115dde0, {{0x0, 0x0, 0x0}, {0x1028b3058, 0x1400120d7a0}, {0x14001223f50, 0xe}, {0x1028861c0, 0x140010e1b70}, ...}, ...)
	/Users/yaruwang/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.45.1/server/start.go:263 +0x768
github.com/cosmos/cosmos-sdk/server.StartCmd.func2(0x1400117ec80, {0x1400121a780, 0x0, 0x1})
	/Users/yaruwang/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.45.1/server/start.go:129 +0x144
github.com/spf13/cobra.(*Command).execute(0x1400117ec80, {0x1400121a770, 0x1, 0x1})
	/Users/yaruwang/go/pkg/mod/github.com/spf13/cobra@v1.3.0/command.go:856 +0x668
github.com/spf13/cobra.(*Command).ExecuteC(0x14000e45b80)
	/Users/yaruwang/go/pkg/mod/github.com/spf13/cobra@v1.3.0/command.go:974 +0x410
github.com/spf13/cobra.(*Command).Execute(...)
	/Users/yaruwang/go/pkg/mod/github.com/spf13/cobra@v1.3.0/command.go:902
github.com/spf13/cobra.(*Command).ExecuteContext(...)
	/Users/yaruwang/go/pkg/mod/github.com/spf13/cobra@v1.3.0/command.go:895
github.com/cosmos/cosmos-sdk/server/cmd.Execute(0x14000e45b80, {0x140010fc2a0, 0x15})
	/Users/yaruwang/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.45.1/server/cmd/execute.go:36 +0x23c
main.main()
	/Users/yaruwang/code/gaia/cmd/gaiad/main.go:16 +0x3c
5:40PM WRN Use of cosmovisor without the 'run' command is deprecated. Use: cosmovisor run [args] module=cosmovisor

if in theta-prepare branch, I change the ibc-go version from rc-1 to rc-0, I will have the same panic cannot delete latest saved version after upgrade and produce a few blocks.

the panic is from iavl tree, https://github.com/cosmos/iavl/blob/d99448032e95a6ce05d342737cd745a20ef0b6ce/mutable_tree.go#L568

How to avoid the above panic?

1. if I delete ica module in the upgradehandler, https://github.com/cosmos/gaia/blob/619e66e3104e03bcf320fb37cc663e5e37da7d32/app/app.go#L655-L668, i can upgrade without panic.

2. Hypha sets pruning="nothing" in app.toml, also can upgrade without panic.
   add initial details about theta testnet testnets#301 (comment)

3. use "fresh" genesis, which means use gaia init to generate genesis, rather than use the exported genesis from livehub, this can also upgrade without panic.
   add initial details about theta testnet testnets#301 (comment)

Version

Steps to Reproduce

Gaia upgrade from v6 to a binary built from theta-prepare branch in local test according to this tutorial.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged/assigned

[yaruwangway]

solved, thank you @colin-axner
the issue is when addding upgrade store, i need to add Added: []string{icacontrollertypes.StoreKey, icahosttypes.StoreKey} both substores, not the module Added: []string{icatypes.ModuleName},
The reason:
loadVersion will check the store upgrades when initializing the stores
https://github.com/cosmos/cosmos-sdk/blob/master/store/rootmulti/store.go#L218-#L228

[colin-axner]

Following up with more information:

The issue is that StoreUpgrades is actually initializing the stores with appropriate versions for modules which are added during an upgrade. It loops over the store keys set in the params and checks if the upgrades.IsAdded(key)

The bug here is that we were recommending the icatypes.ModuleName to be set in StoreUpgrades. We don't have a store key for icatypes.ModuleName, we have 2 stores, one for controller and one for host. So really the StoreUpgrades should be:

    Added: []string{icacontrollertypes.StoreKey, icahosttypes.StoreKey},

So the loadVersion is looping over existing store keys and checking if they are in storeUpgrades (which controller/host weren't and thus the initialVersion was never being set. I'm unsure why it doesn't error until the height tries to be pruned. I'm guessing initial version defaults to 0 and maybe when it pruning it prunes only on stores with an initial version < pruning height, but in reality the initial version of our stores aren't created until after pruning height and thus there is nothing to prune