Skip to content

Commit

Permalink
feat(blockifier): panic on GasVector arithmetic overflow
Browse files Browse the repository at this point in the history
  • Loading branch information
dorimedini-starkware committed Oct 10, 2024
1 parent 71afbf7 commit f291432
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 55 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 7 additions & 5 deletions crates/blockifier/src/fee/fee_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,13 @@ fn test_get_fee_by_gas_vector_regression(
}

#[rstest]
#[case::l1_saturates(u64::MAX, 0, 0)]
#[case::l1_data_saturates(0, u64::MAX, 0)]
#[case::l2_gas_saturates(0, 0, u64::MAX)]
// TODO: Add L2 saturation case once `get_fee_by_gas_vector` implements L2 collection.
fn test_get_fee_by_gas_vector_saturation(
#[should_panic(expected = "L1Gas cost overflowed")]
#[case::l1_overflows(u64::MAX, 0, 0)]
#[should_panic(expected = "L1DataGas cost overflowed")]
#[case::l1_data_overflows(0, u64::MAX, 0)]
#[should_panic(expected = "L2Gas cost overflowed")]
#[case::l2_gas_overflows(0, 0, u64::MAX)]
fn test_get_fee_by_gas_vector_overflow(
#[case] l1_gas: u64,
#[case] l1_data_gas: u64,
#[case] l2_gas: u64,
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/fee/fee_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ pub fn get_fee_by_gas_vector(
gas_vector: GasVector,
fee_type: &FeeType,
) -> Fee {
gas_vector.saturated_cost(block_info.gas_prices.get_gas_prices_by_fee_type(fee_type))
gas_vector.cost(block_info.gas_prices.get_gas_prices_by_fee_type(fee_type))
}

/// Returns the current fee balance and a boolean indicating whether the balance covers the fee.
Expand Down
27 changes: 20 additions & 7 deletions crates/blockifier/src/fee/gas_usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,14 @@ pub fn get_log_message_to_l1_emissions_cost(l2_to_l1_payload_lengths: &[usize])
constants::LOG_MSG_TO_L1_ENCODED_DATA_SIZE + *length,
)
})
.sum()
.fold(GasVector::ZERO, |accumulator, cost| {
accumulator.checked_add(cost).unwrap_or_else(|| {
panic!(
"Overflow in message emission gas costs; attempted to add {accumulator:?} to \
{cost:?}"
)
})
})
}

fn get_event_emission_cost(n_topics: usize, data_length: usize) -> GasVector {
Expand Down Expand Up @@ -184,11 +191,17 @@ pub fn estimate_minimal_gas_vector(
+ versioned_constants.os_kzg_da_resources(data_segment_length).n_steps;

let resources = ExecutionResources { n_steps: os_steps_for_type, ..Default::default() };
get_da_gas_cost(&state_changes_by_account_tx, block_info.use_kzg_da)
+ get_vm_resources_cost(
versioned_constants,
&resources,
0,
gas_usage_vector_computation_mode,
let da_gas_cost = get_da_gas_cost(&state_changes_by_account_tx, block_info.use_kzg_da);
let vm_resources_cost = get_vm_resources_cost(
versioned_constants,
&resources,
0,
gas_usage_vector_computation_mode,
);
da_gas_cost.checked_add(vm_resources_cost).unwrap_or_else(|| {
panic!(
"Overflow in minimal gas estimation; attempted to add {da_gas_cost:?} to \
{vm_resources_cost:?}"
)
})
}
66 changes: 52 additions & 14 deletions crates/blockifier/src/fee/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,18 @@ impl TransactionResources {
use_kzg_da: bool,
computation_mode: &GasVectorComputationMode,
) -> GasVector {
self.starknet_resources.to_gas_vector(versioned_constants, use_kzg_da, computation_mode)
+ self.computation.to_gas_vector(versioned_constants, computation_mode)
let starknet_gas = self.starknet_resources.to_gas_vector(
versioned_constants,
use_kzg_da,
computation_mode,
);
let computation_gas = self.computation.to_gas_vector(versioned_constants, computation_mode);
starknet_gas.checked_add(computation_gas).unwrap_or_else(|| {
panic!(
"Transaction resources to gas vector overflowed: starknet gas cost is \
{starknet_gas:?}, computation gas is {computation_gas:?}",
)
})
}
}

Expand Down Expand Up @@ -112,9 +122,20 @@ impl StarknetResources {
use_kzg_da: bool,
mode: &GasVectorComputationMode,
) -> GasVector {
self.archival_data.to_gas_vector(versioned_constants, mode)
+ self.state.to_gas_vector(use_kzg_da)
+ self.messages.to_gas_vector()
[
self.archival_data.to_gas_vector(versioned_constants, mode),
self.state.to_gas_vector(use_kzg_da),
self.messages.to_gas_vector(),
]
.iter()
.fold(GasVector::ZERO, |accumulator, cost| {
accumulator.checked_add(*cost).unwrap_or_else(|| {
panic!(
"Starknet resources to gas vector overflowed: tried to add {accumulator:?} to \
{cost:?}",
)
})
})
}
}

Expand Down Expand Up @@ -248,7 +269,12 @@ impl MessageResources {
.into(),
);

starknet_gas_usage + sharp_gas_usage
starknet_gas_usage.checked_add(sharp_gas_usage).unwrap_or_else(|| {
panic!(
"Message resources to gas vector overflowed: starknet gas cost is \
{starknet_gas_usage:?}, SHARP gas cost is {sharp_gas_usage:?}.",
)
})
}

/// Returns an estimation of the gas usage for processing L1<>L2 messages on L1. Accounts for
Expand All @@ -257,20 +283,32 @@ impl MessageResources {
let n_l2_to_l1_messages = self.l2_to_l1_payload_lengths.len();
let n_l1_to_l2_messages = usize::from(self.l1_handler_payload_size.is_some());

GasVector::from_l1_gas(
// Starknet's updateState gets the message segment as an argument.
u64_from_usize(
self.message_segment_length * eth_gas_constants::GAS_PER_MEMORY_WORD
[
GasVector::from_l1_gas(
// Starknet's updateState gets the message segment as an argument.
u64_from_usize(
self.message_segment_length * eth_gas_constants::GAS_PER_MEMORY_WORD
// Starknet's updateState increases a (storage) counter for each L2-to-L1 message.
+ n_l2_to_l1_messages * eth_gas_constants::GAS_PER_ZERO_TO_NONZERO_STORAGE_SET
// Starknet's updateState decreases a (storage) counter for each L1-to-L2 consumed
// message (note that we will probably get a refund of 15,000 gas for each consumed
// message but we ignore it since refunded gas cannot be used for the current
// transaction execution).
+ n_l1_to_l2_messages * eth_gas_constants::GAS_PER_COUNTER_DECREASE,
)
.into(),
) + get_consumed_message_to_l2_emissions_cost(self.l1_handler_payload_size)
+ get_log_message_to_l1_emissions_cost(&self.l2_to_l1_payload_lengths)
)
.into(),
),
get_consumed_message_to_l2_emissions_cost(self.l1_handler_payload_size),
get_log_message_to_l1_emissions_cost(&self.l2_to_l1_payload_lengths),
]
.iter()
.fold(GasVector::ZERO, |accumulator, cost| {
accumulator.checked_add(*cost).unwrap_or_else(|| {
panic!(
"Message resources to starknet gas cost overflowed: tried to add \
{accumulator:?} to {cost:?}"
)
})
})
}
}
1 change: 0 additions & 1 deletion crates/starknet_api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ derive_more.workspace = true
hex.workspace = true
indexmap = { workspace = true, features = ["serde"] }
itertools.workspace = true
log.workspace = true
num-bigint.workspace = true
pretty_assertions.workspace = true
primitive-types = { workspace = true, features = ["serde"] }
Expand Down
54 changes: 28 additions & 26 deletions crates/starknet_api/src/execution_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,11 @@ impl GasAmount {
}
}

#[derive(
derive_more::Add,
derive_more::Sum,
derive_more::AddAssign,
Clone,
Copy,
Debug,
Default,
Eq,
PartialEq,
Deserialize,
Serialize,
#[cfg_attr(
any(test, feature = "testing"),
derive(derive_more::Add, derive_more::Sum, derive_more::AddAssign)
)]
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Deserialize, Serialize)]
pub struct GasVector {
pub l1_gas: GasAmount,
pub l1_data_gas: GasAmount,
Expand All @@ -95,6 +87,9 @@ pub struct GasVector {
}

impl GasVector {
pub const ZERO: GasVector =
GasVector { l1_gas: GasAmount(0), l1_data_gas: GasAmount(0), l2_gas: GasAmount(0) };

pub fn from_l1_gas(l1_gas: GasAmount) -> Self {
Self { l1_gas, ..Default::default() }
}
Expand All @@ -107,33 +102,40 @@ impl GasVector {
Self { l2_gas, ..Default::default() }
}

/// Computes the cost (in fee token units) of the gas vector (saturating on overflow).
pub fn saturated_cost(&self, gas_prices: &GasPriceVector) -> Fee {
pub fn checked_add(self, rhs: Self) -> Option<Self> {
match (
self.l1_gas.checked_add(rhs.l1_gas),
self.l1_data_gas.checked_add(rhs.l1_data_gas),
self.l2_gas.checked_add(rhs.l2_gas),
) {
(Some(l1_gas), Some(l1_data_gas), Some(l2_gas)) => {
Some(Self { l1_gas, l1_data_gas, l2_gas })
}
_ => None,
}
}

/// Computes the cost (in fee token units) of the gas vector (panicking on overflow).
pub fn cost(&self, gas_prices: &GasPriceVector) -> Fee {
let mut sum = Fee(0);
for (gas, price, resource) in [
(self.l1_gas, gas_prices.l1_gas_price, Resource::L1Gas),
(self.l1_data_gas, gas_prices.l1_data_gas_price, Resource::L1DataGas),
(self.l2_gas, gas_prices.l2_gas_price, Resource::L2Gas),
] {
let cost = gas.checked_mul(price.get()).unwrap_or_else(|| {
log::warn!(
panic!(
"{} cost overflowed: multiplication of gas amount ({}) by price per unit ({}) \
resulted in overflow.",
resource,
gas,
price
);
Fee(u128::MAX)
resource, gas, price
)
});
sum = sum.checked_add(cost).unwrap_or_else(|| {
log::warn!(
panic!(
"Total cost overflowed: addition of current sum ({}) and cost of {} ({}) \
resulted in overflow.",
sum,
resource,
cost
);
Fee(u128::MAX)
sum, resource, cost
)
});
}
sum
Expand Down

0 comments on commit f291432

Please sign in to comment.