From 49d8c5c07e2f18b5b46b4d57adc8d55d4dd63e3b Mon Sep 17 00:00:00 2001 From: Aviv Greenburg Date: Wed, 4 Sep 2024 14:50:32 +0300 Subject: [PATCH] chore(blockifier): have limit_steps_by_resources flag represent charge_fee flag and enforce_fee return value --- .../src/blockifier/stateful_validator.rs | 2 +- crates/blockifier/src/concurrency/worker_logic.rs | 1 + .../deprecated_syscalls_test.rs | 2 +- crates/blockifier/src/execution/entry_point.rs | 3 ++- .../syscalls/syscall_tests/get_execution_info.rs | 6 +++--- crates/blockifier/src/test_utils/prices.rs | 2 +- crates/blockifier/src/test_utils/struct_impls.rs | 15 ++++++++------- .../src/transaction/account_transaction.rs | 5 +++-- .../src/transaction/transaction_execution.rs | 7 ++++--- crates/papyrus_execution/src/lib.rs | 11 +++++------ 10 files changed, 29 insertions(+), 25 deletions(-) diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index 7cbe0644a3..d472266bc6 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -115,7 +115,7 @@ impl StatefulValidator { let mut execution_resources = ExecutionResources::default(); let tx_context = Arc::new(self.tx_executor.block_context.to_tx_context(tx)); - let limit_steps_by_resources = true; + let limit_steps_by_resources = tx.create_tx_info().enforce_fee(); //aviv: was true; let validate_call_info = tx.validate_tx( self.tx_executor.block_state.as_mut().expect(BLOCK_STATE_ACCESS_ERR), &mut execution_resources, diff --git a/crates/blockifier/src/concurrency/worker_logic.rs b/crates/blockifier/src/concurrency/worker_logic.rs index c432426d59..41288d69b4 100644 --- a/crates/blockifier/src/concurrency/worker_logic.rs +++ b/crates/blockifier/src/concurrency/worker_logic.rs @@ -27,6 +27,7 @@ use crate::transaction::objects::{ TransactionExecutionResult, TransactionInfoCreator, }; + use crate::transaction::transaction_execution::Transaction; use crate::transaction::transactions::{ExecutableTransaction, ExecutionFlags}; diff --git a/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs b/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs index 9aafe0b99c..f82d361b21 100644 --- a/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs +++ b/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs @@ -488,7 +488,7 @@ fn test_tx_info(#[values(false, true)] only_query: bool) { }, max_fee, }); - let limit_steps_by_resources = true; + let limit_steps_by_resources = tx_info.enforce_fee(); //aviv: was true let result = entry_point_call .execute_directly_given_tx_info(&mut state, tx_info, limit_steps_by_resources) .unwrap(); diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index 0f43522e2f..a8cdc45ed4 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -183,7 +183,8 @@ impl EntryPointExecutionContext { .expect("Failed to convert invoke_tx_max_n_steps (u32) to usize."), }; - if !limit_steps_by_resources || !tx_info.enforce_fee() { + if !limit_steps_by_resources { + // aviv: was with '|| !tx_info.enforce_fee()' return block_upper_bound; } 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 be2f24b1c8..611ff2a3de 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 @@ -242,13 +242,13 @@ fn test_get_execution_info( ), ..trivial_external_entry_point_with_address(test_contract_address) }; - + // let limit_steps_by_resources = tx_info.enforce_fee(); // aviv: new let result = match execution_mode { ExecutionMode::Validate => { - entry_point_call.execute_directly_given_tx_info_in_validate_mode(state, tx_info, false) + entry_point_call.execute_directly_given_tx_info_in_validate_mode(state, tx_info, false) //aviv: was ',false)': not needed? } ExecutionMode::Execute => { - entry_point_call.execute_directly_given_tx_info(state, tx_info, false) + entry_point_call.execute_directly_given_tx_info(state, tx_info, false) //aviv: was ',false)': not needed? } }; diff --git a/crates/blockifier/src/test_utils/prices.rs b/crates/blockifier/src/test_utils/prices.rs index 343ebe7ad5..e768f4e2f4 100644 --- a/crates/blockifier/src/test_utils/prices.rs +++ b/crates/blockifier/src/test_utils/prices.rs @@ -74,7 +74,7 @@ fn fee_transfer_resources( &mut EntryPointExecutionContext::new( Arc::new(block_context.to_tx_context(&account_invoke_tx(InvokeTxArgs::default()))), ExecutionMode::Execute, - false, + false, // aviv: limit_steps_by_resources: can be enforce_fee() instead of false.? ), ) .unwrap() diff --git a/crates/blockifier/src/test_utils/struct_impls.rs b/crates/blockifier/src/test_utils/struct_impls.rs index 65be6bba25..2a73103497 100644 --- a/crates/blockifier/src/test_utils/struct_impls.rs +++ b/crates/blockifier/src/test_utils/struct_impls.rs @@ -54,11 +54,9 @@ impl CallEntryPoint { /// Executes the call directly, without account context. Limits the number of steps by resource /// bounds. pub fn execute_directly(self, state: &mut dyn State) -> EntryPointExecutionResult { - self.execute_directly_given_tx_info( - state, - TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()), - true, - ) + let tx_info = TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()); + let limit_steps_by_resources = tx_info.enforce_fee(); // aviv: new + self.execute_directly_given_tx_info(state, tx_info, limit_steps_by_resources) } pub fn execute_directly_given_tx_info( @@ -80,10 +78,12 @@ impl CallEntryPoint { self, state: &mut dyn State, ) -> EntryPointExecutionResult { + let tx_info = TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()); + let limit_steps_by_resources = tx_info.enforce_fee(); // aviv: new self.execute_directly_given_tx_info_in_validate_mode( state, - TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()), - true, + tx_info, + limit_steps_by_resources, ) } @@ -93,6 +93,7 @@ impl CallEntryPoint { tx_info: TransactionInfo, limit_steps_by_resources: bool, ) -> EntryPointExecutionResult { + // aviv: let limit_steps = limit_steps_by_resources && tx_info.enforce_fee(); let tx_context = TransactionContext { block_context: BlockContext::create_for_testing(), tx_info }; let mut context = EntryPointExecutionContext::new_validate( diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 02f51e8d75..d59502dee0 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -404,8 +404,9 @@ impl AccountTransaction { // The fee-token contract is a Cairo 0 contract, hence the initial gas is irrelevant. initial_gas: block_context.versioned_constants.os_constants.gas_costs.initial_gas_cost, }; - - let mut context = EntryPointExecutionContext::new_invoke(tx_context, true); + let limit_steps_by_resources = tx_info.enforce_fee(); //aviv: new + let mut context = + EntryPointExecutionContext::new_invoke(tx_context, limit_steps_by_resources); Ok(fee_transfer_call .execute(state, &mut ExecutionResources::default(), &mut context) diff --git a/crates/blockifier/src/transaction/transaction_execution.rs b/crates/blockifier/src/transaction/transaction_execution.rs index 1c62f51752..4a51b0d0fe 100644 --- a/crates/blockifier/src/transaction/transaction_execution.rs +++ b/crates/blockifier/src/transaction/transaction_execution.rs @@ -113,12 +113,13 @@ impl ExecutableTransaction for L1HandlerTransaction { &self, state: &mut TransactionalState<'_, U>, block_context: &BlockContext, - _execution_flags: ExecutionFlags, + execution_flags: ExecutionFlags, ) -> TransactionExecutionResult { let tx_context = Arc::new(block_context.to_tx_context(self)); - + let limit_steps_by_resources = execution_flags.charge_fee; // aviv: new 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(), limit_steps_by_resources); 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/papyrus_execution/src/lib.rs b/crates/papyrus_execution/src/lib.rs index e4502626ff..bb14af9075 100644 --- a/crates/papyrus_execution/src/lib.rs +++ b/crates/papyrus_execution/src/lib.rs @@ -247,14 +247,13 @@ pub fn execute_call( execution_config, override_kzg_da_to_false, )?; + // TODO(yair): fix when supporting v3 transactions + let tx_info = TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()); // aviv: new + let limit_steps_by_resources = tx_info.enforce_fee(); // aviv: new let mut context = EntryPointExecutionContext::new_invoke( - // TODO(yair): fix when supporting v3 transactions - Arc::new(TransactionContext { - block_context, - tx_info: TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()), - }), - true, // limit_steps_by_resources + Arc::new(TransactionContext { block_context, tx_info }), + limit_steps_by_resources, ); let res = call_entry_point