Skip to content

Commit

Permalink
refactor: replace some references with owned structures in VMLogic (#…
Browse files Browse the repository at this point in the history
…11615)

`VMLogic` containing a lifetime makes it difficult to have it live for
any longer than a short sequence of `instantiate-link-run` and is one of
the reasons why we're forced to have some unsafe code in our linking
code.

This refactor replaces some of the reference fields with `Arc`s,
`Box`es, etc. This is not a complete refactor, I intend to do the
remainder as a follow-up.

Based on #11614
Part of #11319
  • Loading branch information
nagisa authored Jun 20, 2024
1 parent 8189888 commit 591632a
Show file tree
Hide file tree
Showing 38 changed files with 352 additions and 211 deletions.
7 changes: 6 additions & 1 deletion chain/chain/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,12 @@ impl NightshadeRuntime {
let contract_cache = compiled_contract_cache.as_deref();
let slot_sender = slot_sender.clone();
scope.spawn(move |_| {
precompile_contract(&code, &runtime_config.wasm_config, contract_cache).ok();
precompile_contract(
&code,
Arc::clone(&runtime_config.wasm_config),
contract_cache,
)
.ok();
// If this fails, it just means there won't be any more attempts to recv the
// slots
let _ = slot_sender.send(());
Expand Down
16 changes: 8 additions & 8 deletions core/parameters/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
//! Settings of the parameters of the runtime.
use super::parameter_table::InvalidConfigError;
use crate::config_store::INITIAL_TESTNET_CONFIG;
use crate::cost::RuntimeFeesConfig;
use crate::parameter_table::ParameterTable;
use near_account_id::AccountId;
use near_primitives_core::types::{Balance, Gas};
use near_primitives_core::version::PROTOCOL_VERSION;

use super::parameter_table::InvalidConfigError;
use std::sync::Arc;

// Lowered promise yield timeout length used in integration tests.
// The resharding tests for yield timeouts take too long to run otherwise.
Expand All @@ -19,12 +19,12 @@ pub struct RuntimeConfig {
///
/// This contains parameters that are required by the WASM runtime and the
/// transaction runtime.
pub fees: RuntimeFeesConfig,
pub fees: Arc<RuntimeFeesConfig>,
/// Config of wasm operations, also includes wasm gas costs.
///
/// This contains all the configuration parameters that are only required by
/// the WASM runtime.
pub wasm_config: crate::vm::Config,
pub wasm_config: Arc<crate::vm::Config>,
/// Config that defines rules for account creation.
pub account_creation_config: AccountCreationConfig,
/// The configuration for congestion control.
Expand Down Expand Up @@ -54,8 +54,8 @@ impl RuntimeConfig {
wasm_config.limit_config.yield_timeout_length_in_blocks = TEST_CONFIG_YIELD_TIMEOUT_LENGTH;

RuntimeConfig {
fees: RuntimeFeesConfig::test(),
wasm_config,
fees: Arc::new(RuntimeFeesConfig::test()),
wasm_config: Arc::new(wasm_config),
account_creation_config: AccountCreationConfig::default(),
congestion_control_config: runtime_config.congestion_control_config,
witness_config: runtime_config.witness_config,
Expand All @@ -70,8 +70,8 @@ impl RuntimeConfig {
wasm_config.make_free();

Self {
fees: RuntimeFeesConfig::free(),
wasm_config,
fees: Arc::new(RuntimeFeesConfig::free()),
wasm_config: Arc::new(wasm_config),
account_creation_config: AccountCreationConfig::default(),
congestion_control_config: runtime_config.congestion_control_config,
witness_config: runtime_config.witness_config,
Expand Down
24 changes: 17 additions & 7 deletions core/parameters/src/config_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ impl RuntimeConfigStore {
#[cfg(feature = "calimero_zero_storage")]
{
let mut initial_config = RuntimeConfig::new(&params).unwrap_or_else(|err| panic!("Failed generating `RuntimeConfig` from parameters for base parameter file. Error: {err}"));
initial_config.fees.storage_usage_config.storage_amount_per_byte = 0;
let fees = Arc::make_mut(&mut initial_config.fees);
fees.storage_usage_config.storage_amount_per_byte = 0;
store.insert(0, Arc::new(initial_config));
}

Expand All @@ -100,17 +101,26 @@ impl RuntimeConfigStore {
#[cfg(feature = "calimero_zero_storage")]
{
let mut runtime_config = RuntimeConfig::new(&params).unwrap_or_else(|err| panic!("Failed generating `RuntimeConfig` from parameters for version {protocol_version}. Error: {err}"));
runtime_config.fees.storage_usage_config.storage_amount_per_byte = 0;
let fees = Arc::make_mut(&mut runtime_config.fees);
fees.storage_usage_config.storage_amount_per_byte = 0;
store.insert(*protocol_version, Arc::new(runtime_config));
}
}

if let Some(runtime_config) = genesis_runtime_config {
let mut config = runtime_config.clone();
store.insert(0, Arc::new(config.clone()));

config.fees.storage_usage_config.storage_amount_per_byte = 10u128.pow(19);
store.insert(42, Arc::new(config));
let mut fees = crate::RuntimeFeesConfig::clone(&runtime_config.fees);
fees.storage_usage_config.storage_amount_per_byte = 10u128.pow(19);
store.insert(
42,
Arc::new(RuntimeConfig {
fees: Arc::new(fees),
wasm_config: Arc::clone(&runtime_config.wasm_config),
account_creation_config: runtime_config.account_creation_config.clone(),
congestion_control_config: runtime_config.congestion_control_config,
witness_config: runtime_config.witness_config,
}),
);
store.insert(0, Arc::new(runtime_config.clone()));
}

Self { store }
Expand Down
9 changes: 5 additions & 4 deletions core/parameters/src/parameter_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use near_primitives_core::account::id::ParseAccountError;
use near_primitives_core::types::AccountId;
use num_rational::Rational32;
use std::collections::BTreeMap;
use std::sync::Arc;

/// Represents values supported by parameter config.
#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, PartialEq)]
Expand Down Expand Up @@ -290,7 +291,7 @@ impl TryFrom<&ParameterTable> for RuntimeConfig {

fn try_from(params: &ParameterTable) -> Result<Self, Self::Error> {
Ok(RuntimeConfig {
fees: RuntimeFeesConfig {
fees: Arc::new(RuntimeFeesConfig {
action_fees: enum_map::enum_map! {
action_cost => params.get_fee(action_cost)?
},
Expand All @@ -302,8 +303,8 @@ impl TryFrom<&ParameterTable> for RuntimeConfig {
num_bytes_account: params.get(Parameter::StorageNumBytesAccount)?,
num_extra_bytes_record: params.get(Parameter::StorageNumExtraBytesRecord)?,
},
},
wasm_config: Config {
}),
wasm_config: Arc::new(Config {
ext_costs: ExtCostsConfig {
costs: enum_map::enum_map! {
cost => params.get(cost.param())?
Expand All @@ -327,7 +328,7 @@ impl TryFrom<&ParameterTable> for RuntimeConfig {
function_call_weight: params.get(Parameter::FunctionCallWeight)?,
eth_implicit_accounts: params.get(Parameter::EthImplicitAccounts)?,
yield_resume_host_functions: params.get(Parameter::YieldResume)?,
},
}),
account_creation_config: AccountCreationConfig {
min_allowed_top_level_account_length: params
.get(Parameter::MinAllowedTopLevelAccountLength)?,
Expand Down
2 changes: 1 addition & 1 deletion core/parameters/src/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ impl From<crate::RuntimeConfig> for RuntimeConfigView {
.fees
.pessimistic_gas_price_inflation_ratio,
},
wasm_config: VMConfigView::from(config.wasm_config),
wasm_config: VMConfigView::from(crate::vm::Config::clone(&config.wasm_config)),
account_creation_config: AccountCreationConfigView {
min_allowed_top_level_account_length: config
.account_creation_config
Expand Down
2 changes: 1 addition & 1 deletion genesis-tools/genesis-populate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ impl GenesisBuilder {
let mut state_update =
self.state_updates.remove(&shard_idx).expect("State updates are always available");
let protocol_config = self.runtime.get_protocol_config(&EpochId::default())?;
let storage_usage_config = protocol_config.runtime_config.fees.storage_usage_config;
let storage_usage_config = protocol_config.runtime_config.fees.storage_usage_config.clone();

// Compute storage usage and update accounts.
for (account_id, storage_usage) in compute_storage_usage(&records, &storage_usage_config) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ fn setup_runtime(sender_id: AccountId, protocol_version: ProtocolVersion) -> Tes

let mut config = RuntimeConfig::test();
// Make 1 wasm op cost ~4 GGas, to let "loop_forever" finish more quickly.
config.wasm_config.regular_op_cost = u32::MAX;
let wasm_config = Arc::make_mut(&mut config.wasm_config);
wasm_config.regular_op_cost = u32::MAX;
let runtime_configs = vec![RuntimeConfigStore::with_one_config(config)];

TestEnv::builder(&genesis.config)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use assert_matches::assert_matches;

use near_chain_configs::Genesis;
use near_client::test_utils::TestEnv;
use near_crypto::{InMemorySigner, KeyType, PublicKey, Signer};
Expand All @@ -15,6 +14,7 @@ use near_primitives::version::{ProtocolFeature, PROTOCOL_VERSION};
use near_primitives::views::{FinalExecutionStatus, QueryRequest, QueryResponseKind};
use nearcore::test_utils::TestEnvNightshadeSetupExt;
use node_runtime::ZERO_BALANCE_ACCOUNT_STORAGE_LIMIT;
use std::sync::Arc;

/// Assert that an account exists and has zero balance
fn assert_zero_balance_account(env: &TestEnv, account_id: &AccountId) {
Expand Down Expand Up @@ -123,12 +123,14 @@ fn test_zero_balance_account_add_key() {
// create free runtime config for transaction costs to make it easier to assert
// the exact amount of tokens on accounts
let mut runtime_config = RuntimeConfig::free();
runtime_config.fees.storage_usage_config = StorageUsageConfig {
let fees = Arc::make_mut(&mut runtime_config.fees);
fees.storage_usage_config = StorageUsageConfig {
storage_amount_per_byte: 10u128.pow(19),
num_bytes_account: 100,
num_extra_bytes_record: 40,
};
runtime_config.wasm_config.ext_costs = ExtCostsConfig::test();
let wasm_config = Arc::make_mut(&mut runtime_config.wasm_config);
wasm_config.ext_costs = ExtCostsConfig::test();
let runtime_config_store = RuntimeConfigStore::with_one_config(runtime_config);
let mut env = TestEnv::builder(&genesis.config)
.nightshade_runtimes_with_runtime_config_store(&genesis, vec![runtime_config_store])
Expand Down
4 changes: 3 additions & 1 deletion integration-tests/src/tests/standard_cases/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use near_crypto::SecretKey;
use near_primitives::checked_feature;
use near_primitives::state_record::StateRecord;
use near_primitives::version::PROTOCOL_VERSION;
use std::sync::Arc;
use testlib::runtime_utils::{add_test_contract, alice_account, bob_account};

fn create_runtime_node() -> RuntimeNode {
Expand All @@ -21,7 +22,8 @@ fn create_runtime_with_expensive_storage() -> RuntimeNode {
add_test_contract(&mut genesis, &bob_account());
// Set expensive state requirements and add alice more money.
let mut runtime_config = RuntimeConfig::test();
runtime_config.fees.storage_usage_config.storage_amount_per_byte = TESTING_INIT_BALANCE / 1000;
let fees = Arc::make_mut(&mut runtime_config.fees);
fees.storage_usage_config.storage_amount_per_byte = TESTING_INIT_BALANCE / 1000;
let records = genesis.force_read_records().as_mut();
match &mut records[0] {
StateRecord::Account { account, .. } => account.set_amount(TESTING_INIT_BALANCE * 10000),
Expand Down
11 changes: 5 additions & 6 deletions runtime/near-vm-runner/fuzz/fuzz_targets/diffrunner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use near_vm_runner::logic::mocks::mock_external::MockedExternal;
use near_vm_runner::logic::VMOutcome;
use near_vm_runner::ContractCode;
use near_vm_runner_fuzz::{create_context, find_entry_point, ArbitraryModule};
use std::sync::Arc;

libfuzzer_sys::fuzz_target!(|module: ArbitraryModule| {
let code = ContractCode::new(module.0.module.to_bytes(), None);
Expand All @@ -23,20 +24,18 @@ fn run_fuzz(code: &ContractCode, vm_kind: VMKind) -> VMOutcome {
context.prepaid_gas = 10u64.pow(14);
let config_store = RuntimeConfigStore::new(None);
let config = config_store.get_config(PROTOCOL_VERSION);
let fees = &config.fees;
let mut wasm_config = config.wasm_config.clone();
let fees = Arc::clone(&config.fees);
let mut wasm_config = near_parameters::vm::Config::clone(&config.wasm_config);
wasm_config.limit_config.contract_prepare_version =
near_vm_runner::logic::ContractPrepareVersion::V2;

let promise_results = vec![];

let method_name = find_entry_point(code).unwrap_or_else(|| "main".to_string());
let res = vm_kind.runtime(wasm_config).unwrap().run(
let res = vm_kind.runtime(wasm_config.into()).unwrap().run(
&method_name,
&mut fake_external,
&context,
fees,
&promise_results,
[].into(),
None,
);

Expand Down
10 changes: 4 additions & 6 deletions runtime/near-vm-runner/fuzz/fuzz_targets/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,14 @@ fn run_fuzz(code: &ContractCode, config: Arc<RuntimeConfig>) -> VMOutcome {
let mut fake_external = MockedExternal::with_code(code.clone_for_tests());
let mut context = create_context(vec![]);
context.prepaid_gas = 10u64.pow(14);
let mut wasm_config = config.wasm_config.clone();
let mut wasm_config = near_parameters::vm::Config::clone(&config.wasm_config);
wasm_config.limit_config.wasmer2_stack_limit = i32::MAX; // If we can crash wasmer2 even without the secondary stack limit it's still good to know
let vm_kind = config.wasm_config.vm_kind;
let fees = &config.fees;
let promise_results = vec![];

let fees = Arc::clone(&config.fees);
let method_name = find_entry_point(code).unwrap_or_else(|| "main".to_string());
vm_kind
.runtime(wasm_config)
.runtime(wasm_config.into())
.unwrap()
.run(&method_name, &mut fake_external, &context, fees, &promise_results, None)
.run(&method_name, &mut fake_external, &context, fees, [].into(), None)
.unwrap_or_else(|err| panic!("fatal error: {err:?}"))
}
6 changes: 3 additions & 3 deletions runtime/near-vm-runner/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,19 +475,19 @@ impl AnyCache {
/// is already in the cache, or if cache is `None`.
pub fn precompile_contract(
code: &ContractCode,
config: &Config,
config: Arc<Config>,
cache: Option<&dyn ContractRuntimeCache>,
) -> Result<Result<ContractPrecompilatonResult, CompilationError>, CacheError> {
let _span = tracing::debug_span!(target: "vm", "precompile_contract").entered();
let vm_kind = config.vm_kind;
let runtime = vm_kind
.runtime(config.clone())
.runtime(Arc::clone(&config))
.unwrap_or_else(|| panic!("the {vm_kind:?} runtime has not been enabled at compile time"));
let cache = match cache {
Some(it) => it,
None => return Ok(Ok(ContractPrecompilatonResult::CacheNotAvailable)),
};
let key = get_contract_cache_key(*code.hash(), config);
let key = get_contract_cache_key(*code.hash(), &config);
// Check if we already cached with such a key.
if cache.has(&key).map_err(CacheError::ReadError)? {
return Ok(Ok(ContractPrecompilatonResult::ContractAlreadyInCache));
Expand Down
22 changes: 12 additions & 10 deletions runtime/near-vm-runner/src/logic/logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use near_primitives_core::types::{
AccountId, Balance, Compute, EpochHeight, Gas, GasWeight, StorageUsage,
};
use std::mem::size_of;
use std::sync::Arc;
use ExtCosts::*;

pub type Result<T, E = VMLogicError> = ::std::result::Result<T, E>;
Expand All @@ -36,12 +37,12 @@ pub struct VMLogic<'a> {
/// Part of Context API and Economics API that was extracted from the receipt.
context: &'a VMContext,
/// All gas and economic parameters required during contract execution.
pub(crate) config: &'a Config,
/// Fees for creating (async) actions on runtime.
fees_config: &'a RuntimeFeesConfig,
pub(crate) config: Arc<Config>,
/// Fees charged for various operations that contract may execute.
fees_config: Arc<RuntimeFeesConfig>,
/// If this method execution is invoked directly as a callback by one or more contract calls the
/// results of the methods that made the callback are stored in this collection.
promise_results: &'a [PromiseResult],
promise_results: Arc<[PromiseResult]>,
/// Pointer to the guest memory.
memory: super::vmstate::Memory,

Expand Down Expand Up @@ -132,9 +133,9 @@ impl<'a> VMLogic<'a> {
pub fn new(
ext: &'a mut dyn External,
context: &'a VMContext,
config: &'a Config,
fees_config: &'a RuntimeFeesConfig,
promise_results: &'a [PromiseResult],
config: Arc<Config>,
fees_config: Arc<RuntimeFeesConfig>,
promise_results: Arc<[PromiseResult]>,
memory: impl MemoryLike + 'static,
) -> Self {
// Overflow should be checked before calling VMLogic.
Expand All @@ -157,6 +158,7 @@ impl<'a> VMLogic<'a> {
ext.get_recorded_storage_size(),
config.limit_config.per_receipt_storage_proof_size_limit,
);
let remaining_stack = u64::from(config.limit_config.max_stack_height);
Self {
ext,
context,
Expand All @@ -174,7 +176,7 @@ impl<'a> VMLogic<'a> {
registers: Default::default(),
promises: vec![],
total_log_length: 0,
remaining_stack: u64::from(config.limit_config.max_stack_height),
remaining_stack,
}
}

Expand Down Expand Up @@ -1798,14 +1800,14 @@ impl<'a> VMLogic<'a> {
let (receipt_idx, sir) = self.promise_idx_to_receipt_idx_with_sir(promise_idx)?;
let receiver_id = self.ext.get_receipt_receiver(receipt_idx);
let send_fee = transfer_send_fee(
self.fees_config,
&self.fees_config,
sir,
self.config.implicit_account_creation,
self.config.eth_implicit_accounts,
receiver_id.get_account_type(),
);
let exec_fee = transfer_exec_fee(
self.fees_config,
&self.fees_config,
self.config.implicit_account_creation,
self.config.eth_implicit_accounts,
receiver_id.get_account_type(),
Expand Down
4 changes: 2 additions & 2 deletions runtime/near-vm-runner/src/logic/tests/promises.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ fn vm_receipts<'a>(ext: &'a MockedExternal) -> Vec<impl serde::Serialize + 'a> {

#[test]
fn test_promise_results() {
let promise_results = vec![
let promise_results = [
PromiseResult::Successful(b"test".to_vec()),
PromiseResult::Failed,
PromiseResult::NotReady,
];

let mut logic_builder = VMLogicBuilder::default();
logic_builder.promise_results = promise_results;
logic_builder.promise_results = promise_results.into();
let mut logic = logic_builder.build();

assert_eq!(logic.promise_results_count(), Ok(3), "Total count of registers must be 3");
Expand Down
Loading

0 comments on commit 591632a

Please sign in to comment.