From 1e5da20ad7969860d9df4448b539f66762e220b3 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Thu, 2 Sep 2021 12:06:43 +0300 Subject: [PATCH] refactor(core, primitives): ContractCode getters (#4749) Resolves https://github.com/near/nearcore/issues/4738 --- core/primitives-core/src/contract.rs | 18 ++++++++---- core/primitives/src/views.rs | 6 ++-- core/store/src/lib.rs | 2 +- integration-tests/src/node/mod.rs | 2 +- .../tests/runtime/state_viewer.rs | 9 +++--- runtime/near-vm-runner/src/cache.rs | 28 +++++++++---------- runtime/near-vm-runner/src/wasmer1_runner.rs | 2 +- runtime/near-vm-runner/src/wasmer_runner.rs | 2 +- runtime/near-vm-runner/src/wasmtime_runner.rs | 4 +-- .../src/function_call.rs | 2 +- runtime/runtime/src/actions.rs | 8 +++--- runtime/runtime/src/cache.rs | 2 +- runtime/runtime/src/genesis.rs | 2 +- runtime/runtime/src/lib.rs | 2 +- runtime/runtime/src/state_viewer/mod.rs | 2 +- test-utils/state-viewer/src/main.rs | 2 +- 16 files changed, 49 insertions(+), 44 deletions(-) diff --git a/core/primitives-core/src/contract.rs b/core/primitives-core/src/contract.rs index ab5f83294e3..e6a54093821 100644 --- a/core/primitives-core/src/contract.rs +++ b/core/primitives-core/src/contract.rs @@ -1,21 +1,27 @@ use crate::hash::{hash as sha256, CryptoHash}; pub struct ContractCode { - pub code: Vec, - pub hash: CryptoHash, + code: Vec, + hash: CryptoHash, } impl ContractCode { pub fn new(code: Vec, hash: Option) -> ContractCode { let hash = hash.unwrap_or_else(|| sha256(&code)); + debug_assert_eq!(hash, sha256(&code)); + ContractCode { code, hash } } - pub fn get_hash(&self) -> CryptoHash { - self.hash + pub fn code(&self) -> &[u8] { + self.code.as_slice() + } + + pub fn into_code(self) -> Vec { + self.code } - pub fn get_code(&self) -> &Vec { - &self.code + pub fn hash(&self) -> &CryptoHash { + &self.hash } } diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index a8dd48d3b3d..0a7cabccb66 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -127,13 +127,15 @@ impl From for Account { impl From for ContractCodeView { fn from(contract_code: ContractCode) -> Self { - ContractCodeView { code: contract_code.code, hash: contract_code.hash } + let hash = *contract_code.hash(); + let code = contract_code.into_code(); + ContractCodeView { code, hash } } } impl From for ContractCode { fn from(contract_code: ContractCodeView) -> Self { - ContractCode { code: contract_code.code, hash: contract_code.hash } + ContractCode::new(contract_code.code, Some(contract_code.hash)) } } diff --git a/core/store/src/lib.rs b/core/store/src/lib.rs index 73706b13c60..dd1bbc1c2f1 100644 --- a/core/store/src/lib.rs +++ b/core/store/src/lib.rs @@ -420,7 +420,7 @@ pub fn get_access_key_raw( } pub fn set_code(state_update: &mut TrieUpdate, account_id: AccountId, code: &ContractCode) { - state_update.set(TrieKey::ContractCode { account_id }, code.code.clone()); + state_update.set(TrieKey::ContractCode { account_id }, code.code().to_vec()); } pub fn get_code( diff --git a/integration-tests/src/node/mod.rs b/integration-tests/src/node/mod.rs index e686ab6b06a..c09923e6289 100644 --- a/integration-tests/src/node/mod.rs +++ b/integration-tests/src/node/mod.rs @@ -158,7 +158,7 @@ pub fn create_nodes_from_seeds(seeds: Vec) -> Vec { { if record_account_id.as_ref() == &seed { is_account_record_found = true; - account.set_code_hash(ContractCode::new(code.to_vec(), None).get_hash()); + account.set_code_hash(ContractCode::new(code.to_vec(), None).hash().clone()); } } } diff --git a/integration-tests/tests/runtime/state_viewer.rs b/integration-tests/tests/runtime/state_viewer.rs index 886c492306e..52efa2d76df 100644 --- a/integration-tests/tests/runtime/state_viewer.rs +++ b/integration-tests/tests/runtime/state_viewer.rs @@ -3,6 +3,7 @@ use integration_tests::runtime_utils::{ }; use near_primitives::{ account::Account, + hash::hash as sha256, hash::CryptoHash, views::{StateItem, ViewApplyState}, }; @@ -168,15 +169,13 @@ fn test_view_state_too_large() { fn test_view_state_with_large_contract() { let (_, tries, root) = get_runtime_and_trie(); let mut state_update = tries.new_trie_update(TEST_SHARD_UID, root); + let contract_code = [0; Account::MAX_ACCOUNT_DELETION_STORAGE_USAGE as usize].to_vec(); set_account( &mut state_update, alice_account(), - &Account::new(0, 0, CryptoHash::default(), 50_001), - ); - state_update.set( - TrieKey::ContractCode { account_id: alice_account() }, - [0; Account::MAX_ACCOUNT_DELETION_STORAGE_USAGE as usize].to_vec(), + &Account::new(0, 0, sha256(&contract_code), 50_001), ); + state_update.set(TrieKey::ContractCode { account_id: alice_account() }, contract_code); let trie_viewer = TrieViewer::new(Some(50_000), None); let result = trie_viewer.view_state(&state_update, &alice_account(), b""); assert!(result.is_ok()); diff --git a/runtime/near-vm-runner/src/cache.rs b/runtime/near-vm-runner/src/cache.rs index e3388f59056..589c6c4a9c1 100644 --- a/runtime/near-vm-runner/src/cache.rs +++ b/runtime/near-vm-runner/src/cache.rs @@ -56,7 +56,7 @@ pub fn get_contract_cache_key( ) -> CryptoHash { let _span = tracing::debug_span!(target: "vm", "get_key").entered(); let key = ContractCacheKey::Version3 { - code_hash: code.hash, + code_hash: *code.hash(), vm_config_non_crypto_hash: config.non_crypto_hash(), vm_kind, vm_hash: vm_hash(vm_kind), @@ -211,9 +211,9 @@ pub mod wasmer0_cache { ) -> Result { let key = get_contract_cache_key(code, VMKind::Wasmer0, config); #[cfg(not(feature = "no_cache"))] - return memcache_compile_module_cached_wasmer(key, &code.code, config, cache); + return memcache_compile_module_cached_wasmer(key, code.code(), config, cache); #[cfg(feature = "no_cache")] - return compile_module_cached_wasmer_impl(key, &code.code, config, cache); + return compile_module_cached_wasmer_impl(key, code.code(), config, cache); } } @@ -316,9 +316,9 @@ pub mod wasmer1_cache { ) -> Result { let key = get_contract_cache_key(code, VMKind::Wasmer1, config); #[cfg(not(feature = "no_cache"))] - return memcache_compile_module_cached_wasmer1(key, &code.code, config, cache, store); + return memcache_compile_module_cached_wasmer1(key, code.code(), config, cache, store); #[cfg(feature = "no_cache")] - return compile_module_cached_wasmer1_impl(key, &code.code, config, cache, store); + return compile_module_cached_wasmer1_impl(key, code.code(), config, cache, store); } } @@ -340,19 +340,17 @@ pub fn precompile_contract_vm( Ok(None) | Err(_) => {} }; match vm_kind { - VMKind::Wasmer0 => match wasmer0_cache::compile_and_serialize_wasmer( - wasm_code.code.as_slice(), - config, - &key, - cache, - ) { - Ok(_) => Ok(ContractPrecompilatonResult::ContractCompiled), - Err(err) => Err(ContractPrecompilatonError::new(err)), - }, + VMKind::Wasmer0 => { + match wasmer0_cache::compile_and_serialize_wasmer(wasm_code.code(), config, &key, cache) + { + Ok(_) => Ok(ContractPrecompilatonResult::ContractCompiled), + Err(err) => Err(ContractPrecompilatonError::new(err)), + } + } VMKind::Wasmer1 => { let store = default_wasmer1_store(); match wasmer1_cache::compile_and_serialize_wasmer1( - wasm_code.code.as_slice(), + wasm_code.code(), &key, config, cache, diff --git a/runtime/near-vm-runner/src/wasmer1_runner.rs b/runtime/near-vm-runner/src/wasmer1_runner.rs index f3cb09923f4..c8b23e3b900 100644 --- a/runtime/near-vm-runner/src/wasmer1_runner.rs +++ b/runtime/near-vm-runner/src/wasmer1_runner.rs @@ -241,7 +241,7 @@ pub fn run_wasmer1( ); // TODO: remove, as those costs are incorrectly computed, and we shall account it on deployment. - if logic.add_contract_compile_fee(code.code.len() as u64).is_err() { + if logic.add_contract_compile_fee(code.code().len() as u64).is_err() { return ( Some(logic.outcome()), Some(VMError::FunctionCallError(FunctionCallError::HostError( diff --git a/runtime/near-vm-runner/src/wasmer_runner.rs b/runtime/near-vm-runner/src/wasmer_runner.rs index 55e535d05d4..a95ab2ac53a 100644 --- a/runtime/near-vm-runner/src/wasmer_runner.rs +++ b/runtime/near-vm-runner/src/wasmer_runner.rs @@ -265,7 +265,7 @@ pub fn run_wasmer<'a>( ); // TODO: remove, as those costs are incorrectly computed, and we shall account it on deployment. - if logic.add_contract_compile_fee(code.code.len() as u64).is_err() { + if logic.add_contract_compile_fee(code.code().len() as u64).is_err() { return ( Some(logic.outcome()), Some(VMError::FunctionCallError(FunctionCallError::HostError( diff --git a/runtime/near-vm-runner/src/wasmtime_runner.rs b/runtime/near-vm-runner/src/wasmtime_runner.rs index ef9c2164d76..49ce180e06f 100644 --- a/runtime/near-vm-runner/src/wasmtime_runner.rs +++ b/runtime/near-vm-runner/src/wasmtime_runner.rs @@ -161,7 +161,7 @@ pub mod wasmtime_runner { wasm_config.limit_config.max_memory_pages, ) .unwrap(); - let prepared_code = match prepare::prepare_contract(&code.code, wasm_config) { + let prepared_code = match prepare::prepare_contract(code.code(), wasm_config) { Ok(code) => code, Err(err) => return (None, Some(VMError::from(err))), }; @@ -182,7 +182,7 @@ pub mod wasmtime_runner { current_protocol_version, ); // TODO: remove, as those costs are incorrectly computed, and we shall account it on deployment. - if logic.add_contract_compile_fee(code.code.len() as u64).is_err() { + if logic.add_contract_compile_fee(code.code().len() as u64).is_err() { return ( Some(logic.outcome()), Some(VMError::FunctionCallError(FunctionCallError::HostError( diff --git a/runtime/runtime-params-estimator/src/function_call.rs b/runtime/runtime-params-estimator/src/function_call.rs index d4122607838..a9b384141db 100644 --- a/runtime/runtime-params-estimator/src/function_call.rs +++ b/runtime/runtime-params-estimator/src/function_call.rs @@ -21,7 +21,7 @@ fn test_function_call(metric: GasMetric, vm_kind: VMKind) { let contract = make_many_methods_contract(method_count); let cost = compute_function_call_cost(metric, vm_kind, REPEATS, &contract); println!("{:?} {:?} {} {}", vm_kind, metric, method_count, cost / REPEATS); - xs.push(contract.get_code().len() as u64); + xs.push(contract.code().len() as u64); ys.push(cost / REPEATS); } diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index a69f42a8640..db4635c030b 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -429,17 +429,17 @@ pub(crate) fn action_deploy_contract( ) -> Result<(), StorageError> { let code = ContractCode::new(deploy_contract.code.clone(), None); let prev_code = get_code(state_update, account_id, Some(account.code_hash()))?; - let prev_code_length = prev_code.map(|code| code.code.len() as u64).unwrap_or_default(); + let prev_code_length = prev_code.map(|code| code.code().len() as u64).unwrap_or_default(); account.set_storage_usage(account.storage_usage().checked_sub(prev_code_length).unwrap_or(0)); account.set_storage_usage( - account.storage_usage().checked_add(code.code.len() as u64).ok_or_else(|| { + account.storage_usage().checked_add(code.code().len() as u64).ok_or_else(|| { StorageError::StorageInconsistentState(format!( "Storage usage integer overflow for account {}", account_id )) })?, ); - account.set_code_hash(code.get_hash()); + account.set_code_hash(*code.hash()); set_code(state_update, account_id.clone(), &code); // Precompile the contract and store result (compiled code or error) in the database. // Note, that contract compilation costs are already accounted in deploy cost using @@ -464,7 +464,7 @@ pub(crate) fn action_delete_account( let contract_code = get_code(state_update, account_id, Some(account.code_hash()))?; if let Some(code) = contract_code { // account storage usage should be larger than code size - let code_len = code.code.len() as u64; + let code_len = code.code().len() as u64; debug_assert!(account_storage_usage > code_len); account_storage_usage = account_storage_usage.saturating_sub(code_len); } diff --git a/runtime/runtime/src/cache.rs b/runtime/runtime/src/cache.rs index 7bbeb769bcc..02c00156963 100644 --- a/runtime/runtime/src/cache.rs +++ b/runtime/runtime/src/cache.rs @@ -10,7 +10,7 @@ pub(crate) fn get_code( ) -> Result>, StorageError> { let code = f()?; Ok(code.map(|code| { - assert_eq!(code_hash, code.get_hash()); + assert_eq!(code_hash, *code.hash()); Arc::new(code) })) } diff --git a/runtime/runtime/src/genesis.rs b/runtime/runtime/src/genesis.rs index 0bf8066e799..f61fc48fd42 100644 --- a/runtime/runtime/src/genesis.rs +++ b/runtime/runtime/src/genesis.rs @@ -126,7 +126,7 @@ impl GenesisStateApplier { // Recompute contract code hash. let code = ContractCode::new(code, None); set_code(&mut state_update, account_id, &code); - assert_eq!(code.get_hash(), acc.code_hash()); + assert_eq!(*code.hash(), acc.code_hash()); } StateRecord::AccessKey { account_id, public_key, access_key } => { set_access_key(&mut state_update, account_id, public_key, &access_key); diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index ab3bc22dff2..3037cc8365b 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -1421,7 +1421,7 @@ impl Runtime { // Recompute contract code hash. let code = ContractCode::new(code, None); set_code(state_update, account_id, &code); - assert_eq!(code.get_hash(), acc.code_hash()); + assert_eq!(*code.hash(), acc.code_hash()); } StateRecord::AccessKey { account_id, public_key, access_key } => { set_access_key(state_update, account_id, public_key, &access_key); diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index 9af9186f64e..9acc4618ee3 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -120,7 +120,7 @@ impl TrieViewer { match get_account(state_update, account_id)? { Some(account) => { let code_len = get_code(state_update, account_id, Some(account.code_hash()))? - .map(|c| c.code.len() as u64) + .map(|c| c.code().len() as u64) .unwrap_or_default(); if let Some(limit) = self.state_size_limit { if account.storage_usage().saturating_sub(code_len) > limit { diff --git a/test-utils/state-viewer/src/main.rs b/test-utils/state-viewer/src/main.rs index fdeada2f29f..9d1781af0de 100644 --- a/test-utils/state-viewer/src/main.rs +++ b/test-utils/state-viewer/src/main.rs @@ -645,7 +645,7 @@ fn check_block_chunk_existence(store: Arc, near_config: &NearConfig) { fn dump_code(account: &str, contract_code: ContractCode, output: &str) { let mut file = File::create(output).unwrap(); - file.write_all(&contract_code.code).unwrap(); + file.write_all(contract_code.code()).unwrap(); println!("Dump contract of account {} into file {}", account, output); }