[executor] remove usage of Arc<SparseMerkleTree>

Because SparseMerkleTree is a thin wrapper of Arc<Inner>

execution/executor-types/src/lib.rs

@@ -333,7 +333,7 @@ pub struct ExecutedTrees {
     /// tree is presenting the latest commited state, it will have a single Subtree node (or
     /// Empty node) whose hash equals the root hash of the newest Sparse Merkle Tree in
     /// storage.
-    state_tree: Arc<SparseMerkleTree>,
+    state_tree: SparseMerkleTree,
 
     /// The in-memory Merkle Accumulator representing a blockchain state consistent with the
     /// `state_tree`.
@@ -352,7 +352,7 @@ impl From<TreeState> for ExecutedTrees {
 
 impl ExecutedTrees {
     pub fn new_copy(
-        state_tree: Arc<SparseMerkleTree>,
+        state_tree: SparseMerkleTree,
         transaction_accumulator: Arc<InMemoryAccumulator<TransactionAccumulatorHasher>>,
     ) -> Self {
         Self {
@@ -361,7 +361,7 @@ impl ExecutedTrees {
         }
     }
 
-    pub fn state_tree(&self) -> &Arc<SparseMerkleTree> {
+    pub fn state_tree(&self) -> &SparseMerkleTree {
         &self.state_tree
     }
 
@@ -388,7 +388,7 @@ impl ExecutedTrees {
         num_leaves_in_accumulator: u64,
     ) -> ExecutedTrees {
         ExecutedTrees {
-            state_tree: Arc::new(SparseMerkleTree::new(state_root_hash)),
+            state_tree: SparseMerkleTree::new(state_root_hash),
             transaction_accumulator: Arc::new(
                 InMemoryAccumulator::new(frozen_subtrees_in_accumulator, num_leaves_in_accumulator)
                     .expect("The startup info read from storage should be valid."),

execution/executor/src/lib.rs

@@ -433,7 +433,7 @@ where
         Ok(ProcessedVMOutput::new(
             txn_data,
             ExecutedTrees::new_copy(
-                Arc::new(current_state_tree),
+                current_state_tree,
                 Arc::new(current_transaction_accumulator),
             ),
             next_epoch_state,
@@ -463,13 +463,13 @@ where
         cache: &RwLockReadGuard<SpeculationCache>,
         id: StateViewId,
         executed_trees: &'a ExecutedTrees,
-    ) -> VerifiedStateView<'a, DpnProto> {
+    ) -> VerifiedStateView<DpnProto> {
         VerifiedStateView::new(
             id,
             Arc::clone(&self.db.reader),
             cache.committed_trees().version(),
             cache.committed_trees().state_root(),
-            executed_trees.state_tree(),
+            executed_trees.state_tree().clone(),
         )
     }
 
@@ -478,11 +478,11 @@ where
         Self::get_executed_trees_from_lock(&read_lock, block_id)
     }
 
-    fn get_executed_state_view<'a>(
+    fn get_executed_state_view(
         &self,
         id: StateViewId,
-        executed_trees: &'a ExecutedTrees,
-    ) -> VerifiedStateView<'a, DpnProto> {
+        executed_trees: &ExecutedTrees,
+    ) -> VerifiedStateView<DpnProto> {
         let read_lock = self.cache.read();
         self.get_executed_state_view_from_lock(&read_lock, id, executed_trees)
     }
@@ -507,7 +507,7 @@ where
             Arc::clone(&self.db.reader),
             read_lock.synced_trees().version(),
             read_lock.synced_trees().state_root(),
-            read_lock.synced_trees().state_tree(),
+            read_lock.synced_trees().state_tree().clone(),
         );
 
         fail_point!("executor::vm_execute_chunk", |_| {

json-rpc/src/tests/genesis.rs

@@ -9,13 +9,13 @@ use diem_types::{
 use executor::process_write_set;
 use executor_types::ProofReader;
 use scratchpad::SparseMerkleTree;
-use std::{collections::HashMap, convert::TryFrom, sync::Arc};
+use std::{collections::HashMap, convert::TryFrom};
 use vm_genesis::{generate_genesis_change_set_for_testing, GenesisOptions};
 
 // generate genesis state blob
 pub fn generate_genesis_state() -> (
     HashMap<AccountAddress, AccountStateBlob>,
-    Arc<SparseMerkleTree<AccountStateBlob>>,
+    SparseMerkleTree<AccountStateBlob>,
 ) {
     let change_set = generate_genesis_change_set_for_testing(GenesisOptions::Compiled);
     let txn = Transaction::GenesisTransaction(WriteSetPayload::Direct(change_set.clone()));
@@ -29,15 +29,14 @@ pub fn generate_genesis_state() -> (
         .into_iter()
         .map(|(addr, state)| (addr, AccountStateBlob::try_from(&state).unwrap()))
         .collect::<HashMap<_, _>>();
-    let new_tree = Arc::new(
-        tree.batch_update(
+    let new_tree = tree
+        .batch_update(
             blobs
                 .iter()
                 .map(|(addr, value)| (addr.hash(), value))
                 .collect(),
             &proof_reader,
         )
-        .expect("Failed to update state tree."),
-    );
+        .expect("Failed to update state tree.");
     (blobs, new_tree)
 }

specifications/trusted_computing_base/execution_correctness/README.md

@@ -144,7 +144,7 @@ pub struct ExecutedTrees {
     /// tree is presenting the latest committed state, it will have a single Subtree node (or
     /// Empty node) whose hash equals the root hash of the newest Sparse Merkle Tree in
     /// storage.
-    state_tree: Arc<SparseMerkleTree>,
+    state_tree: SparseMerkleTree,
 
     /// The in-memory Merkle Accumulator representing a blockchain state consistent with the
     /// `state_tree`.
@@ -175,7 +175,7 @@ pub struct TransactionData {
     /// The in-memory Sparse Merkle Tree after the write set is applied. This is `Arc` because the
     /// tree has uncommitted state and sometimes `StateVersionView` needs to have a pointer to the
     /// tree so VM can read it.
-    state_tree: Arc<SparseMerkleTree>,
+    state_tree: SparseMerkleTree,
 
     /// The in-memory Merkle Accumulator that has all events emitted by this transaction.
     event_tree: Arc<InMemoryAccumulator<EventAccumulatorHasher>>,

storage/scratchpad/src/sparse_merkle/batches_update.rs

@@ -1,23 +1,26 @@
 // Copyright (c) The Diem Core Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-use std::borrow::Borrow;
-use std::cmp;
-use std::collections::BTreeMap;
-use diem_crypto::hash::{CryptoHash, SPARSE_MERKLE_PLACEHOLDER_HASH};
-use diem_crypto::HashValue;
-use diem_types::proof::{SparseMerkleInternalNode, SparseMerkleLeafNode};
-use crate::{ProofRead};
-use crate::sparse_merkle::node::{NodeInner, SubTree};
-use crate::sparse_merkle::{IntermediateHashes, UpdateError};
-use crate::sparse_merkle::utils::{partition, swap_if};
 use super::SparseMerkleTree;
+use crate::{
+    sparse_merkle::{
+        node::{NodeInner, SubTree},
+        utils::{partition, swap_if},
+        IntermediateHashes, UpdateError,
+    },
+    ProofRead,
+};
+use diem_crypto::{
+    hash::{CryptoHash, SPARSE_MERKLE_PLACEHOLDER_HASH},
+    HashValue,
+};
+use diem_types::proof::{SparseMerkleInternalNode, SparseMerkleLeafNode};
+use std::{borrow::Borrow, cmp, collections::BTreeMap};
 
 impl<V> SparseMerkleTree<V>
-    where
-        V: Clone + CryptoHash + Send + Sync,
+where
+    V: Clone + CryptoHash + Send + Sync,
 {
-
     /// Constructs a new Sparse Merkle Tree, returns the SMT root hash after each update and the
     /// final SMT root. Since the tree is immutable, existing tree remains the same and may
     /// share parts with the new, returned tree. Unlike `serial_update', intermediate trees aren't
@@ -437,4 +440,4 @@ impl<V> SparseMerkleTree<V>
             })
             .collect()
     }
-}
+}

storage/scratchpad/src/sparse_merkle/mod.rs

@@ -73,16 +73,16 @@ mod node;
 mod updater;
 mod utils;
 
+pub mod batches_update;
 #[cfg(test)]
 mod sparse_merkle_test;
 #[cfg(any(test, feature = "bench", feature = "fuzzing"))]
 pub mod test_utils;
-pub mod batches_update;
 
 use crate::sparse_merkle::{
     node::{NodeInner, SubTree},
     updater::SubTreeUpdater,
-    utils::{partition},
+    utils::partition,
 };
 use diem_crypto::{
     hash::{CryptoHash, SPARSE_MERKLE_PLACEHOLDER_HASH},
@@ -91,7 +91,7 @@ use diem_crypto::{
 use diem_infallible::Mutex;
 use diem_types::{
     nibble::{nibble_path::NibblePath, ROOT_NIBBLE_HEIGHT},
-    proof::{SparseMerkleProof},
+    proof::SparseMerkleProof,
 };
 use std::{
     borrow::Borrow,

storage/storage-interface/src/state_view.rs

@@ -24,7 +24,7 @@ use std::{
 
 /// `VerifiedStateView` is like a snapshot of the global state comprised of state view at two
 /// levels, persistent storage and memory.
-pub struct VerifiedStateView<'a, PS: ProtocolSpec> {
+pub struct VerifiedStateView<PS: ProtocolSpec> {
     /// For logging and debugging purpose, identifies what this view is for.
     id: StateViewId,
 
@@ -39,7 +39,7 @@ pub struct VerifiedStateView<'a, PS: ProtocolSpec> {
     latest_persistent_state_root: HashValue,
 
     /// The in-momery version of sparse Merkle tree of which the states haven't been committed.
-    speculative_state: &'a SparseMerkleTree<AccountStateBlob>,
+    speculative_state: SparseMerkleTree<AccountStateBlob>,
 
     /// The cache of verified account states from `reader` and `speculative_state_view`,
     /// represented by a hashmap with an account address as key and a pair of an ordered
@@ -81,7 +81,7 @@ pub struct VerifiedStateView<'a, PS: ProtocolSpec> {
     account_to_proof_cache: RwLock<HashMap<HashValue, SparseMerkleProof<AccountStateBlob>>>,
 }
 
-impl<'a, PS: ProtocolSpec> VerifiedStateView<'a, PS> {
+impl<PS: ProtocolSpec> VerifiedStateView<PS> {
     /// Constructs a [`VerifiedStateView`] with persistent state view represented by
     /// `latest_persistent_state_root` plus a storage reader, and the in-memory speculative state
     /// on top of it represented by `speculative_state`.
@@ -90,7 +90,7 @@ impl<'a, PS: ProtocolSpec> VerifiedStateView<'a, PS> {
         reader: Arc<dyn DbReader<PS>>,
         latest_persistent_version: Option<Version>,
         latest_persistent_state_root: HashValue,
-        speculative_state: &'a SparseMerkleTree<AccountStateBlob>,
+        speculative_state: SparseMerkleTree<AccountStateBlob>,
     ) -> Self {
         // Hack: When there's no transaction in the db but state tree root hash is not the
         // placeholder hash, it implies that there's pre-genesis state present.
@@ -113,21 +113,21 @@ impl<'a, PS: ProtocolSpec> VerifiedStateView<'a, PS> {
     }
 }
 
-impl<'a, PS: ProtocolSpec> From<VerifiedStateView<'a, PS>>
+impl<PS: ProtocolSpec> From<VerifiedStateView<PS>>
     for (
         HashMap<AccountAddress, AccountState>,
         HashMap<HashValue, SparseMerkleProof<AccountStateBlob>>,
     )
 {
-    fn from(view: VerifiedStateView<'a, PS>) -> Self {
+    fn from(view: VerifiedStateView<PS>) -> Self {
         (
             view.account_to_state_cache.into_inner(),
             view.account_to_proof_cache.into_inner(),
         )
     }
 }
 
-impl<'a, PS: ProtocolSpec> StateView for VerifiedStateView<'a, PS> {
+impl<PS: ProtocolSpec> StateView for VerifiedStateView<PS> {
     fn id(&self) -> StateViewId {
         self.id
     }
@@ -201,5 +201,5 @@ impl<'a, PS: ProtocolSpec> StateView for VerifiedStateView<'a, PS> {
 pub mod default_protocol {
     use diem_types::protocol_spec::DpnProto;
 
-    pub type VerifiedStateView<'a> = super::VerifiedStateView<'a, DpnProto>;
+    pub type VerifiedStateView = super::VerifiedStateView<DpnProto>;
 }

vm-validator/Cargo.toml

@@ -12,7 +12,6 @@ edition = "2018"
 [dependencies]
 anyhow = "1.0.38"
 fail = "0.4.0"
-ouroboros = "0.9.2"
 scratchpad = { path = "../storage/scratchpad" }
 diem-state-view = { path = "../storage/state-view" }
 storage-interface = { path = "../storage/storage-interface" }

vm-validator/src/vm_validator.rs

@@ -7,7 +7,6 @@ use diem_types::{
     account_address::AccountAddress,
     account_config::{AccountResource, AccountSequenceInfo},
     account_state::AccountState,
-    account_state_blob::AccountStateBlob,
     on_chain_config::{DiemVersion, OnChainConfigPayload, VMConfig, VMPublishingOption},
     protocol_spec::DpnProto,
     transaction::{SignedTransaction, VMValidatorResult},
@@ -35,35 +34,23 @@ pub trait TransactionValidation: Send + Sync + Clone {
     fn notify_commit(&mut self);
 }
 
-#[ouroboros::self_referencing(pub_extras)]
-struct CachedStateView {
-    smt: Box<SparseMerkleTree<AccountStateBlob>>,
-    #[borrows(smt)]
-    #[covariant]
-    state_view: VerifiedStateView<'this, DpnProto>,
-}
-
-impl CachedStateView {
-    pub fn create(db_reader: &Arc<dyn DbReader<DpnProto>>) -> Self {
-        let (version, state_root) = db_reader.get_latest_state_root().expect("Should not fail.");
-        let smt = SparseMerkleTree::new(state_root);
-        Self::new(Box::new(smt), move |smt| {
-            VerifiedStateView::new(
-                StateViewId::TransactionValidation {
-                    base_version: version,
-                },
-                Arc::clone(db_reader),
-                Some(version),
-                state_root,
-                smt,
-            )
-        })
-    }
+fn latest_state_view(db_reader: &Arc<dyn DbReader<DpnProto>>) -> VerifiedStateView<DpnProto> {
+    let (version, state_root) = db_reader.get_latest_state_root().expect("Should not fail.");
+
+    VerifiedStateView::new(
+        StateViewId::TransactionValidation {
+            base_version: version,
+        },
+        Arc::clone(db_reader),
+        Some(version),
+        state_root,
+        SparseMerkleTree::new(state_root),
+    )
 }
 
 pub struct VMValidator {
     db_reader: Arc<dyn DbReader<DpnProto>>,
-    cached_state_view: CachedStateView,
+    cached_state_view: VerifiedStateView<DpnProto>,
     vm: DiemVM,
 }
 
@@ -75,9 +62,9 @@ impl Clone for VMValidator {
 
 impl VMValidator {
     pub fn new(db_reader: Arc<dyn DbReader<DpnProto>>) -> Self {
-        let cached_state_view = CachedStateView::create(&db_reader);
+        let cached_state_view = latest_state_view(&db_reader);
 
-        let vm = DiemVM::new_for_validation(cached_state_view.borrow_state_view());
+        let vm = DiemVM::new_for_validation(&cached_state_view);
         VMValidator {
             db_reader,
             cached_state_view,
@@ -97,9 +84,7 @@ impl TransactionValidation for VMValidator {
         });
         use diem_vm::VMValidator;
 
-        Ok(self
-            .vm
-            .validate_transaction(txn, self.cached_state_view.borrow_state_view()))
+        Ok(self.vm.validate_transaction(txn, &self.cached_state_view))
     }
 
     fn restart(&mut self, config: OnChainConfigPayload) -> Result<()> {
@@ -113,8 +98,7 @@ impl TransactionValidation for VMValidator {
     }
 
     fn notify_commit(&mut self) {
-        let cached_state_view = CachedStateView::create(&self.db_reader);
-        self.cached_state_view = cached_state_view;
+        self.cached_state_view = latest_state_view(&self.db_reader);
     }
 }