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

imp: investigate why upgrade-client halts gaiad #178

Merged
merged 13 commits into from
Apr 24, 2024
Merged

Conversation

rnbguy
Copy link
Member

@rnbguy rnbguy commented Apr 23, 2024

Closes #177

  • updates CI dependencies (gaia, hermes, cometbft, grpcurl, rust toolchain).
  • in case of failure, prints last 200 lines from the logs from gaia, basecoin, cometbft.
  • uses github workflow concurrency option.
  • uses actions-rust-lang/setup-rust-toolchain to setup rust.
  • implements HealthService::get_syncing.
  • rename one-chain script to one-chain.sh.

@rnbguy rnbguy changed the title fix: integration test when running upgrade-client before token-transfer fix: upgrade-client before token-transfer fails integration test Apr 23, 2024
@rnbguy
Copy link
Member Author

rnbguy commented Apr 23, 2024

Looks like, there is consensus failure at gaiad chain (ibc-0)

UPGRADE "plan" NEEDED at height: 144:  module=consensus stack=goroutine 132 [running]:
runtime/debug.Stack()
	runtime/debug/stack.go:24 +0x65
github.com/tendermint/tendermint/consensus.(*State).receiveRoutine.func2()
	github.com/tendermint/tendermint@v0.34.21/consensus/state.go:732 +0x4c
panic({0x1949de0, 0xc003c99d30})
	runtime/panic.go:838 +0x207
...

@rnbguy rnbguy force-pushed the rano/fix/integration-test branch from c536af0 to bef2d5d Compare April 23, 2024 15:43
@rnbguy rnbguy force-pushed the rano/fix/integration-test branch from bef2d5d to 8d8e8d7 Compare April 23, 2024 16:04
@rnbguy rnbguy changed the title fix: upgrade-client before token-transfer fails integration test fix: upgrade-client halts gaiad Apr 23, 2024
@rnbguy
Copy link
Member Author

rnbguy commented Apr 23, 2024

Latest changes now log the error directly on workflow.

cc @ljoss17

@rnbguy
Copy link
Member Author

rnbguy commented Apr 23, 2024

The error is coming from cosmos/cosmos-sdk/x/upgrade/keeper/abci.go#L79.

That means the upgrade plan does not set any handler. We are using the default plan as the plan name. Should this be anything specific for client upgrade?

@rnbguy
Copy link
Member Author

rnbguy commented Apr 24, 2024

We need to recompile the binary with a handler for the plan. I think it'll be fine to cancel the software upgrade somehow. There is a subcommand gaiad tx upgrade cancel-software-upgrade - but it doesn't work.

@rnbguy
Copy link
Member Author

rnbguy commented Apr 24, 2024

It looks like the upgrade-client test is meant to halt gaiad after the upgrade height. So, this script must be executed at the end of our e2e tests.

We may only run the grpc-service after it, as it doesn't involves gaiad.

@rnbguy rnbguy force-pushed the rano/fix/integration-test branch from dbb8c6b to 16b4a9a Compare April 24, 2024 17:23
@rnbguy rnbguy marked this pull request as ready for review April 24, 2024 17:57
@rnbguy rnbguy requested a review from seanchen1991 April 24, 2024 17:57
@rnbguy rnbguy changed the title fix: upgrade-client halts gaiad imp: investigate why upgrade-client halts gaiad Apr 24, 2024
Copy link
Contributor

@seanchen1991 seanchen1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Makes sense moving the upgrade client test to the end of the testing order 👍

@rnbguy rnbguy merged commit 97611fb into main Apr 24, 2024
9 checks passed
@rnbguy rnbguy deleted the rano/fix/integration-test branch April 24, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

upgrade-client before token-transfer fails integration test
2 participants