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 8, 2024
1 parent a0c7871 commit 8655fd3
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 76 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
5 changes: 5 additions & 0 deletions crates/blockifier/src/execution/entry_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use starknet_api::abi::abi_utils::selector_from_name;
use starknet_api::abi::constants::CONSTRUCTOR_ENTRY_POINT_NAME;
use starknet_api::contract_class::EntryPointType;
use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector};
use starknet_api::execution_resources::GasAmount;
use starknet_api::state::StorageKey;
use starknet_api::transaction::fields::{
AllResourceBounds,
Expand Down Expand Up @@ -375,6 +376,10 @@ impl EntryPointExecutionContext {
&self.versioned_constants().os_constants.gas_costs
}

pub fn mode_sierra_gas_limit(&self) -> GasAmount {
self.tx_context.block_context.versioned_constants.sierra_gas_limit(&self.execution_mode)
}

/// Reverts the state back to the way it was when self.revert_infos.0['revert_idx'] was created.
pub fn revert(&mut self, revert_idx: usize, state: &mut dyn State) -> StateResult<()> {
for contract_revert_info in self.revert_infos.0.drain(revert_idx..).rev() {
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 = invoke_tx_with_default_flags(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)
}
41 changes: 14 additions & 27 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,21 +378,13 @@ impl AccountTransaction {
let limit_steps_by_resources = self.execution_flags.charge_fee;
// 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 @@ -522,24 +514,19 @@ 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.mode_sierra_gas_limit());
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
11 changes: 9 additions & 2 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1677,18 +1677,25 @@ fn test_initial_gas(
resource_bounds: default_all_resource_bounds,
version: TransactionVersion::THREE
});
let user_gas_bound = block_context.to_tx_context(&account_tx).initial_sierra_gas();

let transaction_ex_info = account_tx.execute(state, &block_context).unwrap();

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. Need to add 1 as the check is strictly less than.
let mut prev_initial_gas = block_context
.versioned_constants
.execute_max_sierra_gas
.min(user_gas_bound - GasAmount(validate_gas_consumed) + GasAmount(1))
.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 @@ -146,7 +146,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
94 changes: 68 additions & 26 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 @@ -169,17 +170,27 @@ fn versioned_constants_for_account_testing() -> VersionedConstants {
VERSIONED_CONSTANTS.clone()
}

fn initial_gas_amount_from_block_context(block_context: Option<&BlockContext>) -> GasAmount {
match block_context {
Some(block_context) => block_context.versioned_constants.default_initial_gas_amount(),
None => VERSIONED_CONSTANTS.default_initial_gas_amount(),
}
}

struct ExpectedResultTestInvokeTx {
resources: ExecutionResources,
validate_gas_consumed: u64,
execute_gas_consumed: u64,
inner_call_initial_gas: u64,
}

fn user_initial_gas_from_bounds(bounds: ValidResourceBounds) -> Option<GasAmount> {
fn user_initial_gas_from_bounds(
bounds: ValidResourceBounds,
block_context: Option<&BlockContext>,
) -> GasAmount {
match bounds {
ValidResourceBounds::L1Gas(_) => None,
ValidResourceBounds::AllResources(bounds) => Some(bounds.l2_gas.max_amount),
ValidResourceBounds::L1Gas(_) => initial_gas_amount_from_block_context(block_context),
ValidResourceBounds::AllResources(bounds) => bounds.l2_gas.max_amount,
}
}

Expand Down Expand Up @@ -228,11 +239,17 @@ 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 => initial_gas_amount_from_block_context(None).0,
TrackedResource::SierraGas => {
user_initial_gas
.unwrap_or(initial_gas_amount_from_block_context(None))
.min(VERSIONED_CONSTANTS.validate_max_sierra_gas)
.0
}
},
};

Some(CallInfo {
Expand Down Expand Up @@ -485,14 +502,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, Some(block_context));
let expected_validate_call_info = expected_validate_call_info(
expected_account_class_hash,
constants::VALIDATE_ENTRY_POINT_NAME,
Expand All @@ -501,11 +514,37 @@ fn test_invoke_tx(
sender_address,
account_cairo_version,
tracked_resource,
user_initial_gas_from_bounds(resource_bounds),
Some(initial_gas.min(versioned_constants.validate_max_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
.execute_max_sierra_gas
.min(initial_gas - 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
};
if tracked_resource == TrackedResource::CairoSteps {
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.execute_max_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 @@ -515,13 +554,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 @@ -1549,7 +1586,7 @@ fn test_declare_tx(
.get_runnable_class()
.tracked_resource(&versioned_constants.min_compiler_version_for_sierra_gas, None),
if tx_version >= TransactionVersion::THREE {
user_initial_gas_from_bounds(default_all_resource_bounds)
Some(user_initial_gas_from_bounds(default_all_resource_bounds, Some(block_context)))
} else {
None
},
Expand Down Expand Up @@ -1716,7 +1753,8 @@ fn test_deploy_account_tx(
// account transaction.
let class_hash = deploy_account.class_hash().unwrap();
let deployed_account_address = deploy_account.sender_address();
let user_initial_gas = user_initial_gas_from_bounds(default_all_resource_bounds);
let user_initial_gas =
user_initial_gas_from_bounds(default_all_resource_bounds, Some(block_context));

// Update the balance of the about to be deployed account contract in the erc20 contract, so it
// can pay for the transaction execution.
Expand Down Expand Up @@ -1761,18 +1799,22 @@ fn test_deploy_account_tx(
account
.get_runnable_class()
.tracked_resource(&versioned_constants.min_compiler_version_for_sierra_gas, None),
user_initial_gas,
Some(user_initial_gas),
);

// Build expected execute call info.
let expected_execute_initial_gas = user_initial_gas
// Note that in the case of deploy account, the initial gas in "execute" is limited by
// max_validation_sierra_gas.
.min(versioned_constants.validate_max_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 @@ -2279,7 +2321,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: initial_gas_amount_from_block_context(Some(block_context)).0,
},
execution: CallExecution {
retdata: Retdata(vec![value]),
Expand Down
Loading

0 comments on commit 8655fd3

Please sign in to comment.