priv-val: fix timestamp for signing things that only differ by timestamp

node/node.go

@@ -185,9 +185,9 @@ func NewNode(config *cfg.Config,
 
 	// Log whether this node is a validator or an observer
 	if state.Validators.HasAddress(privValidator.GetAddress()) {
-		consensusLogger.Info("This node is a validator")
+		consensusLogger.Info("This node is a validator", "addr", privValidator.GetAddress(), "pubKey", privValidator.GetPubKey())
 	} else {
-		consensusLogger.Info("This node is not a validator")
+		consensusLogger.Info("This node is not a validator", "addr", privValidator.GetAddress(), "pubKey", privValidator.GetPubKey())
 	}
 
 	// Make MempoolReactor

types/priv_validator.go

@@ -17,10 +17,10 @@ import (
 
 // TODO: type ?
 const (
-	stepNone      = 0 // Used to distinguish the initial state
-	stepPropose   = 1
-	stepPrevote   = 2
-	stepPrecommit = 3
+	stepNone      int8 = 0 // Used to distinguish the initial state
+	stepPropose   int8 = 1
+	stepPrevote   int8 = 2
+	stepPrecommit int8 = 3
 )
 
 func voteToStep(vote *Vote) int8 {
@@ -199,12 +199,9 @@ func (privVal *PrivValidatorFS) Reset() {
 func (privVal *PrivValidatorFS) SignVote(chainID string, vote *Vote) error {
 	privVal.mtx.Lock()
 	defer privVal.mtx.Unlock()
-	signature, err := privVal.signBytesHRS(vote.Height, vote.Round, voteToStep(vote),
-		SignBytes(chainID, vote), checkVotesOnlyDifferByTimestamp)
-	if err != nil {
+	if err := privVal.signVote(chainID, vote); err != nil {
 		return errors.New(cmn.Fmt("Error signing vote: %v", err))
 	}
-	vote.Signature = signature
 	return nil
 }
 
@@ -213,12 +210,9 @@ func (privVal *PrivValidatorFS) SignVote(chainID string, vote *Vote) error {
 func (privVal *PrivValidatorFS) SignProposal(chainID string, proposal *Proposal) error {
 	privVal.mtx.Lock()
 	defer privVal.mtx.Unlock()
-	signature, err := privVal.signBytesHRS(proposal.Height, proposal.Round, stepPropose,
-		SignBytes(chainID, proposal), checkProposalsOnlyDifferByTimestamp)
-	if err != nil {
+	if err := privVal.signProposal(chainID, proposal); err != nil {
 		return fmt.Errorf("Error signing proposal: %v", err)
 	}
-	proposal.Signature = signature
 	return nil
 }
 
@@ -250,36 +244,82 @@ func (privVal *PrivValidatorFS) checkHRS(height int64, round int, step int8) (bo
 	return false, nil
 }
 
-// signBytesHRS signs the given signBytes if the height/round/step (HRS) are
-// greater than the latest state. If the HRS are equal and the only thing changed is the timestamp,
-// it returns the privValidator.LastSignature. Else it returns an error.
-func (privVal *PrivValidatorFS) signBytesHRS(height int64, round int, step int8,
-	signBytes []byte, checkFn checkOnlyDifferByTimestamp) (crypto.Signature, error) {
-	sig := crypto.Signature{}
+// signVote checks if the vote is good to sign and sets the vote signature.
+// It may need to set the timestamp as well if the vote is otherwise the same as
+// a previously signed vote (ie. we crashed after signing but before the vote hit the WAL).
+func (privVal *PrivValidatorFS) signVote(chainID string, vote *Vote) error {
+	height, round, step := vote.Height, vote.Round, voteToStep(vote)
+	signBytes := SignBytes(chainID, vote)
 
 	sameHRS, err := privVal.checkHRS(height, round, step)
 	if err != nil {
-		return sig, err
+		return err
 	}
 
 	// We might crash before writing to the wal,
-	// causing us to try to re-sign for the same HRS
+	// causing us to try to re-sign for the same HRS.
+	// If signbytes are the same, use the last signature.
+	// If they only differ by timestamp, use last timestamp and signature
+	// Otherwise, return error
 	if sameHRS {
-		// if they're the same or only differ by timestamp,
-		// return the LastSignature. Otherwise, error
-		if bytes.Equal(signBytes, privVal.LastSignBytes) ||
-			checkFn(privVal.LastSignBytes, signBytes) {
-			return privVal.LastSignature, nil
+		if bytes.Equal(signBytes, privVal.LastSignBytes) {
+			vote.Signature = privVal.LastSignature
+		} else if timestamp, ok := checkVotesOnlyDifferByTimestamp(privVal.LastSignBytes, signBytes); ok {
+			vote.Timestamp = timestamp
+			vote.Signature = privVal.LastSignature
+		} else {
+			err = fmt.Errorf("Conflicting data")
 		}
-		return sig, fmt.Errorf("Conflicting data")
+		return err
 	}
 
-	sig, err = privVal.Sign(signBytes)
+	// It passed the checks. Sign the vote
+	sig, err := privVal.Sign(signBytes)
 	if err != nil {
-		return sig, err
+		return err
 	}
 	privVal.saveSigned(height, round, step, signBytes, sig)
-	return sig, nil
+	vote.Signature = sig
+	return nil
+}
+
+// signProposal checks if the proposal is good to sign and sets the proposal signature.
+// It may need to set the timestamp as well if the proposal is otherwise the same as
+// a previously signed proposal ie. we crashed after signing but before the proposal hit the WAL).
+func (privVal *PrivValidatorFS) signProposal(chainID string, proposal *Proposal) error {
+	height, round, step := proposal.Height, proposal.Round, stepPropose
+	signBytes := SignBytes(chainID, proposal)
+
+	sameHRS, err := privVal.checkHRS(height, round, step)
+	if err != nil {
+		return err
+	}
+
+	// We might crash before writing to the wal,
+	// causing us to try to re-sign for the same HRS.
+	// If signbytes are the same, use the last signature.
+	// If they only differ by timestamp, use last timestamp and signature
+	// Otherwise, return error
+	if sameHRS {
+		if bytes.Equal(signBytes, privVal.LastSignBytes) {
+			proposal.Signature = privVal.LastSignature
+		} else if timestamp, ok := checkProposalsOnlyDifferByTimestamp(privVal.LastSignBytes, signBytes); ok {
+			proposal.Timestamp = timestamp
+			proposal.Signature = privVal.LastSignature
+		} else {
+			err = fmt.Errorf("Conflicting data")
+		}
+		return err
+	}
+
+	// It passed the checks. Sign the proposal
+	sig, err := privVal.Sign(signBytes)
+	if err != nil {
+		return err
+	}
+	privVal.saveSigned(height, round, step, signBytes, sig)
+	proposal.Signature = sig
+	return nil
 }
 
 // Persist height/round/step and signature
@@ -331,8 +371,9 @@ func (pvs PrivValidatorsByAddress) Swap(i, j int) {
 
 type checkOnlyDifferByTimestamp func([]byte, []byte) bool
 
-// returns true if the only difference in the votes is their timestamp
-func checkVotesOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) bool {
+// returns the timestamp from the lastSignBytes.
+// returns true if the only difference in the votes is their timestamp.
+func checkVotesOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) (time.Time, bool) {
 	var lastVote, newVote CanonicalJSONOnceVote
 	if err := json.Unmarshal(lastSignBytes, &lastVote); err != nil {
 		panic(fmt.Sprintf("LastSignBytes cannot be unmarshalled into vote: %v", err))
@@ -341,6 +382,11 @@ func checkVotesOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) bool {
 		panic(fmt.Sprintf("signBytes cannot be unmarshalled into vote: %v", err))
 	}
 
+	lastTime, err := time.Parse(timeFormat, lastVote.Vote.Timestamp)
+	if err != nil {
+		panic(err)
+	}
+
 	// set the times to the same value and check equality
 	now := CanonicalTime(time.Now())
 	lastVote.Vote.Timestamp = now
@@ -348,11 +394,12 @@ func checkVotesOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) bool {
 	lastVoteBytes, _ := json.Marshal(lastVote)
 	newVoteBytes, _ := json.Marshal(newVote)
 
-	return bytes.Equal(newVoteBytes, lastVoteBytes)
+	return lastTime, bytes.Equal(newVoteBytes, lastVoteBytes)
 }
 
+// returns the timestamp from the lastSignBytes.
 // returns true if the only difference in the proposals is their timestamp
-func checkProposalsOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) bool {
+func checkProposalsOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) (time.Time, bool) {
 	var lastProposal, newProposal CanonicalJSONOnceProposal
 	if err := json.Unmarshal(lastSignBytes, &lastProposal); err != nil {
 		panic(fmt.Sprintf("LastSignBytes cannot be unmarshalled into proposal: %v", err))
@@ -361,6 +408,11 @@ func checkProposalsOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) boo
 		panic(fmt.Sprintf("signBytes cannot be unmarshalled into proposal: %v", err))
 	}
 
+	lastTime, err := time.Parse(timeFormat, lastProposal.Proposal.Timestamp)
+	if err != nil {
+		panic(err)
+	}
+
 	// set the times to the same value and check equality
 	now := CanonicalTime(time.Now())
 	lastProposal.Proposal.Timestamp = now
@@ -368,5 +420,5 @@ func checkProposalsOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) boo
 	lastProposalBytes, _ := json.Marshal(lastProposal)
 	newProposalBytes, _ := json.Marshal(newProposal)
 
-	return bytes.Equal(newProposalBytes, lastProposalBytes)
+	return lastTime, bytes.Equal(newProposalBytes, lastProposalBytes)
 }

types/priv_validator_test.go

@@ -173,6 +173,58 @@ func TestSignProposal(t *testing.T) {
 	assert.Equal(sig, proposal.Signature)
 }
 
+func TestDifferByTimestamp(t *testing.T) {
+	_, tempFilePath := cmn.Tempfile("priv_validator_")
+	privVal := GenPrivValidatorFS(tempFilePath)
+
+	block1 := PartSetHeader{5, []byte{1, 2, 3}}
+	height, round := int64(10), 1
+	chainID := "mychainid"
+
+	// test proposal
+	{
+		proposal := newProposal(height, round, block1)
+		err := privVal.SignProposal(chainID, proposal)
+		assert.NoError(t, err, "expected no error signing proposal")
+		signBytes := SignBytes(chainID, proposal)
+		sig := proposal.Signature
+		timeStamp := clipToMS(proposal.Timestamp)
+
+		// manipulate the timestamp. should get changed back
+		proposal.Timestamp = proposal.Timestamp.Add(time.Millisecond)
+		proposal.Signature = crypto.Signature{}
+		err = privVal.SignProposal("mychainid", proposal)
+		assert.NoError(t, err, "expected no error on signing same proposal")
+
+		assert.Equal(t, timeStamp, proposal.Timestamp)
+		assert.Equal(t, signBytes, SignBytes(chainID, proposal))
+		assert.Equal(t, sig, proposal.Signature)
+	}
+
+	// test vote
+	{
+		voteType := VoteTypePrevote
+		blockID := BlockID{[]byte{1, 2, 3}, PartSetHeader{}}
+		vote := newVote(privVal.Address, 0, height, round, voteType, blockID)
+		err := privVal.SignVote("mychainid", vote)
+		assert.NoError(t, err, "expected no error signing vote")
+
+		signBytes := SignBytes(chainID, vote)
+		sig := vote.Signature
+		timeStamp := clipToMS(vote.Timestamp)
+
+		// manipulate the timestamp. should get changed back
+		vote.Timestamp = vote.Timestamp.Add(time.Millisecond)
+		vote.Signature = crypto.Signature{}
+		err = privVal.SignVote("mychainid", vote)
+		assert.NoError(t, err, "expected no error on signing same vote")
+
+		assert.Equal(t, timeStamp, vote.Timestamp)
+		assert.Equal(t, signBytes, SignBytes(chainID, vote))
+		assert.Equal(t, sig, vote.Signature)
+	}
+}
+
 func newVote(addr data.Bytes, idx int, height int64, round int, typ byte, blockID BlockID) *Vote {
 	return &Vote{
 		ValidatorAddress: addr,
@@ -190,5 +242,13 @@ func newProposal(height int64, round int, partsHeader PartSetHeader) *Proposal {
 		Height:           height,
 		Round:            round,
 		BlockPartsHeader: partsHeader,
+		Timestamp:        time.Now().UTC(),
 	}
 }
+
+func clipToMS(t time.Time) time.Time {
+	nano := t.UnixNano()
+	million := int64(1000000)
+	nano = (nano / million) * million
+	return time.Unix(0, nano).UTC()
+}