Address Michael's comments

This commit is contained in:
Paul Hauner
2019-11-25 15:11:27 +11:00
parent 6bf73e2b55
commit 65cbf601ae
11 changed files with 42 additions and 54 deletions

View File

@@ -525,7 +525,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
} }
} }
/// Produce an `Attestation` that is valid for the given `slot` `shard`. /// Produce an `Attestation` that is valid for the given `slot` and `index`.
/// ///
/// Always attests to the canonical chain. /// Always attests to the canonical chain.
pub fn produce_attestation( pub fn produce_attestation(
@@ -870,8 +870,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
result result
}; };
if block.slot > 0 if block.slot > 0 && block.slot <= finalized_epoch.start_slot(T::EthSpec::slots_per_epoch())
&& (block.slot <= finalized_epoch.start_slot(T::EthSpec::slots_per_epoch()))
{ {
// Ignore any attestation where the slot of `data.beacon_block_root` is equal to or // Ignore any attestation where the slot of `data.beacon_block_root` is equal to or
// prior to the finalized epoch. // prior to the finalized epoch.

View File

@@ -13,28 +13,28 @@ eth2-libp2p = { path = "../eth2-libp2p" }
store = { path = "../store" } store = { path = "../store" }
version = { path = "../version" } version = { path = "../version" }
serde = { version = "1.0", features = ["derive"] } serde = { version = "1.0", features = ["derive"] }
serde_json = "^1.0" serde_json = "1.0"
serde_yaml = "0.8" serde_yaml = "0.8"
slog = "^2.2.3" slog = "2.5"
slog-term = "^2.4.0" slog-term = "2.4"
slog-async = "^2.3.0" slog-async = "2.3"
eth2_ssz = { path = "../../eth2/utils/ssz" } eth2_ssz = { path = "../../eth2/utils/ssz" }
eth2_ssz_derive = { path = "../../eth2/utils/ssz_derive" } eth2_ssz_derive = { path = "../../eth2/utils/ssz_derive" }
state_processing = { path = "../../eth2/state_processing" } state_processing = { path = "../../eth2/state_processing" }
types = { path = "../../eth2/types" } types = { path = "../../eth2/types" }
clap = "2.32.0" clap = "2.33"
http = "^0.1.17" http = "0.1"
hyper = "0.12.35" hyper = "0.12"
exit-future = "0.1.3" exit-future = "0.1.4"
tokio = "0.1.17" tokio = "0.1.22"
url = "2.0" url = "2.1"
lazy_static = "1.3.0" lazy_static = "1.3.0"
eth2_config = { path = "../../eth2/utils/eth2_config" } eth2_config = { path = "../../eth2/utils/eth2_config" }
lighthouse_metrics = { path = "../../eth2/utils/lighthouse_metrics" } lighthouse_metrics = { path = "../../eth2/utils/lighthouse_metrics" }
slot_clock = { path = "../../eth2/utils/slot_clock" } slot_clock = { path = "../../eth2/utils/slot_clock" }
hex = "0.3.2" hex = "0.3"
parking_lot = "0.9" parking_lot = "0.9"
futures = "0.1.25" futures = "0.1.29"
[dev-dependencies] [dev-dependencies]
remote_beacon_node = { path = "../../eth2/utils/remote_beacon_node" } remote_beacon_node = { path = "../../eth2/utils/remote_beacon_node" }

View File

@@ -23,7 +23,7 @@ pub struct HeadResponse {
} }
/// HTTP handler to return a `BeaconBlock` at a given `root` or `slot`. /// HTTP handler to return a `BeaconBlock` at a given `root` or `slot`.
pub fn get_head<T: BeaconChainTypes + 'static>( pub fn get_head<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -64,7 +64,7 @@ pub struct BlockResponse<T: EthSpec> {
} }
/// HTTP handler to return a `BeaconBlock` at a given `root` or `slot`. /// HTTP handler to return a `BeaconBlock` at a given `root` or `slot`.
pub fn get_block<T: BeaconChainTypes + 'static>( pub fn get_block<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -102,7 +102,7 @@ pub fn get_block<T: BeaconChainTypes + 'static>(
} }
/// HTTP handler to return a `BeaconBlock` root at a given `slot`. /// HTTP handler to return a `BeaconBlock` root at a given `slot`.
pub fn get_block_root<T: BeaconChainTypes + 'static>( pub fn get_block_root<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -117,7 +117,7 @@ pub fn get_block_root<T: BeaconChainTypes + 'static>(
} }
/// HTTP handler to return the `Fork` of the current head. /// HTTP handler to return the `Fork` of the current head.
pub fn get_fork<T: BeaconChainTypes + 'static>( pub fn get_fork<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -128,7 +128,7 @@ pub fn get_fork<T: BeaconChainTypes + 'static>(
/// ///
/// The `Epoch` parameter can be any epoch number. If it is not specified, /// The `Epoch` parameter can be any epoch number. If it is not specified,
/// the current epoch is assumed. /// the current epoch is assumed.
pub fn get_validators<T: BeaconChainTypes + 'static>( pub fn get_validators<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -168,7 +168,7 @@ pub struct StateResponse<T: EthSpec> {
/// ///
/// Will not return a state if the request slot is in the future. Will return states higher than /// Will not return a state if the request slot is in the future. Will return states higher than
/// the current head by skipping slots. /// the current head by skipping slots.
pub fn get_state<T: BeaconChainTypes + 'static>( pub fn get_state<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -216,7 +216,7 @@ pub fn get_state<T: BeaconChainTypes + 'static>(
/// ///
/// Will not return a state if the request slot is in the future. Will return states higher than /// Will not return a state if the request slot is in the future. Will return states higher than
/// the current head by skipping slots. /// the current head by skipping slots.
pub fn get_state_root<T: BeaconChainTypes + 'static>( pub fn get_state_root<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -229,7 +229,7 @@ pub fn get_state_root<T: BeaconChainTypes + 'static>(
} }
/// HTTP handler to return the highest finalized slot. /// HTTP handler to return the highest finalized slot.
pub fn get_current_finalized_checkpoint<T: BeaconChainTypes + 'static>( pub fn get_current_finalized_checkpoint<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -241,7 +241,7 @@ pub fn get_current_finalized_checkpoint<T: BeaconChainTypes + 'static>(
} }
/// HTTP handler to return a `BeaconState` at the genesis block. /// HTTP handler to return a `BeaconState` at the genesis block.
pub fn get_genesis_state<T: BeaconChainTypes + 'static>( pub fn get_genesis_state<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -251,7 +251,7 @@ pub fn get_genesis_state<T: BeaconChainTypes + 'static>(
} }
/// Read the genesis time from the current beacon chain state. /// Read the genesis time from the current beacon chain state.
pub fn get_genesis_time<T: BeaconChainTypes + 'static>( pub fn get_genesis_time<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {

View File

@@ -28,7 +28,7 @@ lazy_static! {
/// # Note /// # Note
/// ///
/// This is a HTTP handler method. /// This is a HTTP handler method.
pub fn get_prometheus<T: BeaconChainTypes + 'static>( pub fn get_prometheus<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
db_path: PathBuf, db_path: PathBuf,

View File

@@ -7,7 +7,7 @@ use std::sync::Arc;
use types::EthSpec; use types::EthSpec;
/// HTTP handler to return the full spec object. /// HTTP handler to return the full spec object.
pub fn get_spec<T: BeaconChainTypes + 'static>( pub fn get_spec<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -15,7 +15,7 @@ pub fn get_spec<T: BeaconChainTypes + 'static>(
} }
/// HTTP handler to return the full Eth2Config object. /// HTTP handler to return the full Eth2Config object.
pub fn get_eth2_config<T: BeaconChainTypes + 'static>( pub fn get_eth2_config<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
eth2_config: Arc<Eth2Config>, eth2_config: Arc<Eth2Config>,
) -> ApiResult { ) -> ApiResult {
@@ -23,6 +23,6 @@ pub fn get_eth2_config<T: BeaconChainTypes + 'static>(
} }
/// HTTP handler to return the full spec object. /// HTTP handler to return the full spec object.
pub fn get_slots_per_epoch<T: BeaconChainTypes + 'static>(req: Request<Body>) -> ApiResult { pub fn get_slots_per_epoch<T: BeaconChainTypes>(req: Request<Body>) -> ApiResult {
ResponseBuilder::new(&req)?.body(&T::EthSpec::slots_per_epoch()) ResponseBuilder::new(&req)?.body(&T::EthSpec::slots_per_epoch())
} }

View File

@@ -41,7 +41,7 @@ pub struct BulkValidatorDutiesRequest {
/// HTTP Handler to retrieve a the duties for a set of validators during a particular epoch. This /// HTTP Handler to retrieve a the duties for a set of validators during a particular epoch. This
/// method allows for collecting bulk sets of validator duties without risking exceeding the max /// method allows for collecting bulk sets of validator duties without risking exceeding the max
/// URL length with query pairs. /// URL length with query pairs.
pub fn post_validator_duties<T: BeaconChainTypes + 'static>( pub fn post_validator_duties<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> BoxFut { ) -> BoxFut {
@@ -70,7 +70,7 @@ pub fn post_validator_duties<T: BeaconChainTypes + 'static>(
/// HTTP Handler to retrieve a the duties for a set of validators during a particular epoch /// HTTP Handler to retrieve a the duties for a set of validators during a particular epoch
/// ///
/// The given `epoch` must be within one epoch of the current epoch. /// The given `epoch` must be within one epoch of the current epoch.
pub fn get_validator_duties<T: BeaconChainTypes + 'static>( pub fn get_validator_duties<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -88,7 +88,7 @@ pub fn get_validator_duties<T: BeaconChainTypes + 'static>(
ResponseBuilder::new(&req)?.body_no_ssz(&duties) ResponseBuilder::new(&req)?.body_no_ssz(&duties)
} }
fn return_validator_duties<T: BeaconChainTypes + 'static>( fn return_validator_duties<T: BeaconChainTypes>(
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
epoch: Epoch, epoch: Epoch,
validator_pubkeys: Vec<PublicKey>, validator_pubkeys: Vec<PublicKey>,
@@ -174,7 +174,7 @@ fn return_validator_duties<T: BeaconChainTypes + 'static>(
} }
/// HTTP Handler to produce a new BeaconBlock from the current state, ready to be signed by a validator. /// HTTP Handler to produce a new BeaconBlock from the current state, ready to be signed by a validator.
pub fn get_new_beacon_block<T: BeaconChainTypes + 'static>( pub fn get_new_beacon_block<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -196,7 +196,7 @@ pub fn get_new_beacon_block<T: BeaconChainTypes + 'static>(
} }
/// HTTP Handler to publish a BeaconBlock, which has been signed by a validator. /// HTTP Handler to publish a BeaconBlock, which has been signed by a validator.
pub fn publish_beacon_block<T: BeaconChainTypes + 'static>( pub fn publish_beacon_block<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
network_chan: NetworkChannel, network_chan: NetworkChannel,
@@ -241,7 +241,7 @@ pub fn publish_beacon_block<T: BeaconChainTypes + 'static>(
} }
/// HTTP Handler to produce a new Attestation from the current state, ready to be signed by a validator. /// HTTP Handler to produce a new Attestation from the current state, ready to be signed by a validator.
pub fn get_new_attestation<T: BeaconChainTypes + 'static>( pub fn get_new_attestation<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
) -> ApiResult { ) -> ApiResult {
@@ -258,7 +258,7 @@ pub fn get_new_attestation<T: BeaconChainTypes + 'static>(
} }
/// HTTP Handler to publish an Attestation, which has been signed by a validator. /// HTTP Handler to publish an Attestation, which has been signed by a validator.
pub fn publish_attestation<T: BeaconChainTypes + 'static>( pub fn publish_attestation<T: BeaconChainTypes>(
req: Request<Body>, req: Request<Body>,
beacon_chain: Arc<BeaconChain<T>>, beacon_chain: Arc<BeaconChain<T>>,
network_chan: NetworkChannel, network_chan: NetworkChannel,

View File

@@ -7,7 +7,7 @@ use node_test_rig::{
}; };
use remote_beacon_node::{PublishStatus, ValidatorDuty}; use remote_beacon_node::{PublishStatus, ValidatorDuty};
use std::sync::Arc; use std::sync::Arc;
use tree_hash::{SignedRoot, TreeHash}; use tree_hash::TreeHash;
use types::{ use types::{
test_utils::generate_deterministic_keypair, BeaconBlock, ChainSpec, Domain, Epoch, EthSpec, test_utils::generate_deterministic_keypair, BeaconBlock, ChainSpec, Domain, Epoch, EthSpec,
MinimalEthSpec, PublicKey, RelativeEpoch, Signature, Slot, MinimalEthSpec, PublicKey, RelativeEpoch, Signature, Slot,
@@ -79,8 +79,6 @@ fn validator_produce_attestation() {
.expect("should have attestation duties cache") .expect("should have attestation duties cache")
.expect("should have attestation duties"); .expect("should have attestation duties");
dbg!(&duties);
let mut attestation = env let mut attestation = env
.runtime() .runtime()
.block_on( .block_on(
@@ -150,7 +148,6 @@ fn validator_produce_attestation() {
.publish_attestation(attestation.clone()), .publish_attestation(attestation.clone()),
) )
.expect("should publish attestation"); .expect("should publish attestation");
dbg!(publish_status.clone());
assert!( assert!(
publish_status.is_valid(), publish_status.is_valid(),
"the signed published attestation should be valid" "the signed published attestation should be valid"

View File

@@ -45,5 +45,5 @@ against the Ethereum Foundation specifications.
These tests are quite large (100's of MB) so they're only downloaded if you run These tests are quite large (100's of MB) so they're only downloaded if you run
`$ make test-ef` (or anything that run it). You may want to avoid `$ make test-ef` (or anything that run it). You may want to avoid
downloadingthese tests if you're on a slow or metered Internet connection. CI downloading these tests if you're on a slow or metered Internet connection. CI
will require them to pass, though. will require them to pass, though.

View File

@@ -39,7 +39,7 @@ impl SlotClock for TestingSlotClock {
/// Always returns a duration of `1 * slots_per_epoch` second. /// Always returns a duration of `1 * slots_per_epoch` second.
fn duration_to_next_epoch(&self, slots_per_epoch: u64) -> Option<Duration> { fn duration_to_next_epoch(&self, slots_per_epoch: u64) -> Option<Duration> {
Some(Duration::from_secs(1 + slots_per_epoch)) Some(Duration::from_secs(slots_per_epoch))
} }
/// Always returns a slot duration of 0 seconds. /// Always returns a slot duration of 0 seconds.

View File

@@ -90,16 +90,8 @@ impl DutiesStore {
return InsertOutcome::Invalid; return InsertOutcome::Invalid;
} }
if store.contains_key(&duties.validator_pubkey) { if let Some(validator_map) = store.get_mut(&duties.validator_pubkey) {
let validator_map = store.get_mut(&duties.validator_pubkey).expect( if let Some(known_duties) = validator_map.get_mut(&epoch) {
"Store is exclusively locked and this path is guarded to ensure the key exists.",
);
if validator_map.contains_key(&epoch) {
let known_duties = validator_map.get_mut(&epoch).expect(
"Validator map is exclusively mutable and this path is guarded to ensure the key exists.",
);
if *known_duties == duties { if *known_duties == duties {
InsertOutcome::Identical InsertOutcome::Identical
} else { } else {

View File

@@ -202,11 +202,11 @@ impl ValidatorDirectoryBuilder {
let voting_keypair = self let voting_keypair = self
.voting_keypair .voting_keypair
.clone() .clone()
.ok_or_else(|| "build requires a voting_keypair")?; .ok_or_else(|| "write_keypair_files requires a voting_keypair")?;
let withdrawal_keypair = self let withdrawal_keypair = self
.withdrawal_keypair .withdrawal_keypair
.clone() .clone()
.ok_or_else(|| "build requires a withdrawal_keypair")?; .ok_or_else(|| "write_keypair_files requires a withdrawal_keypair")?;
self.save_keypair(voting_keypair, VOTING_KEY_PREFIX)?; self.save_keypair(voting_keypair, VOTING_KEY_PREFIX)?;
self.save_keypair(withdrawal_keypair, WITHDRAWAL_KEY_PREFIX)?; self.save_keypair(withdrawal_keypair, WITHDRAWAL_KEY_PREFIX)?;