-
Notifications
You must be signed in to change notification settings - Fork 681
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
base: main
Are you sure you want to change the base?
feat: Wormchain v0.47 #4034
Conversation
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-…
…dation Cosmos SDK fork
…bft-migration adr(cometbft): Add ADR for Tendermint to CometBFT migration applied to wormhole-foun…
Misc API Migrations/Fixes
wormchain/ts-sdk/src/modules/cosmos.tx.v1beta1/types/tendermint/abci/types.ts
Show resolved
Hide resolved
ICT running on v4 defaulted to Osmosis chains with a Bech32Prefix of 'cosmos'. However, on v7, it defaults to 'osmo' which was throwing the entire test off.
…dation Cosmos SDK fork
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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.
No description provided.