Do not panic when double-signing, return error.

consensus/pol_test.go

@@ -16,8 +16,13 @@ import (
 // Signs the vote and sets the POL's vote at the desired index
 // Returns the POLVoteSignature pointer, so you can modify it afterwards.
 func signAddPOLVoteSignature(val *state.PrivValidator, valSet *state.ValidatorSet, vote *Vote, pol *POL) *POLVoteSignature {
+	vote = vote.Copy()
+	err := val.SignVote(vote)
+	if err != nil {
+		panic(err)
+	}
 	idx, _ := valSet.GetByAddress(val.Address) // now we have the index
-	pol.Votes[idx] = POLVoteSignature{vote.Round, val.SignVote(vote)}
+	pol.Votes[idx] = POLVoteSignature{vote.Round, vote.Signature}
 	return &pol.Votes[idx]
 }
 

consensus/state.go

@@ -491,8 +491,13 @@ func (cs *ConsensusState) updateToState(state *state.State) {
 			Address: cs.PrivValidator.Address,
 			Height:  cs.Height + 1,
 		}
-		rebondTx.Signature = cs.PrivValidator.SignRebondTx(rebondTx)
-		cs.mempoolReactor.BroadcastTx(rebondTx)
+		err := cs.PrivValidator.SignRebondTx(rebondTx)
+		if err == nil {
+			log.Info("Signed and broadcast RebondTx", "height", cs.Height, "round", cs.Round, "tx", rebondTx)
+			cs.mempoolReactor.BroadcastTx(rebondTx)
+		} else {
+			log.Warn("Error signing RebondTx", "height", cs.Height, "round", cs.Round, "tx", rebondTx, "error", err)
+		}
 	}
 }
 
@@ -609,6 +614,7 @@ func (cs *ConsensusState) RunActionPropose(height uint, round uint) {
 
 		// Set the block.Header.StateHash.
 		// TODO: we could cache the resulting state to cs.stagedState.
+		// TODO: This is confusing, not clear that we're mutating block.
 		cs.state.Copy().AppendBlock(block, PartSetHeader{}, false)
 
 		blockParts = NewPartSetFromData(BinaryBytes(block))
@@ -621,14 +627,18 @@ func (cs *ConsensusState) RunActionPropose(height uint, round uint) {
 
 	// Make proposal
 	proposal := NewProposal(cs.Height, cs.Round, blockParts.Header(), polParts.Header())
-	proposal.Signature = cs.PrivValidator.SignProposal(proposal)
-
-	// Set fields
-	cs.Proposal = proposal
-	cs.ProposalBlock = block
-	cs.ProposalBlockParts = blockParts
-	cs.ProposalPOL = pol
-	cs.ProposalPOLParts = polParts
+	err := cs.PrivValidator.SignProposal(proposal)
+	if err == nil {
+		log.Info("Signed and set proposal", "height", cs.Height, "round", cs.Round, "proposal", proposal)
+		// Set fields
+		cs.Proposal = proposal
+		cs.ProposalBlock = block
+		cs.ProposalBlockParts = blockParts
+		cs.ProposalPOL = pol
+		cs.ProposalPOLParts = polParts
+	} else {
+		log.Warn("Error signing proposal", "height", cs.Height, "round", cs.Round, "error", err)
+	}
 }
 
 // Prevote for LockedBlock if we're locked, or ProposealBlock if valid.
@@ -1015,9 +1025,15 @@ func (cs *ConsensusState) signAddVote(type_ byte, hash []byte, header PartSetHea
 		BlockHash:  hash,
 		BlockParts: header,
 	}
-	vote.Signature = cs.PrivValidator.SignVote(vote)
-	cs.addVote(cs.PrivValidator.Address, vote)
-	return vote
+	err := cs.PrivValidator.SignVote(vote)
+	if err == nil {
+		log.Info("Signed and added vote", "height", cs.Height, "round", cs.Round, "vote", vote)
+		cs.addVote(cs.PrivValidator.Address, vote)
+		return vote
+	} else {
+		log.Warn("Error signing vote", "height", cs.Height, "round", cs.Round, "vote", vote, "error", err)
+		return nil
+	}
 }
 
 func (cs *ConsensusState) saveCommitVoteBlock(block *Block, blockParts *PartSet) {

consensus/state_test.go

@@ -15,7 +15,10 @@ func TestSetupRound(t *testing.T) {
 	voteTypes := []byte{VoteTypePrevote, VoteTypePrecommit, VoteTypeCommit}
 	for _, voteType := range voteTypes {
 		vote := &Vote{Height: 1, Round: 0, Type: voteType} // nil vote
-		vote.Signature = val0.SignVote(vote)
+		err := val0.SignVote(vote)
+		if err != nil {
+			t.Error("Error signing vote: %v", err)
+		}
 		cs.AddVote(val0.Address, vote)
 	}
 
@@ -120,7 +123,10 @@ func TestRunActionPrecommitCommitFinalize(t *testing.T) {
 			BlockHash:  cs.ProposalBlock.Hash(),
 			BlockParts: cs.ProposalBlockParts.Header(),
 		}
-		vote.Signature = privValidators[i].SignVote(vote)
+		err := privValidators[i].SignVote(vote)
+		if err != nil {
+			t.Error("Error signing vote: %v", err)
+		}
 		cs.AddVote(privValidators[i].Address, vote)
 	}
 
@@ -147,7 +153,10 @@ func TestRunActionPrecommitCommitFinalize(t *testing.T) {
 			BlockHash:  cs.ProposalBlock.Hash(),
 			BlockParts: cs.ProposalBlockParts.Header(),
 		}
-		vote.Signature = privValidators[i].SignVote(vote)
+		err := privValidators[i].SignVote(vote)
+		if err != nil {
+			t.Error("Error signing vote: %v", err)
+		}
 		added, _, err := cs.AddVote(privValidators[i].Address, vote)
 		if !added || err != nil {
 			t.Errorf("Error adding precommit: %v", err)
@@ -182,7 +191,10 @@ func TestRunActionPrecommitCommitFinalize(t *testing.T) {
 			BlockHash:  cs.ProposalBlock.Hash(),
 			BlockParts: cs.ProposalBlockParts.Header(),
 		}
-		vote.Signature = privValidators[i].SignVote(vote)
+		err := privValidators[i].SignVote(vote)
+		if err != nil {
+			t.Error("Error signing vote: %v", err)
+		}
 		added, _, err := cs.AddVote(privValidators[i].Address, vote)
 		if !added || err != nil {
 			t.Errorf("Error adding commit: %v", err)

consensus/vote_set_test.go

@@ -49,7 +49,7 @@ func withBlockParts(vote *Vote, blockParts PartSetHeader) *Vote {
 }
 
 func signAddVote(privVal *state.PrivValidator, vote *Vote, voteSet *VoteSet) (bool, error) {
-	vote.Signature = privVal.SignVoteUnsafe(vote)
+	privVal.SignVoteUnsafe(vote)
 	added, _, err := voteSet.Add(privVal.Address, vote)
 	return added, err
 }

state/priv_validator.go

@@ -6,6 +6,7 @@ import (
 	"bytes"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"math"
@@ -151,27 +152,27 @@ func (privVal *PrivValidator) JSONBytes() []byte {
 }
 
 // TODO: test
-func (privVal *PrivValidator) SignVote(vote *Vote) SignatureEd25519 {
+func (privVal *PrivValidator) SignVote(vote *Vote) error {
 	privVal.mtx.Lock()
 	defer privVal.mtx.Unlock()
 
 	// If height regression, panic
 	if privVal.LastHeight > vote.Height {
-		panic("Height regression in SignVote")
+		return errors.New("Height regression in SignVote")
 	}
 	// More cases for when the height matches
 	if privVal.LastHeight == vote.Height {
 		// If attempting any sign after commit, panic
 		if privVal.LastStep == stepCommit {
-			panic("SignVote on matching height after a commit")
+			return errors.New("SignVote on matching height after a commit")
 		}
 		// If round regression, panic
 		if privVal.LastRound > vote.Round {
-			panic("Round regression in SignVote")
+			return errors.New("Round regression in SignVote")
 		}
 		// If step regression, panic
 		if privVal.LastRound == vote.Round && privVal.LastStep > voteToStep(vote) {
-			panic("Step regression in SignVote")
+			return errors.New("Step regression in SignVote")
 		}
 	}
 
@@ -182,14 +183,15 @@ func (privVal *PrivValidator) SignVote(vote *Vote) SignatureEd25519 {
 	privVal.save()
 
 	// Sign
-	return privVal.SignVoteUnsafe(vote)
+	privVal.SignVoteUnsafe(vote)
+	return nil
 }
 
-func (privVal *PrivValidator) SignVoteUnsafe(vote *Vote) SignatureEd25519 {
-	return privVal.PrivKey.Sign(SignBytes(vote)).(SignatureEd25519)
+func (privVal *PrivValidator) SignVoteUnsafe(vote *Vote) {
+	vote.Signature = privVal.PrivKey.Sign(SignBytes(vote)).(SignatureEd25519)
 }
 
-func (privVal *PrivValidator) SignProposal(proposal *Proposal) SignatureEd25519 {
+func (privVal *PrivValidator) SignProposal(proposal *Proposal) error {
 	privVal.mtx.Lock()
 	defer privVal.mtx.Unlock()
 	if privVal.LastHeight < proposal.Height ||
@@ -203,13 +205,14 @@ func (privVal *PrivValidator) SignProposal(proposal *Proposal) SignatureEd25519
 		privVal.save()
 
 		// Sign
-		return privVal.PrivKey.Sign(SignBytes(proposal)).(SignatureEd25519)
+		proposal.Signature = privVal.PrivKey.Sign(SignBytes(proposal)).(SignatureEd25519)
+		return nil
 	} else {
-		panic(fmt.Sprintf("Attempt of duplicate signing of proposal: Height %v, Round %v", proposal.Height, proposal.Round))
+		return errors.New(fmt.Sprintf("Attempt of duplicate signing of proposal: Height %v, Round %v", proposal.Height, proposal.Round))
 	}
 }
 
-func (privVal *PrivValidator) SignRebondTx(rebondTx *RebondTx) SignatureEd25519 {
+func (privVal *PrivValidator) SignRebondTx(rebondTx *RebondTx) error {
 	privVal.mtx.Lock()
 	defer privVal.mtx.Unlock()
 	if privVal.LastHeight < rebondTx.Height {
@@ -221,9 +224,10 @@ func (privVal *PrivValidator) SignRebondTx(rebondTx *RebondTx) SignatureEd25519
 		privVal.save()
 
 		// Sign
-		return privVal.PrivKey.Sign(SignBytes(rebondTx)).(SignatureEd25519)
+		rebondTx.Signature = privVal.PrivKey.Sign(SignBytes(rebondTx)).(SignatureEd25519)
+		return nil
 	} else {
-		panic(fmt.Sprintf("Attempt of duplicate signing of rebondTx: Height %v", rebondTx.Height))
+		return errors.New(fmt.Sprintf("Attempt of duplicate signing of rebondTx: Height %v", rebondTx.Height))
 	}
 }