Skip to content

Commit

Permalink
fix(blockifier): fix tests for sierra_gas usage
Browse files Browse the repository at this point in the history
  • Loading branch information
aner-starkware committed Dec 5, 2024
1 parent 131f0d2 commit ab670f8
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 63 deletions.
2 changes: 1 addition & 1 deletion crates/blockifier/src/blockifier/stateful_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl<S: StateReader> StatefulValidator<S> {

// `__validate__` call.
let (_optional_call_info, actual_cost) =
self.validate(&tx, tx_context.initial_sierra_gas())?;
self.validate(&tx, tx_context.initial_sierra_gas().0)?;

// Post validations.
PostValidationReport::verify(&tx_context, &actual_cost)?;
Expand Down
14 changes: 7 additions & 7 deletions crates/blockifier/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ impl TransactionContext {

/// Returns the initial Sierra gas of the transaction.
/// This value is used to limit the transaction's run.
// TODO(tzahi): replace returned value from u64 to GasAmount.
pub fn initial_sierra_gas(&self) -> u64 {
pub fn initial_sierra_gas(&self) -> GasAmount {
match &self.tx_info {
TransactionInfo::Deprecated(_)
| TransactionInfo::Current(CurrentTransactionInfo {
Expand All @@ -55,7 +54,7 @@ impl TransactionContext {
TransactionInfo::Current(CurrentTransactionInfo {
resource_bounds: ValidResourceBounds::AllResources(AllResourceBounds { l2_gas, .. }),
..
}) => l2_gas.max_amount.0,
}) => l2_gas.max_amount,
}
}
}
Expand All @@ -66,19 +65,20 @@ pub(crate) struct GasCounter {
}

impl GasCounter {
pub(crate) fn new(initial_gas: u64) -> Self {
GasCounter { spent_gas: GasAmount(0), remaining_gas: GasAmount(initial_gas) }
pub(crate) fn new(initial_gas: GasAmount) -> Self {
GasCounter { spent_gas: GasAmount(0), remaining_gas: initial_gas }
}

pub(crate) fn spend(&mut self, amount: GasAmount) {
fn spend(&mut self, amount: GasAmount) {
self.spent_gas = self.spent_gas.checked_add(amount).expect("Gas overflow");
self.remaining_gas = self
.remaining_gas
.checked_sub(amount)
.expect("Overuse of gas; should have been caught earlier");
}

pub(crate) fn cap_usage(&self, amount: GasAmount) -> u64 {
/// Limits the amount of gas that can be used (in validate\execute) by the given global limit.
pub(crate) fn limit_usage(&self, amount: GasAmount) -> u64 {
self.remaining_gas.min(amount).0
}

Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/fee/fee_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ fn test_get_fee_by_gas_vector_overflow(

#[rstest]
#[case::default(
VersionedConstants::create_for_account_testing().default_initial_gas_cost(),
VersionedConstants::create_for_account_testing().default_initial_gas_amount().0,
GasVectorComputationMode::NoL2Gas
)]
#[case::from_l2_gas(4321, GasVectorComputationMode::All)]
Expand All @@ -384,6 +384,6 @@ fn test_initial_sierra_gas(
}),
};
let account_tx = account_invoke_tx(invoke_tx_args!(resource_bounds));
let actual = block_context.to_tx_context(&account_tx).initial_sierra_gas();
let actual = block_context.to_tx_context(&account_tx).initial_sierra_gas().0;
assert_eq!(actual, expected)
}
46 changes: 19 additions & 27 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,21 +354,13 @@ impl AccountTransaction {
if validate {
// TODO(Aner): cap the gas for validation.
let remaining_validation_gas = &mut remaining_gas
.cap_usage(tx_context.block_context.versioned_constants.validate_max_sierra_gas);
let call_info = self.validate_tx(
state,
tx_context,
remaining_validation_gas,
limit_steps_by_resources,
)?;
match call_info {
// TODO(Aner): Update the gas counter.
Some(call_info) => {
remaining_gas.subtract_used_gas(&call_info);
Ok(Some(call_info))
}
None => Ok(None),
}
.limit_usage(tx_context.block_context.versioned_constants.validate_max_sierra_gas);
Ok(self
.validate_tx(state, tx_context, remaining_validation_gas, limit_steps_by_resources)?
.inspect(|call_info| {
// TODO(Aner): Update the gas counter.
remaining_gas.subtract_used_gas(call_info);
}))
} else {
Ok(None)
}
Expand Down Expand Up @@ -498,24 +490,24 @@ impl AccountTransaction {
remaining_gas: &mut GasCounter,
) -> TransactionExecutionResult<Option<CallInfo>> {
// TODO(Aner): cap the gas usage for execution.
let remaining_execution_gas = &mut remaining_gas
.cap_usage(context.tx_context.block_context.versioned_constants.execute_max_sierra_gas);

let call_info = match &self.tx {
let remaining_execution_gas = &mut remaining_gas.limit_usage(
context
.tx_context
.block_context
.versioned_constants
.sierra_gas_limit(&context.execution_mode),
);
Ok(match &self.tx {
Transaction::Declare(tx) => tx.run_execute(state, context, remaining_execution_gas),
Transaction::DeployAccount(tx) => {
tx.run_execute(state, context, remaining_execution_gas)
}
Transaction::Invoke(tx) => tx.run_execute(state, context, remaining_execution_gas),
}?;
match call_info {
}?
.inspect(|call_info| {
// TODO(Aner): Update the gas counter.
Some(call_info) => {
remaining_gas.subtract_used_gas(&call_info);
Ok(Some(call_info))
}
None => Ok(None),
}
remaining_gas.subtract_used_gas(call_info);
}))
}

fn run_non_revertible<S: StateReader>(
Expand Down
13 changes: 11 additions & 2 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1684,13 +1684,22 @@ fn test_initial_gas(

let validate_call_info = &transaction_ex_info.validate_call_info.unwrap();
let validate_initial_gas = validate_call_info.call.initial_gas;
assert_eq!(validate_initial_gas, DEFAULT_L2_GAS_MAX_AMOUNT.0);
assert_eq!(validate_initial_gas, block_context.versioned_constants.validate_max_sierra_gas.0);
let validate_gas_consumed = validate_call_info.execution.gas_consumed;
assert!(validate_gas_consumed > 0, "New Cairo1 contract should consume gas.");

let default_call_info = CallInfo::default();
let mut prev_initial_gas = validate_initial_gas;
let mut execute_call_info = &transaction_ex_info.execute_call_info.unwrap();
// Initial gas for execution is the minimum between the max execution gas and the initial gas
// minus the gas consumed by validate.
let mut prev_initial_gas = block_context
.versioned_constants
.max_execution_sierra_gas()
.min(
block_context.versioned_constants.default_initial_gas_amount()
- GasAmount(validate_gas_consumed),
)
.0;
let mut curr_initial_gas;
let mut started_vm_mode = false;
// The __validate__ call of a the account contract.
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/transaction/transaction_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for L1HandlerTransaction {
let limit_steps_by_resources = false;
let mut context =
EntryPointExecutionContext::new_invoke(tx_context.clone(), limit_steps_by_resources);
let mut remaining_gas = tx_context.initial_sierra_gas();
let mut remaining_gas = tx_context.initial_sierra_gas().0;
let execute_call_info = self.run_execute(state, &mut context, &mut remaining_gas)?;
let l1_handler_payload_size = self.payload_size();
let TransactionReceipt {
Expand Down
83 changes: 62 additions & 21 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::cmp::min;
use std::collections::{HashMap, HashSet};
use std::sync::{Arc, LazyLock};

Expand Down Expand Up @@ -160,6 +161,11 @@ use crate::{
static VERSIONED_CONSTANTS: LazyLock<VersionedConstants> =
LazyLock::new(VersionedConstants::create_for_testing);

#[fixture]
fn default_initial_gas_amount() -> GasAmount {
VERSIONED_CONSTANTS.default_initial_gas_amount()
}

#[fixture]
fn default_initial_gas_cost() -> u64 {
VERSIONED_CONSTANTS.default_initial_gas_cost()
Expand All @@ -179,7 +185,7 @@ struct ExpectedResultTestInvokeTx {

fn user_initial_gas_from_bounds(bounds: ValidResourceBounds) -> Option<GasAmount> {
match bounds {
ValidResourceBounds::L1Gas(_) => None,
ValidResourceBounds::L1Gas(_) => Some(default_initial_gas_amount()),
ValidResourceBounds::AllResources(bounds) => Some(bounds.l2_gas.max_amount),
}
}
Expand Down Expand Up @@ -229,11 +235,19 @@ fn expected_validate_call_info(
builtin_instance_counter: HashMap::from([(BuiltinName::range_check, n_range_checks)]),
}
.filter_unused_builtins();
let initial_gas = match tracked_resource {
TrackedResource::CairoSteps => default_initial_gas_cost(),
TrackedResource::SierraGas => {
user_initial_gas.unwrap_or(GasAmount(default_initial_gas_cost())).0
}
let initial_gas = match cairo_version {
CairoVersion::Cairo0 => default_initial_gas_cost(),
CairoVersion::Cairo1 => match tracked_resource {
TrackedResource::CairoSteps => {
default_initial_gas_amount().min(VERSIONED_CONSTANTS.max_validation_sierra_gas()).0
}
TrackedResource::SierraGas => {
user_initial_gas
.unwrap_or(default_initial_gas_amount())
.min(VERSIONED_CONSTANTS.max_validation_sierra_gas())
.0
}
},
};

Some(CallInfo {
Expand Down Expand Up @@ -487,14 +501,10 @@ fn test_invoke_tx(
let tracked_resource = account_contract
.get_runnable_class()
.tracked_resource(&versioned_constants.min_compiler_version_for_sierra_gas, None);
if tracked_resource == TrackedResource::CairoSteps {
// In CairoSteps mode, the initial gas is set to the default once before the validate call.
expected_arguments.inner_call_initial_gas -=
expected_arguments.validate_gas_consumed + expected_arguments.execute_gas_consumed
}

// Build expected validate call info.
let expected_account_class_hash = account_contract.get_class_hash();
let initial_gas = user_initial_gas_from_bounds(resource_bounds);
let expected_validate_call_info = expected_validate_call_info(
expected_account_class_hash,
constants::VALIDATE_ENTRY_POINT_NAME,
Expand All @@ -503,11 +513,39 @@ fn test_invoke_tx(
sender_address,
account_cairo_version,
tracked_resource,
user_initial_gas_from_bounds(resource_bounds),
initial_gas.min(Some(VERSIONED_CONSTANTS.max_validation_sierra_gas())),
);

// Build expected execute call info.
let expected_return_result_calldata = vec![felt!(2_u8)];

let expected_validated_call = expected_validate_call_info.as_ref().unwrap().call.clone();
let expected_initial_execution_gas = VERSIONED_CONSTANTS
.max_execution_sierra_gas()
.min(initial_gas.unwrap() - GasAmount(expected_arguments.validate_gas_consumed))
.0;
if account_cairo_version == CairoVersion::Cairo0 {
expected_arguments.inner_call_initial_gas = VERSIONED_CONSTANTS.default_initial_gas_cost();
}
let expected_execute_call = CallEntryPoint {
entry_point_selector: selector_from_name(constants::EXECUTE_ENTRY_POINT_NAME),
initial_gas: if account_cairo_version == CairoVersion::Cairo0 {
VERSIONED_CONSTANTS.default_initial_gas_cost()
} else {
expected_initial_execution_gas
}, // expected_validated_call.initial_gas - expected_arguments.validate_gas_consumed,
..expected_validated_call
};
if tracked_resource == TrackedResource::CairoSteps {
// else {
expected_arguments.inner_call_initial_gas -= expected_arguments.validate_gas_consumed;
expected_arguments.inner_call_initial_gas = min(
expected_arguments.inner_call_initial_gas,
VERSIONED_CONSTANTS.max_execution_sierra_gas().0,
);
expected_arguments.inner_call_initial_gas -= expected_arguments.execute_gas_consumed;
// }
}
let expected_return_result_call = CallEntryPoint {
entry_point_selector: selector_from_name("return_result"),
class_hash: Some(test_contract.get_class_hash()),
Expand All @@ -517,13 +555,11 @@ fn test_invoke_tx(
storage_address: test_contract_address,
caller_address: sender_address,
call_type: CallType::Call,
initial_gas: expected_arguments.inner_call_initial_gas,
};
let expected_validated_call = expected_validate_call_info.as_ref().unwrap().call.clone();
let expected_execute_call = CallEntryPoint {
entry_point_selector: selector_from_name(constants::EXECUTE_ENTRY_POINT_NAME),
initial_gas: expected_validated_call.initial_gas - expected_arguments.validate_gas_consumed,
..expected_validated_call
initial_gas: if account_cairo_version == CairoVersion::Cairo0 {
VERSIONED_CONSTANTS.default_initial_gas_cost()
} else {
expected_arguments.inner_call_initial_gas
},
};
let expected_return_result_retdata = Retdata(expected_return_result_calldata);
let expected_execute_call_info = Some(CallInfo {
Expand Down Expand Up @@ -1772,14 +1808,19 @@ fn test_deploy_account_tx(
);

// Build expected execute call info.
let expected_execute_initial_gas = user_initial_gas
.unwrap_or(default_initial_gas_amount())
// Note that in the case of deploy account, the initial gas in "execute" is limited by
// max_validation_sierra_gas.
.min(VERSIONED_CONSTANTS.max_validation_sierra_gas());
let expected_execute_call_info = Some(CallInfo {
call: CallEntryPoint {
class_hash: Some(account_class_hash),
code_address: None,
entry_point_type: EntryPointType::Constructor,
entry_point_selector: selector_from_name(CONSTRUCTOR_ENTRY_POINT_NAME),
storage_address: deployed_account_address,
initial_gas: user_initial_gas.unwrap_or(GasAmount(default_initial_gas_cost())).0,
initial_gas: expected_execute_initial_gas.0,
..Default::default()
},
..Default::default()
Expand Down Expand Up @@ -2289,7 +2330,7 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) {
storage_address: contract_address,
caller_address: ContractAddress::default(),
call_type: CallType::Call,
initial_gas: default_initial_gas_cost(),
initial_gas: default_initial_gas_amount().0,
},
execution: CallExecution {
retdata: Retdata(vec![value]),
Expand Down
12 changes: 10 additions & 2 deletions crates/blockifier/src/versioned_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use starknet_api::transaction::fields::GasVectorComputationMode;
use strum::IntoEnumIterator;
use thiserror::Error;

use crate::execution::common_hints::ExecutionMode;
use crate::execution::deprecated_syscalls::hint_processor::SyscallCounter;
use crate::execution::execution_utils::poseidon_hash_many_cost;
use crate::execution::syscalls::SyscallSelector;
Expand Down Expand Up @@ -249,10 +250,17 @@ impl VersionedConstants {
}

/// Default initial gas amount when L2 gas is not provided.
pub fn default_initial_gas_amount(&self) -> u64 {
pub fn default_initial_gas_amount(&self) -> GasAmount {
(self.execute_max_sierra_gas.checked_add(self.validate_max_sierra_gas))
.expect("The default initial gas cost should be less than the maximum gas amount.")
.0
}

/// Returns the maximum gas amount according to the given mode.
pub fn sierra_gas_limit(&self, mode: &ExecutionMode) -> GasAmount {
match mode {
ExecutionMode::Validate => self.validate_max_sierra_gas,
ExecutionMode::Execute => self.execute_max_sierra_gas,
}
}

/// Returns the maximum gas amount for validation.
Expand Down

0 comments on commit ab670f8

Please sign in to comment.