Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: improve compilation time for neard tests #4263

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion core/primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ chrono = { version = "0.4.4", features = ["serde"] }
derive_more = "0.99.3"
easy-ext = "0.2"
sha2 = "0.9"
lazy_static = "1.4"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
smart-default = "0.6"
validator = "0.12"
once_cell = "1"
rand = "0.7"
reed-solomon-erasure = "4"
jemallocator = { version = "0.3", optional = true }
Expand Down
63 changes: 36 additions & 27 deletions core/primitives/src/runtime/config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//! Settings of the parameters of the runtime.
use once_cell::sync::OnceCell;
use serde::{Deserialize, Serialize};

use crate::checked_feature;
Expand All @@ -7,7 +8,7 @@ use crate::runtime::fees::RuntimeFeesConfig;
use crate::serialize::u128_dec_format;
use crate::types::{AccountId, Balance};
use crate::version::ProtocolVersion;
use std::sync::{Arc, Mutex};
use std::sync::Arc;

/// The structure that holds the parameters of the runtime, mostly economics.
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
Expand All @@ -26,6 +27,34 @@ pub struct RuntimeConfig {
pub account_creation_config: AccountCreationConfig,
}

/// We start with a specific runtime config at the genesis, but we might want to
/// do slight adjustments to it, depending on the protocol version.
///
/// This struct takes care of returning a config appropriate for particular
/// protocol version.
pub struct CurrentRuntimeConfig {
genesis_runtime_config: Arc<RuntimeConfig>,
with_lower_storage_cost: OnceCell<Arc<RuntimeConfig>>,
}

impl CurrentRuntimeConfig {
pub fn new(genesis_runtime_config: RuntimeConfig) -> Self {
Self {
genesis_runtime_config: Arc::new(genesis_runtime_config),
with_lower_storage_cost: OnceCell::new(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively just go ahead and construct the alternative configuration. It would increase memory usage if the protocol feature is not used but simplify things (and minimally speed things up) for the case of the feature being enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's an extremely reasonable suggestion, to say the least! There's really no need for any kind of laziness here.

}
}

pub fn for_protocol_version(&self, protocol_version: ProtocolVersion) -> &Arc<RuntimeConfig> {
if checked_feature!("stable", LowerStorageCost, protocol_version) {
self.with_lower_storage_cost
.get_or_init(|| Arc::new(self.genesis_runtime_config.decrease_storage_cost()))
} else {
&self.genesis_runtime_config
}
}
}

impl Default for RuntimeConfig {
fn default() -> Self {
RuntimeConfig {
Expand All @@ -38,10 +67,6 @@ impl Default for RuntimeConfig {
}
}

lazy_static::lazy_static! {
static ref LOWER_STORAGE_COST_CONFIG: Mutex<Option<Arc<RuntimeConfig>>> = Mutex::new(None);
}

impl RuntimeConfig {
pub fn free() -> Self {
Self {
Expand All @@ -52,23 +77,6 @@ impl RuntimeConfig {
}
}

/// Returns a `RuntimeConfig` for the corresponding protocol version.
/// It uses `genesis_runtime_config` to identify the original
/// config and `protocol_version` for the current protocol version.
pub fn from_protocol_version(
genesis_runtime_config: &Arc<RuntimeConfig>,
protocol_version: ProtocolVersion,
) -> Arc<Self> {
if checked_feature!("stable", LowerStorageCost, protocol_version) {
let mut config = LOWER_STORAGE_COST_CONFIG.lock().unwrap();
config
.get_or_insert_with(|| Arc::new(genesis_runtime_config.decrease_storage_cost()))
.clone()
} else {
genesis_runtime_config.clone()
}
}

/// Returns a new config with decreased storage cost.
fn decrease_storage_cost(&self) -> Self {
let mut config = self.clone();
Expand Down Expand Up @@ -113,15 +121,16 @@ mod tests {

#[test]
fn test_lower_cost() {
let config = Arc::new(RuntimeConfig::default());
let config_same = RuntimeConfig::from_protocol_version(&config, 0);
let genesis_config = RuntimeConfig::default();
let current_config = CurrentRuntimeConfig::new(genesis_config.clone());
let config_same = current_config.for_protocol_version(0);
assert_eq!(
config_same.as_ref().storage_amount_per_byte,
config.as_ref().storage_amount_per_byte
genesis_config.storage_amount_per_byte
);
let config_lower = RuntimeConfig::from_protocol_version(&config, ProtocolVersion::MAX);
let config_lower = current_config.for_protocol_version(ProtocolVersion::MAX);
assert!(
config_lower.as_ref().storage_amount_per_byte < config.as_ref().storage_amount_per_byte
config_lower.as_ref().storage_amount_per_byte < genesis_config.storage_amount_per_byte
);
}
}
3 changes: 3 additions & 0 deletions neard/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ mod migrations;
mod runtime;
mod shard_tracker;

#[cfg(test)]
mod tests;

const STORE_PATH: &str = "data";

pub fn store_path_exists<P: AsRef<Path>>(path: P) -> bool {
Expand Down
34 changes: 15 additions & 19 deletions neard/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use near_primitives::epoch_manager::EpochConfig;
use near_primitives::errors::{EpochError, InvalidTxError, RuntimeError};
use near_primitives::hash::{hash, CryptoHash};
use near_primitives::receipt::Receipt;
use near_primitives::runtime::config::CurrentRuntimeConfig;
use near_primitives::sharding::ChunkHash;
use near_primitives::state_record::StateRecord;
use near_primitives::transaction::SignedTransaction;
Expand All @@ -56,7 +57,6 @@ use node_runtime::{
};

use crate::shard_tracker::{account_id_to_shard_id, ShardTracker};
use near_primitives::runtime::config::RuntimeConfig;

use errors::FromStateViewerErrors;

Expand Down Expand Up @@ -121,7 +121,7 @@ impl EpochInfoProvider for SafeEpochManager {
/// TODO: this possibly should be merged with the runtime cargo or at least reconciled on the interfaces.
pub struct NightshadeRuntime {
genesis_config: GenesisConfig,
genesis_runtime_config: Arc<RuntimeConfig>,
current_runtime_config: CurrentRuntimeConfig,

store: Arc<Store>,
tries: ShardTries,
Expand All @@ -144,7 +144,8 @@ impl NightshadeRuntime {
let runtime = Runtime::new();
let trie_viewer = TrieViewer::new_with_state_size_limit(trie_viewer_state_size_limit);
let genesis_config = genesis.config.clone();
let genesis_runtime_config = Arc::new(genesis_config.runtime_config.clone());
let current_runtime_config =
CurrentRuntimeConfig::new(genesis_config.runtime_config.clone());
let num_shards = genesis.config.num_block_producer_seats_per_shard.len() as NumShards;
let initial_epoch_config = EpochConfig::from(&genesis_config);
let reward_calculator = RewardCalculator::new(&genesis_config);
Expand Down Expand Up @@ -173,7 +174,7 @@ impl NightshadeRuntime {
);
NightshadeRuntime {
genesis_config,
genesis_runtime_config,
current_runtime_config,
store,
tries,
runtime,
Expand Down Expand Up @@ -408,10 +409,10 @@ impl NightshadeRuntime {
gas_limit: Some(gas_limit),
random_seed,
current_protocol_version,
config: RuntimeConfig::from_protocol_version(
&self.genesis_runtime_config,
current_protocol_version,
),
config: self
.current_runtime_config
.for_protocol_version(current_protocol_version)
.clone(),
cache: Some(Arc::new(StoreCompiledContractCache { store: self.store.clone() })),
is_new_chunk,
#[cfg(feature = "protocol_feature_evm")]
Expand Down Expand Up @@ -544,10 +545,8 @@ impl RuntimeAdapter for NightshadeRuntime {
verify_signature: bool,
current_protocol_version: ProtocolVersion,
) -> Result<Option<InvalidTxError>, Error> {
let runtime_config = RuntimeConfig::from_protocol_version(
&self.genesis_runtime_config,
current_protocol_version,
);
let runtime_config =
self.current_runtime_config.for_protocol_version(current_protocol_version);

if let Some(state_root) = state_root {
let shard_id = self.account_id_to_shard_id(&transaction.transaction.signer_id);
Expand Down Expand Up @@ -616,10 +615,8 @@ impl RuntimeAdapter for NightshadeRuntime {
let mut transactions = vec![];
let mut num_checked_transactions = 0;

let runtime_config = RuntimeConfig::from_protocol_version(
&self.genesis_runtime_config,
current_protocol_version,
);
let runtime_config =
self.current_runtime_config.for_protocol_version(current_protocol_version);

while total_gas_burnt < transactions_gas_limit {
if let Some(iter) = pool_iterator.next() {
Expand Down Expand Up @@ -1491,9 +1488,8 @@ impl RuntimeAdapter for NightshadeRuntime {
let mut config = self.genesis_config.clone();
config.protocol_version = protocol_version;
// Currently only runtime config is changed through protocol upgrades.
let runtime_config =
RuntimeConfig::from_protocol_version(&self.genesis_runtime_config, protocol_version);
config.runtime_config = (*runtime_config).clone();
let runtime_config = self.current_runtime_config.for_protocol_version(protocol_version);
config.runtime_config = (**runtime_config).clone();
Ok(config)
}
}
Expand Down
9 changes: 9 additions & 0 deletions neard/tests/node_cluster.rs → neard/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
mod economics;
mod load_genesis;
mod rpc_nodes;
mod run_nodes;
mod stake_nodes;
mod sync_nodes;
mod sync_state_nodes;
mod track_shards;

use futures::future;

use near_actix_test_utils::{run_actix_until_stop, spawn_interruptible};
Expand Down
4 changes: 2 additions & 2 deletions neard/tests/economics.rs → neard/src/tests/economics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ use std::sync::Arc;

use num_rational::Rational;

use crate::config::GenesisExt;
use near_chain::{ChainGenesis, RuntimeAdapter};
use near_chain_configs::Genesis;
use near_client::test_utils::TestEnv;
use near_crypto::{InMemorySigner, KeyType};
use near_logger_utils::init_integration_logger;
use near_primitives::transaction::SignedTransaction;
use near_store::test_utils::create_test_store;
use neard::config::GenesisExt;
use testlib::fees_utils::FeeHelper;

use primitive_types::U256;
Expand All @@ -25,7 +25,7 @@ fn setup_env(f: &mut dyn FnMut(&mut Genesis) -> ()) -> (TestEnv, FeeHelper) {
genesis.config.runtime_config.transaction_costs.clone(),
genesis.config.min_gas_price,
);
let runtimes: Vec<Arc<dyn RuntimeAdapter>> = vec![Arc::new(neard::NightshadeRuntime::new(
let runtimes: Vec<Arc<dyn RuntimeAdapter>> = vec![Arc::new(crate::NightshadeRuntime::new(
Path::new("."),
store1,
&genesis,
Expand Down
File renamed without changes.
5 changes: 2 additions & 3 deletions neard/tests/rpc_nodes.rs → neard/src/tests/rpc_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ use near_primitives::types::{BlockId, BlockReference, Finality, TransactionOrRec
use near_primitives::views::{
ExecutionOutcomeView, ExecutionStatusView, FinalExecutionOutcomeViewEnum, FinalExecutionStatus,
};
use neard::config::TESTING_INIT_BALANCE;
use testlib::genesis_block;

mod node_cluster;
use node_cluster::NodeCluster;
use crate::config::TESTING_INIT_BALANCE;
use crate::tests::NodeCluster;

macro_rules! panic_on_rpc_error {
($e:expr) => {
Expand Down
3 changes: 1 addition & 2 deletions neard/tests/run_nodes.rs → neard/src/tests/run_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ use near_network::test_utils::WaitOrTimeout;
use near_primitives::types::{BlockHeightDelta, NumSeats, NumShards};
use rand::{thread_rng, Rng};

mod node_cluster;
use node_cluster::NodeCluster;
use crate::tests::NodeCluster;

fn run_heavy_nodes(
num_shards: NumShards,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@ use near_primitives::hash::CryptoHash;
use near_primitives::transaction::SignedTransaction;
use near_primitives::types::{AccountId, BlockHeightDelta, BlockReference, NumSeats};
use near_primitives::views::{QueryRequest, QueryResponseKind, ValidatorInfo};
use neard::config::{GenesisExt, TESTING_INIT_BALANCE, TESTING_INIT_STAKE};
use neard::{load_test_config, start_with_config, NearConfig, NEAR_BASE};
use testlib::{genesis_hash, test_helpers::heavy_test};

use {near_primitives::types::BlockId, primitive_types::U256};

use crate::config::{GenesisExt, TESTING_INIT_BALANCE, TESTING_INIT_STAKE};
use crate::{load_test_config, start_with_config, NearConfig, NEAR_BASE};

#[derive(Clone)]
struct TestNode {
account_id: AccountId,
Expand Down
5 changes: 3 additions & 2 deletions neard/tests/sync_nodes.rs → neard/src/tests/sync_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ use near_primitives::types::validator_stake::ValidatorStake;
use near_primitives::types::{BlockHeightDelta, EpochId};
use near_primitives::validator_signer::{InMemoryValidatorSigner, ValidatorSigner};
use near_primitives::version::PROTOCOL_VERSION;
use neard::config::{GenesisExt, TESTING_INIT_STAKE};
use neard::{load_test_config, start_with_config, NearConfig};
use testlib::{genesis_block, test_helpers::heavy_test};

use crate::config::{GenesisExt, TESTING_INIT_STAKE};
use crate::{load_test_config, start_with_config, NearConfig};

// This assumes that there is no height skipped. Otherwise epoch hash calculation will be wrong.
fn add_blocks(
mut blocks: Vec<Block>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ use near_chain_configs::Genesis;
use near_client::GetBlock;
use near_logger_utils::init_integration_logger;
use near_network::test_utils::{convert_boot_nodes, open_port, WaitOrTimeout};
use neard::{config::GenesisExt, load_test_config, start_with_config};
use testlib::test_helpers::heavy_test;

use crate::{config::GenesisExt, load_test_config, start_with_config};

/// One client is in front, another must sync to it using state (fast) sync.
#[test]
fn sync_state_nodes() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ use near_logger_utils::init_integration_logger;
use near_network::test_utils::WaitOrTimeout;
use near_primitives::hash::CryptoHash;

mod node_cluster;
use node_cluster::NodeCluster;
use crate::tests::NodeCluster;

#[test]
fn track_shards() {
Expand Down