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

[Bug]: When querying past data, the timestamp of CommitInfo cannot be used #19037

Closed
1 task done
JoowonYun opened this issue Jan 12, 2024 · 3 comments
Closed
1 task done
Labels

Comments

@JoowonYun
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

I looked at #12226 and #15448. I think this is a very good approach.

However, is this code wanted to migrate the entire CommitInfo for timestamp when upgrading the patch version?

The timestamps of blocks before the upgrade are all 0, causing problems with queries using timestamps. For example, if you use CosmWasm, everything will panic due to 0 validation. https://github.com/CosmWasm/wasmd/blob/07c8beea006cd009e0c7b865fb32753f61221e1b/x/wasm/types/types.go#L276C2-L280C1

Question.

  1. How to avoid this bug?
  2. Why didn't you look up core's env directly? Below the code is a modification of that patch, but I think it was possible to use the core's environment from the beginning.
    https://github.com/xpladev/cosmos-sdk/pull/9/files
    _

Cosmos SDK Version

=v0.47

How to reproduce?

Run the node before the patch(#15448) is applied.

Then apply the patch.

Check the state of the block before applying the patch.

@alexanderbez
Copy link
Contributor

I see. So you're querying against historical blocks (pre-upgrade) that do not have this CommitInfo set? I think you'd have to resync those nodes in such a way that the CommitInfo is set or manually patch/migrate.

@JoowonYun
Copy link
Contributor Author

JoowonYun commented Jan 13, 2024

I see. So you're querying against historical blocks (pre-upgrade) that do not have this CommitInfo set?

Right. It made a quering panic in CosmWasm

I think you'd have to resync those nodes in such a way that the CommitInfo is set or manually patch/migrate.

If we re-sync from height 1 block, we need to a lot of patch. That's because chain has many software-upgrades. Is it right your thought about patch?

About migrate, This is a value that has nothing to do with the state of the block, so I'm curious about implementing the migration during software-update. It seems to be a problem when sync from a snapshot or specific data.

@tac0turtle
Copy link
Member

blocks on disc are non deterministic. If we forced users to keep all blocks for a period of time on disc this would work. But since all nodes have different blocks on disc we couldn't do any sort of migration unless as it would panic and/or end up with the current 0 value. Unfortunately, we dont have a clean way of doing this.

If you wanted to do a migration you would need to decide how far back to go, then provide a db snapshot to users that would be required for the upgrade. We didn't want to force this on users so we decided to leave it as is.

Closing this issue as its not a bug, feel free to implement your own migration as mentioned above or some other way, but for the default sdk we are unable to do this

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

No branches or pull requests

3 participants