feat(types): Deprecate the DBBackend variable in favor of new app-db-…

…backend config entry (#11188)

* [10948]: Add changelog entry.

* [10948]: Deprecate the types.DBBackend variable and the NewLevelDB function. Create a NewDB function to replace them.

* [10948]: Add a DBBackend string to the simulation config and a flag for setting it. Update the simulation setup to use that instead of the compile-time DBBackend variable.

* [10948]: Update the mock app creator to use the NewDB function. Not sure what to do about the db backend in that case though.

* [10948]: Update changelog to reflect new db-backend field name.

* [10948]: Use the tendermint db-backend type for the snapshot db.

* [10948]: Update the last use of NewLevelDB by adding a parameter to openDB and uppdating calls to that to provide the db type to use.

* [10948]: Upddate the NewDB function to also have a default db backend type if an empty string is provided there.

* [10948]: Remove the new TODO in mock.NewApp. After looking through it's uses, there doesn't seem to be any desire to change it, and there's no easy way to communicate it.

* [10948]: Enhance the NewDB defer function to also add info to any err that is being returned.

* [10948]: Add some unit tests for NewDB.

* [10948]: Lint fixes.

* [10948]: Add a changelog entry to the deprecated section.

* [10948]: Update the makefile to no longer set the types.DBBackend value.

* [10948]: Use memdb for the mock app instead of goleveldb. I know it was a goleveldb before, but for a mock app, a memdb feels like a better choice (assuming 'mock' and 'mem' mean what I assume they mean).

* [10948]: Fix the store benchmark tests (had some index-out-of-range issues).

* [10948]: Fix cachekv store bench test calling iter.Key() before checking iter.Valid().

* [10948]: Remove the panic recovery from types.NewDB since dbm.NewDB returns an error now (it didn't originally, when NewLevelDB was first written).

* [10948]: Add changlog entry indicationg an API breaking change due to the DBBackend change.

* [10948]: Get rid of the types.NewDB function in favor of just using the tm-db version of it.

* [10948]: Fix Update the codeql-analysis github action to use go v1.17.

* [10948]: Add config file option for the app db backend type.

* [10948]: Adjust the comment on the app-db-backend config entry to clarify fallback behavior.

* [10948]: Add a default of GoLevelDBBackend to GetAppDBBackend. The old DBBackend variable defaulted to that, and some unit tests assume that behavior still exists.

* [10948]: Add the missing quotes around the app-db-backend value.

* [10948]: Small tweak to the changelog's deprecated entry.

* Add the go version declaration back into the codeql-analysis github action.

* [10948]: Update new use of openDB.

* [10948]: Put a brief delay after closing the test network. Hopefully that helps with address-in-use and non-empty directory errors.

Co-authored-by: Marko <marbar3778@yahoo.com>

.github/workflows/codeql-analysis.yml

@@ -17,6 +17,9 @@ jobs:
     steps:
     - name: Checkout repository
       uses: actions/checkout@v3
+    - uses: actions/setup-go@v3
+      with:
+        go-version: 1.17
 
     - uses: actions/setup-go@v3
       with:

CHANGELOG.md

@@ -71,6 +71,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [#11179](https://github.com/cosmos/cosmos-sdk/pull/11179) Add state rollback command.
 * [\#10794](https://github.com/cosmos/cosmos-sdk/pull/10794) ADR-040: Add State Sync to V2 Store
 * [\#11234](https://github.com/cosmos/cosmos-sdk/pull/11234) Add `GRPCClient` field to Client Context. If `GRPCClient` field is set to nil, the `Invoke` method would use ABCI query, otherwise use gprc.
+* (types) [\#10948](https://github.com/cosmos/cosmos-sdk/issues/10948) Add `app-db-backend` to the `app.toml` config to replace the compile-time `types.DBbackend` variable.
 
 ### API Breaking Changes
 
@@ -131,7 +132,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [\#10816](https://github.com/cosmos/cosmos-sdk/pull/10816) Reuse blocked addresses from the bank module. No need to pass them to distribution. 
 * [\#10852](https://github.com/cosmos/cosmos-sdk/pull/10852) Move `x/gov/types` to `x/gov/types/v1beta2`.
 * [\#10922](https://github.com/cosmos/cosmos-sdk/pull/10922), [/#10957](https://github.com/cosmos/cosmos-sdk/pull/10957) Move key `server.Generate*` functions to testutil and support custom mnemonics in in-process testing network. Moved `TestMnemonic` from `testutil` package to `testdata`.
-* (x/bank) [\#10771](https://github.com/cosmos/cosmos-sdk/pull/10771) Add safety check on bank module perms to allow module-specific mint restrictions (e.g. only minting a certain denom).* (x/bank) [\#10771](https://github.com/cosmos/cosmos-sdk/pull/10771) Add `bank.BaseKeeper.WithMintCoinsRestriction` function to restrict use of bank `MintCoins` usage.
+* (x/bank) [\#10771](https://github.com/cosmos/cosmos-sdk/pull/10771) Add safety check on bank module perms to allow module-specific mint restrictions (e.g. only minting a certain denom).
+* (x/bank) [\#10771](https://github.com/cosmos/cosmos-sdk/pull/10771) Add `bank.BaseKeeper.WithMintCoinsRestriction` function to restrict use of bank `MintCoins` usage.
 * [\#10868](https://github.com/cosmos/cosmos-sdk/pull/10868), [\#10989](https://github.com/cosmos/cosmos-sdk/pull/10989) The Gov keeper accepts now 2 more mandatory arguments, the ServiceMsgRouter and a maximum proposal metadata length.
 * [\#10868](https://github.com/cosmos/cosmos-sdk/pull/10868), [\#10989](https://github.com/cosmos/cosmos-sdk/pull/10989), [\#11093](https://github.com/cosmos/cosmos-sdk/pull/11093) The Gov keeper accepts now 2 more mandatory arguments, the ServiceMsgRouter and a gov Config including the max metadata length.
 * [\#11124](https://github.com/cosmos/cosmos-sdk/pull/11124) Add `GetAllVersions` to application store
@@ -250,6 +252,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Deprecated
 
 * (x/upgrade) [\#9906](https://github.com/cosmos/cosmos-sdk/pull/9906) Deprecate `UpgradeConsensusState` gRPC query since this functionality is only used for IBC, which now has its own [IBC replacement](https://github.com/cosmos/ibc-go/blob/2c880a22e9f9cc75f62b527ca94aa75ce1106001/proto/ibc/core/client/v1/query.proto#L54)
+* (types) [\#10948](https://github.com/cosmos/cosmos-sdk/issues/10948) Deprecate the types.DBBackend variable and types.NewLevelDB function. They are replaced by a new entry in `app.toml`: `app-db-backend` and `tendermint/tm-db`s `NewDB` function. If `app-db-backend` is defined, then it is used. Otherwise, if `types.DBBackend` is defined, it is used (until removed: [\#11241](https://github.com/cosmos/cosmos-sdk/issues/11241)). Otherwise, Tendermint config's `db-backend` is used.
 
 ## [v0.45.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.0) - 2022-01-18
 

Makefile

@@ -46,10 +46,6 @@ ifeq ($(LEDGER_ENABLED),true)
   endif
 endif
 
-ifeq (cleveldb,$(findstring cleveldb,$(COSMOS_BUILD_OPTIONS)))
-  build_tags += gcc
-endif
-
 ifeq (secp,$(findstring secp,$(COSMOS_BUILD_OPTIONS)))
   build_tags += libsecp256k1_sdk
 endif
@@ -77,10 +73,9 @@ endif
 
 # DB backend selection
 ifeq (cleveldb,$(findstring cleveldb,$(COSMOS_BUILD_OPTIONS)))
-  ldflags += -X github.com/cosmos/cosmos-sdk/types.DBBackend=cleveldb
+  build_tags += gcc
 endif
 ifeq (badgerdb,$(findstring badgerdb,$(COSMOS_BUILD_OPTIONS)))
-  ldflags += -X github.com/cosmos/cosmos-sdk/types.DBBackend=badgerdb
   BUILD_TAGS += badgerdb
 endif
 # handle rocksdb
@@ -90,12 +85,10 @@ ifeq (rocksdb,$(findstring rocksdb,$(COSMOS_BUILD_OPTIONS)))
   endif
   CGO_ENABLED=1
   BUILD_TAGS += rocksdb
-  ldflags += -X github.com/cosmos/cosmos-sdk/types.DBBackend=rocksdb
 endif
 # handle boltdb
 ifeq (boltdb,$(findstring boltdb,$(COSMOS_BUILD_OPTIONS)))
   BUILD_TAGS += boltdb
-  ldflags += -X github.com/cosmos/cosmos-sdk/types.DBBackend=boltdb
 endif
 
 ifeq (,$(findstring nostrip,$(COSMOS_BUILD_OPTIONS)))

server/config/config.go

@@ -70,6 +70,10 @@ type BaseConfig struct {
 	IndexEvents []string `mapstructure:"index-events"`
 	// IavlCacheSize set the size of the iavl tree cache.
 	IAVLCacheSize uint64 `mapstructure:"iavl-cache-size"`
+
+	// AppDBBackend defines the type of Database to use for the application and snapshots databases.
+	// An empty string indicates that the Tendermint config's DBBackend value should be used.
+	AppDBBackend string `mapstructure:"app-db-backend"`
 }
 
 // APIConfig defines the API listener configuration.
@@ -210,6 +214,7 @@ func DefaultConfig() *Config {
 			MinRetainBlocks:   0,
 			IndexEvents:       make([]string, 0),
 			IAVLCacheSize:     781250, // 50 MB
+			AppDBBackend:      "",
 		},
 		Telemetry: telemetry.Config{
 			Enabled:      false,
@@ -269,6 +274,7 @@ func GetConfig(v *viper.Viper) Config {
 			IndexEvents:       v.GetStringSlice("index-events"),
 			MinRetainBlocks:   v.GetUint64("min-retain-blocks"),
 			IAVLCacheSize:     v.GetUint64("iavl-cache-size"),
+			AppDBBackend:      v.GetString("app-db-backend"),
 		},
 		Telemetry: telemetry.Config{
 			ServiceName:             v.GetString("telemetry.service-name"),

server/config/toml.go

@@ -75,6 +75,12 @@ index-events = [{{ range .BaseConfig.IndexEvents }}{{ printf "%q, " . }}{{end}}]
 # Default cache size is 50mb.
 iavl-cache-size = {{ .BaseConfig.IAVLCacheSize }}
 
+# 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 = "{{ .BaseConfig.AppDBBackend }}"
+
 ###############################################################################
 ###                         Telemetry Configuration                         ###
 ###############################################################################

server/constructors_test.go

@@ -5,11 +5,13 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+
+	dbm "github.com/tendermint/tm-db"
 )
 
 func Test_openDB(t *testing.T) {
 	t.Parallel()
-	_, err := openDB(t.TempDir())
+	_, err := openDB(t.TempDir(), dbm.GoLevelDBBackend)
 	require.NoError(t, err)
 }
 

server/export.go

@@ -37,7 +37,7 @@ func ExportCmd(appExporter types.AppExporter, defaultNodeHome string) *cobra.Com
 				return err
 			}
 
-			db, err := openDB(config.RootDir)
+			db, err := openDB(config.RootDir, GetAppDBBackend(serverCtx.Viper))
 			if err != nil {
 				return err
 			}

server/mock/app.go

@@ -9,6 +9,7 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/libs/log"
 	"github.com/tendermint/tendermint/types"
+	dbm "github.com/tendermint/tm-db"
 
 	bam "github.com/cosmos/cosmos-sdk/baseapp"
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -34,7 +35,7 @@ func testTxHandler(options middleware.TxHandlerOptions) tx.Handler {
 // similar to a real app. Make sure rootDir is empty before running the test,
 // in order to guarantee consistent results
 func NewApp(rootDir string, logger log.Logger) (abci.Application, error) {
-	db, err := sdk.NewLevelDB("mock", filepath.Join(rootDir, "data"))
+	db, err := dbm.NewDB("mock", dbm.MemDBBackend, filepath.Join(rootDir, "data"))
 	if err != nil {
 		return nil, err
 	}

server/rollback.go

@@ -26,7 +26,7 @@ application.
 			ctx := GetServerContextFromCmd(cmd)
 			cfg := ctx.Config
 			home := cfg.RootDir
-			db, err := openDB(home)
+			db, err := openDB(home, GetAppDBBackend(ctx.Viper))
 			if err != nil {
 				return err
 			}

server/start.go

@@ -171,7 +171,7 @@ func startStandAlone(ctx *Context, appCreator types.AppCreator) error {
 	transport := ctx.Viper.GetString(flagTransport)
 	home := ctx.Viper.GetString(flags.FlagHome)
 
-	db, err := openDB(home)
+	db, err := openDB(home, GetAppDBBackend(ctx.Viper))
 	if err != nil {
 		return err
 	}
@@ -230,12 +230,12 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App
 		}
 	}
 
-	traceWriterFile := ctx.Viper.GetString(flagTraceStore)
-	db, err := openDB(home)
+	db, err := openDB(home, GetAppDBBackend(ctx.Viper))
 	if err != nil {
 		return err
 	}
 
+	traceWriterFile := ctx.Viper.GetString(flagTraceStore)
 	traceWriter, err := openTraceWriter(traceWriterFile)
 	if err != nil {
 		return err

server/util.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
+	"github.com/spf13/cast"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
@@ -348,6 +349,21 @@ func WaitForQuitSignals() ErrorCode {
 	return ErrorCode{Code: int(sig.(syscall.Signal)) + 128}
 }
 
+// GetAppDBBackend gets the backend type to use for the application DBs.
+func GetAppDBBackend(opts types.AppOptions) dbm.BackendType {
+	rv := cast.ToString(opts.Get("app-db-backend"))
+	if len(rv) == 0 {
+		rv = sdk.DBBackend
+	}
+	if len(rv) == 0 {
+		rv = cast.ToString(opts.Get("db-backend"))
+	}
+	if len(rv) != 0 {
+		return dbm.BackendType(rv)
+	}
+	return dbm.GoLevelDBBackend
+}
+
 func skipInterface(iface net.Interface) bool {
 	if iface.Flags&net.FlagUp == 0 {
 		return true // interface down
@@ -372,9 +388,9 @@ func addrToIP(addr net.Addr) net.IP {
 	return ip
 }
 
-func openDB(rootDir string) (dbm.DB, error) {
+func openDB(rootDir string, backendType dbm.BackendType) (dbm.DB, error) {
 	dataDir := filepath.Join(rootDir, "data")
-	return sdk.NewLevelDB("application", dataDir)
+	return dbm.NewDB("application", backendType, dataDir)
 }
 
 func openTraceWriter(traceWriterFile string) (w io.Writer, err error) {

server/util_test.go

@@ -11,14 +11,18 @@ import (
 	"testing"
 
 	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	tmcfg "github.com/tendermint/tendermint/config"
+	dbm "github.com/tendermint/tm-db"
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/server"
 	"github.com/cosmos/cosmos-sdk/server/config"
+	servertypes "github.com/cosmos/cosmos-sdk/server/types"
 	"github.com/cosmos/cosmos-sdk/simapp"
+	"github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	genutilcli "github.com/cosmos/cosmos-sdk/x/genutil/client/cli"
 )
@@ -438,3 +442,84 @@ func TestEmptyMinGasPrices(t *testing.T) {
 	err = cmd.ExecuteContext(ctx)
 	require.Errorf(t, err, sdkerrors.ErrAppConfig.Error())
 }
+
+type mapGetter map[string]interface{}
+
+func (m mapGetter) Get(key string) interface{} {
+	return m[key]
+}
+
+var _ servertypes.AppOptions = mapGetter{}
+
+func TestGetAppDBBackend(t *testing.T) {
+	origDBBackend := types.DBBackend
+	defer func() {
+		types.DBBackend = origDBBackend
+	}()
+	tests := []struct {
+		name   string
+		dbBack string
+		opts   mapGetter
+		exp    dbm.BackendType
+	}{
+		{
+			name:   "nothing set",
+			dbBack: "",
+			opts:   mapGetter{},
+			exp:    dbm.GoLevelDBBackend,
+		},
+
+		{
+			name:   "only db-backend set",
+			dbBack: "",
+			opts:   mapGetter{"db-backend": "db-backend value 1"},
+			exp:    dbm.BackendType("db-backend value 1"),
+		},
+		{
+			name:   "only DBBackend set",
+			dbBack: "DBBackend value 2",
+			opts:   mapGetter{},
+			exp:    dbm.BackendType("DBBackend value 2"),
+		},
+		{
+			name:   "only app-db-backend set",
+			dbBack: "",
+			opts:   mapGetter{"app-db-backend": "app-db-backend value 3"},
+			exp:    dbm.BackendType("app-db-backend value 3"),
+		},
+
+		{
+			name:   "app-db-backend and db-backend set",
+			dbBack: "",
+			opts:   mapGetter{"db-backend": "db-backend value 4", "app-db-backend": "app-db-backend value 5"},
+			exp:    dbm.BackendType("app-db-backend value 5"),
+		},
+		{
+			name:   "app-db-backend and DBBackend set",
+			dbBack: "DBBackend value 6",
+			opts:   mapGetter{"app-db-backend": "app-db-backend value 7"},
+			exp:    dbm.BackendType("app-db-backend value 7"),
+		},
+		{
+			name:   "db-backend and DBBackend set",
+			dbBack: "DBBackend value 8",
+			opts:   mapGetter{"db-backend": "db-backend value 9"},
+			exp:    dbm.BackendType("DBBackend value 8"),
+		},
+
+		{
+			name:   "all of app-db-backend db-backend DBBackend set",
+			dbBack: "DBBackend value 10",
+			opts:   mapGetter{"db-backend": "db-backend value 11", "app-db-backend": "app-db-backend value 12"},
+			exp:    dbm.BackendType("app-db-backend value 12"),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(st *testing.T) {
+			types.DBBackend = tc.dbBack
+			act := server.GetAppDBBackend(tc.opts)
+			assert.Equal(st, tc.exp, act)
+		})
+	}
+}

simapp/config.go

@@ -22,6 +22,7 @@ var (
 	FlagCommitValue             bool
 	FlagOnOperationValue        bool // TODO: Remove in favor of binary search for invariant violation
 	FlagAllInvariantsValue      bool
+	FlagDBBackendValue          string
 
 	FlagEnabledValue     bool
 	FlagVerboseValue     bool
@@ -46,6 +47,7 @@ func GetSimulatorFlags() {
 	flag.BoolVar(&FlagCommitValue, "Commit", false, "have the simulation commit")
 	flag.BoolVar(&FlagOnOperationValue, "SimulateEveryOperation", false, "run slow invariants every operation")
 	flag.BoolVar(&FlagAllInvariantsValue, "PrintAllInvariants", false, "print all invariants if a broken invariant is found")
+	flag.StringVar(&FlagDBBackendValue, "DBBackend", "goleveldb", "custom db backend type")
 
 	// simulation flags
 	flag.BoolVar(&FlagEnabledValue, "Enabled", false, "enable the simulation")
@@ -71,5 +73,6 @@ func NewConfigFromFlags() simulation.Config {
 		Commit:             FlagCommitValue,
 		OnOperation:        FlagOnOperationValue,
 		AllInvariants:      FlagAllInvariantsValue,
+		DBBackend:          FlagDBBackendValue,
 	}
 }

simapp/simd/cmd/root.go

@@ -261,7 +261,7 @@ func (a appCreator) newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, a
 	}
 
 	snapshotDir := filepath.Join(cast.ToString(appOpts.Get(flags.FlagHome)), "data", "snapshots")
-	snapshotDB, err := sdk.NewLevelDB("metadata", snapshotDir)
+	snapshotDB, err := dbm.NewDB("metadata", server.GetAppDBBackend(appOpts), snapshotDir)
 	if err != nil {
 		panic(err)
 	}

simapp/utils.go

@@ -39,7 +39,7 @@ func SetupSimulation(dirPrefix, dbName string) (simtypes.Config, dbm.DB, string,
 		return simtypes.Config{}, nil, "", nil, false, err
 	}
 
-	db, err := sdk.NewLevelDB(dbName, dir)
+	db, err := dbm.NewDB(dbName, dbm.BackendType(config.DBBackend), dir)
 	if err != nil {
 		return simtypes.Config{}, nil, "", nil, false, err
 	}