Skip to content

Commit

Permalink
fix(baseapp): audit changes (backport #16596) (#16783)
Browse files Browse the repository at this point in the history
Co-authored-by: Facundo Medica <[email protected]>
Co-authored-by: Facundo Medica <[email protected]>
Co-authored-by: Julien Robert <[email protected]>
  • Loading branch information
4 people authored Jun 30, 2023
1 parent c256d51 commit cf2dca6
Show file tree
Hide file tree
Showing 8 changed files with 640 additions and 76 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [#16639](https://github.com/cosmos/cosmos-sdk/pull/16639) Make sure we don't execute blocks beyond the halt height.
* (x/auth/vesting) [#16733](https://github.com/cosmos/cosmos-sdk/pull/16733) Panic on overflowing and negative EndTimes when creating a PeriodicVestingAccount.
* (baseapp) [#16700](https://github.com/cosmos/cosmos-sdk/pull/16700) Fix consensus failure in returning no response to malformed transactions.
* (baseapp) [#16596](https://github.com/cosmos/cosmos-sdk/pull/16596) Return error during ExtendVote and VerifyVoteExtension if the request height is earlier than `VoteExtensionsEnableHeight`.

### API Breaking Changes

Expand Down
17 changes: 17 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,23 @@ Additionally, the SDK is starting its abstraction from CometBFT Go types thoroug
* The usage of CometBFT have been replaced to use the Cosmos SDK logger interface (`cosmossdk.io/log.Logger`).
* The usage of `github.com/cometbft/cometbft/libs/bytes.HexByte` have been replaced by `[]byte`.

#### Enable Vote Extensions

:::tip
This is an optional feature that is disabled by default.
:::

Once all the code changes required to implement Vote Extensions are in place,
they can be enabled by setting the consensus param `Abci.VoteExtensionsEnableHeight`
to a value greater than zero.

In a new chain, this can be done in the `genesis.json` file.

For existing chains this can be done in two ways:

* During an upgrade the value is set in an upgrade handler.
* A governance proposal that changes the consensus param **after a coordinated upgrade has taken place**.

### BaseApp

All ABCI methods now accept a pointer to the request and response types defined
Expand Down
9 changes: 7 additions & 2 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,9 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (
// If vote extensions are not enabled, as a safety precaution, we return an
// error.
cp := app.GetConsensusParams(app.voteExtensionState.ctx)
if cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight <= 0 {

extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0
if !extsEnabled {
return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height)
}

Expand All @@ -569,6 +571,7 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (
WithHeaderInfo(coreheader.Info{
ChainID: app.chainID,
Height: req.Height,
Hash: req.Hash,
})

// add a deferred recover handler in case extendVote panics
Expand Down Expand Up @@ -607,7 +610,9 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r
// If vote extensions are not enabled, as a safety precaution, we return an
// error.
cp := app.GetConsensusParams(app.voteExtensionState.ctx)
if cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight <= 0 {

extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0
if !extsEnabled {
return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to VerifyVoteExtension at height %d", req.Height)
}

Expand Down
Loading

0 comments on commit cf2dca6

Please sign in to comment.