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

abci,pg: start using height/hash in postgres, remove badger db in abci/info #606

Closed
jchappelow opened this issue Mar 14, 2024 · 3 comments · Fixed by #610
Closed

abci,pg: start using height/hash in postgres, remove badger db in abci/info #606

jchappelow opened this issue Mar 14, 2024 · 3 comments · Fixed by #610
Assignees

Comments

@jchappelow
Copy link
Member

jchappelow commented Mar 14, 2024

We had intended to be checking height and hash of best block recorded in postgres, but that is not done.

Right now you can nuke your postgresql database, restart kwil, and it thinks the application is at the consensus/state/block height because your abci/info is persisted. This is incorrect.

In (*AbciApp) Info we need to read the best block height and hash from postgres (i think kwild_voting.state with columns height and hash would be just fine) so we can return that to cometbft so it starts replaying blocks.

@jchappelow
Copy link
Member Author

I think we concluded the badger kv for abci is now basically useless, just error prone and redundant, @brennanjl , but I wanted to confirm before @charithabandi or anyone else takes the step of removing it.

@jchappelow
Copy link
Member Author

Regarding the height check bug in this issue, it looks like we use getDBHeight and check height in GenesisInit but not in (*AbciApp) Info. I let a loud code comment sneak in https://github.com/kwilteam/kwil-db/pull/607/files#diff-eaaded10eb74f1d44ce5ad1bc9e5d15c4d7c902e7cc604e01bb6dfe7616b6441. When fixed we can remove that.

@jchappelow jchappelow self-assigned this Mar 18, 2024
@jchappelow jchappelow linked a pull request Mar 20, 2024 that will close this issue
@jchappelow
Copy link
Member Author

Resolved by #610

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

Successfully merging a pull request may close this issue.

1 participant