Skip to content

Commit

Permalink
style: sever some dependencies between primitives and vm-runner (#10425)
Browse files Browse the repository at this point in the history
Most notably things like conversions between error types and re-exports
of e.g. TrieNodesCount now no longer introduce a dependency edge.

There is still a dependency on a few instances however, but those are
much less clear cut on how to resolve. In particular at least
`ProfileDataV3` is in an icky place. It should be split up into a
serialized schema (just like the errors were) and the simple structure
that `near-vm-runner` produces with a conversion in some integration
crate downstream…
  • Loading branch information
nagisa authored Jan 15, 2024
1 parent afdae45 commit ddbb531
Show file tree
Hide file tree
Showing 28 changed files with 174 additions and 150 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions core/primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ calimero_zero_storage = []
assert_matches.workspace = true
bencher.workspace = true
insta.workspace = true
expect-test.workspace = true

[[bench]]
name = "serialization"
Expand Down
57 changes: 0 additions & 57 deletions core/primitives/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1158,63 +1158,6 @@ pub enum FunctionCallError {
ExecutionError(String),
}

impl From<near_vm_runner::logic::errors::MethodResolveError> for MethodResolveError {
fn from(outer_err: near_vm_runner::logic::errors::MethodResolveError) -> Self {
use near_vm_runner::logic::errors::MethodResolveError as MRE;
match outer_err {
MRE::MethodEmptyName => Self::MethodEmptyName,
MRE::MethodNotFound => Self::MethodNotFound,
MRE::MethodInvalidSignature => Self::MethodInvalidSignature,
}
}
}

impl From<near_vm_runner::logic::errors::PrepareError> for PrepareError {
fn from(outer_err: near_vm_runner::logic::errors::PrepareError) -> Self {
use near_vm_runner::logic::errors::PrepareError as PE;
match outer_err {
PE::Serialization => Self::Serialization,
PE::Deserialization => Self::Deserialization,
PE::InternalMemoryDeclared => Self::InternalMemoryDeclared,
PE::GasInstrumentation => Self::GasInstrumentation,
PE::StackHeightInstrumentation => Self::StackHeightInstrumentation,
PE::Instantiate => Self::Instantiate,
PE::Memory => Self::Memory,
PE::TooManyFunctions => Self::TooManyFunctions,
PE::TooManyLocals => Self::TooManyLocals,
}
}
}

impl From<near_vm_runner::logic::errors::CompilationError> for CompilationError {
fn from(outer_err: near_vm_runner::logic::errors::CompilationError) -> Self {
use near_vm_runner::logic::errors::CompilationError as CE;
match outer_err {
CE::CodeDoesNotExist { account_id } => Self::CodeDoesNotExist {
account_id: account_id.parse().expect("account_id in error must be valid"),
},
CE::PrepareError(pe) => Self::PrepareError(pe.into()),
CE::WasmerCompileError { msg } => Self::WasmerCompileError { msg },
}
}
}

impl From<near_vm_runner::logic::errors::FunctionCallError> for FunctionCallError {
fn from(outer_err: near_vm_runner::logic::errors::FunctionCallError) -> Self {
use near_vm_runner::logic::errors::FunctionCallError as FCE;
match outer_err {
FCE::CompilationError(e) => Self::CompilationError(e.into()),
FCE::MethodResolveError(e) => Self::MethodResolveError(e.into()),
// Note: We deliberately collapse all execution errors for
// serialization to make the DB representation less dependent
// on specific types in Rust code.
FCE::HostError(ref _e) => Self::ExecutionError(outer_err.to_string()),
FCE::LinkError { msg } => Self::ExecutionError(format!("Link Error: {}", msg)),
FCE::WasmTrap(ref _e) => Self::ExecutionError(outer_err.to_string()),
}
}
}

#[cfg(feature = "new_epoch_sync")]
pub mod epoch_sync {
use near_primitives_core::hash::CryptoHash;
Expand Down
1 change: 1 addition & 0 deletions core/primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub mod epoch_sync;
pub mod errors;
pub mod merkle;
pub mod network;
pub mod profile_data_v2;
pub mod rand;
pub mod receipt;
pub mod runtime;
Expand Down
File renamed without changes.
18 changes: 17 additions & 1 deletion core/primitives/src/receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,23 @@ use std::borrow::Borrow;
use std::collections::HashMap;
use std::fmt;

pub use near_vm_runner::logic::DataReceiver;
/// The outgoing (egress) data which will be transformed
/// to a `DataReceipt` to be sent to a `receipt.receiver`
#[derive(
BorshSerialize,
BorshDeserialize,
Hash,
Clone,
Debug,
PartialEq,
Eq,
serde::Serialize,
serde::Deserialize,
)]
pub struct DataReceiver {
pub data_id: CryptoHash,
pub receiver_id: AccountId,
}

/// Receipts are used for a cross-shard communication.
/// Receipts could be 2 types (determined by a `ReceiptEnum`): `ReceiptEnum::Action` of `ReceiptEnum::Data`.
Expand Down
4 changes: 2 additions & 2 deletions core/primitives/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use near_crypto::{PublicKey, Signature};
use near_fmt::{AbbrBytes, Slice};
use near_primitives_core::serialize::{from_base64, to_base64};
use near_primitives_core::types::Compute;
use near_vm_runner::{ProfileDataV2, ProfileDataV3};
use near_vm_runner::ProfileDataV3;
use serde::de::Error as DecodeError;
use serde::ser::Error as EncodeError;
use std::borrow::Borrow;
Expand Down Expand Up @@ -237,7 +237,7 @@ pub enum ExecutionMetadata {
#[default]
V1,
/// V2: With ProfileData by legacy `Cost` enum
V2(ProfileDataV2),
V2(crate::profile_data_v2::ProfileDataV2),
/// V3: With ProfileData by gas parameters
V3(Box<ProfileDataV3>),
}
Expand Down
1 change: 0 additions & 1 deletion core/primitives/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use borsh::{BorshDeserialize, BorshSerialize};
use near_crypto::PublicKey;
/// Reexport primitive types
pub use near_primitives_core::types::*;
pub use near_vm_runner::logic::TrieNodesCount;
use once_cell::sync::Lazy;
use serde_with::base64::Base64;
use serde_with::serde_as;
Expand Down
21 changes: 3 additions & 18 deletions core/primitives/src/views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ use chrono::DateTime;
use near_crypto::{PublicKey, Signature};
use near_fmt::{AbbrBytes, Slice};
use near_parameters::{ActionCosts, ExtCosts};
use near_vm_runner::logic::CompiledContractCache;
use near_vm_runner::ContractCode;
use serde_with::base64::Base64;
use serde_with::serde_as;
use std::collections::HashMap;
Expand Down Expand Up @@ -94,7 +92,7 @@ pub struct ViewApplyState {
/// Current Protocol version when we apply the state transition
pub current_protocol_version: ProtocolVersion,
/// Cache for compiled contracts.
pub cache: Option<Box<dyn CompiledContractCache>>,
pub cache: Option<Box<dyn near_vm_runner::logic::CompiledContractCache>>,
}

impl From<&Account> for AccountView {
Expand Down Expand Up @@ -127,20 +125,6 @@ impl From<AccountView> for Account {
}
}

impl From<ContractCode> for ContractCodeView {
fn from(contract_code: ContractCode) -> Self {
let hash = *contract_code.hash();
let code = contract_code.into_code();
ContractCodeView { code, hash }
}
}

impl From<ContractCodeView> for ContractCode {
fn from(contract_code: ContractCodeView) -> Self {
ContractCode::new(contract_code.code, Some(contract_code.hash))
}
}

#[derive(
BorshSerialize,
BorshDeserialize,
Expand Down Expand Up @@ -2350,8 +2334,9 @@ pub struct SplitStorageInfoView {
#[cfg(test)]
mod tests {
use super::ExecutionMetadataView;
use crate::profile_data_v2::ProfileDataV2;
use crate::transaction::ExecutionMetadata;
use near_vm_runner::{ProfileDataV2, ProfileDataV3};
use near_vm_runner::ProfileDataV3;

/// The JSON representation used in RPC responses must not remove or rename
/// fields, only adding fields is allowed or we risk breaking clients.
Expand Down
5 changes: 2 additions & 3 deletions core/store/src/trie/accounting_cache.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
use super::TrieNodesCount;
use crate::{metrics, TrieStorage};
use near_o11y::metrics::prometheus;
use near_o11y::metrics::prometheus::core::{GenericCounter, GenericGauge};
use near_primitives::errors::StorageError;
use near_primitives::hash::CryptoHash;
use near_primitives::shard_layout::ShardUId;
use near_vm_runner::logic::TrieNodesCount;
use std::collections::HashMap;
use std::sync::Arc;

use crate::{metrics, TrieStorage};

/// Deterministic cache to store trie nodes that have been accessed so far
/// during the cache's lifetime. It is used for deterministic gas accounting
/// so that previously accessed trie nodes and values are charged at a
Expand Down
20 changes: 19 additions & 1 deletion core/store/src/trie/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use near_primitives::state::{FlatStateValue, ValueRef};
use near_primitives::state_record::StateRecord;
use near_primitives::trie_key::trie_key_parsers::parse_account_id_prefix;
use near_primitives::trie_key::TrieKey;
pub use near_primitives::types::TrieNodesCount;
use near_primitives::types::{AccountId, StateRoot, StateRootNode};
use near_vm_runner::ContractCode;
pub use raw_node::{Children, RawTrieNode, RawTrieNodeWithSize};
Expand Down Expand Up @@ -1493,6 +1492,25 @@ impl TrieAccess for Trie {
}
}

/// Counts trie nodes reads during tx/receipt execution for proper storage costs charging.
#[derive(Debug, PartialEq)]
pub struct TrieNodesCount {
/// Potentially expensive trie node reads which are served from disk in the worst case.
pub db_reads: u64,
/// Cheap trie node reads which are guaranteed to be served from RAM.
pub mem_reads: u64,
}

impl TrieNodesCount {
/// Used to determine the number of trie nodes charged during some operation.
pub fn checked_sub(self, other: &Self) -> Option<Self> {
Some(Self {
db_reads: self.db_reads.checked_sub(other.db_reads)?,
mem_reads: self.mem_reads.checked_sub(other.mem_reads)?,
})
}
}

/// Methods used in the runtime-parameter-estimator for measuring trie internal
/// operations.
pub mod estimator {
Expand Down
2 changes: 1 addition & 1 deletion core/store/src/trie/trie_recording.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ mod trie_recording_tests {
TestTriesBuilder,
};
use crate::trie::mem::metrics::MEM_TRIE_NUM_LOOKUPS;
use crate::trie::TrieNodesCount;
use crate::{DBCol, Store, Trie};
use near_primitives::hash::{hash, CryptoHash};
use near_primitives::shard_layout::{get_block_shard_uid, get_block_shard_uid_rev, ShardUId};
use near_primitives::state::ValueRef;
use near_primitives::types::chunk_extra::ChunkExtra;
use near_primitives::types::StateRoot;
use near_vm_runner::logic::TrieNodesCount;
use rand::{thread_rng, Rng};
use std::collections::{HashMap, HashSet};
use std::num::NonZeroU32;
Expand Down
2 changes: 1 addition & 1 deletion core/store/src/trie/trie_tests.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::test_utils::{gen_changes, simplify_changes, test_populate_trie, TestTriesBuilder};
use crate::trie::trie_storage::{TrieMemoryPartialStorage, TrieStorage};
use crate::trie::TrieNodesCount;
use crate::{PartialStorage, Trie, TrieUpdate};
use assert_matches::assert_matches;
use near_primitives::challenge::PartialState;
use near_primitives::errors::{MissingTrieValueContext, StorageError};
use near_primitives::hash::{hash, CryptoHash};
use near_primitives::shard_layout::ShardUId;
use near_primitives::types::TrieNodesCount;
use rand::seq::SliceRandom;
use rand::Rng;
use std::cell::RefCell;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ use near_primitives::test_utils::encode;
use near_primitives::transaction::{
Action, ExecutionMetadata, FunctionCallAction, SignedTransaction,
};
use near_primitives::types::{BlockHeightDelta, Gas, TrieNodesCount};
use near_primitives::types::{BlockHeightDelta, Gas};
use near_primitives::version::{ProtocolFeature, ProtocolVersion};
use near_primitives::views::FinalExecutionStatus;
use near_vm_runner::logic::TrieNodesCount;
use nearcore::config::GenesisExt;
use nearcore::test_utils::TestEnvNightshadeSetupExt;

Expand Down
2 changes: 1 addition & 1 deletion integration-tests/src/tests/client/flat_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use near_store::flat::{
FlatStorageReadyStatus, FlatStorageStatus, NUM_PARTS_IN_ONE_STEP,
};
use near_store::test_utils::create_test_store;
use near_store::trie::TrieNodesCount;
use near_store::{KeyLookupMode, Store, TrieTraversalItem};
use near_vm_runner::logic::TrieNodesCount;
use nearcore::config::GenesisExt;
use nearcore::test_utils::TestEnvNightshadeSetupExt;
use std::str::FromStr;
Expand Down
6 changes: 3 additions & 3 deletions integration-tests/src/tests/standard_cases/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::sync::Arc;

mod rpc;
mod runtime;

Expand All @@ -15,13 +13,15 @@ use near_primitives::errors::{
MethodResolveError, TxExecutionError,
};
use near_primitives::hash::{hash, CryptoHash};
use near_primitives::types::{AccountId, Balance, TrieNodesCount};
use near_primitives::types::{AccountId, Balance};
use near_primitives::utils::{derive_eth_implicit_account_id, derive_near_implicit_account_id};
use near_primitives::views::{
AccessKeyView, AccountView, ExecutionMetadataView, FinalExecutionOutcomeView,
FinalExecutionStatus,
};
use near_store::trie::TrieNodesCount;
use nearcore::config::{NEAR_BASE, TESTING_INIT_BALANCE, TESTING_INIT_STAKE};
use std::sync::Arc;

use crate::node::Node;
use crate::user::User;
Expand Down
5 changes: 4 additions & 1 deletion integration-tests/src/user/runtime_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,10 @@ impl User for RuntimeUser {
let state_update = self.client.read().expect(POISONED_LOCK_ERR).get_state_update();
self.trie_viewer
.view_contract_code(&state_update, account_id)
.map(|contract_code| contract_code.into())
.map(|contract_code| {
let hash = *contract_code.hash();
ContractCodeView { hash, code: contract_code.into_code() }
})
.map_err(|err| err.to_string())
}

Expand Down
8 changes: 5 additions & 3 deletions nearcore/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ use near_primitives::types::{
};
use near_primitives::version::ProtocolVersion;
use near_primitives::views::{
AccessKeyInfoView, CallResult, QueryRequest, QueryResponse, QueryResponseKind, ViewApplyState,
ViewStateResult,
AccessKeyInfoView, CallResult, ContractCodeView, QueryRequest, QueryResponse,
QueryResponseKind, ViewApplyState, ViewStateResult,
};
use near_store::config::StateSnapshotType;
use near_store::flat::FlatStorageManager;
Expand Down Expand Up @@ -929,8 +929,10 @@ impl RuntimeAdapter for NightshadeRuntime {
let contract_code = self
.view_contract_code(&shard_uid, *state_root, account_id)
.map_err(|err| near_chain::near_chain_primitives::error::QueryError::from_view_contract_code_error(err, block_height, *block_hash))?;
let hash = *contract_code.hash();
let contract_code_view = ContractCodeView { hash, code: contract_code.into_code() };
Ok(QueryResponse {
kind: QueryResponseKind::ViewCode(contract_code.into()),
kind: QueryResponseKind::ViewCode(contract_code_view),
block_height,
block_hash: *block_hash,
})
Expand Down
1 change: 0 additions & 1 deletion runtime/near-vm-runner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ mod wasmtime_runner;
pub use crate::logic::with_ext_cost_counter;
pub use cache::{get_contract_cache_key, precompile_contract, MockCompiledContractCache};
pub use code::ContractCode;
pub use profile::ProfileDataV2;
pub use profile::ProfileDataV3;
pub use runner::{run, VM};

Expand Down
Loading

0 comments on commit ddbb531

Please sign in to comment.