Skip to content

feat(blockifier): exclude l1_data_gas from accounts in VC #5185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions crates/blockifier/src/execution/syscalls/hint_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,15 @@ impl ResourceAsFelts {

pub fn valid_resource_bounds_as_felts(
resource_bounds: &ValidResourceBounds,
exclude_l1_data_gas: bool,
) -> SyscallResult<Vec<ResourceAsFelts>> {
let mut resource_bounds_vec: Vec<_> = vec![
ResourceAsFelts::new(Resource::L1Gas, &resource_bounds.get_l1_bounds())?,
ResourceAsFelts::new(Resource::L2Gas, &resource_bounds.get_l2_bounds())?,
];
if exclude_l1_data_gas {
return Ok(resource_bounds_vec);
}
if let ValidResourceBounds::AllResources(AllResourceBounds { l1_data_gas, .. }) =
resource_bounds
{
Expand Down Expand Up @@ -472,9 +476,10 @@ impl<'a> SyscallHintProcessor<'a> {
&mut self,
vm: &mut VirtualMachine,
tx_info: &CurrentTransactionInfo,
exclude_l1_data_gas: bool,
) -> SyscallResult<(Relocatable, Relocatable)> {
let flat_resource_bounds: Vec<_> =
valid_resource_bounds_as_felts(&tx_info.resource_bounds)?
valid_resource_bounds_as_felts(&tx_info.resource_bounds, exclude_l1_data_gas)?
.into_iter()
.flat_map(ResourceAsFelts::flatten)
.collect();
Expand Down Expand Up @@ -656,8 +661,10 @@ impl<'a> SyscallHintProcessor<'a> {

match tx_info {
TransactionInfo::Current(context) => {
let (tx_resource_bounds_start_ptr, tx_resource_bounds_end_ptr) =
&self.allocate_tx_resource_bounds_segment(vm, context)?;
// See comment above about the returned version.
let should_exclude_l1_data_gas = self.base.should_exclude_l1_data_gas();
let (tx_resource_bounds_start_ptr, tx_resource_bounds_end_ptr) = &self
.allocate_tx_resource_bounds_segment(vm, context, should_exclude_l1_data_gas)?;

let (tx_paymaster_data_start_ptr, tx_paymaster_data_end_ptr) =
&self.allocate_data_segment(vm, &context.paymaster_data.0)?;
Expand Down
7 changes: 7 additions & 0 deletions crates/blockifier/src/execution/syscalls/syscall_base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,13 @@ impl<'state> SyscallHandlerBase<'state> {
tx_context.tx_info.signed_version()
}

/// Return whether the L1 data gas should be excluded for the `get_execution_info` syscall.
pub fn should_exclude_l1_data_gas(&self) -> bool {
let class_hash = self.call.class_hash;
let versioned_constants = &self.context.tx_context.block_context.versioned_constants;
versioned_constants.os_constants.data_gas_accounts.contains(&class_hash)
}

pub fn emit_event(&mut self, event: EventContent) -> SyscallResult<()> {
exceeds_event_size_limit(
self.context.versioned_constants(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ use crate::transaction::objects::{
TransactionVersion::ONE,
false,
false,
false,
false;
"Native: Validate execution mode: block info fields should be zeroed. Transaction V1."
)
Expand All @@ -63,6 +64,7 @@ use crate::transaction::objects::{
TransactionVersion::ONE,
false,
false,
false,
false;
"Native: Execute execution mode: block info should be as usual. Transaction V1."
)
Expand All @@ -75,6 +77,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
false,
false,
false,
false;
"Native: Validate execution mode: block info fields should be zeroed. Transaction V3."
)
Expand All @@ -87,6 +90,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
false,
false,
false,
false;
"Native: Execute execution mode: block info should be as usual. Transaction V3."
)
Expand All @@ -99,6 +103,7 @@ use crate::transaction::objects::{
TransactionVersion::ONE,
false,
false,
false,
false;
"Native: Legacy contract. Execute execution mode: block info should be as usual. Transaction
V1."
Expand All @@ -112,6 +117,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
false,
false,
false,
false;
"Native: Legacy contract. Execute execution mode: block info should be as usual. Transaction
V3."
Expand All @@ -125,6 +131,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
true,
false,
false,
false;
"Native: Execute execution mode: block info should be as usual. Transaction V3. Query"
)
Expand All @@ -137,6 +144,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
false,
true,
false,
false;
"Native: V1 bound account: execute"
)
Expand All @@ -149,6 +157,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
true,
true,
false,
false;
"Native: V1 bound account: query"
)
Expand All @@ -159,6 +168,7 @@ use crate::transaction::objects::{
TransactionVersion::ONE,
false,
false,
false,
false;
"Validate execution mode: block info fields should be zeroed. Transaction V1.")]
#[test_case(
Expand All @@ -167,6 +177,7 @@ use crate::transaction::objects::{
TransactionVersion::ONE,
false,
false,
false,
false;
"Execute execution mode: block info should be as usual. Transaction V1.")]
#[test_case(
Expand All @@ -175,6 +186,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
false,
false,
false,
false;
"Validate execution mode: block info fields should be zeroed. Transaction V3.")]
#[test_case(
Expand All @@ -183,6 +195,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
false,
false,
false,
false;
"Execute execution mode: block info should be as usual. Transaction V3.")]
#[test_case(
Expand All @@ -191,6 +204,7 @@ use crate::transaction::objects::{
TransactionVersion::ONE,
false,
false,
false,
false;
"Legacy contract. Execute execution mode: block info should be as usual. Transaction V1.")]
#[test_case(
Expand All @@ -199,6 +213,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
false,
false,
false,
false;
"Legacy contract. Execute execution mode: block info should be as usual. Transaction V3.")]
#[test_case(
Expand All @@ -207,6 +222,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
true,
false,
false,
false;
"Execute execution mode: block info should be as usual. Transaction V3. Query.")]
#[test_case(
Expand All @@ -215,6 +231,7 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
false,
true,
false,
false;
"V1 bound account: execute")]
#[test_case(
Expand All @@ -223,16 +240,36 @@ use crate::transaction::objects::{
TransactionVersion::THREE,
false,
true,
true;
true,
false;
"V1 bound account: execute, high tip")]
#[test_case(
FeatureContract::TestContract(CairoVersion::Cairo1(RunnableCairo1::Casm)),
ExecutionMode::Execute,
TransactionVersion::THREE,
true,
true,
false,
false;
"V1 bound account: query")]
#[test_case(
FeatureContract::TestContract(CairoVersion::Cairo1(RunnableCairo1::Casm)),
ExecutionMode::Execute,
TransactionVersion::THREE,
false,
false,
false,
true;
"Exclude l1 data gas: execute")]
#[test_case(
FeatureContract::TestContract(CairoVersion::Cairo1(RunnableCairo1::Casm)),
ExecutionMode::Execute,
TransactionVersion::THREE,
true,
false,
false,
true;
"Exclude l1 data gas: query")]
fn test_get_execution_info(
test_contract: FeatureContract,
execution_mode: ExecutionMode,
Expand All @@ -241,13 +278,21 @@ fn test_get_execution_info(
v1_bound_account: bool,
// Whether the tip is larger than `v1_bound_accounts_max_tip`.
high_tip: bool,
exclude_l1_data_gas: bool,
) {
let mut test_contract_data: FeatureContractData = test_contract.into();
if v1_bound_account {
assert!(
!exclude_l1_data_gas,
"Unable to set both exclude_l1_data_gas and v1_bound_account."
);
let optional_class_hash =
VersionedConstants::latest_constants().os_constants.v1_bound_accounts_cairo1.first();
test_contract_data.class_hash =
*optional_class_hash.expect("No v1 bound accounts found in versioned constants.");
} else if exclude_l1_data_gas {
test_contract_data.class_hash =
*VersionedConstants::latest_constants().os_constants.data_gas_accounts.first().unwrap();
}
let state =
&mut test_state_ex(&ChainInfo::create_for_testing(), BALANCE, &[(test_contract_data, 1)]);
Expand Down Expand Up @@ -329,15 +374,17 @@ fn test_get_execution_info(
(_, version) if version == TransactionVersion::ONE => vec![
felt!(0_u16), // Length of resource bounds array.
],
(_, _) => vec![felt!(3_u8)] // Length of resource bounds array.
.into_iter()
.chain(
valid_resource_bounds_as_felts(&all_resource_bounds)
.unwrap()
.into_iter()
.flat_map(|bounds| bounds.flatten()),
)
.collect(),
(_, _) => {
vec![felt!(if exclude_l1_data_gas { 2_u8 } else { 3_u8 })] // Length of resource bounds array.
.into_iter()
.chain(
valid_resource_bounds_as_felts(&all_resource_bounds, exclude_l1_data_gas)
.unwrap()
.into_iter()
.flat_map(|bounds| bounds.flatten()),
)
.collect()
}
};

let expected_tx_info: Vec<Felt>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,9 @@ impl<IG: IdentifierGetter> LoadCairoObject<IG> for ValidResourceBounds {
address: Relocatable,
constants: &HashMap<String, Felt>,
) -> OsHintResult {
valid_resource_bounds_as_felts(self).map_err(OsHintError::ResourceBoundsParsing)?.load_into(
vm,
identifier_getter,
address,
constants,
)
valid_resource_bounds_as_felts(self, false)
.map_err(OsHintError::ResourceBoundsParsing)?
.load_into(vm, identifier_getter, address, constants)
}
}

Expand Down
Loading