diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cc91d2382686..04f0cf345e77d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (baseapp) [#18727](https://github.com/cosmos/cosmos-sdk/pull/18727) Ensure that BaseApp.Init firstly returns any errors from a nil commit multistore instead of panicking on nil dereferencing and before sealing the app. * (client) [#18622](https://github.com/cosmos/cosmos-sdk/pull/18622) Fixed a potential under/overflow from `uint64->int64` when computing gas fees as a LegacyDec. * (client/keys) [#18562](https://github.com/cosmos/cosmos-sdk/pull/18562) `keys delete` won't terminate when a key is not found. * (baseapp) [#18383](https://github.com/cosmos/cosmos-sdk/pull/18383) Fixed a data race inside BaseApp.getContext, found by end-to-end (e2e) tests. diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 87cd2294c1853..9afa9259d076b 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -424,16 +424,16 @@ func (app *BaseApp) Init() error { panic("cannot call initFromMainStore: baseapp already sealed") } + if app.cms == nil { + return errors.New("commit multi-store must not be nil") + } + emptyHeader := cmtproto.Header{ChainID: app.chainID} // needed for the export command which inits from store but never calls initchain app.setState(execModeCheck, emptyHeader) app.Seal() - if app.cms == nil { - return errors.New("commit multi-store must not be nil") - } - return app.cms.GetPruning().Validate() } diff --git a/baseapp/regression_test.go b/baseapp/regression_test.go new file mode 100644 index 0000000000000..77471eda4b4ae --- /dev/null +++ b/baseapp/regression_test.go @@ -0,0 +1,41 @@ +package baseapp + +import ( + "testing" + + dbm "github.com/cosmos/cosmos-db" + "github.com/stretchr/testify/require" + + "cosmossdk.io/log" + "cosmossdk.io/store" + storemetrics "cosmossdk.io/store/metrics" +) + +// Ensures that error checks are performed before sealing the app. +// Please see https://github.com/cosmos/cosmos-sdk/issues/18726 +func TestNilCmsCheckBeforeSeal(t *testing.T) { + app := new(BaseApp) + + // 1. Invoking app.Init with a nil cms MUST not seal the app + // and should return an error firstly, which can later be reversed. + for i := 0; i < 10; i++ { // N times, the app shouldn't be sealed. + err := app.Init() + require.Error(t, err) + require.Contains(t, err.Error(), "commit multi-store must not be nil") + require.False(t, app.IsSealed(), "the app MUST not be sealed") + } + + // 2. Now that we've figured out and gotten back an error, let's rectify the problem. + // and we should be able to set the commit multistore then reinvoke app.Init successfully! + db := dbm.NewMemDB() + logger := log.NewTestLogger(t) + app.cms = store.NewCommitMultiStore(db, logger, storemetrics.NewNoOpMetrics()) + err := app.Init() + require.Nil(t, err, "app.Init MUST now succeed") + require.True(t, app.IsSealed(), "the app must now be sealed") + + // 3. Now we should expect that panic. + require.Panics(t, func() { + _ = app.Init() + }) +}