From 3b405f10ea8c5a1676da792c01c1e33200956102 Mon Sep 17 00:00:00 2001
From: Kirk Baird <baird.k@outlook.com>
Date: Fri, 20 Nov 2020 03:37:20 +0000
Subject: [PATCH] Ensure deposit signatures do not use aggregate functions
 (#1935)

## Issue Addressed

Resolves #1333

## Proposed Changes

- Remove `deposit_signature_set()` function
- Prevent deposits from being in `SignatureSets`
- User `Signature.verify()` to verify deposit signatures rather than a signature set which uses `fast_aggregate_verify()`

## Additional Info

n/a
---
 beacon_node/eth1/src/deposit_log.rs                |  8 ++++----
 .../src/per_block_processing/signature_sets.rs     | 14 --------------
 .../src/per_block_processing/verify_deposit.rs     |  8 +++-----
 3 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/beacon_node/eth1/src/deposit_log.rs b/beacon_node/eth1/src/deposit_log.rs
index 22ead3db85..e46bcc27aa 100644
--- a/beacon_node/eth1/src/deposit_log.rs
+++ b/beacon_node/eth1/src/deposit_log.rs
@@ -1,8 +1,6 @@
 use super::http::Log;
 use ssz::Decode;
-use state_processing::per_block_processing::signature_sets::{
-    deposit_pubkey_signature_message, deposit_signature_set,
-};
+use state_processing::per_block_processing::signature_sets::deposit_pubkey_signature_message;
 use types::{ChainSpec, DepositData, Hash256, PublicKeyBytes, SignatureBytes};
 
 pub use eth2::lighthouse::DepositLog;
@@ -53,7 +51,9 @@ impl Log {
         };
 
         let signature_is_valid = deposit_pubkey_signature_message(&deposit_data, spec)
-            .map_or(false, |msg| deposit_signature_set(&msg).verify());
+            .map_or(false, |(public_key, signature, msg)| {
+                signature.verify(&public_key, msg)
+            });
 
         Ok(DepositLog {
             deposit_data,
diff --git a/consensus/state_processing/src/per_block_processing/signature_sets.rs b/consensus/state_processing/src/per_block_processing/signature_sets.rs
index 80d1b936a8..857eca13a0 100644
--- a/consensus/state_processing/src/per_block_processing/signature_sets.rs
+++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs
@@ -288,8 +288,6 @@ where
 }
 
 /// Returns the BLS values in a `Deposit`, if they're all valid. Otherwise, returns `None`.
-///
-/// This method is separate to `deposit_signature_set` to satisfy lifetime requirements.
 pub fn deposit_pubkey_signature_message(
     deposit_data: &DepositData,
     spec: &ChainSpec,
@@ -301,18 +299,6 @@ pub fn deposit_pubkey_signature_message(
     Some((pubkey, signature, message))
 }
 
-/// Returns the signature set for some set of deposit signatures, made with
-/// `deposit_pubkey_signature_message`.
-pub fn deposit_signature_set(
-    pubkey_signature_message: &(PublicKey, Signature, Hash256),
-) -> SignatureSet {
-    let (pubkey, signature, message) = pubkey_signature_message;
-
-    // Note: Deposits are valid across forks, thus the deposit domain is computed
-    // with the fok zeroed.
-    SignatureSet::single_pubkey(signature, Cow::Borrowed(pubkey), *message)
-}
-
 /// Returns a signature set that is valid if the `SignedVoluntaryExit` was signed by the indicated
 /// validator.
 pub fn exit_signature_set<'a, T, F>(
diff --git a/consensus/state_processing/src/per_block_processing/verify_deposit.rs b/consensus/state_processing/src/per_block_processing/verify_deposit.rs
index 5e7e6f1ad1..7290df1a09 100644
--- a/consensus/state_processing/src/per_block_processing/verify_deposit.rs
+++ b/consensus/state_processing/src/per_block_processing/verify_deposit.rs
@@ -1,7 +1,5 @@
 use super::errors::{BlockOperationError, DepositInvalid};
-use crate::per_block_processing::signature_sets::{
-    deposit_pubkey_signature_message, deposit_signature_set,
-};
+use crate::per_block_processing::signature_sets::deposit_pubkey_signature_message;
 use merkle_proof::verify_merkle_proof;
 use safe_arith::SafeArith;
 use tree_hash::TreeHash;
@@ -17,11 +15,11 @@ fn error(reason: DepositInvalid) -> BlockOperationError<DepositInvalid> {
 ///
 /// Spec v0.12.1
 pub fn verify_deposit_signature(deposit_data: &DepositData, spec: &ChainSpec) -> Result<()> {
-    let deposit_signature_message = deposit_pubkey_signature_message(&deposit_data, spec)
+    let (public_key, signature, msg) = deposit_pubkey_signature_message(&deposit_data, spec)
         .ok_or_else(|| error(DepositInvalid::BadBlsBytes))?;
 
     verify!(
-        deposit_signature_set(&deposit_signature_message).verify(),
+        signature.verify(&public_key, msg),
         DepositInvalid::BadSignature
     );