Merge pull request #1365 from starkware-libs/gilad/account-tx-info

refactor: make `AccountTransactionContext` hold BlockContext (#1365)

Co-Authored-By: Gilad Chase <gilad@starkware.com>

crates/blockifier/src/context.rs

@@ -1,10 +1,16 @@
 use starknet_api::core::{ChainId, ContractAddress};
 
 use crate::block::BlockInfo;
-use crate::transaction::objects::FeeType;
+use crate::transaction::objects::{FeeType, TransactionInfo, TransactionInfoCreator};
 use crate::versioned_constants::VersionedConstants;
 
 /// Create via [`crate::block::pre_process_block`] to ensure correctness.
+#[derive(Clone, Debug)]
+pub struct TransactionContext {
+    pub block_context: BlockContext,
+    pub tx_info: TransactionInfo,
+}
+
 #[derive(Clone, Debug)]
 pub struct BlockContext {
     pub(crate) block_info: BlockInfo,
@@ -37,13 +43,28 @@ impl BlockContext {
     }
 }
 
+impl BlockContext {
+    pub fn to_tx_context(
+        &self,
+        tx_info_creator: &impl TransactionInfoCreator,
+    ) -> TransactionContext {
+        TransactionContext {
+            block_context: self.clone(),
+            tx_info: tx_info_creator.create_tx_info(),
+        }
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct ChainInfo {
     pub chain_id: ChainId,
     pub fee_token_addresses: FeeTokenAddresses,
 }
 
 impl ChainInfo {
+    // TODO(Gilad): since fee_type comes from TransactionInfo, we can move this method into
+    // TransactionContext, which has both the chain_info (through BlockContext) and the tx_info.
+    // That is, add to BlockContext with the signature `pub fn fee_token_address(&self)`.
     pub fn fee_token_address(&self, fee_type: &FeeType) -> ContractAddress {
         self.fee_token_addresses.get_by_fee_type(fee_type)
     }

crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs

@@ -37,7 +37,7 @@ use crate::test_utils::{
 };
 use crate::transaction::constants::QUERY_VERSION_BASE_BIT;
 use crate::transaction::objects::{
-    AccountTransactionContext, CommonAccountFields, DeprecatedAccountTransactionContext,
+    CommonAccountFields, DeprecatedTransactionInfo, TransactionInfo,
 };
 use crate::{check_entry_point_execution_error_for_custom_hint, retdata};
 
@@ -463,20 +463,20 @@ fn test_tx_info(#[case] only_query: bool) {
         calldata: expected_tx_info,
         ..trivial_external_entry_point()
     };
-    let account_tx_context =
-        AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext {
-            common_fields: CommonAccountFields {
-                transaction_hash: tx_hash,
-                version: TransactionVersion::ONE,
-                nonce,
-                sender_address,
-                only_query,
-                ..Default::default()
-            },
-            max_fee,
-        });
+    let tx_info = TransactionInfo::Deprecated(DeprecatedTransactionInfo {
+        common_fields: CommonAccountFields {
+            transaction_hash: tx_hash,
+            version: TransactionVersion::ONE,
+            nonce,
+            sender_address,
+            only_query,
+            ..Default::default()
+        },
+        max_fee,
+    });
+    let limit_steps_by_resources = true;
     let result = entry_point_call
-        .execute_directly_given_account_context(&mut state, account_tx_context, true)
+        .execute_directly_given_tx_info(&mut state, tx_info, limit_steps_by_resources)
         .unwrap();
 
     assert!(!result.execution.failed)

crates/blockifier/src/execution/deprecated_syscalls/hint_processor.rs

@@ -25,6 +25,8 @@ use starknet_api::StarknetApiError;
 use thiserror::Error;
 
 use crate::abi::constants;
+use crate::block::BlockInfo;
+use crate::context::TransactionContext;
 use crate::execution::call_info::{CallInfo, OrderedEvent, OrderedL2ToL1Message};
 use crate::execution::common_hints::{
     extended_builtin_hint_processor, ExecutionMode, HintExecutionResult,
@@ -295,7 +297,7 @@ impl<'a> DeprecatedSyscallHintProcessor<'a> {
         &mut self,
         vm: &mut VirtualMachine,
     ) -> DeprecatedSyscallResult<Relocatable> {
-        let signature = &self.context.account_tx_context.signature().0;
+        let signature = &self.context.tx_context.tx_info.signature().0;
         let signature =
             signature.iter().map(|&x| MaybeRelocatable::from(stark_felt_to_felt(x))).collect();
         let signature_segment_start_ptr = self.read_only_segments.allocate(vm, &signature)?;
@@ -308,18 +310,17 @@ impl<'a> DeprecatedSyscallHintProcessor<'a> {
         vm: &mut VirtualMachine,
     ) -> DeprecatedSyscallResult<Relocatable> {
         let tx_signature_start_ptr = self.get_or_allocate_tx_signature_segment(vm)?;
-        let account_tx_context = &self.context.account_tx_context;
-        let tx_signature_length = account_tx_context.signature().0.len();
+        let TransactionContext { block_context, tx_info } = self.context.tx_context.as_ref();
+        let tx_signature_length = tx_info.signature().0.len();
         let tx_info: Vec<MaybeRelocatable> = vec![
-            stark_felt_to_felt(account_tx_context.signed_version().0).into(),
-            stark_felt_to_felt(*account_tx_context.sender_address().0.key()).into(),
-            max_fee_for_execution_info(account_tx_context).into(),
+            stark_felt_to_felt(tx_info.signed_version().0).into(),
+            stark_felt_to_felt(*tx_info.sender_address().0.key()).into(),
+            max_fee_for_execution_info(tx_info).into(),
             tx_signature_length.into(),
             tx_signature_start_ptr.into(),
-            stark_felt_to_felt(account_tx_context.transaction_hash().0).into(),
-            Felt252::from_bytes_be(self.context.block_context.chain_info.chain_id.0.as_bytes())
-                .into(),
-            stark_felt_to_felt(account_tx_context.nonce().0).into(),
+            stark_felt_to_felt(tx_info.transaction_hash().0).into(),
+            Felt252::from_bytes_be(block_context.chain_info.chain_id.0.as_bytes()).into(),
+            stark_felt_to_felt(tx_info.nonce().0).into(),
         ];
 
         let tx_info_start_ptr = self.read_only_segments.allocate(vm, &tx_info)?;
@@ -347,6 +348,10 @@ impl<'a> DeprecatedSyscallHintProcessor<'a> {
 
         Ok(StorageWriteResponse {})
     }
+
+    pub fn get_block_info(&self) -> &BlockInfo {
+        &self.context.tx_context.block_context.block_info
+    }
 }
 
 impl ResourceTracker for DeprecatedSyscallHintProcessor<'_> {

crates/blockifier/src/execution/deprecated_syscalls/mod.rs

@@ -397,9 +397,7 @@ pub fn get_block_number(
     syscall_handler: &mut DeprecatedSyscallHintProcessor<'_>,
 ) -> DeprecatedSyscallResult<GetBlockNumberResponse> {
     // TODO(Yoni, 1/5/2024): disable for validate.
-    Ok(GetBlockNumberResponse {
-        block_number: syscall_handler.context.block_context.block_info.block_number,
-    })
+    Ok(GetBlockNumberResponse { block_number: syscall_handler.get_block_info().block_number })
 }
 
 // GetBlockTimestamp syscall.
@@ -425,7 +423,7 @@ pub fn get_block_timestamp(
 ) -> DeprecatedSyscallResult<GetBlockTimestampResponse> {
     // TODO(Yoni, 1/5/2024): disable for validate.
     Ok(GetBlockTimestampResponse {
-        block_timestamp: syscall_handler.context.block_context.block_info.block_timestamp,
+        block_timestamp: syscall_handler.get_block_info().block_timestamp,
     })
 }
 
@@ -477,9 +475,7 @@ pub fn get_sequencer_address(
     syscall_handler: &mut DeprecatedSyscallHintProcessor<'_>,
 ) -> DeprecatedSyscallResult<GetSequencerAddressResponse> {
     syscall_handler.verify_not_in_validate_mode("get_sequencer_address")?;
-    Ok(GetSequencerAddressResponse {
-        address: syscall_handler.context.block_context.block_info.sequencer_address,
-    })
+    Ok(GetSequencerAddressResponse { address: syscall_handler.get_block_info().sequencer_address })
 }
 
 // GetTxInfo syscall.
@@ -518,7 +514,7 @@ pub fn get_tx_signature(
     syscall_handler: &mut DeprecatedSyscallHintProcessor<'_>,
 ) -> DeprecatedSyscallResult<GetTxSignatureResponse> {
     let start_ptr = syscall_handler.get_or_allocate_tx_signature_segment(vm)?;
-    let length = syscall_handler.context.account_tx_context.signature().0.len();
+    let length = syscall_handler.context.tx_context.tx_info.signature().0.len();
 
     Ok(GetTxSignatureResponse { segment: ReadOnlySegment { start_ptr, length } })
 }

crates/blockifier/src/execution/entry_point.rs

@@ -13,17 +13,15 @@ use starknet_api::transaction::{Calldata, TransactionVersion};
 
 use crate::abi::abi_utils::selector_from_name;
 use crate::abi::constants;
-use crate::context::BlockContext;
+use crate::context::{BlockContext, TransactionContext};
 use crate::execution::call_info::CallInfo;
 use crate::execution::common_hints::ExecutionMode;
 use crate::execution::deprecated_syscalls::hint_processor::SyscallCounter;
 use crate::execution::errors::{EntryPointExecutionError, PreExecutionError};
 use crate::execution::execution_utils::execute_entry_point_call;
 use crate::fee::os_resources::OS_RESOURCES;
 use crate::state::state_api::State;
-use crate::transaction::objects::{
-    AccountTransactionContext, HasRelatedFeeType, TransactionExecutionResult,
-};
+use crate::transaction::objects::{HasRelatedFeeType, TransactionExecutionResult, TransactionInfo};
 use crate::transaction::transaction_types::TransactionType;
 
 #[cfg(test)]
@@ -68,9 +66,10 @@ impl CallEntryPoint {
         resources: &mut ExecutionResources,
         context: &mut EntryPointExecutionContext,
     ) -> EntryPointExecutionResult<CallInfo> {
+        let tx_context = &context.tx_context;
         let mut decrement_when_dropped = RecursionDepthGuard::new(
             context.current_recursion_depth.clone(),
-            context.block_context.versioned_constants.max_recursion_depth,
+            tx_context.block_context.versioned_constants.max_recursion_depth,
         );
         decrement_when_dropped.try_increment_and_check_depth()?;
 
@@ -86,7 +85,7 @@ impl CallEntryPoint {
             None => storage_class_hash, // If not given, take the storage contract class hash.
         };
         // Hack to prevent version 0 attack on argent accounts.
-        if context.account_tx_context.version() == TransactionVersion::ZERO
+        if tx_context.tx_info.version() == TransactionVersion::ZERO
             && class_hash
                 == ClassHash(
                     StarkFelt::try_from(FAULTY_CLASS_HASH).expect("A class hash must be a felt."),
@@ -139,8 +138,9 @@ pub struct ExecutionResources {
 
 #[derive(Debug)]
 pub struct EntryPointExecutionContext {
-    pub block_context: BlockContext,
-    pub account_tx_context: AccountTransactionContext,
+    // We use `Arc` to avoid the clone of this potentially large object, as inner calls
+    // are created during execution.
+    pub tx_context: Arc<TransactionContext>,
     // VM execution limits.
     pub vm_run_resources: RunResources,
     /// Used for tracking events order during the current execution.
@@ -159,61 +159,46 @@ pub struct EntryPointExecutionContext {
 
 impl EntryPointExecutionContext {
     pub fn new(
-        block_context: &BlockContext,
-        account_tx_context: &AccountTransactionContext,
+        tx_context: Arc<TransactionContext>,
         mode: ExecutionMode,
         limit_steps_by_resources: bool,
     ) -> TransactionExecutionResult<Self> {
-        let max_steps =
-            Self::max_steps(block_context, account_tx_context, &mode, limit_steps_by_resources)?;
+        let max_steps = Self::max_steps(&tx_context, &mode, limit_steps_by_resources)?;
         Ok(Self {
             vm_run_resources: RunResources::new(max_steps),
             n_emitted_events: 0,
             n_sent_messages_to_l1: 0,
             error_stack: vec![],
-            account_tx_context: account_tx_context.clone(),
+            tx_context: tx_context.clone(),
             current_recursion_depth: Default::default(),
-            block_context: block_context.clone(),
             execution_mode: mode,
         })
     }
 
     pub fn new_validate(
-        block_context: &BlockContext,
-        account_tx_context: &AccountTransactionContext,
+        tx_context: Arc<TransactionContext>,
         limit_steps_by_resources: bool,
     ) -> TransactionExecutionResult<Self> {
-        Self::new(
-            block_context,
-            account_tx_context,
-            ExecutionMode::Validate,
-            limit_steps_by_resources,
-        )
+        Self::new(tx_context, ExecutionMode::Validate, limit_steps_by_resources)
     }
 
     pub fn new_invoke(
-        block_context: &BlockContext,
-        account_tx_context: &AccountTransactionContext,
+        tx_context: Arc<TransactionContext>,
         limit_steps_by_resources: bool,
     ) -> TransactionExecutionResult<Self> {
-        Self::new(
-            block_context,
-            account_tx_context,
-            ExecutionMode::Execute,
-            limit_steps_by_resources,
-        )
+        Self::new(tx_context, ExecutionMode::Execute, limit_steps_by_resources)
     }
 
     /// Returns the maximum number of cairo steps allowed, given the max fee, gas price and the
     /// execution mode.
     /// If fee is disabled, returns the global maximum.
     fn max_steps(
-        block_context: &BlockContext,
-        account_tx_context: &AccountTransactionContext,
+        tx_context: &TransactionContext,
         mode: &ExecutionMode,
         limit_steps_by_resources: bool,
     ) -> TransactionExecutionResult<usize> {
-        let versioned_constants = &block_context.versioned_constants;
+        let TransactionContext { block_context, tx_info } = tx_context;
+        let BlockContext { block_info, versioned_constants, .. } = block_context;
         let block_upper_bound = match mode {
             // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the conversion
             // works.
@@ -233,7 +218,7 @@ impl EntryPointExecutionContext {
             ),
         };
 
-        if !limit_steps_by_resources || !account_tx_context.enforce_fee()? {
+        if !limit_steps_by_resources || !tx_info.enforce_fee()? {
             return Ok(block_upper_bound);
         }
 
@@ -246,16 +231,20 @@ impl EntryPointExecutionContext {
 
         // New transactions derive the step limit by the L1 gas resource bounds; deprecated
         // transactions derive this value from the `max_fee`.
-        let tx_gas_upper_bound = match account_tx_context {
-            AccountTransactionContext::Deprecated(context) => {
-                (context.max_fee.0
-                    / block_context
-                        .block_info
-                        .gas_prices
-                        .get_gas_price_by_fee_type(&account_tx_context.fee_type()))
-                    as usize
+        let tx_gas_upper_bound = match tx_info {
+            TransactionInfo::Deprecated(context) => {
+                let max_cairo_steps = context.max_fee.0
+                    / block_info.gas_prices.get_gas_price_by_fee_type(&tx_info.fee_type());
+                // FIXME: This is saturating in the python bootstrapping test. Fix the value so
+                // that it'll fit in a usize and remove the `as`.
+                usize::try_from(max_cairo_steps).unwrap_or_else(|_| {
+                    log::error!(
+                        "Performed a saturating cast from u128 to usize: {max_cairo_steps:?}"
+                    );
+                    usize::MAX
+                })
             }
-            AccountTransactionContext::Current(context) => {
+            TransactionInfo::Current(context) => {
                 // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the
                 // convertion works.
                 context

crates/blockifier/src/execution/execution_utils.rs

@@ -28,7 +28,7 @@ use crate::execution::errors::PostExecutionError;
 use crate::execution::{deprecated_entry_point_execution, entry_point_execution};
 use crate::state::errors::StateError;
 use crate::state::state_api::State;
-use crate::transaction::objects::AccountTransactionContext;
+use crate::transaction::objects::TransactionInfo;
 
 pub type Args = Vec<CairoArg>;
 
@@ -269,10 +269,10 @@ pub fn write_maybe_relocatable<T: Into<MaybeRelocatable>>(
     Ok(())
 }
 
-pub fn max_fee_for_execution_info(account_tx_context: &AccountTransactionContext) -> Felt252 {
-    match account_tx_context {
-        AccountTransactionContext::Current(_) => 0,
-        AccountTransactionContext::Deprecated(context) => context.max_fee.0,
+pub fn max_fee_for_execution_info(tx_info: &TransactionInfo) -> Felt252 {
+    match tx_info {
+        TransactionInfo::Current(_) => 0,
+        TransactionInfo::Deprecated(tx_info) => tx_info.max_fee.0,
     }
     .into()
 }