search

cosmos / ibc-go

Inter-Blockchain Communication Protocol (IBC) implementation in Golang.
https://ibc.cosmos.network/
MIT License
548 stars 586 forks source link

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

Closed yaruwangway closed 2 years ago

yaruwangway commented 2 years ago

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. https://github.com/cosmos/testnets/pull/301#issuecomment-1061261930

  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. https://github.com/cosmos/testnets/pull/301#issuecomment-1061980037

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

yaruwangway commented 2 years ago

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 commented 2 years ago

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