fix(ve-validation): account for BlockID flag in vote-extensions (#17394)
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
This commit is contained in:
Nikhil Vasan
GitHub
parent
dded2e9921
commit
104ebe60f6
@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
|
||||
|
||||
* (x/bank) [#16795](https://github.com/cosmos/cosmos-sdk/pull/16852) Add `DenomMetadataByQueryString` query in bank module to support metadata query by query string.
|
||||
* (baseapp) [#16239](https://github.com/cosmos/cosmos-sdk/pull/16239) Add Gas Limits to allow node operators to resource bound queries.
|
||||
* (baseapp) [#17393](https://github.com/cosmos/cosmos-sdk/pull/17394) Check BlockID Flag on Votes in `ValidateVoteExtensions`
|
||||
|
||||
### Improvements
|
||||
|
||||
|
||||
@ -1871,6 +1871,7 @@ func TestABCI_PrepareProposal_VoteExtensions(t *testing.T) {
|
||||
},
|
||||
VoteExtension: ext,
|
||||
ExtensionSignature: extSig,
|
||||
BlockIdFlag: cmtproto.BlockIDFlagCommit,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@ -64,6 +64,12 @@ func ValidateVoteExtensions(
|
||||
|
||||
sumVP := math.NewInt(0)
|
||||
for _, vote := range extCommit.Votes {
|
||||
// Only check + include power if the vote is a commit vote. There must be super-majority, otherwise the
|
||||
// previous block (the block vote is for) could not have been committed.
|
||||
if vote.BlockIdFlag != cmtproto.BlockIDFlagCommit {
|
||||
continue
|
||||
}
|
||||
|
||||
if !extsEnabled {
|
||||
if len(vote.VoteExtension) > 0 {
|
||||
return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight)
|
||||
|
||||
286
baseapp/abci_utils_test.go
Normal file
286
baseapp/abci_utils_test.go
Normal file
@ -0,0 +1,286 @@
|
||||
package baseapp_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"testing"
|
||||
|
||||
abci "github.com/cometbft/cometbft/abci/types"
|
||||
"github.com/cometbft/cometbft/crypto/secp256k1"
|
||||
cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto"
|
||||
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
|
||||
protoio "github.com/cosmos/gogoproto/io"
|
||||
"github.com/cosmos/gogoproto/proto"
|
||||
"github.com/golang/mock/gomock"
|
||||
"github.com/stretchr/testify/suite"
|
||||
|
||||
"cosmossdk.io/math"
|
||||
|
||||
"github.com/cosmos/cosmos-sdk/baseapp"
|
||||
"github.com/cosmos/cosmos-sdk/baseapp/testutil/mock"
|
||||
sdk "github.com/cosmos/cosmos-sdk/types"
|
||||
)
|
||||
|
||||
const (
|
||||
chainID = "chain-id"
|
||||
)
|
||||
|
||||
type testValidator struct {
|
||||
consAddr sdk.ConsAddress
|
||||
tmPk cmtprotocrypto.PublicKey
|
||||
privKey secp256k1.PrivKey
|
||||
}
|
||||
|
||||
func newTestValidator() testValidator {
|
||||
privkey := secp256k1.GenPrivKey()
|
||||
pubkey := privkey.PubKey()
|
||||
tmPk := cmtprotocrypto.PublicKey{
|
||||
Sum: &cmtprotocrypto.PublicKey_Secp256K1{
|
||||
Secp256K1: pubkey.Bytes(),
|
||||
},
|
||||
}
|
||||
|
||||
return testValidator{
|
||||
consAddr: sdk.ConsAddress(pubkey.Address()),
|
||||
tmPk: tmPk,
|
||||
privKey: privkey,
|
||||
}
|
||||
}
|
||||
|
||||
func (t testValidator) toValidator() abci.Validator {
|
||||
return abci.Validator{
|
||||
Address: t.consAddr.Bytes(),
|
||||
Power: 0, // ignored for now
|
||||
}
|
||||
}
|
||||
|
||||
type ABCIUtilsTestSuite struct {
|
||||
suite.Suite
|
||||
|
||||
valStore *mock.MockValidatorStore
|
||||
vals [3]testValidator
|
||||
ctx sdk.Context
|
||||
}
|
||||
|
||||
func NewABCIUtilsTestSuite(t *testing.T) *ABCIUtilsTestSuite {
|
||||
t.Helper()
|
||||
// create 3 validators
|
||||
s := &ABCIUtilsTestSuite{
|
||||
vals: [3]testValidator{
|
||||
newTestValidator(),
|
||||
newTestValidator(),
|
||||
newTestValidator(),
|
||||
},
|
||||
}
|
||||
|
||||
// create mock
|
||||
ctrl := gomock.NewController(t)
|
||||
valStore := mock.NewMockValidatorStore(ctrl)
|
||||
s.valStore = valStore
|
||||
|
||||
// set up mock
|
||||
s.valStore.EXPECT().BondedTokensAndPubKeyByConsAddr(gomock.Any(), s.vals[0].consAddr.Bytes()).Return(math.NewInt(333), s.vals[0].tmPk, nil).AnyTimes()
|
||||
s.valStore.EXPECT().BondedTokensAndPubKeyByConsAddr(gomock.Any(), s.vals[1].consAddr.Bytes()).Return(math.NewInt(333), s.vals[1].tmPk, nil).AnyTimes()
|
||||
s.valStore.EXPECT().BondedTokensAndPubKeyByConsAddr(gomock.Any(), s.vals[2].consAddr.Bytes()).Return(math.NewInt(334), s.vals[2].tmPk, nil).AnyTimes()
|
||||
s.valStore.EXPECT().TotalBondedTokens(gomock.Any()).Return(math.NewInt(1000), nil).AnyTimes()
|
||||
|
||||
// create context
|
||||
s.ctx = sdk.Context{}.WithConsensusParams(cmtproto.ConsensusParams{
|
||||
Abci: &cmtproto.ABCIParams{
|
||||
VoteExtensionsEnableHeight: 2,
|
||||
},
|
||||
})
|
||||
return s
|
||||
}
|
||||
|
||||
func TestACITUtilsTestSuite(t *testing.T) {
|
||||
suite.Run(t, NewABCIUtilsTestSuite(t))
|
||||
}
|
||||
|
||||
// check ValidateVoteExtensions works when all nodes have CommitBlockID votes
|
||||
func (s *ABCIUtilsTestSuite) TestValidateVoteExtensionsHappyPath() {
|
||||
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)
|
||||
|
||||
extSig1, err := s.vals[1].privKey.Sign(bz)
|
||||
s.Require().NoError(err)
|
||||
|
||||
extSig2, err := s.vals[2].privKey.Sign(bz)
|
||||
s.Require().NoError(err)
|
||||
|
||||
llc := abci.ExtendedCommitInfo{
|
||||
Round: 0,
|
||||
Votes: []abci.ExtendedVoteInfo{
|
||||
{
|
||||
Validator: s.vals[0].toValidator(),
|
||||
VoteExtension: ext,
|
||||
ExtensionSignature: extSig0,
|
||||
BlockIdFlag: cmtproto.BlockIDFlagCommit,
|
||||
},
|
||||
{
|
||||
Validator: s.vals[1].toValidator(),
|
||||
VoteExtension: ext,
|
||||
ExtensionSignature: extSig1,
|
||||
BlockIdFlag: cmtproto.BlockIDFlagCommit,
|
||||
},
|
||||
{
|
||||
Validator: s.vals[2].toValidator(),
|
||||
VoteExtension: ext,
|
||||
ExtensionSignature: extSig2,
|
||||
BlockIdFlag: cmtproto.BlockIDFlagCommit,
|
||||
},
|
||||
},
|
||||
}
|
||||
// expect-pass (votes of height 2 are included in next block)
|
||||
s.Require().NoError(baseapp.ValidateVoteExtensions(s.ctx, s.valStore, 3, chainID, llc))
|
||||
}
|
||||
|
||||
// check ValidateVoteExtensions works when a single node has submitted a BlockID_Absent
|
||||
func (s *ABCIUtilsTestSuite) TestValidateVoteExtensionsSingleVoteAbsent() {
|
||||
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)
|
||||
|
||||
extSig2, err := s.vals[2].privKey.Sign(bz)
|
||||
s.Require().NoError(err)
|
||||
|
||||
llc := abci.ExtendedCommitInfo{
|
||||
Round: 0,
|
||||
Votes: []abci.ExtendedVoteInfo{
|
||||
{
|
||||
Validator: s.vals[0].toValidator(),
|
||||
VoteExtension: ext,
|
||||
ExtensionSignature: extSig0,
|
||||
BlockIdFlag: cmtproto.BlockIDFlagCommit,
|
||||
},
|
||||
// validator of power >1/3 is missing, so commit-info shld still be valid
|
||||
{
|
||||
Validator: s.vals[1].toValidator(),
|
||||
BlockIdFlag: cmtproto.BlockIDFlagAbsent,
|
||||
},
|
||||
{
|
||||
Validator: s.vals[2].toValidator(),
|
||||
VoteExtension: ext,
|
||||
ExtensionSignature: extSig2,
|
||||
BlockIdFlag: cmtproto.BlockIDFlagCommit,
|
||||
},
|
||||
},
|
||||
}
|
||||
// expect-pass (votes of height 2 are included in next block)
|
||||
s.Require().NoError(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")
|
||||
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)
|
||||
|
||||
extSig2, err := s.vals[2].privKey.Sign(bz)
|
||||
s.Require().NoError(err)
|
||||
|
||||
llc := abci.ExtendedCommitInfo{
|
||||
Round: 0,
|
||||
Votes: []abci.ExtendedVoteInfo{
|
||||
{
|
||||
Validator: s.vals[0].toValidator(),
|
||||
VoteExtension: ext,
|
||||
ExtensionSignature: extSig0,
|
||||
BlockIdFlag: cmtproto.BlockIDFlagCommit,
|
||||
},
|
||||
// validator of power <1/3 is missing, so commit-info shld still be valid
|
||||
{
|
||||
Validator: s.vals[1].toValidator(),
|
||||
BlockIdFlag: cmtproto.BlockIDFlagNil,
|
||||
},
|
||||
{
|
||||
Validator: s.vals[2].toValidator(),
|
||||
VoteExtension: ext,
|
||||
ExtensionSignature: extSig2,
|
||||
BlockIdFlag: cmtproto.BlockIDFlagCommit,
|
||||
},
|
||||
},
|
||||
}
|
||||
// expect-pass (votes of height 2 are included in next block)
|
||||
s.Require().NoError(baseapp.ValidateVoteExtensions(s.ctx, s.valStore, 3, chainID, llc))
|
||||
}
|
||||
|
||||
// check ValidateVoteExtensions works when two nodes have submitted a BlockID_Nil / BlockID_Absent
|
||||
func (s *ABCIUtilsTestSuite) TestValidateVoteExtensionsTwoVotesNilAbsent() {
|
||||
ext := []byte("vote-extension")
|
||||
cve := cmtproto.CanonicalVoteExtension{
|
||||
Extension: ext,
|
||||
Height: 2,
|
||||
Round: int64(0),
|
||||
ChainId: chainID,
|
||||
}
|
||||
|
||||
bz, err := marshalDelimitedFn(&cve)
|
||||
s.Require().NoError(err)
|
||||
|
||||
extSig2, err := s.vals[2].privKey.Sign(bz)
|
||||
s.Require().NoError(err)
|
||||
|
||||
llc := abci.ExtendedCommitInfo{
|
||||
Round: 0,
|
||||
Votes: []abci.ExtendedVoteInfo{
|
||||
// validator of power >2/3 is missing, so commit-info shld still be valid
|
||||
{
|
||||
Validator: s.vals[0].toValidator(),
|
||||
BlockIdFlag: cmtproto.BlockIDFlagCommit,
|
||||
},
|
||||
{
|
||||
Validator: s.vals[1].toValidator(),
|
||||
BlockIdFlag: cmtproto.BlockIDFlagNil,
|
||||
},
|
||||
{
|
||||
Validator: s.vals[2].toValidator(),
|
||||
VoteExtension: ext,
|
||||
ExtensionSignature: extSig2,
|
||||
BlockIdFlag: cmtproto.BlockIDFlagAbsent,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
// expect-pass (votes of height 2 are included in next block)
|
||||
s.Require().Error(baseapp.ValidateVoteExtensions(s.ctx, s.valStore, 3, chainID, llc))
|
||||
}
|
||||
|
||||
func marshalDelimitedFn(msg proto.Message) ([]byte, error) {
|
||||
var buf bytes.Buffer
|
||||
if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return buf.Bytes(), nil
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user