Merge pull request #3675 from alexanderbez/bez/3672-improve-AddSignatureFromPubKey-error

Improve error message returned from AddSignatureFromPubKey

crypto/multisig/multisignature.go

@@ -1,7 +1,8 @@
 package multisig
 
 import (
-	"errors"
+	"fmt"
+	"strings"
 
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/crypto/multisig/bitarray"
@@ -53,13 +54,19 @@ func (mSig *Multisignature) AddSignature(sig []byte, index int) {
 	mSig.Sigs[newSigIndex] = sig
 }
 
-// AddSignatureFromPubKey adds a signature to the multisig,
-// at the index in keys corresponding to the provided pubkey.
+// AddSignatureFromPubKey adds a signature to the multisig, at the index in
+// keys corresponding to the provided pubkey.
 func (mSig *Multisignature) AddSignatureFromPubKey(sig []byte, pubkey crypto.PubKey, keys []crypto.PubKey) error {
 	index := getIndex(pubkey, keys)
 	if index == -1 {
-		return errors.New("provided key didn't exist in pubkeys")
+		keysStr := make([]string, len(keys))
+		for i, k := range keys {
+			keysStr[i] = fmt.Sprintf("%X", k.Bytes())
+		}
+
+		return fmt.Errorf("provided key %X doesn't exist in pubkeys: \n%s", pubkey.Bytes(), strings.Join(keysStr, "\n"))
 	}
+
 	mSig.AddSignature(sig, index)
 	return nil
 }

crypto/multisig/threshold_pubkey_test.go

@@ -36,30 +36,68 @@ func TestThresholdMultisigValidCases(t *testing.T) {
 	for tcIndex, tc := range cases {
 		multisigKey := NewPubKeyMultisigThreshold(tc.k, tc.pubkeys)
 		multisignature := NewMultisig(len(tc.pubkeys))
+
 		for i := 0; i < tc.k-1; i++ {
 			signingIndex := tc.signingIndices[i]
-			multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys)
-			require.False(t, multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()),
-				"multisig passed when i < k, tc %d, i %d", tcIndex, i)
-			multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys)
-			require.Equal(t, i+1, len(multisignature.Sigs),
-				"adding a signature for the same pubkey twice increased signature count by 2, tc %d", tcIndex)
+			require.NoError(
+				t,
+				multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys),
+			)
+			require.False(
+				t,
+				multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()),
+				"multisig passed when i < k, tc %d, i %d", tcIndex, i,
+			)
+			require.NoError(
+				t,
+				multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys),
+			)
+			require.Equal(
+				t,
+				i+1,
+				len(multisignature.Sigs),
+				"adding a signature for the same pubkey twice increased signature count by 2, tc %d", tcIndex,
+			)
 		}
-		require.False(t, multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()),
-			"multisig passed with k - 1 sigs, tc %d", tcIndex)
-		multisignature.AddSignatureFromPubKey(tc.signatures[tc.signingIndices[tc.k]], tc.pubkeys[tc.signingIndices[tc.k]], tc.pubkeys)
-		require.True(t, multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()),
-			"multisig failed after k good signatures, tc %d", tcIndex)
+
+		require.False(
+			t,
+			multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()),
+			"multisig passed with k - 1 sigs, tc %d", tcIndex,
+		)
+		require.NoError(
+			t,
+			multisignature.AddSignatureFromPubKey(tc.signatures[tc.signingIndices[tc.k]], tc.pubkeys[tc.signingIndices[tc.k]], tc.pubkeys),
+		)
+		require.True(
+			t,
+			multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()),
+			"multisig failed after k good signatures, tc %d", tcIndex,
+		)
+
 		for i := tc.k + 1; i < len(tc.signingIndices); i++ {
 			signingIndex := tc.signingIndices[i]
-			multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys)
-			require.Equal(t, tc.passAfterKSignatures[i-tc.k-1],
-				multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()),
-				"multisig didn't verify as expected after k sigs, tc %d, i %d", tcIndex, i)
 
-			multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys)
-			require.Equal(t, i+1, len(multisignature.Sigs),
-				"adding a signature for the same pubkey twice increased signature count by 2, tc %d", tcIndex)
+			require.NoError(
+				t,
+				multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys),
+			)
+			require.Equal(
+				t,
+				tc.passAfterKSignatures[i-tc.k-1],
+				multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()),
+				"multisig didn't verify as expected after k sigs, tc %d, i %d", tcIndex, i,
+			)
+			require.NoError(
+				t,
+				multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys),
+			)
+			require.Equal(
+				t,
+				i+1,
+				len(multisignature.Sigs),
+				"adding a signature for the same pubkey twice increased signature count by 2, tc %d", tcIndex,
+			)
 		}
 	}
 }