diff --git a/chain/chain/src/tests/simple_chain.rs b/chain/chain/src/tests/simple_chain.rs index 22796d9ed08..392b0c1c83d 100644 --- a/chain/chain/src/tests/simple_chain.rs +++ b/chain/chain/src/tests/simple_chain.rs @@ -39,7 +39,7 @@ fn build_chain() { // cargo insta test --accept -p near-chain --features nightly -- tests::simple_chain::build_chain let hash = chain.head().unwrap().last_block_hash; if cfg!(feature = "nightly") { - insta::assert_display_snapshot!(hash, @"8F4vXPPNevoQXVGdwKAZQfzzxhSyqWp3xJiik4RMUKSk"); + insta::assert_display_snapshot!(hash, @"7Db9S56zVuTKvYGnHXvsJYH6CvQBBEm2TC7Y3S85SZps"); } else { insta::assert_display_snapshot!(hash, @"6sAno2uEwwQ5yiDscePWY8HWmRJLpGNv39uoff3BCpxT"); } @@ -68,7 +68,7 @@ fn build_chain() { let hash = chain.head().unwrap().last_block_hash; if cfg!(feature = "nightly") { - insta::assert_display_snapshot!(hash, @"DrW7MsRaFhEdjQcxjqrTXvNmQ1eptgURQ7RUTeZnrBXC"); + insta::assert_display_snapshot!(hash, @"HbPMs5o1Twqb7ZqrrhaCGawwZbYJVaPHZ7tfoihcnYxc"); } else { insta::assert_display_snapshot!(hash, @"Fn9MgjUx6VXhPYNqqDtf2C9kBVveY2vuSLXNLZUNJCqK"); } diff --git a/core/primitives/Cargo.toml b/core/primitives/Cargo.toml index 6025ef80081..4938cc85a1d 100644 --- a/core/primitives/Cargo.toml +++ b/core/primitives/Cargo.toml @@ -44,12 +44,14 @@ 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 = [ "nightly_protocol", "protocol_feature_chunk_only_producers", "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 302fb6633ec..b895c7445f2 100644 --- a/core/primitives/src/version.rs +++ b/core/primitives/src/version.rs @@ -161,6 +161,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` @@ -177,7 +180,7 @@ const STABLE_PROTOCOL_VERSION: ProtocolVersion = 55; 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)] @@ -241,6 +244,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 66e2e138237..c4dd40acc34 100644 --- a/nearcore/Cargo.toml +++ b/nearcore/Cargo.toml @@ -128,6 +128,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 = [ "nightly_protocol", "near-primitives/nightly", @@ -138,6 +141,7 @@ nightly = [ "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/Cargo.toml b/runtime/near-vm-logic/Cargo.toml index 5af666de113..4e13964906a 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", +] # Use this feature to enable counting of fees and costs applied. costs_counting = [] diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index a1e0e2a5539..a0888233b17 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -7,9 +7,11 @@ 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, 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, @@ -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; @@ -2670,6 +2672,13 @@ impl<'a> VMLogic<'a> { } } + /// Add a cost for loading the contract code in the VM. + /// + /// 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) @@ -2686,6 +2695,62 @@ 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) } + + /// 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. + pub fn before_loading_executable( + &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(()) + } + + /// Legacy code to preserve old gas charging behaviour in old protocol versions. + pub fn after_loading_executable( + &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/Cargo.toml b/runtime/near-vm-runner/Cargo.toml index 2c7bdc1b382..a54fdfbca36 100644 --- a/runtime/near-vm-runner/Cargo.toml +++ b/runtime/near-vm-runner/Cargo.toml @@ -92,8 +92,14 @@ no_cpu_compatibility_checks = [] no_cache = [] +protocol_feature_fix_contract_loading_cost = [ + "near-primitives/protocol_feature_fix_contract_loading_cost", + "near-vm-logic/protocol_feature_fix_contract_loading_cost", +] + nightly = [ "near-primitives/nightly", + "protocol_feature_fix_contract_loading_cost", ] sandbox = ["near-vm-logic/sandbox"] 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/tests.rs b/runtime/near-vm-runner/src/tests.rs index f2265509a89..a7f920b128c 100644 --- a/runtime/near-vm-runner/src/tests.rs +++ b/runtime/near-vm-runner/src/tests.rs @@ -55,21 +55,29 @@ fn create_context(input: Vec<u8>) -> 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<ProtocolVersion>, + prepaid_gas: u64, ) -> (VMOutcome, Option<VMError>) { 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( @@ -79,7 +87,7 @@ fn make_simple_contract_call_with_gas_vm( context, &fees, &promise_results, - LATEST_PROTOCOL_VERSION, + protocol_version.unwrap_or(LATEST_PROTOCOL_VERSION), None, ) .outcome_error() @@ -91,28 +99,16 @@ fn make_simple_contract_call_with_protocol_version_vm( protocol_version: ProtocolVersion, vm_kind: VMKind, ) -> (VMOutcome, Option<VMError>) { - 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<VMError>) { + make_simple_contract_call_ex(code, method_name, vm_kind, None, prepaid_gas) } fn make_simple_contract_call_vm( @@ -120,7 +116,7 @@ fn make_simple_contract_call_vm( method_name: &str, vm_kind: VMKind, ) -> (VMOutcome, Option<VMError>) { - 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] @@ -134,3 +130,16 @@ fn gas_and_error_match( assert_eq!(outcome.burnt_gas, expected_gas, "burnt gas differs"); assert_eq!(outcome_and_error.1, expected_error); } + +/// Small helper to compute expected loading gas cost charged before loading. +/// +/// 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") { + 35_445_963 + bytes as u64 * 21_6750 + } else { + 0 + } +} diff --git a/runtime/near-vm-runner/src/tests/cache.rs b/runtime/near-vm-runner/src/tests/cache.rs index 02df020bfe0..1abad48931c 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)) diff --git a/runtime/near-vm-runner/src/tests/compile_errors.rs b/runtime/near-vm-runner/src/tests/compile_errors.rs index 8877ec3d7e5..02fb523e6b1 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, - 0, + 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 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."); } } } diff --git a/runtime/near-vm-runner/src/tests/runtime_errors.rs b/runtime/near-vm-runner/src/tests/runtime_errors.rs index 5acb12223b2..9550a7beb75 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), - 0, + 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<u8> { #[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); 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), - 0, + 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), 0, Some(VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodEmptyName, @@ -369,9 +374,11 @@ fn wrong_signature_contract() -> Vec<u8> { #[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), - 0, + 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<u8> { #[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), - 0, + 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<u8> { // 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), - 0, + 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), - 0, + 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,295 @@ 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, + 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<u8>, u64) { + let code = wat::parse_str( + r#" + (module + (func (export "main") + i32.const 1 + i32.const 1 + i32.div_s + return + ) + )"#, + ) + .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()); + 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, 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, 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()); + + let res = make_simple_contract_call_ex( + &code, + "main", + vm_kind, + Some(ProtocolFeature::FixContractLoadingCost.protocol_version() - 1), + loading_gas, + ); + gas_and_error_match( + res, + 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, + 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()); + 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, + 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, + total_gas - 1, + Some(VMError::FunctionCallError(FunctionCallError::HostError( + HostError::GasExceeded, + ))), + ); + }); + } + + fn function_not_defined_contract() -> Vec<u8> { + 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 = 0; + 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()); + 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/wasmer2_runner.rs b/runtime/near-vm-runner/src/wasmer2_runner.rs index acf4a342927..b03347455e7 100644 --- a/runtime/near-vm-runner/src/wasmer2_runner.rs +++ b/runtime/near-vm-runner/src/wasmer2_runner.rs @@ -516,19 +516,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, @@ -547,12 +534,28 @@ impl crate::runner::VM for Wasmer2VM { &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 result = logic.before_loading_executable( + method_name, + current_protocol_version, + code.code().len(), + ); + if let Err(e) = result { + return VMResult::abort(logic, e); + } + + 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); + } + }; + + 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, @@ -561,8 +564,11 @@ 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); + 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 eb8faf5306f..cbeb9c76d07 100644 --- a/runtime/near-vm-runner/src/wasmer_runner.rs +++ b/runtime/near-vm-runner/src/wasmer_runner.rs @@ -311,19 +311,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, @@ -342,20 +330,43 @@ impl crate::runner::VM for Wasmer0VM { 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); + let result = 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. + 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), + }; + + 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) { - // TODO: This should return an outcome to account for loading cost - 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 b75270ff78c..fda05bc43b8 100644 --- a/runtime/near-vm-runner/src/wasmtime_runner.rs +++ b/runtime/near-vm-runner/src/wasmtime_runner.rs @@ -205,15 +205,6 @@ 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, @@ -225,25 +216,34 @@ impl crate::runner::VM for WasmtimeVM { 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 result = 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) { + 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); + + 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 // 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 +252,33 @@ 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); + return VMResult::abort_but_nop_outcome_in_old_protocol( + logic, + err, + current_protocol_version, + ); } } _ => { let err = VMError::FunctionCallError(FunctionCallError::MethodResolveError( MethodResolveError::MethodNotFound, )); - // TODO: This should return an outcome to account for loading cost - 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, )); - // TODO: This should return an outcome to account for loading cost - 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) { @@ -285,8 +294,11 @@ 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) + return VMResult::abort_but_nop_outcome_in_old_protocol( + logic, + err, + current_protocol_version, + ); } }, Err(err) => VMResult::abort(logic, err.into_vm_error()),