chore: remove duplicate calculation of sender_address and tx_hash in …

…the gateway (#151)

crates/gateway/src/errors.rs

@@ -55,8 +55,6 @@ pub enum GatewayError {
     #[error("Error sending message: {0}")]
     MessageSendError(String),
     #[error(transparent)]
-    StarknetApiError(#[from] StarknetApiError),
-    #[error(transparent)]
     StatefulTransactionValidatorError(#[from] StatefulTransactionValidatorError),
     #[error(transparent)]
     StatelessTransactionValidatorError(#[from] StatelessTransactionValidatorError),

crates/gateway/src/gateway.rs

@@ -10,7 +10,7 @@ use starknet_api::rpc_transaction::RpcTransaction;
 use starknet_api::transaction::TransactionHash;
 use starknet_mempool_infra::component_runner::{ComponentStartError, ComponentStarter};
 use starknet_mempool_types::communication::SharedMempoolClient;
-use starknet_mempool_types::mempool_types::{Account, MempoolInput};
+use starknet_mempool_types::mempool_types::{Account, AccountState, MempoolInput};
 use tracing::{info, instrument};
 
 use crate::compilation::GatewayCompiler;
@@ -134,13 +134,17 @@ fn process_tx(
     };
 
     let validator = stateful_tx_validator.instantiate_validator(state_reader_factory)?;
-    let tx_hash = stateful_tx_validator.run_validate(&tx, optional_class_info, validator)?;
+    // TODO(Yael 31/7/24): refactor after IntrnalTransaction is ready, delete validate_info and
+    // compute all the info outside of run_validate.
+    let validate_info = stateful_tx_validator.run_validate(&tx, optional_class_info, validator)?;
 
     // TODO(Arni): Add the Sierra and the Casm to the mempool input.
-    let sender_address = tx.calculate_sender_address()?;
     Ok(MempoolInput {
-        tx: external_tx_to_thin_tx(&tx, tx_hash, sender_address),
-        account: Account { sender_address, ..Default::default() },
+        tx: external_tx_to_thin_tx(&tx, validate_info.tx_hash, validate_info.sender_address),
+        account: Account {
+            sender_address: validate_info.sender_address,
+            state: AccountState { nonce: validate_info.account_nonce },
+        },
     })
 }
 

crates/gateway/src/stateful_transaction_validator.rs

@@ -19,7 +19,7 @@ use starknet_types_core::felt::Felt;
 use crate::config::StatefulTransactionValidatorConfig;
 use crate::errors::StatefulTransactionValidatorResult;
 use crate::state_reader::{MempoolStateReader, StateReaderFactory};
-use crate::utils::{external_tx_to_account_tx, get_tx_hash};
+use crate::utils::{external_tx_to_account_tx, get_sender_address, get_tx_hash};
 
 #[cfg(test)]
 #[path = "stateful_transaction_validator_test.rs"]
@@ -69,17 +69,18 @@ impl StatefulTransactionValidator {
         external_tx: &RpcTransaction,
         optional_class_info: Option<ClassInfo>,
         mut validator: V,
-    ) -> StatefulTransactionValidatorResult<TransactionHash> {
+    ) -> StatefulTransactionValidatorResult<ValidateInfo> {
         let account_tx = external_tx_to_account_tx(
             external_tx,
             optional_class_info,
             &self.config.chain_info.chain_id,
         )?;
         let tx_hash = get_tx_hash(&account_tx);
-        let account_nonce = validator.get_nonce(external_tx.calculate_sender_address()?)?;
+        let sender_address = get_sender_address(&account_tx);
+        let account_nonce = validator.get_nonce(sender_address)?;
         let skip_validate = skip_stateful_validations(external_tx, account_nonce);
         validator.validate(account_tx, skip_validate)?;
-        Ok(tx_hash)
+        Ok(ValidateInfo { tx_hash, sender_address, account_nonce })
     }
 
     pub fn instantiate_validator(
@@ -131,3 +132,12 @@ pub fn get_latest_block_info(
     let state_reader = state_reader_factory.get_state_reader_from_latest_block();
     Ok(state_reader.get_block_info()?)
 }
+
+/// Holds members created by the stateful transaction validator, needed for
+/// [`MempoolInput`](starknet_mempool_types::mempool_types::MempoolInput).
+#[derive(Debug)]
+pub struct ValidateInfo {
+    pub tx_hash: TransactionHash,
+    pub sender_address: ContractAddress,
+    pub account_nonce: Nonce,
+}

crates/gateway/src/stateful_transaction_validator_test.rs

@@ -1,4 +1,4 @@
-use blockifier::blockifier::stateful_validator::StatefulValidatorError;
+use blockifier::blockifier::stateful_validator::{StatefulValidatorError, StatefulValidatorResult};
 use blockifier::context::BlockContext;
 use blockifier::test_utils::CairoVersion;
 use blockifier::transaction::errors::{TransactionFeeError, TransactionPreValidationError};
@@ -31,6 +31,20 @@ use crate::stateful_transaction_validator::{
     StatefulTransactionValidator,
 };
 
+pub const STATEFUL_VALIDATOR_FEE_ERROR: StatefulValidatorError =
+    StatefulValidatorError::TransactionPreValidationError(
+        TransactionPreValidationError::TransactionFeeError(
+            TransactionFeeError::L1GasBoundsExceedBalance {
+                max_amount: VALID_L1_GAS_MAX_AMOUNT,
+                max_price: VALID_L1_GAS_MAX_PRICE_PER_UNIT,
+                balance: BigUint::ZERO,
+            },
+        ),
+    );
+
+pub const STATEFUL_TRANSACTION_VALIDATOR_FEE_ERROR: StatefulTransactionValidatorError =
+    StatefulTransactionValidatorError::StatefulValidatorError(STATEFUL_VALIDATOR_FEE_ERROR);
+
 #[fixture]
 fn block_context() -> BlockContext {
     BlockContext::create_for_testing()
@@ -51,26 +65,19 @@ fn stateful_validator(block_context: BlockContext) -> StatefulTransactionValidat
 #[rstest]
 #[case::valid_tx(
     invoke_tx(CairoVersion::Cairo1),
+    Ok(()),
     Ok(TransactionHash(felt!(
         "0x152b8dd0c30e95fa3a4ee7a9398fcfc46fb00c048b4fdcfa9958c64d65899b8"
     )))
 )]
 #[case::invalid_tx(
     invoke_tx(CairoVersion::Cairo1),
-    Err(StatefulTransactionValidatorError::StatefulValidatorError(
-        StatefulValidatorError::TransactionPreValidationError(
-            TransactionPreValidationError::TransactionFeeError(
-                TransactionFeeError::L1GasBoundsExceedBalance {
-                    max_amount: VALID_L1_GAS_MAX_AMOUNT,
-                    max_price: VALID_L1_GAS_MAX_PRICE_PER_UNIT,
-                    balance: BigUint::ZERO,
-                }
-            )
-        )
-    ))
+    Err(STATEFUL_VALIDATOR_FEE_ERROR),
+    Err(STATEFUL_TRANSACTION_VALIDATOR_FEE_ERROR)
 )]
 fn test_stateful_tx_validator(
     #[case] external_tx: RpcTransaction,
+    #[case] expected_blockifier_response: StatefulValidatorResult<()>,
     #[case] expected_result: StatefulTransactionValidatorResult<TransactionHash>,
     stateful_validator: StatefulTransactionValidator,
 ) {
@@ -83,18 +90,15 @@ fn test_stateful_tx_validator(
         _ => None,
     };
 
-    let expected_result_msg = format!("{:?}", expected_result);
-
     let mut mock_validator = MockStatefulTransactionValidatorTrait::new();
-    mock_validator.expect_validate().return_once(|_, _| match expected_result {
-        Ok(_) => Ok(()),
-        Err(StatefulTransactionValidatorError::StatefulValidatorError(err)) => Err(err),
-        _ => panic!("Expecting StatefulTransactionValidatorError::StatefulValidatorError"),
-    });
+    mock_validator.expect_validate().return_once(|_, _| expected_blockifier_response);
     mock_validator.expect_get_nonce().returning(|_| Ok(Nonce(Felt::ZERO)));
 
     let result = stateful_validator.run_validate(&external_tx, optional_class_info, mock_validator);
-    assert_eq!(format!("{:?}", result), expected_result_msg);
+    match expected_result {
+        Ok(expected_result) => assert_eq!(result.unwrap().tx_hash, expected_result),
+        Err(_) => assert_eq!(format!("{:?}", result), format!("{:?}", expected_result)),
+    }
 }
 
 #[test]

crates/gateway/src/utils.rs

@@ -115,11 +115,26 @@ pub fn external_tx_to_account_tx(
     }
 }
 
-// TODO(yael 9/5/54): Remove once we we transition to InternalTransaction
+// TODO(yael 9/5/54): Should be implemented as part of InternalTransaction in starknet-api
 pub fn get_tx_hash(tx: &AccountTransaction) -> TransactionHash {
     match tx {
         AccountTransaction::Declare(tx) => tx.tx_hash,
         AccountTransaction::DeployAccount(tx) => tx.tx_hash,
         AccountTransaction::Invoke(tx) => tx.tx_hash,
     }
 }
+
+// TODO(yael 9/5/54): Should be implemented as part of InternalTransaction in starknet-api
+pub fn get_sender_address(tx: &AccountTransaction) -> ContractAddress {
+    match tx {
+        AccountTransaction::Declare(tx) => match &tx.tx {
+            DeclareTransaction::V3(tx) => tx.sender_address,
+            _ => panic!("Unsupported transaction version"),
+        },
+        AccountTransaction::DeployAccount(tx) => tx.contract_address,
+        AccountTransaction::Invoke(tx) => match &tx.tx {
+            InvokeTransaction::V3(tx) => tx.sender_address,
+            _ => panic!("Unsupported transaction version"),
+        },
+    }
+}