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

CLI --height does not set timestamp #15256

Closed
ValarDragon opened this issue Mar 3, 2023 · 3 comments
Closed

CLI --height does not set timestamp #15256

ValarDragon opened this issue Mar 3, 2023 · 3 comments

Comments

@ValarDragon
Copy link
Contributor

ValarDragon commented Mar 3, 2023

related #12226

Summary of Bug

Setting --height for queries in the CLI does not setup the timestamp correctly, which results in erroneous query results. https://github.com/cosmos/cosmos-sdk/blob/8fb95f06ed65416f800ce092c7fb6e7fe460711f/client/cmd.go#L190C65-L193

So doing any historical query today will return the incorrect result, if the query relies on a timestamp.

imo this is a notable bug, as contract devs / app devs should expect to be able to run historic queries to simulate things at past states, or recreate indexes. (For their end user application & data analysis, ofc not in the state machine)

Its possible that there are corrupt datasets due to this bug, in ways noone's today aware of.

Version

All versions as far as I can tell?

Suggested solution

When --height is set, have the CLI require reading the timestamp by querying something (I'm not sure where its stored, Tendermint DB's, or in IAVL somewhere?), and use that as the correct block timestamp. Ideally we would query the full header, and have the context populated with the correct header.

@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Mar 3, 2023
@github-project-automation github-project-automation bot moved this to 📝 Todo in Cosmos-SDK Mar 3, 2023
@alexanderbez
Copy link
Contributor

In order for the timestamp to be set on the Context in the query, the node would have to query Comet's RPC to get the block at that height.

@tac0turtle tac0turtle removed the needs-triage Issue that needs to be triaged label Mar 6, 2023
@ValarDragon
Copy link
Contributor Author

ValarDragon commented Mar 7, 2023

Seems like the node should do this then (and later on figure out how to lower time overhead for this)

@alexanderbez
Copy link
Contributor

@ValarDragon I'm gonna close this as a duplicate of #12226 if that's OK with you. Feel free to re-open if you think otherwise :)

@github-project-automation github-project-automation bot moved this from 📝 Todo to 👏 Done in Cosmos-SDK Mar 17, 2023
@tac0turtle tac0turtle moved this to 🥳 Done in Cosmos-SDK Nov 16, 2023
@tac0turtle tac0turtle removed this from Cosmos-SDK Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants