From c56a17157c8d0630cf5fbe8e9d6a471207a44c1c Mon Sep 17 00:00:00 2001 From: Nimrod Weiss Date: Sun, 18 Aug 2024 14:30:24 +0300 Subject: [PATCH] build(fee): current tx_info holds validResourceBounds --- .../src/concurrency/versioned_state_test.rs | 2 +- .../blockifier/src/execution/entry_point.rs | 27 +++++----- .../src/execution/syscalls/hint_processor.rs | 52 ++++++++++++------- .../syscall_tests/get_execution_info.rs | 24 ++++++++- crates/blockifier/src/fee/actual_cost.rs | 2 +- crates/blockifier/src/fee/fee_checks.rs | 14 ++--- crates/blockifier/src/fee/fee_utils.rs | 4 +- crates/blockifier/src/test_utils/prices.rs | 3 +- .../blockifier/src/test_utils/struct_impls.rs | 6 +-- .../src/transaction/account_transaction.rs | 24 ++++----- .../transaction/account_transactions_test.rs | 6 +-- crates/blockifier/src/transaction/objects.rs | 23 ++++---- .../src/transaction/transaction_execution.rs | 2 +- .../src/transaction/transactions.rs | 6 +-- .../src/transaction/transactions_test.rs | 2 +- crates/papyrus_execution/src/lib.rs | 3 +- crates/starknet_api/src/transaction.rs | 1 + 17 files changed, 114 insertions(+), 87 deletions(-) diff --git a/crates/blockifier/src/concurrency/versioned_state_test.rs b/crates/blockifier/src/concurrency/versioned_state_test.rs index 71a6c460e1..f3e0ff0bf3 100644 --- a/crates/blockifier/src/concurrency/versioned_state_test.rs +++ b/crates/blockifier/src/concurrency/versioned_state_test.rs @@ -233,7 +233,7 @@ fn test_run_parallel_txs(max_resource_bounds: DeprecatedResourceBoundsMapping) { &mut NonceManager::default(), ); let account_tx_1 = AccountTransaction::DeployAccount(deploy_account_tx_1); - let enforce_fee = account_tx_1.create_tx_info().enforce_fee().unwrap(); + let enforce_fee = account_tx_1.create_tx_info().enforce_fee(); let class_hash = grindy_account.get_class_hash(); let ctor_storage_arg = felt!(1_u8); diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index a2b1482f53..2e55de33ef 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -22,7 +22,7 @@ use crate::execution::errors::{ }; use crate::execution::execution_utils::execute_entry_point_call; use crate::state::state_api::State; -use crate::transaction::objects::{HasRelatedFeeType, TransactionExecutionResult, TransactionInfo}; +use crate::transaction::objects::{HasRelatedFeeType, TransactionInfo}; use crate::transaction::transaction_types::TransactionType; use crate::utils::{u128_from_usize, usize_from_u128}; use crate::versioned_constants::{GasCosts, VersionedConstants}; @@ -135,29 +135,26 @@ impl EntryPointExecutionContext { tx_context: Arc, mode: ExecutionMode, limit_steps_by_resources: bool, - ) -> TransactionExecutionResult { - let max_steps = Self::max_steps(&tx_context, &mode, limit_steps_by_resources)?; - Ok(Self { + ) -> Self { + let max_steps = Self::max_steps(&tx_context, &mode, limit_steps_by_resources); + Self { vm_run_resources: RunResources::new(max_steps), n_emitted_events: 0, n_sent_messages_to_l1: 0, tx_context: tx_context.clone(), current_recursion_depth: Default::default(), execution_mode: mode, - }) + } } pub fn new_validate( tx_context: Arc, limit_steps_by_resources: bool, - ) -> TransactionExecutionResult { + ) -> Self { Self::new(tx_context, ExecutionMode::Validate, limit_steps_by_resources) } - pub fn new_invoke( - tx_context: Arc, - limit_steps_by_resources: bool, - ) -> TransactionExecutionResult { + pub fn new_invoke(tx_context: Arc, limit_steps_by_resources: bool) -> Self { Self::new(tx_context, ExecutionMode::Execute, limit_steps_by_resources) } @@ -168,7 +165,7 @@ impl EntryPointExecutionContext { tx_context: &TransactionContext, mode: &ExecutionMode, limit_steps_by_resources: bool, - ) -> TransactionExecutionResult { + ) -> usize { let TransactionContext { block_context, tx_info } = tx_context; let BlockContext { block_info, versioned_constants, .. } = block_context; let block_upper_bound = match mode { @@ -184,8 +181,8 @@ impl EntryPointExecutionContext { .expect("Failed to convert invoke_tx_max_n_steps (u32) to usize."), }; - if !limit_steps_by_resources || !tx_info.enforce_fee()? { - return Ok(block_upper_bound); + if !limit_steps_by_resources || !tx_info.enforce_fee() { + return block_upper_bound; } let gas_per_step = versioned_constants @@ -214,7 +211,7 @@ impl EntryPointExecutionContext { // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the // convertion works. context - .l1_resource_bounds()? + .l1_resource_bounds() .max_amount .try_into() .expect("Failed to convert u64 to usize.") @@ -236,7 +233,7 @@ impl EntryPointExecutionContext { ); usize::MAX }); - Ok(min(tx_upper_bound, block_upper_bound)) + min(tx_upper_bound, block_upper_bound) } /// Returns the available steps in run resources. diff --git a/crates/blockifier/src/execution/syscalls/hint_processor.rs b/crates/blockifier/src/execution/syscalls/hint_processor.rs index df847f4c59..146f3183f7 100644 --- a/crates/blockifier/src/execution/syscalls/hint_processor.rs +++ b/crates/blockifier/src/execution/syscalls/hint_processor.rs @@ -17,7 +17,7 @@ use cairo_vm::vm::vm_core::VirtualMachine; use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector}; use starknet_api::deprecated_contract_class::EntryPointType; use starknet_api::state::StorageKey; -use starknet_api::transaction::{Calldata, Resource}; +use starknet_api::transaction::{AllResourceBounds, Calldata, ValidResourceBounds}; use starknet_api::StarknetApiError; use starknet_types_core::felt::{Felt, FromStrError}; use thiserror::Error; @@ -205,6 +205,8 @@ pub const INVALID_ARGUMENT: &str = pub const L1_GAS: &str = "0x00000000000000000000000000000000000000000000000000004c315f474153"; // "L2_GAS"; pub const L2_GAS: &str = "0x00000000000000000000000000000000000000000000000000004c325f474153"; +// "L1_DATA_GAS" +pub const L1_DATA_GAS: &str = "0x0000000000000000000000000000000000000000004c315f444154415f474153"; /// Executes Starknet syscalls (stateful protocol hints) during the execution of an entry point /// call. @@ -462,26 +464,40 @@ impl<'a> SyscallHintProcessor<'a> { vm: &mut VirtualMachine, tx_info: &CurrentTransactionInfo, ) -> SyscallResult<(Relocatable, Relocatable)> { - let l1_gas = Felt::from_hex(L1_GAS).map_err(SyscallExecutionError::from)?; - let l2_gas = Felt::from_hex(L2_GAS).map_err(SyscallExecutionError::from)?; - let flat_resource_bounds: Vec = tx_info - .resource_bounds - .0 - .iter() - .flat_map(|(resource, resource_bounds)| { - let resource = match resource { - Resource::L1Gas => l1_gas, - Resource::L2Gas => l2_gas, - Resource::L1DataGas => todo!(), - }; + let l1_gas_as_felt = Felt::from_hex(L1_GAS).map_err(SyscallExecutionError::from)?; + let l2_gas_as_felt = Felt::from_hex(L2_GAS).map_err(SyscallExecutionError::from)?; + let l1_data_gas_as_felt = + Felt::from_hex(L1_DATA_GAS).map_err(SyscallExecutionError::from)?; + let flat_resource_bounds = match tx_info.resource_bounds { + ValidResourceBounds::L1Gas(l1_bounds) => { vec![ - resource, - Felt::from(resource_bounds.max_amount), - Felt::from(resource_bounds.max_price_per_unit), + l1_gas_as_felt, + Felt::from(l1_bounds.max_amount), + Felt::from(l1_bounds.max_price_per_unit), + l2_gas_as_felt, + Felt::ZERO, + Felt::ZERO, ] - }) - .collect(); + } + ValidResourceBounds::AllResources(AllResourceBounds { + l1_gas, + l2_gas, + l1_data_gas, + }) => { + vec![ + l1_gas_as_felt, + Felt::from(l1_gas.max_amount), + Felt::from(l1_gas.max_price_per_unit), + l2_gas_as_felt, + Felt::from(l2_gas.max_amount), + Felt::from(l2_gas.max_price_per_unit), + l1_data_gas_as_felt, + Felt::from(l1_data_gas.max_amount), + Felt::from(l1_data_gas.max_price_per_unit), + ] + } + }; self.allocate_data_segment(vm, &flat_resource_bounds) } diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs index fb6f4d0c9d..7e505bd03d 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs @@ -24,7 +24,7 @@ use crate::abi::abi_utils::selector_from_name; use crate::context::ChainInfo; use crate::execution::common_hints::ExecutionMode; use crate::execution::entry_point::CallEntryPoint; -use crate::execution::syscalls::hint_processor::{L1_GAS, L2_GAS}; +use crate::execution::syscalls::hint_processor::{L1_DATA_GAS, L1_GAS, L2_GAS}; use crate::nonce; use crate::test_utils::contracts::FeatureContract; use crate::test_utils::initial_test_state::test_state; @@ -227,7 +227,9 @@ fn test_get_execution_info( }, ), (Resource::L2Gas, ResourceBounds { max_amount: 0, max_price_per_unit: 0 }), - ])), + ])) + .try_into() + .unwrap(), tip: Tip::default(), nonce_data_availability_mode: DataAvailabilityMode::L1, fee_data_availability_mode: DataAvailabilityMode::L1, @@ -269,3 +271,21 @@ fn test_get_execution_info( assert!(!result.unwrap().execution.failed); } + +#[test] +fn test_gas_types_constants() { + assert_eq!(str_to_32_bytes_in_hex("L1_GAS"), L1_GAS); + assert_eq!(str_to_32_bytes_in_hex("L2_GAS"), L2_GAS); + assert_eq!(str_to_32_bytes_in_hex("L1_DATA_GAS"), L1_DATA_GAS); +} + +fn str_to_32_bytes_in_hex(s: &str) -> String { + if s.len() > 32 { + panic!("Unsupported input of length > 32.") + } + let prefix = "0x"; + let padding_zeros = "0".repeat(64 - s.len() * 2); // Each string char is 2 chars in hex. + let word_in_hex: String = + s.as_bytes().iter().fold(String::new(), |s, byte| s + (&format!("{:02x}", byte))); + [prefix, &padding_zeros, &word_in_hex].into_iter().collect() +} diff --git a/crates/blockifier/src/fee/actual_cost.rs b/crates/blockifier/src/fee/actual_cost.rs index 36967058b8..1e4802216f 100644 --- a/crates/blockifier/src/fee/actual_cost.rs +++ b/crates/blockifier/src/fee/actual_cost.rs @@ -91,7 +91,7 @@ impl TransactionReceipt { )?; // L1 handler transactions are not charged an L2 fee but it is compared to the L1 fee. - let fee = if tx_context.tx_info.enforce_fee()? || tx_type == TransactionType::L1Handler { + let fee = if tx_context.tx_info.enforce_fee() || tx_type == TransactionType::L1Handler { tx_context.tx_info.get_fee_by_gas_vector(&tx_context.block_context.block_info, gas) } else { Fee(0) diff --git a/crates/blockifier/src/fee/fee_checks.rs b/crates/blockifier/src/fee/fee_checks.rs index d09c96de80..4cbcf6bd31 100644 --- a/crates/blockifier/src/fee/fee_checks.rs +++ b/crates/blockifier/src/fee/fee_checks.rs @@ -61,7 +61,7 @@ impl FeeCheckReport { actual_fee: Fee, error: FeeCheckError, tx_context: &TransactionContext, - ) -> TransactionExecutionResult { + ) -> Self { let recommended_fee = match error { // If the error is insufficient balance, the recommended fee is the actual fee. // This recommendation assumes (a) the pre-validation checks were applied and pass (i.e. @@ -75,14 +75,14 @@ impl FeeCheckReport { match &tx_context.tx_info { TransactionInfo::Current(info) => get_fee_by_gas_vector( &tx_context.block_context.block_info, - GasVector::from_l1_gas(info.l1_resource_bounds()?.max_amount.into()), + GasVector::from_l1_gas(info.l1_resource_bounds().max_amount.into()), &FeeType::Strk, ), TransactionInfo::Deprecated(context) => context.max_fee, } } }; - Ok(Self { recommended_fee, error: Some(error) }) + Self { recommended_fee, error: Some(error) } } /// If the actual cost exceeds the resource bounds on the transaction, returns a fee check @@ -100,7 +100,7 @@ impl FeeCheckReport { match tx_info { TransactionInfo::Current(context) => { // Check L1 gas limit. - let max_l1_gas = context.l1_resource_bounds()?.max_amount.into(); + let max_l1_gas = context.l1_resource_bounds().max_amount.into(); // TODO(Dori, 1/7/2024): When data gas limit is added (and enforced) in resource // bounds, check it here as well (separately, with a different error variant if @@ -172,7 +172,7 @@ impl PostValidationReport { tx_receipt: &TransactionReceipt, ) -> TransactionExecutionResult<()> { // If fee is not enforced, no need to check post-execution. - if !tx_context.tx_info.enforce_fee()? { + if !tx_context.tx_info.enforce_fee() { return Ok(()); } @@ -192,7 +192,7 @@ impl PostExecutionReport { let TransactionReceipt { fee, .. } = tx_receipt; // If fee is not enforced, no need to check post-execution. - if !charge_fee || !tx_context.tx_info.enforce_fee()? { + if !charge_fee || !tx_context.tx_info.enforce_fee() { return Ok(Self(FeeCheckReport::success_report(*fee))); } @@ -216,7 +216,7 @@ impl PostExecutionReport { *fee, fee_check_error, tx_context, - )?)); + ))); } Err(other_error) => return Err(other_error), } diff --git a/crates/blockifier/src/fee/fee_utils.rs b/crates/blockifier/src/fee/fee_utils.rs index cce1b85227..d725e06c2a 100644 --- a/crates/blockifier/src/fee/fee_utils.rs +++ b/crates/blockifier/src/fee/fee_utils.rs @@ -107,7 +107,7 @@ pub fn verify_can_pay_committed_bounds( let tx_info = &tx_context.tx_info; let committed_fee = match tx_info { TransactionInfo::Current(context) => { - let l1_bounds = context.l1_resource_bounds()?; + let l1_bounds = context.l1_resource_bounds(); let max_amount: u128 = l1_bounds.max_amount.into(); // Sender will not be charged by `max_price_per_unit`, but this check should not depend // on the current gas price. @@ -122,7 +122,7 @@ pub fn verify_can_pay_committed_bounds( } else { Err(match tx_info { TransactionInfo::Current(context) => { - let l1_bounds = context.l1_resource_bounds()?; + let l1_bounds = context.l1_resource_bounds(); TransactionFeeError::L1GasBoundsExceedBalance { max_amount: l1_bounds.max_amount, max_price: l1_bounds.max_price_per_unit, diff --git a/crates/blockifier/src/test_utils/prices.rs b/crates/blockifier/src/test_utils/prices.rs index bc2592aade..f605d42ff4 100644 --- a/crates/blockifier/src/test_utils/prices.rs +++ b/crates/blockifier/src/test_utils/prices.rs @@ -75,8 +75,7 @@ fn fee_transfer_resources( Arc::new(block_context.to_tx_context(&account_invoke_tx(InvokeTxArgs::default()))), ExecutionMode::Execute, false, - ) - .unwrap(), + ), ) .unwrap() .resources diff --git a/crates/blockifier/src/test_utils/struct_impls.rs b/crates/blockifier/src/test_utils/struct_impls.rs index 664041671c..594b682435 100644 --- a/crates/blockifier/src/test_utils/struct_impls.rs +++ b/crates/blockifier/src/test_utils/struct_impls.rs @@ -70,8 +70,7 @@ impl CallEntryPoint { let tx_context = TransactionContext { block_context: BlockContext::create_for_testing(), tx_info }; let mut context = - EntryPointExecutionContext::new_invoke(Arc::new(tx_context), limit_steps_by_resources) - .unwrap(); + EntryPointExecutionContext::new_invoke(Arc::new(tx_context), limit_steps_by_resources); self.execute(state, &mut ExecutionResources::default(), &mut context) } @@ -99,8 +98,7 @@ impl CallEntryPoint { let mut context = EntryPointExecutionContext::new_validate( Arc::new(tx_context), limit_steps_by_resources, - ) - .unwrap(); + ); self.execute(state, &mut ExecutionResources::default(), &mut context) } } diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index fec8820e94..0dbfe9dfc1 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -215,7 +215,7 @@ impl AccountTransaction { let tx_info = &tx_context.tx_info; Self::handle_nonce(state, tx_info, strict_nonce_check)?; - if charge_fee && tx_info.enforce_fee()? { + if charge_fee && tx_info.enforce_fee() { self.check_fee_bounds(tx_context)?; verify_can_pay_committed_bounds(state, tx_context)?; @@ -242,7 +242,7 @@ impl AccountTransaction { let ResourceBounds { max_amount: max_l1_gas_amount, max_price_per_unit: max_l1_gas_price, - } = context.l1_resource_bounds()?; + } = context.l1_resource_bounds(); let max_l1_gas_amount_as_u128: u128 = max_l1_gas_amount.into(); if max_l1_gas_amount_as_u128 < minimal_l1_gas_amount { @@ -320,16 +320,13 @@ impl AccountTransaction { } } - fn assert_actual_fee_in_bounds( - tx_context: &Arc, - actual_fee: Fee, - ) -> TransactionExecutionResult<()> { + fn assert_actual_fee_in_bounds(tx_context: &Arc, actual_fee: Fee) { match &tx_context.tx_info { TransactionInfo::Current(context) => { let ResourceBounds { max_amount: max_l1_gas_amount, max_price_per_unit: max_l1_gas_price, - } = context.l1_resource_bounds()?; + } = context.l1_resource_bounds(); if actual_fee > Fee(u128::from(max_l1_gas_amount) * max_l1_gas_price) { panic!( "Actual fee {:#?} exceeded bounds; max amount is {:#?}, max price is @@ -347,7 +344,6 @@ impl AccountTransaction { } } } - Ok(()) } fn handle_fee( @@ -364,7 +360,7 @@ impl AccountTransaction { } // TODO(Amos, 8/04/2024): Add test for this assert. - Self::assert_actual_fee_in_bounds(&tx_context, actual_fee)?; + Self::assert_actual_fee_in_bounds(&tx_context, actual_fee); let fee_transfer_call_info = if concurrency_mode && !tx_context.is_sequencer_the_sender() { Self::concurrency_execute_fee_transfer(state, tx_context, actual_fee)? @@ -404,7 +400,7 @@ impl AccountTransaction { initial_gas: block_context.versioned_constants.os_constants.gas_costs.initial_gas_cost, }; - let mut context = EntryPointExecutionContext::new_invoke(tx_context, true)?; + let mut context = EntryPointExecutionContext::new_invoke(tx_context, true); Ok(fee_transfer_call .execute(state, &mut ExecutionResources::default(), &mut context) @@ -473,7 +469,7 @@ impl AccountTransaction { // Also, the execution context required form the `DeployAccount` execute phase is // validation context. let mut execution_context = - EntryPointExecutionContext::new_validate(tx_context.clone(), charge_fee)?; + EntryPointExecutionContext::new_validate(tx_context.clone(), charge_fee); execute_call_info = self.run_execute(state, &mut resources, &mut execution_context, remaining_gas)?; validate_call_info = self.handle_validate_tx( @@ -486,7 +482,7 @@ impl AccountTransaction { )?; } else { let mut execution_context = - EntryPointExecutionContext::new_invoke(tx_context.clone(), charge_fee)?; + EntryPointExecutionContext::new_invoke(tx_context.clone(), charge_fee); validate_call_info = self.handle_validate_tx( state, &mut resources, @@ -530,7 +526,7 @@ impl AccountTransaction { ) -> TransactionExecutionResult { let mut resources = ExecutionResources::default(); let mut execution_context = - EntryPointExecutionContext::new_invoke(tx_context.clone(), charge_fee)?; + EntryPointExecutionContext::new_invoke(tx_context.clone(), charge_fee); // Run the validation, and if execution later fails, only keep the validation diff. let validate_call_info = self.handle_validate_tx( state, @@ -791,7 +787,7 @@ impl ValidatableTransaction for AccountTransaction { limit_steps_by_resources: bool, ) -> TransactionExecutionResult> { let mut context = - EntryPointExecutionContext::new_validate(tx_context, limit_steps_by_resources)?; + EntryPointExecutionContext::new_validate(tx_context, limit_steps_by_resources); let tx_info = &context.tx_context.tx_info; if tx_info.is_v0() { return Ok(None); diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 933c2bae65..c745d2347c 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -174,7 +174,7 @@ fn test_fee_enforcement( ); let account_tx = AccountTransaction::DeployAccount(deploy_account_tx); - let enforce_fee = account_tx.create_tx_info().enforce_fee().unwrap(); + let enforce_fee = account_tx.create_tx_info().enforce_fee(); let result = account_tx.execute(state, &block_context, true, true); assert_eq!(result.is_err(), enforce_fee); } @@ -933,7 +933,7 @@ fn test_max_fee_to_max_steps_conversion( nonce: nonce_manager.next(account_address), }); let tx_context1 = Arc::new(block_context.to_tx_context(&account_tx1)); - let execution_context1 = EntryPointExecutionContext::new_invoke(tx_context1, true).unwrap(); + let execution_context1 = EntryPointExecutionContext::new_invoke(tx_context1, true); let max_steps_limit1 = execution_context1.vm_run_resources.get_n_steps(); let tx_execution_info1 = account_tx1.execute(&mut state, &block_context, true, true).unwrap(); let n_steps1 = tx_execution_info1.receipt.resources.vm_resources.n_steps; @@ -953,7 +953,7 @@ fn test_max_fee_to_max_steps_conversion( nonce: nonce_manager.next(account_address), }); let tx_context2 = Arc::new(block_context.to_tx_context(&account_tx2)); - let execution_context2 = EntryPointExecutionContext::new_invoke(tx_context2, true).unwrap(); + let execution_context2 = EntryPointExecutionContext::new_invoke(tx_context2, true); let max_steps_limit2 = execution_context2.vm_run_resources.get_n_steps(); let tx_execution_info2 = account_tx2.execute(&mut state, &block_context, true, true).unwrap(); let n_steps2 = tx_execution_info2.receipt.resources.vm_resources.n_steps; diff --git a/crates/blockifier/src/transaction/objects.rs b/crates/blockifier/src/transaction/objects.rs index 5b6a697211..4769eb6072 100644 --- a/crates/blockifier/src/transaction/objects.rs +++ b/crates/blockifier/src/transaction/objects.rs @@ -8,15 +8,14 @@ use starknet_api::core::{ContractAddress, Nonce}; use starknet_api::data_availability::DataAvailabilityMode; use starknet_api::transaction::{ AccountDeploymentData, - DeprecatedResourceBoundsMapping, Fee, PaymasterData, - Resource, ResourceBounds, Tip, TransactionHash, TransactionSignature, TransactionVersion, + ValidResourceBounds, }; use starknet_types_core::felt::Felt; use strum_macros::EnumIter; @@ -100,14 +99,14 @@ impl TransactionInfo { TransactionVersion(query_version) } - pub fn enforce_fee(&self) -> TransactionFeeResult { + pub fn enforce_fee(&self) -> bool { match self { TransactionInfo::Current(context) => { - let l1_bounds = context.l1_resource_bounds()?; + let l1_bounds = context.l1_resource_bounds(); let max_amount: u128 = l1_bounds.max_amount.into(); - Ok(max_amount * l1_bounds.max_price_per_unit > 0) + max_amount * l1_bounds.max_price_per_unit > 0 } - TransactionInfo::Deprecated(context) => Ok(context.max_fee != Fee(0)), + TransactionInfo::Deprecated(context) => context.max_fee != Fee(0), } } } @@ -125,7 +124,7 @@ impl HasRelatedFeeType for TransactionInfo { #[derive(Clone, Debug, Eq, PartialEq)] pub struct CurrentTransactionInfo { pub common_fields: CommonAccountFields, - pub resource_bounds: DeprecatedResourceBoundsMapping, + pub resource_bounds: ValidResourceBounds, pub tip: Tip, pub nonce_data_availability_mode: DataAvailabilityMode, pub fee_data_availability_mode: DataAvailabilityMode, @@ -135,10 +134,12 @@ pub struct CurrentTransactionInfo { impl CurrentTransactionInfo { /// Fetch the L1 resource bounds, if they exist. - pub fn l1_resource_bounds(&self) -> TransactionFeeResult { - match self.resource_bounds.0.get(&Resource::L1Gas).copied() { - Some(bounds) => Ok(bounds), - None => Err(TransactionFeeError::MissingL1GasBounds), + // TODO(Nimrod): Consider removing this function and add equivalent method to + // `ValidResourceBounds`. + pub fn l1_resource_bounds(&self) -> ResourceBounds { + match self.resource_bounds { + ValidResourceBounds::L1Gas(bounds) => bounds, + ValidResourceBounds::AllResources { .. } => todo!(), } } } diff --git a/crates/blockifier/src/transaction/transaction_execution.rs b/crates/blockifier/src/transaction/transaction_execution.rs index f2fb174a84..2d4fee1fb2 100644 --- a/crates/blockifier/src/transaction/transaction_execution.rs +++ b/crates/blockifier/src/transaction/transaction_execution.rs @@ -118,7 +118,7 @@ impl ExecutableTransaction for L1HandlerTransaction { let tx_context = Arc::new(block_context.to_tx_context(self)); let mut execution_resources = ExecutionResources::default(); - let mut context = EntryPointExecutionContext::new_invoke(tx_context.clone(), true)?; + let mut context = EntryPointExecutionContext::new_invoke(tx_context.clone(), true); let mut remaining_gas = block_context.versioned_constants.tx_initial_gas(); let execute_call_info = self.run_execute(state, &mut execution_resources, &mut context, &mut remaining_gas)?; diff --git a/crates/blockifier/src/transaction/transactions.rs b/crates/blockifier/src/transaction/transactions.rs index 8ee1412eec..0d61f2f938 100644 --- a/crates/blockifier/src/transaction/transactions.rs +++ b/crates/blockifier/src/transaction/transactions.rs @@ -296,7 +296,7 @@ impl TransactionInfoCreator for DeclareTransaction { starknet_api::transaction::DeclareTransaction::V3(tx) => { TransactionInfo::Current(CurrentTransactionInfo { common_fields, - resource_bounds: tx.resource_bounds.clone(), + resource_bounds: tx.resource_bounds.clone().try_into().expect("todo"), tip: tx.tip, nonce_data_availability_mode: tx.nonce_data_availability_mode, fee_data_availability_mode: tx.fee_data_availability_mode, @@ -411,7 +411,7 @@ impl TransactionInfoCreator for DeployAccountTransaction { starknet_api::transaction::DeployAccountTransaction::V3(tx) => { TransactionInfo::Current(CurrentTransactionInfo { common_fields, - resource_bounds: tx.resource_bounds.clone(), + resource_bounds: tx.resource_bounds.clone().try_into().expect("todo"), tip: tx.tip, nonce_data_availability_mode: tx.nonce_data_availability_mode, fee_data_availability_mode: tx.fee_data_availability_mode, @@ -535,7 +535,7 @@ impl TransactionInfoCreator for InvokeTransaction { starknet_api::transaction::InvokeTransaction::V3(tx) => { TransactionInfo::Current(CurrentTransactionInfo { common_fields, - resource_bounds: tx.resource_bounds.clone(), + resource_bounds: tx.resource_bounds.clone().try_into().expect("todo"), tip: tx.tip, nonce_data_availability_mode: tx.nonce_data_availability_mode, fee_data_availability_mode: tx.fee_data_availability_mode, diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 4e61d1a94e..2f9b400f50 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -802,7 +802,7 @@ fn assert_failure_if_resource_bounds_exceed_balance( ); } TransactionInfo::Current(context) => { - let l1_bounds = context.l1_resource_bounds().unwrap(); + let l1_bounds = context.l1_resource_bounds(); assert_matches!( invalid_tx.execute(state, block_context, true, true).unwrap_err(), TransactionExecutionError::TransactionPreValidationError( diff --git a/crates/papyrus_execution/src/lib.rs b/crates/papyrus_execution/src/lib.rs index f08fadcebd..1465e20757 100644 --- a/crates/papyrus_execution/src/lib.rs +++ b/crates/papyrus_execution/src/lib.rs @@ -267,8 +267,7 @@ pub fn execute_call( tx_info: TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()), }), true, // limit_steps_by_resources - ) - .map_err(|err| ExecutionError::ContractError(err.into()))?; + ); let res = call_entry_point .execute(&mut cached_state, &mut ExecutionResources::default(), &mut context) diff --git a/crates/starknet_api/src/transaction.rs b/crates/starknet_api/src/transaction.rs index 807849436a..99f238d7e3 100644 --- a/crates/starknet_api/src/transaction.rs +++ b/crates/starknet_api/src/transaction.rs @@ -958,6 +958,7 @@ impl TryFrom> for DeprecatedResourceBoundsMappin } } +#[derive(Clone, Debug, Eq, PartialEq)] pub enum ValidResourceBounds { L1Gas(ResourceBounds), // Pre 0.13.3. Only L1 gas. L2 bounds are signed but never used. AllResources(AllResourceBounds),