Skip to content

Commit

Permalink
fix(blockifier): avoid negative consumed gas
Browse files Browse the repository at this point in the history
  • Loading branch information
avivg-starkware committed Dec 29, 2024
1 parent 6f09863 commit cb1a1ed
Show file tree
Hide file tree
Showing 19 changed files with 146 additions and 40 deletions.
6 changes: 3 additions & 3 deletions crates/blockifier/resources/versioned_constants_0_13_4.json
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@
"validated": "VALID",
"syscall_gas_costs": {
"call_contract": {
"entry_point_initial_budget": 1,
"entry_point_initial_budget": 0,
"step_gas_cost": 860,
"range_check": 15
},
"deploy": {
"entry_point_initial_budget": 1,
"entry_point_initial_budget": 0,
"step_gas_cost": 1128,
"range_check": 18,
"pedersen": 7
Expand All @@ -146,7 +146,7 @@
},
"keccak_round_cost": 180000,
"library_call": {
"entry_point_initial_budget": 1,
"entry_point_initial_budget": 0,
"step_gas_cost": 836,
"range_check": 15
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,15 @@ fn test_nested_library_call() {
felt!(key), // Calldata: address.
felt!(value) // Calldata: value.
];
let vesrioned_const = VersionedConstants::create_for_testing();
let initial_gas = vesrioned_const.os_constants.gas_costs.base.default_initial_gas_cost;

// Create expected call info tree.
let main_entry_point = CallEntryPoint {
entry_point_selector: selector_from_name("test_nested_library_call"),
calldata: main_entry_point_calldata,
class_hash: Some(test_contract.get_class_hash()),
initial_gas,
..trivial_external_entry_point_new(test_contract)
};
let nested_storage_entry_point = CallEntryPoint {
Expand All @@ -127,6 +130,7 @@ fn test_nested_library_call() {
class_hash: Some(test_contract.get_class_hash()),
code_address: None,
call_type: CallType::Delegate,
initial_gas,
..trivial_external_entry_point_new(test_contract)
};
let library_entry_point = CallEntryPoint {
Expand All @@ -141,6 +145,7 @@ fn test_nested_library_call() {
class_hash: Some(test_contract.get_class_hash()),
code_address: None,
call_type: CallType::Delegate,
initial_gas,
..trivial_external_entry_point_new(test_contract)
};
let storage_entry_point = CallEntryPoint {
Expand Down Expand Up @@ -235,13 +240,16 @@ fn test_call_contract() {
};
let call_info = entry_point_call.execute_directly(&mut state).unwrap();

let vesrioned_const = VersionedConstants::create_for_testing();
let initial_gas = vesrioned_const.os_constants.gas_costs.base.default_initial_gas_cost;
let expected_execution = CallExecution { retdata: retdata![value], ..Default::default() };
let expected_inner_call_info = CallInfo {
call: CallEntryPoint {
class_hash: Some(test_contract.get_class_hash()),
entry_point_selector: inner_entry_point_selector,
calldata: inner_calldata,
caller_address: test_address,
initial_gas,
..trivial_external_entry_point
},
execution: expected_execution.clone(),
Expand All @@ -260,6 +268,7 @@ fn test_call_contract() {
class_hash: Some(test_contract.get_class_hash()),
entry_point_selector: outer_entry_point_selector,
calldata,
initial_gas,
..trivial_external_entry_point
},
execution: expected_execution,
Expand Down
34 changes: 30 additions & 4 deletions crates/blockifier/src/execution/entry_point_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::execution::execution_utils::{
ReadOnlySegments,
SEGMENT_ARENA_BUILTIN_SIZE,
};
use crate::execution::syscalls::hint_processor::SyscallHintProcessor;
use crate::execution::syscalls::hint_processor::{SyscallHintProcessor, OUT_OF_GAS_ERROR};
use crate::state::state_api::State;
use crate::versioned_constants::GasCosts;

Expand Down Expand Up @@ -61,6 +61,8 @@ pub fn execute_entry_point_call(
) -> EntryPointExecutionResult<CallInfo> {
let tracked_resource =
*context.tracked_resource_stack.last().expect("Unexpected empty tracked resource.");
// let enable_reverts = context.versioned_constants().enable_reverts;
let entry_point_initial_budget = context.gas_costs().base.entry_point_initial_budget;
let VmExecutionContext {
mut runner,
mut syscall_handler,
Expand All @@ -69,13 +71,31 @@ pub fn execute_entry_point_call(
program_extra_data_length,
} = initialize_execution_context(call, &compiled_class, state, context)?;

let args = prepare_call_arguments(
let args = match prepare_call_arguments(
&syscall_handler.base.call,
&mut runner,
initial_syscall_ptr,
&mut syscall_handler.read_only_segments,
&entry_point,
)?;
entry_point_initial_budget,
) {
Ok(args) => args,
Err(PreExecutionError::OutOfGas) => {
return Ok(CallInfo {
call: syscall_handler.base.call,
execution: CallExecution {
retdata: Retdata(vec![Felt::from_hex(OUT_OF_GAS_ERROR).unwrap()]),
failed: true,
gas_consumed: 0,
..CallExecution::default()
},
tracked_resource,
..CallInfo::default()
});
}
Err(err) => return Err(EntryPointExecutionError::from(err)),
};

let n_total_args = args.len();

// Execute.
Expand Down Expand Up @@ -180,6 +200,7 @@ pub fn prepare_call_arguments(
initial_syscall_ptr: Relocatable,
read_only_segments: &mut ReadOnlySegments,
entrypoint: &EntryPointV1,
entry_point_initial_budget: u64,
) -> Result<Args, PreExecutionError> {
let mut args: Args = vec![];

Expand Down Expand Up @@ -208,8 +229,13 @@ pub fn prepare_call_arguments(
}
return Err(PreExecutionError::InvalidBuiltin(*builtin_name));
}
// Include the initial entry point budget in the initial gas calculation.
if call.initial_gas < entry_point_initial_budget {
return Err(PreExecutionError::OutOfGas);
}
let initial_gas = call.initial_gas - entry_point_initial_budget;
// Push gas counter.
args.push(CairoArg::Single(MaybeRelocatable::from(Felt::from(call.initial_gas))));
args.push(CairoArg::Single(MaybeRelocatable::from(Felt::from(initial_gas))));
// Push syscall ptr.
args.push(CairoArg::Single(MaybeRelocatable::from(initial_syscall_ptr)));

Expand Down
2 changes: 2 additions & 0 deletions crates/blockifier/src/execution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ pub enum PreExecutionError {
UninitializedStorageAddress(ContractAddress),
#[error("Called builtins: {0:?} are unsupported in a Cairo0 contract")]
UnsupportedCairo0Builtin(HashSet<BuiltinName>),
#[error("Out of gas.")]
OutOfGas,
}

impl From<RunnerError> for PreExecutionError {
Expand Down
27 changes: 26 additions & 1 deletion crates/blockifier/src/execution/native/entry_point_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use cairo_native::execution_result::ContractExecutionResult;
use cairo_native::utils::BuiltinCosts;
use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
use stacker;
use starknet_types_core::felt::Felt;

use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources, Retdata};
use crate::execution::contract_class::TrackedResource;
Expand All @@ -14,6 +15,7 @@ use crate::execution::entry_point_execution::gas_consumed_without_inner_calls;
use crate::execution::errors::{EntryPointExecutionError, PostExecutionError};
use crate::execution::native::contract_class::NativeCompiledClassV1;
use crate::execution::native::syscall_handler::NativeSyscallHandler;
use crate::execution::syscalls::hint_processor::{SyscallExecutionError, OUT_OF_GAS_ERROR};
use crate::state::state_api::State;

// todo(rodrigo): add an `entry point not found` test for Native
Expand All @@ -23,6 +25,8 @@ pub fn execute_entry_point_call(
state: &mut dyn State,
context: &mut EntryPointExecutionContext,
) -> EntryPointExecutionResult<CallInfo> {
let orig_call = call.clone();

let entry_point = compiled_class.get_entry_point(&call)?;

let mut syscall_handler: NativeSyscallHandler<'_> =
Expand Down Expand Up @@ -56,13 +60,34 @@ pub fn execute_entry_point_call(
// TODO(Aviv/Yonatan): add these numbers to overridable VC.
let stack_size_red_zone = 160 * 1024 * 1024;
let target_stack_size = stack_size_red_zone + 10 * 1024 * 1024;
// Include the initial entry point budget in the initial gas calculation.
let entry_point_initial_budget =
syscall_handler.base.context.gas_costs().base.entry_point_initial_budget;
if syscall_handler.base.call.initial_gas < entry_point_initial_budget {
let current_tracked_resource = compiled_class
.casm()
.tracked_resource(&context.versioned_constants().min_sierra_version_for_sierra_gas);

return Ok(CallInfo {
call: orig_call,
execution: CallExecution {
retdata: Retdata(vec![Felt::from_hex(OUT_OF_GAS_ERROR).unwrap()]),
failed: true,
gas_consumed: 0,
..CallExecution::default()
},
tracked_resource: current_tracked_resource,
..CallInfo::default()
});
}
let initial_gas = syscall_handler.base.call.initial_gas - entry_point_initial_budget;
// Use `maybe_grow` and not `grow` for performance, since in happy flows, only the main call
// should trigger the growth.
let execution_result = stacker::maybe_grow(stack_size_red_zone, target_stack_size, || {
compiled_class.executor.run(
entry_point.selector.0,
&syscall_handler.base.call.calldata.0.clone(),
syscall_handler.base.call.initial_gas,
initial_gas,
Some(builtin_costs),
&mut syscall_handler,
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
pub const REQUIRED_GAS_CALL_CONTRACT_TEST: u64 = 120370;
pub const REQUIRED_GAS_STORAGE_READ_WRITE_TEST: u64 = 16990;
pub const REQUIRED_GAS_GET_CLASS_HASH_AT_TEST: u64 = 7830;
pub const REQUIRED_GAS_LIBRARY_CALL_TEST: u64 = 117970;
pub const REQUIRED_GAS_CALL_CONTRACT_TEST: u64 = 130370;
pub const REQUIRED_GAS_STORAGE_READ_WRITE_TEST: u64 = 26990;
pub const REQUIRED_GAS_GET_CLASS_HASH_AT_TEST: u64 = 17830;
pub const REQUIRED_GAS_LIBRARY_CALL_TEST: u64 = 127970;
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ fn with_constructor(runnable_version: RunnableCairo1) {
let deploy_call = &entry_point_call.execute_directly(&mut state).unwrap();
assert_eq!(
deploy_call.execution,
CallExecution { retdata: retdata![], gas_consumed: 164550, ..CallExecution::default() }
CallExecution { retdata: retdata![], gas_consumed: 174550, ..CallExecution::default() }
);

let constructor_call = &deploy_call.inner_calls[0];
Expand All @@ -136,7 +136,7 @@ fn with_constructor(runnable_version: RunnableCairo1) {
// The test contract constructor returns its first argument.
retdata: retdata![constructor_calldata[0]],
// This reflects the gas cost of storage write syscall.
gas_consumed: 4610,
gas_consumed: 14610,
..CallExecution::default()
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn positive_flow(runnable_version: RunnableCairo1) {
call_info.execution,
CallExecution {
events: vec![OrderedEvent { order: 0, event }],
gas_consumed: 47330,
gas_consumed: 57330,
..Default::default()
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ fn positive_flow(runnable_version: RunnableCairo1) {

assert_eq!(
entry_point_call.clone().execute_directly(&mut state).unwrap().execution,
CallExecution { gas_consumed: 5220, ..CallExecution::from_retdata(retdata![block_hash]) }
CallExecution { gas_consumed: 15220, ..CallExecution::from_retdata(retdata![block_hash]) }
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ fn test_keccak(runnable_version: RunnableCairo1) {

pretty_assertions::assert_eq!(
entry_point_call.execute_directly(&mut state).unwrap().execution,
CallExecution { gas_consumed: 254240, ..CallExecution::from_retdata(retdata![]) }
CallExecution { gas_consumed: 264240, ..CallExecution::from_retdata(retdata![]) }
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ fn test_library_call_assert_fails(runnable_version: RunnableCairo1) {
// 'ENTRYPOINT_FAILED'.
felt!("0x454e545259504f494e545f4641494c4544")
]),
gas_consumed: 100980,
gas_consumed: 110980,
failed: true,
..Default::default()
}
Expand Down Expand Up @@ -148,7 +148,7 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) {
..nested_storage_entry_point
};

let storage_entry_point_gas = GasAmount(16990);
let storage_entry_point_gas = GasAmount(26990);

// The default VersionedConstants is used in the execute_directly call bellow.
let tracked_resource = test_contract.get_runnable_class().tracked_resource(
Expand Down Expand Up @@ -177,7 +177,7 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) {
gas_consumed: REQUIRED_GAS_LIBRARY_CALL_TEST,
..CallExecution::default()
},
charged_resources: ChargedResources::from_gas(GasAmount(117970)),
charged_resources: ChargedResources::from_gas(GasAmount(127970)),
inner_calls: vec![nested_storage_call_info],
tracked_resource,
..Default::default()
Expand All @@ -197,7 +197,7 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) {
..Default::default()
};

let main_gas_consumed = 325110;
let main_gas_consumed = 335110;
let expected_call_info = CallInfo {
call: main_entry_point.clone(),
execution: CallExecution {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fn test_stack_overflow() {
CallExecution {
// 'Out of gas'
retdata: retdata![felt!["0x4f7574206f6620676173"]],
gas_consumed: MAX_POSSIBLE_SIERRA_GAS - 6590,
gas_consumed: MAX_POSSIBLE_SIERRA_GAS - 5380,
failed: true,
..Default::default()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fn positive_flow(runnable_version: RunnableCairo1) {
};
assert_eq!(
entry_point_call.execute_directly(&mut state).unwrap().execution,
CallExecution { gas_consumed: 5220, ..Default::default() }
CallExecution { gas_consumed: 15220, ..Default::default() }
);
assert_eq!(state.get_class_hash_at(contract_address).unwrap(), new_class_hash);
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fn test_secp256k1(runnable_version: RunnableCairo1) {

pretty_assertions::assert_eq!(
entry_point_call.execute_directly(&mut state).unwrap().execution,
CallExecution { gas_consumed: 17033486, ..Default::default() }
CallExecution { gas_consumed: 17043486, ..Default::default() }
);
}

Expand All @@ -45,6 +45,6 @@ fn test_secp256r1(runnable_version: RunnableCairo1) {

pretty_assertions::assert_eq!(
entry_point_call.execute_directly(&mut state).unwrap().execution,
CallExecution { gas_consumed: 27562930, ..Default::default() }
CallExecution { gas_consumed: 27572930, ..Default::default() }
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ fn test_send_message_to_l1(runnable_version: RunnableCairo1) {
entry_point_call.execute_directly(&mut state).unwrap().execution,
CallExecution {
l2_to_l1_messages: vec![OrderedL2ToL1Message { order: 0, message }],
gas_consumed: 20960,
gas_consumed: 30960,
..Default::default()
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ fn test_sha256(runnable_version: RunnableCairo1) {

pretty_assertions::assert_eq!(
entry_point_call.execute_directly(&mut state).unwrap().execution,
CallExecution { gas_consumed: 881425, ..CallExecution::from_retdata(retdata![]) }
CallExecution { gas_consumed: 891425, ..CallExecution::from_retdata(retdata![]) }
);
}
14 changes: 9 additions & 5 deletions crates/blockifier/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,18 @@ pub fn trivial_external_entry_point_new(contract: FeatureContract) -> CallEntryP
pub fn trivial_external_entry_point_with_address(
contract_address: ContractAddress,
) -> CallEntryPoint {
let vesrioned_const = VersionedConstants::create_for_testing();
let initial_gas = vesrioned_const.os_constants
.gas_costs
.base
.default_initial_gas_cost +
// Caller's initial budget is refunded for paying entry point's initial budget.
vesrioned_const.os_constants.gas_costs.base.entry_point_initial_budget;

CallEntryPoint {
code_address: Some(contract_address),
storage_address: contract_address,
initial_gas: VersionedConstants::create_for_testing()
.os_constants
.gas_costs
.base
.default_initial_gas_cost,
initial_gas,
..Default::default()
}
}
Expand Down
Loading

0 comments on commit cb1a1ed

Please sign in to comment.