[storage] receive TransactionInfo from execution, instead of calculat…

…ing it from DB

The DB being dumb, different strategies of generating the TransactionInfo can be controlled on the executor side only.

Notice that before this we've already make it that the storage doesn't fully check all the authentication structures generated by the executor by calculating them all by itself, specifically the state tree calculation leverages the internal node hash values computed by the executor.

We can remove the event tree, and even the top level accumulator calculation from the DB as well, but they are not too costly so it's not prioritized.

Closes: #10030

execution/executor-types/src/executed_chunk.rs

@@ -49,12 +49,11 @@ impl ExecutedChunk {
             .map(|(txn, txn_data)| {
                 Ok(TransactionToCommit::new(
                     txn.clone(),
+                    txn_data.txn_info.clone(),
                     txn_data.account_blobs().clone(),
                     Some(txn_data.jf_node_hashes().clone()),
                     txn_data.write_set().clone(),
                     txn_data.events().to_vec(),
-                    txn_data.gas_used(),
-                    txn_data.status().as_kept_status()?,
                 ))
             })
             .collect()
@@ -90,7 +89,7 @@ impl ExecutedChunk {
         let txn_info_hashes = self
             .to_commit
             .iter()
-            .map(|(_, txn_data)| txn_data.txn_info_hash().unwrap())
+            .map(|(_, txn_data)| txn_data.txn_info_hash())
             .collect::<Vec<_>>();
         let expected_txn_info_hashes = transaction_infos
             .iter()
@@ -177,7 +176,7 @@ impl ExecutedChunk {
         let mut reconfig_events = Vec::new();
 
         for (_, txn_data) in &self.to_commit {
-            transaction_info_hashes.push(txn_data.txn_info_hash().expect("Txn to be kept."));
+            transaction_info_hashes.push(txn_data.txn_info_hash());
             reconfig_events.extend(
                 txn_data
                     .events()

execution/executor-types/src/lib.rs

@@ -431,8 +431,11 @@ pub struct TransactionData {
     /// The amount of gas used.
     gas_used: u64,
 
-    /// The transaction info hash if the VM status output was keep, None otherwise
-    txn_info_hash: Option<HashValue>,
+    /// TransactionInfo
+    txn_info: TransactionInfo,
+
+    /// TransactionInfo.hash()
+    txn_info_hash: HashValue,
 }
 
 impl TransactionData {
@@ -445,7 +448,8 @@ impl TransactionData {
         state_root_hash: HashValue,
         event_tree: Arc<InMemoryAccumulator<EventAccumulatorHasher>>,
         gas_used: u64,
-        txn_info_hash: Option<HashValue>,
+        txn_info: TransactionInfo,
+        txn_info_hash: HashValue,
     ) -> Self {
         TransactionData {
             account_blobs,
@@ -456,6 +460,7 @@ impl TransactionData {
             state_root_hash,
             event_tree,
             gas_used,
+            txn_info,
             txn_info_hash,
         }
     }
@@ -492,7 +497,7 @@ impl TransactionData {
         self.gas_used
     }
 
-    pub fn txn_info_hash(&self) -> Option<HashValue> {
+    pub fn txn_info_hash(&self) -> HashValue {
         self.txn_info_hash
     }
 }

execution/executor/src/components/apply_chunk_output.rs

@@ -289,7 +289,8 @@ impl ApplyChunkOutput {
                     state_tree_hash,
                     Arc::new(event_tree),
                     txn_output.gas_used(),
-                    Some(txn_info_hash),
+                    txn_info,
+                    txn_info_hash,
                 ),
             ))
         }

storage/diemdb/src/lib.rs

@@ -57,7 +57,7 @@ use crate::{
 };
 use anyhow::{ensure, format_err, Result};
 use diem_config::config::RocksdbConfig;
-use diem_crypto::hash::{CryptoHash, HashValue, SPARSE_MERKLE_PLACEHOLDER_HASH};
+use diem_crypto::hash::{HashValue, SPARSE_MERKLE_PLACEHOLDER_HASH};
 use diem_logger::prelude::*;
 use diem_types::{
     account_address::AccountAddress,
@@ -78,7 +78,7 @@ use diem_types::{
         PRE_GENESIS_VERSION,
     },
 };
-use itertools::{izip, zip_eq};
+use itertools::zip_eq;
 use move_core_types::{
     language_storage::{ModuleId, StructTag},
     resolver::{ModuleResolver, ResourceResolver},
@@ -569,7 +569,7 @@ impl DiemDB {
         let last_version = first_version + txns_to_commit.len() as u64 - 1;
 
         // Account state updates. Gather account state root hashes
-        let state_root_hashes = {
+        {
             let _timer = DIEM_STORAGE_OTHER_TIMERS_SECONDS
                 .with_label_values(&["save_transactions_state"])
                 .start_timer();
@@ -588,11 +588,11 @@ impl DiemDB {
                 node_hashes,
                 first_version,
                 &mut cs,
-            )?
-        };
+            )?;
+        }
 
         // Event updates. Gather event accumulator root hashes.
-        let event_root_hashes = {
+        {
             let _timer = DIEM_STORAGE_OTHER_TIMERS_SECONDS
                 .with_label_values(&["save_transactions_events"])
                 .start_timer();
@@ -601,8 +601,8 @@ impl DiemDB {
                     self.event_store
                         .put_events(ver, txn_to_commit.events(), &mut cs)
                 })
-                .collect::<Result<Vec<_>>>()?
-        };
+                .collect::<Result<Vec<_>>>()?;
+        }
 
         let new_root_hash = {
             let _timer = DIEM_STORAGE_OTHER_TIMERS_SECONDS
@@ -621,19 +621,11 @@ impl DiemDB {
                 },
             )?;
             // Transaction accumulator updates. Get result root hash.
-            let txn_infos = izip!(txns_to_commit, state_root_hashes, event_root_hashes)
-                .map(|(t, s, e)| {
-                    Ok(TransactionInfo::new(
-                        t.transaction().hash(),
-                        s,
-                        e,
-                        t.gas_used(),
-                        t.status().clone(),
-                    ))
-                })
-                .collect::<Result<Vec<_>>>()?;
-            assert_eq!(txn_infos.len(), txns_to_commit.len());
-
+            let txn_infos: Vec<_> = txns_to_commit
+                .iter()
+                .map(|t| t.transaction_info())
+                .cloned()
+                .collect();
             self.ledger_store
                 .put_transaction_infos(first_version, &txn_infos, &mut cs)?
         };
@@ -1277,12 +1269,12 @@ impl DbWriter for DiemDB {
             if let Some(x) = ledger_info_with_sigs {
                 let claimed_last_version = x.ledger_info().version();
                 ensure!(
-                claimed_last_version + 1 == first_version + num_txns,
-                "Transaction batch not applicable: first_version {}, num_txns {}, last_version {}",
-                first_version,
-                num_txns,
-                claimed_last_version,
-            );
+                    claimed_last_version + 1 == first_version + num_txns,
+                    "Transaction batch not applicable: first_version {}, num_txns {}, last_version {}",
+                    first_version,
+                    num_txns,
+                    claimed_last_version,
+                );
             }
 
             // Gather db mutations to `batch`.

storage/diemdb/src/test_helper.rs

@@ -26,8 +26,8 @@ fn to_blocks_to_commit(
     let mut cur_txn_accu_hash = HashValue::zero();
     let blocks_to_commit = partial_blocks
         .into_iter()
-        .map(|(txns_to_commit, partial_ledger_info, validator_set)| {
-            for txn_to_commit in txns_to_commit.iter() {
+        .map(|(mut txns_to_commit, partial_ledger_info, validator_set)| {
+            for txn_to_commit in txns_to_commit.iter_mut() {
                 let mut cs = ChangeSet::new();
 
                 let txn_hash = txn_to_commit.transaction().hash();
@@ -48,6 +48,7 @@ fn to_blocks_to_commit(
                     txn_to_commit.gas_used(),
                     txn_to_commit.status().clone(),
                 );
+                txn_to_commit.set_transaction_info(txn_info.clone());
                 let txn_accu_hash =
                     db.ledger_store
                         .put_transaction_infos(cur_ver, &[txn_info], &mut cs)?;

types/src/proptest_types.rs

@@ -20,8 +20,9 @@ use crate::{
     proof::TransactionInfoListWithProof,
     transaction::{
         ChangeSet, Module, ModuleBundle, RawTransaction, Script, SignatureCheckedTransaction,
-        SignedTransaction, Transaction, TransactionArgument, TransactionListWithProof,
-        TransactionPayload, TransactionStatus, TransactionToCommit, Version, WriteSetPayload,
+        SignedTransaction, Transaction, TransactionArgument, TransactionInfo,
+        TransactionListWithProof, TransactionPayload, TransactionStatus, TransactionToCommit,
+        Version, WriteSetPayload,
     },
     validator_info::ValidatorInfo,
     validator_signer::ValidatorSigner,
@@ -849,14 +850,21 @@ impl TransactionToCommitGen {
             })
             .collect();
 
+        let txn_info = TransactionInfo::new(
+            HashValue::default(),
+            HashValue::default(),
+            HashValue::default(),
+            self.gas_used,
+            self.status,
+        );
+
         TransactionToCommit::new(
             Transaction::UserTransaction(transaction),
+            txn_info,
             account_states,
             None,
             self.write_set,
             events,
-            self.gas_used,
-            self.status,
         )
     }
 }

types/src/transaction/mod.rs

@@ -1085,39 +1085,45 @@ impl Display for TransactionInfo {
 #[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
 pub struct TransactionToCommit {
     transaction: Transaction,
+    transaction_info: TransactionInfo,
     account_states: HashMap<AccountAddress, AccountStateBlob>,
     jf_node_hashes: Option<HashMap<NibblePath, HashValue>>,
     write_set: WriteSet,
     events: Vec<ContractEvent>,
-    gas_used: u64,
-    status: KeptVMStatus,
 }
 
 impl TransactionToCommit {
     pub fn new(
         transaction: Transaction,
+        transaction_info: TransactionInfo,
         account_states: HashMap<AccountAddress, AccountStateBlob>,
         jf_node_hashes: Option<HashMap<NibblePath, HashValue>>,
         write_set: WriteSet,
         events: Vec<ContractEvent>,
-        gas_used: u64,
-        status: KeptVMStatus,
     ) -> Self {
         TransactionToCommit {
             transaction,
+            transaction_info,
             account_states,
             jf_node_hashes,
             write_set,
             events,
-            gas_used,
-            status,
         }
     }
 
     pub fn transaction(&self) -> &Transaction {
         &self.transaction
     }
 
+    pub fn transaction_info(&self) -> &TransactionInfo {
+        &self.transaction_info
+    }
+
+    #[cfg(any(test, feature = "fuzzing"))]
+    pub fn set_transaction_info(&mut self, txn_info: TransactionInfo) {
+        self.transaction_info = txn_info
+    }
+
     pub fn account_states(&self) -> &HashMap<AccountAddress, AccountStateBlob> {
         &self.account_states
     }
@@ -1135,11 +1141,11 @@ impl TransactionToCommit {
     }
 
     pub fn gas_used(&self) -> u64 {
-        self.gas_used
+        self.transaction_info.gas_used
     }
 
     pub fn status(&self) -> &KeptVMStatus {
-        &self.status
+        &self.transaction_info.status
     }
 }