From e9e334aeccb6b15c363b3f3c193384c1d2c3fc12 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Mon, 9 May 2022 16:41:46 +0200 Subject: [PATCH 01/16] feat: Precharge fn loading gas cost The code prepartion steps for executing a smart contract function call used to be charged after the work has been done. This protocol change reorders this. The effect of the procotocol change is that when a function call fails during contract loading, it will now have a non-zero gas cost. Incidentally, this change also brings wasmtime failure behavior during loading in-line with wasmer0 and wasmer2. Test plan --------- Tests for this feature must ensure several things. 1. The change does not affect the behavior with disabled feature. 2. Enabled feature has the desired effect of charging gas for loading code even when contract loading fails on new versions. 3. Future changes do not accidentally make old versions incompatible. Point 1 is mostly covered by existing tests that already check that gas is zero during failures of all kinds. These tests on the stable version are unchanged but the nightly_protocol_feature version checks that the expected amount of gas is charged. The test `test_gas_exceed_loading`has been added to also check the scenario where gas runs out during contract loading. Upon feature stabilization, these tests will only check the new behaviour, therefore they cannot cover point 3. Point 2 and 3 are covered with new tests in the module `fix_contract_loading_cost_protocol_upgrade` which compare the results of different control flows in old/new version. --- core/primitives/Cargo.toml | 2 + core/primitives/src/version.rs | 7 +- nearcore/Cargo.toml | 4 + runtime/near-vm-logic/src/context.rs | 15 + runtime/near-vm-logic/src/logic.rs | 15 +- .../src/tests/vm_logic_builder.rs | 2 + .../near-vm-runner-standalone/src/script.rs | 4 +- runtime/near-vm-runner/Cargo.toml | 4 + .../fuzz/fuzz_targets/runner.rs | 2 + runtime/near-vm-runner/src/preload.rs | 4 + runtime/near-vm-runner/src/runner.rs | 88 ++++- runtime/near-vm-runner/src/tests.rs | 97 ++--- runtime/near-vm-runner/src/tests/cache.rs | 2 + .../src/tests/compile_errors.rs | 3 +- .../src/tests/contract_preload.rs | 7 +- .../near-vm-runner/src/tests/rs_contract.rs | 30 +- .../src/tests/runtime_errors.rs | 342 +++++++++++++++++- .../near-vm-runner/src/tests/ts_contract.rs | 8 +- runtime/near-vm-runner/src/wasmer2_runner.rs | 58 +-- runtime/near-vm-runner/src/wasmer_runner.rs | 60 ++- runtime/near-vm-runner/src/wasmtime_runner.rs | 91 +++-- .../src/function_call.rs | 3 + .../src/gas_metering.rs | 5 + runtime/runtime-params-estimator/src/lib.rs | 2 + runtime/runtime-params-estimator/src/utils.rs | 14 +- 25 files changed, 711 insertions(+), 158 deletions(-) diff --git a/core/primitives/Cargo.toml b/core/primitives/Cargo.toml index cdf0e0d4cbd..61c42a368ea 100644 --- a/core/primitives/Cargo.toml +++ b/core/primitives/Cargo.toml @@ -47,6 +47,7 @@ protocol_feature_chunk_only_producers = [] protocol_feature_routing_exchange_algorithm = ["near-primitives-core/protocol_feature_routing_exchange_algorithm"] protocol_feature_access_key_nonce_for_implicit_accounts = [] protocol_feature_fix_staking_threshold = [] +protocol_feature_fix_contract_loading_cost = [] nightly_protocol_features = [ "nightly_protocol", "protocol_feature_alt_bn128", @@ -54,6 +55,7 @@ nightly_protocol_features = [ "protocol_feature_routing_exchange_algorithm", "protocol_feature_access_key_nonce_for_implicit_accounts", "protocol_feature_fix_staking_threshold", + "protocol_feature_fix_contract_loading_cost", ] nightly_protocol = [] deepsize_feature = [ diff --git a/core/primitives/src/version.rs b/core/primitives/src/version.rs index be016206892..981d88460e5 100644 --- a/core/primitives/src/version.rs +++ b/core/primitives/src/version.rs @@ -162,6 +162,9 @@ pub enum ProtocolFeature { /// alpha is min stake ratio #[cfg(feature = "protocol_feature_fix_staking_threshold")] FixStakingThreshold, + /// Charge for contract loading before it happens. + #[cfg(feature = "protocol_feature_fix_contract_loading_cost")] + FixContractLoadingCost, } /// Both, outgoing and incoming tcp connections to peers, will be rejected if `peer's` @@ -178,7 +181,7 @@ const STABLE_PROTOCOL_VERSION: ProtocolVersion = 53; pub const PROTOCOL_VERSION: ProtocolVersion = STABLE_PROTOCOL_VERSION; /// Current latest nightly version of the protocol. #[cfg(feature = "nightly_protocol")] -pub const PROTOCOL_VERSION: ProtocolVersion = 128; +pub const PROTOCOL_VERSION: ProtocolVersion = 129; /// The points in time after which the voting for the protocol version should start. #[allow(dead_code)] @@ -243,6 +246,8 @@ impl ProtocolFeature { ProtocolFeature::RoutingExchangeAlgorithm => 117, #[cfg(feature = "protocol_feature_fix_staking_threshold")] ProtocolFeature::FixStakingThreshold => 126, + #[cfg(feature = "protocol_feature_fix_contract_loading_cost")] + ProtocolFeature::FixContractLoadingCost => 129, } } } diff --git a/nearcore/Cargo.toml b/nearcore/Cargo.toml index 4879bb290e4..2505336654c 100644 --- a/nearcore/Cargo.toml +++ b/nearcore/Cargo.toml @@ -133,6 +133,9 @@ protocol_feature_fix_staking_threshold = [ "near-primitives/protocol_feature_fix_staking_threshold", "near-epoch-manager/protocol_feature_fix_staking_threshold", ] +protocol_feature_fix_contract_loading_cost = [ + "near-vm-runner/protocol_feature_fix_contract_loading_cost", +] nightly_protocol_features = [ "nightly_protocol", "near-primitives/nightly_protocol_features", @@ -144,6 +147,7 @@ nightly_protocol_features = [ "protocol_feature_routing_exchange_algorithm", "protocol_feature_access_key_nonce_for_implicit_accounts", "protocol_feature_fix_staking_threshold", + "protocol_feature_fix_contract_loading_cost", ] nightly_protocol = [ "near-primitives/nightly_protocol", diff --git a/runtime/near-vm-logic/src/context.rs b/runtime/near-vm-logic/src/context.rs index cff4a0a6bb7..b2119791c59 100644 --- a/runtime/near-vm-logic/src/context.rs +++ b/runtime/near-vm-logic/src/context.rs @@ -1,4 +1,6 @@ +use crate::gas_counter::GasCounter; use crate::types::PublicKey; +use near_primitives::config::VMConfig; use near_primitives_core::config::ViewConfig; use near_primitives_core::serialize::u64_dec_format; use near_primitives_core::types::{ @@ -67,4 +69,17 @@ impl VMContext { pub fn is_view(&self) -> bool { self.view_config.is_some() } + pub fn new_gas_counter(&self, wasm_config: &VMConfig) -> GasCounter { + let max_gas_burnt = match self.view_config { + Some(ViewConfig { max_gas_burnt: max_gas_burnt_view }) => max_gas_burnt_view, + None => wasm_config.limit_config.max_gas_burnt, + }; + GasCounter::new( + wasm_config.ext_costs.clone(), + max_gas_burnt, + wasm_config.regular_op_cost, + self.prepaid_gas, + self.is_view(), + ) + } } diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index 7dffc9e003c..b668de0aa67 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -9,7 +9,7 @@ use byteorder::ByteOrder; use near_crypto::Secp256K1Signature; use near_primitives::version::is_implicit_account_creation_enabled; use near_primitives_core::config::ExtCosts::*; -use near_primitives_core::config::{ActionCosts, ExtCosts, VMConfig, ViewConfig}; +use near_primitives_core::config::{ActionCosts, ExtCosts, VMConfig}; use near_primitives_core::profile::ProfileData; use near_primitives_core::runtime::fees::{ transfer_exec_fee, transfer_send_fee, RuntimeFeesConfig, @@ -106,6 +106,7 @@ impl<'a> VMLogic<'a> { context: VMContext, config: &'a VMConfig, fees_config: &'a RuntimeFeesConfig, + gas_counter: GasCounter, promise_results: &'a [PromiseResult], memory: &'a mut dyn MemoryLike, current_protocol_version: ProtocolVersion, @@ -113,19 +114,8 @@ impl<'a> VMLogic<'a> { // Overflow should be checked before calling VMLogic. let current_account_balance = context.account_balance + context.attached_deposit; let current_storage_usage = context.storage_usage; - let max_gas_burnt = match context.view_config { - Some(ViewConfig { max_gas_burnt: max_gas_burnt_view }) => max_gas_burnt_view, - None => config.limit_config.max_gas_burnt, - }; let current_account_locked_balance = context.account_locked_balance; - let gas_counter = GasCounter::new( - config.ext_costs.clone(), - max_gas_burnt, - config.regular_op_cost, - context.prepaid_gas, - context.is_view(), - ); Self { ext, context, @@ -2663,6 +2653,7 @@ impl<'a> VMLogic<'a> { } } + #[cfg(not(feature = "protocol_feature_fix_contract_loading_cost"))] pub fn add_contract_loading_fee(&mut self, code_len: u64) -> Result<()> { self.gas_counter.pay_per(contract_loading_bytes, code_len)?; self.gas_counter.pay_base(contract_loading_base) diff --git a/runtime/near-vm-logic/src/tests/vm_logic_builder.rs b/runtime/near-vm-logic/src/tests/vm_logic_builder.rs index c4715c26df1..30d6503a72b 100644 --- a/runtime/near-vm-logic/src/tests/vm_logic_builder.rs +++ b/runtime/near-vm-logic/src/tests/vm_logic_builder.rs @@ -32,11 +32,13 @@ impl Default for VMLogicBuilder { impl VMLogicBuilder { pub fn build(&mut self, context: VMContext) -> VMLogic<'_> { + let gas_counter = context.new_gas_counter(&self.config); VMLogic::new_with_protocol_version( &mut self.ext, context, &self.config, &self.fees_config, + gas_counter, &self.promise_results, &mut self.memory, self.current_protocol_version, diff --git a/runtime/near-vm-runner-standalone/src/script.rs b/runtime/near-vm-runner-standalone/src/script.rs index 1185a82dd10..e311de3c31a 100644 --- a/runtime/near-vm-runner-standalone/src/script.rs +++ b/runtime/near-vm-runner-standalone/src/script.rs @@ -117,7 +117,8 @@ impl Script { } let config_store = RuntimeConfigStore::new(None); - let runtime_fees_config = &config_store.get_config(self.protocol_version).transaction_costs; + let config = &config_store.get_config(self.protocol_version); + let runtime_fees_config = &config.transaction_costs; let mut outcomes = Vec::new(); if let Some(runtime) = self.vm_kind.runtime(self.vm_config.clone()) { for step in &self.steps { @@ -128,6 +129,7 @@ impl Script { &mut external, step.vm_context.clone(), runtime_fees_config, + step.vm_context.new_gas_counter(&config.wasm_config), &step.promise_results, self.protocol_version, self.contract_cache.as_deref(), diff --git a/runtime/near-vm-runner/Cargo.toml b/runtime/near-vm-runner/Cargo.toml index 48c0f2d0c8f..4418b7e8598 100644 --- a/runtime/near-vm-runner/Cargo.toml +++ b/runtime/near-vm-runner/Cargo.toml @@ -97,10 +97,14 @@ protocol_feature_alt_bn128 = [ "near-primitives/protocol_feature_alt_bn128", "near-vm-errors/protocol_feature_alt_bn128" ] +protocol_feature_fix_contract_loading_cost = [ + "near-primitives/protocol_feature_fix_contract_loading_cost", +] nightly_protocol_features = [ "near-primitives/nightly_protocol_features", "protocol_feature_alt_bn128", + "protocol_feature_fix_contract_loading_cost", ] sandbox = ["near-vm-logic/sandbox"] diff --git a/runtime/near-vm-runner/fuzz/fuzz_targets/runner.rs b/runtime/near-vm-runner/fuzz/fuzz_targets/runner.rs index 4920e2f9c0f..547c14a62b9 100644 --- a/runtime/near-vm-runner/fuzz/fuzz_targets/runner.rs +++ b/runtime/near-vm-runner/fuzz/fuzz_targets/runner.rs @@ -22,6 +22,7 @@ fn run_fuzz(code: &ContractCode, vm_kind: VMKind) -> VMResult { context.prepaid_gas = 10u64.pow(14); let config = VMConfig::test(); let fees = RuntimeFeesConfig::test(); + let gas_counter = context.new_gas_counter(&config); let promise_results = vec![]; @@ -32,6 +33,7 @@ fn run_fuzz(code: &ContractCode, vm_kind: VMKind) -> VMResult { &mut fake_external, context, &fees, + gas_counter, &promise_results, PROTOCOL_VERSION, None, diff --git a/runtime/near-vm-runner/src/preload.rs b/runtime/near-vm-runner/src/preload.rs index 7970cebab9a..5c789acf0bd 100644 --- a/runtime/near-vm-runner/src/preload.rs +++ b/runtime/near-vm-runner/src/preload.rs @@ -2,6 +2,7 @@ use std::sync::mpsc::{channel, Receiver, Sender}; use std::sync::Arc; use near_primitives::contract::ContractCode; +use near_vm_logic::gas_counter::GasCounter; use threadpool::ThreadPool; use near_primitives::runtime::fees::RuntimeFeesConfig; @@ -124,6 +125,7 @@ impl ContractCaller { ext: &mut dyn External, context: VMContext, fees_config: &'a RuntimeFeesConfig, + gas_counter: GasCounter, promise_results: &'a [PromiseResult], current_protocol_version: ProtocolVersion, ) -> VMResult { @@ -153,6 +155,7 @@ impl ContractCaller { context, &self.vm_config, fees_config, + gas_counter, promise_results, current_protocol_version, ) @@ -177,6 +180,7 @@ impl ContractCaller { ext, context, fees_config, + gas_counter, promise_results, current_protocol_version, ) diff --git a/runtime/near-vm-runner/src/runner.rs b/runtime/near-vm-runner/src/runner.rs index 95b8220d185..808a0775be8 100644 --- a/runtime/near-vm-runner/src/runner.rs +++ b/runtime/near-vm-runner/src/runner.rs @@ -1,3 +1,4 @@ +use near_primitives::checked_feature; use near_primitives::config::VMConfig; use near_primitives::contract::ContractCode; use near_primitives::hash::CryptoHash; @@ -5,9 +6,10 @@ use near_primitives::profile::ProfileData; use near_primitives::runtime::fees::RuntimeFeesConfig; use near_primitives::types::CompiledContractCache; use near_primitives::version::ProtocolVersion; -use near_vm_errors::VMError; +use near_vm_errors::{FunctionCallError, MethodResolveError, VMError}; +use near_vm_logic::gas_counter::GasCounter; use near_vm_logic::types::PromiseResult; -use near_vm_logic::{External, ReturnData, VMContext, VMLogic, VMOutcome}; +use near_vm_logic::{ExtCosts, External, ReturnData, VMContext, VMLogic, VMOutcome}; use crate::vm_kind::VMKind; @@ -36,6 +38,62 @@ pub fn run( cache: Option<&dyn CompiledContractCache>, ) -> VMResult { let vm_kind = VMKind::for_protocol_version(current_protocol_version); + + run_with_vm_kind( + code, + method_name, + ext, + context, + wasm_config, + fees_config, + promise_results, + current_protocol_version, + cache, + vm_kind, + ) +} + +/// Usually the VM is defined by the protocol version. For test and benchmarks, +/// however, it is useful to have an entry point that can specify protocol +/// version and VM kind independently. +pub(crate) fn run_with_vm_kind( + code: &ContractCode, + method_name: &str, + ext: &mut dyn External, + context: VMContext, + wasm_config: &VMConfig, + fees_config: &RuntimeFeesConfig, + promise_results: &[PromiseResult], + current_protocol_version: ProtocolVersion, + cache: Option<&dyn CompiledContractCache>, + vm_kind: VMKind, +) -> VMResult { + if method_name.is_empty() { + let error = VMError::FunctionCallError(FunctionCallError::MethodResolveError( + MethodResolveError::MethodEmptyName, + )); + return VMResult::nop_outcome(error); + } + + let mut gas_counter = context.new_gas_counter(wasm_config); + if checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + let code_bytes = code.code().len() as u64; + + if gas_counter.pay_per(ExtCosts::contract_loading_bytes, code_bytes).is_err() + || gas_counter.pay_base(ExtCosts::contract_loading_base).is_err() + { + let error = VMError::FunctionCallError(FunctionCallError::HostError( + near_vm_errors::HostError::GasExceeded, + )); + + return VMResult::not_run_with_gas_counter(gas_counter, error); + } + } + if let Some(runtime) = vm_kind.runtime(wasm_config.clone()) { runtime.run( code, @@ -43,6 +101,7 @@ pub fn run( ext, context, fees_config, + gas_counter, promise_results, current_protocol_version, cache, @@ -62,7 +121,7 @@ pub trait VM { /// /// [`VMContext::input`] will be passed to the contract entrypoint as an argument. /// - /// The gas cost for contract preparation will be subtracted by the VM implementation. + /// XXX The gas cost for contract preparation will be subtracted by the VM implementation. fn run( &self, code: &ContractCode, @@ -70,6 +129,7 @@ pub trait VM { ext: &mut dyn External, context: VMContext, fees_config: &RuntimeFeesConfig, + gas_counter: GasCounter, promise_results: &[PromiseResult], current_protocol_version: ProtocolVersion, cache: Option<&dyn CompiledContractCache>, @@ -153,6 +213,28 @@ impl VMResult { VMResult::Aborted(outcome, error) } + /// Creates an outcome with a no-op outcome other than spending some gas. + pub fn not_run_with_gas_counter(gas_counter: GasCounter, error: VMError) -> VMResult { + let burnt_gas = gas_counter.burnt_gas(); + let used_gas = gas_counter.used_gas(); + + let mut profile = gas_counter.profile_data(); + profile.compute_wasm_instruction_cost(burnt_gas); + + let outcome = VMOutcome { + // Note: Balance and storage fields are ignored on a failed outcome. + balance: 0, + storage_usage: 0, + return_data: ReturnData::None, + burnt_gas, + used_gas, + logs: Vec::new(), + profile, + action_receipts: Vec::new(), + }; + VMResult::Aborted(outcome, error) + } + /// Borrow the internal outcome, if there is one. pub fn outcome(&self) -> &VMOutcome { match self { diff --git a/runtime/near-vm-runner/src/tests.rs b/runtime/near-vm-runner/src/tests.rs index 50662df8bce..68edcd53e7a 100644 --- a/runtime/near-vm-runner/src/tests.rs +++ b/runtime/near-vm-runner/src/tests.rs @@ -8,6 +8,7 @@ mod wasm_validation; use crate::vm_kind::VMKind; +use near_primitives::checked_feature; use near_primitives::contract::ContractCode; use near_primitives::runtime::config_store::RuntimeConfigStore; use near_primitives::runtime::fees::RuntimeFeesConfig; @@ -55,34 +56,42 @@ fn create_context(input: Vec) -> VMContext { } } -fn make_simple_contract_call_with_gas_vm( +/// Exhaustive parameters for making a simple contract call. +/// Calling with no protocol version specified will use a test configuration. +fn make_simple_contract_call_ex( code: &[u8], method_name: &str, - prepaid_gas: u64, vm_kind: VMKind, + protocol_version: Option, + prepaid_gas: u64, ) -> (VMOutcome, Option) { let mut fake_external = MockedExternal::new(); let mut context = create_context(vec![]); context.prepaid_gas = prepaid_gas; - let config = VMConfig::test(); - let fees = RuntimeFeesConfig::test(); + let (config, fees) = if let Some(protocol_version) = protocol_version { + let runtime_config_store = RuntimeConfigStore::new(None); + let runtime_config = runtime_config_store.get_config(protocol_version); + (runtime_config.wasm_config.clone(), runtime_config.transaction_costs.clone()) + } else { + (VMConfig::test(), RuntimeFeesConfig::test()) + }; let promise_results = vec![]; - let code = ContractCode::new(code.to_vec(), None); - let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); - runtime - .run( - &code, - method_name, - &mut fake_external, - context, - &fees, - &promise_results, - LATEST_PROTOCOL_VERSION, - None, - ) - .outcome_error() + + crate::runner::run_with_vm_kind( + &code, + method_name, + &mut fake_external, + context, + &config, + &fees, + &promise_results, + protocol_version.unwrap_or(LATEST_PROTOCOL_VERSION), + None, + vm_kind, + ) + .outcome_error() } fn make_simple_contract_call_with_protocol_version_vm( @@ -91,28 +100,16 @@ fn make_simple_contract_call_with_protocol_version_vm( protocol_version: ProtocolVersion, vm_kind: VMKind, ) -> (VMOutcome, Option) { - let mut fake_external = MockedExternal::new(); - let context = create_context(vec![]); - let runtime_config_store = RuntimeConfigStore::new(None); - let runtime_config = runtime_config_store.get_config(protocol_version); - let fees = &runtime_config.transaction_costs; - let runtime = - vm_kind.runtime(runtime_config.wasm_config.clone()).expect("runtime has not been compiled"); + make_simple_contract_call_ex(code, method_name, vm_kind, Some(protocol_version), 10u64.pow(14)) +} - let promise_results = vec![]; - let code = ContractCode::new(code.to_vec(), None); - runtime - .run( - &code, - method_name, - &mut fake_external, - context, - fees, - &promise_results, - protocol_version, - None, - ) - .outcome_error() +fn make_simple_contract_call_with_gas_vm( + code: &[u8], + method_name: &str, + prepaid_gas: u64, + vm_kind: VMKind, +) -> (VMOutcome, Option) { + make_simple_contract_call_ex(code, method_name, vm_kind, None, prepaid_gas) } fn make_simple_contract_call_vm( @@ -120,7 +117,7 @@ fn make_simple_contract_call_vm( method_name: &str, vm_kind: VMKind, ) -> (VMOutcome, Option) { - make_simple_contract_call_with_gas_vm(code, method_name, 10u64.pow(14), vm_kind) + make_simple_contract_call_ex(code, method_name, vm_kind, None, 10u64.pow(14)) } #[track_caller] @@ -136,10 +133,26 @@ fn gas_and_error_match( assert_eq!(outcome.burnt_gas, gas, "burnt gas differs"); } None => { - assert_eq!(outcome_and_error.0.used_gas, 0); - assert_eq!(outcome_and_error.0.burnt_gas, 0); + assert_eq!(outcome_and_error.0.used_gas, 0, "used gas non-zero"); + assert_eq!(outcome_and_error.0.burnt_gas, 0, "burnt gas non-zero"); } } assert_eq!(outcome_and_error.1, expected_error); } + +/// Small helper to compute expected loading gas cost charged before loading. +/// Includes some hard-coded parameter values that would have to be updated +/// if they change in the future. +fn prepaid_loading_gas(bytes: usize) -> Option { + let expected_gas = if checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + LATEST_PROTOCOL_VERSION + ) { + Some(35445963 + bytes as u64 * 216750) + } else { + None + }; + expected_gas +} diff --git a/runtime/near-vm-runner/src/tests/cache.rs b/runtime/near-vm-runner/src/tests/cache.rs index a08017550b0..f4cb9a27cc5 100644 --- a/runtime/near-vm-runner/src/tests/cache.rs +++ b/runtime/near-vm-runner/src/tests/cache.rs @@ -83,6 +83,7 @@ fn make_cached_contract_call_vm( let promise_results = vec![]; context.prepaid_gas = prepaid_gas; let code = ContractCode::new(code.to_vec(), None); + let gas_counter = context.new_gas_counter(&config); let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); runtime.run( &code, @@ -90,6 +91,7 @@ fn make_cached_contract_call_vm( &mut fake_external, context.clone(), &fees, + gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, Some(cache), diff --git a/runtime/near-vm-runner/src/tests/compile_errors.rs b/runtime/near-vm-runner/src/tests/compile_errors.rs index 05c0701ca6e..7b846ae90a4 100644 --- a/runtime/near-vm-runner/src/tests/compile_errors.rs +++ b/runtime/near-vm-runner/src/tests/compile_errors.rs @@ -245,9 +245,10 @@ fn test_limit_locals() { } .make(); let res = make_simple_contract_call_vm(&wasm_err, "main", vm_kind); + let expected_gas = super::prepaid_loading_gas(wasm_err.len()); gas_and_error_match( res, - None, + expected_gas, Some(VMError::FunctionCallError(FunctionCallError::CompilationError( CompilationError::PrepareError(PrepareError::Deserialization), ))), diff --git a/runtime/near-vm-runner/src/tests/contract_preload.rs b/runtime/near-vm-runner/src/tests/contract_preload.rs index 324bc34898c..f420e7440fb 100644 --- a/runtime/near-vm-runner/src/tests/contract_preload.rs +++ b/runtime/near-vm-runner/src/tests/contract_preload.rs @@ -112,7 +112,7 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { if preloaded { let mut requests = Vec::new(); - let mut caller = ContractCaller::new(4, vm_kind, vm_config); + let mut caller = ContractCaller::new(4, vm_kind, vm_config.clone()); for _ in 0..repeat { requests.push(ContractCallPrepareRequest { code: Arc::clone(&code1), @@ -131,6 +131,7 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { &mut fake_external, context.clone(), &fees, + context.new_gas_counter(&vm_config), &promise_results, ProtocolVersion::MAX, ); @@ -139,7 +140,7 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { errs += err; } } else { - let runtime = vm_kind.runtime(vm_config).expect("runtime is has not been compiled"); + let runtime = vm_kind.runtime(vm_config.clone()).expect("runtime is has not been compiled"); for _ in 0..repeat { let result1 = runtime.run( &code1, @@ -147,6 +148,7 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { &mut fake_external, context.clone(), &fees, + context.new_gas_counter(&vm_config), &promise_results, ProtocolVersion::MAX, cache.as_deref(), @@ -160,6 +162,7 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { &mut fake_external, context.clone(), &fees, + context.new_gas_counter(&vm_config), &promise_results, ProtocolVersion::MAX, cache.as_deref(), diff --git a/runtime/near-vm-runner/src/tests/rs_contract.rs b/runtime/near-vm-runner/src/tests/rs_contract.rs index 7f30eddf708..35ae1a6907a 100644 --- a/runtime/near-vm-runner/src/tests/rs_contract.rs +++ b/runtime/near-vm-runner/src/tests/rs_contract.rs @@ -52,13 +52,15 @@ pub fn test_read_write() { let fees = RuntimeFeesConfig::test(); let promise_results = vec![]; - let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); + let gas_counter = context.new_gas_counter(&config); + let runtime = vm_kind.runtime(config.clone()).expect("runtime has not been compiled"); let result = runtime.run( &code, "write_key_value", &mut fake_external, context, &fees, + gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, @@ -66,12 +68,14 @@ pub fn test_read_write() { assert_run_result(result, 0); let context = create_context(encode(&[10u64])); + let gas_counter = context.new_gas_counter(&config); let result = runtime.run( &code, "read_value", &mut fake_external, context, &fees, + gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, @@ -91,25 +95,28 @@ pub fn test_stablized_host_function() { let fees = RuntimeFeesConfig::test(); let promise_results = vec![]; - let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); + let runtime = vm_kind.runtime(config.clone()).expect("runtime has not been compiled"); let result = runtime.run( &code, "do_ripemd", &mut fake_external, context.clone(), &fees, + context.new_gas_counter(&config), &promise_results, LATEST_PROTOCOL_VERSION, None, ); assert_eq!(result.error(), None); + let gas_counter = context.new_gas_counter(&config); let result = runtime.run( &code, "do_ripemd", &mut fake_external, context, &fees, + gas_counter, &promise_results, ProtocolFeature::MathExtension.protocol_version() - 1, None, @@ -162,10 +169,21 @@ fn run_test_ext( let config = VMConfig::test(); let fees = RuntimeFeesConfig::test(); let context = create_context(input.to_vec()); + let gas_counter = context.new_gas_counter(&config); let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let (outcome, err) = runtime - .run(&code, method, &mut fake_external, context, &fees, &[], LATEST_PROTOCOL_VERSION, None) + .run( + &code, + method, + &mut fake_external, + context, + &fees, + gas_counter, + &[], + LATEST_PROTOCOL_VERSION, + None, + ) .outcome_error(); assert_eq!(outcome.profile.action_gas(), 0); @@ -258,6 +276,7 @@ pub fn test_out_of_memory() { let context = create_context(Vec::new()); let config = VMConfig::free(); let fees = RuntimeFeesConfig::free(); + let gas_counter = context.new_gas_counter(&config); let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let promise_results = vec![]; @@ -267,6 +286,7 @@ pub fn test_out_of_memory() { &mut fake_external, context, &fees, + gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, @@ -299,6 +319,7 @@ fn attach_unspent_gas_but_burn_all_gas() { let prepaid_gas = 100 * 10u64.pow(12); context.prepaid_gas = prepaid_gas; config.limit_config.max_gas_burnt = context.prepaid_gas / 3; + let gas_counter = context.new_gas_counter(&config); let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let (outcome, err) = runtime @@ -308,6 +329,7 @@ fn attach_unspent_gas_but_burn_all_gas() { &mut external, context, &fees, + gas_counter, &[], LATEST_PROTOCOL_VERSION, None, @@ -342,6 +364,7 @@ fn attach_unspent_gas_but_use_all_gas() { context.prepaid_gas = 100 * 10u64.pow(12); config.limit_config.max_gas_burnt = context.prepaid_gas / 3; + let gas_counter = context.new_gas_counter(&config); let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let (outcome, err) = runtime @@ -351,6 +374,7 @@ fn attach_unspent_gas_but_use_all_gas() { &mut external, context, &fees, + gas_counter, &[], LATEST_PROTOCOL_VERSION, None, diff --git a/runtime/near-vm-runner/src/tests/runtime_errors.rs b/runtime/near-vm-runner/src/tests/runtime_errors.rs index 670ee43fa35..acc15a6aa41 100644 --- a/runtime/near-vm-runner/src/tests/runtime_errors.rs +++ b/runtime/near-vm-runner/src/tests/runtime_errors.rs @@ -1,7 +1,6 @@ -use super::gas_and_error_match; use crate::tests::{ - make_simple_contract_call_vm, make_simple_contract_call_with_gas_vm, - make_simple_contract_call_with_protocol_version_vm, with_vm_variants, + gas_and_error_match, make_simple_contract_call_vm, make_simple_contract_call_with_gas_vm, + make_simple_contract_call_with_protocol_version_vm, prepaid_loading_gas, with_vm_variants, }; use crate::vm_kind::VMKind; use near_primitives::version::ProtocolFeature; @@ -38,9 +37,10 @@ fn test_infinite_initializer() { #[test] fn test_infinite_initializer_export_not_found() { with_vm_variants(|vm_kind: VMKind| { + let code = infinite_initializer_contract(); gas_and_error_match( - make_simple_contract_call_vm(&infinite_initializer_contract(), "hello2", vm_kind), - None, + make_simple_contract_call_vm(&code, "hello2", vm_kind), + prepaid_loading_gas(code.len()), Some(VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodNotFound, ))), @@ -81,9 +81,10 @@ fn multi_memories_contract() -> Vec { #[test] fn test_multiple_memories() { with_vm_variants(|vm_kind: VMKind| { - let (result, error) = - make_simple_contract_call_vm(&multi_memories_contract(), "hello", vm_kind); - assert_eq!(result.used_gas, 0); + let code = multi_memories_contract(); + let (result, error) = make_simple_contract_call_vm(&code, "hello", vm_kind); + let expected_gas = prepaid_loading_gas(code.len()); + assert_eq!(result.used_gas, expected_gas.unwrap_or(0)); match error { Some(VMError::FunctionCallError(FunctionCallError::CompilationError( CompilationError::WasmerCompileError { .. }, @@ -112,9 +113,10 @@ fn test_multiple_memories() { #[test] fn test_export_not_found() { with_vm_variants(|vm_kind: VMKind| { + let code = simple_contract(); gas_and_error_match( - make_simple_contract_call_vm(&simple_contract(), "hello2", vm_kind), - None, + make_simple_contract_call_vm(&code, "hello2", vm_kind), + prepaid_loading_gas(code.len()), Some(VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodNotFound, ))), @@ -125,8 +127,11 @@ fn test_export_not_found() { #[test] fn test_empty_method() { with_vm_variants(|vm_kind: VMKind| { + // Empty method should be checked before anything that require gas. + // Running this test without prepaid gas to check that. + let code = simple_contract(); gas_and_error_match( - make_simple_contract_call_vm(&simple_contract(), "", vm_kind), + make_simple_contract_call_with_gas_vm(&code, "", 0, vm_kind), None, Some(VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodEmptyName, @@ -369,9 +374,11 @@ fn wrong_signature_contract() -> Vec { #[test] fn test_wrong_signature_contract() { with_vm_variants(|vm_kind: VMKind| { + let code = wrong_signature_contract(); + let expected_gas = prepaid_loading_gas(code.len()); gas_and_error_match( - make_simple_contract_call_vm(&wrong_signature_contract(), "hello", vm_kind), - None, + make_simple_contract_call_vm(&code, "hello", vm_kind), + expected_gas, Some(VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodInvalidSignature, ))), @@ -393,9 +400,10 @@ fn export_wrong_type() -> Vec { #[test] fn test_export_wrong_type() { with_vm_variants(|vm_kind: VMKind| { + let code = export_wrong_type(); gas_and_error_match( - make_simple_contract_call_vm(&export_wrong_type(), "hello", vm_kind), - None, + make_simple_contract_call_vm(&code, "hello", vm_kind), + prepaid_loading_gas(code.len()), Some(VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodNotFound, ))), @@ -597,9 +605,10 @@ fn bad_import_func(env: &str) -> Vec { // Invalid import from "env" -> LinkError fn test_bad_import_1() { with_vm_variants(|vm_kind: VMKind| { + let code = bad_import_global("wtf"); gas_and_error_match( - make_simple_contract_call_vm(&bad_import_global("wtf"), "hello", vm_kind), - None, + make_simple_contract_call_vm(&code, "hello", vm_kind), + prepaid_loading_gas(code.len()), Some(VMError::FunctionCallError(FunctionCallError::CompilationError( CompilationError::PrepareError(PrepareError::Instantiate), ))), @@ -610,9 +619,10 @@ fn test_bad_import_1() { #[test] fn test_bad_import_2() { with_vm_variants(|vm_kind: VMKind| { + let code = bad_import_func("wtf"); gas_and_error_match( - make_simple_contract_call_vm(&bad_import_func("wtf"), "hello", vm_kind), - None, + make_simple_contract_call_vm(&code, "hello", vm_kind), + prepaid_loading_gas(code.len()), Some(VMError::FunctionCallError(FunctionCallError::CompilationError( CompilationError::PrepareError(PrepareError::Instantiate), ))), @@ -873,3 +883,297 @@ fn test_nan_sign() { } }); } + +// Check that a `GasExceeded` error is returned when there is not enough gas to +// even load a contract. +#[test] +fn test_gas_exceed_loading() { + with_vm_variants(|vm_kind: VMKind| { + // Loading is the first thing that happens on the VM entrypoint that + // requires gas, thus attaching only 1 gas triggers the desired + // behavior. + let gas = 1; + let method_name = "non_empty_non_existing"; + let actual = + make_simple_contract_call_with_gas_vm(&simple_contract(), method_name, gas, vm_kind); + gas_and_error_match( + actual, + Some(gas), //< It is import to be non-zero gas here. + Some(VMError::FunctionCallError(FunctionCallError::HostError(HostError::GasExceeded))), + ); + }); +} + +#[cfg(feature = "protocol_feature_fix_contract_loading_cost")] +mod fix_contract_loading_cost_protocol_upgrade { + use super::*; + use crate::tests::make_simple_contract_call_ex; + + /// Simple contract that uses non-zero gas to execute main. + #[cfg(feature = "protocol_feature_fix_contract_loading_cost")] + fn almost_trivial_contract_and_exec_gas() -> (Vec, u64) { + let code = wat::parse_str( + r#" + (module + (type (;0;) (func)) + (func (;0;) (type 0) + i32.const 1 + i32.const 1 + i32.div_s + return + ) + (export "main" (func 0)) + )"#, + ) + .unwrap(); + (code, 3_291_024) + } + + // Normal execution should be unchanged before and after. + #[test] + fn test_fn_loading_gas_protocol_upgrade() { + with_vm_variants(|vm_kind: VMKind| { + let (code, exec_gas) = almost_trivial_contract_and_exec_gas(); + let loading_gas = prepaid_loading_gas(code.len()).unwrap(); + let total_gas = exec_gas + loading_gas; + + let res = make_simple_contract_call_ex( + &code, + "main", + vm_kind, + Some(ProtocolFeature::FixContractLoadingCost.protocol_version() - 1), + total_gas + 1, + ); + gas_and_error_match(res, Some(total_gas), None); + let res = make_simple_contract_call_ex( + &code, + "main", + vm_kind, + Some(ProtocolFeature::FixContractLoadingCost.protocol_version()), + total_gas + 1, + ); + gas_and_error_match(res, Some(total_gas), None); + }); + } + + // Executing with just enough gas to load the contract will fail before and + // after. Both charge the same amount of gas. + #[test] + fn test_fn_loading_gas_protocol_upgrade_exceed_loading() { + with_vm_variants(|vm_kind: VMKind| { + let (code, _exec_gas) = almost_trivial_contract_and_exec_gas(); + let loading_gas = prepaid_loading_gas(code.len()).unwrap(); + + let res = make_simple_contract_call_ex( + &code, + "main", + vm_kind, + Some(ProtocolFeature::FixContractLoadingCost.protocol_version() - 1), + loading_gas, + ); + gas_and_error_match( + res, + Some(loading_gas), + Some(VMError::FunctionCallError(FunctionCallError::HostError( + HostError::GasExceeded, + ))), + ); + let res = make_simple_contract_call_ex( + &code, + "main", + vm_kind, + Some(ProtocolFeature::FixContractLoadingCost.protocol_version()), + loading_gas, + ); + gas_and_error_match( + res, + Some(loading_gas), + Some(VMError::FunctionCallError(FunctionCallError::HostError( + HostError::GasExceeded, + ))), + ); + }); + } + + /// Executing with enough gas to finish loading but not to execute the full + /// contract should have the same outcome before and after. + #[test] + fn test_fn_loading_gas_protocol_upgrade_exceed_executing() { + with_vm_variants(|vm_kind: VMKind| { + let (code, exec_gas) = almost_trivial_contract_and_exec_gas(); + let loading_gas = prepaid_loading_gas(code.len()).unwrap(); + let total_gas = exec_gas + loading_gas; + + let res = make_simple_contract_call_ex( + &code, + "main", + vm_kind, + Some(ProtocolFeature::FixContractLoadingCost.protocol_version() - 1), + total_gas - 1, + ); + gas_and_error_match( + res, + Some(total_gas - 1), + Some(VMError::FunctionCallError(FunctionCallError::HostError( + HostError::GasExceeded, + ))), + ); + let res = make_simple_contract_call_ex( + &code, + "main", + vm_kind, + Some(ProtocolFeature::FixContractLoadingCost.protocol_version()), + total_gas - 1, + ); + gas_and_error_match( + res, + Some(total_gas - 1), + Some(VMError::FunctionCallError(FunctionCallError::HostError( + HostError::GasExceeded, + ))), + ); + }); + } + + fn function_not_defined_contract() -> Vec { + wat::parse_str( + r#" + (module + (export "hello" (func 0)) + )"#, + ) + .unwrap() + } + + #[track_caller] + fn check_failed_compilation(vm_kind: VMKind, code: &[u8], expected_error: CompilationError) { + let new_version = ProtocolFeature::FixContractLoadingCost.protocol_version(); + let old_version = new_version - 1; + { + let res = make_simple_contract_call_with_protocol_version_vm( + &code, + "main", + old_version, + vm_kind, + ); + // This is the first property the tests check: Gas cost before + // compilation errors must remain zero for old versions. + let expected_gas = None; + gas_and_error_match( + res, + expected_gas, + Some(VMError::FunctionCallError(FunctionCallError::CompilationError( + expected_error.clone(), + ))), + ); + } + + { + let res = make_simple_contract_call_with_protocol_version_vm( + &code, + "main", + new_version, + vm_kind, + ); + + // This is the second property the tests check: Gas costs + // for loading gas are charged when failing during + // compilation. + let expected_gas = prepaid_loading_gas(code.len()); + gas_and_error_match( + res, + expected_gas, + Some(VMError::FunctionCallError(FunctionCallError::CompilationError( + expected_error, + ))), + ); + } + } + + /// Failure during preparation must remain free of gas charges for old versions + /// but new versions must charge the loading gas. + #[test] + fn test_fn_loading_gas_protocol_upgrade_fail_preparing() { + with_vm_variants(|vm_kind: VMKind| { + // This list covers all control flows that are expected to change + // with the protocol feature. + // Having a test for every possible preparation error would be even + // better, to ensure triggering any of them will always remain + // compatible with versions before this upgrade. Unfortunately, we + // currently do not have tests ready to trigger each error. + check_failed_compilation( + vm_kind, + &function_not_defined_contract(), + CompilationError::PrepareError(PrepareError::Deserialization), + ); + check_failed_compilation( + vm_kind, + &bad_import_global("wtf"), + CompilationError::PrepareError(PrepareError::Instantiate), + ); + check_failed_compilation( + vm_kind, + &bad_import_func("wtf"), + CompilationError::PrepareError(PrepareError::Instantiate), + ); + check_failed_compilation( + vm_kind, + &near_test_contracts::LargeContract { + functions: 101, + locals_per_function: 9901, + ..Default::default() + } + .make(), + CompilationError::PrepareError(PrepareError::TooManyLocals), + ); + let functions_number_limit: u32 = 10_000; + check_failed_compilation( + vm_kind, + &near_test_contracts::LargeContract { + functions: functions_number_limit / 2, + panic_imports: functions_number_limit / 2 + 1, + ..Default::default() + } + .make(), + CompilationError::PrepareError(PrepareError::TooManyFunctions), + ); + }); + } + + /// For compilation tests other than checked in `test_fn_loading_gas_protocol_upgrade_fail_preparing`. + #[test] + fn test_fn_loading_gas_protocol_upgrade_fail_compiling() { + with_vm_variants(|vm_kind: VMKind| { + // This triggers `WasmerCompileError` or, for wasmtime, `LinkerError`. + let code = multi_memories_contract(); + let new_version = ProtocolFeature::FixContractLoadingCost.protocol_version(); + let old_version = new_version - 1; + let old_error = { + let (outcome, error) = make_simple_contract_call_with_protocol_version_vm( + &code, + "main", + old_version, + vm_kind, + ); + assert_eq!(0, outcome.used_gas); + assert_eq!(0, outcome.burnt_gas); + error + }; + + let new_error = { + let (outcome, error) = make_simple_contract_call_with_protocol_version_vm( + &code, + "main", + new_version, + vm_kind, + ); + let expected_gas = prepaid_loading_gas(code.len()).unwrap(); + assert_eq!(expected_gas, outcome.used_gas); + assert_eq!(expected_gas, outcome.burnt_gas); + error + }; + + assert_eq!(old_error, new_error); + }); + } +} diff --git a/runtime/near-vm-runner/src/tests/ts_contract.rs b/runtime/near-vm-runner/src/tests/ts_contract.rs index 41f124b18fb..2bcbd4bd8ca 100644 --- a/runtime/near-vm-runner/src/tests/ts_contract.rs +++ b/runtime/near-vm-runner/src/tests/ts_contract.rs @@ -20,13 +20,15 @@ pub fn test_ts_contract() { // Call method that panics. let promise_results = vec![]; - let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); + let gas_counter = context.new_gas_counter(&config); + let runtime = vm_kind.runtime(config.clone()).expect("runtime has not been compiled"); let result = runtime.run( &code, "try_panic", &mut fake_external, context, &fees, + gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, @@ -40,12 +42,14 @@ pub fn test_ts_contract() { // Call method that writes something into storage. let context = create_context(b"foo bar".to_vec()); + let gas_counter = context.new_gas_counter(&config); runtime.run( &code, "try_storage_write", &mut fake_external, context, &fees, + gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, @@ -61,12 +65,14 @@ pub fn test_ts_contract() { // Call method that reads the value from storage using registers. let context = create_context(b"foo".to_vec()); + let gas_counter = context.new_gas_counter(&config); let result = runtime.run( &code, "try_storage_read", &mut fake_external, context, &fees, + gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, diff --git a/runtime/near-vm-runner/src/wasmer2_runner.rs b/runtime/near-vm-runner/src/wasmer2_runner.rs index 73e98abf87d..b972a135ce8 100644 --- a/runtime/near-vm-runner/src/wasmer2_runner.rs +++ b/runtime/near-vm-runner/src/wasmer2_runner.rs @@ -4,12 +4,13 @@ use crate::prepare::WASM_FEATURES; use crate::runner::VMResult; use crate::{cache, imports}; use memoffset::offset_of; +use near_primitives::checked_feature; use near_primitives::contract::ContractCode; use near_primitives::runtime::fees::RuntimeFeesConfig; use near_primitives::types::CompiledContractCache; use near_stable_hasher::StableHasher; use near_vm_errors::{CompilationError, FunctionCallError, MethodResolveError, VMError, WasmTrap}; -use near_vm_logic::gas_counter::FastGasCounter; +use near_vm_logic::gas_counter::{FastGasCounter, GasCounter}; use near_vm_logic::types::{PromiseResult, ProtocolVersion}; use near_vm_logic::{External, MemoryLike, VMConfig, VMContext, VMLogic}; use std::hash::{Hash, Hasher}; @@ -416,6 +417,7 @@ impl Wasmer2VM { ext: &mut dyn External, context: VMContext, fees_config: &'a RuntimeFeesConfig, + gas_counter: GasCounter, promise_results: &'a [PromiseResult], current_protocol_version: ProtocolVersion, ) -> VMResult { @@ -425,6 +427,7 @@ impl Wasmer2VM { context, &self.config, fees_config, + gas_counter, promise_results, memory, current_protocol_version, @@ -512,6 +515,7 @@ impl crate::runner::VM for Wasmer2VM { ext: &mut dyn External, context: VMContext, fees_config: &RuntimeFeesConfig, + gas_counter: GasCounter, promise_results: &[PromiseResult], current_protocol_version: ProtocolVersion, cache: Option<&dyn CompiledContractCache>, @@ -524,19 +528,6 @@ impl crate::runner::VM for Wasmer2VM { ) .entered(); - if method_name.is_empty() { - let error = VMError::FunctionCallError(FunctionCallError::MethodResolveError( - MethodResolveError::MethodEmptyName, - )); - return VMResult::nop_outcome(error); - } - let artifact = - cache::wasmer2_cache::compile_module_cached_wasmer2(code, &self.config, cache); - let artifact = match into_vm_result(artifact) { - Ok(it) => it, - Err(err) => return VMResult::nop_outcome(err), - }; - let mut memory = Wasmer2Memory::new( self.config.limit_config.initial_memory_pages, self.config.limit_config.max_memory_pages, @@ -551,16 +542,32 @@ impl crate::runner::VM for Wasmer2VM { context, &self.config, fees_config, + gas_counter, promise_results, &mut memory, current_protocol_version, ); - // TODO: charge this before artifact is loaded - if logic.add_contract_loading_fee(code.code().len() as u64).is_err() { - let error = VMError::FunctionCallError(FunctionCallError::HostError( - near_vm_errors::HostError::GasExceeded, - )); - return VMResult::abort(logic, error); + + let artifact = + cache::wasmer2_cache::compile_module_cached_wasmer2(code, &self.config, cache); + let artifact = match into_vm_result(artifact) { + Ok(it) => it, + Err(err) => { + return VMResult::abort(logic, err); + } + }; + + if !checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + if logic.add_contract_loading_fee(code.code().len() as u64).is_err() { + let error = VMError::FunctionCallError(FunctionCallError::HostError( + near_vm_errors::HostError::GasExceeded, + )); + return VMResult::abort(logic, error); + } } let import = imports::wasmer2::build( vmmemory, @@ -569,8 +576,15 @@ impl crate::runner::VM for Wasmer2VM { artifact.engine(), ); if let Err(e) = get_entrypoint_index(&*artifact, method_name) { - // TODO: This should return an outcome to account for loading cost - return VMResult::nop_outcome(e); + if checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + return VMResult::abort(logic, e); + } else { + return VMResult::nop_outcome(e); + } } match self.run_method(&artifact, import, method_name) { Ok(()) => VMResult::ok(logic), diff --git a/runtime/near-vm-runner/src/wasmer_runner.rs b/runtime/near-vm-runner/src/wasmer_runner.rs index eb8faf5306f..71466485c20 100644 --- a/runtime/near-vm-runner/src/wasmer_runner.rs +++ b/runtime/near-vm-runner/src/wasmer_runner.rs @@ -4,12 +4,14 @@ use crate::memory::WasmerMemory; use crate::prepare::WASM_FEATURES; use crate::runner::VMResult; use crate::{cache, imports}; +use near_primitives::checked_feature; use near_primitives::config::VMConfig; use near_primitives::contract::ContractCode; use near_primitives::runtime::fees::RuntimeFeesConfig; use near_primitives::types::CompiledContractCache; use near_primitives::version::ProtocolVersion; use near_vm_errors::{CompilationError, FunctionCallError, MethodResolveError, VMError, WasmTrap}; +use near_vm_logic::gas_counter::GasCounter; use near_vm_logic::types::PromiseResult; use near_vm_logic::{External, VMContext, VMLogic, VMLogicError}; use wasmer_runtime::{ImportObject, Module}; @@ -231,6 +233,7 @@ pub(crate) fn run_wasmer0_module<'a>( context: VMContext, wasm_config: &'a VMConfig, fees_config: &'a RuntimeFeesConfig, + gas_counter: GasCounter, promise_results: &'a [PromiseResult], current_protocol_version: ProtocolVersion, ) -> VMResult { @@ -248,6 +251,7 @@ pub(crate) fn run_wasmer0_module<'a>( context, wasm_config, fees_config, + gas_counter, promise_results, memory, current_protocol_version, @@ -288,6 +292,7 @@ impl crate::runner::VM for Wasmer0VM { ext: &mut dyn External, context: VMContext, fees_config: &RuntimeFeesConfig, + gas_counter: GasCounter, promise_results: &[PromiseResult], current_protocol_version: ProtocolVersion, cache: Option<&dyn CompiledContractCache>, @@ -311,19 +316,7 @@ impl crate::runner::VM for Wasmer0VM { if !is_x86_feature_detected!("avx") { panic!("AVX support is required in order to run Wasmer VM Singlepass backend."); } - if method_name.is_empty() { - let err = VMError::FunctionCallError(FunctionCallError::MethodResolveError( - MethodResolveError::MethodEmptyName, - )); - return VMResult::nop_outcome(err); - } - // TODO: consider using get_module() here, once we'll go via deployment path. - let module = cache::wasmer0_cache::compile_module_cached_wasmer0(code, &self.config, cache); - let module = match into_vm_result(module) { - Ok(x) => x, - Err(err) => return VMResult::nop_outcome(err), - }; let mut memory = WasmerMemory::new( self.config.limit_config.initial_memory_pages, self.config.limit_config.max_memory_pages, @@ -337,25 +330,52 @@ impl crate::runner::VM for Wasmer0VM { context, &self.config, fees_config, + gas_counter, promise_results, &mut memory, current_protocol_version, ); - // TODO: charge this before module is loaded - if logic.add_contract_loading_fee(code.code().len() as u64).is_err() { - let err = VMError::FunctionCallError(FunctionCallError::HostError( - near_vm_errors::HostError::GasExceeded, - )); - return VMResult::abort(logic, err); + // TODO: consider using get_module() here, once we'll go via deployment path. + let module = cache::wasmer0_cache::compile_module_cached_wasmer0(code, &self.config, cache); + let module = match into_vm_result(module) { + Ok(x) => x, + // Note on backwards-compatibility: This error used to be an error + // without result, later refactored to NOP outcome. Now this returns + // an actual outcome, including gas costs that occurred before this + // point. This is compatible with earlier versions because those + // version do not have gas costs before reaching this code. (Also + // see `test_old_fn_loading_behavior_preserved` for a test that + // verifies future changes do not counteract this assumption.) + Err(err) => return VMResult::abort(logic, err), + }; + + if !checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + if logic.add_contract_loading_fee(code.code().len() as u64).is_err() { + let err = VMError::FunctionCallError(FunctionCallError::HostError( + near_vm_errors::HostError::GasExceeded, + )); + return VMResult::abort(logic, err); + } } let import_object = imports::wasmer::build(memory_copy, &mut logic, current_protocol_version); if let Err(e) = check_method(&module, method_name) { - // TODO: This should return an outcome to account for loading cost - return VMResult::nop_outcome(e); + if checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + return VMResult::abort(logic, e); + } else { + return VMResult::nop_outcome(e); + } } match run_method(&module, &import_object, method_name) { diff --git a/runtime/near-vm-runner/src/wasmtime_runner.rs b/runtime/near-vm-runner/src/wasmtime_runner.rs index b75270ff78c..82b0a0c1283 100644 --- a/runtime/near-vm-runner/src/wasmtime_runner.rs +++ b/runtime/near-vm-runner/src/wasmtime_runner.rs @@ -2,6 +2,7 @@ use crate::errors::IntoVMError; use crate::prepare::WASM_FEATURES; use crate::runner::VMResult; use crate::{imports, prepare}; +use near_primitives::checked_feature; use near_primitives::config::VMConfig; use near_primitives::contract::ContractCode; use near_primitives::hash::CryptoHash; @@ -12,6 +13,7 @@ use near_vm_errors::{ CompilationError, FunctionCallError, MethodResolveError, PrepareError, VMError, VMLogicError, WasmTrap, }; +use near_vm_logic::gas_counter::GasCounter; use near_vm_logic::types::PromiseResult; use near_vm_logic::{External, MemoryLike, VMContext, VMLogic}; use std::cell::RefCell; @@ -185,6 +187,7 @@ impl crate::runner::VM for WasmtimeVM { ext: &mut dyn External, context: VMContext, fees_config: &RuntimeFeesConfig, + gas_counter: GasCounter, promise_results: &[PromiseResult], current_protocol_version: ProtocolVersion, _cache: Option<&dyn CompiledContractCache>, @@ -205,45 +208,45 @@ impl crate::runner::VM for WasmtimeVM { self.config.limit_config.max_memory_pages, ) .unwrap(); - let prepared_code = match prepare::prepare_contract(code.code(), &self.config) { - Ok(code) => code, - Err(err) => return VMResult::nop_outcome(VMError::from(err)), - }; - let module = match Module::new(&engine, prepared_code) { - Ok(module) => module, - Err(err) => return VMResult::nop_outcome(err.into_vm_error()), - }; - let mut linker = Linker::new(&engine); let memory_copy = memory.0; let mut logic = VMLogic::new_with_protocol_version( ext, context, &self.config, fees_config, + gas_counter, promise_results, &mut memory, current_protocol_version, ); - // TODO: charge this before preparing contract - if logic.add_contract_loading_fee(code.code().len() as u64).is_err() { - let err = VMError::FunctionCallError(FunctionCallError::HostError( - near_vm_errors::HostError::GasExceeded, - )); - return VMResult::abort(logic, err); + let prepared_code = match prepare::prepare_contract(code.code(), &self.config) { + Ok(code) => code, + Err(err) => return VMResult::abort(logic, VMError::from(err)), + }; + let module = match Module::new(&engine, prepared_code) { + Ok(module) => module, + Err(err) => return VMResult::abort(logic, err.into_vm_error()), + }; + let mut linker = Linker::new(&engine); + + if !checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + if logic.add_contract_loading_fee(code.code().len() as u64).is_err() { + let err = VMError::FunctionCallError(FunctionCallError::HostError( + near_vm_errors::HostError::GasExceeded, + )); + return VMResult::abort(logic, err); + } } // Unfortunately, due to the Wasmtime implementation we have to do tricks with the // lifetimes of the logic instance and pass raw pointers here. let raw_logic = &mut logic as *mut _ as *mut c_void; imports::wasmtime::link(&mut linker, memory_copy, raw_logic, current_protocol_version); - // TODO: While fixing other loading cost, check this before loading contract - if method_name.is_empty() { - let err = VMError::FunctionCallError(FunctionCallError::MethodResolveError( - MethodResolveError::MethodEmptyName, - )); - return VMResult::nop_outcome(err); - } match module.get_export(method_name) { Some(export) => match export { Func(func_type) => { @@ -252,24 +255,45 @@ impl crate::runner::VM for WasmtimeVM { VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodInvalidSignature, )); - // TODO: This should return an outcome to account for loading cost - return VMResult::nop_outcome(err); + if checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + return VMResult::abort(logic, err); + } else { + return VMResult::nop_outcome(err); + } } } _ => { let err = VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodNotFound, )); - // TODO: This should return an outcome to account for loading cost - return VMResult::nop_outcome(err); + if checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + return VMResult::abort(logic, err); + } else { + return VMResult::nop_outcome(err); + } } }, None => { let err = VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodNotFound, )); - // TODO: This should return an outcome to account for loading cost - return VMResult::nop_outcome(err); + if checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + return VMResult::abort(logic, err); + } else { + return VMResult::nop_outcome(err); + } } } match linker.instantiate(&mut store, &module) { @@ -285,8 +309,15 @@ impl crate::runner::VM for WasmtimeVM { let err = VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodNotFound, )); - // TODO: This should return an outcome to account for loading cost - VMResult::nop_outcome(err) + if checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + return VMResult::abort(logic, err); + } else { + return VMResult::nop_outcome(err); + } } }, Err(err) => VMResult::abort(logic, err.into_vm_error()), diff --git a/runtime/runtime-params-estimator/src/function_call.rs b/runtime/runtime-params-estimator/src/function_call.rs index 70753cef88c..6036ac74bd1 100644 --- a/runtime/runtime-params-estimator/src/function_call.rs +++ b/runtime/runtime-params-estimator/src/function_call.rs @@ -1,6 +1,7 @@ use crate::config::GasMetric; use crate::gas_cost::GasCost; use crate::least_squares::least_squares_method; +use crate::utils::estimator_gas_counter; use crate::vm_estimator::create_context; use near_primitives::contract::ContractCode; use near_primitives::runtime::config_store::RuntimeConfigStore; @@ -89,6 +90,7 @@ pub fn compute_function_call_cost( &mut fake_external, fake_context.clone(), &fees, + estimator_gas_counter(&runtime_config.wasm_config), &promise_results, protocol_version, cache, @@ -104,6 +106,7 @@ pub fn compute_function_call_cost( &mut fake_external, fake_context.clone(), &fees, + estimator_gas_counter(&runtime_config.wasm_config), &promise_results, protocol_version, cache, diff --git a/runtime/runtime-params-estimator/src/gas_metering.rs b/runtime/runtime-params-estimator/src/gas_metering.rs index a5f210528e8..2b3741dd88c 100644 --- a/runtime/runtime-params-estimator/src/gas_metering.rs +++ b/runtime/runtime-params-estimator/src/gas_metering.rs @@ -1,5 +1,6 @@ use crate::config::Config; use crate::gas_cost::{GasCost, LeastSquaresTolerance}; +use crate::utils::estimator_gas_counter; use crate::vm_estimator::create_context; use near_primitives::config::VMConfig; use near_primitives::contract::ContractCode; @@ -151,6 +152,7 @@ pub(crate) fn compute_gas_metering_cost(config: &Config, contract: &ContractCode &mut fake_external, fake_context.clone(), &fees, + estimator_gas_counter(&runtime_config.wasm_config), &promise_results, PROTOCOL_VERSION, cache, @@ -170,6 +172,7 @@ pub(crate) fn compute_gas_metering_cost(config: &Config, contract: &ContractCode &mut fake_external, fake_context.clone(), &fees, + estimator_gas_counter(&runtime_config.wasm_config), &promise_results, PROTOCOL_VERSION, cache, @@ -186,6 +189,7 @@ pub(crate) fn compute_gas_metering_cost(config: &Config, contract: &ContractCode &mut fake_external, fake_context.clone(), &fees, + estimator_gas_counter(&runtime_config.wasm_config), &promise_results, PROTOCOL_VERSION, cache, @@ -202,6 +206,7 @@ pub(crate) fn compute_gas_metering_cost(config: &Config, contract: &ContractCode &mut fake_external, fake_context.clone(), &fees, + estimator_gas_counter(&runtime_config.wasm_config), &promise_results, PROTOCOL_VERSION, cache, diff --git a/runtime/runtime-params-estimator/src/lib.rs b/runtime/runtime-params-estimator/src/lib.rs index c1bad5f69f6..e91e551ba26 100644 --- a/runtime/runtime-params-estimator/src/lib.rs +++ b/runtime/runtime-params-estimator/src/lib.rs @@ -107,6 +107,7 @@ use crate::estimator_context::EstimatorContext; use crate::gas_cost::GasCost; use crate::rocksdb::{rocks_db_inserts_cost, rocks_db_read_cost}; use crate::transaction_builder::TransactionBuilder; +use crate::utils::estimator_gas_counter; use crate::vm_estimator::create_context; pub use crate::cost::Cost; @@ -749,6 +750,7 @@ fn wasm_instruction(ctx: &mut EstimatorContext) -> GasCost { &mut fake_external, context, &fees, + estimator_gas_counter(&config), &promise_results, PROTOCOL_VERSION, Some(&cache), diff --git a/runtime/runtime-params-estimator/src/utils.rs b/runtime/runtime-params-estimator/src/utils.rs index a7a6414d1b8..02b393004fa 100644 --- a/runtime/runtime-params-estimator/src/utils.rs +++ b/runtime/runtime-params-estimator/src/utils.rs @@ -8,7 +8,8 @@ use std::collections::HashMap; use near_primitives::transaction::SignedTransaction; use near_primitives::types::AccountId; -use near_vm_logic::ExtCosts; +use near_vm_logic::gas_counter::GasCounter; +use near_vm_logic::{ExtCosts, VMConfig}; use rand::distributions::Alphanumeric; use rand::Rng; use rand_xorshift::XorShiftRng; @@ -295,6 +296,17 @@ pub(crate) fn generate_data_only_contract(data_size: usize) -> Vec { wat::parse_str(wat_code).unwrap() } +pub(crate) fn estimator_gas_counter(wasm_config: &VMConfig) -> GasCounter { + let max_gas = wasm_config.limit_config.max_gas_burnt; + GasCounter::new( + wasm_config.ext_costs.clone(), + max_gas, + wasm_config.regular_op_cost, + max_gas, + false, + ) +} + #[cfg(test)] mod test { use super::percentiles; From cb5e4dbab17e09748340fe8d841949dd08f56e5b Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Mon, 9 May 2022 17:12:54 +0200 Subject: [PATCH 02/16] Update hash for increased nightly protcol version --- chain/chain/src/tests/simple_chain.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/chain/chain/src/tests/simple_chain.rs b/chain/chain/src/tests/simple_chain.rs index 84400fac9e0..e1907acffc5 100644 --- a/chain/chain/src/tests/simple_chain.rs +++ b/chain/chain/src/tests/simple_chain.rs @@ -25,7 +25,7 @@ fn empty_chain() { let hash = chain.head().unwrap().last_block_hash; // The hashes here will have to be modified after each change to genesis file. #[cfg(feature = "nightly_protocol")] - assert_eq!(hash, CryptoHash::from_str("3eSPNwhSs9pT2jeG8Y8M14cCqXwZ5ikGA6c4bhubcHWv").unwrap()); + assert_eq!(hash, CryptoHash::from_str("DPDaJGghKBJe4ZaYSoETjTN5w9F49ix5nuYpX1tbxPfQ").unwrap()); #[cfg(not(feature = "nightly_protocol"))] assert_eq!(hash, CryptoHash::from_str("8t6f63ezCoqS2nNxT7KivhvHH5tvNND4dj7RY3Hwhn64").unwrap()); assert_eq!(count_utc, 1); @@ -55,7 +55,7 @@ fn build_chain() { #[cfg(feature = "nightly_protocol")] assert_eq!( prev_hash, - CryptoHash::from_str("8F4vXPPNevoQXVGdwKAZQfzzxhSyqWp3xJiik4RMUKSk").unwrap() + CryptoHash::from_str("7Db9S56zVuTKvYGnHXvsJYH6CvQBBEm2TC7Y3S85SZps").unwrap() ); #[cfg(not(feature = "nightly_protocol"))] assert_eq!( @@ -78,7 +78,7 @@ fn build_chain() { #[cfg(feature = "nightly_protocol")] assert_eq!( chain.head().unwrap().last_block_hash, - CryptoHash::from_str("DrW7MsRaFhEdjQcxjqrTXvNmQ1eptgURQ7RUTeZnrBXC").unwrap() + CryptoHash::from_str("HbPMs5o1Twqb7ZqrrhaCGawwZbYJVaPHZ7tfoihcnYxc").unwrap() ); #[cfg(not(feature = "nightly_protocol"))] assert_eq!( From 6b0428815817403c2c5b39b4759ad06d7440002d Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Tue, 31 May 2022 11:43:47 +0200 Subject: [PATCH 03/16] Human-readable WAT in test --- .../near-vm-runner/src/tests/runtime_errors.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/runtime/near-vm-runner/src/tests/runtime_errors.rs b/runtime/near-vm-runner/src/tests/runtime_errors.rs index acc15a6aa41..0bb5afbbf17 100644 --- a/runtime/near-vm-runner/src/tests/runtime_errors.rs +++ b/runtime/near-vm-runner/src/tests/runtime_errors.rs @@ -914,16 +914,14 @@ mod fix_contract_loading_cost_protocol_upgrade { fn almost_trivial_contract_and_exec_gas() -> (Vec, u64) { let code = wat::parse_str( r#" - (module - (type (;0;) (func)) - (func (;0;) (type 0) - i32.const 1 - i32.const 1 - i32.div_s - return - ) - (export "main" (func 0)) - )"#, + (module + (func (export "main") + i32.const 1 + i32.const 1 + i32.div_s + return + ) + )"#, ) .unwrap(); (code, 3_291_024) From e31957a876591435fa109b85c2404e13fd45df90 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 11:27:33 +0200 Subject: [PATCH 04/16] Push common VM code to checks_before_code_loading --- runtime/near-vm-logic/src/logic.rs | 54 +++++++++++- .../near-vm-runner-standalone/src/script.rs | 1 - .../fuzz/fuzz_targets/runner.rs | 2 - runtime/near-vm-runner/src/preload.rs | 4 - runtime/near-vm-runner/src/runner.rs | 87 +------------------ runtime/near-vm-runner/src/tests.rs | 29 ++++--- runtime/near-vm-runner/src/tests/cache.rs | 2 - .../src/tests/contract_preload.rs | 7 +- .../near-vm-runner/src/tests/rs_contract.rs | 30 +------ .../near-vm-runner/src/tests/ts_contract.rs | 8 +- runtime/near-vm-runner/src/wasmer2_runner.rs | 15 ++-- runtime/near-vm-runner/src/wasmer_runner.rs | 14 +-- runtime/near-vm-runner/src/wasmtime_runner.rs | 12 ++- .../src/function_call.rs | 3 - .../src/gas_metering.rs | 5 -- runtime/runtime-params-estimator/src/lib.rs | 2 - runtime/runtime-params-estimator/src/utils.rs | 14 +-- 17 files changed, 105 insertions(+), 184 deletions(-) diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index b668de0aa67..1b999f87f73 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -7,6 +7,8 @@ use crate::utils::split_method_names; use crate::{ReceiptMetadata, ValuePtr}; use byteorder::ByteOrder; use near_crypto::Secp256K1Signature; +use near_primitives::checked_feature; +use near_primitives::config::ViewConfig; use near_primitives::version::is_implicit_account_creation_enabled; use near_primitives_core::config::ExtCosts::*; use near_primitives_core::config::{ActionCosts, ExtCosts, VMConfig}; @@ -18,8 +20,8 @@ use near_primitives_core::types::{ AccountId, Balance, EpochHeight, Gas, ProtocolVersion, StorageUsage, }; use near_primitives_core::types::{GasDistribution, GasWeight}; -use near_vm_errors::InconsistentStateError; use near_vm_errors::{HostError, VMLogicError}; +use near_vm_errors::{InconsistentStateError, VMError}; use std::collections::HashMap; use std::mem::size_of; @@ -106,7 +108,6 @@ impl<'a> VMLogic<'a> { context: VMContext, config: &'a VMConfig, fees_config: &'a RuntimeFeesConfig, - gas_counter: GasCounter, promise_results: &'a [PromiseResult], memory: &'a mut dyn MemoryLike, current_protocol_version: ProtocolVersion, @@ -114,8 +115,20 @@ impl<'a> VMLogic<'a> { // Overflow should be checked before calling VMLogic. let current_account_balance = context.account_balance + context.attached_deposit; let current_storage_usage = context.storage_usage; + let max_gas_burnt = match context.view_config { + Some(ViewConfig { max_gas_burnt: max_gas_burnt_view }) => max_gas_burnt_view, + None => config.limit_config.max_gas_burnt, + }; let current_account_locked_balance = context.account_locked_balance; + let gas_counter = GasCounter::new( + config.ext_costs.clone(), + max_gas_burnt, + config.regular_op_cost, + context.prepaid_gas, + context.is_view(), + ); + Self { ext, context, @@ -2653,7 +2666,12 @@ impl<'a> VMLogic<'a> { } } - #[cfg(not(feature = "protocol_feature_fix_contract_loading_cost"))] + /// Add a cost for loading the contract code in the VM. + /// + /// This cost is "dumb", in that it does not consider the structure of the + /// contract code, only the size. This is currently the only loading fee. + /// A smarter fee could be added, although that would have to happen slightly + /// later, as this dumb fee can be pre-charged without looking at the code. pub fn add_contract_loading_fee(&mut self, code_len: u64) -> Result<()> { self.gas_counter.pay_per(contract_loading_bytes, code_len)?; self.gas_counter.pay_base(contract_loading_base) @@ -2670,6 +2688,36 @@ impl<'a> VMLogic<'a> { let new_used_gas = self.gas_counter.used_gas(); self.gas_counter.process_gas_limit(new_burn_gas, new_used_gas) } + + pub fn checks_before_code_loading( + &mut self, + method_name: &str, + current_protocol_version: u32, + wasm_code_bytes: usize, + ) -> std::result::Result<(), VMError> { + if method_name.is_empty() { + let error = + VMError::FunctionCallError(near_vm_errors::FunctionCallError::MethodResolveError( + near_vm_errors::MethodResolveError::MethodEmptyName, + )); + return Err(error); + } + if checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + if self.add_contract_loading_fee(wasm_code_bytes as u64).is_err() { + let error = + VMError::FunctionCallError(near_vm_errors::FunctionCallError::HostError( + near_vm_errors::HostError::GasExceeded, + )); + + return Err(error); + } + } + Ok(()) + } } #[derive(Clone, PartialEq)] diff --git a/runtime/near-vm-runner-standalone/src/script.rs b/runtime/near-vm-runner-standalone/src/script.rs index e311de3c31a..4815f908bb6 100644 --- a/runtime/near-vm-runner-standalone/src/script.rs +++ b/runtime/near-vm-runner-standalone/src/script.rs @@ -129,7 +129,6 @@ impl Script { &mut external, step.vm_context.clone(), runtime_fees_config, - step.vm_context.new_gas_counter(&config.wasm_config), &step.promise_results, self.protocol_version, self.contract_cache.as_deref(), diff --git a/runtime/near-vm-runner/fuzz/fuzz_targets/runner.rs b/runtime/near-vm-runner/fuzz/fuzz_targets/runner.rs index 547c14a62b9..4920e2f9c0f 100644 --- a/runtime/near-vm-runner/fuzz/fuzz_targets/runner.rs +++ b/runtime/near-vm-runner/fuzz/fuzz_targets/runner.rs @@ -22,7 +22,6 @@ fn run_fuzz(code: &ContractCode, vm_kind: VMKind) -> VMResult { context.prepaid_gas = 10u64.pow(14); let config = VMConfig::test(); let fees = RuntimeFeesConfig::test(); - let gas_counter = context.new_gas_counter(&config); let promise_results = vec![]; @@ -33,7 +32,6 @@ fn run_fuzz(code: &ContractCode, vm_kind: VMKind) -> VMResult { &mut fake_external, context, &fees, - gas_counter, &promise_results, PROTOCOL_VERSION, None, diff --git a/runtime/near-vm-runner/src/preload.rs b/runtime/near-vm-runner/src/preload.rs index 5c789acf0bd..7970cebab9a 100644 --- a/runtime/near-vm-runner/src/preload.rs +++ b/runtime/near-vm-runner/src/preload.rs @@ -2,7 +2,6 @@ use std::sync::mpsc::{channel, Receiver, Sender}; use std::sync::Arc; use near_primitives::contract::ContractCode; -use near_vm_logic::gas_counter::GasCounter; use threadpool::ThreadPool; use near_primitives::runtime::fees::RuntimeFeesConfig; @@ -125,7 +124,6 @@ impl ContractCaller { ext: &mut dyn External, context: VMContext, fees_config: &'a RuntimeFeesConfig, - gas_counter: GasCounter, promise_results: &'a [PromiseResult], current_protocol_version: ProtocolVersion, ) -> VMResult { @@ -155,7 +153,6 @@ impl ContractCaller { context, &self.vm_config, fees_config, - gas_counter, promise_results, current_protocol_version, ) @@ -180,7 +177,6 @@ impl ContractCaller { ext, context, fees_config, - gas_counter, promise_results, current_protocol_version, ) diff --git a/runtime/near-vm-runner/src/runner.rs b/runtime/near-vm-runner/src/runner.rs index 808a0775be8..54ad6e12eaa 100644 --- a/runtime/near-vm-runner/src/runner.rs +++ b/runtime/near-vm-runner/src/runner.rs @@ -1,4 +1,3 @@ -use near_primitives::checked_feature; use near_primitives::config::VMConfig; use near_primitives::contract::ContractCode; use near_primitives::hash::CryptoHash; @@ -6,10 +5,9 @@ use near_primitives::profile::ProfileData; use near_primitives::runtime::fees::RuntimeFeesConfig; use near_primitives::types::CompiledContractCache; use near_primitives::version::ProtocolVersion; -use near_vm_errors::{FunctionCallError, MethodResolveError, VMError}; -use near_vm_logic::gas_counter::GasCounter; +use near_vm_errors::VMError; use near_vm_logic::types::PromiseResult; -use near_vm_logic::{ExtCosts, External, ReturnData, VMContext, VMLogic, VMOutcome}; +use near_vm_logic::{External, ReturnData, VMContext, VMLogic, VMOutcome}; use crate::vm_kind::VMKind; @@ -39,61 +37,6 @@ pub fn run( ) -> VMResult { let vm_kind = VMKind::for_protocol_version(current_protocol_version); - run_with_vm_kind( - code, - method_name, - ext, - context, - wasm_config, - fees_config, - promise_results, - current_protocol_version, - cache, - vm_kind, - ) -} - -/// Usually the VM is defined by the protocol version. For test and benchmarks, -/// however, it is useful to have an entry point that can specify protocol -/// version and VM kind independently. -pub(crate) fn run_with_vm_kind( - code: &ContractCode, - method_name: &str, - ext: &mut dyn External, - context: VMContext, - wasm_config: &VMConfig, - fees_config: &RuntimeFeesConfig, - promise_results: &[PromiseResult], - current_protocol_version: ProtocolVersion, - cache: Option<&dyn CompiledContractCache>, - vm_kind: VMKind, -) -> VMResult { - if method_name.is_empty() { - let error = VMError::FunctionCallError(FunctionCallError::MethodResolveError( - MethodResolveError::MethodEmptyName, - )); - return VMResult::nop_outcome(error); - } - - let mut gas_counter = context.new_gas_counter(wasm_config); - if checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - current_protocol_version - ) { - let code_bytes = code.code().len() as u64; - - if gas_counter.pay_per(ExtCosts::contract_loading_bytes, code_bytes).is_err() - || gas_counter.pay_base(ExtCosts::contract_loading_base).is_err() - { - let error = VMError::FunctionCallError(FunctionCallError::HostError( - near_vm_errors::HostError::GasExceeded, - )); - - return VMResult::not_run_with_gas_counter(gas_counter, error); - } - } - if let Some(runtime) = vm_kind.runtime(wasm_config.clone()) { runtime.run( code, @@ -101,7 +44,6 @@ pub(crate) fn run_with_vm_kind( ext, context, fees_config, - gas_counter, promise_results, current_protocol_version, cache, @@ -121,7 +63,7 @@ pub trait VM { /// /// [`VMContext::input`] will be passed to the contract entrypoint as an argument. /// - /// XXX The gas cost for contract preparation will be subtracted by the VM implementation. + /// The gas cost for contract preparation will be subtracted by the VM implementation. fn run( &self, code: &ContractCode, @@ -129,7 +71,6 @@ pub trait VM { ext: &mut dyn External, context: VMContext, fees_config: &RuntimeFeesConfig, - gas_counter: GasCounter, promise_results: &[PromiseResult], current_protocol_version: ProtocolVersion, cache: Option<&dyn CompiledContractCache>, @@ -213,28 +154,6 @@ impl VMResult { VMResult::Aborted(outcome, error) } - /// Creates an outcome with a no-op outcome other than spending some gas. - pub fn not_run_with_gas_counter(gas_counter: GasCounter, error: VMError) -> VMResult { - let burnt_gas = gas_counter.burnt_gas(); - let used_gas = gas_counter.used_gas(); - - let mut profile = gas_counter.profile_data(); - profile.compute_wasm_instruction_cost(burnt_gas); - - let outcome = VMOutcome { - // Note: Balance and storage fields are ignored on a failed outcome. - balance: 0, - storage_usage: 0, - return_data: ReturnData::None, - burnt_gas, - used_gas, - logs: Vec::new(), - profile, - action_receipts: Vec::new(), - }; - VMResult::Aborted(outcome, error) - } - /// Borrow the internal outcome, if there is one. pub fn outcome(&self) -> &VMOutcome { match self { diff --git a/runtime/near-vm-runner/src/tests.rs b/runtime/near-vm-runner/src/tests.rs index 68edcd53e7a..cf25c48e6f0 100644 --- a/runtime/near-vm-runner/src/tests.rs +++ b/runtime/near-vm-runner/src/tests.rs @@ -79,19 +79,22 @@ fn make_simple_contract_call_ex( let promise_results = vec![]; let code = ContractCode::new(code.to_vec(), None); - crate::runner::run_with_vm_kind( - &code, - method_name, - &mut fake_external, - context, - &config, - &fees, - &promise_results, - protocol_version.unwrap_or(LATEST_PROTOCOL_VERSION), - None, - vm_kind, - ) - .outcome_error() + if let Some(runtime) = vm_kind.runtime(config) { + runtime + .run( + &code, + method_name, + &mut fake_external, + context, + &fees, + &promise_results, + protocol_version.unwrap_or(LATEST_PROTOCOL_VERSION), + None, + ) + .outcome_error() + } else { + panic!("the {:?} runtime has not been enabled at compile time", vm_kind); + } } fn make_simple_contract_call_with_protocol_version_vm( diff --git a/runtime/near-vm-runner/src/tests/cache.rs b/runtime/near-vm-runner/src/tests/cache.rs index f4cb9a27cc5..a08017550b0 100644 --- a/runtime/near-vm-runner/src/tests/cache.rs +++ b/runtime/near-vm-runner/src/tests/cache.rs @@ -83,7 +83,6 @@ fn make_cached_contract_call_vm( let promise_results = vec![]; context.prepaid_gas = prepaid_gas; let code = ContractCode::new(code.to_vec(), None); - let gas_counter = context.new_gas_counter(&config); let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); runtime.run( &code, @@ -91,7 +90,6 @@ fn make_cached_contract_call_vm( &mut fake_external, context.clone(), &fees, - gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, Some(cache), diff --git a/runtime/near-vm-runner/src/tests/contract_preload.rs b/runtime/near-vm-runner/src/tests/contract_preload.rs index f420e7440fb..324bc34898c 100644 --- a/runtime/near-vm-runner/src/tests/contract_preload.rs +++ b/runtime/near-vm-runner/src/tests/contract_preload.rs @@ -112,7 +112,7 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { if preloaded { let mut requests = Vec::new(); - let mut caller = ContractCaller::new(4, vm_kind, vm_config.clone()); + let mut caller = ContractCaller::new(4, vm_kind, vm_config); for _ in 0..repeat { requests.push(ContractCallPrepareRequest { code: Arc::clone(&code1), @@ -131,7 +131,6 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { &mut fake_external, context.clone(), &fees, - context.new_gas_counter(&vm_config), &promise_results, ProtocolVersion::MAX, ); @@ -140,7 +139,7 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { errs += err; } } else { - let runtime = vm_kind.runtime(vm_config.clone()).expect("runtime is has not been compiled"); + let runtime = vm_kind.runtime(vm_config).expect("runtime is has not been compiled"); for _ in 0..repeat { let result1 = runtime.run( &code1, @@ -148,7 +147,6 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { &mut fake_external, context.clone(), &fees, - context.new_gas_counter(&vm_config), &promise_results, ProtocolVersion::MAX, cache.as_deref(), @@ -162,7 +160,6 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { &mut fake_external, context.clone(), &fees, - context.new_gas_counter(&vm_config), &promise_results, ProtocolVersion::MAX, cache.as_deref(), diff --git a/runtime/near-vm-runner/src/tests/rs_contract.rs b/runtime/near-vm-runner/src/tests/rs_contract.rs index 35ae1a6907a..7f30eddf708 100644 --- a/runtime/near-vm-runner/src/tests/rs_contract.rs +++ b/runtime/near-vm-runner/src/tests/rs_contract.rs @@ -52,15 +52,13 @@ pub fn test_read_write() { let fees = RuntimeFeesConfig::test(); let promise_results = vec![]; - let gas_counter = context.new_gas_counter(&config); - let runtime = vm_kind.runtime(config.clone()).expect("runtime has not been compiled"); + let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let result = runtime.run( &code, "write_key_value", &mut fake_external, context, &fees, - gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, @@ -68,14 +66,12 @@ pub fn test_read_write() { assert_run_result(result, 0); let context = create_context(encode(&[10u64])); - let gas_counter = context.new_gas_counter(&config); let result = runtime.run( &code, "read_value", &mut fake_external, context, &fees, - gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, @@ -95,28 +91,25 @@ pub fn test_stablized_host_function() { let fees = RuntimeFeesConfig::test(); let promise_results = vec![]; - let runtime = vm_kind.runtime(config.clone()).expect("runtime has not been compiled"); + let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let result = runtime.run( &code, "do_ripemd", &mut fake_external, context.clone(), &fees, - context.new_gas_counter(&config), &promise_results, LATEST_PROTOCOL_VERSION, None, ); assert_eq!(result.error(), None); - let gas_counter = context.new_gas_counter(&config); let result = runtime.run( &code, "do_ripemd", &mut fake_external, context, &fees, - gas_counter, &promise_results, ProtocolFeature::MathExtension.protocol_version() - 1, None, @@ -169,21 +162,10 @@ fn run_test_ext( let config = VMConfig::test(); let fees = RuntimeFeesConfig::test(); let context = create_context(input.to_vec()); - let gas_counter = context.new_gas_counter(&config); let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let (outcome, err) = runtime - .run( - &code, - method, - &mut fake_external, - context, - &fees, - gas_counter, - &[], - LATEST_PROTOCOL_VERSION, - None, - ) + .run(&code, method, &mut fake_external, context, &fees, &[], LATEST_PROTOCOL_VERSION, None) .outcome_error(); assert_eq!(outcome.profile.action_gas(), 0); @@ -276,7 +258,6 @@ pub fn test_out_of_memory() { let context = create_context(Vec::new()); let config = VMConfig::free(); let fees = RuntimeFeesConfig::free(); - let gas_counter = context.new_gas_counter(&config); let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let promise_results = vec![]; @@ -286,7 +267,6 @@ pub fn test_out_of_memory() { &mut fake_external, context, &fees, - gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, @@ -319,7 +299,6 @@ fn attach_unspent_gas_but_burn_all_gas() { let prepaid_gas = 100 * 10u64.pow(12); context.prepaid_gas = prepaid_gas; config.limit_config.max_gas_burnt = context.prepaid_gas / 3; - let gas_counter = context.new_gas_counter(&config); let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let (outcome, err) = runtime @@ -329,7 +308,6 @@ fn attach_unspent_gas_but_burn_all_gas() { &mut external, context, &fees, - gas_counter, &[], LATEST_PROTOCOL_VERSION, None, @@ -364,7 +342,6 @@ fn attach_unspent_gas_but_use_all_gas() { context.prepaid_gas = 100 * 10u64.pow(12); config.limit_config.max_gas_burnt = context.prepaid_gas / 3; - let gas_counter = context.new_gas_counter(&config); let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let (outcome, err) = runtime @@ -374,7 +351,6 @@ fn attach_unspent_gas_but_use_all_gas() { &mut external, context, &fees, - gas_counter, &[], LATEST_PROTOCOL_VERSION, None, diff --git a/runtime/near-vm-runner/src/tests/ts_contract.rs b/runtime/near-vm-runner/src/tests/ts_contract.rs index 2bcbd4bd8ca..41f124b18fb 100644 --- a/runtime/near-vm-runner/src/tests/ts_contract.rs +++ b/runtime/near-vm-runner/src/tests/ts_contract.rs @@ -20,15 +20,13 @@ pub fn test_ts_contract() { // Call method that panics. let promise_results = vec![]; - let gas_counter = context.new_gas_counter(&config); - let runtime = vm_kind.runtime(config.clone()).expect("runtime has not been compiled"); + let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); let result = runtime.run( &code, "try_panic", &mut fake_external, context, &fees, - gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, @@ -42,14 +40,12 @@ pub fn test_ts_contract() { // Call method that writes something into storage. let context = create_context(b"foo bar".to_vec()); - let gas_counter = context.new_gas_counter(&config); runtime.run( &code, "try_storage_write", &mut fake_external, context, &fees, - gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, @@ -65,14 +61,12 @@ pub fn test_ts_contract() { // Call method that reads the value from storage using registers. let context = create_context(b"foo".to_vec()); - let gas_counter = context.new_gas_counter(&config); let result = runtime.run( &code, "try_storage_read", &mut fake_external, context, &fees, - gas_counter, &promise_results, LATEST_PROTOCOL_VERSION, None, diff --git a/runtime/near-vm-runner/src/wasmer2_runner.rs b/runtime/near-vm-runner/src/wasmer2_runner.rs index b972a135ce8..33de355e5b8 100644 --- a/runtime/near-vm-runner/src/wasmer2_runner.rs +++ b/runtime/near-vm-runner/src/wasmer2_runner.rs @@ -10,7 +10,7 @@ use near_primitives::runtime::fees::RuntimeFeesConfig; use near_primitives::types::CompiledContractCache; use near_stable_hasher::StableHasher; use near_vm_errors::{CompilationError, FunctionCallError, MethodResolveError, VMError, WasmTrap}; -use near_vm_logic::gas_counter::{FastGasCounter, GasCounter}; +use near_vm_logic::gas_counter::FastGasCounter; use near_vm_logic::types::{PromiseResult, ProtocolVersion}; use near_vm_logic::{External, MemoryLike, VMConfig, VMContext, VMLogic}; use std::hash::{Hash, Hasher}; @@ -417,7 +417,6 @@ impl Wasmer2VM { ext: &mut dyn External, context: VMContext, fees_config: &'a RuntimeFeesConfig, - gas_counter: GasCounter, promise_results: &'a [PromiseResult], current_protocol_version: ProtocolVersion, ) -> VMResult { @@ -427,7 +426,6 @@ impl Wasmer2VM { context, &self.config, fees_config, - gas_counter, promise_results, memory, current_protocol_version, @@ -515,7 +513,6 @@ impl crate::runner::VM for Wasmer2VM { ext: &mut dyn External, context: VMContext, fees_config: &RuntimeFeesConfig, - gas_counter: GasCounter, promise_results: &[PromiseResult], current_protocol_version: ProtocolVersion, cache: Option<&dyn CompiledContractCache>, @@ -542,12 +539,20 @@ impl crate::runner::VM for Wasmer2VM { context, &self.config, fees_config, - gas_counter, promise_results, &mut memory, current_protocol_version, ); + let result = logic.checks_before_code_loading( + method_name, + current_protocol_version, + code.code().len(), + ); + if result.is_err() { + return VMResult::abort(logic, result.unwrap_err()); + } + let artifact = cache::wasmer2_cache::compile_module_cached_wasmer2(code, &self.config, cache); let artifact = match into_vm_result(artifact) { diff --git a/runtime/near-vm-runner/src/wasmer_runner.rs b/runtime/near-vm-runner/src/wasmer_runner.rs index 71466485c20..b13cd95af21 100644 --- a/runtime/near-vm-runner/src/wasmer_runner.rs +++ b/runtime/near-vm-runner/src/wasmer_runner.rs @@ -11,7 +11,6 @@ use near_primitives::runtime::fees::RuntimeFeesConfig; use near_primitives::types::CompiledContractCache; use near_primitives::version::ProtocolVersion; use near_vm_errors::{CompilationError, FunctionCallError, MethodResolveError, VMError, WasmTrap}; -use near_vm_logic::gas_counter::GasCounter; use near_vm_logic::types::PromiseResult; use near_vm_logic::{External, VMContext, VMLogic, VMLogicError}; use wasmer_runtime::{ImportObject, Module}; @@ -233,7 +232,6 @@ pub(crate) fn run_wasmer0_module<'a>( context: VMContext, wasm_config: &'a VMConfig, fees_config: &'a RuntimeFeesConfig, - gas_counter: GasCounter, promise_results: &'a [PromiseResult], current_protocol_version: ProtocolVersion, ) -> VMResult { @@ -251,7 +249,6 @@ pub(crate) fn run_wasmer0_module<'a>( context, wasm_config, fees_config, - gas_counter, promise_results, memory, current_protocol_version, @@ -292,7 +289,6 @@ impl crate::runner::VM for Wasmer0VM { ext: &mut dyn External, context: VMContext, fees_config: &RuntimeFeesConfig, - gas_counter: GasCounter, promise_results: &[PromiseResult], current_protocol_version: ProtocolVersion, cache: Option<&dyn CompiledContractCache>, @@ -330,12 +326,20 @@ impl crate::runner::VM for Wasmer0VM { context, &self.config, fees_config, - gas_counter, promise_results, &mut memory, current_protocol_version, ); + let result = logic.checks_before_code_loading( + method_name, + current_protocol_version, + code.code().len(), + ); + if result.is_err() { + return VMResult::abort(logic, result.unwrap_err()); + } + // TODO: consider using get_module() here, once we'll go via deployment path. let module = cache::wasmer0_cache::compile_module_cached_wasmer0(code, &self.config, cache); let module = match into_vm_result(module) { diff --git a/runtime/near-vm-runner/src/wasmtime_runner.rs b/runtime/near-vm-runner/src/wasmtime_runner.rs index 82b0a0c1283..fb0f0f36ea5 100644 --- a/runtime/near-vm-runner/src/wasmtime_runner.rs +++ b/runtime/near-vm-runner/src/wasmtime_runner.rs @@ -13,7 +13,6 @@ use near_vm_errors::{ CompilationError, FunctionCallError, MethodResolveError, PrepareError, VMError, VMLogicError, WasmTrap, }; -use near_vm_logic::gas_counter::GasCounter; use near_vm_logic::types::PromiseResult; use near_vm_logic::{External, MemoryLike, VMContext, VMLogic}; use std::cell::RefCell; @@ -187,7 +186,6 @@ impl crate::runner::VM for WasmtimeVM { ext: &mut dyn External, context: VMContext, fees_config: &RuntimeFeesConfig, - gas_counter: GasCounter, promise_results: &[PromiseResult], current_protocol_version: ProtocolVersion, _cache: Option<&dyn CompiledContractCache>, @@ -214,12 +212,20 @@ impl crate::runner::VM for WasmtimeVM { context, &self.config, fees_config, - gas_counter, promise_results, &mut memory, current_protocol_version, ); + let result = logic.checks_before_code_loading( + method_name, + current_protocol_version, + code.code().len(), + ); + if result.is_err() { + return VMResult::abort(logic, result.unwrap_err()); + } + let prepared_code = match prepare::prepare_contract(code.code(), &self.config) { Ok(code) => code, Err(err) => return VMResult::abort(logic, VMError::from(err)), diff --git a/runtime/runtime-params-estimator/src/function_call.rs b/runtime/runtime-params-estimator/src/function_call.rs index 6036ac74bd1..70753cef88c 100644 --- a/runtime/runtime-params-estimator/src/function_call.rs +++ b/runtime/runtime-params-estimator/src/function_call.rs @@ -1,7 +1,6 @@ use crate::config::GasMetric; use crate::gas_cost::GasCost; use crate::least_squares::least_squares_method; -use crate::utils::estimator_gas_counter; use crate::vm_estimator::create_context; use near_primitives::contract::ContractCode; use near_primitives::runtime::config_store::RuntimeConfigStore; @@ -90,7 +89,6 @@ pub fn compute_function_call_cost( &mut fake_external, fake_context.clone(), &fees, - estimator_gas_counter(&runtime_config.wasm_config), &promise_results, protocol_version, cache, @@ -106,7 +104,6 @@ pub fn compute_function_call_cost( &mut fake_external, fake_context.clone(), &fees, - estimator_gas_counter(&runtime_config.wasm_config), &promise_results, protocol_version, cache, diff --git a/runtime/runtime-params-estimator/src/gas_metering.rs b/runtime/runtime-params-estimator/src/gas_metering.rs index 2b3741dd88c..a5f210528e8 100644 --- a/runtime/runtime-params-estimator/src/gas_metering.rs +++ b/runtime/runtime-params-estimator/src/gas_metering.rs @@ -1,6 +1,5 @@ use crate::config::Config; use crate::gas_cost::{GasCost, LeastSquaresTolerance}; -use crate::utils::estimator_gas_counter; use crate::vm_estimator::create_context; use near_primitives::config::VMConfig; use near_primitives::contract::ContractCode; @@ -152,7 +151,6 @@ pub(crate) fn compute_gas_metering_cost(config: &Config, contract: &ContractCode &mut fake_external, fake_context.clone(), &fees, - estimator_gas_counter(&runtime_config.wasm_config), &promise_results, PROTOCOL_VERSION, cache, @@ -172,7 +170,6 @@ pub(crate) fn compute_gas_metering_cost(config: &Config, contract: &ContractCode &mut fake_external, fake_context.clone(), &fees, - estimator_gas_counter(&runtime_config.wasm_config), &promise_results, PROTOCOL_VERSION, cache, @@ -189,7 +186,6 @@ pub(crate) fn compute_gas_metering_cost(config: &Config, contract: &ContractCode &mut fake_external, fake_context.clone(), &fees, - estimator_gas_counter(&runtime_config.wasm_config), &promise_results, PROTOCOL_VERSION, cache, @@ -206,7 +202,6 @@ pub(crate) fn compute_gas_metering_cost(config: &Config, contract: &ContractCode &mut fake_external, fake_context.clone(), &fees, - estimator_gas_counter(&runtime_config.wasm_config), &promise_results, PROTOCOL_VERSION, cache, diff --git a/runtime/runtime-params-estimator/src/lib.rs b/runtime/runtime-params-estimator/src/lib.rs index e91e551ba26..c1bad5f69f6 100644 --- a/runtime/runtime-params-estimator/src/lib.rs +++ b/runtime/runtime-params-estimator/src/lib.rs @@ -107,7 +107,6 @@ use crate::estimator_context::EstimatorContext; use crate::gas_cost::GasCost; use crate::rocksdb::{rocks_db_inserts_cost, rocks_db_read_cost}; use crate::transaction_builder::TransactionBuilder; -use crate::utils::estimator_gas_counter; use crate::vm_estimator::create_context; pub use crate::cost::Cost; @@ -750,7 +749,6 @@ fn wasm_instruction(ctx: &mut EstimatorContext) -> GasCost { &mut fake_external, context, &fees, - estimator_gas_counter(&config), &promise_results, PROTOCOL_VERSION, Some(&cache), diff --git a/runtime/runtime-params-estimator/src/utils.rs b/runtime/runtime-params-estimator/src/utils.rs index 02b393004fa..a7a6414d1b8 100644 --- a/runtime/runtime-params-estimator/src/utils.rs +++ b/runtime/runtime-params-estimator/src/utils.rs @@ -8,8 +8,7 @@ use std::collections::HashMap; use near_primitives::transaction::SignedTransaction; use near_primitives::types::AccountId; -use near_vm_logic::gas_counter::GasCounter; -use near_vm_logic::{ExtCosts, VMConfig}; +use near_vm_logic::ExtCosts; use rand::distributions::Alphanumeric; use rand::Rng; use rand_xorshift::XorShiftRng; @@ -296,17 +295,6 @@ pub(crate) fn generate_data_only_contract(data_size: usize) -> Vec { wat::parse_str(wat_code).unwrap() } -pub(crate) fn estimator_gas_counter(wasm_config: &VMConfig) -> GasCounter { - let max_gas = wasm_config.limit_config.max_gas_burnt; - GasCounter::new( - wasm_config.ext_costs.clone(), - max_gas, - wasm_config.regular_op_cost, - max_gas, - false, - ) -} - #[cfg(test)] mod test { use super::percentiles; From eb65475a5e15073c5a359bc8705c83813db56cf7 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 12:03:38 +0200 Subject: [PATCH 05/16] Use cfg over checked feature --- runtime/near-vm-runner/src/tests.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/runtime/near-vm-runner/src/tests.rs b/runtime/near-vm-runner/src/tests.rs index 4dd9934f252..e71ea463d83 100644 --- a/runtime/near-vm-runner/src/tests.rs +++ b/runtime/near-vm-runner/src/tests.rs @@ -8,7 +8,6 @@ mod wasm_validation; use crate::vm_kind::VMKind; -use near_primitives::checked_feature; use near_primitives::contract::ContractCode; use near_primitives::runtime::config_store::RuntimeConfigStore; use near_primitives::runtime::fees::RuntimeFeesConfig; @@ -139,11 +138,7 @@ fn gas_and_error_match( /// Includes some hard-coded parameter values that would have to be updated /// if they change in the future. fn prepaid_loading_gas(bytes: usize) -> u64 { - if checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - LATEST_PROTOCOL_VERSION - ) { + if cfg!(feature = "protocol_feature_fix_contract_loading_cost") { 35445963 + bytes as u64 * 216750 } else { 0 From c5d962b41ab017c212b4336967c01d8ed31f446c Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 12:09:06 +0200 Subject: [PATCH 06/16] Remove dead code --- runtime/near-vm-logic/src/context.rs | 15 --------------- runtime/near-vm-runner/src/runner.rs | 1 - 2 files changed, 16 deletions(-) diff --git a/runtime/near-vm-logic/src/context.rs b/runtime/near-vm-logic/src/context.rs index b2119791c59..cff4a0a6bb7 100644 --- a/runtime/near-vm-logic/src/context.rs +++ b/runtime/near-vm-logic/src/context.rs @@ -1,6 +1,4 @@ -use crate::gas_counter::GasCounter; use crate::types::PublicKey; -use near_primitives::config::VMConfig; use near_primitives_core::config::ViewConfig; use near_primitives_core::serialize::u64_dec_format; use near_primitives_core::types::{ @@ -69,17 +67,4 @@ impl VMContext { pub fn is_view(&self) -> bool { self.view_config.is_some() } - pub fn new_gas_counter(&self, wasm_config: &VMConfig) -> GasCounter { - let max_gas_burnt = match self.view_config { - Some(ViewConfig { max_gas_burnt: max_gas_burnt_view }) => max_gas_burnt_view, - None => wasm_config.limit_config.max_gas_burnt, - }; - GasCounter::new( - wasm_config.ext_costs.clone(), - max_gas_burnt, - wasm_config.regular_op_cost, - self.prepaid_gas, - self.is_view(), - ) - } } diff --git a/runtime/near-vm-runner/src/runner.rs b/runtime/near-vm-runner/src/runner.rs index 54ad6e12eaa..95b8220d185 100644 --- a/runtime/near-vm-runner/src/runner.rs +++ b/runtime/near-vm-runner/src/runner.rs @@ -36,7 +36,6 @@ pub fn run( cache: Option<&dyn CompiledContractCache>, ) -> VMResult { let vm_kind = VMKind::for_protocol_version(current_protocol_version); - if let Some(runtime) = vm_kind.runtime(wasm_config.clone()) { runtime.run( code, From f2688e23cc5b19ee3ef03b2c1a8592a883326c36 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 12:25:03 +0200 Subject: [PATCH 07/16] after_code_loading --- runtime/near-vm-logic/src/logic.rs | 28 +++++++++++++++++-- runtime/near-vm-runner/src/wasmer2_runner.rs | 21 ++++---------- runtime/near-vm-runner/src/wasmer_runner.rs | 21 ++++---------- runtime/near-vm-runner/src/wasmtime_runner.rs | 21 ++++---------- 4 files changed, 41 insertions(+), 50 deletions(-) diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index 229be355d4e..810f2d4b38b 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -128,7 +128,6 @@ impl<'a> VMLogic<'a> { context.prepaid_gas, context.is_view(), ); - Self { ext, context, @@ -2699,7 +2698,10 @@ impl<'a> VMLogic<'a> { self.gas_counter.process_gas_limit(new_burn_gas, new_used_gas) } - pub fn checks_before_code_loading( + /// Perform VM independent checks that happen after the instantiation of + /// VMLogic but before loading the code. This includes pre-charging gas + /// costs for loading the code. + pub fn before_code_loading( &mut self, method_name: &str, current_protocol_version: u32, @@ -2728,6 +2730,28 @@ impl<'a> VMLogic<'a> { } Ok(()) } + + /// Legacy code to preserve old gas charging behaviour in old protocol versions. + pub fn after_code_loading( + &mut self, + current_protocol_version: u32, + wasm_code_bytes: usize, + ) -> std::result::Result<(), VMError> { + if !checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + if self.add_contract_loading_fee(wasm_code_bytes as u64).is_err() { + return Err(VMError::FunctionCallError( + near_vm_errors::FunctionCallError::HostError( + near_vm_errors::HostError::GasExceeded, + ), + )); + } + } + Ok(()) + } } #[derive(Clone, PartialEq)] diff --git a/runtime/near-vm-runner/src/wasmer2_runner.rs b/runtime/near-vm-runner/src/wasmer2_runner.rs index 61fcb68e84d..7129b9e77ce 100644 --- a/runtime/near-vm-runner/src/wasmer2_runner.rs +++ b/runtime/near-vm-runner/src/wasmer2_runner.rs @@ -536,11 +536,8 @@ impl crate::runner::VM for Wasmer2VM { current_protocol_version, ); - let result = logic.checks_before_code_loading( - method_name, - current_protocol_version, - code.code().len(), - ); + let result = + logic.before_code_loading(method_name, current_protocol_version, code.code().len()); if result.is_err() { return VMResult::abort(logic, result.unwrap_err()); } @@ -554,17 +551,9 @@ impl crate::runner::VM for Wasmer2VM { } }; - if !checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - current_protocol_version - ) { - if logic.add_contract_loading_fee(code.code().len() as u64).is_err() { - let error = VMError::FunctionCallError(FunctionCallError::HostError( - near_vm_errors::HostError::GasExceeded, - )); - return VMResult::abort(logic, error); - } + let result = logic.after_code_loading(current_protocol_version, code.code().len()); + if result.is_err() { + return VMResult::abort(logic, result.unwrap_err()); } let import = imports::wasmer2::build( vmmemory, diff --git a/runtime/near-vm-runner/src/wasmer_runner.rs b/runtime/near-vm-runner/src/wasmer_runner.rs index b13cd95af21..1e6057acd05 100644 --- a/runtime/near-vm-runner/src/wasmer_runner.rs +++ b/runtime/near-vm-runner/src/wasmer_runner.rs @@ -331,11 +331,8 @@ impl crate::runner::VM for Wasmer0VM { current_protocol_version, ); - let result = logic.checks_before_code_loading( - method_name, - current_protocol_version, - code.code().len(), - ); + let result = + logic.before_code_loading(method_name, current_protocol_version, code.code().len()); if result.is_err() { return VMResult::abort(logic, result.unwrap_err()); } @@ -354,17 +351,9 @@ impl crate::runner::VM for Wasmer0VM { Err(err) => return VMResult::abort(logic, err), }; - if !checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - current_protocol_version - ) { - if logic.add_contract_loading_fee(code.code().len() as u64).is_err() { - let err = VMError::FunctionCallError(FunctionCallError::HostError( - near_vm_errors::HostError::GasExceeded, - )); - return VMResult::abort(logic, err); - } + let result = logic.after_code_loading(current_protocol_version, code.code().len()); + if result.is_err() { + return VMResult::abort(logic, result.unwrap_err()); } let import_object = diff --git a/runtime/near-vm-runner/src/wasmtime_runner.rs b/runtime/near-vm-runner/src/wasmtime_runner.rs index fb0f0f36ea5..b2b21cb40c4 100644 --- a/runtime/near-vm-runner/src/wasmtime_runner.rs +++ b/runtime/near-vm-runner/src/wasmtime_runner.rs @@ -217,11 +217,8 @@ impl crate::runner::VM for WasmtimeVM { current_protocol_version, ); - let result = logic.checks_before_code_loading( - method_name, - current_protocol_version, - code.code().len(), - ); + let result = + logic.before_code_loading(method_name, current_protocol_version, code.code().len()); if result.is_err() { return VMResult::abort(logic, result.unwrap_err()); } @@ -236,17 +233,9 @@ impl crate::runner::VM for WasmtimeVM { }; let mut linker = Linker::new(&engine); - if !checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - current_protocol_version - ) { - if logic.add_contract_loading_fee(code.code().len() as u64).is_err() { - let err = VMError::FunctionCallError(FunctionCallError::HostError( - near_vm_errors::HostError::GasExceeded, - )); - return VMResult::abort(logic, err); - } + let result = logic.after_code_loading(current_protocol_version, code.code().len()); + if result.is_err() { + return VMResult::abort(logic, result.unwrap_err()); } // Unfortunately, due to the Wasmtime implementation we have to do tricks with the From 1f58031ff432f3d01fd205b82cc7484d10e2a0ee Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 12:33:44 +0200 Subject: [PATCH 08/16] Undo unrelated code change --- runtime/near-vm-runner-standalone/src/script.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/runtime/near-vm-runner-standalone/src/script.rs b/runtime/near-vm-runner-standalone/src/script.rs index 4815f908bb6..1185a82dd10 100644 --- a/runtime/near-vm-runner-standalone/src/script.rs +++ b/runtime/near-vm-runner-standalone/src/script.rs @@ -117,8 +117,7 @@ impl Script { } let config_store = RuntimeConfigStore::new(None); - let config = &config_store.get_config(self.protocol_version); - let runtime_fees_config = &config.transaction_costs; + let runtime_fees_config = &config_store.get_config(self.protocol_version).transaction_costs; let mut outcomes = Vec::new(); if let Some(runtime) = self.vm_kind.runtime(self.vm_config.clone()) { for step in &self.steps { From be878b5945c294df6eb07d8f108de261e1b9dcf5 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 13:23:42 +0200 Subject: [PATCH 09/16] Fix test compilation errors after merge --- .../src/tests/runtime_errors.rs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/runtime/near-vm-runner/src/tests/runtime_errors.rs b/runtime/near-vm-runner/src/tests/runtime_errors.rs index 50b11327ea5..9550a7beb75 100644 --- a/runtime/near-vm-runner/src/tests/runtime_errors.rs +++ b/runtime/near-vm-runner/src/tests/runtime_errors.rs @@ -932,7 +932,7 @@ mod fix_contract_loading_cost_protocol_upgrade { fn test_fn_loading_gas_protocol_upgrade() { with_vm_variants(|vm_kind: VMKind| { let (code, exec_gas) = almost_trivial_contract_and_exec_gas(); - let loading_gas = prepaid_loading_gas(code.len()).unwrap(); + let loading_gas = prepaid_loading_gas(code.len()); let total_gas = exec_gas + loading_gas; let res = make_simple_contract_call_ex( @@ -942,7 +942,7 @@ mod fix_contract_loading_cost_protocol_upgrade { Some(ProtocolFeature::FixContractLoadingCost.protocol_version() - 1), total_gas + 1, ); - gas_and_error_match(res, Some(total_gas), None); + gas_and_error_match(res, total_gas, None); let res = make_simple_contract_call_ex( &code, "main", @@ -950,7 +950,7 @@ mod fix_contract_loading_cost_protocol_upgrade { Some(ProtocolFeature::FixContractLoadingCost.protocol_version()), total_gas + 1, ); - gas_and_error_match(res, Some(total_gas), None); + gas_and_error_match(res, total_gas, None); }); } @@ -960,7 +960,7 @@ mod fix_contract_loading_cost_protocol_upgrade { fn test_fn_loading_gas_protocol_upgrade_exceed_loading() { with_vm_variants(|vm_kind: VMKind| { let (code, _exec_gas) = almost_trivial_contract_and_exec_gas(); - let loading_gas = prepaid_loading_gas(code.len()).unwrap(); + let loading_gas = prepaid_loading_gas(code.len()); let res = make_simple_contract_call_ex( &code, @@ -971,7 +971,7 @@ mod fix_contract_loading_cost_protocol_upgrade { ); gas_and_error_match( res, - Some(loading_gas), + loading_gas, Some(VMError::FunctionCallError(FunctionCallError::HostError( HostError::GasExceeded, ))), @@ -985,7 +985,7 @@ mod fix_contract_loading_cost_protocol_upgrade { ); gas_and_error_match( res, - Some(loading_gas), + loading_gas, Some(VMError::FunctionCallError(FunctionCallError::HostError( HostError::GasExceeded, ))), @@ -999,7 +999,7 @@ mod fix_contract_loading_cost_protocol_upgrade { fn test_fn_loading_gas_protocol_upgrade_exceed_executing() { with_vm_variants(|vm_kind: VMKind| { let (code, exec_gas) = almost_trivial_contract_and_exec_gas(); - let loading_gas = prepaid_loading_gas(code.len()).unwrap(); + let loading_gas = prepaid_loading_gas(code.len()); let total_gas = exec_gas + loading_gas; let res = make_simple_contract_call_ex( @@ -1011,7 +1011,7 @@ mod fix_contract_loading_cost_protocol_upgrade { ); gas_and_error_match( res, - Some(total_gas - 1), + total_gas - 1, Some(VMError::FunctionCallError(FunctionCallError::HostError( HostError::GasExceeded, ))), @@ -1025,7 +1025,7 @@ mod fix_contract_loading_cost_protocol_upgrade { ); gas_and_error_match( res, - Some(total_gas - 1), + total_gas - 1, Some(VMError::FunctionCallError(FunctionCallError::HostError( HostError::GasExceeded, ))), @@ -1056,7 +1056,7 @@ mod fix_contract_loading_cost_protocol_upgrade { ); // This is the first property the tests check: Gas cost before // compilation errors must remain zero for old versions. - let expected_gas = None; + let expected_gas = 0; gas_and_error_match( res, expected_gas, @@ -1165,7 +1165,7 @@ mod fix_contract_loading_cost_protocol_upgrade { new_version, vm_kind, ); - let expected_gas = prepaid_loading_gas(code.len()).unwrap(); + let expected_gas = prepaid_loading_gas(code.len()); assert_eq!(expected_gas, outcome.used_gas); assert_eq!(expected_gas, outcome.burnt_gas); error From e65de48ffb22675d6e3813ee65ceaa776b042935 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 14:50:19 +0200 Subject: [PATCH 10/16] Add missing feature in near-vm-logic --- runtime/near-vm-logic/Cargo.toml | 3 +++ runtime/near-vm-logic/src/logic.rs | 1 - runtime/near-vm-runner/Cargo.toml | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/runtime/near-vm-logic/Cargo.toml b/runtime/near-vm-logic/Cargo.toml index f69875d1b3d..6166b5a188e 100644 --- a/runtime/near-vm-logic/Cargo.toml +++ b/runtime/near-vm-logic/Cargo.toml @@ -39,6 +39,9 @@ serde_json = { version = "1", features = ["preserve_order"] } [features] default = [] +protocol_feature_fix_contract_loading_cost = [ + "near-primitives/protocol_feature_fix_contract_loading_cost", +] protocol_feature_alt_bn128 = [ "bn", "near-primitives-core/protocol_feature_alt_bn128", diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index 810f2d4b38b..fc7d8e518ed 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -2724,7 +2724,6 @@ impl<'a> VMLogic<'a> { VMError::FunctionCallError(near_vm_errors::FunctionCallError::HostError( near_vm_errors::HostError::GasExceeded, )); - return Err(error); } } diff --git a/runtime/near-vm-runner/Cargo.toml b/runtime/near-vm-runner/Cargo.toml index 7750acc5ea1..3241d8de7a8 100644 --- a/runtime/near-vm-runner/Cargo.toml +++ b/runtime/near-vm-runner/Cargo.toml @@ -99,6 +99,7 @@ protocol_feature_alt_bn128 = [ ] protocol_feature_fix_contract_loading_cost = [ "near-primitives/protocol_feature_fix_contract_loading_cost", + "near-vm-logic/protocol_feature_fix_contract_loading_cost", ] nightly = [ From a1280c95ee5a8e98ed63b0b7642bc213e4ebb448 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 16:02:08 +0200 Subject: [PATCH 11/16] Fix gas expectation for existing test Reason: It is an expected change that compilation failures (or cache errors for that matter) will burn the loading gas in new versions. --- runtime/near-vm-runner/src/tests/cache.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/runtime/near-vm-runner/src/tests/cache.rs b/runtime/near-vm-runner/src/tests/cache.rs index 68a36b08f02..a3d24e496a2 100644 --- a/runtime/near-vm-runner/src/tests/cache.rs +++ b/runtime/near-vm-runner/src/tests/cache.rs @@ -53,7 +53,8 @@ fn test_does_not_cache_io_error() { cache.set_read_fault(true); let result = make_cached_contract_call_vm(&cache, &code, "main", prepaid_gas, vm_kind); - assert_eq!(result.outcome().used_gas, 0); + let expected_gas = crate::tests::prepaid_loading_gas(code.len()); + assert_eq!(result.outcome().used_gas, expected_gas); assert_matches!( result.error(), Some(&VMError::CacheError(near_vm_errors::CacheError::ReadError)) @@ -61,7 +62,7 @@ fn test_does_not_cache_io_error() { cache.set_write_fault(true); let result = make_cached_contract_call_vm(&cache, &code, "main", prepaid_gas, vm_kind); - assert_eq!(result.outcome().used_gas, 0); + assert_eq!(result.outcome().used_gas, expected_gas); assert_matches!( result.error(), Some(&VMError::CacheError(near_vm_errors::CacheError::WriteError)) From 53c3382641218a1af3c0e549349a846582bc32f4 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 16:47:11 +0200 Subject: [PATCH 12/16] Clean up according to review suggestions --- runtime/near-vm-logic/src/logic.rs | 12 ++-- runtime/near-vm-runner/src/runner.rs | 19 +++++ runtime/near-vm-runner/src/wasmer2_runner.rs | 27 +++----- runtime/near-vm-runner/src/wasmer_runner.rs | 27 +++----- runtime/near-vm-runner/src/wasmtime_runner.rs | 69 +++++++------------ 5 files changed, 74 insertions(+), 80 deletions(-) diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index fc7d8e518ed..c1037420908 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -2698,10 +2698,12 @@ impl<'a> VMLogic<'a> { self.gas_counter.process_gas_limit(new_burn_gas, new_used_gas) } - /// Perform VM independent checks that happen after the instantiation of - /// VMLogic but before loading the code. This includes pre-charging gas - /// costs for loading the code. - pub fn before_code_loading( + /// VM independent setup before performing executable loading. + /// + /// Does VM independent checks that happen after the instantiation of + /// VMLogic but before loading the executable. This includes pre-charging gas + /// costs for loading the executable, which depends on the size of the WASM code. + pub fn before_loading_executable( &mut self, method_name: &str, current_protocol_version: u32, @@ -2731,7 +2733,7 @@ impl<'a> VMLogic<'a> { } /// Legacy code to preserve old gas charging behaviour in old protocol versions. - pub fn after_code_loading( + pub fn after_loading_executable( &mut self, current_protocol_version: u32, wasm_code_bytes: usize, diff --git a/runtime/near-vm-runner/src/runner.rs b/runtime/near-vm-runner/src/runner.rs index 95b8220d185..91fc02f30af 100644 --- a/runtime/near-vm-runner/src/runner.rs +++ b/runtime/near-vm-runner/src/runner.rs @@ -1,3 +1,4 @@ +use near_primitives::checked_feature; use near_primitives::config::VMConfig; use near_primitives::contract::ContractCode; use near_primitives::hash::CryptoHash; @@ -153,6 +154,24 @@ impl VMResult { VMResult::Aborted(outcome, error) } + /// Like `VMResult::abort()` but without feature `FixContractLoadingCost` it + /// will return a NOP outcome. This is used for backwards-compatibility only. + pub fn abort_but_nop_outcome_in_old_protocol( + logic: VMLogic, + error: VMError, + current_protocol_version: u32, + ) -> VMResult { + if checked_feature!( + "protocol_feature_fix_contract_loading_cost", + FixContractLoadingCost, + current_protocol_version + ) { + VMResult::abort(logic, error) + } else { + VMResult::nop_outcome(error) + } + } + /// Borrow the internal outcome, if there is one. pub fn outcome(&self) -> &VMOutcome { match self { diff --git a/runtime/near-vm-runner/src/wasmer2_runner.rs b/runtime/near-vm-runner/src/wasmer2_runner.rs index 7129b9e77ce..b94161889a4 100644 --- a/runtime/near-vm-runner/src/wasmer2_runner.rs +++ b/runtime/near-vm-runner/src/wasmer2_runner.rs @@ -4,7 +4,6 @@ use crate::prepare::WASM_FEATURES; use crate::runner::VMResult; use crate::{cache, imports}; use memoffset::offset_of; -use near_primitives::checked_feature; use near_primitives::contract::ContractCode; use near_primitives::runtime::fees::RuntimeFeesConfig; use near_primitives::types::CompiledContractCache; @@ -537,9 +536,9 @@ impl crate::runner::VM for Wasmer2VM { ); let result = - logic.before_code_loading(method_name, current_protocol_version, code.code().len()); - if result.is_err() { - return VMResult::abort(logic, result.unwrap_err()); + logic.before_loading_executable(method_name, current_protocol_version, code.code().len()); + if let Err(e) = result { + return VMResult::abort(logic, e); } let artifact = @@ -551,9 +550,9 @@ impl crate::runner::VM for Wasmer2VM { } }; - let result = logic.after_code_loading(current_protocol_version, code.code().len()); - if result.is_err() { - return VMResult::abort(logic, result.unwrap_err()); + let result = logic.after_loading_executable(current_protocol_version, code.code().len()); + if let Err(e) = result { + return VMResult::abort(logic, e); } let import = imports::wasmer2::build( vmmemory, @@ -562,15 +561,11 @@ impl crate::runner::VM for Wasmer2VM { artifact.engine(), ); if let Err(e) = get_entrypoint_index(&*artifact, method_name) { - if checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - current_protocol_version - ) { - return VMResult::abort(logic, e); - } else { - return VMResult::nop_outcome(e); - } + return VMResult::abort_but_nop_outcome_in_old_protocol( + logic, + e, + current_protocol_version, + ); } match self.run_method(&artifact, import, method_name) { Ok(()) => VMResult::ok(logic), diff --git a/runtime/near-vm-runner/src/wasmer_runner.rs b/runtime/near-vm-runner/src/wasmer_runner.rs index 1e6057acd05..c162459ecee 100644 --- a/runtime/near-vm-runner/src/wasmer_runner.rs +++ b/runtime/near-vm-runner/src/wasmer_runner.rs @@ -4,7 +4,6 @@ use crate::memory::WasmerMemory; use crate::prepare::WASM_FEATURES; use crate::runner::VMResult; use crate::{cache, imports}; -use near_primitives::checked_feature; use near_primitives::config::VMConfig; use near_primitives::contract::ContractCode; use near_primitives::runtime::fees::RuntimeFeesConfig; @@ -332,9 +331,9 @@ impl crate::runner::VM for Wasmer0VM { ); let result = - logic.before_code_loading(method_name, current_protocol_version, code.code().len()); - if result.is_err() { - return VMResult::abort(logic, result.unwrap_err()); + logic.before_loading_executable(method_name, current_protocol_version, code.code().len()); + if let Err(e) = result { + return VMResult::abort(logic, e); } // TODO: consider using get_module() here, once we'll go via deployment path. @@ -351,24 +350,20 @@ impl crate::runner::VM for Wasmer0VM { Err(err) => return VMResult::abort(logic, err), }; - let result = logic.after_code_loading(current_protocol_version, code.code().len()); - if result.is_err() { - return VMResult::abort(logic, result.unwrap_err()); + let result = logic.after_loading_executable(current_protocol_version, code.code().len()); + if let Err(e) = result { + return VMResult::abort(logic, e); } let import_object = imports::wasmer::build(memory_copy, &mut logic, current_protocol_version); if let Err(e) = check_method(&module, method_name) { - if checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - current_protocol_version - ) { - return VMResult::abort(logic, e); - } else { - return VMResult::nop_outcome(e); - } + return VMResult::abort_but_nop_outcome_in_old_protocol( + logic, + e, + current_protocol_version, + ); } match run_method(&module, &import_object, method_name) { diff --git a/runtime/near-vm-runner/src/wasmtime_runner.rs b/runtime/near-vm-runner/src/wasmtime_runner.rs index b2b21cb40c4..372b7753bca 100644 --- a/runtime/near-vm-runner/src/wasmtime_runner.rs +++ b/runtime/near-vm-runner/src/wasmtime_runner.rs @@ -2,7 +2,6 @@ use crate::errors::IntoVMError; use crate::prepare::WASM_FEATURES; use crate::runner::VMResult; use crate::{imports, prepare}; -use near_primitives::checked_feature; use near_primitives::config::VMConfig; use near_primitives::contract::ContractCode; use near_primitives::hash::CryptoHash; @@ -218,9 +217,9 @@ impl crate::runner::VM for WasmtimeVM { ); let result = - logic.before_code_loading(method_name, current_protocol_version, code.code().len()); - if result.is_err() { - return VMResult::abort(logic, result.unwrap_err()); + logic.before_loading_executable(method_name, current_protocol_version, code.code().len()); + if let Err(e) = result { + return VMResult::abort(logic, e); } let prepared_code = match prepare::prepare_contract(code.code(), &self.config) { @@ -233,9 +232,9 @@ impl crate::runner::VM for WasmtimeVM { }; let mut linker = Linker::new(&engine); - let result = logic.after_code_loading(current_protocol_version, code.code().len()); - if result.is_err() { - return VMResult::abort(logic, result.unwrap_err()); + let result = logic.after_loading_executable(current_protocol_version, code.code().len()); + if let Err(e) = result { + return VMResult::abort(logic, e); } // Unfortunately, due to the Wasmtime implementation we have to do tricks with the @@ -250,45 +249,33 @@ impl crate::runner::VM for WasmtimeVM { VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodInvalidSignature, )); - if checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - current_protocol_version - ) { - return VMResult::abort(logic, err); - } else { - return VMResult::nop_outcome(err); - } + return VMResult::abort_but_nop_outcome_in_old_protocol( + logic, + err, + current_protocol_version, + ); } } _ => { let err = VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodNotFound, )); - if checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - current_protocol_version - ) { - return VMResult::abort(logic, err); - } else { - return VMResult::nop_outcome(err); - } + return VMResult::abort_but_nop_outcome_in_old_protocol( + logic, + err, + current_protocol_version, + ); } }, None => { let err = VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodNotFound, )); - if checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - current_protocol_version - ) { - return VMResult::abort(logic, err); - } else { - return VMResult::nop_outcome(err); - } + return VMResult::abort_but_nop_outcome_in_old_protocol( + logic, + err, + current_protocol_version, + ); } } match linker.instantiate(&mut store, &module) { @@ -304,15 +291,11 @@ impl crate::runner::VM for WasmtimeVM { let err = VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodNotFound, )); - if checked_feature!( - "protocol_feature_fix_contract_loading_cost", - FixContractLoadingCost, - current_protocol_version - ) { - return VMResult::abort(logic, err); - } else { - return VMResult::nop_outcome(err); - } + return VMResult::abort_but_nop_outcome_in_old_protocol( + logic, + err, + current_protocol_version, + ); } }, Err(err) => VMResult::abort(logic, err.into_vm_error()), From a471dcd54c52847831a5c9632ce0cc5089dc59c5 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 17:50:23 +0200 Subject: [PATCH 13/16] Reviewer suggestions --- runtime/near-vm-logic/src/logic.rs | 2 +- runtime/near-vm-runner/src/tests.rs | 37 ++++++++++++++--------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index c1037420908..a78afea8d9e 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -2698,7 +2698,7 @@ impl<'a> VMLogic<'a> { self.gas_counter.process_gas_limit(new_burn_gas, new_used_gas) } - /// VM independent setup before performing executable loading. + /// VM independent setup before loading the executable. /// /// Does VM independent checks that happen after the instantiation of /// VMLogic but before loading the executable. This includes pre-charging gas diff --git a/runtime/near-vm-runner/src/tests.rs b/runtime/near-vm-runner/src/tests.rs index e71ea463d83..a7f920b128c 100644 --- a/runtime/near-vm-runner/src/tests.rs +++ b/runtime/near-vm-runner/src/tests.rs @@ -78,22 +78,19 @@ fn make_simple_contract_call_ex( let promise_results = vec![]; let code = ContractCode::new(code.to_vec(), None); - if let Some(runtime) = vm_kind.runtime(config) { - runtime - .run( - &code, - method_name, - &mut fake_external, - context, - &fees, - &promise_results, - protocol_version.unwrap_or(LATEST_PROTOCOL_VERSION), - None, - ) - .outcome_error() - } else { - panic!("the {:?} runtime has not been enabled at compile time", vm_kind); - } + let runtime = vm_kind.runtime(config).expect("runtime has not been compiled"); + runtime + .run( + &code, + method_name, + &mut fake_external, + context, + &fees, + &promise_results, + protocol_version.unwrap_or(LATEST_PROTOCOL_VERSION), + None, + ) + .outcome_error() } fn make_simple_contract_call_with_protocol_version_vm( @@ -135,11 +132,13 @@ fn gas_and_error_match( } /// Small helper to compute expected loading gas cost charged before loading. -/// Includes some hard-coded parameter values that would have to be updated -/// if they change in the future. +/// +/// Includes hard-coded value for runtime parameter values +/// `wasm_contract_loading_base` and `wasm_contract_loading_bytes` which would +/// have to be updated if they change in the future. fn prepaid_loading_gas(bytes: usize) -> u64 { if cfg!(feature = "protocol_feature_fix_contract_loading_cost") { - 35445963 + bytes as u64 * 216750 + 35_445_963 + bytes as u64 * 21_6750 } else { 0 } From 3d5bcf0a31b4b54f265d936638db0c89e5a1bc10 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 17:57:25 +0200 Subject: [PATCH 14/16] Update comment on `add_contract_loading_fee` --- runtime/near-vm-logic/src/logic.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index a78afea8d9e..5af22a5f00a 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -2677,10 +2677,11 @@ impl<'a> VMLogic<'a> { /// Add a cost for loading the contract code in the VM. /// - /// This cost is "dumb", in that it does not consider the structure of the - /// contract code, only the size. This is currently the only loading fee. - /// A smarter fee could be added, although that would have to happen slightly - /// later, as this dumb fee can be pre-charged without looking at the code. + /// This cost does not consider the structure of the contract code, only the + /// size. This is currently the only loading fee. A fee that takes the code + /// structure into consideration could be added. But since that would have + /// to happen after loading, we cannot pre-charge it. This is the main + /// motivation to (only) have this simple fee. pub fn add_contract_loading_fee(&mut self, code_len: u64) -> Result<()> { self.gas_counter.pay_per(contract_loading_bytes, code_len)?; self.gas_counter.pay_base(contract_loading_base) From 097e519506280309c6aa8dbbdf7d61c944f8d61a Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Wed, 1 Jun 2022 18:25:44 +0200 Subject: [PATCH 15/16] cargo fmt --- runtime/near-vm-logic/src/logic.rs | 2 +- runtime/near-vm-runner/src/wasmer2_runner.rs | 7 +++++-- runtime/near-vm-runner/src/wasmer_runner.rs | 7 +++++-- runtime/near-vm-runner/src/wasmtime_runner.rs | 7 +++++-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index 5af22a5f00a..af8d6bab42c 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -2700,7 +2700,7 @@ impl<'a> VMLogic<'a> { } /// VM independent setup before loading the executable. - /// + /// /// Does VM independent checks that happen after the instantiation of /// VMLogic but before loading the executable. This includes pre-charging gas /// costs for loading the executable, which depends on the size of the WASM code. diff --git a/runtime/near-vm-runner/src/wasmer2_runner.rs b/runtime/near-vm-runner/src/wasmer2_runner.rs index b94161889a4..b03347455e7 100644 --- a/runtime/near-vm-runner/src/wasmer2_runner.rs +++ b/runtime/near-vm-runner/src/wasmer2_runner.rs @@ -535,8 +535,11 @@ impl crate::runner::VM for Wasmer2VM { current_protocol_version, ); - let result = - logic.before_loading_executable(method_name, current_protocol_version, code.code().len()); + let result = logic.before_loading_executable( + method_name, + current_protocol_version, + code.code().len(), + ); if let Err(e) = result { return VMResult::abort(logic, e); } diff --git a/runtime/near-vm-runner/src/wasmer_runner.rs b/runtime/near-vm-runner/src/wasmer_runner.rs index c162459ecee..cbeb9c76d07 100644 --- a/runtime/near-vm-runner/src/wasmer_runner.rs +++ b/runtime/near-vm-runner/src/wasmer_runner.rs @@ -330,8 +330,11 @@ impl crate::runner::VM for Wasmer0VM { current_protocol_version, ); - let result = - logic.before_loading_executable(method_name, current_protocol_version, code.code().len()); + let result = logic.before_loading_executable( + method_name, + current_protocol_version, + code.code().len(), + ); if let Err(e) = result { return VMResult::abort(logic, e); } diff --git a/runtime/near-vm-runner/src/wasmtime_runner.rs b/runtime/near-vm-runner/src/wasmtime_runner.rs index 372b7753bca..fda05bc43b8 100644 --- a/runtime/near-vm-runner/src/wasmtime_runner.rs +++ b/runtime/near-vm-runner/src/wasmtime_runner.rs @@ -216,8 +216,11 @@ impl crate::runner::VM for WasmtimeVM { current_protocol_version, ); - let result = - logic.before_loading_executable(method_name, current_protocol_version, code.code().len()); + let result = logic.before_loading_executable( + method_name, + current_protocol_version, + code.code().len(), + ); if let Err(e) = result { return VMResult::abort(logic, e); } From 7764a2d9f5462606a31755598249a35d8a55a6e5 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Thu, 2 Jun 2022 14:48:05 +0200 Subject: [PATCH 16/16] Update test_run_sequential to include new cost --- runtime/near-vm-runner/src/tests/contract_preload.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/runtime/near-vm-runner/src/tests/contract_preload.rs b/runtime/near-vm-runner/src/tests/contract_preload.rs index 02efb84c1d8..3969fc29108 100644 --- a/runtime/near-vm-runner/src/tests/contract_preload.rs +++ b/runtime/near-vm-runner/src/tests/contract_preload.rs @@ -172,7 +172,8 @@ fn test_vm_runner(preloaded: bool, vm_kind: VMKind, repeat: i32) { ProtocolVersion::MAX, cache.as_deref(), ); - test_result(result2, 0).expect_err("Call expected to fail."); + let expected_gas = crate::tests::prepaid_loading_gas(code2.code().len()); + test_result(result2, expected_gas).expect_err("Call expected to fail."); } } }