From d60ce6098b7e637306d55251795f16e6378d563a Mon Sep 17 00:00:00 2001
From: Arni Hod <arnon@starkware.co>
Date: Sun, 24 Nov 2024 13:29:29 +0200
Subject: [PATCH] chore(blockifier): separate create and validate logic on gas
 prices

---
 crates/blockifier/src/blockifier/block.rs     | 44 +++++++++++--------
 crates/blockifier/src/fee/fee_test.rs         |  6 +--
 .../blockifier/src/test_utils/struct_impls.rs |  2 +-
 crates/native_blockifier/src/py_state_diff.rs |  2 +-
 crates/papyrus_execution/src/lib.rs           |  2 +-
 crates/starknet_batcher/src/block_builder.rs  |  2 +-
 crates/starknet_gateway/src/rpc_objects.rs    |  2 +-
 7 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/crates/blockifier/src/blockifier/block.rs b/crates/blockifier/src/blockifier/block.rs
index 83aca48b1c..760afc8861 100644
--- a/crates/blockifier/src/blockifier/block.rs
+++ b/crates/blockifier/src/blockifier/block.rs
@@ -35,40 +35,45 @@ pub struct BlockInfo {
 #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Clone, Debug)]
 pub struct GasPrices {
-    eth_gas_prices: GasPriceVector,  // In wei.
-    strk_gas_prices: GasPriceVector, // In fri.
+    pub eth_gas_prices: GasPriceVector,  // In wei.
+    pub strk_gas_prices: GasPriceVector, // In fri.
 }
 
 impl GasPrices {
-    pub fn new(
-        eth_l1_gas_price: NonzeroGasPrice,
-        strk_l1_gas_price: NonzeroGasPrice,
-        eth_l1_data_gas_price: NonzeroGasPrice,
-        strk_l1_data_gas_price: NonzeroGasPrice,
-        eth_l2_gas_price: NonzeroGasPrice,
-        strk_l2_gas_price: NonzeroGasPrice,
-    ) -> Self {
+    /// Warns if the submitted gas prices do not match the expected gas prices.
+    fn validate_l2_gas_price(&self) {
         // TODO(Aner): fix backwards compatibility.
+        let eth_l2_gas_price = self.eth_gas_prices.l2_gas_price;
         let expected_eth_l2_gas_price = VersionedConstants::latest_constants()
-            .convert_l1_to_l2_gas_price_round_up(eth_l1_gas_price.into());
+            .convert_l1_to_l2_gas_price_round_up(self.eth_gas_prices.l1_gas_price.into());
         if GasPrice::from(eth_l2_gas_price) != expected_eth_l2_gas_price {
             // TODO!(Aner): change to panic! Requires fixing several tests.
             warn!(
-                "eth_l2_gas_price {eth_l2_gas_price} does not match expected eth_l2_gas_price \
-                 {expected_eth_l2_gas_price}."
+                "eth_l2_gas_price {} does not match expected eth_l2_gas_price {}.",
+                eth_l2_gas_price, expected_eth_l2_gas_price
             )
         }
+        let strk_l2_gas_price = self.strk_gas_prices.l2_gas_price;
         let expected_strk_l2_gas_price = VersionedConstants::latest_constants()
-            .convert_l1_to_l2_gas_price_round_up(strk_l1_gas_price.into());
+            .convert_l1_to_l2_gas_price_round_up(self.strk_gas_prices.l1_gas_price.into());
         if GasPrice::from(strk_l2_gas_price) != expected_strk_l2_gas_price {
             // TODO!(Aner): change to panic! Requires fixing test_discounted_gas_overdraft
             warn!(
-                "strk_l2_gas_price {strk_l2_gas_price} does not match expected strk_l2_gas_price \
-                 {expected_strk_l2_gas_price}."
+                "strk_l2_gas_price {} does not match expected strk_l2_gas_price {}.",
+                strk_l2_gas_price, expected_strk_l2_gas_price
             )
         }
+    }
 
-        Self {
+    pub fn validated_new(
+        eth_l1_gas_price: NonzeroGasPrice,
+        strk_l1_gas_price: NonzeroGasPrice,
+        eth_l1_data_gas_price: NonzeroGasPrice,
+        strk_l1_data_gas_price: NonzeroGasPrice,
+        eth_l2_gas_price: NonzeroGasPrice,
+        strk_l2_gas_price: NonzeroGasPrice,
+    ) -> Self {
+        let gas_prices = Self {
             eth_gas_prices: GasPriceVector {
                 l1_gas_price: eth_l1_gas_price,
                 l1_data_gas_price: eth_l1_data_gas_price,
@@ -79,7 +84,10 @@ impl GasPrices {
                 l1_data_gas_price: strk_l1_data_gas_price,
                 l2_gas_price: strk_l2_gas_price,
             },
-        }
+        };
+        gas_prices.validate_l2_gas_price();
+
+        gas_prices
     }
 
     pub fn get_l1_gas_price_by_fee_type(&self, fee_type: &FeeType) -> NonzeroGasPrice {
diff --git a/crates/blockifier/src/fee/fee_test.rs b/crates/blockifier/src/fee/fee_test.rs
index e96f443431..2d090a7871 100644
--- a/crates/blockifier/src/fee/fee_test.rs
+++ b/crates/blockifier/src/fee/fee_test.rs
@@ -178,7 +178,7 @@ fn test_discounted_gas_overdraft(
         NonzeroGasPrice::try_from(data_gas_price).unwrap(),
     );
     let mut block_context = BlockContext::create_for_account_testing();
-    block_context.block_info.gas_prices = GasPrices::new(
+    block_context.block_info.gas_prices = GasPrices::validated_new(
         DEFAULT_ETH_L1_GAS_PRICE,
         gas_price,
         DEFAULT_ETH_L1_DATA_GAS_PRICE,
@@ -313,7 +313,7 @@ fn test_get_fee_by_gas_vector_regression(
     #[case] expected_fee_strk: u128,
 ) {
     let mut block_info = BlockContext::create_for_account_testing().block_info;
-    block_info.gas_prices = GasPrices::new(
+    block_info.gas_prices = GasPrices::validated_new(
         1_u8.try_into().unwrap(),
         2_u8.try_into().unwrap(),
         3_u8.try_into().unwrap(),
@@ -347,7 +347,7 @@ fn test_get_fee_by_gas_vector_overflow(
 ) {
     let huge_gas_price = NonzeroGasPrice::try_from(2_u128 * u128::from(u64::MAX)).unwrap();
     let mut block_info = BlockContext::create_for_account_testing().block_info;
-    block_info.gas_prices = GasPrices::new(
+    block_info.gas_prices = GasPrices::validated_new(
         huge_gas_price,
         huge_gas_price,
         huge_gas_price,
diff --git a/crates/blockifier/src/test_utils/struct_impls.rs b/crates/blockifier/src/test_utils/struct_impls.rs
index dee7020eac..af42fb4eb8 100644
--- a/crates/blockifier/src/test_utils/struct_impls.rs
+++ b/crates/blockifier/src/test_utils/struct_impls.rs
@@ -152,7 +152,7 @@ impl BlockInfo {
             block_number: BlockNumber(CURRENT_BLOCK_NUMBER),
             block_timestamp: BlockTimestamp(CURRENT_BLOCK_TIMESTAMP),
             sequencer_address: contract_address!(TEST_SEQUENCER_ADDRESS),
-            gas_prices: GasPrices::new(
+            gas_prices: GasPrices::validated_new(
                 DEFAULT_ETH_L1_GAS_PRICE,
                 DEFAULT_STRK_L1_GAS_PRICE,
                 DEFAULT_ETH_L1_DATA_GAS_PRICE,
diff --git a/crates/native_blockifier/src/py_state_diff.rs b/crates/native_blockifier/src/py_state_diff.rs
index 70f15e98e3..6a78360363 100644
--- a/crates/native_blockifier/src/py_state_diff.rs
+++ b/crates/native_blockifier/src/py_state_diff.rs
@@ -182,7 +182,7 @@ impl TryFrom<PyBlockInfo> for BlockInfo {
             block_number: BlockNumber(block_info.block_number),
             block_timestamp: BlockTimestamp(block_info.block_timestamp),
             sequencer_address: ContractAddress::try_from(block_info.sequencer_address.0)?,
-            gas_prices: GasPrices::new(
+            gas_prices: GasPrices::validated_new(
                 NonzeroGasPrice::try_from(block_info.l1_gas_price.price_in_wei).map_err(|_| {
                     NativeBlockifierInputError::InvalidNativeBlockifierInputError(
                         InvalidNativeBlockifierInputError::InvalidL1GasPriceWei(
diff --git a/crates/papyrus_execution/src/lib.rs b/crates/papyrus_execution/src/lib.rs
index 4e7fada6bd..d997f58f04 100644
--- a/crates/papyrus_execution/src/lib.rs
+++ b/crates/papyrus_execution/src/lib.rs
@@ -369,7 +369,7 @@ fn create_block_context(
         use_kzg_da,
         block_number,
         // TODO(yair): What to do about blocks pre 0.13.1 where the data gas price were 0?
-        gas_prices: GasPrices::new(
+        gas_prices: GasPrices::validated_new(
             NonzeroGasPrice::new(l1_gas_price.price_in_wei).unwrap_or(NonzeroGasPrice::MIN),
             NonzeroGasPrice::new(l1_gas_price.price_in_fri).unwrap_or(NonzeroGasPrice::MIN),
             NonzeroGasPrice::new(l1_data_gas_price.price_in_wei).unwrap_or(NonzeroGasPrice::MIN),
diff --git a/crates/starknet_batcher/src/block_builder.rs b/crates/starknet_batcher/src/block_builder.rs
index ecb944c347..94a8fbb69a 100644
--- a/crates/starknet_batcher/src/block_builder.rs
+++ b/crates/starknet_batcher/src/block_builder.rs
@@ -312,7 +312,7 @@ impl BlockBuilderFactory {
             // TODO (yael 7/10/2024): add logic to compute gas prices
             gas_prices: {
                 let tmp_val = NonzeroGasPrice::MIN;
-                GasPrices::new(tmp_val, tmp_val, tmp_val, tmp_val, tmp_val, tmp_val)
+                GasPrices::validated_new(tmp_val, tmp_val, tmp_val, tmp_val, tmp_val, tmp_val)
             },
             use_kzg_da: block_builder_config.use_kzg_da,
         };
diff --git a/crates/starknet_gateway/src/rpc_objects.rs b/crates/starknet_gateway/src/rpc_objects.rs
index f9b7d17cc2..1209a4f459 100644
--- a/crates/starknet_gateway/src/rpc_objects.rs
+++ b/crates/starknet_gateway/src/rpc_objects.rs
@@ -86,7 +86,7 @@ impl TryInto<BlockInfo> for BlockHeader {
             block_number: self.block_number,
             sequencer_address: self.sequencer_address,
             block_timestamp: self.timestamp,
-            gas_prices: GasPrices::new(
+            gas_prices: GasPrices::validated_new(
                 parse_gas_price(self.l1_gas_price.price_in_wei)?,
                 parse_gas_price(self.l1_gas_price.price_in_fri)?,
                 parse_gas_price(self.l1_data_gas_price.price_in_wei)?,