Skip to content

Commit

Permalink
Ensure deposit signatures do not use aggregate functions (#1935)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
kirk-baird committed Nov 20, 2020
1 parent d727e55 commit 3b405f1
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 23 deletions.
8 changes: 4 additions & 4 deletions beacon_node/eth1/src/deposit_log.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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>(
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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
);

Expand Down

0 comments on commit 3b405f1

Please sign in to comment.