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

feat: Wormchain v0.47 #4034

Draft
wants to merge 89 commits into
base: main
Choose a base branch
from

Conversation

joelsmith-2019
Copy link

No description provided.

jonathanpberger and others added 25 commits June 24, 2024 11:06
docs: add ADR for wasmvm versioning decision
…s-sdk-v0.47-fork-upgrade

adr(cosmos sdk fork upgrade):Add ADR for Cosmos SDK v0.47 upgrade applied to wormhole-foundation C…
patch(adr): Update 0003-cosmos-sdk-v0-47-upgrade-will-be-implemented-in-wormhole-…
…bft-migration

adr(cometbft): Add ADR for Tendermint to CometBFT migration applied to wormhole-foun…
Misc API Migrations/Fixes
wormchain/app/app.go Outdated Show resolved Hide resolved
wormchain/app/apptesting/test_helpers.go Outdated Show resolved Hide resolved
wormchain/cmd/wormchaind/root.go Show resolved Hide resolved
wormchain/cmd/wormchaind/root.go Outdated Show resolved Hide resolved
wormchain/x/wormhole/governance.go Outdated Show resolved Hide resolved
wormchain/Makefile Outdated Show resolved Hide resolved
wormchain/Dockerfile Outdated Show resolved Hide resolved
github.com/CosmWasm/wasmd v0.30.0 => github.com/wormhole-foundation/wasmd v0.30.0-wormchain-2
github.com/cosmos/cosmos-sdk => github.com/wormhole-foundation/cosmos-sdk v0.45.9-wormhole-2
github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
github.com/cosmos/cosmos-sdk => github.com/strangelove-ventures/wh-cosmos-sdk v0.0.0-20240903174520-1c952abb4033

Choose a reason for hiding this comment

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

Looks like we still need to make a release in the wormhole-foundation/cosmos-sdk repo with the changes at this SL fork version and then update the replace statement.

Copy link
Author

Choose a reason for hiding this comment

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

Good point! We'll definitely need to get that merged in first.

ictest-cancel-upgrade: rm-testcache
cd interchaintest && go test -race -v -run ^TestCancelUpgrade$$ ./...

ictest-chain-start: rm-testcache

Choose a reason for hiding this comment

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

When running this test, it fails with:

wormchain:local: pull image wormchain:local: Error response from daemon: pull access denied for wormchain

It then only passed if I tagged the local docker image built for wormchain ICT as "local" instead of "latest", we may need to update the make command on line 102 above to build the image with the tag local instead of latest, like:

local-image: build/wormchaind
	docker build -t wormchain:local -f Dockerfile.ict ..

We may want to consider a way to ensure the local docker image is built with the latest version of the current branch's wormchaind software before running the tests.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for catching that! I had it on 'local' for the longest time, but I switched it to 'latest' for a quick side test and I never changed it back.

With regards to ensuring the docker image is built, the Github workflow will always do that for pull requests. WIth regards to local development, any changes to the chain code will require a quick 'make local-image'.

I didn't want the make targets to automatically re-run local-image just because it's unnecessary unless code changes were made to the chain rather than the interchaintests directory. Additionally, when we run the Github Actions, each interchain test would try to rebuild the image.

If you could think of a better way to force compile the local-image only for local testing, let me know!

Choose a reason for hiding this comment

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

Good points there, I agree that rebuilding the image every test would be an unnecessary annoyance and make test runs take longer, especially if chain code stays the same. I will spend some time thinking about it, but this should work just fine for now.

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.

5 participants