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

fix(baseapp): allow empty consensus params #18531

Merged
merged 4 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

* (client/keys) [#18562](https://github.com/cosmos/cosmos-sdk/pull/18562) `keys delete` won't terminate when a key is not found
* (server) [#18537](https://github.com/cosmos/cosmos-sdk/pull/18537) Fix panic when defining minimum gas config as `100stake;100uatom`. Use a `,` delimiter instead of `;`. Fixes the server config getter to use the correct delimiter.
* [#18531](https://github.com/cosmos/cosmos-sdk/pull/18531) Baseapp's `GetConsensusParams` returns an empty struct instead of panicking if no params are found.
* (client/tx) [#18472](https://github.com/cosmos/cosmos-sdk/pull/18472) Utilizes the correct Pubkey when simulating a transaction.
* (baseapp) [#18486](https://github.com/cosmos/cosmos-sdk/pull/18486) Fixed FinalizeBlock calls not being passed to ABCIListeners
* (baseapp) [#18383](https://github.com/cosmos/cosmos-sdk/pull/18383) Fixed a data race inside BaseApp.getContext, found by end-to-end (e2e) tests.
Expand Down
6 changes: 5 additions & 1 deletion baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,11 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) cmtproto.ConsensusParams

cp, err := app.paramStore.Get(ctx)
if err != nil {
panic(fmt.Errorf("consensus key is nil: %w", err))
// This could happen while migrating from v0.45/v0.46 to v0.50, we should
// allow it to happen so during preblock the upgrade plan can be executed
// and the consensus params set for the first time in the new format.
app.logger.Error("failed to get consensus params", "err", err)
return cmtproto.ConsensusParams{}
Comment on lines 522 to +529
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change potentially affects state.

Call sequence:

(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).GetConsensusParams (baseapp/baseapp.go:518)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).PrepareProposal (baseapp/baseapp.go:402)

}

return cp
Expand Down
11 changes: 11 additions & 0 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,17 @@ func TestGetMaximumBlockGas(t *testing.T) {
require.Panics(t, func() { suite.baseApp.GetMaximumBlockGas(ctx) })
}

func TestGetEmptyConsensusParams(t *testing.T) {
suite := NewBaseAppSuite(t)
_, err := suite.baseApp.InitChain(&abci.RequestInitChain{})
require.NoError(t, err)
ctx := suite.baseApp.NewContext(true)

cp := suite.baseApp.GetConsensusParams(ctx)
require.Equal(t, cmtproto.ConsensusParams{}, cp)
require.Equal(t, uint64(0), suite.baseApp.GetMaximumBlockGas(ctx))
}
Comment on lines +723 to +732
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test TestGetEmptyConsensusParams should also verify that an error is logged when no consensus parameters are found, as described in the PR summary. Consider using a mock logger to capture and assert the log output.


func TestLoadVersionPruning(t *testing.T) {
logger := log.NewNopLogger()
pruningOptions := pruningtypes.NewCustomPruningOptions(10, 15)
Expand Down
Loading