Support multiple BLS implementations (#1335)

## Issue Addressed

NA

## Proposed Changes

- Refactor the `bls` crate to support multiple BLS "backends" (e.g., milagro, blst, etc).
- Removes some duplicate, unused code in `common/rest_types/src/validator.rs`.
- Removes the old "upgrade legacy keypairs" functionality (these were unencrypted keys that haven't been supported for a few testnets, no one should be using them anymore).

## Additional Info

Most of the files changed are just inconsequential changes to function names.

## TODO

- [x] Optimization levels
- [x] Infinity point: https://github.com/supranational/blst/issues/11
- [x] Ensure milagro *and* blst are tested via CI
- [x] What to do with unsafe code?
- [x] Test infinity point in signature sets

beacon_node/beacon_chain/Cargo.toml

@@ -9,6 +9,9 @@ default = ["participation_metrics"]
 write_ssz_files = []  # Writes debugging .ssz files to /tmp during block processing.
 participation_metrics = []  # Exposes validator participation metrics to Prometheus.
 
+[dev-dependencies]
+int_to_bytes = { path = "../../consensus/int_to_bytes" }
+
 [dependencies]
 eth2_config = { path = "../../common/eth2_config" }
 merkle_proof = { path = "../../consensus/merkle_proof" }

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -521,7 +521,7 @@ pub fn verify_attestation_signature<T: BeaconChainTypes>(
     let _signature_verification_timer =
         metrics::start_timer(&metrics::ATTESTATION_PROCESSING_SIGNATURE_TIMES);
 
-    if signature_set.is_valid() {
+    if signature_set.verify() {
         Ok(())
     } else {
         Err(Error::InvalidSignature)
@@ -589,7 +589,7 @@ pub fn verify_signed_aggregate_signatures<T: BeaconChainTypes>(
         .map_err(BeaconChainError::SignatureSetError)?,
     ];
 
-    Ok(verify_signature_sets(signature_sets))
+    Ok(verify_signature_sets(signature_sets.iter()))
 }
 
 /// Assists in readability.

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -862,7 +862,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     root: target_root,
                 },
             },
-            signature: AggregateSignature::empty_signature(),
+            signature: AggregateSignature::empty(),
         })
     }
 
@@ -1654,7 +1654,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 },
             },
             // The block is not signed here, that is the task of a validator client.
-            signature: Signature::empty_signature(),
+            signature: Signature::empty(),
         };
 
         per_block_processing(

beacon_node/beacon_chain/src/builder.rs

@@ -666,7 +666,7 @@ fn genesis_block<T: EthSpec>(
         message: BeaconBlock::empty(&spec),
         // Empty signature, which should NEVER be read. This isn't to-spec, but makes the genesis
         // block consistent with every other block.
-        signature: Signature::empty_signature(),
+        signature: Signature::empty(),
     };
     genesis_block.message.state_root = genesis_state
         .update_tree_hash_cache()

beacon_node/beacon_chain/src/errors.rs

@@ -6,7 +6,6 @@ use crate::observed_attesters::Error as ObservedAttestersError;
 use crate::observed_block_producers::Error as ObservedBlockProducersError;
 use operation_pool::OpPoolError;
 use safe_arith::ArithError;
-use ssz::DecodeError;
 use ssz_types::Error as SszTypesError;
 use state_processing::{
     block_signature_verifier::Error as BlockSignatureVerifierError,
@@ -69,7 +68,7 @@ pub enum BeaconChainError {
     AttestationCacheLockTimeout,
     ValidatorPubkeyCacheLockTimeout,
     IncorrectStateForAttestation(RelativeEpochError),
-    InvalidValidatorPubkeyBytes(DecodeError),
+    InvalidValidatorPubkeyBytes(bls::Error),
     ValidatorPubkeyCacheIncomplete(usize),
     SignatureSetError(SignatureSetError),
     BlockSignatureVerifierError(state_processing::block_signature_verifier::Error),

beacon_node/beacon_chain/src/snapshot_cache.rs

@@ -99,7 +99,7 @@ mod test {
     use super::*;
     use types::{
         test_utils::{generate_deterministic_keypair, TestingBeaconStateBuilder},
-        BeaconBlock, Epoch, MainnetEthSpec, Signature, SignedBeaconBlock, Slot,
+        BeaconBlock, Epoch, MainnetEthSpec, SignedBeaconBlock, Slot,
     };
 
     const CACHE_SIZE: usize = 4;
@@ -115,7 +115,9 @@ mod test {
             beacon_state_root: Hash256::from_low_u64_be(i),
             beacon_block: SignedBeaconBlock {
                 message: BeaconBlock::empty(&spec),
-                signature: Signature::new(&[42], &generate_deterministic_keypair(0).sk),
+                signature: generate_deterministic_keypair(0)
+                    .sk
+                    .sign(Hash256::from_low_u64_be(42)),
             },
             beacon_block_root: Hash256::from_low_u64_be(i),
         }

beacon_node/beacon_chain/src/test_utils.rs

@@ -23,8 +23,8 @@ use tempfile::{tempdir, TempDir};
 use tree_hash::TreeHash;
 use types::{
     AggregateSignature, Attestation, BeaconState, BeaconStateHash, ChainSpec, Domain, EthSpec,
-    Hash256, Keypair, SecretKey, SelectionProof, Signature, SignedAggregateAndProof,
-    SignedBeaconBlock, SignedBeaconBlockHash, SignedRoot, Slot, SubnetId,
+    Hash256, Keypair, SecretKey, SelectionProof, SignedAggregateAndProof, SignedBeaconBlock,
+    SignedBeaconBlockHash, SignedRoot, Slot, SubnetId,
 };
 
 pub use types::test_utils::generate_deterministic_keypairs;
@@ -515,7 +515,7 @@ where
                 self.spec
                     .get_domain(epoch, Domain::Randao, fork, state.genesis_validators_root);
             let message = epoch.signing_root(domain);
-            Signature::new(message.as_bytes(), sk)
+            sk.sign(message)
         };
 
         let (block, state) = self
@@ -586,12 +586,9 @@ where
 
                             let message = attestation.data.signing_root(domain);
 
-                            let mut agg_sig = AggregateSignature::new();
+                            let mut agg_sig = AggregateSignature::infinity();
 
-                            agg_sig.add(&Signature::new(
-                                message.as_bytes(),
-                                self.get_sk(*validator_index),
-                            ));
+                            agg_sig.add_assign(&self.get_sk(*validator_index).sign(message));
 
                             agg_sig
                         };

beacon_node/beacon_chain/src/validator_pubkey_cache.rs

@@ -140,6 +140,7 @@ struct ValidatorPubkeyCacheFile(File);
 enum Error {
     Io(io::Error),
     Ssz(DecodeError),
+    PubkeyDecode(bls::Error),
     /// The file read from disk does not have a contiguous list of validator public keys. The file
     /// has become corrupted.
     InconsistentIndex {
@@ -200,7 +201,7 @@ impl ValidatorPubkeyCacheFile {
             let expected = last.map(|n| n + 1);
             if expected.map_or(true, |expected| index == expected) {
                 last = Some(index);
-                pubkeys.push((&pubkey).try_into().map_err(Error::Ssz)?);
+                pubkeys.push((&pubkey).try_into().map_err(Error::PubkeyDecode)?);
                 indices.insert(pubkey, index);
             } else {
                 return Err(Error::InconsistentIndex {

beacon_node/beacon_chain/tests/attestation_production.rs

@@ -111,7 +111,7 @@ fn produces_attestations() {
             );
             assert_eq!(
                 attestation.signature,
-                AggregateSignature::empty_signature(),
+                AggregateSignature::empty(),
                 "bad signature"
             );
             assert_eq!(data.index, index, "bad index");

beacon_node/beacon_chain/tests/attestation_verification.rs

@@ -8,13 +8,14 @@ use beacon_chain::{
     test_utils::{AttestationStrategy, BeaconChainHarness, BlockStrategy, HarnessType},
     BeaconChain, BeaconChainTypes,
 };
+use int_to_bytes::int_to_bytes32;
 use state_processing::per_slot_processing;
 use store::config::StoreConfig;
 use tree_hash::TreeHash;
 use types::{
     test_utils::generate_deterministic_keypair, AggregateSignature, Attestation, EthSpec, Hash256,
-    Keypair, MainnetEthSpec, SecretKey, SelectionProof, Signature, SignedAggregateAndProof,
-    SignedBeaconBlock, SubnetId, Unsigned,
+    Keypair, MainnetEthSpec, SecretKey, SelectionProof, SignedAggregateAndProof, SignedBeaconBlock,
+    SubnetId, Unsigned,
 };
 
 pub type E = MainnetEthSpec;
@@ -311,7 +312,7 @@ fn aggregated_gossip_verification() {
             let aggregation_bits = &mut a.message.aggregate.aggregation_bits;
             aggregation_bits.difference_inplace(&aggregation_bits.clone());
             assert!(aggregation_bits.is_zero());
-            a.message.aggregate.signature = AggregateSignature::new();
+            a.message.aggregate.signature = AggregateSignature::infinity();
             a
         },
         AttnError::EmptyAggregationBitfield
@@ -330,7 +331,7 @@ fn aggregated_gossip_verification() {
         {
             let mut a = valid_aggregate.clone();
 
-            a.signature = Signature::new(&[42, 42], &validator_sk);
+            a.signature = validator_sk.sign(Hash256::from_low_u64_be(42));
 
             a
         },
@@ -370,7 +371,9 @@ fn aggregated_gossip_verification() {
             let mut i: u64 = 0;
             a.message.selection_proof = loop {
                 i += 1;
-                let proof: SelectionProof = Signature::new(&i.to_le_bytes(), &validator_sk).into();
+                let proof: SelectionProof = validator_sk
+                    .sign(Hash256::from_slice(&int_to_bytes32(i)))
+                    .into();
                 if proof
                     .is_aggregator(committee_len, &harness.chain.spec)
                     .unwrap()
@@ -397,8 +400,8 @@ fn aggregated_gossip_verification() {
         {
             let mut a = valid_aggregate.clone();
 
-            let mut agg_sig = AggregateSignature::new();
-            agg_sig.add(&Signature::new(&[42, 42], &aggregator_sk));
+            let mut agg_sig = AggregateSignature::infinity();
+            agg_sig.add_assign(&aggregator_sk.sign(Hash256::from_low_u64_be(42)));
             a.message.aggregate.signature = agg_sig;
 
             a
@@ -727,8 +730,8 @@ fn unaggregated_gossip_verification() {
         {
             let mut a = valid_attestation.clone();
 
-            let mut agg_sig = AggregateSignature::new();
-            agg_sig.add(&Signature::new(&[42, 42], &validator_sk));
+            let mut agg_sig = AggregateSignature::infinity();
+            agg_sig.add_assign(&validator_sk.sign(Hash256::from_low_u64_be(42)));
             a.signature = agg_sig;
 
             a
@@ -737,13 +740,10 @@ fn unaggregated_gossip_verification() {
         AttnError::InvalidSignature
     );
 
-    assert!(
-        harness
-            .chain
-            .verify_unaggregated_attestation_for_gossip(valid_attestation.clone(), subnet_id)
-            .is_ok(),
-        "valid attestation should be verified"
-    );
+    harness
+        .chain
+        .verify_unaggregated_attestation_for_gossip(valid_attestation.clone(), subnet_id)
+        .expect("valid attestation should be verified");
 
     /*
      * The following test ensures that:

beacon_node/beacon_chain/tests/block_verification.rs

@@ -68,13 +68,13 @@ fn chain_segment_blocks() -> Vec<SignedBeaconBlock<E>> {
 
 fn junk_signature() -> Signature {
     let kp = generate_deterministic_keypair(VALIDATOR_COUNT);
-    let message = &[42, 42];
-    Signature::new(message, &kp.sk)
+    let message = Hash256::from_slice(&[42; 32]);
+    kp.sk.sign(message)
 }
 
 fn junk_aggregate_signature() -> AggregateSignature {
-    let mut agg_sig = AggregateSignature::new();
-    agg_sig.add(&junk_signature());
+    let mut agg_sig = AggregateSignature::empty();
+    agg_sig.add_assign(&junk_signature());
     agg_sig
 }
 

beacon_node/beacon_chain/tests/op_verification.rs

@@ -201,7 +201,11 @@ fn attester_slashing() {
     // Last half of the validators
     let second_half = (VALIDATOR_COUNT as u64 / 2..VALIDATOR_COUNT as u64).collect::<Vec<_>>();
 
-    let signer = |idx: u64, message: &[u8]| Signature::new(message, &KEYPAIRS[idx as usize].sk);
+    let signer = |idx: u64, message: &[u8]| {
+        KEYPAIRS[idx as usize]
+            .sk
+            .sign(Hash256::from_slice(&message))
+    };
 
     let make_slashing = |validators| {
         TestingAttesterSlashingBuilder::double_vote::<_, E>(