feat(execution): calculate transaction hashes for execution (#1140)

crates/papyrus_execution/Cargo.toml

@@ -19,6 +19,7 @@ cairo-lang-starknet.workspace = true
 cairo-vm.workspace = true
 itertools.workspace = true
 lazy_static.workspace = true
+papyrus_common = { path = "../papyrus_common", version = "0.0.5" }
 papyrus_config = { path = "../papyrus_config", version = "0.0.5" }
 papyrus_storage = { path = "../papyrus_storage", version = "0.0.5" }
 rand = { workspace = true, optional = true }

crates/papyrus_execution/src/execution_test.rs

@@ -134,7 +134,6 @@ fn execute_call_cairo1() {
 
 // TODO(yair): Compare to the expected fee instead of asserting that it is not zero (all
 // estimate_fee tests).
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn estimate_fee_invoke() {
     let tx = TxsScenarioBuilder::default()
@@ -147,7 +146,6 @@ fn estimate_fee_invoke() {
     }
 }
 
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn estimate_fee_declare_deprecated_class() {
     let tx = TxsScenarioBuilder::default().declare_deprecated_class(*ACCOUNT_ADDRESS).collect();
@@ -159,7 +157,6 @@ fn estimate_fee_declare_deprecated_class() {
     }
 }
 
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn estimate_fee_declare_class() {
     let tx = TxsScenarioBuilder::default().declare_class(*ACCOUNT_ADDRESS).collect();
@@ -171,7 +168,6 @@ fn estimate_fee_declare_class() {
     }
 }
 
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn estimate_fee_deploy_account() {
     let tx = TxsScenarioBuilder::default().deploy_account().collect();
@@ -183,7 +179,6 @@ fn estimate_fee_deploy_account() {
     }
 }
 
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn estimate_fee_combination() {
     let txs = TxsScenarioBuilder::default()
@@ -225,7 +220,6 @@ fn serialization_precision() {
     assert_eq!(input, deserialized);
 }
 
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn simulate_invoke() {
     let ((storage_reader, storage_writer), _temp_dir) = get_test_storage();
@@ -303,7 +297,6 @@ fn simulate_invoke() {
     }
 }
 
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn simulate_declare_deprecated() {
     let ((storage_reader, storage_writer), _temp_dir) = get_test_storage();
@@ -363,7 +356,6 @@ fn simulate_declare_deprecated() {
     }
 }
 
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn simulate_declare() {
     let ((storage_reader, storage_writer), _temp_dir) = get_test_storage();
@@ -423,7 +415,6 @@ fn simulate_declare() {
     }
 }
 
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn simulate_deploy_account() {
     let ((storage_reader, storage_writer), _temp_dir) = get_test_storage();
@@ -497,7 +488,6 @@ fn simulate_deploy_account() {
     }
 }
 
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn simulate_invoke_from_new_account() {
     let ((storage_reader, storage_writer), _temp_dir) = get_test_storage();
@@ -534,7 +524,6 @@ fn simulate_invoke_from_new_account() {
     assert_matches!(invoke_trace.execute_invocation, FunctionInvocationResult::Ok(_));
 }
 
-#[ignore = "need to pass tx hashes"]
 #[test]
 fn simulate_invoke_from_new_account_validate_and_charge() {
     let ((storage_reader, storage_writer), _temp_dir) = get_test_storage();

crates/papyrus_execution/src/lib.rs

@@ -12,7 +12,6 @@ pub mod testing_instances;
 
 pub mod objects;
 use std::collections::{BTreeMap, HashMap};
-use std::iter;
 use std::sync::Arc;
 
 use blockifier::block_context::{BlockContext, FeeTokenAddresses, GasPrices};
@@ -35,6 +34,7 @@ use cairo_lang_starknet::casm_contract_class::CasmContractClass;
 use cairo_vm::types::errors::program_errors::ProgramError;
 use execution_utils::get_trace_constructor;
 use objects::TransactionTrace;
+use papyrus_common::transaction_hash::get_transaction_hash;
 use papyrus_storage::compiled_class::CasmStorageReader;
 use papyrus_storage::db::RO;
 use papyrus_storage::header::HeaderStorageReader;
@@ -62,7 +62,9 @@ use starknet_api::transaction::{
     Transaction,
     TransactionHash,
 };
+use starknet_api::StarknetApiError;
 use state_reader::ExecutionStateReader;
+use tracing::trace;
 
 /// Result type for execution functions.
 pub type ExecutionResult<T> = Result<T, ExecutionError>;
@@ -140,6 +142,8 @@ pub enum ExecutionError {
     NotSynced { state_number: StateNumber, compiled_class_marker: BlockNumber },
     #[error(transparent)]
     StateError(#[from] StateError),
+    #[error("Failed to calculate transaction hash.")]
+    TransactionHashCalculationFailed(StarknetApiError),
     #[error(transparent)]
     PreExecutionError(#[from] PreExecutionError),
     #[error(transparent)]
@@ -273,6 +277,95 @@ pub enum ExecutableTransactionInput {
     L1Handler(L1HandlerTransaction, Fee),
 }
 
+impl ExecutableTransactionInput {
+    fn calc_tx_hash(self, chain_id: &ChainId) -> ExecutionResult<(Self, TransactionHash)> {
+        match self.apply_on_transaction(|tx| get_transaction_hash(tx, chain_id)) {
+            (original_tx, Ok(tx_hash)) => Ok((original_tx, tx_hash)),
+            (_, Err(err)) => Err(ExecutionError::TransactionHashCalculationFailed(err)),
+        }
+    }
+
+    /// Applies a non consuming function on the transaction as if it was of type [Transaction] of
+    /// StarknetAPI and returns the result without cloning the original transaction.
+    // TODO(yair): Refactor this.
+    fn apply_on_transaction<F, T>(self, func: F) -> (Self, T)
+    where
+        F: Fn(&Transaction) -> T,
+    {
+        match self {
+            ExecutableTransactionInput::Invoke(tx) => {
+                let as_transaction = Transaction::Invoke(tx);
+                let res = func(&as_transaction);
+                let Transaction::Invoke(tx) = as_transaction else {
+                    unreachable!("Should be invoke transaction.")
+                };
+                (Self::Invoke(tx), res)
+            }
+            ExecutableTransactionInput::DeclareV0(tx, class) => {
+                let as_transaction = Transaction::Declare(DeclareTransaction::V0(tx));
+                let res = func(&as_transaction);
+                let Transaction::Declare(DeclareTransaction::V0(tx)) = as_transaction else {
+                    unreachable!("Should be declare v0 transaction.")
+                };
+                (Self::DeclareV0(tx, class), res)
+            }
+            ExecutableTransactionInput::DeclareV1(tx, class) => {
+                let as_transaction = Transaction::Declare(DeclareTransaction::V1(tx));
+                let res = func(&as_transaction);
+                let Transaction::Declare(DeclareTransaction::V1(tx)) = as_transaction else {
+                    unreachable!("Should be declare v1 transaction.")
+                };
+                (Self::DeclareV1(tx, class), res)
+            }
+            ExecutableTransactionInput::DeclareV2(tx, class) => {
+                let as_transaction = Transaction::Declare(DeclareTransaction::V2(tx));
+                let res = func(&as_transaction);
+                let Transaction::Declare(DeclareTransaction::V2(tx)) = as_transaction else {
+                    unreachable!("Should be declare v2 transaction.")
+                };
+                (Self::DeclareV2(tx, class), res)
+            }
+            ExecutableTransactionInput::DeclareV3(tx, class) => {
+                let as_transaction = Transaction::Declare(DeclareTransaction::V3(tx));
+                let res = func(&as_transaction);
+                let Transaction::Declare(DeclareTransaction::V3(tx)) = as_transaction else {
+                    unreachable!("Should be declare v3 transaction.")
+                };
+                (Self::DeclareV3(tx, class), res)
+            }
+            ExecutableTransactionInput::DeployAccount(tx) => {
+                let as_transaction = Transaction::DeployAccount(tx);
+                let res = func(&as_transaction);
+                let Transaction::DeployAccount(tx) = as_transaction else {
+                    unreachable!("Should be deploy account transaction.")
+                };
+                (Self::DeployAccount(tx), res)
+            }
+            ExecutableTransactionInput::L1Handler(tx, fee) => {
+                let as_transaction = Transaction::L1Handler(tx);
+                let res = func(&as_transaction);
+                let Transaction::L1Handler(tx) = as_transaction else {
+                    unreachable!("Should be L1 handler transaction.")
+                };
+                (Self::L1Handler(tx, fee), res)
+            }
+        }
+    }
+}
+
+/// Calculates the transaction hashes for a series of transactions without cloning the transactions.
+fn calc_tx_hashes(
+    txs: Vec<ExecutableTransactionInput>,
+    chain_id: &ChainId,
+) -> ExecutionResult<(Vec<ExecutableTransactionInput>, Vec<TransactionHash>)> {
+    Ok(txs
+        .into_iter()
+        .map(|tx| tx.calc_tx_hash(chain_id))
+        .collect::<Result<Vec<_>, _>>()?
+        .into_iter()
+        .unzip())
+}
+
 /// Returns the fee estimation for a series of transactions.
 pub fn estimate_fee(
     txs: Vec<ExecutableTransactionInput>,
@@ -333,13 +426,17 @@ fn execute_transactions(
         execution_config,
     );
 
-    let tx_hashes_iter: Box<dyn Iterator<Item = Option<TransactionHash>>> = match tx_hashes {
-        Some(hashes) => Box::new(hashes.into_iter().map(Some)),
-        None => Box::new(iter::repeat(None)),
+    let (txs, tx_hashes) = match tx_hashes {
+        Some(tx_hashes) => (txs, tx_hashes),
+        None => {
+            let tx_hashes = calc_tx_hashes(txs, chain_id)?;
+            trace!("Calculated tx hashes: {:?}", tx_hashes);
+            tx_hashes
+        }
     };
 
     let mut res = vec![];
-    for (tx, tx_hash) in txs.into_iter().zip(tx_hashes_iter) {
+    for (tx, tx_hash) in txs.into_iter().zip(tx_hashes.into_iter()) {
         let blockifier_tx = to_blockifier_tx(tx, tx_hash)?;
         let tx_execution_info =
             blockifier_tx.execute(&mut cached_state, &block_context, charge_fee, validate)?;
@@ -351,10 +448,8 @@ fn execute_transactions(
 
 fn to_blockifier_tx(
     tx: ExecutableTransactionInput,
-    tx_hash: Option<TransactionHash>,
+    tx_hash: TransactionHash,
 ) -> ExecutionResult<BlockifierTransaction> {
-    // TODO(yair): Remove the unwrap once the blockifier calculates the tx hash.
-    let tx_hash = tx_hash.unwrap_or_default();
     match tx {
         ExecutableTransactionInput::Invoke(invoke_tx) => Ok(BlockifierTransaction::from_api(
             Transaction::Invoke(invoke_tx),

crates/papyrus_rpc/src/v0_4_0/api/api_impl.rs

@@ -628,18 +628,13 @@ impl JsonRpcV0_4Server for JsonRpcServerV0_4Impl {
         transactions: Vec<BroadcastedTransaction>,
         block_id: BlockId,
     ) -> RpcResult<Vec<FeeEstimate>> {
-        // TODO(yair): remove this once the transaction hash is calculated in the blockifier.
-        if true {
-            return Err(internal_server_error("Fee estimation not supported yet."));
-        }
-
         trace!("Estimating fee of transactions: {:#?}", transactions);
         let executable_txns =
             transactions.into_iter().map(|tx| tx.try_into()).collect::<Result<_, _>>()?;
 
         let txn = self.storage_reader.begin_ro_txn().map_err(internal_server_error)?;
         let block_number = get_block_number(&txn, block_id)?;
-        let state_number = StateNumber::right_after_block(block_number);
+        let state_number = StateNumber::right_before_block(block_number);
         let block_execution_config =
             self.execution_config.get_execution_config_for_block(block_number).map_err(|err| {
                 internal_server_error(format!("Failed to get execution config: {}", err))
@@ -668,11 +663,6 @@ impl JsonRpcV0_4Server for JsonRpcServerV0_4Impl {
         transactions: Vec<BroadcastedTransaction>,
         simulation_flags: Vec<SimulationFlag>,
     ) -> RpcResult<Vec<SimulatedTransaction>> {
-        // TODO(yair): remove this once the transaction hash is calculated in the blockifier.
-        if true {
-            return Err(internal_server_error("Simulate transactions not supported yet."));
-        }
-
         trace!("Simulating transactions: {:#?}", transactions);
         let executable_txns =
             transactions.into_iter().map(|tx| tx.try_into()).collect::<Result<_, _>>()?;

crates/papyrus_rpc/src/v0_4_0/execution_test.rs

@@ -149,7 +149,6 @@ async fn execution_call() {
     assert_matches!(err, Error::Call(err) if err == CONTRACT_ERROR.into());
 }
 
-#[ignore = "need to pass tx hashes"]
 #[tokio::test]
 async fn call_estimate_fee() {
     let (module, storage_writer) =
@@ -183,15 +182,14 @@ async fn call_estimate_fee() {
     // TODO(yair): verify this is the correct fee, got this value by printing the result of the
     // call.
     let expected_fee_estimate = vec![FeeEstimate {
-        gas_consumed: stark_felt!("0x19a2"),
+        gas_consumed: stark_felt!("0x9ba"),
         gas_price: *GAS_PRICE,
-        overall_fee: Fee(656200000000000),
+        overall_fee: Fee(249000000000000),
     }];
 
     assert_eq!(res, expected_fee_estimate);
 }
 
-#[ignore = "need to pass tx hashes"]
 #[tokio::test]
 async fn call_simulate() {
     let (module, storage_writer) =
@@ -232,9 +230,9 @@ async fn call_simulate() {
     // call.
     // Why is it different from the estimate_fee call?
     let expected_fee_estimate = FeeEstimate {
-        gas_consumed: stark_felt!("0x19b7"),
+        gas_consumed: stark_felt!("0x9ba"),
         gas_price: *GAS_PRICE,
-        overall_fee: Fee(658300000000000),
+        overall_fee: Fee(249000000000000),
     };
 
     assert_eq!(simulated_tx.fee_estimation, expected_fee_estimate);
@@ -250,7 +248,6 @@ async fn call_simulate() {
     assert_matches!(invoke_trace.fee_transfer_invocation, Some(_));
 }
 
-#[ignore = "need to pass tx hashes"]
 #[tokio::test]
 async fn call_simulate_skip_validate() {
     let (module, storage_writer) =
@@ -291,9 +288,9 @@ async fn call_simulate_skip_validate() {
     // call.
     // Why is it different from the estimate_fee call?
     let expected_fee_estimate = FeeEstimate {
-        gas_consumed: stark_felt!("0x19a2"),
+        gas_consumed: stark_felt!("0x9ba"),
         gas_price: *GAS_PRICE,
-        overall_fee: Fee(656200000000000),
+        overall_fee: Fee(249000000000000),
     };
 
     assert_eq!(simulated_tx.fee_estimation, expected_fee_estimate);
@@ -309,7 +306,6 @@ async fn call_simulate_skip_validate() {
     assert_matches!(invoke_trace.fee_transfer_invocation, Some(_));
 }
 
-#[ignore = "need to pass tx hashes"]
 #[tokio::test]
 async fn call_simulate_skip_fee_charge() {
     let (module, storage_writer) =
@@ -350,9 +346,9 @@ async fn call_simulate_skip_fee_charge() {
     // call.
     // Why is it different from the estimate_fee call?
     let expected_fee_estimate = FeeEstimate {
-        gas_consumed: stark_felt!("0x19b7"),
+        gas_consumed: stark_felt!("9ba"),
         gas_price: *GAS_PRICE,
-        overall_fee: Fee(658300000000000),
+        overall_fee: Fee(249000000000000),
     };
 
     assert_eq!(simulated_tx.fee_estimation, expected_fee_estimate);