diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 4678e6581..a2ae6c27c 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -25,6 +25,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi ### IMPROVEMENTS: +- [consensus] \#5143 Only call `privValidator.GetPubKey` once per block (@melekes) ### BUG FIXES: diff --git a/consensus/state.go b/consensus/state.go index 686d0d693..067716359 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -10,6 +10,7 @@ import ( "github.com/pkg/errors" + "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/libs/fail" "github.com/tendermint/tendermint/libs/log" tmos "github.com/tendermint/tendermint/libs/os" @@ -32,6 +33,8 @@ var ( ErrInvalidProposalPOLRound = errors.New("error invalid proposal POL round") ErrAddingVote = errors.New("error adding vote") ErrVoteHeightMismatch = errors.New("error vote height mismatch") + + errPubKeyIsNotSet = errors.New("pubkey is not set. Look for \"Can't get private validator pubkey\" errors") ) //----------------------------------------------------------------------------- @@ -96,6 +99,9 @@ type State struct { mtx sync.RWMutex cstypes.RoundState state sm.State // State until height-1. + // privValidator pubkey, memoized for the duration of one block + // to avoid extra requests to HSM + privValidatorPubKey crypto.PubKey // state changes may be triggered by: msgs from peers, // msgs from ourself, or by timeouts @@ -252,11 +258,17 @@ func (cs *State) GetValidators() (int64, []*types.Validator) { return cs.state.LastBlockHeight, cs.state.Validators.Copy().Validators } -// SetPrivValidator sets the private validator account for signing votes. +// SetPrivValidator sets the private validator account for signing votes. It +// immediately requests pubkey and caches it. func (cs *State) SetPrivValidator(priv types.PrivValidator) { cs.mtx.Lock() + defer cs.mtx.Unlock() + cs.privValidator = priv - cs.mtx.Unlock() + + if err := cs.updatePrivValidatorPubKey(); err != nil { + cs.Logger.Error("Can't get private validator pubkey", "err", err) + } } // SetTimeoutTicker sets the local timer. It may be useful to overwrite for testing. @@ -930,14 +942,13 @@ func (cs *State) enterPropose(height int64, round int) { } logger.Debug("This node is a validator") - pubKey, err := cs.privValidator.GetPubKey() - if err != nil { + if cs.privValidatorPubKey == nil { // If this node is a validator & proposer in the current round, it will // miss the opportunity to create a block. - logger.Error("Error on retrival of pubkey", "err", err) + logger.Error(fmt.Sprintf("enterPropose: %v", errPubKeyIsNotSet)) return } - address := pubKey.Address() + address := cs.privValidatorPubKey.Address() // if not a validator, we're done if !cs.Validators.HasAddress(address) { @@ -1027,6 +1038,10 @@ func (cs *State) isProposalComplete() bool { // NOTE: keep it side-effect free for clarity. // CONTRACT: cs.privValidator is not nil. func (cs *State) createProposalBlock() (block *types.Block, blockParts *types.PartSet) { + if cs.privValidator == nil { + panic("entered createProposalBlock with privValidator being nil") + } + var commit *types.Commit switch { case cs.Height == 1: @@ -1041,17 +1056,13 @@ func (cs *State) createProposalBlock() (block *types.Block, blockParts *types.Pa return } - if cs.privValidator == nil { - panic("entered createProposalBlock with privValidator being nil") - } - pubKey, err := cs.privValidator.GetPubKey() - if err != nil { + if cs.privValidatorPubKey == nil { // If this node is a validator & proposer in the current round, it will // miss the opportunity to create a block. - cs.Logger.Error("Error on retrival of pubkey", "err", err) + cs.Logger.Error(fmt.Sprintf("enterPropose: %v", errPubKeyIsNotSet)) return } - proposerAddr := pubKey.Address() + proposerAddr := cs.privValidatorPubKey.Address() return cs.blockExec.CreateProposalBlock(cs.Height, cs.state, commit, proposerAddr) } @@ -1489,6 +1500,11 @@ func (cs *State) finalizeCommit(height int64) { fail.Fail() // XXX + // Private validator might have changed it's key pair => refetch pubkey. + if err := cs.updatePrivValidatorPubKey(); err != nil { + cs.Logger.Error("Can't get private validator pubkey", "err", err) + } + // cs.StartTime is already set. // Schedule Round0 to start soon. cs.scheduleRound0(&cs.RoundState) @@ -1540,12 +1556,11 @@ func (cs *State) recordMetrics(height int64, block *types.Block) { } if cs.privValidator != nil { - pubkey, err := cs.privValidator.GetPubKey() - if err != nil { + if cs.privValidatorPubKey == nil { // Metrics won't be updated, but it's not critical. - cs.Logger.Error("Error on retrieval of pubkey", "err", err) + cs.Logger.Error(fmt.Sprintf("recordMetrics: %v", errPubKeyIsNotSet)) } else { - address = pubkey.Address() + address = cs.privValidatorPubKey.Address() } } @@ -1716,12 +1731,11 @@ func (cs *State) tryAddVote(vote *types.Vote, peerID p2p.ID) (bool, error) { if err == ErrVoteHeightMismatch { return added, err } else if voteErr, ok := err.(*types.ErrVoteConflictingVotes); ok { - pubKey, err := cs.privValidator.GetPubKey() - if err != nil { - return false, errors.Wrap(err, "can't get pubkey") + if cs.privValidatorPubKey == nil { + return false, errPubKeyIsNotSet } - if bytes.Equal(vote.ValidatorAddress, pubKey.Address()) { + if bytes.Equal(vote.ValidatorAddress, cs.privValidatorPubKey.Address()) { cs.Logger.Error( "Found conflicting vote from ourselves. Did you unsafe_reset a validator?", "height", @@ -1920,11 +1934,10 @@ func (cs *State) signVote( // and the privValidator will refuse to sign anything. cs.wal.FlushAndSync() - pubKey, err := cs.privValidator.GetPubKey() - if err != nil { - return nil, errors.Wrap(err, "can't get pubkey") + if cs.privValidatorPubKey == nil { + return nil, errPubKeyIsNotSet } - addr := pubKey.Address() + addr := cs.privValidatorPubKey.Address() valIdx, _ := cs.Validators.GetByAddress(addr) vote := &types.Vote{ @@ -1937,7 +1950,7 @@ func (cs *State) signVote( BlockID: types.BlockID{Hash: hash, PartsHeader: header}, } - err = cs.privValidator.SignVote(cs.state.ChainID, vote) + err := cs.privValidator.SignVote(cs.state.ChainID, vote) return vote, err } @@ -1966,15 +1979,14 @@ func (cs *State) signAddVote(msgType types.SignedMsgType, hash []byte, header ty return nil } - pubKey, err := cs.privValidator.GetPubKey() - if err != nil { + if cs.privValidatorPubKey == nil { // Vote won't be signed, but it's not critical. - cs.Logger.Error("Error on retrival of pubkey", "err", err) + cs.Logger.Error(fmt.Sprintf("signAddVote: %v", errPubKeyIsNotSet)) return nil } // If the node not in the validator set, do nothing. - if !cs.Validators.HasAddress(pubKey.Address()) { + if !cs.Validators.HasAddress(cs.privValidatorPubKey.Address()) { return nil } @@ -1991,6 +2003,22 @@ func (cs *State) signAddVote(msgType types.SignedMsgType, hash []byte, header ty return nil } +// updatePrivValidatorPubKey get's the private validator public key and +// memoizes it. This func returns an error if the private validator is not +// responding or responds with an error. +func (cs *State) updatePrivValidatorPubKey() error { + if cs.privValidator == nil { + return nil + } + + pubKey, err := cs.privValidator.GetPubKey() + if err != nil { + return err + } + cs.privValidatorPubKey = pubKey + return nil +} + //--------------------------------------------------------- func CompareHRS(h1 int64, r1 int, s1 cstypes.RoundStepType, h2 int64, r2 int, s2 cstypes.RoundStepType) int {