BLS code cleanup (#5773)

* removes unused code
* add specs comments
* removes unused VerifyAggregate
* fixes benchmark test
* Merge branch 'master' into blc-cleanup
* updates benchmark test
* Merge branch 'master' into blc-cleanup

shared/bls/BUILD.bazel

@@ -10,7 +10,6 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//shared/featureconfig:go_default_library",
-        "//shared/hashutil:go_default_library",
         "//shared/params:go_default_library",
         "@com_github_dgraph_io_ristretto//:go_default_library",
         "@com_github_pkg_errors//:go_default_library",

shared/bls/bls.go

@@ -4,14 +4,12 @@
 package bls
 
 import (
-	"encoding/binary"
 	"fmt"
 
 	"github.com/dgraph-io/ristretto"
 	bls12 "github.com/herumi/bls-eth-go-binary/bls"
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/shared/featureconfig"
-	"github.com/prysmaticlabs/prysm/shared/hashutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
 )
 
@@ -37,9 +35,6 @@ var pubkeyCache, _ = ristretto.NewCache(&ristretto.Config{
 // CurveOrder for the BLS12-381 curve.
 const CurveOrder = "52435875175126190479447740508185965837690552500527637822603658699938581184513"
 
-// The size would be a combination of both the message(32 bytes) and domain(8 bytes) size.
-const concatMsgDomainSize = 40
-
 // Signature used in the BLS signature scheme.
 type Signature struct {
 	s *bls12.Sign
@@ -83,22 +78,21 @@ func PublicKeyFromBytes(pub []byte) (*PublicKey, error) {
 	if len(pub) != params.BeaconConfig().BLSPubkeyLength {
 		return nil, fmt.Errorf("public key must be %d bytes", params.BeaconConfig().BLSPubkeyLength)
 	}
-	cv, ok := pubkeyCache.Get(string(pub))
-	if ok {
+	if cv, ok := pubkeyCache.Get(string(pub)); ok {
 		return cv.(*PublicKey).Copy()
 	}
 	pubKey := &bls12.PublicKey{}
 	err := pubKey.Deserialize(pub)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not unmarshal bytes into public key")
 	}
-	pubkeyObj := &PublicKey{p: pubKey}
-	copiedKey, err := pubkeyObj.Copy()
+	pubKeyObj := &PublicKey{p: pubKey}
+	copiedKey, err := pubKeyObj.Copy()
 	if err != nil {
-		return nil, errors.Wrap(err, "could not copy pubkey")
+		return nil, errors.Wrap(err, "could not copy public key")
 	}
 	pubkeyCache.Set(string(pub), copiedKey, 48)
-	return pubkeyObj, nil
+	return pubKeyObj, nil
 }
 
 // SignatureFromBytes creates a BLS signature from a LittleEndian byte slice.
@@ -122,14 +116,14 @@ func (s *SecretKey) PublicKey() *PublicKey {
 	return &PublicKey{p: s.p.GetPublicKey()}
 }
 
-func concatMsgAndDomain(msg []byte, domain uint64) []byte {
-	b := [concatMsgDomainSize]byte{}
-	binary.LittleEndian.PutUint64(b[32:], domain)
-	copy(b[0:32], msg)
-	return b[:]
-}
-
 // Sign a message using a secret key - in a beacon/validator client.
+//
+// In IETF draft BLS specification:
+// Sign(SK, message) -> signature: a signing algorithm that generates
+//      a deterministic signature given a secret key SK and a message.
+//
+// In ETH2.0 specification:
+// def Sign(SK: int, message: Bytes) -> BLSSignature
 func (s *SecretKey) Sign(msg []byte) *Signature {
 	if featureconfig.Get().SkipBLSVerify {
 		return &Signature{}
@@ -169,39 +163,34 @@ func (p *PublicKey) Aggregate(p2 *PublicKey) *PublicKey {
 }
 
 // Verify a bls signature given a public key, a message.
+//
+// In IETF draft BLS specification:
+// Verify(PK, message, signature) -> VALID or INVALID: a verification
+//      algorithm that outputs VALID if signature is a valid signature of
+//      message under public key PK, and INVALID otherwise.
+//
+// In ETH2.0 specification:
+// def Verify(PK: BLSPubkey, message: Bytes, signature: BLSSignature) -> bool
 func (s *Signature) Verify(msg []byte, pub *PublicKey) bool {
 	if featureconfig.Get().SkipBLSVerify {
 		return true
 	}
 	return s.s.VerifyByte(pub.p, msg)
 }
 
-// VerifyAggregate verifies each public key against its respective message.
-// This is vulnerable to rogue public-key attack. Each user must
-// provide a proof-of-knowledge of the public key.
-func (s *Signature) VerifyAggregate(pubKeys []*PublicKey, msg [][32]byte) bool {
-	if featureconfig.Get().SkipBLSVerify {
-		return true
-	}
-	size := len(pubKeys)
-	if size == 0 {
-		return false
-	}
-	if size != len(msg) {
-		return false
-	}
-	hashes := make([][]byte, 0, len(msg))
-	var rawKeys []bls12.PublicKey
-	for i := 0; i < size; i++ {
-		hashes = append(hashes, msg[i][:])
-		rawKeys = append(rawKeys, *pubKeys[i].p)
-	}
-	return s.s.VerifyAggregateHashes(rawKeys, hashes)
-}
-
 // AggregateVerify verifies each public key against its respective message.
 // This is vulnerable to rogue public-key attack. Each user must
 // provide a proof-of-knowledge of the public key.
+//
+// In IETF draft BLS specification:
+// AggregateVerify((PK_1, message_1), ..., (PK_n, message_n),
+//      signature) -> VALID or INVALID: an aggregate verification
+//      algorithm that outputs VALID if signature is a valid aggregated
+//      signature for a collection of public keys and messages, and
+//      outputs INVALID otherwise.
+//
+// In ETH2.0 specification:
+// def AggregateVerify(pairs: Sequence[PK: BLSPubkey, message: Bytes], signature: BLSSignature) -> boo
 func (s *Signature) AggregateVerify(pubKeys []*PublicKey, msgs [][32]byte) bool {
 	if featureconfig.Get().SkipBLSVerify {
 		return true
@@ -222,7 +211,16 @@ func (s *Signature) AggregateVerify(pubKeys []*PublicKey, msgs [][32]byte) bool
 	return s.s.AggregateVerify(rawKeys, msgSlices)
 }
 
-// FastAggregateVerify verifies all the provided pubkeys with their aggregated signature.
+// FastAggregateVerify verifies all the provided public keys with their aggregated signature.
+//
+// In IETF draft BLS specification:
+// FastAggregateVerify(PK_1, ..., PK_n, message, signature) -> VALID
+//      or INVALID: a verification algorithm for the aggregate of multiple
+//      signatures on the same message.  This function is faster than
+//      AggregateVerify.
+//
+// In ETH2.0 specification:
+// def FastAggregateVerify(PKs: Sequence[BLSPubkey], message: Bytes, signature: BLSSignature) -> bool
 func (s *Signature) FastAggregateVerify(pubKeys []*PublicKey, msg [32]byte) bool {
 	if featureconfig.Get().SkipBLSVerify {
 		return true
@@ -243,11 +241,6 @@ func NewAggregateSignature() *Signature {
 	return &Signature{s: bls12.HashAndMapToSignature([]byte{'m', 'o', 'c', 'k'})}
 }
 
-// NewAggregatePubkey creates a blank public key.
-func NewAggregatePubkey() *PublicKey {
-	return &PublicKey{p: RandKey().PublicKey().p}
-}
-
 // AggregateSignatures converts a list of signatures into a single, aggregated sig.
 func AggregateSignatures(sigs []*Signature) *Signature {
 	if len(sigs) == 0 {
@@ -265,6 +258,19 @@ func AggregateSignatures(sigs []*Signature) *Signature {
 	return &Signature{s: &signature}
 }
 
+// Aggregate is an alias for AggregateSignatures, defined to conform to BLS specification.
+//
+// In IETF draft BLS specification:
+// Aggregate(signature_1, ..., signature_n) -> signature: an
+//      aggregation algorithm that compresses a collection of signatures
+//      into a single signature.
+//
+// In ETH2.0 specification:
+// def Aggregate(signatures: Sequence[BLSSignature]) -> BLSSignature
+func Aggregate(sigs []*Signature) *Signature {
+	return AggregateSignatures(sigs)
+}
+
 // Marshal a signature into a LittleEndian byte slice.
 func (s *Signature) Marshal() []byte {
 	if featureconfig.Get().SkipBLSVerify {
@@ -273,21 +279,3 @@ func (s *Signature) Marshal() []byte {
 
 	return s.s.Serialize()
 }
-
-// HashWithDomain hashes 32 byte message and uint64 domain parameters a Fp2 element
-func HashWithDomain(messageHash [32]byte, domain [8]byte) []byte {
-	xReBytes := [41]byte{}
-	xImBytes := [41]byte{}
-	xBytes := make([]byte, 96)
-	copy(xReBytes[:32], messageHash[:])
-	copy(xReBytes[32:40], domain[:])
-	copy(xReBytes[40:41], []byte{0x01})
-	copy(xImBytes[:32], messageHash[:])
-	copy(xImBytes[32:40], domain[:])
-	copy(xImBytes[40:41], []byte{0x02})
-	hashedxImBytes := hashutil.Hash(xImBytes[:])
-	copy(xBytes[16:48], hashedxImBytes[:])
-	hashedxReBytes := hashutil.Hash(xReBytes[:])
-	copy(xBytes[64:], hashedxReBytes[:])
-	return xBytes
-}

shared/bls/bls_benchmark_test.go

@@ -5,14 +5,16 @@ import (
 
 	bls2 "github.com/herumi/bls-eth-go-binary/bls"
 	"github.com/prysmaticlabs/prysm/shared/bls"
-	"github.com/prysmaticlabs/prysm/shared/bytesutil"
 	"github.com/prysmaticlabs/prysm/shared/hashutil"
 )
 
 func BenchmarkPairing(b *testing.B) {
 	if err := bls2.Init(bls2.BLS12_381); err != nil {
 		b.Fatal(err)
 	}
+	if err := bls2.SetETHmode(bls2.EthModeDraft05); err != nil {
+		panic(err)
+	}
 	newGt := &bls2.GT{}
 	newG1 := &bls2.G1{}
 	newG2 := &bls2.G2{}
@@ -39,39 +41,36 @@ func BenchmarkSignature_Verify(b *testing.B) {
 	sk := bls.RandKey()
 
 	msg := []byte("Some msg")
-	domain := uint64(42)
-	sig := sk.Sign(msg, domain)
+	sig := sk.Sign(msg)
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		if !sig.Verify(msg, sk.PublicKey(), domain) {
+		if !sig.Verify(msg, sk.PublicKey()) {
 			b.Fatal("could not verify sig")
 		}
 	}
 }
 
-func BenchmarkSignature_VerifyAggregate(b *testing.B) {
+func BenchmarkSignature_AggregateVerify(b *testing.B) {
 	sigN := 128 // MAX_ATTESTATIONS per block.
-	msg := [32]byte{'s', 'i', 'g', 'n', 'e', 'd'}
-	domain := uint64(0)
 
-	var aggregated *bls.Signature
 	var pks []*bls.PublicKey
+	var sigs []*bls.Signature
+	var msgs [][32]byte
 	for i := 0; i < sigN; i++ {
+		msg := [32]byte{'s', 'i', 'g', 'n', 'e', 'd', byte(i)}
 		sk := bls.RandKey()
-		sig := sk.Sign(msg[:], domain)
-		if aggregated == nil {
-			aggregated = bls.AggregateSignatures([]*bls.Signature{sig})
-		} else {
-			aggregated = bls.AggregateSignatures([]*bls.Signature{aggregated, sig})
-		}
+		sig := sk.Sign(msg[:])
 		pks = append(pks, sk.PublicKey())
+		sigs = append(sigs, sig)
+		msgs = append(msgs, msg)
 	}
+	aggregated := bls.Aggregate(sigs)
 
 	b.ResetTimer()
 	b.ReportAllocs()
 	for i := 0; i < b.N; i++ {
-		if !aggregated.VerifyAggregateCommon(pks, msg, domain) {
+		if !aggregated.AggregateVerify(pks, msgs) {
 			b.Fatal("could not verify aggregate sig")
 		}
 	}
@@ -83,21 +82,7 @@ func BenchmarkSecretKey_Marshal(b *testing.B) {
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		_, _ = bls.SecretKeyFromBytes(d)
-	}
-}
-
-func BenchmarkHashWithDomain(b *testing.B) {
-	for i := 0; i < b.N; i++ {
-		bls.HashWithDomain(
-			bytesutil.ToBytes32([]byte("foobar")),
-			bytesutil.ToBytes8([]byte("buzz")),
-		)
-	}
-}
-
-func BenchmarkDomain(b *testing.B) {
-	for i := 0; i < b.N; i++ {
-		bls.Domain([4]byte{'A', 'B', 'C', 'D'}, [4]byte{'E', 'F', 'G', 'H'})
+		_, err := bls.SecretKeyFromBytes(d)
+		_ = err
 	}
 }

shared/bls/bls_test.go

@@ -35,7 +35,7 @@ func TestSignVerify(t *testing.T) {
 	}
 }
 
-func TestVerifyAggregate(t *testing.T) {
+func TestAggregateVerify(t *testing.T) {
 	pubkeys := make([]*bls.PublicKey, 0, 100)
 	sigs := make([]*bls.Signature, 0, 100)
 	var msgs [][32]byte
@@ -48,13 +48,13 @@ func TestVerifyAggregate(t *testing.T) {
 		sigs = append(sigs, sig)
 		msgs = append(msgs, msg)
 	}
-	aggSig := bls.AggregateSignatures(sigs)
-	if !aggSig.VerifyAggregate(pubkeys, msgs) {
+	aggSig := bls.Aggregate(sigs)
+	if !aggSig.AggregateVerify(pubkeys, msgs) {
 		t.Error("Signature did not verify")
 	}
 }
 
-func TestVerifyAggregateCommon(t *testing.T) {
+func TestFastAggregateVerify(t *testing.T) {
 	pubkeys := make([]*bls.PublicKey, 0, 100)
 	sigs := make([]*bls.Signature, 0, 100)
 	msg := [32]byte{'h', 'e', 'l', 'l', 'o'}
@@ -71,14 +71,14 @@ func TestVerifyAggregateCommon(t *testing.T) {
 	}
 }
 
-func TestVerifyAggregate_ReturnsFalseOnEmptyPubKeyList(t *testing.T) {
+func TestFastAggregateVerify_ReturnsFalseOnEmptyPubKeyList(t *testing.T) {
 	var pubkeys []*bls.PublicKey
 	sigs := make([]*bls.Signature, 0, 100)
 	msg := [32]byte{'h', 'e', 'l', 'l', 'o'}
 
 	aggSig := bls.AggregateSignatures(sigs)
 	if aggSig.FastAggregateVerify(pubkeys, msg) != false {
-		t.Error("Expected VerifyAggregate to return false with empty input " +
+		t.Error("Expected FastAggregateVerify to return false with empty input " +
 			"of public keys.")
 	}
 }