Skip to content

Commit

Permalink
fix: Dedup Vote Extensions in ValidateVoteExtensions (#18895)
Browse files Browse the repository at this point in the history 
(cherry picked from commit 5166c9f)

# Conflicts:
#	CHANGELOG.md
  • Loading branch information
@davidterpay @mergify
davidterpay authored and mergify[bot] committed Dec 27, 2023
1 parent f3e18df commit 1a4cad1
Show file tree
Hide file tree
Showing 4 changed files with 180 additions and 9 deletions.
116 changes: 116 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,122 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Bug Fixes

* (baseapp) [#18609](https://github.com/cosmos/cosmos-sdk/issues/18609) Fixed accounting in the block gas meter after BeginBlock and before DeliverTx, ensuring transaction processing always starts with the expected zeroed out block gas meter.
<<<<<<< HEAD
=======
* (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.
* (client/server) [#18345](https://github.com/cosmos/cosmos-sdk/pull/18345) Consistently set viper prefix in client and server. It defaults for the binary name for both client and server.
* (simulation) [#17911](https://github.com/cosmos/cosmos-sdk/pull/17911) Fix all problems with executing command `make test-sim-custom-genesis-fast` for simulation test.
* (simulation) [#18196](https://github.com/cosmos/cosmos-sdk/pull/18196) Fix the problem of `validator set is empty after InitGenesis` in simulation test.
* (baseapp) [#18551](https://github.com/cosmos/cosmos-sdk/pull/18551) Fix SelectTxForProposal the calculation method of tx bytes size is inconsistent with CometBFT
* (baseapp) [#18895](https://github.com/cosmos/cosmos-sdk/pull/18895) Fix de-duplicating vote extensions during validation in ValidateVoteExtensions.

### API Breaking Changes

* (server) [#18303](https://github.com/cosmos/cosmos-sdk/pull/18303) `x/genutil` now handles the application export. `server.AddCommands` does not take an `AppExporter` but instead `genutilcli.Commands` does.
* (x/gov/testutil) [#17986](https://github.com/cosmos/cosmos-sdk/pull/18036) `MsgDeposit` has been removed because of AutoCLI migration.
* (x/staking/testutil) [#17986](https://github.com/cosmos/cosmos-sdk/pull/17986) `MsgRedelegateExec`, `MsgUnbondExec` has been removed because of AutoCLI migration.
* (x/bank/testutil) [#17868](https://github.com/cosmos/cosmos-sdk/pull/17868) `MsgSendExec` has been removed because of AutoCLI migration.
* (app) [#17838](https://github.com/cosmos/cosmos-sdk/pull/17838) Params module was removed from simapp and all imports of the params module removed throughout the repo.
* The Cosmos SDK has migrated away from using params, if your app still uses it, then you can leave it plugged into your app
* (x/staking) [#17778](https://github.com/cosmos/cosmos-sdk/pull/17778) Use collections for `Params`
* remove from `Keeper`: `GetParams`, `SetParams`
* (types/simulation) [#17737](https://github.com/cosmos/cosmos-sdk/pull/17737) Remove unused parameter from `RandomFees`
* (x/staking) [#17486](https://github.com/cosmos/cosmos-sdk/pull/17486) Use collections for `RedelegationQueueKey`:
* remove from `types`: `GetRedelegationTimeKey`
* remove from `Keeper`: `RedelegationQueueIterator`
* (x/staking) [#17562](https://github.com/cosmos/cosmos-sdk/pull/17562) Use collections for `ValidatorQueue`
* remove from `types`: `GetValidatorQueueKey`, `ParseValidatorQueueKey`
* remove from `Keeper`: `ValidatorQueueIterator`
* (x/staking) [#17498](https://github.com/cosmos/cosmos-sdk/pull/17498) Use collections for `LastValidatorPower`:
* remove from `types`: `GetLastValidatorPowerKey`
* remove from `Keeper`: `LastValidatorsIterator`, `IterateLastValidators`
* (x/staking) [#17291](https://github.com/cosmos/cosmos-sdk/pull/17291) Use collections for `UnbondingDelegationByValIndex`:
* remove from `types`: `GetUBDKeyFromValIndexKey`, `GetUBDsByValIndexKey`, `GetUBDByValIndexKey`
* (x/slashing) [#17568](https://github.com/cosmos/cosmos-sdk/pull/17568) Use collections for `ValidatorMissedBlockBitmap`:
* remove from `types`: `ValidatorMissedBlockBitmapPrefixKey`, `ValidatorMissedBlockBitmapKey`
* (x/staking) [#17481](https://github.com/cosmos/cosmos-sdk/pull/17481) Use collections for `UnbondingQueue`:
* remove from `Keeper`: `UBDQueueIterator`
* remove from `types`: `GetUnbondingDelegationTimeKey`
* (x/staking) [#17123](https://github.com/cosmos/cosmos-sdk/pull/17123) Use collections for `Validators`
* (x/staking) [#17270](https://github.com/cosmos/cosmos-sdk/pull/17270) Use collections for `UnbondingDelegation`:
* remove from `types`: `GetUBDsKey`
* remove from `Keeper`: `IterateUnbondingDelegations`, `IterateDelegatorUnbondingDelegations`
* (client/keys) [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503) `clientkeys.NewKeyOutput`, `MkConsKeyOutput`, `MkValKeyOutput`, `MkAccKeyOutput`, `MkAccKeysOutput` now take their corresponding address codec instead of using the global SDK config.
* (x/staking) [#17336](https://github.com/cosmos/cosmos-sdk/pull/17336) Use collections for `RedelegationByValDstIndexKey`:
* remove from `types`: `GetREDByValDstIndexKey`, `GetREDsToValDstIndexKey`
* (x/staking) [#17332](https://github.com/cosmos/cosmos-sdk/pull/17332) Use collections for `RedelegationByValSrcIndexKey`:
* remove from `types`: `GetREDKeyFromValSrcIndexKey`, `GetREDsFromValSrcIndexKey`
* (x/staking) [#17315](https://github.com/cosmos/cosmos-sdk/pull/17315) Use collections for `RedelegationKey`:
* remove from `keeper`: `GetRedelegation`
* (types) `module.BeginBlockAppModule` has been replaced by Core API `appmodule.HasBeginBlocker`.
* (types) `module.EndBlockAppModule` has been replaced by Core API `appmodule.HasEndBlocker` or `module.HasABCIEndBlock` when needing validator updates.
* (x/slashing) [#17044](https://github.com/cosmos/cosmos-sdk/pull/17044) Use collections for `AddrPubkeyRelation`:
* remove from `types`: `AddrPubkeyRelationKey`
* remove from `Keeper`: `AddPubkey`
* (x/staking) [#17260](https://github.com/cosmos/cosmos-sdk/pull/17260) Use collections for `DelegationKey`:
* remove from `types`: `GetDelegationKey`, `GetDelegationsKey`
* (x/staking) [#17288](https://github.com/cosmos/cosmos-sdk/pull/17288) Use collections for `UnbondingIndex`:
* remove from `types`: `GetUnbondingIndexKey`.
* (x/staking) [#17256](https://github.com/cosmos/cosmos-sdk/pull/17256) Use collections for `UnbondingID`.
* (x/staking) [#17260](https://github.com/cosmos/cosmos-sdk/pull/17260) Use collections for `ValidatorByConsAddr`:
* remove from `types`: `GetValidatorByConsAddrKey`
* (x/staking) [#17248](https://github.com/cosmos/cosmos-sdk/pull/17248) Use collections for `UnbondingType`.
* remove from `types`: `GetUnbondingTypeKey`.
* (client) [#17259](https://github.com/cosmos/cosmos-sdk/pull/17259) Remove deprecated `clientCtx.PrintObjectLegacy`. Use `clientCtx.PrintProto` or `clientCtx.PrintRaw` instead.
* (x/feegrant) [#16535](https://github.com/cosmos/cosmos-sdk/pull/16535) Use collections for `FeeAllowance`, `FeeAllowanceQueue`.
* (x/staking) [#17063](https://github.com/cosmos/cosmos-sdk/pull/17063) Use collections for `HistoricalInfo`:
* remove `Keeper`: `GetHistoricalInfo`, `SetHistoricalInfo`
* (x/staking) [#17062](https://github.com/cosmos/cosmos-sdk/pull/17062) Use collections for `ValidatorUpdates`:
* remove `Keeper`: `SetValidatorUpdates`, `GetValidatorUpdates`
* (x/slashing) [#17023](https://github.com/cosmos/cosmos-sdk/pull/17023) Use collections for `ValidatorSigningInfo`:
* remove `Keeper`: `SetValidatorSigningInfo`, `GetValidatorSigningInfo`, `IterateValidatorSigningInfos`
* (x/staking) [#17026](https://github.com/cosmos/cosmos-sdk/pull/17026) Use collections for `LastTotalPower`:
* remove `Keeper`: `SetLastTotalPower`, `GetLastTotalPower`
* (x/authz) [#16509](https://github.com/cosmos/cosmos-sdk/pull/16509) `AcceptResponse` has been moved to sdk/types/authz and the `Updated` field is now of the type `sdk.Msg` instead of `authz.Authorization`.
* (x/slashing) [#16441](https://github.com/cosmos/cosmos-sdk/pull/16441) Params state is migrated to collections. `GetParams` has been removed.
* (types) [#16918](https://github.com/cosmos/cosmos-sdk/pull/16918) Remove `IntProto` and `DecProto`. Instead, `math.Int` and `math.LegacyDec` should be used respectively. Both types support `Marshal` and `Unmarshal` which should be used for binary marshaling.
* (client) [#17215](https://github.com/cosmos/cosmos-sdk/pull/17215) `server.StartCmd`,`server.ExportCmd`,`server.NewRollbackCmd`,`pruning.Cmd`,`genutilcli.InitCmd`,`genutilcli.GenTxCmd`,`genutilcli.CollectGenTxsCmd`,`genutilcli.AddGenesisAccountCmd`, do not take a home directory anymore. It is inferred from the root command.
* (baseapp) [#16244](https://github.com/cosmos/cosmos-sdk/pull/16244) `SetProtocolVersion` has been renamed to `SetAppVersion`. It now updates the consensus params in baseapp's `ParamStore`.
* (types) [#17348](https://github.com/cosmos/cosmos-sdk/pull/17348) Remove the `WrapServiceResult` function.
* The `*sdk.Result` returned by the msg server router will not contain the `.Data` field.
* (x/staking) [#17335](https://github.com/cosmos/cosmos-sdk/pull/17335) Remove usage of `"cosmossdk.io/x/staking/types".Infraction_*` in favour of `"cosmossdk.io/api/cosmos/staking/v1beta1".Infraction_` in order to remove dependency between modules on staking
* (types) [#17426](https://github.com/cosmos/cosmos-sdk/pull/17426) `NewContext` does not take a `cmtproto.Header{}` any longer.
* `WithChainID` / `WithBlockHeight` / `WithBlockHeader` must be used to set values on the context
* (x/bank) [#17569](https://github.com/cosmos/cosmos-sdk/pull/17569) `BurnCoins` takes an address instead of a module name
* (types) [#17738](https://github.com/cosmos/cosmos-sdk/pull/17738) `WithBlockTime()` was removed & `BlockTime()` were deprecated in favor of `WithHeaderInfo()` & `HeaderInfo()`. `BlockTime` now gets data from `HeaderInfo()` instead of `BlockHeader()`.
* (client) [#17746](https://github.com/cosmos/cosmos-sdk/pull/17746) `txEncodeAmino` & `txDecodeAmino` txs via grpc and rest were removed
* `RegisterLegacyAmino` was removed from `AppModuleBasic`
* (x/staking) [#17655](https://github.com/cosmos/cosmos-sdk/pull/17655) `QueryHistoricalInfo` was adjusted to return `HistoricalRecord` and marked `Hist` as deprecated.
* (types) [#17885](https://github.com/cosmos/cosmos-sdk/pull/17885) `InitGenesis` & `ExportGenesis` now take `context.Context` instead of `sdk.Context`
* (x/auth) [#17985](https://github.com/cosmos/cosmos-sdk/pull/17985) Remove `StdTxConfig`
* Remove deprecated `MakeTestingEncodingParams` from `simapp/params`
* (x/group) [#17937](https://github.com/cosmos/cosmos-sdk/pull/17937) Groups module was moved to its own go.mod `cosmossdk.io/x/group`
* (x/gov) [#18197](https://github.com/cosmos/cosmos-sdk/pull/18197) Gov module was moved to its own go.mod `cosmossdk.io/x/gov`
* (x/distribution) [#18199](https://github.com/cosmos/cosmos-sdk/pull/18199) Distribution module was moved to its own go.mod `cosmossdk.io/x/distribution`
* (x/slashing) [#18201](https://github.com/cosmos/cosmos-sdk/pull/18201) Slashing module was moved to its own go.mod `cosmossdk.io/x/slashing`
* (x/staking) [#18257](https://github.com/cosmos/cosmos-sdk/pull/18257) Staking module was moved to its own go.mod `cosmossdk.io/x/staking`
* (x/authz) [#18265](https://github.com/cosmos/cosmos-sdk/pull/18265) Authz module was moved to its own go.mod `cosmossdk.io/x/authz`
* (x/mint) [#18283](https://github.com/cosmos/cosmos-sdk/pull/18283) Mint module was moved to its own go.mod `cosmossdk.io/x/mint`
* (x/consensus) [#18041](https://github.com/cosmos/cosmos-sdk/pull/18041) `ToProtoConsensusParams()` returns an error
* (x/slashing) [#18115](https://github.com/cosmos/cosmos-sdk/pull/18115) `NewValidatorSigningInfo` takes strings instead of `sdk.AccAddress`
* (types) [#18268](https://github.com/cosmos/cosmos-sdk/pull/18268) Remove global setting of basedenom. Use the staking module parameter instead
* (x/auth) [#18351](https://github.com/cosmos/cosmos-sdk/pull/18351) Auth module was moved to its own go.mod `cosmossdk.io/x/auth`
* (types) [#18372](https://github.com/cosmos/cosmos-sdk/pull/18372) Removed global configuration for coin type and purpose. Setters and getters should be removed and access directly to defined types.
* (types) [#18695](https://github.com/cosmos/cosmos-sdk/pull/18695) Removed global configuration for txEncoder.

### CLI Breaking Changes

* (server) [#18303](https://github.com/cosmos/cosmos-sdk/pull/18303) `appd export` has moved with other genesis commands, use `appd genesis export` instead.
* (x/auth/vesting) [#18100](https://github.com/cosmos/cosmos-sdk/pull/18100) `appd tx vesting create-vesting-account` takes an amount of coin as last argument instead of second. Coins are space separated.

### State Machine Breaking

* (x/upgrade) [#16244](https://github.com/cosmos/cosmos-sdk/pull/16244) Upgrade module no longer stores the app version but gets and sets the app version stored in the `ParamStore` of baseapp.
* (x/staking) [#17655](https://github.com/cosmos/cosmos-sdk/pull/17655) `HistoricalInfo` was replaced with `HistoricalRecord`, it removes the validator set and comet header and only keep what is needed for IBC.
>>>>>>> 5166c9f89 (fix: Dedup Vote Extensions in `ValidateVoteExtensions` (#18895))
## [v0.50.2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.2) - 2023-12-11

Expand Down
32 changes: 23 additions & 9 deletions baseapp/abci_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2055,15 +2055,25 @@ func TestBaseApp_VoteExtensions(t *testing.T) {
ctrl := gomock.NewController(t)
valStore := mock.NewMockValidatorStore(ctrl)

// for brevity and simplicity, all validators have the same key
privKey := secp256k1.GenPrivKey()
pubKey := privKey.PubKey()
tmPk := cmtprotocrypto.PublicKey{
Sum: &cmtprotocrypto.PublicKey_Secp256K1{
Secp256K1: pubKey.Bytes(),
},
// 10 good vote extensions, 2 bad ones from 12 total validators
numVals := 12
privKeys := make([]secp256k1.PrivKey, numVals)
vals := make([]sdk.ConsAddress, numVals)
for i := 0; i < numVals; i++ {
privKey := secp256k1.GenPrivKey()
privKeys[i] = privKey

pubKey := privKey.PubKey()
val := sdk.ConsAddress(pubKey.Bytes())
vals[i] = val

tmPk := cmtprotocrypto.PublicKey{
Sum: &cmtprotocrypto.PublicKey_Secp256K1{
Secp256K1: pubKey.Bytes(),
},
}
valStore.EXPECT().GetPubKeyByConsAddr(gomock.Any(), val).Return(tmPk, nil)
}
valStore.EXPECT().GetPubKeyByConsAddr(gomock.Any(), gomock.Any()).Return(tmPk, nil).AnyTimes()

baseappOpts := func(app *baseapp.BaseApp) {
app.SetExtendVoteHandler(func(sdk.Context, *abci.RequestExtendVote) (*abci.ResponseExtendVote, error) {
Expand Down Expand Up @@ -2229,7 +2239,7 @@ func TestBaseApp_VoteExtensions(t *testing.T) {

// Now onto the second block, this time we process vote extensions from the
// previous block (which we sign now)
for _, ve := range allVEs {
for i, ve := range allVEs {
cve := cmtproto.CanonicalVoteExtension{
Extension: ve,
Height: 1,
Expand All @@ -2240,13 +2250,17 @@ func TestBaseApp_VoteExtensions(t *testing.T) {
bz, err := marshalDelimitedFn(&cve)
require.NoError(t, err)

privKey := privKeys[i]
extSig, err := privKey.Sign(bz)
require.NoError(t, err)

prepPropReq.LocalLastCommit.Votes = append(prepPropReq.LocalLastCommit.Votes, abci.ExtendedVoteInfo{
VoteExtension: ve,
BlockIdFlag: cmtproto.BlockIDFlagCommit,
ExtensionSignature: extSig,
Validator: abci.Validator{
Address: vals[i].Bytes(),
},
})
}

Expand Down
7 changes: 7 additions & 0 deletions baseapp/abci_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ func ValidateVoteExtensions(
sumVP int64
)

cache := make(map[string]struct{})
for _, vote := range extCommit.Votes {
totalVP += vote.Validator.Power

Expand All @@ -95,7 +96,13 @@ func ValidateVoteExtensions(
return fmt.Errorf("vote extensions enabled; received empty vote extension signature at height %d", currentHeight)
}

// Ensure that the validator has not already submitted a vote extension.
valConsAddr := sdk.ConsAddress(vote.Validator.Address)
if _, ok := cache[valConsAddr.String()]; ok {
return fmt.Errorf("duplicate validator; validator %s has already submitted a vote extension", valConsAddr.String())
}
cache[valConsAddr.String()] = struct{}{}

pubKeyProto, err := valStore.GetPubKeyByConsAddr(ctx, valConsAddr)
if err != nil {
return fmt.Errorf("failed to get validator %X public key: %w", valConsAddr, err)
Expand Down
34 changes: 34 additions & 0 deletions baseapp/abci_utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,40 @@ func (s *ABCIUtilsTestSuite) TestValidateVoteExtensionsSingleVoteAbsent() {
s.Require().NoError(baseapp.ValidateVoteExtensions(s.ctx, s.valStore, 3, chainID, llc))
}

// check ValidateVoteExtensions works with duplicate votes
func (s *ABCIUtilsTestSuite) TestValidateVoteExtensionsDuplicateVotes() {
ext := []byte("vote-extension")
cve := cmtproto.CanonicalVoteExtension{
Extension: ext,
Height: 2,
Round: int64(0),
ChainId: chainID,
}

bz, err := marshalDelimitedFn(&cve)
s.Require().NoError(err)

extSig0, err := s.vals[0].privKey.Sign(bz)
s.Require().NoError(err)

ve := abci.ExtendedVoteInfo{
Validator: s.vals[0].toValidator(333),
VoteExtension: ext,
ExtensionSignature: extSig0,
BlockIdFlag: cmtproto.BlockIDFlagCommit,
}

llc := abci.ExtendedCommitInfo{
Round: 0,
Votes: []abci.ExtendedVoteInfo{
ve,
ve,
},
}
// expect fail (duplicate votes)
s.Require().Error(baseapp.ValidateVoteExtensions(s.ctx, s.valStore, 3, chainID, llc))
}

// check ValidateVoteExtensions works when a single node has submitted a BlockID_Nil
func (s *ABCIUtilsTestSuite) TestValidateVoteExtensionsSingleVoteNil() {
ext := []byte("vote-extension")
Expand Down

0 comments on commit 1a4cad1

Please sign in to comment.