From 44f1f045e25a16eb2f93c9028a98d0a6487b6602 Mon Sep 17 00:00:00 2001 From: William Banfield Date: Mon, 16 May 2022 19:08:34 -0400 Subject: [PATCH] update the VoteSet methods to only validate extensions when enabled --- internal/consensus/reactor_test.go | 2 +- internal/consensus/state.go | 6 +++--- internal/consensus/types/height_vote_set.go | 12 ++++++------ types/block.go | 7 ++++--- types/block_test.go | 4 ++-- types/vote_set.go | 16 ++++++++++------ types/vote_set_test.go | 6 +++--- 7 files changed, 29 insertions(+), 24 deletions(-) diff --git a/internal/consensus/reactor_test.go b/internal/consensus/reactor_test.go index 55b62589f..f77173bbb 100644 --- a/internal/consensus/reactor_test.go +++ b/internal/consensus/reactor_test.go @@ -669,7 +669,7 @@ func TestSwitchToConsensusVoteExtensions(t *testing.T) { var voteSet *types.VoteSet if testCase.includeExtensions { - voteSet = types.NewStrictVoteSet(cs.state.ChainID, testCase.storedHeight, 0, tmproto.PrecommitType, cs.state.Validators) + voteSet = types.NewExtendedVoteSet(cs.state.ChainID, testCase.storedHeight, 0, tmproto.PrecommitType, cs.state.Validators) } else { voteSet = types.NewVoteSet(cs.state.ChainID, testCase.storedHeight, 0, tmproto.PrecommitType, cs.state.Validators) } diff --git a/internal/consensus/state.go b/internal/consensus/state.go index 8577c5aad..409bb3deb 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -704,7 +704,7 @@ func (cs *State) reconstructLastCommit(state sm.State) { return } if extensionsEnabled { - panic(fmt.Sprintf("failed to reconstruct last commit; %s", err)) + panic(fmt.Sprintf("failed to reconstruct last extended commit; %s", err)) } votes, err = cs.votesFromSeenCommit(state) if err != nil { @@ -720,7 +720,7 @@ func (cs *State) votesFromExtendedCommit(state sm.State, extensionsEnabled bool) } var vs *types.VoteSet if extensionsEnabled { - vs = ec.ToStrictVoteSet(state.ChainID, state.LastValidators) + vs = ec.ToExtendedVoteSet(state.ChainID, state.LastValidators) } else { vs = ec.ToVoteSet(state.ChainID, state.LastValidators) } @@ -846,7 +846,7 @@ func (cs *State) updateToState(state sm.State) { cs.ValidBlock = nil cs.ValidBlockParts = nil if state.ConsensusParams.ABCI.VoteExtensionsEnabled(height) { - cs.Votes = cstypes.NewStrictHeightVoteSet(state.ChainID, height, validators) + cs.Votes = cstypes.NewExtendedHeightVoteSet(state.ChainID, height, validators) } else { cs.Votes = cstypes.NewHeightVoteSet(state.ChainID, height, validators) } diff --git a/internal/consensus/types/height_vote_set.go b/internal/consensus/types/height_vote_set.go index b57a3636a..389c02356 100644 --- a/internal/consensus/types/height_vote_set.go +++ b/internal/consensus/types/height_vote_set.go @@ -41,7 +41,7 @@ type HeightVoteSet struct { chainID string height int64 valSet *types.ValidatorSet - requireExtensions bool + extensionsEnabled bool mtx sync.Mutex round int32 // max tracked round @@ -52,16 +52,16 @@ type HeightVoteSet struct { func NewHeightVoteSet(chainID string, height int64, valSet *types.ValidatorSet) *HeightVoteSet { hvs := &HeightVoteSet{ chainID: chainID, - requireExtensions: false, + extensionsEnabled: false, } hvs.Reset(height, valSet) return hvs } -func NewStrictHeightVoteSet(chainID string, height int64, valSet *types.ValidatorSet) *HeightVoteSet { +func NewExtendedHeightVoteSet(chainID string, height int64, valSet *types.ValidatorSet) *HeightVoteSet { hvs := &HeightVoteSet{ chainID: chainID, - requireExtensions: true, + extensionsEnabled: true, } hvs.Reset(height, valSet) return hvs @@ -120,8 +120,8 @@ func (hvs *HeightVoteSet) addRound(round int32) { // log.Debug("addRound(round)", "round", round) prevotes := types.NewVoteSet(hvs.chainID, hvs.height, round, tmproto.PrevoteType, hvs.valSet) var precommits *types.VoteSet - if hvs.requireExtensions { - precommits = types.NewStrictVoteSet(hvs.chainID, hvs.height, round, tmproto.PrecommitType, hvs.valSet) + if hvs.extensionsEnabled { + precommits = types.NewExtendedVoteSet(hvs.chainID, hvs.height, round, tmproto.PrecommitType, hvs.valSet) } else { precommits = types.NewVoteSet(hvs.chainID, hvs.height, round, tmproto.PrecommitType, hvs.valSet) } diff --git a/types/block.go b/types/block.go index 57ca3b540..3c42fbafe 100644 --- a/types/block.go +++ b/types/block.go @@ -1014,18 +1014,19 @@ func (ec *ExtendedCommit) Clone() *ExtendedCommit { return &ecc } -// ToStrictVoteSet constructs a VoteSet from the Commit and validator set. +// ToExtendedVoteSet constructs a VoteSet from the Commit and validator set. // Panics if signatures from the ExtendedCommit can't be added to the voteset. // Panics if any of the votes have invalid or absent vote extension data. // Inverse of VoteSet.MakeExtendedCommit(). -func (ec *ExtendedCommit) ToStrictVoteSet(chainID string, vals *ValidatorSet) *VoteSet { - voteSet := NewStrictVoteSet(chainID, ec.Height, ec.Round, tmproto.PrecommitType, vals) +func (ec *ExtendedCommit) ToExtendedVoteSet(chainID string, vals *ValidatorSet) *VoteSet { + voteSet := NewExtendedVoteSet(chainID, ec.Height, ec.Round, tmproto.PrecommitType, vals) ec.addSigsToVoteSet(voteSet) return voteSet } // ToVoteSet constructs a VoteSet from the Commit and validator set. // Panics if signatures from the ExtendedCommit can't be added to the voteset. +// Panics if any of the votes have extension data. // Inverse of VoteSet.MakeExtendedCommit(). func (ec *ExtendedCommit) ToVoteSet(chainID string, vals *ValidatorSet) *VoteSet { voteSet := NewVoteSet(chainID, ec.Height, ec.Round, tmproto.PrecommitType, vals) diff --git a/types/block_test.go b/types/block_test.go index 365189049..86ee90c27 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -583,7 +583,7 @@ func TestVoteSetToExtendedCommit(t *testing.T) { valSet, vals := randValidatorPrivValSet(ctx, t, 10, 1) var voteSet *VoteSet if testCase.includeExtension { - voteSet = NewStrictVoteSet("test_chain_id", 3, 1, tmproto.PrecommitType, valSet) + voteSet = NewExtendedVoteSet("test_chain_id", 3, 1, tmproto.PrecommitType, valSet) } else { voteSet = NewVoteSet("test_chain_id", 3, 1, tmproto.PrecommitType, valSet) } @@ -668,7 +668,7 @@ func TestExtendedCommitToVoteSet(t *testing.T) { chainID := voteSet.ChainID() var voteSet2 *VoteSet if testCase.includeExtension { - voteSet2 = extCommit.ToStrictVoteSet(chainID, valSet) + voteSet2 = extCommit.ToExtendedVoteSet(chainID, valSet) } else { voteSet2 = extCommit.ToVoteSet(chainID, valSet) } diff --git a/types/vote_set.go b/types/vote_set.go index e8615db8c..a0a95a89b 100644 --- a/types/vote_set.go +++ b/types/vote_set.go @@ -3,6 +3,7 @@ package types import ( "bytes" "encoding/json" + "errors" "fmt" "strings" "sync" @@ -58,7 +59,7 @@ type VoteSet struct { round int32 signedMsgType tmproto.SignedMsgType valSet *ValidatorSet - requireExtensions bool + extensionsEnabled bool mtx sync.Mutex votesBitArray *bits.BitArray @@ -93,13 +94,13 @@ func NewVoteSet(chainID string, height int64, round int32, } } -// NewStrictVoteSet constructs a vote set with additional vote verification logic. -// The VoteSet constructed with NewStrictVoteSet verifies the vote extension +// NewExtendedVoteSet constructs a vote set with additional vote verification logic. +// The VoteSet constructed with NewExtendedVoteSet verifies the vote extension // data for every vote added to the set. -func NewStrictVoteSet(chainID string, height int64, round int32, +func NewExtendedVoteSet(chainID string, height int64, round int32, signedMsgType tmproto.SignedMsgType, valSet *ValidatorSet) *VoteSet { vs := NewVoteSet(chainID, height, round, signedMsgType, valSet) - vs.requireExtensions = true + vs.extensionsEnabled = true return vs } @@ -208,7 +209,7 @@ func (voteSet *VoteSet) addVote(vote *Vote) (added bool, err error) { } // Check signature. - if voteSet.requireExtensions || len(vote.ExtensionSignature) > 0 { + if voteSet.extensionsEnabled { if err := vote.VerifyVoteAndExtension(voteSet.chainID, val.PubKey); err != nil { return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s: %w", voteSet.chainID, val.PubKey, err) } @@ -216,6 +217,9 @@ func (voteSet *VoteSet) addVote(vote *Vote) (added bool, err error) { if err := vote.Verify(voteSet.chainID, val.PubKey); err != nil { return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s: %w", voteSet.chainID, val.PubKey, err) } + if len(vote.ExtensionSignature) > 0 || len(vote.Extension) > 0 { + return false, errors.New("unexpected vote extension data present in vote") + } } // Add vote and get conflicting vote if any. diff --git a/types/vote_set_test.go b/types/vote_set_test.go index 58832cc04..28cabe8d0 100644 --- a/types/vote_set_test.go +++ b/types/vote_set_test.go @@ -534,7 +534,7 @@ func TestVoteSet_VoteExtensionsEnabled(t *testing.T) { valSet, privValidators := randValidatorPrivValSet(ctx, t, 5, 10) var voteSet *VoteSet if tc.requireExtensions { - voteSet = NewStrictVoteSet("test_chain_id", height, round, tmproto.PrecommitType, valSet) + voteSet = NewExtendedVoteSet("test_chain_id", height, round, tmproto.PrecommitType, valSet) } else { voteSet = NewVoteSet("test_chain_id", height, round, tmproto.PrecommitType, valSet) } @@ -590,7 +590,7 @@ func randVoteSet( ) (*VoteSet, *ValidatorSet, []PrivValidator) { t.Helper() valSet, privValidators := randValidatorPrivValSet(ctx, t, numValidators, votingPower) - return NewStrictVoteSet("test_chain_id", height, round, signedMsgType, valSet), valSet, privValidators + return NewExtendedVoteSet("test_chain_id", height, round, signedMsgType, valSet), valSet, privValidators } func deterministicVoteSet( @@ -603,7 +603,7 @@ func deterministicVoteSet( ) (*VoteSet, *ValidatorSet, []PrivValidator) { t.Helper() valSet, privValidators := deterministicValidatorSet(ctx, t) - return NewStrictVoteSet("test_chain_id", height, round, signedMsgType, valSet), valSet, privValidators + return NewExtendedVoteSet("test_chain_id", height, round, signedMsgType, valSet), valSet, privValidators } func randValidatorPrivValSet(ctx context.Context, t testing.TB, numValidators int, votingPower int64) (*ValidatorSet, []PrivValidator) {