crypto: Change the batch verification API to be not-stupid

Having to redo quite a bit of computation in the event of a batch
failure if the caller is actually interested in which signature failed,
is rather sub-optimal.  Change the batch verification interface to
expose a one-shot batch verify + fallback call, so that sensible
libraries can handle this case faster.

This commit also leverages the failure information so that validation
will only ever call one of `verifyCommitBatch` or `verifyCommitSingle`.

crypto/batch/batch.go

@@ -20,3 +20,14 @@ func CreateBatchVerifier(pk crypto.PubKey) (crypto.BatchVerifier, bool) {
 	// case where the key does not support batch verification
 	return nil, false
 }
+
+// SupportsBatchVerifier checks if a key type implements the batch verifier
+// interface.
+func SupportsBatchVerifier(pk crypto.PubKey) bool {
+	switch pk.Type() {
+	case ed25519.KeyType, sr25519.KeyType:
+		return true
+	}
+
+	return false
+}

crypto/crypto.go

@@ -46,7 +46,9 @@ type Symmetric interface {
 type BatchVerifier interface {
 	// Add appends an entry into the BatchVerifier.
 	Add(key PubKey, message, signature []byte) error
-	// Verify verifies all the entries in the BatchVerifier.
-	// If the verification fails it is unknown which entry failed and each entry will need to be verified individually.
-	Verify() bool
+	// Verify verifies all the entries in the BatchVerifier, and returns
+	// if every signature in the batch is valid, and a vector of bools
+	// indicating the verification status of each signature (in the order
+	// that signatures were added to the batch).
+	Verify() (bool, []bool)
 }

crypto/ed25519/bench_test.go

@@ -59,7 +59,7 @@ func BenchmarkVerifyBatch(b *testing.B) {
 					require.NoError(b, err)
 				}
 
-				if !v.Verify() {
+				if ok, _ := v.Verify(); !ok {
 					b.Fatal("signature set failed batch verification")
 				}
 			}

crypto/ed25519/ed25519.go

@@ -215,6 +215,6 @@ func (b *BatchVerifier) Add(key crypto.PubKey, msg, signature []byte) error {
 	return nil
 }
 
-func (b *BatchVerifier) Verify() bool {
-	return b.BatchVerifier.VerifyBatchOnly(crypto.CReader())
+func (b *BatchVerifier) Verify() (bool, []bool) {
+	return b.BatchVerifier.Verify(crypto.CReader())
 }

crypto/ed25519/ed25519_test.go

@@ -50,5 +50,6 @@ func TestBatchSafe(t *testing.T) {
 		require.NoError(t, err)
 	}
 
-	require.True(t, v.Verify())
+	ok, _ := v.Verify()
+	require.True(t, ok)
 }

crypto/sr25519/batch.go

@@ -8,19 +8,31 @@ import (
 	"github.com/tendermint/tendermint/crypto"
 )
 
-var _ crypto.BatchVerifier = BatchVerifier{}
+var _ crypto.BatchVerifier = &BatchVerifier{}
 
 // BatchVerifier implements batch verification for sr25519.
 // https://github.com/ChainSafe/go-schnorrkel is used for batch verification
 type BatchVerifier struct {
 	*schnorrkel.BatchVerifier
+
+	// The go-schnorrkel API is terrible for performance if the chance
+	// that a batch fails is non-negligible, exact information about
+	// which signature failed is something that is desired, and the
+	// system is not resource constrained.
+	//
+	// The tendermint case meets all of the criteria, so emulate a
+	// one-shot with fallback API so that libraries that do provide
+	// sensible behavior aren't penalized.
+	pubKeys    []crypto.PubKey
+	messages   [][]byte
+	signatures [][]byte
 }
 
 func NewBatchVerifier() crypto.BatchVerifier {
-	return BatchVerifier{schnorrkel.NewBatchVerifier()}
+	return &BatchVerifier{schnorrkel.NewBatchVerifier(), nil, nil, nil}
 }
 
-func (b BatchVerifier) Add(key crypto.PubKey, msg, sig []byte) error {
+func (b *BatchVerifier) Add(key crypto.PubKey, msg, sig []byte) error {
 	var sig64 [SignatureSize]byte
 	copy(sig64[:], sig)
 	signature := new(schnorrkel.Signature)
@@ -34,9 +46,40 @@ func (b BatchVerifier) Add(key crypto.PubKey, msg, sig []byte) error {
 	var pk [PubKeySize]byte
 	copy(pk[:], key.Bytes())
 
-	return b.BatchVerifier.Add(signingContext, signature, schnorrkel.NewPublicKey(pk))
+	err = b.BatchVerifier.Add(signingContext, signature, schnorrkel.NewPublicKey(pk))
+	if err == nil {
+		b.pubKeys = append(b.pubKeys, key)
+		b.messages = append(b.messages, msg)
+		b.signatures = append(b.signatures, sig)
+	}
+
+	return err
 }
 
-func (b BatchVerifier) Verify() bool {
-	return b.BatchVerifier.Verify()
+func (b *BatchVerifier) Verify() (bool, []bool) {
+	// Explicitly mimic ed25519consensus/curve25519-voi behavior.
+	if len(b.pubKeys) == 0 {
+		return false, nil
+	}
+
+	// Optimistically assume everything will succeed.
+	valid := make([]bool, len(b.pubKeys))
+	for i := range valid {
+		valid[i] = true
+	}
+
+	// Fast path, every signature may be valid.
+	if b.BatchVerifier.Verify() {
+		return true, valid
+	}
+
+	// Fall-back to serial verification.
+	allValid := true
+	for i := range b.pubKeys {
+		pk, msg, sig := b.pubKeys[i], b.messages[i], b.signatures[i]
+		valid[i] = pk.VerifySignature(msg, sig)
+		allValid = allValid && valid[i]
+	}
+
+	return allValid, valid
 }

crypto/sr25519/bench_test.go

@@ -43,7 +43,7 @@ func BenchmarkVerifyBatch(b *testing.B) {
 			}
 			// NOTE: dividing by n so that metrics are per-signature
 			for i := 0; i < b.N/n; i++ {
-				if !v.Verify() {
+				if ok, _ := v.Verify(); !ok {
 					b.Fatal("signature set failed batch verification")
 				}
 			}

crypto/sr25519/sr25519_test.go

@@ -11,7 +11,6 @@ import (
 )
 
 func TestSignAndValidateSr25519(t *testing.T) {
-
 	privKey := sr25519.GenPrivKey()
 	pubKey := privKey.PubKey()
 
@@ -32,6 +31,7 @@ func TestSignAndValidateSr25519(t *testing.T) {
 
 func TestBatchSafe(t *testing.T) {
 	v := sr25519.NewBatchVerifier()
+	vFail := sr25519.NewBatchVerifier()
 	for i := 0; i <= 38; i++ {
 		priv := sr25519.GenPrivKey()
 		pub := priv.PubKey()
@@ -48,9 +48,27 @@ func TestBatchSafe(t *testing.T) {
 
 		err = v.Add(pub, msg, sig)
 		require.NoError(t, err)
+
+		switch i % 2 {
+		case 0:
+			err = vFail.Add(pub, msg, sig)
+		case 1:
+			msg[2] ^= byte(0x01)
+			err = vFail.Add(pub, msg, sig)
+		}
+		require.NoError(t, err)
+	}
+
+	ok, valid := v.Verify()
+	require.True(t, ok, "failed batch verification")
+	for i, ok := range valid {
+		require.Truef(t, ok, "sig[%d] should be marked valid", i)
 	}
 
-	if !v.Verify() {
-		t.Error("failed batch verification")
+	ok, valid = vFail.Verify()
+	require.False(t, ok, "succeeded batch verification (invalid batch)")
+	for i, ok := range valid {
+		expected := (i % 2) == 0
+		require.Equalf(t, expected, ok, "sig[%d] should be %v", i, expected)
 	}
 }