[api] add accumulator_root_hash to onchain Transaction for forge fork…

…_check Closes: diem#10041
tnowacki · Dec 17, 2021 · ee0624f · ee0624f
1 parent 2e6733c
commit ee0624f
Show file tree

Hide file tree

Showing 7 changed files with 129 additions and 42 deletions.
diff --git a/api/doc/openapi.yaml b/api/doc/openapi.yaml
@@ -1061,6 +1061,7 @@ components:
         - gas_used
         - success
         - vm_status
+        - accumulator_root_hash
       properties:
         version:
           $ref: '#/components/schemas/Uint64'
@@ -1081,6 +1082,8 @@ components:
           type: string
           description: |
             Human readable transaction execution result message from Diem VM.
+        accumulator_root_hash:
+          $ref: '#/components/schemas/HexEncodedBytes'
     UserTransaction:
       title: User Transaction
       type: object

diff --git a/api/src/context.rs b/api/src/context.rs
@@ -6,9 +6,14 @@ use diem_config::config::{ApiConfig, JsonRpcConfig, RoleType};
 use diem_crypto::HashValue;
 use diem_mempool::{MempoolClientRequest, MempoolClientSender, SubmissionStatus};
 use diem_types::{
-    account_address::AccountAddress, account_state::AccountState,
-    account_state_blob::AccountStateBlob, chain_id::ChainId, contract_event::ContractEvent,
-    event::EventKey, ledger_info::LedgerInfoWithSignatures, transaction::SignedTransaction,
+    account_address::AccountAddress,
+    account_state::AccountState,
+    account_state_blob::AccountStateBlob,
+    chain_id::ChainId,
+    contract_event::ContractEvent,
+    event::EventKey,
+    ledger_info::LedgerInfoWithSignatures,
+    transaction::{SignedTransaction, TransactionWithProof},
 };
 use storage_interface::{MoveDbReader, Order};
 
@@ -138,6 +143,7 @@ impl Context {
         let txns = data.transactions;
         let infos = data.proof.transaction_infos;
         let events = data.events.unwrap_or_default();
+
         ensure!(
             txns.len() == infos.len() && txns.len() == events.len(),
             "invalid data size from database: {}, {}, {}",
@@ -146,13 +152,16 @@ impl Context {
             events.len()
         );
 
-        Ok(txns
-            .into_iter()
+        txns.into_iter()
             .zip(infos.into_iter())
             .zip(events.into_iter())
             .enumerate()
-            .map(|(i, ((txn, info), events))| (start_version + i as u64, txn, info, events).into())
-            .collect())
+            .map(|(i, ((txn, info), events))| {
+                let version = start_version + i as u64;
+                self.get_accumulator_root_hash(version)
+                    .map(|h| (version, txn, info, events, h).into())
+            })
+            .collect()
     }
 
     pub fn get_account_transactions(
@@ -169,18 +178,21 @@ impl Context {
             true,
             ledger_version,
         )?;
-        Ok(txns.into_inner().into_iter().map(|t| t.into()).collect())
+        txns.into_inner()
+            .into_iter()
+            .map(|t| self.convert_into_transaction_on_chain_data(t))
+            .collect::<Result<Vec<_>>>()
     }
 
     pub fn get_transaction_by_hash(
         &self,
         hash: HashValue,
         ledger_version: u64,
     ) -> Result<Option<TransactionOnChainData>> {
-        Ok(self
-            .db
+        self.db
             .get_transaction_by_hash(hash, ledger_version, true)?
-            .map(|t| t.into()))
+            .map(|t| self.convert_into_transaction_on_chain_data(t))
+            .transpose()
     }
 
     pub async fn get_pending_transaction_by_hash(
@@ -203,10 +215,23 @@ impl Context {
         version: u64,
         ledger_version: u64,
     ) -> Result<TransactionOnChainData> {
-        Ok(self
-            .db
-            .get_transaction_by_version(version, ledger_version, true)?
-            .into())
+        self.convert_into_transaction_on_chain_data(self.db.get_transaction_by_version(
+            version,
+            ledger_version,
+            true,
+        )?)
+    }
+
+    pub fn get_accumulator_root_hash(&self, version: u64) -> Result<HashValue> {
+        self.db.get_accumulator_root_hash(version)
+    }
+
+    fn convert_into_transaction_on_chain_data(
+        &self,
+        txn: TransactionWithProof,
+    ) -> Result<TransactionOnChainData> {
+        self.get_accumulator_root_hash(txn.version)
+            .map(|h| (txn, h).into())
     }
 
     pub fn get_events(

diff --git a/api/src/tests/transactions_test.rs b/api/src/tests/transactions_test.rs
@@ -3,7 +3,7 @@
 
 use crate::tests::{assert_json, find_value, new_test_context, pretty, TestContext};
 
-use diem_api_types::HexEncodedBytes;
+use diem_api_types::{HashValue, HexEncodedBytes};
 use diem_crypto::{
     hash::CryptoHash,
     multi_ed25519::{MultiEd25519PrivateKey, MultiEd25519PublicKey},
@@ -291,6 +291,7 @@ async fn test_get_transactions_output_user_transaction_with_script_function_payl
             "previous_block_votes": [],
             "proposer": context.validator_owner.to_hex_literal(),
             "timestamp": metadata_txn.timestamp_usec().to_string(),
+            "accumulator_root_hash": HashValue::from(context.context.get_accumulator_root_hash(1).unwrap()).to_string(),
         }),
     );
 
@@ -353,6 +354,7 @@ async fn test_get_transactions_output_user_transaction_with_script_function_payl
                 "signature": format!("0x{}", hex::encode(sig.to_bytes())),
             },
             "timestamp": metadata_txn.timestamp_usec().to_string(),
+            "accumulator_root_hash": HashValue::from(context.context.get_accumulator_root_hash(2).unwrap()).to_string(),
         }),
     )
 }

diff --git a/api/types/src/convert.rs b/api/types/src/convert.rs
@@ -7,6 +7,7 @@ use crate::{
     ScriptPayload, ScriptWriteSet, Transaction, TransactionInfo, TransactionOnChainData,
     TransactionPayload, UserTransactionRequest, WriteSet, WriteSetChange, WriteSetPayload,
 };
+use diem_crypto::HashValue;
 use diem_transaction_builder::error_explain;
 use diem_types::{
     access_path::{AccessPath, Path},
@@ -74,7 +75,7 @@ impl<'a, R: MoveResolver + ?Sized> MoveConverter<'a, R> {
         data: TransactionOnChainData,
     ) -> Result<Transaction> {
         use diem_types::transaction::Transaction::*;
-        let info = self.into_transaction_info(data.version, &data.info);
+        let info = self.into_transaction_info(data.version, &data.info, data.accumulator_root_hash);
         let events = self.try_into_events(&data.events)?;
         Ok(match data.transaction {
             UserTransaction(txn) => {
@@ -93,6 +94,7 @@ impl<'a, R: MoveResolver + ?Sized> MoveConverter<'a, R> {
         &self,
         version: u64,
         info: &diem_types::transaction::TransactionInfo,
+        accumulator_root_hash: HashValue,
     ) -> TransactionInfo {
         TransactionInfo {
             version: version.into(),
@@ -102,6 +104,7 @@ impl<'a, R: MoveResolver + ?Sized> MoveConverter<'a, R> {
             gas_used: info.gas_used().into(),
             success: info.status().is_success(),
             vm_status: self.explain_vm_status(info.status()),
+            accumulator_root_hash: accumulator_root_hash.into(),
         }
     }
 

diff --git a/api/types/src/transaction.rs b/api/types/src/transaction.rs
@@ -6,6 +6,7 @@ use crate::{
     MoveScriptBytecode, MoveStructTag, MoveType, MoveValue, ScriptFunctionId, U64,
 };
 
+use anyhow::bail;
 use diem_crypto::{
     ed25519::{self, Ed25519PublicKey},
     multi_ed25519::{self, MultiEd25519PublicKey},
@@ -53,15 +54,17 @@ pub struct TransactionOnChainData {
     pub transaction: diem_types::transaction::Transaction,
     pub info: diem_types::transaction::TransactionInfo,
     pub events: Vec<ContractEvent>,
+    pub accumulator_root_hash: diem_crypto::HashValue,
 }
 
-impl From<TransactionWithProof> for TransactionOnChainData {
-    fn from(txn: TransactionWithProof) -> Self {
+impl From<(TransactionWithProof, diem_crypto::HashValue)> for TransactionOnChainData {
+    fn from((txn, accumulator_root_hash): (TransactionWithProof, diem_crypto::HashValue)) -> Self {
         Self {
             version: txn.version,
             transaction: txn.transaction,
             info: txn.proof.transaction_info,
             events: txn.events.unwrap_or_default(),
+            accumulator_root_hash,
         }
     }
 }
@@ -72,21 +75,24 @@ impl
         diem_types::transaction::Transaction,
         diem_types::transaction::TransactionInfo,
         Vec<ContractEvent>,
+        diem_crypto::HashValue,
     )> for TransactionOnChainData
 {
     fn from(
-        (version, transaction, info, events): (
+        (version, transaction, info, events, accumulator_root_hash): (
             u64,
             diem_types::transaction::Transaction,
             diem_types::transaction::TransactionInfo,
             Vec<ContractEvent>,
+            diem_crypto::HashValue,
         ),
     ) -> Self {
         Self {
             version,
             transaction,
             info,
             events,
+            accumulator_root_hash,
         }
     }
 }
@@ -131,6 +137,17 @@ impl Transaction {
             Transaction::GenesisTransaction(txn) => txn.info.vm_status.clone(),
         }
     }
+
+    pub fn transaction_info(&self) -> anyhow::Result<&TransactionInfo> {
+        Ok(match self {
+            Transaction::UserTransaction(txn) => &txn.info,
+            Transaction::BlockMetadataTransaction(txn) => &txn.info,
+            Transaction::PendingTransaction(_txn) => {
+                bail!("pending transaction does not have TransactionInfo")
+            }
+            Transaction::GenesisTransaction(txn) => &txn.info,
+        })
+    }
 }
 
 impl From<(SignedTransaction, TransactionPayload)> for Transaction {
@@ -221,6 +238,7 @@ pub struct TransactionInfo {
     pub gas_used: U64,
     pub success: bool,
     pub vm_status: String,
+    pub accumulator_root_hash: HashValue,
 }
 
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]

diff --git a/crates/diem-rest-client/src/lib.rs b/crates/diem-rest-client/src/lib.rs
@@ -176,7 +176,9 @@ impl Client {
 
         let start = std::time::Instant::now();
         while start.elapsed() < DEFAULT_TIMEOUT {
-            let resp = self.get_transaction_inner(hash).await?;
+            let resp = self
+                .get_transaction_by_version_or_hash(hash.to_hex_literal())
+                .await?;
             if resp.status() != StatusCode::NOT_FOUND {
                 let txn_resp: Response<Transaction> = self.json(resp).await?;
                 let (transaction, state) = txn_resp.into_parts();
@@ -222,13 +224,28 @@ impl Client {
     }
 
     pub async fn get_transaction(&self, hash: HashValue) -> Result<Response<Transaction>> {
-        self.json(self.get_transaction_inner(hash).await?).await
+        self.json(
+            self.get_transaction_by_version_or_hash(hash.to_hex_literal())
+                .await?,
+        )
+        .await
+    }
+
+    pub async fn get_transaction_by_version(&self, version: u64) -> Result<Response<Transaction>> {
+        self.json(
+            self.get_transaction_by_version_or_hash(version.to_string())
+                .await?,
+        )
+        .await
     }
 
-    async fn get_transaction_inner(&self, hash: HashValue) -> Result<reqwest::Response> {
+    async fn get_transaction_by_version_or_hash(
+        &self,
+        version_or_hash: String,
+    ) -> Result<reqwest::Response> {
         let url = self
             .base_url
-            .join(&format!("transactions/{}", hash.to_hex_literal()))?;
+            .join(&format!("transactions/{}", version_or_hash))?;
 
         Ok(self.inner.get(url).send().await?)
     }

diff --git a/testsuite/forge/src/interface/swarm.rs b/testsuite/forge/src/interface/swarm.rs
@@ -4,11 +4,14 @@
 use crate::{ChainInfo, FullNode, NodeExt, Result, Validator, Version};
 use anyhow::anyhow;
 use diem_config::config::NodeConfig;
-use diem_sdk::{client::BlockingClient, types::PeerId};
+use diem_rest_client::Client as RestClient;
+use diem_sdk::types::PeerId;
+use futures::future::try_join_all;
 use std::{
     thread,
     time::{Duration, Instant},
 };
+use tokio::runtime::Runtime;
 
 /// Trait used to represent a running network comprised of Validators and FullNodes
 pub trait Swarm {
@@ -116,31 +119,47 @@ pub trait SwarmExt: Swarm {
     /// Perform a safety check, ensuring that no forks have occurred in the network.
     fn fork_check(&self) -> Result<()> {
         // Checks if root_hashes are equal across all nodes at a given version
-        fn are_root_hashes_equal_at_version(
-            clients: &[BlockingClient],
+        async fn are_root_hashes_equal_at_version(
+            clients: &[RestClient],
             version: u64,
         ) -> Result<bool> {
-            let root_hashes = clients
-                .iter()
-                .map(|node| {
-                    node.get_metadata_by_version(version)
-                        .map(|r| r.into_inner().accumulator_root_hash)
-                })
-                .collect::<Result<Vec<_>, _>>()?;
+            let root_hashes = try_join_all(
+                clients
+                    .iter()
+                    .map(|node| node.get_transaction_by_version(version))
+                    .collect::<Vec<_>>(),
+            )
+            .await?
+            .into_iter()
+            .map(|r| {
+                r.into_inner()
+                    .transaction_info()
+                    .unwrap()
+                    .accumulator_root_hash
+            })
+            .collect::<Vec<_>>();
 
             Ok(root_hashes.windows(2).all(|w| w[0] == w[1]))
         }
 
+        let runtime = Runtime::new().unwrap();
+
         let clients = self
             .validators()
-            .map(|node| node.json_rpc_client())
-            .chain(self.full_nodes().map(|node| node.json_rpc_client()))
+            .map(|node| node.rest_client())
+            .chain(self.full_nodes().map(|node| node.rest_client()))
             .collect::<Vec<_>>();
 
-        let versions = clients
-            .iter()
-            .map(|node| node.get_metadata().map(|r| r.into_inner().version))
-            .collect::<Result<Vec<_>, _>>()?;
+        let versions = runtime
+            .block_on(try_join_all(
+                clients
+                    .iter()
+                    .map(|node| node.get_ledger_information())
+                    .collect::<Vec<_>>(),
+            ))?
+            .into_iter()
+            .map(|resp| resp.into_inner().version)
+            .collect::<Vec<u64>>();
         let min_version = versions
             .iter()
             .min()
@@ -152,7 +171,7 @@ pub trait SwarmExt: Swarm {
             .copied()
             .ok_or_else(|| anyhow!("Unable to query nodes for their latest version"))?;
 
-        if !are_root_hashes_equal_at_version(&clients, min_version)? {
+        if !runtime.block_on(are_root_hashes_equal_at_version(&clients, min_version))? {
             return Err(anyhow!("Fork check failed"));
         }
 
@@ -161,7 +180,7 @@ pub trait SwarmExt: Swarm {
             Instant::now() + Duration::from_secs(10),
         )?;
 
-        if !are_root_hashes_equal_at_version(&clients, max_version)? {
+        if !runtime.block_on(are_root_hashes_equal_at_version(&clients, max_version))? {
             return Err(anyhow!("Fork check failed"));
         }