From 7406ef8e8ad5c3b8586a7229b474e5b722d9e3b3 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 13 Nov 2019 19:42:17 +1100 Subject: [PATCH] Fix signature parsing --- beacon_node/rest_api/src/helpers.rs | 27 +++++++++++++++--- beacon_node/rest_api/src/validator.rs | 12 +++----- beacon_node/rest_api/tests/test.rs | 40 +++++++++++++++++---------- tests/node_test_rig/src/lib.rs | 2 ++ 4 files changed, 54 insertions(+), 27 deletions(-) diff --git a/beacon_node/rest_api/src/helpers.rs b/beacon_node/rest_api/src/helpers.rs index 45eda8b705..eba54e8670 100644 --- a/beacon_node/rest_api/src/helpers.rs +++ b/beacon_node/rest_api/src/helpers.rs @@ -10,11 +10,13 @@ use http::header; use hyper::{Body, Request}; use network::NetworkMessage; use parking_lot::RwLock; -use ssz::Encode; +use ssz::{Decode, Encode}; use std::sync::Arc; use store::{iter::AncestorIter, Store}; use tokio::sync::mpsc; -use types::{Attestation, BeaconBlock, BeaconState, EthSpec, Hash256, RelativeEpoch, Slot}; +use types::{ + Attestation, BeaconBlock, BeaconState, EthSpec, Hash256, RelativeEpoch, Signature, Slot, +}; /// Parse a slot from a `0x` preixed string. /// @@ -41,6 +43,23 @@ pub fn check_content_type_for_json(req: &Request) -> Result<(), ApiError> } } +/// Parse a signature from a `0x` preixed string. +pub fn parse_signature(string: &str) -> Result { + const PREFIX: &str = "0x"; + + if string.starts_with(PREFIX) { + let trimmed = string.trim_start_matches(PREFIX); + let bytes = hex::decode(trimmed) + .map_err(|e| ApiError::BadRequest(format!("Unable to parse signature hex: {:?}", e)))?; + Signature::from_ssz_bytes(&bytes) + .map_err(|e| ApiError::BadRequest(format!("Unable to parse signature bytes: {:?}", e))) + } else { + Err(ApiError::BadRequest( + "Signature must have a '0x' prefix".to_string(), + )) + } +} + /// Parse a root from a `0x` preixed string. /// /// E.g., `"0x0000000000000000000000000000000000000000000000000000000000000000"` @@ -54,7 +73,7 @@ pub fn parse_root(string: &str) -> Result { .map_err(|e| ApiError::BadRequest(format!("Unable to parse root: {:?}", e))) } else { Err(ApiError::BadRequest( - "Root must have a '0x' prefix".to_string(), + "Root must have a '0x' prefix".to_string(), )) } } @@ -71,7 +90,7 @@ pub fn parse_pubkey(string: &str) -> Result { Ok(pubkey) } else { Err(ApiError::BadRequest( - "Public key must have a '0x' prefix".to_string(), + "Public key must have a '0x' prefix".to_string(), )) } } diff --git a/beacon_node/rest_api/src/validator.rs b/beacon_node/rest_api/src/validator.rs index ae48ad864d..9d0899b951 100644 --- a/beacon_node/rest_api/src/validator.rs +++ b/beacon_node/rest_api/src/validator.rs @@ -1,11 +1,11 @@ use crate::helpers::{ check_content_type_for_json, get_beacon_chain_from_request, get_logger_from_request, - parse_pubkey, publish_attestation_to_network, publish_beacon_block_to_network, + parse_pubkey, parse_signature, publish_attestation_to_network, publish_beacon_block_to_network, }; use crate::response_builder::ResponseBuilder; use crate::{ApiError, ApiResult, BoxFut, UrlQuery}; use beacon_chain::{AttestationProcessingOutcome, BeaconChainTypes, BlockProcessingOutcome}; -use bls::{AggregateSignature, PublicKey, Signature, BLS_PUBLIC_KEY_BYTE_SIZE}; +use bls::{AggregateSignature, PublicKey, BLS_PUBLIC_KEY_BYTE_SIZE}; use futures::future::Future; use futures::stream::Stream; use hyper::{Body, Request}; @@ -166,16 +166,12 @@ pub fn get_new_beacon_block(req: Request) - .map_err(|e| { ApiError::BadRequest(format!("Invalid slot parameter, must be a u64. {:?}", e)) })?; - let randao_bytes = query + let randao_reveal = query .first_of(&["randao_reveal"]) - .map(|(_key, value)| value) - .map(hex::decode)? + .and_then(|(_key, value)| parse_signature(&value)) .map_err(|e| { ApiError::BadRequest(format!("Invalid hex string for randao_reveal: {:?}", e)) })?; - let randao_reveal = Signature::from_bytes(randao_bytes.as_slice()).map_err(|e| { - ApiError::BadRequest(format!("randao_reveal is not a valid signature: {:?}", e)) - })?; let (new_block, _state) = beacon_chain .produce_block(randao_reveal, slot) diff --git a/beacon_node/rest_api/tests/test.rs b/beacon_node/rest_api/tests/test.rs index 1ec75c8419..3ab04729a2 100644 --- a/beacon_node/rest_api/tests/test.rs +++ b/beacon_node/rest_api/tests/test.rs @@ -1,12 +1,15 @@ #![cfg(test)] +use beacon_chain::{BeaconChain, BeaconChainTypes}; use node_test_rig::{ environment::{Environment, EnvironmentBuilder}, LocalBeaconNode, }; +use std::sync::Arc; use tree_hash::TreeHash; use types::{ - test_utils::generate_deterministic_keypair, Domain, EthSpec, MinimalEthSpec, Signature, Slot, + test_utils::generate_deterministic_keypair, ChainSpec, Domain, EthSpec, MinimalEthSpec, + Signature, Slot, }; type E = MinimalEthSpec; @@ -21,11 +24,29 @@ fn build_env() -> Environment { .expect("environment should build") } +/// Returns the randao reveal for the given slot (assuming the given `beacon_chain` uses +/// deterministic keypairs). +fn get_randao_reveal( + beacon_chain: Arc>, + slot: Slot, + spec: &ChainSpec, +) -> Signature { + let fork = beacon_chain.head().beacon_state.fork.clone(); + let proposer_index = beacon_chain + .block_proposer(slot) + .expect("should get proposer index"); + let keypair = generate_deterministic_keypair(proposer_index); + let epoch = slot.epoch(E::slots_per_epoch()); + let message = epoch.tree_hash_root(); + let domain = spec.get_domain(epoch, Domain::Randao, &fork); + Signature::new(&message, domain, &keypair.sk) +} + #[test] -fn validator_block() { +fn validator_block_get() { let mut env = build_env(); - let spec = E::default_spec(); + let spec = &E::default_spec(); let node = LocalBeaconNode::production(env.core_context()); let remote_node = node.remote_node().expect("should produce remote node"); @@ -35,19 +56,8 @@ fn validator_block() { .beacon_chain() .expect("client should have beacon chain"); - let fork = beacon_chain.head().beacon_state.fork.clone(); - let slot = Slot::new(1); - let randao_reveal = { - let proposer_index = beacon_chain - .block_proposer(slot) - .expect("should get proposer index"); - let keypair = generate_deterministic_keypair(proposer_index); - let epoch = slot.epoch(E::slots_per_epoch()); - let message = epoch.tree_hash_root(); - let domain = spec.get_domain(epoch, Domain::Randao, &fork); - Signature::new(&message, domain, &keypair.sk) - }; + let randao_reveal = get_randao_reveal(beacon_chain.clone(), slot, spec); let block = env .runtime() diff --git a/tests/node_test_rig/src/lib.rs b/tests/node_test_rig/src/lib.rs index 5a0f21e097..63d87fe044 100644 --- a/tests/node_test_rig/src/lib.rs +++ b/tests/node_test_rig/src/lib.rs @@ -63,5 +63,7 @@ fn testing_client_config() -> (ClientConfig, TempDir) { genesis_time: 13_371_337, }; + client_config.dummy_eth1_backend = true; + (client_config, tempdir) }