From fc96ab888fc44e3c0624a5455d7652d5615234f2 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Fri, 18 Nov 2022 11:01:56 +0100 Subject: [PATCH] use extended vote sets --- consensus/common_test.go | 12 +++--- consensus/state.go | 61 +++++++++++++++++++++++------- consensus/state_test.go | 6 ++- consensus/types/height_vote_set.go | 23 +++++++++-- 4 files changed, 76 insertions(+), 26 deletions(-) diff --git a/consensus/common_test.go b/consensus/common_test.go index e8d3facda..0f2598e37 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -385,10 +385,10 @@ func subscribeToVoter(cs *State, addr []byte) <-chan tmpubsub.Message { // consensus states type makeStateArgs struct { - config *config.Config - consensusParams *types.ConsensusParams - validators int - application abci.Application + config *config.Config + params *types.ConsensusParams + validators int + application abci.Application } func makeState(t *testing.T, args makeStateArgs) (*State, []*validatorStub) { @@ -409,8 +409,8 @@ func makeState(t *testing.T, args makeStateArgs) (*State, []*validatorStub) { args.config.SetRoot(t.TempDir()) cp := test.ConsensusParams() - if args.consensusParams != nil { - cp = args.consensusParams + if args.params != nil { + cp = args.params } state, privVals := makeGenesisState(t, genesisStateArgs{ diff --git a/consensus/state.go b/consensus/state.go index 0655daea0..002481864 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -562,23 +562,51 @@ func (cs *State) sendInternalMessage(mi msgInfo) { } } -// Reconstruct LastCommit from SeenCommit, which we saved along with the block, -// (which happens even before saving the state) +// Reconstruct the LastCommit from either SeenCommit or the ExtendedCommit. SeenCommit +// and ExtendedCommit are saved along with the block. If VoteExtensions are required +// the method will panic on an absent ExtendedCommit or an ExtendedCommit without +// extension data. func (cs *State) reconstructLastCommit(state sm.State) { - seenCommit := cs.blockStore.LoadSeenCommit(state.LastBlockHeight) - if seenCommit == nil { - panic(fmt.Sprintf( - "failed to reconstruct last commit; seen commit for height %v not found", - state.LastBlockHeight, - )) + extensionsEnabled := cs.state.ConsensusParams.ABCI.VoteExtensionsEnabled(state.LastBlockHeight) + if !extensionsEnabled { + votes, err := cs.votesFromSeenCommit(state) + if err != nil { + panic(fmt.Sprintf("failed to reconstruct last commit; %s", err)) + } + cs.LastCommit = votes + return } - lastPrecommits := types.CommitToVoteSet(state.ChainID, seenCommit, state.LastValidators) - if !lastPrecommits.HasTwoThirdsMajority() { - panic("failed to reconstruct last commit; does not have +2/3 maj") + votes, err := cs.votesFromExtendedCommit(state) + if err != nil { + panic(fmt.Sprintf("failed to reconstruct last extended commit; %s", err)) + } + cs.LastCommit = votes +} + +func (cs *State) votesFromExtendedCommit(state sm.State) (*types.VoteSet, error) { + ec := cs.blockStore.LoadBlockExtendedCommit(state.LastBlockHeight) + if ec == nil { + return nil, fmt.Errorf("extended commit for height %v not found", state.LastBlockHeight) + } + vs := ec.ToExtendedVoteSet(state.ChainID, state.LastValidators) + if !vs.HasTwoThirdsMajority() { + return nil, errors.New("extended commit does not have +2/3 majority") + } + return vs, nil +} + +func (cs *State) votesFromSeenCommit(state sm.State) (*types.VoteSet, error) { + commit := cs.blockStore.LoadSeenCommit(state.LastBlockHeight) + if commit == nil { + return nil, fmt.Errorf("commit for height %v not found", state.LastBlockHeight) } - cs.LastCommit = lastPrecommits + vs := commit.ToVoteSet(state.ChainID, state.LastValidators) + if !vs.HasTwoThirdsMajority() { + return nil, errors.New("commit does not have +2/3 majority") + } + return vs, nil } // Updates State and increments height to match that of state. @@ -679,7 +707,11 @@ func (cs *State) updateToState(state sm.State) { cs.ValidRound = -1 cs.ValidBlock = nil cs.ValidBlockParts = nil - cs.Votes = cstypes.NewHeightVoteSet(state.ChainID, height, validators) + if state.ConsensusParams.ABCI.VoteExtensionsEnabled(height) { + cs.Votes = cstypes.NewExtendedHeightVoteSet(state.ChainID, height, validators) + } else { + cs.Votes = cstypes.NewHeightVoteSet(state.ChainID, height, validators) + } cs.CommitRound = -1 cs.LastValidators = state.LastValidators cs.TriggeredTimeoutPrecommit = false @@ -1212,6 +1244,7 @@ func (cs *State) createProposalBlock() (*types.Block, error) { return nil, errors.New("entered createProposalBlock with privValidator being nil") } + // TODO(sergio): wouldn't it be easier if CreateProposalBlock accepted cs.LastCommit directly? var lastExtCommit *types.ExtendedCommit switch { case cs.Height == cs.state.InitialHeight: @@ -2290,7 +2323,7 @@ func (cs *State) signVote( v := vote.ToProto() err := cs.privValidator.SignVote(cs.state.ChainID, v) vote.Signature = v.Signature - // append the extension signature if expect it + // append the extension signature if it is expected if msgType == tmproto.PrecommitType && !vote.BlockID.IsNil() && cs.state.ConsensusParams.ABCI.VoteExtensionsEnabled(cs.Height) { // defensively check that the signer correctly signed the vote extension diff --git a/consensus/state_test.go b/consensus/state_test.go index 9e5fd495f..307fd90cd 100644 --- a/consensus/state_test.go +++ b/consensus/state_test.go @@ -1540,7 +1540,7 @@ func TestExtendVoteCalledWhenEnabled(t *testing.T) { m.On("FinalizeBlock", mock.Anything, mock.Anything).Return(r, nil).Maybe() cs1, vss := makeState(t, makeStateArgs{application: m}) if !testCase.enabled { - cs1.state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = 0 + cs1, vss = makeState(t, makeStateArgs{application: m, params: types.DefaultConsensusParams()}) } height, round := cs1.Height, cs1.Round @@ -1823,7 +1823,9 @@ func TestVoteExtensionEnableHeight(t *testing.T) { r := &abci.ResponseFinalizeBlock{AgreedAppData: []byte("hashyHash")} m.On("FinalizeBlock", mock.Anything, mock.Anything).Return(r, nil).Maybe() m.On("Commit", mock.Anything, mock.Anything).Return(&abci.ResponseCommit{}, nil).Maybe() - cs1, vss := makeState(t, makeStateArgs{application: m}) + c := test.ConsensusParams() + c.ABCI.VoteExtensionsEnableHeight = testCase.enableHeight + cs1, vss := makeState(t, makeStateArgs{application: m, params: c}) cs1.state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = testCase.enableHeight height, round := cs1.Height, cs1.Round diff --git a/consensus/types/height_vote_set.go b/consensus/types/height_vote_set.go index 6a5c0b495..8ae31ee75 100644 --- a/consensus/types/height_vote_set.go +++ b/consensus/types/height_vote_set.go @@ -39,9 +39,10 @@ We let each peer provide us with up to 2 unexpected "catchup" rounds. One for their LastCommit round, and another for the official commit round. */ type HeightVoteSet struct { - chainID string - height int64 - valSet *types.ValidatorSet + chainID string + height int64 + valSet *types.ValidatorSet + extensionsEnabled bool mtx sync.Mutex round int32 // max tracked round @@ -57,6 +58,15 @@ func NewHeightVoteSet(chainID string, height int64, valSet *types.ValidatorSet) return hvs } +func NewExtendedHeightVoteSet(chainID string, height int64, valSet *types.ValidatorSet) *HeightVoteSet { + hvs := &HeightVoteSet{ + chainID: chainID, + extensionsEnabled: true, + } + hvs.Reset(height, valSet) + return hvs +} + func (hvs *HeightVoteSet) Reset(height int64, valSet *types.ValidatorSet) { hvs.mtx.Lock() defer hvs.mtx.Unlock() @@ -105,7 +115,12 @@ func (hvs *HeightVoteSet) addRound(round int32) { } // log.Debug("addRound(round)", "round", round) prevotes := types.NewVoteSet(hvs.chainID, hvs.height, round, tmproto.PrevoteType, hvs.valSet) - precommits := types.NewVoteSet(hvs.chainID, hvs.height, round, tmproto.PrecommitType, hvs.valSet) + var precommits *types.VoteSet + 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) + } hvs.roundVoteSets[round] = RoundVoteSet{ Prevotes: prevotes, Precommits: precommits,