diff --git a/Cargo.lock b/Cargo.lock index 6ffef90840..0a750bfe17 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3019,7 +3019,6 @@ dependencies = [ "parking_lot 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.104 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.104 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)", "serde_yaml 0.8.11 (registry+https://github.com/rust-lang/crates.io-index)", "types 0.1.0", ] diff --git a/beacon_node/beacon_chain/src/fork_choice.rs b/beacon_node/beacon_chain/src/fork_choice.rs index a41628d72a..17a0abfacb 100644 --- a/beacon_node/beacon_chain/src/fork_choice.rs +++ b/beacon_node/beacon_chain/src/fork_choice.rs @@ -2,20 +2,14 @@ mod checkpoint_manager; use crate::{errors::BeaconChainError, metrics, BeaconChain, BeaconChainTypes}; use checkpoint_manager::{CheckpointManager, CheckpointWithBalances}; -use parking_lot::RwLock; -use proto_array_fork_choice::ProtoArrayForkChoice; +use parking_lot::{RwLock, RwLockReadGuard}; +use proto_array_fork_choice::{core::ProtoArray, ProtoArrayForkChoice}; use ssz_derive::{Decode, Encode}; use state_processing::common::get_attesting_indices; -use std::fs::File; -use std::io::Write; use std::marker::PhantomData; -use std::time::{SystemTime, UNIX_EPOCH}; use store::Error as StoreError; use types::{Attestation, BeaconBlock, BeaconState, BeaconStateError, Epoch, Hash256}; -/// If `true`, fork choice will be dumped to a JSON file in `/tmp` whenever find head fail. -pub const FORK_CHOICE_DEBUGGING: bool = true; - type Result = std::result::Result; #[derive(Debug, PartialEq)] @@ -103,20 +97,6 @@ impl ForkChoice { metrics::stop_timer(timer); - if FORK_CHOICE_DEBUGGING { - if let Err(e) = &result { - if let Ok(duration) = SystemTime::now().duration_since(UNIX_EPOCH) { - let time = duration.as_millis(); - if let Ok(mut file) = File::create(format!("/tmp/fork-choice-{}", time)) { - let _ = write!(file, "{:?}\n", e); - if let Ok(json) = self.backend.as_json() { - let _ = write!(file, "{}", json); - } - } - } - } - } - result } @@ -231,8 +211,11 @@ impl ForkChoice { self.backend.maybe_prune(finalized_root).map_err(Into::into) } - pub fn as_json(&self) -> Result { - self.backend.as_json().map_err(Error::UnableToJsonEncode) + /// Returns a read-lock to core `ProtoArray` struct. + /// + /// Should only be used when encoding/decoding during troubleshooting. + pub fn core_proto_array(&self) -> RwLockReadGuard { + self.backend.core_proto_array() } /// Returns a `SszForkChoice` which contains the current state of `Self`. diff --git a/beacon_node/rest_api/src/advanced.rs b/beacon_node/rest_api/src/advanced.rs index 6761f707b9..d7ab80299a 100644 --- a/beacon_node/rest_api/src/advanced.rs +++ b/beacon_node/rest_api/src/advanced.rs @@ -1,5 +1,5 @@ use crate::response_builder::ResponseBuilder; -use crate::{ApiError, ApiResult}; +use crate::ApiResult; use beacon_chain::{BeaconChain, BeaconChainTypes}; use hyper::{Body, Request}; use std::sync::Arc; @@ -11,8 +11,5 @@ pub fn get_fork_choice( req: Request, beacon_chain: Arc>, ) -> ApiResult { - let json = beacon_chain.fork_choice.as_json().map_err(|e| { - ApiError::ServerError(format!("Unable to encode fork choice as JSON: {:?}", e)) - })?; - ResponseBuilder::new(&req)?.body_no_ssz(&json) + ResponseBuilder::new(&req)?.body_no_ssz(&*beacon_chain.fork_choice.core_proto_array()) } diff --git a/beacon_node/rest_api/tests/test.rs b/beacon_node/rest_api/tests/test.rs index d8d370dc09..12eed8bbc5 100644 --- a/beacon_node/rest_api/tests/test.rs +++ b/beacon_node/rest_api/tests/test.rs @@ -799,15 +799,21 @@ fn get_fork_choice() { let node = build_node(&mut env, testing_client_config()); let remote_node = node.remote_node().expect("should produce remote node"); - // Ideally we would check that the returned fork choice is the same as the one in the - // `beacon_chain`, however that would involve exposing (making public) the core fork choice - // struct that is a bit messy. - // - // Given that serializing the fork choice is just vanilla serde, I think it's fair to assume it - // works. - env.runtime() + let fork_choice = env + .runtime() .block_on(remote_node.http.advanced().get_fork_choice()) .expect("should not error when getting fork choice"); + + assert_eq!( + fork_choice, + *node + .client + .beacon_chain() + .expect("node should have beacon chain") + .fork_choice + .core_proto_array(), + "result should be as expected" + ); } fn compare_validator_response( diff --git a/eth2/proto_array_fork_choice/Cargo.toml b/eth2/proto_array_fork_choice/Cargo.toml index 14995a2172..687b6aefcc 100644 --- a/eth2/proto_array_fork_choice/Cargo.toml +++ b/eth2/proto_array_fork_choice/Cargo.toml @@ -17,4 +17,3 @@ eth2_ssz_derive = "0.1.0" serde = "1.0.102" serde_derive = "1.0.102" serde_yaml = "0.8.11" -serde_json = "1.0" diff --git a/eth2/proto_array_fork_choice/src/proto_array.rs b/eth2/proto_array_fork_choice/src/proto_array.rs index 6c01d60289..5a5791601c 100644 --- a/eth2/proto_array_fork_choice/src/proto_array.rs +++ b/eth2/proto_array_fork_choice/src/proto_array.rs @@ -18,7 +18,7 @@ pub struct ProtoNode { best_descendant: Option, } -#[derive(PartialEq, Serialize, Deserialize)] +#[derive(PartialEq, Debug, Serialize, Deserialize)] pub struct ProtoArray { /// Do not attempt to prune the tree unless it has at least this many nodes. Small prunes /// simply waste time. diff --git a/eth2/proto_array_fork_choice/src/proto_array_fork_choice.rs b/eth2/proto_array_fork_choice/src/proto_array_fork_choice.rs index 4224212bf4..a22fd4fb63 100644 --- a/eth2/proto_array_fork_choice/src/proto_array_fork_choice.rs +++ b/eth2/proto_array_fork_choice/src/proto_array_fork_choice.rs @@ -1,8 +1,7 @@ use crate::error::Error; use crate::proto_array::ProtoArray; use crate::ssz_container::SszContainer; -use parking_lot::RwLock; -use serde_json; +use parking_lot::{RwLock, RwLockReadGuard}; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use std::collections::HashMap; @@ -213,9 +212,11 @@ impl ProtoArrayForkChoice { .map_err(|e| format!("Failed to decode ProtoArrayForkChoice: {:?}", e)) } - pub fn as_json(&self) -> Result { - serde_json::to_string(&*self.proto_array.read()) - .map_err(|e| format!("Failed to JSON encode proto_array: {:?}", e)) + /// Returns a read-lock to core `ProtoArray` struct. + /// + /// Should only be used when encoding/decoding during troubleshooting. + pub fn core_proto_array(&self) -> RwLockReadGuard { + self.proto_array.read() } }