Verify aggregator signature in sync

beacon-chain/blockchain/process_attestation_helpers.go

@@ -123,7 +123,7 @@ func (s *Service) verifyAttestation(ctx context.Context, baseState *stateTrie.Be
 	}
 	indexedAtt := attestationutil.ConvertToIndexed(ctx, a, committee)
 	if err := blocks.VerifyIndexedAttestation(ctx, baseState, indexedAtt); err != nil {
-		if err == blocks.ErrSigFailedToVerify {
+		if err == helpers.ErrSigFailedToVerify {
 			// When sig fails to verify, check if there's a differences in committees due to
 			// different seeds.
 			var aState *stateTrie.BeaconState

beacon-chain/core/blocks/block_operations.go

@@ -35,29 +35,6 @@ var log = logrus.WithField("prefix", "blocks")
 
 var eth1DataCache = cache.NewEth1DataVoteCache()
 
-// ErrSigFailedToVerify returns when a signature of a block object(ie attestation, slashing, exit... etc)
-// failed to verify.
-var ErrSigFailedToVerify = errors.New("signature did not verify")
-
-func verifySigningRoot(obj interface{}, pub []byte, signature []byte, domain []byte) error {
-	publicKey, err := bls.PublicKeyFromBytes(pub)
-	if err != nil {
-		return errors.Wrap(err, "could not convert bytes to public key")
-	}
-	sig, err := bls.SignatureFromBytes(signature)
-	if err != nil {
-		return errors.Wrap(err, "could not convert bytes to signature")
-	}
-	root, err := helpers.ComputeSigningRoot(obj, domain)
-	if err != nil {
-		return errors.Wrap(err, "could not compute signing root")
-	}
-	if !sig.Verify(root[:], publicKey) {
-		return ErrSigFailedToVerify
-	}
-	return nil
-}
-
 // Deprecated: This method uses deprecated ssz.SigningRoot.
 func verifyDepositDataSigningRoot(obj *ethpb.Deposit_Data, pub []byte, signature []byte, domain []byte) error {
 	publicKey, err := bls.PublicKeyFromBytes(pub)
@@ -81,7 +58,7 @@ func verifyDepositDataSigningRoot(obj *ethpb.Deposit_Data, pub []byte, signature
 		return errors.Wrap(err, "could not get container root")
 	}
 	if !sig.Verify(ctrRoot[:], publicKey) {
-		return ErrSigFailedToVerify
+		return helpers.ErrSigFailedToVerify
 	}
 	return nil
 }
@@ -104,7 +81,7 @@ func verifySignature(signedData []byte, pub []byte, signature []byte, domain []b
 		return errors.Wrap(err, "could not hash container")
 	}
 	if !sig.Verify(root[:], publicKey) {
-		return ErrSigFailedToVerify
+		return helpers.ErrSigFailedToVerify
 	}
 	return nil
 }
@@ -247,7 +224,7 @@ func VerifyBlockHeaderSignature(beaconState *stateTrie.BeaconState, block *ethpb
 	if err != nil {
 		return err
 	}
-	return verifySigningRoot(block.Block, proposer.PublicKey, block.Signature, domain)
+	return helpers.VerifySigningRoot(block.Block, proposer.PublicKey, block.Signature, domain)
 }
 
 // ProcessBlockHeaderNoVerify validates a block by its header but skips proposer
@@ -485,7 +462,7 @@ func VerifyProposerSlashing(
 	}
 	headers := []*ethpb.SignedBeaconBlockHeader{slashing.Header_1, slashing.Header_2}
 	for _, header := range headers {
-		if err := verifySigningRoot(header.Header, proposer.PublicKey, header.Signature, domain); err != nil {
+		if err := helpers.VerifySigningRoot(header.Header, proposer.PublicKey, header.Signature, domain); err != nil {
 			return errors.Wrap(err, "could not verify beacon block header")
 		}
 	}
@@ -862,7 +839,7 @@ func VerifyIndexedAttestation(ctx context.Context, beaconState *stateTrie.Beacon
 
 	voted := len(indices) > 0
 	if voted && !sig.FastAggregateVerify(pubkeys, messageHash) {
-		return ErrSigFailedToVerify
+		return helpers.ErrSigFailedToVerify
 	}
 	return nil
 }
@@ -1199,8 +1176,8 @@ func VerifyExit(validator *ethpb.Validator, currentSlot uint64, fork *pb.Fork, s
 	if err != nil {
 		return err
 	}
-	if err := verifySigningRoot(exit, validator.PublicKey, signed.Signature, domain); err != nil {
-		return ErrSigFailedToVerify
+	if err := helpers.VerifySigningRoot(exit, validator.PublicKey, signed.Signature, domain); err != nil {
+		return helpers.ErrSigFailedToVerify
 	}
 	return nil
 }

beacon-chain/core/blocks/block_operations_fuzz_test.go

@@ -43,30 +43,6 @@ func TestFuzzProcessBlockHeader_10000(t *testing.T) {
 	}
 }
 
-func TestFuzzverifySigningRoot_10000(t *testing.T) {
-	fuzzer := fuzz.NewWithSeed(0)
-	state := &ethereum_beacon_p2p_v1.BeaconState{}
-	pubkey := [48]byte{}
-	sig := [96]byte{}
-	domain := [4]byte{}
-	p := []byte{}
-	s := []byte{}
-	d := []byte{}
-	for i := 0; i < 10000; i++ {
-		fuzzer.Fuzz(state)
-		fuzzer.Fuzz(&pubkey)
-		fuzzer.Fuzz(&sig)
-		fuzzer.Fuzz(&domain)
-		fuzzer.Fuzz(state)
-		fuzzer.Fuzz(&p)
-		fuzzer.Fuzz(&s)
-		fuzzer.Fuzz(&d)
-		verifySigningRoot(state, pubkey[:], sig[:], domain[:])
-		verifySigningRoot(state, p, s, d)
-
-	}
-}
-
 func TestFuzzverifyDepositDataSigningRoot_10000(t *testing.T) {
 	fuzzer := fuzz.NewWithSeed(0)
 	ba := []byte{}

beacon-chain/core/helpers/BUILD.bazel

@@ -69,6 +69,7 @@ go_test(
         "//shared/params:go_default_library",
         "//shared/sliceutil:go_default_library",
         "//shared/testutil:go_default_library",
+        "@com_github_google_gofuzz//:go_default_library",
         "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library",
         "@com_github_prysmaticlabs_go_bitfield//:go_default_library",
         "@com_github_prysmaticlabs_go_ssz//:go_default_library",

beacon-chain/core/helpers/signing_root.go

@@ -1,9 +1,11 @@
 package helpers
 
 import (
+	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/go-ssz"
 	p2ppb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
 	pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
+	"github.com/prysmaticlabs/prysm/shared/bls"
 	"github.com/prysmaticlabs/prysm/shared/bytesutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
 )
@@ -14,6 +16,10 @@ const ForkVersionByteLength = 4
 // DomainByteLength length of domain byte array.
 const DomainByteLength = 4
 
+// ErrSigFailedToVerify returns when a signature of a block object(ie attestation, slashing, exit... etc)
+// failed to verify.
+var ErrSigFailedToVerify = errors.New("signature did not verify")
+
 // ComputeSigningRoot computes the root of the object by calculating the root of the object domain tree.
 //
 // Spec pseudocode definition:
@@ -38,6 +44,26 @@ func ComputeSigningRoot(object interface{}, domain []byte) ([32]byte, error) {
 	return ssz.HashTreeRoot(container)
 }
 
+// VerifySigningRoot verifies the signing root of an object given it's public key, signature and domain.
+func VerifySigningRoot(obj interface{}, pub []byte, signature []byte, domain []byte) error {
+	publicKey, err := bls.PublicKeyFromBytes(pub)
+	if err != nil {
+		return errors.Wrap(err, "could not convert bytes to public key")
+	}
+	sig, err := bls.SignatureFromBytes(signature)
+	if err != nil {
+		return errors.Wrap(err, "could not convert bytes to signature")
+	}
+	root, err := ComputeSigningRoot(obj, domain)
+	if err != nil {
+		return errors.Wrap(err, "could not compute signing root")
+	}
+	if !sig.Verify(root[:], publicKey) {
+		return ErrSigFailedToVerify
+	}
+	return nil
+}
+
 // ComputeDomain returns the domain version for BLS private key to sign and verify with a zeroed 4-byte
 // array as the fork version.
 //

beacon-chain/core/helpers/signing_root_test.go

@@ -4,7 +4,9 @@ import (
 	"bytes"
 	"testing"
 
+	fuzz "github.com/google/gofuzz"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
+	ethereum_beacon_p2p_v1 "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
 	"github.com/prysmaticlabs/prysm/shared/params"
 )
 
@@ -55,3 +57,27 @@ func TestComputeForkDigest_OK(t *testing.T) {
 		}
 	}
 }
+
+func TestFuzzverifySigningRoot_10000(t *testing.T) {
+	fuzzer := fuzz.NewWithSeed(0)
+	state := &ethereum_beacon_p2p_v1.BeaconState{}
+	pubkey := [48]byte{}
+	sig := [96]byte{}
+	domain := [4]byte{}
+	p := []byte{}
+	s := []byte{}
+	d := []byte{}
+	for i := 0; i < 10000; i++ {
+		fuzzer.Fuzz(state)
+		fuzzer.Fuzz(&pubkey)
+		fuzzer.Fuzz(&sig)
+		fuzzer.Fuzz(&domain)
+		fuzzer.Fuzz(state)
+		fuzzer.Fuzz(&p)
+		fuzzer.Fuzz(&s)
+		fuzzer.Fuzz(&d)
+		VerifySigningRoot(state, pubkey[:], sig[:], domain[:])
+		VerifySigningRoot(state, p, s, d)
+
+	}
+}

beacon-chain/sync/pending_attestations_queue.go

@@ -72,7 +72,7 @@ func (s *Service) processPendingAtts(ctx context.Context) error {
 				if helpers.IsAggregated(att.Aggregate) {
 					// Save the pending aggregated attestation to the pool if it passes the aggregated
 					// validation steps.
-					if s.validateBlockInAttestation(ctx, signedAtt) && s.validateAggregatedAtt(ctx, att) {
+					if s.validateBlockInAttestation(ctx, signedAtt) && s.validateAggregatedAtt(ctx, signedAtt) {
 						if err := s.attPool.SaveAggregatedAttestation(att.Aggregate); err != nil {
 							return err
 						}

beacon-chain/sync/pending_attestations_queue_test.go

@@ -159,6 +159,15 @@ func TestProcessPendingAtts_HasBlockSaveAggregatedAtt(t *testing.T) {
 		Aggregate:       att,
 		AggregatorIndex: 33,
 	}
+	domain, err = helpers.Domain(beaconState.Fork(), 0, params.BeaconConfig().DomainAggregateAndProof, beaconState.GenesisValidatorRoot())
+	if err != nil {
+		t.Fatal(err)
+	}
+	signingRoot, err := helpers.ComputeSigningRoot(aggregateAndProof, domain)
+	if err != nil {
+		t.Error(err)
+	}
+	aggreSig := privKeys[33].Sign(signingRoot[:]).Marshal()
 
 	if err := beaconState.SetGenesisTime(uint64(time.Now().Unix())); err != nil {
 		t.Fatal(err)
@@ -182,7 +191,7 @@ func TestProcessPendingAtts_HasBlockSaveAggregatedAtt(t *testing.T) {
 	s, _ := beaconstate.InitializeFromProto(&pb.BeaconState{})
 	r.db.SaveState(context.Background(), s, r32)
 
-	r.blkRootToPendingAtts[r32] = []*ethpb.SignedAggregateAttestationAndProof{{Message: aggregateAndProof}}
+	r.blkRootToPendingAtts[r32] = []*ethpb.SignedAggregateAttestationAndProof{{Message: aggregateAndProof, Signature: aggreSig}}
 	if err := r.processPendingAtts(context.Background()); err != nil {
 		t.Fatal(err)
 	}

beacon-chain/sync/validate_aggregate_proof.go

@@ -70,7 +70,7 @@ func (r *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms
 		return false
 	}
 
-	if !r.validateAggregatedAtt(ctx, m.Message) {
+	if !r.validateAggregatedAtt(ctx, m) {
 		return false
 	}
 
@@ -85,11 +85,11 @@ func (r *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms
 	return true
 }
 
-func (r *Service) validateAggregatedAtt(ctx context.Context, a *ethpb.AggregateAttestationAndProof) bool {
+func (r *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.SignedAggregateAttestationAndProof) bool {
 	ctx, span := trace.StartSpan(ctx, "sync.validateAggregatedAtt")
 	defer span.End()
 
-	attSlot := a.Aggregate.Data.Slot
+	attSlot := signed.Message.Aggregate.Data.Slot
 	if err := validateAggregateAttTime(attSlot, uint64(r.chain.GenesisTime().Unix())); err != nil {
 		traceutil.AnnotateError(span, err)
 		return false
@@ -111,19 +111,25 @@ func (r *Service) validateAggregatedAtt(ctx context.Context, a *ethpb.AggregateA
 	}
 
 	// Verify validator index is within the aggregate's committee.
-	if err := validateIndexInCommittee(ctx, s, a.Aggregate, a.AggregatorIndex); err != nil {
+	if err := validateIndexInCommittee(ctx, s, signed.Message.Aggregate, signed.Message.AggregatorIndex); err != nil {
 		traceutil.AnnotateError(span, errors.Wrapf(err, "Could not validate index in committee"))
 		return false
 	}
 
 	// Verify selection proof reflects to the right validator and signature is valid.
-	if err := validateSelection(ctx, s, a.Aggregate.Data, a.AggregatorIndex, a.SelectionProof); err != nil {
-		traceutil.AnnotateError(span, errors.Wrapf(err, "Could not validate selection for validator %d", a.AggregatorIndex))
+	if err := validateSelection(ctx, s, signed.Message.Aggregate.Data, signed.Message.AggregatorIndex, signed.Message.SelectionProof); err != nil {
+		traceutil.AnnotateError(span, errors.Wrapf(err, "Could not validate selection for validator %d", signed.Message.AggregatorIndex))
+		return false
+	}
+
+	// Verify the aggregator's signature is valid.
+	if err := validateAggregatorSignature(s, signed); err != nil {
+		traceutil.AnnotateError(span, errors.Wrapf(err, "Could not verify aggregator signature %d", signed.Message.AggregatorIndex))
 		return false
 	}
 
 	// Verify aggregated attestation has a valid signature.
-	if err := blocks.VerifyAttestation(ctx, s, a.Aggregate); err != nil {
+	if err := blocks.VerifyAttestation(ctx, s, signed.Message.Aggregate); err != nil {
 		traceutil.AnnotateError(span, err)
 		return false
 	}
@@ -245,3 +251,20 @@ func validateSelection(ctx context.Context, s *stateTrie.BeaconState, data *ethp
 
 	return nil
 }
+
+// This verifies aggregator signature over the signed aggregate and proof object.
+func validateAggregatorSignature(s *stateTrie.BeaconState, a *ethpb.SignedAggregateAttestationAndProof) error {
+	aggregator, err := s.ValidatorAtIndex(a.Message.AggregatorIndex)
+	if err != nil {
+		return err
+	}
+
+	currentEpoch := helpers.SlotToEpoch(a.Message.Aggregate.Data.Slot)
+	domain, err := helpers.Domain(s.Fork(), currentEpoch, params.BeaconConfig().DomainAggregateAndProof, s.GenesisValidatorRoot())
+	if err != nil {
+		return err
+	}
+
+	return helpers.VerifySigningRoot(a.Message, aggregator.PublicKey, a.Signature, domain)
+
+}

beacon-chain/sync/validate_aggregate_proof_test.go

@@ -373,6 +373,17 @@ func TestValidateAggregateAndProof_CanValidate(t *testing.T) {
 	}
 	signedAggregateAndProof := &ethpb.SignedAggregateAttestationAndProof{Message: aggregateAndProof}
 
+	domain, err = helpers.Domain(beaconState.Fork(), 0, params.BeaconConfig().DomainAggregateAndProof, beaconState.GenesisValidatorRoot())
+	if err != nil {
+		t.Fatal(err)
+	}
+	signingRoot, err := helpers.ComputeSigningRoot(signedAggregateAndProof.Message, domain)
+	if err != nil {
+		t.Error(err)
+	}
+	aggreSig := privKeys[33].Sign(signingRoot[:]).Marshal()
+	signedAggregateAndProof.Signature = aggreSig[:]
+
 	if err := beaconState.SetGenesisTime(uint64(time.Now().Unix())); err != nil {
 		t.Fatal(err)
 	}
@@ -472,6 +483,17 @@ func TestVerifyIndexInCommittee_SeenAggregatorSlot(t *testing.T) {
 	}
 	signedAggregateAndProof := &ethpb.SignedAggregateAttestationAndProof{Message: aggregateAndProof}
 
+	domain, err = helpers.Domain(beaconState.Fork(), 0, params.BeaconConfig().DomainAggregateAndProof, beaconState.GenesisValidatorRoot())
+	if err != nil {
+		t.Fatal(err)
+	}
+	signingRoot, err := helpers.ComputeSigningRoot(signedAggregateAndProof.Message, domain)
+	if err != nil {
+		t.Error(err)
+	}
+	aggreSig := privKeys[33].Sign(signingRoot[:]).Marshal()
+	signedAggregateAndProof.Signature = aggreSig[:]
+
 	if err := beaconState.SetGenesisTime(uint64(time.Now().Unix())); err != nil {
 		t.Fatal(err)
 	}