Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

E2E upgrade tests are failing #3131

Closed
3 tasks
damiannolan opened this issue Feb 9, 2023 · 3 comments · Fixed by #3132 or #3136
Closed
3 tasks

E2E upgrade tests are failing #3131

damiannolan opened this issue Feb 9, 2023 · 3 comments · Fixed by #3132 or #3136
Assignees
Labels
e2e type: bug Something isn't working as expected
Milestone

Comments

@damiannolan
Copy link
Member

damiannolan commented Feb 9, 2023

Summary of Bug

End-to-end upgrade tests are failing for ibc-go/v7 after upgrading to sdk 0.47.

Also effected by the broadcast mode changes in interchaintest.
We no longer rely on broadcast-mode block and now query for the sdk.TxResponse using authtx.QueryTx(hash).

We need to register more types on the e2e global codec to ensure interchaintest can parse response data such as the Tx Any -> https://github.com/cosmos/cosmos-sdk/blob/main/proto/cosmos/base/abci/v1beta1/abci.proto#L37


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@damiannolan damiannolan moved this to In progress in ibc-go Feb 9, 2023
@damiannolan damiannolan added the type: bug Something isn't working as expected label Feb 9, 2023
@damiannolan damiannolan moved this from In progress to In review in ibc-go Feb 9, 2023
@damiannolan damiannolan changed the title E2E upgrade tests are failing after broadcast mode changes E2E upgrade tests are failing Feb 10, 2023
@damiannolan
Copy link
Member Author

SDK v0.47 auto migrations failing due to no registration of key table / param set pairs for migrating params.

We must add WithKeyTable() to each module which is migrating params in app.go

paramsKeeper.Subspace(banktypes.ModuleName).WithKeyTable(banktypes.ParamKeyTable())

@damiannolan
Copy link
Member Author

damiannolan commented Feb 10, 2023

Upgrade handler looks to succeed now with migrations successfully being run.
Subsequent queries to ibc client status now failing due to:

rpc error: code = Unknown desc = codespace sdk code 18: invalid request: failed to load state at height 121;
version mismatch on immutable IAVL tree; version does not exist. Version has either been pruned, 
or is for a future block height (latest height: 121)

https://github.com/cosmos/ibc-go/actions/runs/4142858377

We may need to add the consensus store key to StoreUpgrades:

upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
if err != nil {
	panic(err)
}

if upgradeInfo.Name == v7.UpgradeName && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
	storeUpgrades := storetypes.StoreUpgrades{
		Added: []string{consensusparamtypes.StoreKey},
	}

	// configure store loader that checks if version == upgradeHeight and applies store upgrades
	app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades))
}

@damiannolan damiannolan moved this from In review to In progress in ibc-go Feb 10, 2023
@damiannolan damiannolan added this to the v7.0.0 milestone Feb 10, 2023
@github-project-automation github-project-automation bot moved this from In progress to Todo in ibc-go Feb 14, 2023
@crodriguezvega crodriguezvega moved this from Todo to Done in ibc-go Feb 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e type: bug Something isn't working as expected
Projects
Archived in project
3 participants
@damiannolan @chatton and others