From cd0a847a7400e0fda5c44f807852b4208ebc3da4 Mon Sep 17 00:00:00 2001 From: Mikhail Date: Mon, 24 Aug 2020 10:34:23 -0700 Subject: [PATCH] fix(tests): Fix adversarial tests with view client Adversarial tests broke after running multiple view client actors, fix them. Fixes #3137 Test plan --------- wrong_sync_info.py --- chain/client/src/client_actor.rs | 30 +++++++++++++-------------- chain/client/src/lib.rs | 2 ++ chain/client/src/test_utils.rs | 10 +++++++++ chain/client/src/view_client.rs | 34 +++++++++++++++++-------------- chain/network/tests/runner/mod.rs | 7 +++++++ neard/src/lib.rs | 9 ++++++++ 6 files changed, 61 insertions(+), 31 deletions(-) diff --git a/chain/client/src/client_actor.rs b/chain/client/src/client_actor.rs index 3ca7b97f9eb..16415371ab4 100644 --- a/chain/client/src/client_actor.rs +++ b/chain/client/src/client_actor.rs @@ -1,15 +1,17 @@ //! Client actor orchestrates Client and facilitates network connection. -use chrono::Duration as OldDuration; use std::collections::HashMap; use std::sync::{Arc, RwLock}; use std::thread; use std::time::{Duration, Instant}; use actix::{Actor, Addr, Arbiter, AsyncContext, Context, Handler}; +use chrono::Duration as OldDuration; use chrono::{DateTime, Utc}; use log::{debug, error, info, trace, warn}; +#[cfg(feature = "delay_detector")] +use delay_detector::DelayDetector; use near_chain::test_utils::format_hash; use near_chain::types::AcceptedBlock; #[cfg(feature = "adversarial")] @@ -49,9 +51,9 @@ use crate::types::{ Error, GetNetworkInfo, NetworkInfoResponse, ShardSyncDownload, ShardSyncStatus, Status, StatusSyncInfo, SyncStatus, }; +#[cfg(feature = "adversarial")] +use crate::AdversarialControls; use crate::StatusResponse; -#[cfg(feature = "delay_detector")] -use delay_detector::DelayDetector; /// Multiplier on `max_block_time` to wait until deciding that chain stalled. const STATUS_WAIT_TIME_MULTIPLIER: u64 = 10; @@ -62,11 +64,7 @@ const HEAD_STALL_MULTIPLIER: u32 = 4; pub struct ClientActor { /// Adversarial controls #[cfg(feature = "adversarial")] - pub adv_sync_height: Option, - #[cfg(feature = "adversarial")] - pub adv_disable_header_sync: bool, - #[cfg(feature = "adversarial")] - pub adv_disable_doomslug: bool, + pub adv: Arc>, client: Client, network_adapter: Arc, @@ -117,6 +115,7 @@ impl ClientActor { validator_signer: Option>, telemetry_actor: Addr, enable_doomslug: bool, + #[cfg(feature = "adversarial")] adv: Arc>, ) -> Result { wait_until_genesis(&chain_genesis.time); if let Some(vs) = &validator_signer { @@ -135,11 +134,7 @@ impl ClientActor { let now = Utc::now(); Ok(ClientActor { #[cfg(feature = "adversarial")] - adv_sync_height: None, - #[cfg(feature = "adversarial")] - adv_disable_header_sync: false, - #[cfg(feature = "adversarial")] - adv_disable_doomslug: false, + adv, client, network_adapter, node_id, @@ -202,14 +197,14 @@ impl Handler for ClientActor { return match adversarial_msg { NetworkAdversarialMessage::AdvDisableDoomslug => { info!(target: "adversary", "Turning Doomslug off"); - self.adv_disable_doomslug = true; + self.adv.write().unwrap().adv_disable_doomslug = true; self.client.doomslug.adv_disable(); self.client.chain.adv_disable_doomslug(); NetworkClientResponses::NoResponse } NetworkAdversarialMessage::AdvDisableHeaderSync => { info!(target: "adversary", "Blocking header sync"); - self.adv_disable_header_sync = true; + self.adv.write().unwrap().adv_disable_header_sync = true; NetworkClientResponses::NoResponse } NetworkAdversarialMessage::AdvProduceBlocks(num_blocks, only_valid) => { @@ -1030,7 +1025,7 @@ impl ClientActor { fn needs_syncing(&self, needs_syncing: bool) -> bool { #[cfg(feature = "adversarial")] { - if self.adv_disable_header_sync { + if self.adv.read().unwrap().adv_disable_header_sync { return false; } } @@ -1340,6 +1335,7 @@ pub fn start_client( network_adapter: Arc, validator_signer: Option>, telemetry_actor: Addr, + #[cfg(feature = "adversarial")] adv: Arc>, ) -> (Addr, Arbiter) { let client_arbiter = Arbiter::current(); let client_addr = ClientActor::start_in_arbiter(&client_arbiter, move |_ctx| { @@ -1352,6 +1348,8 @@ pub fn start_client( validator_signer, telemetry_actor, true, + #[cfg(feature = "adversarial")] + adv, ) .unwrap() }); diff --git a/chain/client/src/lib.rs b/chain/client/src/lib.rs index 7e0c4482c83..b72fc919009 100644 --- a/chain/client/src/lib.rs +++ b/chain/client/src/lib.rs @@ -9,6 +9,8 @@ pub use crate::types::{ GetNextLightClientBlock, GetStateChanges, GetStateChangesInBlock, GetValidatorInfo, GetValidatorOrdered, Query, Status, StatusResponse, SyncStatus, TxStatus, TxStatusError, }; +#[cfg(feature = "adversarial")] +pub use crate::view_client::AdversarialControls; pub use crate::view_client::{start_view_client, ViewClientActor}; mod client; diff --git a/chain/client/src/test_utils.rs b/chain/client/src/test_utils.rs index 8cd7b1adf22..db63c77b901 100644 --- a/chain/client/src/test_utils.rs +++ b/chain/client/src/test_utils.rs @@ -41,6 +41,8 @@ use near_store::test_utils::create_test_store; use near_store::Store; use near_telemetry::TelemetryActor; +#[cfg(feature = "adversarial")] +use crate::AdversarialControls; use crate::{start_view_client, Client, ClientActor, SyncStatus, ViewClientActor}; use near_network::test_utils::MockNetworkAdapter; use near_primitives::merkle::{merklize, MerklePath}; @@ -108,12 +110,18 @@ pub fn setup( num_validator_seats, archive, ); + + #[cfg(feature = "adversarial")] + let adv = Arc::new(RwLock::new(AdversarialControls::default())); + let view_client_addr = start_view_client( Some(signer.validator_id().clone()), chain_genesis.clone(), runtime.clone(), network_adapter.clone(), config.clone(), + #[cfg(feature = "adversarial")] + adv.clone(), ); let client = ClientActor::new( @@ -125,6 +133,8 @@ pub fn setup( Some(signer), telemetry, enable_doomslug, + #[cfg(feature = "adversarial")] + adv, ) .unwrap(); (genesis_block, client, view_client_addr) diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index 1d718528647..448582e9a74 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -65,14 +65,18 @@ pub struct ViewClientRequestManager { pub receipt_outcome_requests: SizedCache, } -/// View client provides currently committed (to the storage) view of the current chain and state. -pub struct ViewClientActor { - #[cfg(feature = "adversarial")] +#[cfg(feature = "adversarial")] +#[derive(Default)] +pub struct AdversarialControls { pub adv_disable_header_sync: bool, - #[cfg(feature = "adversarial")] pub adv_disable_doomslug: bool, - #[cfg(feature = "adversarial")] pub adv_sync_height: Option, +} + +/// View client provides currently committed (to the storage) view of the current chain and state. +pub struct ViewClientActor { + #[cfg(feature = "adversarial")] + pub adv: Arc>, /// Validator account (if present). validator_account_id: Option, @@ -103,6 +107,7 @@ impl ViewClientActor { network_adapter: Arc, config: ClientConfig, request_manager: Arc>, + #[cfg(feature = "adversarial")] adv: Arc>, ) -> Result { // TODO: should we create shared ChainStore that is passed to both Client and ViewClient? let chain = Chain::new_for_view_client( @@ -112,11 +117,7 @@ impl ViewClientActor { )?; Ok(ViewClientActor { #[cfg(feature = "adversarial")] - adv_disable_header_sync: false, - #[cfg(feature = "adversarial")] - adv_disable_doomslug: false, - #[cfg(feature = "adversarial")] - adv_sync_height: None, + adv, validator_account_id, chain, runtime_adapter, @@ -401,7 +402,7 @@ impl ViewClientActor { fn get_height(&self, head: &Tip) -> BlockHeight { #[cfg(feature = "adversarial")] { - if let Some(height) = self.adv_sync_height { + if let Some(height) = self.adv.read().unwrap().adv_sync_height { return height; } } @@ -725,18 +726,18 @@ impl Handler for ViewClientActor { return match adversarial_msg { NetworkAdversarialMessage::AdvDisableDoomslug => { info!(target: "adversary", "Turning Doomslug off"); - self.adv_disable_doomslug = true; + self.adv.write().unwrap().adv_disable_doomslug = true; self.chain.adv_disable_doomslug(); NetworkViewClientResponses::NoResponse } NetworkAdversarialMessage::AdvSetSyncInfo(height) => { info!(target: "adversary", "Setting adversarial sync height: {}", height); - self.adv_sync_height = Some(height); + self.adv.write().unwrap().adv_sync_height = Some(height); NetworkViewClientResponses::NoResponse } NetworkAdversarialMessage::AdvDisableHeaderSync => { info!(target: "adversary", "Blocking header sync"); - self.adv_disable_header_sync = true; + self.adv.write().unwrap().adv_disable_header_sync = true; NetworkViewClientResponses::NoResponse } NetworkAdversarialMessage::AdvSwitchToHeight(height) => { @@ -842,7 +843,7 @@ impl Handler for ViewClientActor { NetworkViewClientMessages::BlockHeadersRequest(hashes) => { #[cfg(feature = "adversarial")] { - if self.adv_disable_header_sync { + if self.adv.read().unwrap().adv_disable_header_sync { return NetworkViewClientResponses::NoResponse; } } @@ -1004,6 +1005,7 @@ pub fn start_view_client( runtime_adapter: Arc, network_adapter: Arc, config: ClientConfig, + #[cfg(feature = "adversarial")] adv: Arc>, ) -> Addr { let request_manager = Arc::new(RwLock::new(ViewClientRequestManager::new())); SyncArbiter::start(config.view_client_threads, move || { @@ -1020,6 +1022,8 @@ pub fn start_view_client( network_adapter1, config1, request_manager1, + #[cfg(feature = "adversarial")] + adv.clone(), ) .unwrap() }) diff --git a/chain/network/tests/runner/mod.rs b/chain/network/tests/runner/mod.rs index df53daa75d5..9b778ea89d4 100644 --- a/chain/network/tests/runner/mod.rs +++ b/chain/network/tests/runner/mod.rs @@ -79,6 +79,9 @@ pub fn setup_network_node( let network_adapter = NetworkRecipient::new(); network_adapter.set_recipient(ctx.address().recipient()); let network_adapter = Arc::new(network_adapter); + #[cfg(feature = "adversarial")] + let adv = Arc::new(RwLock::new(Default::default())); + let client_actor = ClientActor::new( client_config.clone(), chain_genesis.clone(), @@ -88,6 +91,8 @@ pub fn setup_network_node( Some(signer), telemetry_actor, false, + #[cfg(feature = "adversarial")] + adv.clone(), ) .unwrap() .start(); @@ -97,6 +102,8 @@ pub fn setup_network_node( runtime.clone(), network_adapter.clone(), client_config, + #[cfg(feature = "adversarial")] + adv.clone(), ); PeerManagerActor::new( diff --git a/neard/src/lib.rs b/neard/src/lib.rs index 5277a0585af..a41980e12be 100644 --- a/neard/src/lib.rs +++ b/neard/src/lib.rs @@ -7,6 +7,8 @@ use log::{error, info}; use tracing::trace; use near_chain::ChainGenesis; +#[cfg(feature = "adversarial")] +use near_client::AdversarialControls; use near_client::{start_client, start_view_client, ClientActor, ViewClientActor}; use near_jsonrpc::start_http; use near_network::{NetworkRecipient, PeerManagerActor}; @@ -151,12 +153,17 @@ pub fn start_with_config( let node_id = config.network_config.public_key.clone().into(); let network_adapter = Arc::new(NetworkRecipient::new()); + #[cfg(feature = "adversarial")] + let adv = Arc::new(std::sync::RwLock::new(AdversarialControls::default())); + let view_client = start_view_client( config.validator_signer.as_ref().map(|signer| signer.validator_id().clone()), chain_genesis.clone(), runtime.clone(), network_adapter.clone(), config.client_config.clone(), + #[cfg(feature = "adversarial")] + adv.clone(), ); let (client_actor, client_arbiter) = start_client( config.client_config, @@ -166,6 +173,8 @@ pub fn start_with_config( network_adapter.clone(), config.validator_signer, telemetry, + #[cfg(feature = "adversarial")] + adv.clone(), ); start_http( config.rpc_config,