fix(server): use CometBFT fallback for `db_backend` #17181

k-yang · 2023-07-28T20:54:13Z

Description

Our team uses rocksdb as the underlying database, but the application still wrote LevelDB files under data/application.db. Our config.toml file had db_backend = "rocksdb" and the documentation in app.toml says the following:

# AppDBBackend defines the database backend type to use for the application and snapshots DBs.
# An empty string indicates that a fallback will be used.
# First fallback is the deprecated compile-time types.DBBackend value.
# Second fallback (if the types.DBBackend also isn't set), is the db-backend value set in Tendermint's config.toml.
app-db-backend = ""

There are two things wrong with this comment:

We set the legacy -X github.com/cosmos/cosmos-sdk/types.DBBackend=rocksdb linker flag at build time, but it had no effect. Just searching the cosmos-sdk/types directory shows no DBBackend variable anymore, so it was removed. The TOML comment is thus outdated, and this PR removes that misleading comment.
The CosmosSDK application doesn't actually use the db_backend value from config.toml, due to a typo in the lookup key. When running the startup code through a debugger, we saw that the Viper config contains a db_backend field, but the code is looking for a db-backend field (note the underscore and hyphen mismatch). Please see the screenshot below. This PR fixes the lookup and fixes the typo in the TOML comment.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

julienrbrt

Nice find!

julienrbrt · 2023-07-28T22:11:36Z

server/config/toml.go

@@ -79,8 +79,7 @@ iavl-disable-fastnode = {{ .BaseConfig.IAVLDisableFastNode }}

 # AppDBBackend defines the database backend type to use for the application and snapshots DBs.
 # An empty string indicates that a fallback will be used.
-# First fallback is the deprecated compile-time types.DBBackend value.
-# Second fallback (if the types.DBBackend also isn't set), is the db-backend value set in CometBFT's config.toml.
+# The fallback is the db_backend value set in CometBFT's config.toml.


Could you update this comment in tools/confix/data/v0.46-app.toml, v0.47-app.toml and v0.50-app.toml?

julienrbrt · 2023-07-28T22:14:18Z

CHANGELOG.md

@@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (baseapp) [#17159](https://github.com/cosmos/cosmos-sdk/pull/17159) Validators can propose blocks that exceed the gas limit.
 * (x/group) [#17146](https://github.com/cosmos/cosmos-sdk/pull/17146) Rename x/group legacy ORM package's error codespace from "orm" to "legacy_orm", preventing collisions with ORM's error codespace "orm".
 * (x/bank) [#17170](https://github.com/cosmos/cosmos-sdk/pull/17170) Avoid empty spendable error message on send coins.
+* (db_backend) [#17181](https://github.com/cosmos/cosmos-sdk/pull/17181) Fix `db_backend` lookup fallback from `config.toml`


Suggested change

* (db_backend) [#17181](https://github.com/cosmos/cosmos-sdk/pull/17181) Fix `db_backend` lookup fallback from `config.toml`

* (server) [#17181](https://github.com/cosmos/cosmos-sdk/pull/17181) Fix `db_backend` lookup fallback from `config.toml`.

odeke-em

Thank you for this fix @k-yang. Just chiming in after review to kindly request that we add a simple test that checks that the different backends are correctly returned otherwise this change can regress quietly and corrupt your data silently as y'all found before.

julienrbrt · 2023-08-08T20:40:40Z

@k-yang We would gladly include this fix in our following patch releases, would you be up to implement the comments to make this PR good to go?

see: cosmos#17181

julienrbrt · 2023-08-16T11:52:34Z

Thank you for your contribution. I am going to take over this and make the required changes.
We will include this fix in our next patch releases that are coming very soon.

* feat: add before and after ModuleEndBlock * feat: add pruning-start-height flag to prune cmd * fix: db_backend lookup in config.toml see: cosmos#17181 * add refund-handler to base-app * fix bug with refundHandler msCache * append refund events to anteEvents --------- Co-authored-by: yan-soon <soonxiangyan@gmail.com>

see: cosmos#17181

* Add liquidity provider fee pool * Add deliver tx and before commit hooks * Implements Valuer and Scanner interface for Int and Dec * Add after committer hook * Add liquidity provider fee pool to module account invariant * Add liquidity provider fee pool to distribution genesis check * Revert "Emit anthandler events even if txn fails or panics" This reverts commit c5adf7a. * Fix deliverTxer not including ante events on failed txns * Change authz module name from const to var * feat: increase DefaultGasLimit for cli tx * Add msg handler middleware * add: Hooks for send keeper * fix: Typos * fix: Type error * add error handling to hooks * add IsModuleAccount function in AccountKeeper to check if a supplied address is a module account. * add transfer hooks BeforeMultiSend and AfterMultiSend for multisend functionality * fix make install issue * remove unneeded hooks * Delete version specifally * Delete version * Panic on error * Add logs * Hardcode version delete * Force overwrite * Update iavl * Force delete version * Fix ver deletion * Rollback properly * Fix merge regressions * Fix rollback cmd * add evm mapping logic in accountKeeper and bankKeeper * edit getAccount to stay in line with initial cosmos sdk contract, ie return nil when address is nil instead of panic * revert HasAccount to its original definition in case future libraries/code uses it. set function which checks for account with mapping condition included name to be AccountExists instead * reverts commit b45c030 "revert HasAccount to its original definition in case future libraries/code uses it." * create GetMappedAccountAddressIfExists helper method in accountKeeper * refactor GetMappedAccountAddressIfExists to GetMergedAccountAddressIfExists and remove GetMappedAccountAddressIfExists in BaseSendKeeper * remove getting merged account logic in getAllBalances as it is already present in IterateAccountBalances * code clean up * move extraction of address from key to be in app logic rather than in accountKeeper * Revert "move extraction of address from key to be in app logic rather than in accountKeeper" This reverts commit e7af659. * add guard for iteration of EthAddressToCosmosAddress and CosmosAddressToEthAddressKey to prevent panic * update iavl * Code Refactoring: Refactor telemetry to begin and end block (#316) * Remove hardcoded telemetry from modules * Add telemetry to begin/endblock * Remove test labels * add GetMappedAddress helper method * ensure that event emitted from transfer of funds is from mapped address * Add telemetry to GRPC handler (#320) * feat/api node for off oracle service [WIP] (#333) * Add new address field to `app.toml` * Refactor typo * Refactor naming * Update oracle address port * add refund-handler to base-app * fix bug with refundHandler msCache * append refund events to anteEvents * feat: add before and after ModuleEndBlock * fix: db_backend lookup in config.toml see: cosmos#17181 * feat: telemetry monitor-store-size-interval config * fix conflicts from cherry-picking * add back legacy router to handle legacy sdk.msg routing * update proto related files * Revert "update proto related files" This reverts commit 8145796. * update distribution proto * add back value and scanner interface for LegacyDec * update cosmossdk.io/math pseudo ver * feat: add pruning-start-height flag to prune cmd * update distribution proto and rerun make proto-gen * update distribution proto and rerun make proto-gen again * change begin and end blockers for gov, staking and upgrade modules to accept ptr keeper instead * remove duplicate pruning cmd flag * use cosmos iavl instead of our fork * remove previous forked rollback code * update cometbft to v0.38.0 to fully utilise ABCI++ * Revert "update cometbft to v0.38.0 to fully utilise ABCI++" This reverts commit 57a9676. --------- Co-authored-by: Ivan Poon <ravenxce@gmail.com> Co-authored-by: Lee Yik Jiun <yikjiun.lee@switcheo.network> Co-authored-by: holyxiaoxin <jr.loves.maths@gmail.com> Co-authored-by: Ivan Poon <ivan.poon@switcheo.network> Co-authored-by: PC <spclight@gmail.com> Co-authored-by: Randy <randy75828@gmail.com> Co-authored-by: lance.lan <lmengsplist96@gmail.com> Co-authored-by: Nicholas Chung <84615041+chungnicholas@users.noreply.github.com>

k-yang added 3 commits July 28, 2023 16:45

fix: use correct config key for db_backend

3c5a26e

chore: update toml comment

c42a5f1

Update CHANGELOG.md

a556e8f

k-yang marked this pull request as ready for review July 28, 2023 21:02

k-yang requested a review from a team as a code owner July 28, 2023 21:02

k-yang changed the title ~~fix: use correct config key for db_backend~~ fix: use CometBFT fallback for db_backend Jul 28, 2023

julienrbrt added backport/0.46.x PR scheduled for inclusion in the v0.46's next stable release backport/v0.47.x PR scheduled for inclusion in the v0.47's next stable release backport/v0.50.x PR scheduled for inclusion in the v0.50's next stable release labels Jul 28, 2023

julienrbrt approved these changes Jul 28, 2023

View reviewed changes

julienrbrt changed the title ~~fix: use CometBFT fallback for db_backend~~ fix(server): use CometBFT fallback for db_backend Jul 28, 2023

julienrbrt reviewed Jul 28, 2023

View reviewed changes

julienrbrt mentioned this pull request Jul 30, 2023

ci: skip fix_registration.go when using LINT_DIFF=1 #17185

Merged

20 tasks

julienrbrt self-assigned this Jul 30, 2023

odeke-em requested changes Jul 31, 2023

View reviewed changes

holyxiaoxin added a commit to Switcheo/cosmos-sdk that referenced this pull request Aug 16, 2023

fix: db_backend lookup in config.toml

413a745

see: cosmos#17181

holyxiaoxin added a commit to Switcheo/cosmos-sdk that referenced this pull request Aug 16, 2023

fix: db_backend lookup in config.toml

82201f5

see: cosmos#17181

julienrbrt closed this Aug 16, 2023

julienrbrt mentioned this pull request Aug 16, 2023

fix: use correct config key for db_backend #17406

Merged

20 tasks

PoisonPhang mentioned this pull request Sep 12, 2023

chore: update cosmos-sdk to v0.47.5 unionlabs/union#689

Merged

yan-soon pushed a commit to Switcheo/cosmos-sdk that referenced this pull request Sep 19, 2023

fix: db_backend lookup in config.toml

ff36c94

see: cosmos#17181

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(server): use CometBFT fallback for `db_backend` #17181

fix(server): use CometBFT fallback for `db_backend` #17181

k-yang commented Jul 28, 2023 •

edited

Loading

julienrbrt left a comment

julienrbrt Jul 28, 2023

julienrbrt Jul 28, 2023

odeke-em left a comment

julienrbrt commented Aug 8, 2023

julienrbrt commented Aug 16, 2023

	* (db_backend) [#17181](https://github.com/cosmos/cosmos-sdk/pull/17181) Fix `db_backend` lookup fallback from `config.toml`
	* (server) [#17181](https://github.com/cosmos/cosmos-sdk/pull/17181) Fix `db_backend` lookup fallback from `config.toml`.

fix(server): use CometBFT fallback for db_backend #17181

fix(server): use CometBFT fallback for db_backend #17181

Conversation

k-yang commented Jul 28, 2023 • edited Loading

Description

Author Checklist

Reviewers Checklist

julienrbrt left a comment

Choose a reason for hiding this comment

julienrbrt Jul 28, 2023

Choose a reason for hiding this comment

julienrbrt Jul 28, 2023

Choose a reason for hiding this comment

odeke-em left a comment

Choose a reason for hiding this comment

julienrbrt commented Aug 8, 2023

julienrbrt commented Aug 16, 2023

fix(server): use CometBFT fallback for `db_backend` #17181

fix(server): use CometBFT fallback for `db_backend` #17181

k-yang commented Jul 28, 2023 •

edited

Loading