From c552b0a549070aee6631b5a3d6af016fb5f2f37f Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 26 Aug 2020 12:50:26 +1000 Subject: [PATCH] Ensure skip slot states are loaded --- beacon_node/beacon_chain/src/beacon_chain.rs | 12 ++ beacon_node/http_api/src/state_id.rs | 29 +++-- beacon_node/http_api/tests/tests.rs | 114 ++++++++++++++++--- 3 files changed, 133 insertions(+), 22 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index c6b81cea39f..6d306c7cc4a 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -460,6 +460,18 @@ impl BeaconChain { } } + /// Returns the block at the given slot, if any. Only returns blocks in the canonical chain. + /// + /// ## Errors + /// + /// May return a database error. + pub fn state_root_at_slot(&self, slot: Slot) -> Result, Error> { + process_results(self.rev_iter_state_roots()?, |mut iter| { + iter.find(|(_, this_slot)| *this_slot == slot) + .map(|(root, _)| root) + }) + } + /// Returns the block root at the given slot, if any. Only returns roots in the canonical chain. /// /// ## Errors diff --git a/beacon_node/http_api/src/state_id.rs b/beacon_node/http_api/src/state_id.rs index 7f3dfb0540b..a35ad8bbe3a 100644 --- a/beacon_node/http_api/src/state_id.rs +++ b/beacon_node/http_api/src/state_id.rs @@ -1,8 +1,7 @@ -use crate::BlockId; use beacon_chain::{BeaconChain, BeaconChainTypes}; -use eth2::types::{BlockId as CoreBlockId, StateId as CoreStateId}; +use eth2::types::StateId as CoreStateId; use std::str::FromStr; -use types::{BeaconState, Fork, Hash256}; +use types::{BeaconState, EthSpec, Fork, Hash256}; pub struct StateId(CoreStateId); @@ -11,7 +10,7 @@ impl StateId { &self, chain: &BeaconChain, ) -> Result { - let block = match &self.0 { + let slot = match &self.0 { CoreStateId::Head => { return chain .head_info() @@ -19,13 +18,25 @@ impl StateId { .map_err(crate::reject::beacon_chain_error) } CoreStateId::Genesis => return Ok(chain.genesis_state_root), - CoreStateId::Finalized => BlockId(CoreBlockId::Finalized).block(chain), - CoreStateId::Justified => BlockId(CoreBlockId::Justified).block(chain), - CoreStateId::Slot(slot) => BlockId(CoreBlockId::Slot(*slot)).block(chain), + CoreStateId::Finalized => chain.head_info().map(|head| { + head.finalized_checkpoint + .epoch + .start_slot(T::EthSpec::slots_per_epoch()) + }), + CoreStateId::Justified => chain.head_info().map(|head| { + head.current_justified_checkpoint + .epoch + .start_slot(T::EthSpec::slots_per_epoch()) + }), + CoreStateId::Slot(slot) => Ok(*slot), CoreStateId::Root(root) => return Ok(*root), - }?; + } + .map_err(crate::reject::beacon_chain_error)?; - Ok(block.state_root()) + chain + .state_root_at_slot(slot) + .map_err(crate::reject::beacon_chain_error)? + .ok_or_else(|| warp::reject::not_found()) } pub fn fork( diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 0f56193ee95..bf340f919ea 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -7,13 +7,28 @@ use http_api::Context; use std::sync::Arc; use store::config::StoreConfig; use tokio::sync::oneshot; -use types::{test_utils::generate_deterministic_keypairs, MainnetEthSpec}; +use types::{test_utils::generate_deterministic_keypairs, EthSpec, MainnetEthSpec, Slot}; + +type E = MainnetEthSpec; const VALIDATOR_COUNT: usize = 24; -const CHAIN_LENGTH: usize = 32 * 6; +const SLOTS_PER_EPOCH: u64 = 32; +const CHAIN_LENGTH: u64 = SLOTS_PER_EPOCH * 5; +const JUSTIFIED_EPOCH: u64 = 4; +const FINALIZED_EPOCH: u64 = 3; + +/// Skipping the slots around the epoch boundary allows us to check that we're obtaining states +/// from skipped slots for the finalized and justified checkpoints (instead of the state from the +/// block that those roots point to). +const SKIPPED_SLOTS: &[u64] = &[ + JUSTIFIED_EPOCH * SLOTS_PER_EPOCH - 1, + JUSTIFIED_EPOCH * SLOTS_PER_EPOCH, + FINALIZED_EPOCH * SLOTS_PER_EPOCH - 1, + FINALIZED_EPOCH * SLOTS_PER_EPOCH, +]; pub struct ApiTester { - chain: Arc>>, + chain: Arc>>, client: BeaconNodeClient, _server_shutdown: oneshot::Sender<()>, } @@ -28,14 +43,37 @@ impl ApiTester { harness.advance_slot(); - harness.extend_chain( - CHAIN_LENGTH, - BlockStrategy::OnCanonicalHead, - AttestationStrategy::AllValidators, - ); + for _ in 0..CHAIN_LENGTH { + let slot = harness.chain.slot().unwrap().as_u64(); + + if !SKIPPED_SLOTS.contains(&slot) { + harness.extend_chain( + 1, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ); + } + + harness.advance_slot(); + } let chain = Arc::new(harness.chain); + assert_eq!( + chain.head_info().unwrap().finalized_checkpoint.epoch, + 3, + "precondition: finality" + ); + assert_eq!( + chain + .head_info() + .unwrap() + .current_justified_checkpoint + .epoch, + 4, + "precondition: justification" + ); + let context = Arc::new(Context { chain: Some(chain.clone()), listen_address: [127, 0, 0, 1], @@ -43,7 +81,7 @@ impl ApiTester { }); let ctx = context.clone(); let (listening_socket, server, server_shutdown) = http_api::serve(ctx).unwrap(); - dbg!(listening_socket); + tokio::spawn(async { server.await }); let client = BeaconNodeClient::new(format!( @@ -64,16 +102,66 @@ impl ApiTester { let expected = match state_id { StateId::Head => self.chain.head_info().unwrap().state_root, - _ => todo!(), + StateId::Genesis => self.chain.genesis_state_root, + StateId::Finalized => { + let finalized_slot = self + .chain + .head_info() + .unwrap() + .finalized_checkpoint + .epoch + .start_slot(E::slots_per_epoch()); + + self.chain + .state_root_at_slot(finalized_slot) + .unwrap() + .unwrap() + } + StateId::Justified => { + let justified_slot = self + .chain + .head_info() + .unwrap() + .current_justified_checkpoint + .epoch + .start_slot(E::slots_per_epoch()); + + self.chain + .state_root_at_slot(justified_slot) + .unwrap() + .unwrap() + } + StateId::Slot(slot) => self.chain.state_root_at_slot(slot).unwrap().unwrap(), + StateId::Root(root) => root, }; - assert_eq!(result.data.root, expected); + assert_eq!(result.data.root, expected, "{:?}", state_id); self } } #[tokio::test(core_threads = 2)] -async fn my_test() { - ApiTester::new().test_beacon_state_root(StateId::Head).await; +async fn beacon_state_root() { + ApiTester::new() + .test_beacon_state_root(StateId::Head) + .await + .test_beacon_state_root(StateId::Genesis) + .await + .test_beacon_state_root(StateId::Finalized) + .await + .test_beacon_state_root(StateId::Justified) + .await + .test_beacon_state_root(StateId::Slot(Slot::new(0))) + .await + .test_beacon_state_root(StateId::Slot(Slot::new(32))) + .await + .test_beacon_state_root(StateId::Slot(Slot::from(SKIPPED_SLOTS[0]))) + .await + .test_beacon_state_root(StateId::Slot(Slot::from(SKIPPED_SLOTS[1]))) + .await + .test_beacon_state_root(StateId::Slot(Slot::from(SKIPPED_SLOTS[2]))) + .await + .test_beacon_state_root(StateId::Slot(Slot::from(SKIPPED_SLOTS[3]))) + .await; }