From 1765eb2cd22843c0393c4b38137e96ba8528f2da Mon Sep 17 00:00:00 2001 From: Ismail Khoffi Date: Fri, 7 Dec 2018 11:31:14 +0100 Subject: [PATCH] more review comments: - consistently wrap error - add changelog entry --- CHANGELOG_PENDING.md | 30 ++++-------------------------- consensus/common_test.go | 2 +- consensus/state.go | 12 ++++++------ node/node.go | 6 +++--- privval/priv_validator_test.go | 4 ++-- privval/remote_signer.go | 2 +- 6 files changed, 17 insertions(+), 39 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index b9a5454c..18978d7e 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,48 +1,26 @@ -# Pending - -## v0.27.0 +## v0.27.1 *TBD* Special thanks to external contributors on this release: -Friendly reminder, we have a [bug bounty -program](https://hackerone.com/tendermint). - ### BREAKING CHANGES: * CLI/RPC/Config - - [rpc] \#2932 Rename `accum` to `proposer_priority` * Apps * Go API - - [db] [\#2913](https://github.com/tendermint/tendermint/pull/2913) - ReverseIterator API change -- start < end, and end is exclusive. - - [types] \#2932 Rename `Validator.Accum` to `Validator.ProposerPriority` +- [types] \#2926 Return error on `PrivValidator.GetAddress()`/`PrivValidator.GetPubKey()` instead of panic'ing * Blockchain Protocol - - [state] \#2714 Validators can now only use pubkeys allowed within - ConsensusParams.ValidatorParams * P2P Protocol - - [consensus] [\#2871](https://github.com/tendermint/tendermint/issues/2871) - Remove *ProposalHeartbeat* message as it serves no real purpose - - [state] Fixes for proposer selection: - - \#2785 Accum for new validators is `-1.125*totalVotingPower` instead of 0 - - \#2941 val.Accum is preserved during ValidatorSet.Update to avoid being - reset to 0 ### FEATURES: ### IMPROVEMENTS: ### BUG FIXES: -- [types] \#2938 Fix regression in v0.26.4 where we panic on empty - genDoc.Validators -- [state] \#2785 Fix accum for new validators to be `-1.125*totalVotingPower` - instead of 0, forcing them to wait before becoming the proposer. Also: - - do not batch clip - - keep accums averaged near 0 -- [types] \#2941 Preserve val.Accum during ValidatorSet.Update to avoid it being - reset to 0 every time a validator is updated +- [kv indexer] \#2912 don't ignore key when executing CONTAINS +- [types] \#2926 Return error on `PrivValidator.GetAddress()`/`PrivValidator.GetPubKey()` instead of panic'ing diff --git a/consensus/common_test.go b/consensus/common_test.go index fe6e5111..09b247ae 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -76,7 +76,7 @@ func NewValidatorStub(privValidator types.PrivValidator, valIndex int) *validato func (vs *validatorStub) signVote(voteType types.SignedMsgType, hash []byte, header types.PartSetHeader) (*types.Vote, error) { addr, err := vs.PrivValidator.GetAddress() if err != nil { - return nil, errors.Wrap(err, "Error while retrieving private validator's address") + return nil, errors.Wrap(err, "failed to get private validator's address") } vote := &types.Vote{ ValidatorIndex: vs.Index, diff --git a/consensus/state.go b/consensus/state.go index 97231f21..2ce4838d 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -833,7 +833,7 @@ func (cs *ConsensusState) enterPropose(height int64, round int) { // if not a validator, we're done address, err := cs.privValidator.GetAddress() if err != nil { - logger.Error("Failed to get private validator address", "err", err) + logger.Error("Failed to get private validator's address", "err", err) return } if !cs.Validators.HasAddress(address) { @@ -938,7 +938,7 @@ func (cs *ConsensusState) createProposalBlock() (block *types.Block, blockParts ), maxGas) proposerAddr, err := cs.privValidator.GetAddress() if err != nil { - cs.Logger.Error("Failed to get private validator address", "err", err) + cs.Logger.Error("Failed to get private validator's address", "err", err) return } block, parts := cs.state.MakeBlock(cs.Height, txs, commit, evidence, proposerAddr) @@ -1487,7 +1487,7 @@ func (cs *ConsensusState) tryAddVote(vote *types.Vote, peerID p2p.ID) (bool, err } else if voteErr, ok := err.(*types.ErrVoteConflictingVotes); ok { addr, err := cs.privValidator.GetAddress() if err != nil { - cs.Logger.Error("Failed to get private validator address", "err", err) + cs.Logger.Error("Failed to get private validator's address", "err", err) return added, err } if bytes.Equal(vote.ValidatorAddress, addr) { @@ -1657,8 +1657,8 @@ func (cs *ConsensusState) addVote(vote *types.Vote, peerID p2p.ID) (added bool, func (cs *ConsensusState) signVote(type_ types.SignedMsgType, hash []byte, header types.PartSetHeader) (*types.Vote, error) { addr, err := cs.privValidator.GetAddress() if err != nil { - cs.Logger.Error("Failed to get private validator address", "err", err) - return nil, errs.Wrap(err, "Failed to get private validator address") + cs.Logger.Error("Failed to get private validator's address", "err", err) + return nil, errs.Wrap(err, "Failed to get private validator's address") } valIndex, _ := cs.Validators.GetByAddress(addr) @@ -1697,7 +1697,7 @@ func (cs *ConsensusState) signAddVote(type_ types.SignedMsgType, hash []byte, he // if we don't have a key or we're not in the validator set, do nothing privValAddr, err := cs.privValidator.GetAddress() if err != nil { - cs.Logger.Error("Failed to get private validator address", "err", err, "height", cs.Height, "round", cs.Round) + cs.Logger.Error("Failed to get private validator's address", "err", err, "height", cs.Height, "round", cs.Round) return nil } if cs.privValidator == nil || !cs.Validators.HasAddress(privValAddr) { diff --git a/node/node.go b/node/node.go index b4286620..a7bd2e35 100644 --- a/node/node.go +++ b/node/node.go @@ -237,7 +237,7 @@ func NewNode(config *cfg.Config, addr, _ := state.Validators.GetByIndex(0) privValAddr, err := privValidator.GetAddress() if err != nil { - return nil, errors.Wrap(err, "Error while retrieving private validator's address") + return nil, errors.Wrap(err, "failed to get private validator's address") } if bytes.Equal(privValAddr, addr) { fastSync = false @@ -246,7 +246,7 @@ func NewNode(config *cfg.Config, pubKey, err := privValidator.GetPubKey() if err != nil { - return nil, errors.Wrap(err, "Error while retrieving private validator's public key") + return nil, errors.Wrap(err, "failed to get private validator's public key") } addr := pubKey.Address() // Log whether this node is a validator or an observer @@ -622,7 +622,7 @@ func (n *Node) ConfigureRPC() { rpccore.SetP2PTransport(n) pubKey, err := n.privValidator.GetPubKey() if err != nil { - n.Logger.Error("Error configuring RPC. Can not retrieve privValidator's public key", "err", err) + n.Logger.Error("Error configuring RPC. Failed to get private validator's public key", "err", err) } rpccore.SetPubKey(pubKey) rpccore.SetGenesisDoc(n.genesisDoc) diff --git a/privval/priv_validator_test.go b/privval/priv_validator_test.go index 419a9382..6b7f1b03 100644 --- a/privval/priv_validator_test.go +++ b/privval/priv_validator_test.go @@ -31,7 +31,7 @@ func TestGenLoadValidator(t *testing.T) { privVal = LoadFilePV(tempFile.Name()) loadedAddr, err := privVal.GetAddress() require.NoError(t, err) - assert.Equal(addr, loadedAddr, "expected privval addr to be the same") + assert.Equal(addr, loadedAddr) assert.Equal(height, privVal.LastHeight, "expected privval.LastHeight to have been saved") } @@ -49,7 +49,7 @@ func TestLoadOrGenValidator(t *testing.T) { require.NoError(t, err) privVal = LoadOrGenFilePV(tempFilePath) loadedAddr, err := privVal.GetAddress() - assert.Equal(addr, loadedAddr, "expected privval addr to be the same") + assert.Equal(addr, loadedAddr) } func TestUnmarshalValidator(t *testing.T) { diff --git a/privval/remote_signer.go b/privval/remote_signer.go index 87f829ba..0d6a47a7 100644 --- a/privval/remote_signer.go +++ b/privval/remote_signer.go @@ -235,7 +235,7 @@ func handleRequest(req RemoteSignerMsg, chainID string, privVal types.PrivValida if err != nil { // TODO: split up PubKeyMsg into PubKeyRequest / PubKeyResponse and wrap the error // into the response as done below. For now we just return the error: - return nil, errors.Wrap(err, "Error while retrieving private validator's public key") + return nil, errors.Wrap(err, "failed to get private validator's public key") } res = &PubKeyMsg{p} case *SignVoteRequest: