-
Notifications
You must be signed in to change notification settings - Fork 126
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
fix: remove redundant ValidateBasic check in BroadcastTx #357
fix: remove redundant ValidateBasic check in BroadcastTx #357
Conversation
looks like the test failure is coming from an rpc not found in the interchain accounts test, but something went wrong with the docker container not starting :) |
I'm re-running the failed one. As common with e2e tests, there's flakiness (especially when dealing with networks). |
interesting, it looks like the same test is consistently failing, even on your main branch. it looks like the interchain account creation is failing, which is why the rpc query returns an error. looks like this error started with this commit: 9359b06 |
You're right. See #354 (comment) |
@DavidNix is it possible to merge this PR before the failing test is fixed? |
Sure. I normally dislike merging when CI is failing. But this seems like a valid exception. |
Co-authored-by: Charly <[email protected]>
this check is redundant because ValidateBasic on the messages will be run by the chain binary anyway, running it prematurely leads to strange version issues
context: writing an e2e test for upgrading ibc-go v6 -> v7 runs automatic migrations which bump the client and consensusstate versions of the tendermint client and solo machine, we would need the msg to be broadcasted and validated by the v6 binary rather than the v7 code.