Skip to content

Commit

Permalink
fix: Dedup Vote Extensions in ValidateVoteExtensions (#18895)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidterpay authored Dec 27, 2023
1 parent ae19acc commit 5166c9f
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (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

Expand Down
32 changes: 23 additions & 9 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2082,15 +2082,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 @@ -2256,7 +2266,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 @@ -2267,13 +2277,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
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func ValidateVoteExtensions(
sumVP int64
)

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

Expand All @@ -96,7 +97,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
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,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 5166c9f

Please sign in to comment.