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

WaitForHeight test util doesn't check the node's "local" height #14520

Closed
facundomedica opened this issue Jan 6, 2023 · 2 comments · Fixed by #14609
Closed

WaitForHeight test util doesn't check the node's "local" height #14520

facundomedica opened this issue Jan 6, 2023 · 2 comments · Fixed by #14609
Assignees
Labels

Comments

@facundomedica
Copy link
Member

facundomedica commented Jan 6, 2023

We've seen many times tests that do stuff like "1. send tx, 2. check output, 3. query something related to the tx/get tx with hash" which should fine, but they would fail sometimes in the CI or even locally. The errors would always be stuff like "tx not found", "x was expected to be equal to y", etc Like the tx never happened or the height that we are querying for doesn't exist.
These tests use the util method WaitForHeight which waits until a certain height is returned by queries to the Tendermint endpoint Status.
In these tests, both the query to get the height and the query to get whatever the test is querying for are done on the same node.
So what's happening here is that Tendermint is already replying with the latest block height before the Cosmos app had the chance to call Commit() thus storing this height.

My proposed fix is to add a way to query the baseApp's height, which is the one that matters when doing queries. Or Tendermint should return the latest block after its app calls Commit()?

@facundomedica facundomedica self-assigned this Jan 6, 2023
@facundomedica
Copy link
Member Author

On Tendermint there's a similar issue, you can query BlockResults without passing a height and sometimes it returns an error stating that could not find results for height #3. So Tendermint is picking the wrong height?

@alexanderbez
Copy link
Contributor

Thanks for creating this issue. Yes, I agree to avoid these annoying timing issues, using baseapp directly should work. I think the network type, we have access to the app. If so, we can just call that and avoid a network call too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants