From e43edbb005ffe01fd21fd129114957329796e6ef Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Sun, 25 Aug 2024 23:20:46 -0400 Subject: [PATCH 01/11] refactor: increment nonce only after passing tx validation --- .../src/blockifier/stateful_validator.rs | 31 +++++++++++++------ .../src/transaction/account_transaction.rs | 31 ++++++++++++++++++- .../src/transaction/transactions_test.rs | 9 ++++++ 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index d4fa4b6466..577a7189f0 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -16,7 +16,7 @@ use crate::fee::actual_cost::TransactionReceipt; use crate::fee::fee_checks::PostValidationReport; use crate::state::cached_state::CachedState; use crate::state::errors::StateError; -use crate::state::state_api::StateReader; +use crate::state::state_api::{State, StateReader}; use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::errors::{TransactionExecutionError, TransactionPreValidationError}; use crate::transaction::objects::TransactionInfo; @@ -81,17 +81,28 @@ impl StatefulValidator { )?; self.perform_pre_validation_stage(&tx, &tx_context)?; - if skip_validate { - return Ok(()); - } + if !skip_validate { + // `__validate__` call. + let versioned_constants = &tx_context.block_context.versioned_constants(); + let (_optional_call_info, actual_cost) = + self.validate(&tx, versioned_constants.tx_initial_gas())?; - // `__validate__` call. - let versioned_constants = &tx_context.block_context.versioned_constants(); - let (_optional_call_info, actual_cost) = - self.validate(&tx, versioned_constants.tx_initial_gas())?; + // Post validations. + PostValidationReport::verify(&tx_context, &actual_cost)?; + } - // Post validations. - PostValidationReport::verify(&tx_context, &actual_cost)?; + // See similar comment in `run_revertible` for context. + // + // From what I've seen there is not suitable method that is used by both the validator and + // the normal transaction flow where the nonce increment logic can be placed. So + // this is manually placed here. + // + // TODO: find a better place to put this without needing this duplication. + self.tx_executor + .block_state + .as_mut() + .expect(BLOCK_STATE_ACCESS_ERR) + .increment_nonce(tx_context.tx_info.sender_address())?; Ok(()) } diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 17bc4303c9..66b65fab5c 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -230,6 +230,12 @@ impl AccountTransaction { Ok(()) } + /// This method no longer increments the nonce. Refer to individual functions to see + /// in which stage the nonce is incremented. + /// + /// Nonce incremental logics manually placed in + /// [`AccountTransaction::run_revertible`], [`AccountTransaction::run_non_revertible`], + /// [`StatefulValidator::perform_validations`](crate::blockifier::stateful_validator::StatefulValidator::perform_validations) fn handle_nonce( state: &mut dyn State, tx_info: &TransactionInfo, @@ -248,7 +254,8 @@ impl AccountTransaction { account_nonce <= incoming_tx_nonce }; if valid_nonce { - return Ok(state.increment_nonce(address)?); + // return Ok(state.increment_nonce(address)?); + return Ok(()); } Err(TransactionPreValidationError::InvalidNonce { address, @@ -422,7 +429,13 @@ impl AccountTransaction { let mut resources = ExecutionResources::default(); let validate_call_info: Option; let execute_call_info: Option; + if matches!(self, Self::DeployAccount(_)) { + // See similar comment in `run_revertible` for context. + // + // Not sure for this case if incrementing the nonce at this stage is correct. + state.increment_nonce(tx_context.tx_info.sender_address())?; + // Handle `DeployAccount` transactions separately, due to different order of things. // Also, the execution context required form the `DeployAccount` execute phase is // validation context. @@ -449,6 +462,10 @@ impl AccountTransaction { validate, charge_fee, )?; + + // See similar comment in `run_revertible` for context. + state.increment_nonce(tx_context.tx_info.sender_address())?; + execute_call_info = self.run_execute(state, &mut resources, &mut execution_context, remaining_gas)?; } @@ -495,6 +512,18 @@ impl AccountTransaction { charge_fee, )?; + // Increment the sender nonce only after the tx has passed validation. + // + // NOTE: + // + // Before this, the nonce is incremented in `AccountTransaction::handle_nonce` which is + // called at the initial stage of transaction processing (ie in + // ExecutableTransaction::execute_raw of AccountTransaction), which is even before the + // account validation logic is being run. Which is weird because the nonce would get + // incremented even if the transaction failed validation. And this is not what I observed + // from mainnet/testnet. So, I moved the nonce incrementation here. + state.increment_nonce(tx_context.tx_info.sender_address())?; + let n_allotted_execution_steps = execution_context.subtract_validation_and_overhead_steps( &validate_call_info, &self.tx_type(), diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 1a63f1a4ac..2c4b5de366 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -1015,6 +1015,15 @@ fn test_invalid_nonce( .perform_pre_validation_stage(&mut transactional_state, &valid_tx_context, false, false) .unwrap(); + // See comments on changing where nonce is incremented in `ExecutableTransaction::execute_raw` + // of AccountTransaction. + // + // Basically, before this, the nonce is incremented in `perform_pre_validation_stage` but that + // method is called even before the account validation stage is executed. So, now we have to + // increment the sender's account nonce manually here. + let account = valid_tx_context.tx_info.sender_address(); + transactional_state.increment_nonce(account).expect("failed to increment nonce"); + // Negative flow: account nonce = 1, incoming tx nonce = 0. let invalid_nonce = nonce!(0_u8); let invalid_tx = From cf95729308564b9afac505d74b06fd8b412ab22a Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Sun, 25 Aug 2024 23:22:13 -0400 Subject: [PATCH 02/11] refactor: add flag to skip fee check in stateful validator --- .../src/blockifier/stateful_validator.rs | 22 ++++++++++++++++--- .../src/blockifier/stateful_validator_test.rs | 2 +- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index 577a7189f0..c351bd40e4 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -58,10 +58,26 @@ impl StatefulValidator { Self { tx_executor, max_nonce_for_validation_skip } } + /// Perform validations on an account transaction. + /// + /// # Arguments + /// + /// * `tx` - The account transaction to validate. + /// * `skip_validate` - If true, skip the account validation. + /// * `skip_fee_check` - If true, ignore any fee related checks on the transaction and account + /// balance. + /// + /// NOTE: + /// + /// We add a flag specifically for avoiding fee checks to allow the pool validator + /// in Katana to run in 'fee disabled' mode. Basically, to adapt StatefulValidator to Katana's + /// execution flag abstraction (Katana's config that allows running in fee-disabled or + /// no-validation mode). pub fn perform_validations( &mut self, tx: AccountTransaction, deploy_account_tx_hash: Option, + skip_fee_check: bool, ) -> StatefulValidatorResult<()> { // Deploy account transactions should be fully executed, since the constructor must run // before `__validate_deploy__`. The execution already includes all necessary validations, @@ -79,7 +95,7 @@ impl StatefulValidator { &tx_context.tx_info, deploy_account_tx_hash, )?; - self.perform_pre_validation_stage(&tx, &tx_context)?; + self.perform_pre_validation_stage(&tx, &tx_context, !skip_fee_check)?; if !skip_validate { // `__validate__` call. @@ -116,14 +132,14 @@ impl StatefulValidator { &mut self, tx: &AccountTransaction, tx_context: &TransactionContext, + fee_check: bool, ) -> StatefulValidatorResult<()> { let strict_nonce_check = false; // Run pre-validation in charge fee mode to perform fee and balance related checks. - let charge_fee = true; tx.perform_pre_validation_stage( self.tx_executor.block_state.as_mut().expect(BLOCK_STATE_ACCESS_ERR), tx_context, - charge_fee, + fee_check, strict_nonce_check, )?; diff --git a/crates/blockifier/src/blockifier/stateful_validator_test.rs b/crates/blockifier/src/blockifier/stateful_validator_test.rs index cf1568e316..9d61d039cb 100644 --- a/crates/blockifier/src/blockifier/stateful_validator_test.rs +++ b/crates/blockifier/src/blockifier/stateful_validator_test.rs @@ -65,6 +65,6 @@ fn test_transaction_validator( // Test the stateful validator. let mut stateful_validator = StatefulValidator::create(state, block_context, nonce!(0_u32)); - let reuslt = stateful_validator.perform_validations(tx, None); + let reuslt = stateful_validator.perform_validations(tx, None, false); assert!(reuslt.is_ok(), "Validation failed: {:?}", reuslt.unwrap_err()); } From f7bce8f1498b197a528d7c20cc11e645854987b8 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Tue, 27 Aug 2024 15:40:52 -0400 Subject: [PATCH 03/11] refactor: make field public --- .../src/blockifier/stateful_validator.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index c351bd40e4..21b0184f73 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; -use starknet_api::core::Nonce; +use starknet_api::core::{ContractAddress, Nonce}; use starknet_api::transaction::TransactionHash; use starknet_types_core::felt::Felt; use thiserror::Error; @@ -43,7 +43,7 @@ pub type StatefulValidatorResult = Result; /// Manages state related transaction validations for pre-execution flows. pub struct StatefulValidator { - tx_executor: TransactionExecutor, + pub tx_executor: TransactionExecutor, max_nonce_for_validation_skip: Nonce, } @@ -208,4 +208,16 @@ impl StatefulValidator { Ok((validate_call_info, tx_receipt)) } + + pub fn get_nonce( + &mut self, + account_address: ContractAddress, + ) -> StatefulValidatorResult { + Ok(self + .tx_executor + .block_state + .as_ref() + .expect(BLOCK_STATE_ACCESS_ERR) + .get_nonce_at(account_address)?) + } } From 100c83af66f7c215f1c5fd225e4ed716a119153d Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Wed, 28 Aug 2024 11:22:36 -0400 Subject: [PATCH 04/11] refactor: remove post validation --- crates/blockifier/src/blockifier/stateful_validator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index 21b0184f73..beac66e942 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -104,7 +104,7 @@ impl StatefulValidator { self.validate(&tx, versioned_constants.tx_initial_gas())?; // Post validations. - PostValidationReport::verify(&tx_context, &actual_cost)?; + // PostValidationReport::verify(&tx_context, &actual_cost)?; } // See similar comment in `run_revertible` for context. From ded92c844d7f8fe522b0b8e1b422e671cb1765ed Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Wed, 28 Aug 2024 11:38:48 -0400 Subject: [PATCH 05/11] refactor: skip validate --- .../src/blockifier/stateful_validator.rs | 16 +++++++++------- .../src/blockifier/stateful_validator_test.rs | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index beac66e942..15fd3fed02 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -1,3 +1,5 @@ +#![allow(unused)] + use std::sync::Arc; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; @@ -13,7 +15,7 @@ use crate::blockifier::transaction_executor::{ use crate::context::{BlockContext, TransactionContext}; use crate::execution::call_info::CallInfo; use crate::fee::actual_cost::TransactionReceipt; -use crate::fee::fee_checks::PostValidationReport; +// use crate::fee::fee_checks::PostValidationReport; use crate::state::cached_state::CachedState; use crate::state::errors::StateError; use crate::state::state_api::{State, StateReader}; @@ -76,7 +78,7 @@ impl StatefulValidator { pub fn perform_validations( &mut self, tx: AccountTransaction, - deploy_account_tx_hash: Option, + skip_validate: bool, skip_fee_check: bool, ) -> StatefulValidatorResult<()> { // Deploy account transactions should be fully executed, since the constructor must run @@ -91,16 +93,16 @@ impl StatefulValidator { // processed. It is done before the pre-validations checks because, in these checks, we // change the state (more precisely, we increment the nonce). let tx_context = self.tx_executor.block_context.to_tx_context(&tx); - let skip_validate = self.skip_validate_due_to_unprocessed_deploy_account( - &tx_context.tx_info, - deploy_account_tx_hash, - )?; + // let skip_validate = self.skip_validate_due_to_unprocessed_deploy_account( + // &tx_context.tx_info, + // deploy_account_tx_hash, + // )?; self.perform_pre_validation_stage(&tx, &tx_context, !skip_fee_check)?; if !skip_validate { // `__validate__` call. let versioned_constants = &tx_context.block_context.versioned_constants(); - let (_optional_call_info, actual_cost) = + let (_optional_call_info, _actual_cost) = self.validate(&tx, versioned_constants.tx_initial_gas())?; // Post validations. diff --git a/crates/blockifier/src/blockifier/stateful_validator_test.rs b/crates/blockifier/src/blockifier/stateful_validator_test.rs index 9d61d039cb..4a8104fae8 100644 --- a/crates/blockifier/src/blockifier/stateful_validator_test.rs +++ b/crates/blockifier/src/blockifier/stateful_validator_test.rs @@ -65,6 +65,6 @@ fn test_transaction_validator( // Test the stateful validator. let mut stateful_validator = StatefulValidator::create(state, block_context, nonce!(0_u32)); - let reuslt = stateful_validator.perform_validations(tx, None, false); + let reuslt = stateful_validator.perform_validations(tx, false, false); assert!(reuslt.is_ok(), "Validation failed: {:?}", reuslt.unwrap_err()); } From 6105be747a5629a3fe44c8b898c6434a62c21377 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Thu, 29 Aug 2024 11:05:57 -0400 Subject: [PATCH 06/11] refactor: re-enable post validation --- crates/blockifier/src/blockifier/stateful_validator.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index 15fd3fed02..b39be0a083 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -15,13 +15,13 @@ use crate::blockifier::transaction_executor::{ use crate::context::{BlockContext, TransactionContext}; use crate::execution::call_info::CallInfo; use crate::fee::actual_cost::TransactionReceipt; -// use crate::fee::fee_checks::PostValidationReport; +use crate::fee::fee_checks::PostValidationReport; use crate::state::cached_state::CachedState; use crate::state::errors::StateError; use crate::state::state_api::{State, StateReader}; use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::errors::{TransactionExecutionError, TransactionPreValidationError}; -use crate::transaction::objects::TransactionInfo; +use crate::transaction::objects::{TransactionExecutionResult, TransactionInfo}; use crate::transaction::transaction_execution::Transaction; use crate::transaction::transactions::ValidatableTransaction; @@ -102,11 +102,11 @@ impl StatefulValidator { if !skip_validate { // `__validate__` call. let versioned_constants = &tx_context.block_context.versioned_constants(); - let (_optional_call_info, _actual_cost) = + let (_optional_call_info, actual_cost) = self.validate(&tx, versioned_constants.tx_initial_gas())?; // Post validations. - // PostValidationReport::verify(&tx_context, &actual_cost)?; + PostValidationReport::verify(&tx_context, &actual_cost)?; } // See similar comment in `run_revertible` for context. From 726fcfa280c6fc8680fe8bf57566ebb9179dc2f0 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Thu, 5 Sep 2024 15:36:49 -0400 Subject: [PATCH 07/11] fix: don't update nonce for V0 transactions --- .../src/transaction/account_transaction.rs | 30 ++++++++++++++----- .../src/transaction/transactions_test.rs | 3 ++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 66b65fab5c..cfcc0a412f 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -431,10 +431,15 @@ impl AccountTransaction { let execute_call_info: Option; if matches!(self, Self::DeployAccount(_)) { - // See similar comment in `run_revertible` for context. + // See similar comment in `run_revertible` for context. Not sure for this case if + // incrementing the nonce at this stage is correct. // - // Not sure for this case if incrementing the nonce at this stage is correct. - state.increment_nonce(tx_context.tx_info.sender_address())?; + // Only increment the nonce if it's not V0 transaction. Why? not exactly sure but + // that's what the tests implies. + if !tx_context.tx_info.is_v0() { + // See similar comment in `run_revertible` for context. + state.increment_nonce(tx_context.tx_info.sender_address())?; + } // Handle `DeployAccount` transactions separately, due to different order of things. // Also, the execution context required form the `DeployAccount` execute phase is @@ -463,8 +468,12 @@ impl AccountTransaction { charge_fee, )?; - // See similar comment in `run_revertible` for context. - state.increment_nonce(tx_context.tx_info.sender_address())?; + // Only increment the nonce if it's not V0 transaction. Why? not exactly sure but + // that's what the tests implies. + if !execution_context.tx_context.tx_info.is_v0() { + // See similar comment in `run_revertible` for context. + state.increment_nonce(tx_context.tx_info.sender_address())?; + } execute_call_info = self.run_execute(state, &mut resources, &mut execution_context, remaining_gas)?; @@ -520,9 +529,14 @@ impl AccountTransaction { // called at the initial stage of transaction processing (ie in // ExecutableTransaction::execute_raw of AccountTransaction), which is even before the // account validation logic is being run. Which is weird because the nonce would get - // incremented even if the transaction failed validation. And this is not what I observed - // from mainnet/testnet. So, I moved the nonce incrementation here. - state.increment_nonce(tx_context.tx_info.sender_address())?; + // incremented even if the transaction failed validation. And this is not what I + // observed from mainnet/testnet. So, I moved the nonce incrementation here. + // + // Only increment the nonce if it's not V0 transaction. Why? not exactly sure but + // that's what the tests implies. + if !execution_context.tx_context.tx_info.is_v0() { + state.increment_nonce(tx_context.tx_info.sender_address())?; + } let n_allotted_execution_steps = execution_context.subtract_validation_and_overhead_steps( &validate_call_info, diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 2c4b5de366..1058977217 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -1072,6 +1072,9 @@ fn declare_expected_state_changes_count(version: TransactionVersion) -> StateCha if version == TransactionVersion::ZERO { StateChangesCount { n_storage_updates: 1, // Sender balance. + // Supposed to be ZERO, but because due to the changes made for supporting declaring + // Cairo 0 contracts, `n_compiled_class_hash_updates` is 1 for V0 declare transactions. + n_compiled_class_hash_updates: 1, // Also set compiled class hash. ..StateChangesCount::default() } } else if version == TransactionVersion::ONE { From 0e70d2f2b7a9d81aaca1a3997b1f621c8273bde6 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Thu, 5 Sep 2024 15:47:37 -0400 Subject: [PATCH 08/11] fix: add missing values --- crates/blockifier/src/concurrency/test_utils.rs | 3 ++- crates/blockifier/src/concurrency/versioned_state_test.rs | 4 ++-- crates/blockifier/src/concurrency/worker_logic.rs | 8 ++++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/blockifier/src/concurrency/test_utils.rs b/crates/blockifier/src/concurrency/test_utils.rs index 87722b1171..24f2ea996b 100644 --- a/crates/blockifier/src/concurrency/test_utils.rs +++ b/crates/blockifier/src/concurrency/test_utils.rs @@ -76,7 +76,8 @@ pub fn create_fee_transfer_call_info( ) -> CallInfo { let block_context = BlockContext::create_for_account_testing(); let mut transactional_state = TransactionalState::create_transactional(state); - let execution_flags = ExecutionFlags { charge_fee: true, validate: true, concurrency_mode }; + let execution_flags = + ExecutionFlags { charge_fee: true, validate: true, concurrency_mode, nonce_check: true }; let execution_info = account_tx.execute_raw(&mut transactional_state, &block_context, execution_flags).unwrap(); diff --git a/crates/blockifier/src/concurrency/versioned_state_test.rs b/crates/blockifier/src/concurrency/versioned_state_test.rs index d50fac2f48..220aa60548 100644 --- a/crates/blockifier/src/concurrency/versioned_state_test.rs +++ b/crates/blockifier/src/concurrency/versioned_state_test.rs @@ -251,11 +251,11 @@ fn test_run_parallel_txs() { // Execute transactions thread::scope(|s| { s.spawn(move || { - let result = account_tx_1.execute(&mut state_1, &block_context_1, true, true); + let result = account_tx_1.execute(&mut state_1, &block_context_1, true, true, true); assert_eq!(result.is_err(), enforce_fee); }); s.spawn(move || { - account_tx_2.execute(&mut state_2, &block_context_2, true, true).unwrap(); + account_tx_2.execute(&mut state_2, &block_context_2, true, true, true).unwrap(); // Check that the constructor wrote ctor_arg to the storage. let storage_key = get_storage_var_address("ctor_arg", &[]); diff --git a/crates/blockifier/src/concurrency/worker_logic.rs b/crates/blockifier/src/concurrency/worker_logic.rs index 60d5b19e71..4317748ae8 100644 --- a/crates/blockifier/src/concurrency/worker_logic.rs +++ b/crates/blockifier/src/concurrency/worker_logic.rs @@ -126,8 +126,12 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> { let tx = &self.chunk[tx_index]; let mut transactional_state = TransactionalState::create_transactional(&mut tx_versioned_state); - let execution_flags = - ExecutionFlags { charge_fee: true, validate: true, concurrency_mode: true }; + let execution_flags = ExecutionFlags { + charge_fee: true, + validate: true, + concurrency_mode: true, + nonce_check: true, + }; let execution_result = tx.execute_raw(&mut transactional_state, self.block_context, execution_flags); From 4d8541885d17a6b8ec47071b042bf0c996b77508 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Thu, 5 Sep 2024 15:50:41 -0400 Subject: [PATCH 09/11] chore: rust fmt --- crates/blockifier/src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/blockifier/src/lib.rs b/crates/blockifier/src/lib.rs index a78a9c5465..577f2502de 100644 --- a/crates/blockifier/src/lib.rs +++ b/crates/blockifier/src/lib.rs @@ -2,11 +2,7 @@ // length to pointer type ([not necessarily true](https://github.com/rust-lang/rust/issues/65473), // but it is a reasonable assumption for now), this attribute protects against potential overflow // when converting usize to u128. -#![cfg(any( - target_pointer_width = "16", - target_pointer_width = "32", - target_pointer_width = "64", -))] +#![cfg(any(target_pointer_width = "16", target_pointer_width = "32", target_pointer_width = "64",))] #[cfg(feature = "jemalloc")] // Override default allocator. From 6505c115f90f296ea9f78769aa33c07916e5a019 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Thu, 5 Sep 2024 16:04:26 -0400 Subject: [PATCH 10/11] style: clippy --- crates/blockifier/src/execution/entry_point.rs | 6 +++--- crates/blockifier/src/execution/execution_utils.rs | 1 + scripts/clippy.sh | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index 527a298b6a..d93f337f37 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -3,7 +3,7 @@ use std::cmp::min; use std::sync::Arc; use cairo_vm::vm::runners::cairo_runner::{ExecutionResources, ResourceTracker, RunResources}; -use num_traits::{Inv, Zero}; +use num_traits::{Inv, ToPrimitive, Zero}; use serde::Serialize; use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector}; use starknet_api::deprecated_contract_class::EntryPointType; @@ -173,10 +173,10 @@ impl EntryPointExecutionContext { // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the conversion // works. ExecutionMode::Validate => { - block_context.versioned_constants.validate_max_n_steps as usize + block_context.versioned_constants.validate_max_n_steps.to_usize().unwrap() } ExecutionMode::Execute => { - block_context.versioned_constants.invoke_tx_max_n_steps as usize + block_context.versioned_constants.invoke_tx_max_n_steps.to_usize().unwrap() } }; diff --git a/crates/blockifier/src/execution/execution_utils.rs b/crates/blockifier/src/execution/execution_utils.rs index 326e1a6337..1e8be550b3 100644 --- a/crates/blockifier/src/execution/execution_utils.rs +++ b/crates/blockifier/src/execution/execution_utils.rs @@ -152,6 +152,7 @@ pub struct ReadOnlySegment { pub struct ReadOnlySegments(Vec); impl ReadOnlySegments { + #[allow(clippy::ptr_arg)] pub fn allocate( &mut self, vm: &mut VirtualMachine, diff --git a/scripts/clippy.sh b/scripts/clippy.sh index 63cfd37126..226f11d337 100755 --- a/scripts/clippy.sh +++ b/scripts/clippy.sh @@ -1,4 +1,4 @@ #!/bin/bash -cargo clippy "$@" --all-targets --all-features -- -D warnings -D future-incompatible \ +cargo +nightly-2024-01-12 clippy "$@" --all-targets --all-features -- -D warnings -D future-incompatible \ -D nonstandard-style -D rust-2018-idioms -D unused From 835c026db06e989fad9235bc95615b6606356952 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Thu, 5 Sep 2024 16:44:43 -0400 Subject: [PATCH 11/11] fix: update test due to Cairo 0 declaration support --- .../blockifier/src/blockifier/transaction_executor_test.rs | 7 +++++-- crates/blockifier/src/transaction/transactions_test.rs | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/blockifier/src/blockifier/transaction_executor_test.rs b/crates/blockifier/src/blockifier/transaction_executor_test.rs index b0139de8ba..8fa507063c 100644 --- a/crates/blockifier/src/blockifier/transaction_executor_test.rs +++ b/crates/blockifier/src/blockifier/transaction_executor_test.rs @@ -56,7 +56,10 @@ fn tx_executor_test_body( TransactionVersion::ZERO, CairoVersion::Cairo0, BouncerWeights { - state_diff_size: 0, + // The state diff size is 2 because to support declaring Cairo0 contracts, we + // need to include the compiled class hash and declared contracts of the Cairo + // 0 contract in the state diff. + state_diff_size: 2, message_segment_length: 0, n_events: 0, ..Default::default() @@ -66,7 +69,7 @@ fn tx_executor_test_body( TransactionVersion::ONE, CairoVersion::Cairo0, BouncerWeights { - state_diff_size: 2, + state_diff_size: 4, message_segment_length: 0, n_events: 0, ..Default::default() diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 1058977217..2426ffc622 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -1081,6 +1081,9 @@ fn declare_expected_state_changes_count(version: TransactionVersion) -> StateCha StateChangesCount { n_storage_updates: 1, // Sender balance. n_modified_contracts: 1, // Nonce. + // Supposed to be ZERO, but because due to the changes made for supporting declaring + // Cairo 0 contracts, `n_compiled_class_hash_updates` is 1 for V0 declare transactions. + n_compiled_class_hash_updates: 1, // Also set compiled class hash. ..StateChangesCount::default() } } else if version == TransactionVersion::TWO || version == TransactionVersion::THREE {