From 4ded3677158e62e69b61d24c524eddd1dcb25c9d Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Tue, 16 Jul 2024 13:31:26 -0600 Subject: [PATCH] Remove the config warnings from the store loader. (#2095) * Remove the config warnings from the store loader. * Add changelog entry. --- CHANGELOG.md | 4 +- app/app.go | 7 +- app/store_loader.go | 91 ---------- app/store_loader_test.go | 359 --------------------------------------- 4 files changed, 5 insertions(+), 456 deletions(-) delete mode 100644 app/store_loader.go delete mode 100644 app/store_loader_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c77bca7be..41991f0cdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] -* nothing +### Improvements + +* Remove the warnings about some config settings [2095](https://github.com/provenance-io/provenance/pull/2095). --- diff --git a/app/app.go b/app/app.go index 9ad39f5931..85a741bb35 100644 --- a/app/app.go +++ b/app/app.go @@ -988,7 +988,7 @@ func New( // Register upgrade handlers and set the store loader. // This must be done after the module manager, configurator, and pre-blocker are set, // but before the baseapp is sealed via LoadLatestVersion() below. - app.registerUpgradeHandlers(appOpts) + app.registerUpgradeHandlers() if loadLatest { if err := app.LoadLatestVersion(); err != nil { @@ -1045,7 +1045,7 @@ func (app *App) setFeeHandler() { app.SetFeeHandler(msgFeeHandler) } -func (app *App) registerUpgradeHandlers(appOpts servertypes.AppOptions) { +func (app *App) registerUpgradeHandlers() { // Add the upgrade handlers for each release. InstallCustomUpgradeHandlers(app) @@ -1078,9 +1078,6 @@ func (app *App) registerUpgradeHandlers(appOpts servertypes.AppOptions) { if storeLoader == nil { storeLoader = baseapp.DefaultStoreLoader } - - // Verify configuration settings - storeLoader = ValidateWrapper(app.Logger(), appOpts, storeLoader) app.SetStoreLoader(storeLoader) } diff --git a/app/store_loader.go b/app/store_loader.go deleted file mode 100644 index e1f6b5579a..0000000000 --- a/app/store_loader.go +++ /dev/null @@ -1,91 +0,0 @@ -package app - -import ( - "fmt" - "os" - "strconv" - "strings" - "time" - - "github.com/spf13/cast" - - "cosmossdk.io/log" - storetypes "cosmossdk.io/store/types" - - dbm "github.com/cosmos/cosmos-db" - "github.com/cosmos/cosmos-sdk/baseapp" - "github.com/cosmos/cosmos-sdk/server" - servertypes "github.com/cosmos/cosmos-sdk/server/types" -) - -// ValidateWrapperSleeper is the sleeper that the ValidateWrapper will use. -// It primarily exists so it can be changed for unit tests on ValidateWrapper so they don't take so long. -var ValidateWrapperSleeper Sleeper = &DefaultSleeper{} - -// ValidateWrapper creates a new StoreLoader that first checks the config settings before calling the provided StoreLoader. -func ValidateWrapper(logger log.Logger, appOpts servertypes.AppOptions, storeLoader baseapp.StoreLoader) baseapp.StoreLoader { - return func(ms storetypes.CommitMultiStore) error { - IssueConfigWarnings(logger, appOpts, ValidateWrapperSleeper) - return storeLoader(ms) - } -} - -// Sleeper is an interface for something with a Sleep function. -type Sleeper interface { - Sleep(d time.Duration) -} - -// DefaultSleeper uses the time.Sleep function for sleeping. -type DefaultSleeper struct{} - -// Sleep is a wrapper for time.Sleep(d). -func (s DefaultSleeper) Sleep(d time.Duration) { - time.Sleep(d) -} - -// IssueConfigWarnings checks a few values in the configs and issues warnings and sleeps if appropriate. -func IssueConfigWarnings(logger log.Logger, appOpts servertypes.AppOptions, sleeper Sleeper) { - const MaxPruningInterval = 999 - const SleepSeconds = 30 - interval := cast.ToUint64(appOpts.Get("pruning-interval")) - txIndexer := cast.ToStringMap(appOpts.Get("tx_index")) - indexer := cast.ToString(txIndexer["indexer"]) - backend := server.GetAppDBBackend(appOpts) - var errs []string - - if interval > MaxPruningInterval { - errs = append(errs, fmt.Sprintf("pruning-interval %d EXCEEDS %d AND IS NOT RECOMMENDED, AS IT CAN LEAD TO MISSED BLOCKS ON VALIDATORS.", interval, MaxPruningInterval)) - } - - if indexer != "" && indexer != "null" { - errs = append(errs, fmt.Sprintf("indexer \"%s\" IS NOT RECOMMENDED, AND IT IS RECOMMENDED TO USE \"%s\".", indexer, "null")) - } - - if backend != dbm.GoLevelDBBackend { - errs = append(errs, fmt.Sprintf("%s IS NO LONGER SUPPORTED. MIGRATE TO %s.", backend, dbm.GoLevelDBBackend)) - } - - if len(errs) > 0 { - for _, err := range errs { - logger.Error(err) - } - if !HaveAckWarn() { - logger.Error(fmt.Sprintf("NODE WILL CONTINUE AFTER %d SECONDS.", SleepSeconds)) - logger.Error("This wait can be bypassed by fixing the above warnings or setting the PIO_ACKWARN environment variable to \"1\".") - sleeper.Sleep(SleepSeconds * time.Second) - } - } -} - -// HaveAckWarn returns true if the PIO_ACKWARN env var is set and isn't a false value (e.g. "0", "f" or "false"). -func HaveAckWarn() bool { - ackWarn := strings.TrimSpace(os.Getenv("PIO_ACKWARN")) - if len(ackWarn) == 0 { - return false - } - - rv, err := strconv.ParseBool(ackWarn) - // We return false only if it parsed successfully to a false value. - // If parsing failed or it parsed to a true value, we return true. - return err != nil || rv -} diff --git a/app/store_loader_test.go b/app/store_loader_test.go deleted file mode 100644 index b6a622d5de..0000000000 --- a/app/store_loader_test.go +++ /dev/null @@ -1,359 +0,0 @@ -package app - -import ( - "bytes" - "errors" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - storetypes "cosmossdk.io/store/types" - - "github.com/provenance-io/provenance/internal" - "github.com/provenance-io/provenance/testutil/assertions" -) - -// StoreLoaderMocker is a struct with a StoreLoader method that records that the store loader was called and returns a pre-determined error message. -type StoreLoaderMocker struct { - Called bool - ErrMsg string -} - -func NewStoreLoaderMocker(errMsg string) *StoreLoaderMocker { - return &StoreLoaderMocker{ - ErrMsg: errMsg, - } -} - -func (s *StoreLoaderMocker) StoreLoader(_ storetypes.CommitMultiStore) error { - s.Called = true - if len(s.ErrMsg) > 0 { - return errors.New(s.ErrMsg) - } - return nil -} - -// MockSleeper is a Sleeper that only records what sleep was requested (instead of actually sleeping). -type MockSleeper struct { - LastSleep time.Duration -} - -var _ Sleeper = (*MockSleeper)(nil) - -func NewMockSleeper() *MockSleeper { - return &MockSleeper{} -} - -func (s *MockSleeper) Sleep(d time.Duration) { - s.LastSleep = d -} - -// MockAppOptions is a mocked version of AppOpts that allows the developer to provide the pruning attribute. -type MockAppOptions struct { - pruning string - indexer string - db string -} - -// Get returns the value for the provided option. -func (m MockAppOptions) Get(opt string) interface{} { - switch opt { - case "pruning-interval": - return m.pruning - case "tx_index": - return map[string]interface{}{ - "indexer": m.indexer, - } - case "app-db-backend": - return m.db - case "db-backend": - return m.db - } - - return nil -} - -func TestValidateWrapper(t *testing.T) { - defer func() { - ValidateWrapperSleeper = &DefaultSleeper{} - }() - - recAppOpts := MockAppOptions{ - pruning: "10", - db: "goleveldb", - indexer: "null", - } - - tests := []struct { - name string - appOpts MockAppOptions - pioAckWarn bool - expErr string - expLogMsgs bool - expSleep bool - }{ - { - name: "empty opts", - appOpts: MockAppOptions{}, - expErr: "", - expLogMsgs: false, - expSleep: false, - }, - { - name: "bad config", - appOpts: MockAppOptions{ - pruning: "10000", - }, - expLogMsgs: true, - expSleep: true, - }, - { - name: "bad config no sleep", - appOpts: MockAppOptions{ - pruning: "10000", - }, - pioAckWarn: true, - expLogMsgs: true, - expSleep: false, - }, - { - name: "err from store loader", - appOpts: recAppOpts, - expErr: "injected test error", - }, - { - name: "bad config and err from store loader", - appOpts: MockAppOptions{ - db: "somethingelse", - }, - expErr: "another injected error for testing", - expLogMsgs: true, - expSleep: true, - }, - { - name: "bad config and err from store loader no sleep", - appOpts: MockAppOptions{ - db: "somethingelse", - }, - pioAckWarn: true, - expErr: "another injected error for testing", - expLogMsgs: true, - expSleep: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - sleeper := NewMockSleeper() - ValidateWrapperSleeper = sleeper - - if tc.pioAckWarn { - t.Setenv("PIO_ACKWARN", "1") - } - - var buffer bytes.Buffer - logger := internal.NewBufferedInfoLogger(&buffer) - slMocker := NewStoreLoaderMocker(tc.expErr) - storeLoader := ValidateWrapper(logger, tc.appOpts, slMocker.StoreLoader) - - var err error - testFunc := func() { - err = storeLoader(nil) - } - require.NotPanics(t, testFunc, "calling the storeLoader that was returned by ValidateWrapper") - assertions.AssertErrorValue(t, err, tc.expErr, "error from storeLoader") - - logMsgs := buffer.String() - if tc.expLogMsgs { - assert.NotEmpty(t, logMsgs, "messages logged during storeLoader") - } else { - assert.Empty(t, logMsgs, "messages logged during storeLoader") - } - - didSleep := sleeper.LastSleep != 0 - assert.Equal(t, tc.expSleep, didSleep, "whether sleep was called") - }) - } -} - -func TestIssueConfigWarnings(t *testing.T) { - sleepErr1 := "ERR NODE WILL CONTINUE AFTER 30 SECONDS." - sleepErr2 := "ERR This wait can be bypassed by fixing the above warnings or setting the PIO_ACKWARN environment variable to \"1\"." - - tests := []struct { - name string - appOpts MockAppOptions - pioAckWarn string - expLogLines []string // can be in any order, but all must be there. - expSleep bool - }{ - { - name: "no app opts", - appOpts: MockAppOptions{}, - expLogLines: nil, - }, - { - name: "recommended app opts", - appOpts: MockAppOptions{ - pruning: "10", - db: "goleveldb", - indexer: "null", - }, - expLogLines: nil, - }, - { - name: "bad pruning interval", - appOpts: MockAppOptions{ - pruning: "1000", - db: "goleveldb", - indexer: "null", - }, - expLogLines: []string{ - "ERR pruning-interval 1000 EXCEEDS 999 AND IS NOT RECOMMENDED, AS IT CAN LEAD TO MISSED BLOCKS ON VALIDATORS.", - sleepErr1, - sleepErr2, - }, - expSleep: true, - }, - { - name: "bad indexer", - appOpts: MockAppOptions{ - pruning: "10", - db: "goleveldb", - indexer: "kv", - }, - expLogLines: []string{ - "ERR indexer \"kv\" IS NOT RECOMMENDED, AND IT IS RECOMMENDED TO USE \"null\".", - sleepErr1, - sleepErr2, - }, - expSleep: true, - }, - { - name: "bad db", - appOpts: MockAppOptions{ - pruning: "10", - db: "otherdb", - indexer: "null", - }, - expLogLines: []string{ - "ERR otherdb IS NO LONGER SUPPORTED. MIGRATE TO goleveldb.", - sleepErr1, - sleepErr2, - }, - expSleep: true, - }, - { - name: "all bad with sleep", - appOpts: MockAppOptions{ - pruning: "1001", - db: "thisdb", - indexer: "psql", - }, - expLogLines: []string{ - "ERR pruning-interval 1001 EXCEEDS 999 AND IS NOT RECOMMENDED, AS IT CAN LEAD TO MISSED BLOCKS ON VALIDATORS.", - "ERR indexer \"psql\" IS NOT RECOMMENDED, AND IT IS RECOMMENDED TO USE \"null\".", - "ERR thisdb IS NO LONGER SUPPORTED. MIGRATE TO goleveldb.", - sleepErr1, - sleepErr2, - }, - expSleep: true, - }, - { - name: "all bad no sleep", - appOpts: MockAppOptions{ - pruning: "1001", - db: "thatdb", - indexer: "psql", - }, - pioAckWarn: "1", - expLogLines: []string{ - "ERR pruning-interval 1001 EXCEEDS 999 AND IS NOT RECOMMENDED, AS IT CAN LEAD TO MISSED BLOCKS ON VALIDATORS.", - "ERR indexer \"psql\" IS NOT RECOMMENDED, AND IT IS RECOMMENDED TO USE \"null\".", - "ERR thatdb IS NO LONGER SUPPORTED. MIGRATE TO goleveldb.", - }, - expSleep: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - var expSleepDur time.Duration - if tc.expSleep { - expSleepDur = 30 * time.Second - } - - if len(tc.pioAckWarn) > 0 { - t.Setenv("PIO_ACKWARN", tc.pioAckWarn) - } - var buffer bytes.Buffer - logger := internal.NewBufferedInfoLogger(&buffer) - sleeper := NewMockSleeper() - - testFunc := func() { - IssueConfigWarnings(logger, tc.appOpts, sleeper) - } - require.NotPanics(t, testFunc, "IssueConfigWarnings") - - loggedLines := internal.SplitLogLines(buffer.String()) - assert.ElementsMatch(t, tc.expLogLines, loggedLines, "Lines logged during IssueConfigWarnings. List A is the expected lines.") - actSleepDur := sleeper.LastSleep - assert.Equal(t, expSleepDur.String(), actSleepDur.String(), "sleep duration during IssueConfigWarnings") - }) - } -} - -func TestHaveAckWarn(t *testing.T) { - tests := []struct { - pioAckWarn string - noPioAckWarn bool - expected bool - }{ - {noPioAckWarn: true, expected: false}, - {pioAckWarn: "", expected: false}, - {pioAckWarn: " ", expected: false}, - {pioAckWarn: "0", expected: false}, - {pioAckWarn: " 0 ", expected: false}, - {pioAckWarn: "false", expected: false}, - {pioAckWarn: " False", expected: false}, - {pioAckWarn: "FALSE ", expected: false}, - {pioAckWarn: "f", expected: false}, - {pioAckWarn: " F ", expected: false}, - - {pioAckWarn: "1", expected: true}, - {pioAckWarn: "yes", expected: true}, - {pioAckWarn: "t", expected: true}, - {pioAckWarn: "true", expected: true}, - {pioAckWarn: " T", expected: true}, - {pioAckWarn: "TRUE ", expected: true}, - {pioAckWarn: " True ", expected: true}, - {pioAckWarn: "whatever", expected: true}, - {pioAckWarn: "X", expected: true}, - {pioAckWarn: "ff", expected: true}, - {pioAckWarn: "farse", expected: true}, - } - - for _, tc := range tests { - name := tc.pioAckWarn - if tc.noPioAckWarn { - name = "no PIO_ACKWARN set" - } - if len(name) == 0 { - name = "empty string" - } - - t.Run(name, func(t *testing.T) { - if !tc.noPioAckWarn { - t.Setenv("PIO_ACKWARN", tc.pioAckWarn) - } - var actual bool - testFunc := func() { - actual = HaveAckWarn() - } - require.NotPanics(t, testFunc, "HaveAckWarn") - assert.Equal(t, tc.expected, actual, "HaveAckWarn result") - }) - } -}