Skip to content

Commit

Permalink
fix(tests): Fix adversarial tests with view client
Browse files Browse the repository at this point in the history
Adversarial tests broke after running multiple view client actors,
fix them.

Fixes #3137

Test plan
---------
wrong_sync_info.py
  • Loading branch information
mikhailOK committed Aug 24, 2020
1 parent 6eb8ab5 commit 1dc2e8e
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 31 deletions.
23 changes: 10 additions & 13 deletions chain/client/src/client_actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ use crate::types::{
Error, GetNetworkInfo, NetworkInfoResponse, ShardSyncDownload, ShardSyncStatus, Status,
StatusSyncInfo, SyncStatus,
};
use crate::AdversarialControls;
use crate::StatusResponse;
#[cfg(feature = "delay_detector")]
use delay_detector::DelayDetector;
Expand All @@ -62,11 +63,7 @@ const HEAD_STALL_MULTIPLIER: u32 = 4;
pub struct ClientActor {
/// Adversarial controls
#[cfg(feature = "adversarial")]
pub adv_sync_height: Option<u64>,
#[cfg(feature = "adversarial")]
pub adv_disable_header_sync: bool,
#[cfg(feature = "adversarial")]
pub adv_disable_doomslug: bool,
pub adv: Arc<RwLock<AdversarialControls>>,

client: Client,
network_adapter: Arc<dyn NetworkAdapter>,
Expand Down Expand Up @@ -117,6 +114,7 @@ impl ClientActor {
validator_signer: Option<Arc<dyn ValidatorSigner>>,
telemetry_actor: Addr<TelemetryActor>,
enable_doomslug: bool,
#[cfg(feature = "adversarial")] adv: Arc<RwLock<AdversarialControls>>,
) -> Result<Self, Error> {
wait_until_genesis(&chain_genesis.time);
if let Some(vs) = &validator_signer {
Expand All @@ -135,11 +133,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,
Expand Down Expand Up @@ -202,14 +196,14 @@ impl Handler<NetworkClientMessages> 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) => {
Expand Down Expand Up @@ -1030,7 +1024,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;
}
}
Expand Down Expand Up @@ -1340,6 +1334,7 @@ pub fn start_client(
network_adapter: Arc<dyn NetworkAdapter>,
validator_signer: Option<Arc<dyn ValidatorSigner>>,
telemetry_actor: Addr<TelemetryActor>,
#[cfg(feature = "adversarial")] adv: Arc<RwLock<AdversarialControls>>,
) -> (Addr<ClientActor>, Arbiter) {
let client_arbiter = Arbiter::current();
let client_addr = ClientActor::start_in_arbiter(&client_arbiter, move |_ctx| {
Expand All @@ -1352,6 +1347,8 @@ pub fn start_client(
validator_signer,
telemetry_actor,
true,
#[cfg(feature = "adversarial")]
adv,
)
.unwrap()
});
Expand Down
2 changes: 1 addition & 1 deletion chain/client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub use crate::types::{
GetNextLightClientBlock, GetStateChanges, GetStateChangesInBlock, GetValidatorInfo,
GetValidatorOrdered, Query, Status, StatusResponse, SyncStatus, TxStatus, TxStatusError,
};
pub use crate::view_client::{start_view_client, ViewClientActor};
pub use crate::view_client::{start_view_client, AdversarialControls, ViewClientActor};

mod client;
mod client_actor;
Expand Down
9 changes: 9 additions & 0 deletions chain/client/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use near_store::test_utils::create_test_store;
use near_store::Store;
use near_telemetry::TelemetryActor;

use crate::AdversarialControls;
use crate::{start_view_client, Client, ClientActor, SyncStatus, ViewClientActor};
use near_network::test_utils::MockNetworkAdapter;
use near_primitives::merkle::{merklize, MerklePath};
Expand Down Expand Up @@ -108,12 +109,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(
Expand All @@ -125,6 +132,8 @@ pub fn setup(
Some(signer),
telemetry,
enable_doomslug,
#[cfg(feature = "adversarial")]
adv,
)
.unwrap();
(genesis_block, client, view_client_addr)
Expand Down
34 changes: 19 additions & 15 deletions chain/client/src/view_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,18 @@ pub struct ViewClientRequestManager {
pub receipt_outcome_requests: SizedCache<CryptoHash, Instant>,
}

/// 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<u64>,
}

/// View client provides currently committed (to the storage) view of the current chain and state.
pub struct ViewClientActor {
#[cfg(feature = "adversarial")]
pub adv: Arc<RwLock<AdversarialControls>>,

/// Validator account (if present).
validator_account_id: Option<AccountId>,
Expand Down Expand Up @@ -103,6 +107,7 @@ impl ViewClientActor {
network_adapter: Arc<dyn NetworkAdapter>,
config: ClientConfig,
request_manager: Arc<RwLock<ViewClientRequestManager>>,
#[cfg(feature = "adversarial")] adv: Arc<RwLock<AdversarialControls>>,
) -> Result<Self, Error> {
// TODO: should we create shared ChainStore that is passed to both Client and ViewClient?
let chain = Chain::new_for_view_client(
Expand All @@ -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,
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -725,18 +726,18 @@ impl Handler<NetworkViewClientMessages> 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) => {
Expand Down Expand Up @@ -842,7 +843,7 @@ impl Handler<NetworkViewClientMessages> 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;
}
}
Expand Down Expand Up @@ -1004,6 +1005,7 @@ pub fn start_view_client(
runtime_adapter: Arc<dyn RuntimeAdapter>,
network_adapter: Arc<dyn NetworkAdapter>,
config: ClientConfig,
#[cfg(feature = "adversarial")] adv: Arc<RwLock<AdversarialControls>>,
) -> Addr<ViewClientActor> {
let request_manager = Arc::new(RwLock::new(ViewClientRequestManager::new()));
SyncArbiter::start(config.view_client_threads, move || {
Expand All @@ -1020,6 +1022,8 @@ pub fn start_view_client(
network_adapter1,
config1,
request_manager1,
#[cfg(feature = "adversarial")]
adv.clone(),
)
.unwrap()
})
Expand Down
13 changes: 11 additions & 2 deletions neard/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
use std::fs;
use std::path::Path;
use std::sync::Arc;
use std::sync::{Arc, RwLock};

use actix::{Actor, Addr, Arbiter};
use log::{error, info};
use tracing::trace;

use near_chain::ChainGenesis;
use near_client::{start_client, start_view_client, ClientActor, ViewClientActor};
use near_client::{
start_client, start_view_client, AdversarialControls, ClientActor, ViewClientActor,
};
use near_jsonrpc::start_http;
use near_network::{NetworkRecipient, PeerManagerActor};
use near_store::migrations::{
Expand Down Expand Up @@ -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(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,
Expand All @@ -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,
Expand Down

0 comments on commit 1dc2e8e

Please sign in to comment.