improved logging, verification of threshold signatures immediately af…

…ter them being made, dont allow state signing when voting nil on blocks

privval/dash_core_signer_client.go

@@ -204,17 +204,11 @@ func (sc *DashCoreSignerClient) SignVote(chainID string, quorumType btcjson.LLMQ
 	if len(quorumHash) != crypto.DefaultHashSize {
 		return fmt.Errorf("quorum hash is not the right length %s", quorumHash.String())
 	}
-	blockSignBytes := types.VoteBlockSignBytes(chainID, protoVote)
-	stateSignBytes := types.VoteStateSignBytes(chainID, protoVote)
 
+	blockSignBytes := types.VoteBlockSignBytes(chainID, protoVote)
 	blockMessageHash := crypto.Sha256(blockSignBytes)
-
-	stateMessageHash := crypto.Sha256(stateSignBytes)
-
 	blockRequestId := types.VoteBlockRequestIdProto(protoVote)
 
-	stateRequestId := types.VoteStateRequestIdProto(protoVote)
-
 	blockResponse, err := sc.dashCoreRpcClient.QuorumSign(quorumType, blockRequestId, blockMessageHash, quorumHash)
 
 	if blockResponse == nil {
@@ -228,21 +222,24 @@ func (sc *DashCoreSignerClient) SignVote(chainID string, quorumType btcjson.LLMQ
 	//
 	blockDecodedSignature, err := hex.DecodeString(blockResponse.Signature)
 	if err != nil {
-		return fmt.Errorf("error decoding signature when signing proposal : %v", err)
+		return fmt.Errorf("error decoding signature when signing vote : %v", err)
 	}
 	if len(blockDecodedSignature) != bls12381.SignatureSize {
-		return fmt.Errorf("decoding signature %d is incorrect size when signing proposal : %v", len(blockDecodedSignature), err)
+		return fmt.Errorf("decoding signature %d is incorrect size when signing vote : %v", len(blockDecodedSignature), err)
 	}
 
 	proTxHash, err := sc.GetProTxHash()
 
-
 	signID := crypto.SignId(quorumType, bls12381.ReverseBytes(quorumHash), bls12381.ReverseBytes(blockRequestId), bls12381.ReverseBytes(blockMessageHash))
 
-	logger.Debug("signed vote", "height", protoVote.Height, "round", protoVote.Round, "quorumType", quorumType,
-		"quorumHash", quorumHash, "signature", blockDecodedSignature, "signBytes", blockSignBytes, "proTxHash", proTxHash,
-		"signId", signID, "coreBlockRequestId", blockResponse.ID, "blockRequestId", hex.EncodeToString(blockRequestId), "coreSignId", blockResponse.SignHash,
-		"signId", hex.EncodeToString(signID))
+	coreSignID, err := hex.DecodeString(blockResponse.SignHash)
+	if err != nil {
+		return fmt.Errorf("error decoding coreSignID when signing vote : %v", err)
+	}
+	logger.Debug("signed vote", "height", protoVote.Height, "round", protoVote.Round, "voteType", protoVote.Type,
+		"quorumType", quorumType, "quorumHash", quorumHash, "signature", blockDecodedSignature, "signBytes", blockSignBytes,
+		"proTxHash", proTxHash, "signId", signID, "coreBlockRequestId", blockResponse.ID, "blockRequestId", hex.EncodeToString(blockRequestId),
+		"coreSignId", bls12381.ReverseBytes(coreSignID), "signId", hex.EncodeToString(signID))
 
 	pubKey, err := sc.GetPubKey(quorumHash)
 	verified := pubKey.VerifySignatureDigest(signID, blockDecodedSignature)
@@ -252,23 +249,34 @@ func (sc *DashCoreSignerClient) SignVote(chainID string, quorumType btcjson.LLMQ
 		logger.Error("Unable to verify signature", "height", protoVote.Height, "round", protoVote.Round, "pubkey", pubKey)
 	}
 
-	stateResponse, err := sc.dashCoreRpcClient.QuorumSign(sc.defaultQuorumType, stateRequestId, stateMessageHash, quorumHash)
+	protoVote.BlockSignature = blockDecodedSignature
 
-	if stateResponse == nil {
-		return ErrUnexpectedResponse
-	}
-	if err != nil {
-		return &RemoteSignerError{Code: 500, Description: err.Error()}
-	}
+	// Only sign the state when voting for the block
+	if protoVote.BlockID.Hash != nil {
+		stateSignBytes := types.VoteStateSignBytes(chainID, protoVote)
+		stateMessageHash := crypto.Sha256(stateSignBytes)
+		stateRequestId := types.VoteStateRequestIdProto(protoVote)
 
-	stateDecodedSignature, err := hex.DecodeString(stateResponse.Signature)
-	if err != nil {
-		return fmt.Errorf("error decoding signature when signing proposal : %v", err)
-	}
-	if len(stateDecodedSignature) != bls12381.SignatureSize {
-		return fmt.Errorf("decoding signature %d is incorrect size when signing proposal : %v", len(stateDecodedSignature), err)
+		stateResponse, err := sc.dashCoreRpcClient.QuorumSign(sc.defaultQuorumType, stateRequestId, stateMessageHash, quorumHash)
+
+		if stateResponse == nil {
+			return ErrUnexpectedResponse
+		}
+		if err != nil {
+			return &RemoteSignerError{Code: 500, Description: err.Error()}
+		}
+
+		stateDecodedSignature, err := hex.DecodeString(stateResponse.Signature)
+		if err != nil {
+			return fmt.Errorf("error decoding signature when signing proposal : %v", err)
+		}
+		if len(stateDecodedSignature) != bls12381.SignatureSize {
+			return fmt.Errorf("decoding signature %d is incorrect size when signing proposal : %v", len(stateDecodedSignature), err)
+		}
+		protoVote.StateSignature = stateDecodedSignature
 	}
 
+
 	// fmt.Printf("Signed Vote proTxHash %s stateSignBytes %s block signature %s \n", proTxHash, hex.EncodeToString(stateSignBytes),
 	// 	hex.EncodeToString(stateDecodedSignature))
 
@@ -285,8 +293,6 @@ func (sc *DashCoreSignerClient) SignVote(chainID string, quorumType btcjson.LLMQ
 	//	fmt.Printf("Unable to verify state signature %v\n", pubKey)
 	//}
 
-	protoVote.BlockSignature = blockDecodedSignature
-	protoVote.StateSignature = stateDecodedSignature
 
 	return nil
 }

state/execution.go

@@ -231,7 +231,8 @@ func (blockExec *BlockExecutor) ApplyBlockWithLogger(
 		return state, 0, err
 	}
 	if len(validatorUpdates) > 0 {
-		blockExec.logger.Debug("updates to validators", "updates", types.ValidatorListString(validatorUpdates))
+		blockExec.logger.Debug("updates to validators", "quorumHash", quorumHash, "thresholdPublicKey",
+			thresholdPublicKeyUpdate, "updates", types.ValidatorListString(validatorUpdates))
 	}
 
 	blockExec.store.Load()

state/validation.go

@@ -105,7 +105,7 @@ func validateBlock(state State, block *types.Block) error {
 		// LastPrecommits.Signatures length is checked in VerifyCommit.
 		if err := state.LastValidators.VerifyCommit(
 			state.ChainID, state.LastBlockID, state.LastStateID, block.Height-1, block.LastCommit); err != nil {
-			return err
+			return fmt.Errorf("error validating block: %v", err)
 		}
 	}
 

state/validation_test.go

@@ -173,17 +173,18 @@ func TestValidateBlockCommit(t *testing.T) {
 			block, _ := state.MakeBlock(height, nextChainLock, makeTxs(height), wrongHeightCommit, nil,
 				proTxHash)
 			err = blockExec.ValidateBlock(state, block)
-			_, isErrInvalidCommitHeight := err.(types.ErrInvalidCommitHeight)
-			require.True(t, isErrInvalidCommitHeight, "expected ErrInvalidCommitHeight at height %d but got: %v",
-				height, err)
-
+			require.True(t, strings.HasPrefix(err.Error(), "error validating block: Invalid commit -- wrong height:"),
+				"expected error on block threshold signature at height %d, but got: %v",
+				height,
+				err,
+			)
 			/*
 				Test that the threshold block signatures are good
 			*/
 			block, _ = state.MakeBlock(height, nextChainLock, makeTxs(height), wrongSignorCommit, nil, proTxHash)
 			err = blockExec.ValidateBlock(state, block)
 			require.Error(t, err)
-			require.True(t, strings.HasPrefix(err.Error(), "incorrect threshold block signature"),
+			require.True(t, strings.HasPrefix(err.Error(), "error validating block: incorrect threshold block signature"),
 				"expected error on block threshold signature at height %d, but got: %v",
 				height,
 				err,
@@ -195,7 +196,7 @@ func TestValidateBlockCommit(t *testing.T) {
 			block, _ = state.MakeBlock(height, nextChainLock, makeTxs(height), wrongVoteMessageSignedCommit, nil, proTxHash)
 			err = blockExec.ValidateBlock(state, block)
 			require.Error(t, err)
-			require.True(t, strings.HasPrefix(err.Error(), "incorrect threshold block signature"),
+			require.True(t, strings.HasPrefix(err.Error(), "error validating block: incorrect threshold block signature"),
 				"expected error on block threshold signature at height %d, but got: %v",
 				height,
 				err,

types/priv_validator.go

@@ -185,7 +185,7 @@ func (pv *MockPV) SignVote(chainID string, quorumType btcjson.LLMQType, quorumHa
 	}
 
 	blockSignID := VoteBlockSignId(useChainID, vote, quorumType, quorumHash)
-	stateSignId := VoteStateSignId(useChainID, vote, quorumType, quorumHash)
+
 
 	var privKey crypto.PrivKey
 	if quorumKeys, ok := pv.PrivateKeys[quorumHash.String()]; ok {
@@ -204,7 +204,8 @@ func (pv *MockPV) SignVote(chainID string, quorumType btcjson.LLMQType, quorumHa
 	}
 	vote.BlockSignature = blockSignature
 
-	if stateSignId != nil {
+	if vote.BlockID.Hash != nil {
+		stateSignId := VoteStateSignId(useChainID, vote, quorumType, quorumHash)
 		stateSignature, err := privKey.SignDigest(stateSignId)
 		if err != nil {
 			return err

types/validator_set.go

@@ -881,8 +881,8 @@ func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID, stateID
 
 	if !vals.ThresholdPublicKey.VerifySignatureDigest(blockSignID, commit.ThresholdBlockSignature) {
 		canonicalVoteBlockSignBytes := commit.CanonicalVoteVerifySignBytes(chainID)
-		return fmt.Errorf("incorrect threshold block signature bytes: %X signId %X commit: %v valQuorumType %d valQuorumHash %X",
-			canonicalVoteBlockSignBytes, blockSignID, commit, vals.QuorumType, vals.QuorumHash)
+		return fmt.Errorf("incorrect threshold block signature bytes: %X signId %X commit: %v valQuorumType %d valQuorumHash %X valThresholdPublicKey %X",
+			canonicalVoteBlockSignBytes, blockSignID, commit, vals.QuorumType, vals.QuorumHash, vals.ThresholdPublicKey)
 	}
 
 	stateSignId := commit.CanonicalVoteStateSignId(chainID, vals.QuorumType, vals.QuorumHash)

types/vote.go

@@ -40,6 +40,7 @@ var (
 	ErrVoteInvalidValidatorPubKeySize = errors.New("invalid validator public key size")
 	ErrVoteInvalidBlockSignature      = errors.New("invalid block signature")
 	ErrVoteInvalidStateSignature      = errors.New("invalid state signature")
+	ErrVoteStateSignatureShouldBeNil  = errors.New("state signature when voting for nil block")
 	ErrVoteInvalidBlockHash           = errors.New("invalid block hash")
 	ErrVoteNonDeterministicSignature  = errors.New("non-deterministic signature")
 	ErrVoteNil                        = errors.New("nil vote")
@@ -228,12 +229,12 @@ func VoteStateRequestIdProto(vote *tmproto.Vote) []byte {
 	return crypto.Sha256(requestIdMessage)
 }
 
-func (vote *Vote) Verify(chainID string, quorumType btcjson.LLMQType, quorumHash []byte, pubKey crypto.PubKey, proTxHash crypto.ProTxHash) error {
+func (vote *Vote) Verify(chainID string, quorumType btcjson.LLMQType, quorumHash []byte, pubKey crypto.PubKey, proTxHash crypto.ProTxHash) ([]byte, []byte, error) {
 	if !bytes.Equal(proTxHash, vote.ValidatorProTxHash) {
-		return ErrVoteInvalidValidatorProTxHash
+		return nil, nil, ErrVoteInvalidValidatorProTxHash
 	}
 	if len(pubKey.Bytes()) != bls12381.PubKeySize {
-		return ErrVoteInvalidValidatorPubKeySize
+		return nil, nil, ErrVoteInvalidValidatorPubKeySize
 	}
 	v := vote.ToProto()
 	voteBlockSignBytes := VoteBlockSignBytes(chainID, v)
@@ -248,28 +249,31 @@ func (vote *Vote) Verify(chainID string, quorumType btcjson.LLMQType, quorumHash
 	//	hex.EncodeToString(quorumHash), hex.EncodeToString(blockRequestId), hex.EncodeToString(blockMessageHash))
 
 	if !pubKey.VerifySignatureDigest(signID, vote.BlockSignature) {
-		return fmt.Errorf("%s proTxHash %s pubKey %v vote %v sign bytes %s block signature %s", ErrVoteInvalidBlockSignature.Error(),
+		return nil, nil, fmt.Errorf("%s proTxHash %s pubKey %v vote %v sign bytes %s block signature %s", ErrVoteInvalidBlockSignature.Error(),
 			proTxHash, pubKey, vote, hex.EncodeToString(voteBlockSignBytes), hex.EncodeToString(vote.BlockSignature))
 	}
 
+	stateSignId := []byte(nil)
 	// we must verify the stateID but only if the blockID isn't nil
 	if vote.BlockID.Hash != nil {
 		voteStateSignBytes := VoteStateSignBytes(chainID, v)
 		stateMessageHash := crypto.Sha256(voteStateSignBytes)
 
 		stateRequestId := VoteStateRequestId(vote)
 
-		stateSignId := crypto.SignId(quorumType, bls12381.ReverseBytes(quorumHash), bls12381.ReverseBytes(stateRequestId), bls12381.ReverseBytes(stateMessageHash))
+		stateSignId = crypto.SignId(quorumType, bls12381.ReverseBytes(quorumHash), bls12381.ReverseBytes(stateRequestId), bls12381.ReverseBytes(stateMessageHash))
 
 		// fmt.Printf("state vote verify sign Id %s (%d - %s  - %s  - %s)\n", hex.EncodeToString(stateSignId), quorumType,
 		//	hex.EncodeToString(quorumHash), hex.EncodeToString(stateRequestId), hex.EncodeToString(stateMessageHash))
 
 		if !pubKey.VerifySignatureDigest(stateSignId, vote.StateSignature) {
-			return ErrVoteInvalidStateSignature
+			return nil, nil, ErrVoteInvalidStateSignature
 		}
+	} else if vote.StateSignature != nil {
+		return nil, nil, ErrVoteStateSignatureShouldBeNil
 	}
 
-	return nil
+	return signID, stateSignId, nil
 }
 
 // ValidateBasic performs basic validation.

types/vote_set.go

@@ -211,13 +211,14 @@ func (voteSet *VoteSet) addVote(vote *Vote) (added bool, err error) {
 	}
 
 	// Check signature.
-	if err := vote.Verify(voteSet.chainID, voteSet.valSet.QuorumType, voteSet.valSet.QuorumHash, val.PubKey, val.ProTxHash); err != nil {
+	signId, stateSignId, err := vote.Verify(voteSet.chainID, voteSet.valSet.QuorumType, voteSet.valSet.QuorumHash, val.PubKey, val.ProTxHash)
+	if err != nil {
 		return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s ProTxHash %s: %w",
 			voteSet.chainID, val.PubKey, val.ProTxHash, err)
 	}
 
 	// Add vote and get conflicting vote if any.
-	added, conflicting := voteSet.addVerifiedVote(vote, blockKey, val.VotingPower)
+	added, conflicting := voteSet.addVerifiedVote(vote, blockKey, val.VotingPower, signId, stateSignId)
 	if conflicting != nil {
 		fmt.Printf("-----\n")
 		debug.PrintStack()
@@ -247,6 +248,8 @@ func (voteSet *VoteSet) addVerifiedVote(
 	vote *Vote,
 	blockKey string,
 	votingPower int64,
+	signId []byte,
+	stateSignId []byte,
 ) (added bool, conflicting *Vote) {
 	valIndex := vote.ValidatorIndex
 
@@ -309,7 +312,7 @@ func (voteSet *VoteSet) addVerifiedVote(
 			voteSet.maj23 = &maj23BlockID
 			voteSet.stateMaj23 = &stateMaj23StateID
 			if len(votesByBlock.votes) > 1 {
-				err := voteSet.recoverThresholdSigs(votesByBlock)
+				err := voteSet.recoverThresholdSigsAndVerify(votesByBlock, signId, stateSignId)
 				if err != nil {
 					// fmt.Printf("error %v quorum %d\n", err, quorum)
 					// for i, vote := range votesByBlock.votes {
@@ -334,6 +337,26 @@ func (voteSet *VoteSet) addVerifiedVote(
 	return true, conflicting
 }
 
+func (voteSet *VoteSet) recoverThresholdSigsAndVerify(blockVotes *blockVotes, signId []byte, stateSignId []byte) error {
+	err := voteSet.recoverThresholdSigs(blockVotes)
+	if err != nil {
+		return err
+	}
+	verified := voteSet.valSet.ThresholdPublicKey.VerifySignatureDigest(signId, voteSet.thresholdBlockSig)
+	if !verified {
+		thresholdBlockSig := voteSet.thresholdBlockSig
+		return fmt.Errorf("recovered incorrect threshold signature %v", thresholdBlockSig)
+	}
+	if voteSet.thresholdStateSig != nil {
+		verified = voteSet.valSet.ThresholdPublicKey.VerifySignatureDigest(stateSignId, voteSet.thresholdStateSig)
+		if !verified {
+			thresholdStateSig := voteSet.thresholdStateSig
+			return fmt.Errorf("recovered incorrect state threshold signature %v", thresholdStateSig)
+		}
+	}
+	return nil
+}
+
 func (voteSet *VoteSet) recoverThresholdSigs(blockVotes *blockVotes) error {
 	if len(blockVotes.votes) < 2 {
 		return fmt.Errorf("attempting to recover a threshold signature with only 1 vote")
@@ -353,6 +376,7 @@ func (voteSet *VoteSet) recoverThresholdSigs(blockVotes *blockVotes) error {
 		return fmt.Errorf("error recovering threshold block sig: %v", err)
 	}
 	voteSet.thresholdBlockSig = thresholdBlockSig
+
 	if voteSet.maj23 != nil && voteSet.maj23.Hash != nil {
 		// if the vote is voting for nil, then we do not care to recover the state signature
 		thresholdStateSig, err := bls12381.RecoverThresholdSignatureFromShares(stateSigs, blsIDs)

types/vote_test.go

@@ -238,12 +238,12 @@ func TestVoteVerify(t *testing.T) {
 	vote := examplePrevote()
 	vote.ValidatorProTxHash = proTxHash
 
-	err = vote.Verify("test_chain_id", quorumType, quorumHash, bls12381.GenPrivKey().PubKey(), crypto.RandProTxHash())
+	_, _, err = vote.Verify("test_chain_id", quorumType, quorumHash, bls12381.GenPrivKey().PubKey(), crypto.RandProTxHash())
 	if assert.Error(t, err) {
 		assert.Equal(t, ErrVoteInvalidValidatorProTxHash, err)
 	}
 
-	err = vote.Verify("test_chain_id", quorumType, quorumHash, pubkey, proTxHash)
+	_, _, err = vote.Verify("test_chain_id", quorumType, quorumHash, pubkey, proTxHash)
 	if assert.Error(t, err) {
 		assert.True(t, strings.HasPrefix(err.Error(), ErrVoteInvalidBlockSignature.Error())) // since block signatures are verified first
 	}