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,txapp,kwild: new chain meta store #610

Merged
merged 2 commits into from
Mar 20, 2024
Merged

Conversation

jchappelow
Copy link
Member

@jchappelow jchappelow commented Mar 19, 2024

This change move's the ABCI application's meta datastore from a badger DB file to postgres.

The ABCI metadata store must contain the following for the current block processed by the application:

  • height
  • app hash

These data are required primarily to satisfy the Info method of the ABCI application, which is used by cometbft as both an integrity check and to signal if and where to start reapplying blocks to catch the application up to the node's internal block store. The new (*TxApp).ChainInfo method provides this information to the application and back to cometbft on startup.

These data are also required for InitChain to ensure that when cometbft is starting at genesis (empty block store), the application state does not include existing unexpected data, which can happen when resetting cometbft's data (rm -r ~/.kwild) but forgetting to drop kwild's postgresql database.

To make this change while supporting an upgrade from v0.7.0 nodes, it is necessary to attempt to load the data from an existing badger DB is the meta tables do not already exist in postgres. This is unfortunate, but necessary, and can be removed at some point when we are convinced everyone is migrated. Also note that it was not possible to use the existing kwil_voting.height table for this upgrade since it lacked app hash.

Although this change now allows the latest committed block height to be included with the main consensus DB transaction, it is still necessary to update the app hash following Precommit (and thus in a subsequent DB transactions). This is an improvement since it makes a surprise apphash mismatch with a peer node impossible on account of the non-atomic storage in the old badger DB, but there still exists a recognizable manual recovery scenario if the subsequent transaction fails to store the app hash. I believe this previous failure mode was the cause for the app hash mismatch I was diagnosing last week following a poorly time suspend of my laptop.


There are several less notable changes below

  • the kwild build process now initializes all of the data stores (accts, voting, meta) in a single db transaction that can be rolled back if any fail
  • an upgrade to the voting store is defined (version 1) that drops the obsolete height table. The query strings are removed
  • removed the common/sql.TxCloser interface since it is unused on it's own, although it was once relevant when Execute was not used as a method of a transaction and instead via the top level DB. We can factor away the pg.DB.tx field soon

@jchappelow jchappelow marked this pull request as draft March 19, 2024 22:55
@jchappelow jchappelow marked this pull request as ready for review March 20, 2024 04:12
@brennanjl brennanjl merged commit ab7e80f into kwilteam:main Mar 20, 2024
2 checks passed
@jchappelow jchappelow deleted the no-badger branch March 20, 2024 15:25
jchappelow added a commit that referenced this pull request Mar 22, 2024
* abci,txapp,kwild: new chain meta store

* pg: remove the TxCloser relic
jchappelow added a commit that referenced this pull request Mar 25, 2024
* abci,txapp,kwild: new chain meta store

* pg: remove the TxCloser relic
@jchappelow jchappelow added this to the v0.8.0 milestone May 2, 2024
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 this pull request may close these issues.

abci,pg: start using height/hash in postgres, remove badger db in abci/info
2 participants