allow zero as gas prices (#416)

* allow zero as gas prices

* improve felt_to_u128 conversion

* improve gas price conversion

* export FeltConversionError

* handle errors properly

* clippy

* fix unit test

* avoid using unwrap

* use expect instead of unwrap

* add test with non-zero gas price values

* remove unnecesary Ok - clippy

---------

Co-authored-by: Herman Obst Demaestri <[email protected]>

crates/bin/prove_block/src/lib.rs

@@ -13,6 +13,7 @@ use rpc_client::RpcClient;
 use rpc_replay::block_context::build_block_context;
 use rpc_replay::rpc_state_reader::AsyncRpcStateReader;
 use rpc_replay::transactions::{starknet_rs_to_blockifier, ToBlockifierError};
+use rpc_replay::utils::FeltConversionError;
 use rpc_utils::{get_class_proofs, get_storage_proofs};
 use starknet::core::types::{BlockId, MaybePendingBlockWithTxHashes, MaybePendingBlockWithTxs, StarknetError};
 use starknet::providers::{Provider, ProviderError};
@@ -67,6 +68,8 @@ pub enum ProveBlockError {
     StarknetApiError(StarknetApiError),
     #[error("To Blockifier Error: {0}")]
     ToBlockifierError(#[from] ToBlockifierError),
+    #[error("Felt Conversion Error: {0}")]
+    FeltConversionError(#[from] FeltConversionError),
 }
 
 fn compute_class_commitment(
@@ -160,7 +163,7 @@ pub async fn prove_block(
         };
     let old_block_number = Felt252::from(older_block.block_number);
     let old_block_hash = older_block.block_hash;
-    let block_context = build_block_context(chain_id.clone(), &block_with_txs, starknet_version);
+    let block_context = build_block_context(chain_id.clone(), &block_with_txs, starknet_version)?;
 
     // TODO: nasty clone, the conversion fns don't take references
     let transactions: Vec<_> =

crates/rpc-replay/src/block_context.rs

@@ -1,19 +1,34 @@
+use std::num::NonZeroU128;
+
 use blockifier::blockifier::block::{BlockInfo, GasPrices};
 use blockifier::bouncer::BouncerConfig;
 use blockifier::context::{BlockContext, ChainInfo, FeeTokenAddresses};
 use blockifier::versioned_constants::VersionedConstants;
-use starknet::core::types::{BlockWithTxs, L1DataAvailabilityMode};
+use starknet::core::types::{BlockWithTxs, Felt, L1DataAvailabilityMode};
 use starknet_api::block::{BlockNumber, BlockTimestamp};
 use starknet_api::core::{ChainId, ContractAddress, PatriciaKey};
 use starknet_api::{contract_address, felt, patricia_key};
 
-use crate::utils::felt_to_u128;
+use crate::utils::{felt_to_u128, FeltConversionError};
+
+fn felt_to_gas_price(price: &Felt) -> Result<NonZeroU128, FeltConversionError> {
+    // Inspiration taken from Papyrus:
+    // https://github.com/starkware-libs/sequencer/blob/7218aa1f7ca3fe21c0a2bede2570820939ffe069/crates/papyrus_execution/src/lib.rs#L363-L371
+    if *price == Felt::ZERO {
+        return Ok(NonZeroU128::MIN);
+    }
+
+    // Catch here if price > U128::MAX
+    let gas_price = felt_to_u128(price)?;
+    // If felt_to_u128 conversion is ok, it won't fail cause we're catching the zero above
+    NonZeroU128::new(gas_price).ok_or(FeltConversionError::CustomError("Gas price cannot be zero".to_string()))
+}
 
 pub fn build_block_context(
     chain_id: ChainId,
     block: &BlockWithTxs,
     starknet_version: blockifier::versioned_constants::StarknetVersion,
-) -> BlockContext {
+) -> Result<BlockContext, FeltConversionError> {
     let sequencer_address_hex = block.sequencer_address.to_hex_string();
     let sequencer_address = contract_address!(sequencer_address_hex.as_str());
     let use_kzg_da = match block.l1_da_mode {
@@ -26,10 +41,10 @@ pub fn build_block_context(
         block_timestamp: BlockTimestamp(block.timestamp),
         sequencer_address,
         gas_prices: GasPrices {
-            eth_l1_gas_price: felt_to_u128(&block.l1_gas_price.price_in_wei).try_into().unwrap(),
-            strk_l1_gas_price: felt_to_u128(&block.l1_gas_price.price_in_fri).try_into().unwrap(),
-            eth_l1_data_gas_price: felt_to_u128(&block.l1_data_gas_price.price_in_wei).try_into().unwrap(),
-            strk_l1_data_gas_price: felt_to_u128(&block.l1_data_gas_price.price_in_fri).try_into().unwrap(),
+            eth_l1_gas_price: felt_to_gas_price(&block.l1_gas_price.price_in_wei)?,
+            strk_l1_gas_price: felt_to_gas_price(&block.l1_gas_price.price_in_fri)?,
+            eth_l1_data_gas_price: felt_to_gas_price(&block.l1_data_gas_price.price_in_wei)?,
+            strk_l1_data_gas_price: felt_to_gas_price(&block.l1_data_gas_price.price_in_fri)?,
         },
         use_kzg_da,
     };
@@ -50,5 +65,93 @@ pub fn build_block_context(
     let versioned_constants = VersionedConstants::get(starknet_version);
     let bouncer_config = BouncerConfig::max();
 
-    BlockContext::new(block_info, chain_info, versioned_constants.clone(), bouncer_config)
+    Ok(BlockContext::new(block_info, chain_info, versioned_constants.clone(), bouncer_config))
+}
+
+#[cfg(test)]
+mod tests {
+
+    use starknet::core::types::{Felt, ResourcePrice};
+    use starknet_api::core::ChainId;
+
+    use super::*;
+
+    #[test]
+    fn test_build_block_context_with_zero_gas_prices() {
+        let chain_id = ChainId::Mainnet;
+        // We don't really care about most of the fields.
+        // What's important here is to set to zero different gas prices
+        let block = BlockWithTxs {
+            status: starknet::core::types::BlockStatus::AcceptedOnL1,
+            block_hash: Felt::ZERO,
+            parent_hash: Felt::ZERO,
+            block_number: 1,
+            new_root: Felt::ZERO,
+            timestamp: 0,
+            sequencer_address: Felt::ZERO,
+            l1_gas_price: ResourcePrice { price_in_wei: Felt::ZERO, price_in_fri: Felt::ZERO },
+            l1_data_gas_price: ResourcePrice { price_in_wei: Felt::ZERO, price_in_fri: Felt::ZERO },
+            l1_da_mode: L1DataAvailabilityMode::Blob,
+            starknet_version: String::from("0.13.2.1"),
+            transactions: vec![],
+        };
+
+        let starknet_version = blockifier::versioned_constants::StarknetVersion::Latest;
+
+        // Call this function must not fail
+        let block_context = build_block_context(chain_id, &block, starknet_version).unwrap();
+
+        // Verify that gas prices were set to NonZeroU128::MIN
+        assert_eq!(block_context.block_info().gas_prices.eth_l1_gas_price, NonZeroU128::MIN);
+        assert_eq!(block_context.block_info().gas_prices.strk_l1_gas_price, NonZeroU128::MIN);
+        assert_eq!(block_context.block_info().gas_prices.eth_l1_data_gas_price, NonZeroU128::MIN);
+        assert_eq!(block_context.block_info().gas_prices.strk_l1_data_gas_price, NonZeroU128::MIN);
+    }
+
+    #[test]
+    fn test_build_block_context_with_custom_gas_prices() {
+        let chain_id = ChainId::Mainnet;
+
+        // Expected values for gas price
+        let wei_l1_price = 1234;
+        let fri_l1_price = 5678;
+        let wei_l1_data_price = 9012;
+        let fri_l1_data_price = 3456;
+
+        let block = BlockWithTxs {
+            status: starknet::core::types::BlockStatus::AcceptedOnL1,
+            block_hash: Felt::ZERO,
+            parent_hash: Felt::ZERO,
+            block_number: 1,
+            new_root: Felt::ZERO,
+            timestamp: 0,
+            sequencer_address: Felt::ZERO,
+            l1_gas_price: ResourcePrice {
+                price_in_wei: Felt::from(wei_l1_price),
+                price_in_fri: Felt::from(fri_l1_price),
+            },
+            l1_data_gas_price: ResourcePrice {
+                price_in_wei: Felt::from(wei_l1_data_price),
+                price_in_fri: Felt::from(fri_l1_data_price),
+            },
+            l1_da_mode: L1DataAvailabilityMode::Blob,
+            starknet_version: String::from("0.13.2.1"),
+            transactions: vec![],
+        };
+
+        let starknet_version = blockifier::versioned_constants::StarknetVersion::Latest;
+        let block_context = build_block_context(chain_id, &block, starknet_version).unwrap();
+
+        // Verify that gas prices match our input values
+        assert_eq!(block_context.block_info().gas_prices.eth_l1_gas_price, NonZeroU128::new(wei_l1_price).unwrap());
+        assert_eq!(block_context.block_info().gas_prices.strk_l1_gas_price, NonZeroU128::new(fri_l1_price).unwrap());
+        assert_eq!(
+            block_context.block_info().gas_prices.eth_l1_data_gas_price,
+            NonZeroU128::new(wei_l1_data_price).unwrap()
+        );
+        assert_eq!(
+            block_context.block_info().gas_prices.strk_l1_data_gas_price,
+            NonZeroU128::new(fri_l1_data_price).unwrap()
+        );
+    }
 }

crates/rpc-replay/src/lib.rs

@@ -1,4 +1,4 @@
 pub mod block_context;
 pub mod rpc_state_reader;
 pub mod transactions;
-pub(crate) mod utils;
+pub mod utils;

crates/rpc-replay/src/transactions.rs

@@ -21,7 +21,7 @@ use starknet_os_types::sierra_contract_class::GenericSierraContractClass;
 use starknet_os_types::starknet_core_addons::LegacyContractDecompressionError;
 use thiserror::Error;
 
-use crate::utils::felt_to_u128;
+use crate::utils::{felt_to_u128, FeltConversionError};
 
 #[derive(Error, Debug)]
 pub enum ToBlockifierError {
@@ -37,6 +37,8 @@ pub enum ToBlockifierError {
     StarknetApiError(#[from] StarknetApiError),
     #[error("Transaction Execution Error: {0}")]
     TransactionExecutionError(#[from] TransactionExecutionError),
+    #[error("Felt Conversion Error: {0}")]
+    FeltConversionError(#[from] FeltConversionError),
 }
 
 pub fn resource_bounds_core_to_api(
@@ -74,7 +76,7 @@ fn invoke_v1_to_blockifier(
 ) -> Result<blockifier::transaction::transaction_execution::Transaction, ToBlockifierError> {
     let tx_hash = TransactionHash(tx.transaction_hash);
     let api_tx = starknet_api::transaction::InvokeTransaction::V1(starknet_api::transaction::InvokeTransactionV1 {
-        max_fee: Fee(felt_to_u128(&tx.max_fee)),
+        max_fee: Fee(felt_to_u128(&tx.max_fee)?),
         signature: starknet_api::transaction::TransactionSignature(tx.signature.to_vec()),
         nonce: starknet_api::core::Nonce(tx.nonce),
         sender_address: starknet_api::core::ContractAddress(PatriciaKey::try_from(tx.sender_address)?),
@@ -153,7 +155,7 @@ async fn declare_v1_to_blockifier(
 ) -> Result<blockifier::transaction::transaction_execution::Transaction, ToBlockifierError> {
     let tx_hash = TransactionHash(tx.transaction_hash);
     let api_tx = starknet_api::transaction::DeclareTransaction::V1(starknet_api::transaction::DeclareTransactionV0V1 {
-        max_fee: starknet_api::transaction::Fee(felt_to_u128(&tx.max_fee)),
+        max_fee: starknet_api::transaction::Fee(felt_to_u128(&tx.max_fee)?),
         signature: starknet_api::transaction::TransactionSignature(tx.signature.clone()),
         nonce: starknet_api::core::Nonce(tx.nonce),
         class_hash: starknet_api::core::ClassHash(tx.class_hash),
@@ -174,7 +176,7 @@ async fn declare_v2_to_blockifier(
 ) -> Result<blockifier::transaction::transaction_execution::Transaction, ToBlockifierError> {
     let tx_hash = TransactionHash(tx.transaction_hash);
     let api_tx = starknet_api::transaction::DeclareTransaction::V2(starknet_api::transaction::DeclareTransactionV2 {
-        max_fee: starknet_api::transaction::Fee(felt_to_u128(&tx.max_fee)),
+        max_fee: starknet_api::transaction::Fee(felt_to_u128(&tx.max_fee)?),
         signature: starknet_api::transaction::TransactionSignature(tx.signature.clone()),
         nonce: starknet_api::core::Nonce(tx.nonce),
         class_hash: starknet_api::core::ClassHash(tx.class_hash),
@@ -276,11 +278,11 @@ fn recalculate_contract_address(
 
 fn deploy_account_v1_to_blockifier(
     tx: &DeployAccountTransactionV1,
-) -> Result<blockifier::transaction::transaction_execution::Transaction, StarknetApiError> {
+) -> Result<blockifier::transaction::transaction_execution::Transaction, ToBlockifierError> {
     let tx_hash = TransactionHash(tx.transaction_hash);
 
     let (max_fee, signature, nonce, class_hash, constructor_calldata, contract_address_salt) = (
-        Fee(felt_to_u128(&tx.max_fee)),
+        Fee(felt_to_u128(&tx.max_fee)?),
         starknet_api::transaction::TransactionSignature(tx.signature.to_vec()),
         starknet_api::core::Nonce(tx.nonce),
         starknet_api::core::ClassHash(tx.class_hash),
@@ -292,8 +294,8 @@ fn deploy_account_v1_to_blockifier(
         class_hash,
         &constructor_calldata,
         ContractAddress::default(),
-    )
-    .unwrap();
+    )?;
+
     let api_tx = starknet_api::transaction::DeployAccountTransaction::V1(
         starknet_api::transaction::DeployAccountTransactionV1 {
             max_fee,

crates/rpc-replay/src/utils.rs

@@ -1,4 +1,5 @@
 use starknet::core::types::Felt;
+use thiserror::Error;
 
 /// Executes a coroutine from a synchronous context.
 /// Fails if no Tokio runtime is present.
@@ -10,7 +11,54 @@ where
     Ok(tokio::task::block_in_place(|| tokio_runtime_handle.block_on(coroutine)))
 }
 
-pub fn felt_to_u128(felt: &Felt) -> u128 {
+#[derive(Error, Debug)]
+pub enum FeltConversionError {
+    #[error("Overflow Error: Felt exceeds u128 max value")]
+    OverflowError,
+    #[error("{0}")]
+    CustomError(String),
+}
+
+pub fn felt_to_u128(felt: &Felt) -> Result<u128, FeltConversionError> {
     let digits = felt.to_be_digits();
-    ((digits[2] as u128) << 64) + digits[3] as u128
+
+    // Check if there are any significant bits in the higher 128 bits
+    if digits[0] != 0 || digits[1] != 0 {
+        return Err(FeltConversionError::OverflowError);
+    }
+
+    // Safe conversion since we've checked for overflow
+    Ok(((digits[2] as u128) << 64) + digits[3] as u128)
+}
+
+#[cfg(test)]
+mod tests {
+    use starknet::core::types::Felt;
+
+    use super::*;
+
+    #[test]
+    fn test_felt_to_u128_overflow() {
+        // digits[0] || digits[1] != 0
+        let overflow_felt = Felt::from(u128::MAX) + Felt::ONE;
+        assert!(felt_to_u128(&overflow_felt).is_err());
+    }
+
+    #[test]
+    fn test_felt_to_u128_ok() {
+        let felt_ok = Felt::from(u128::MAX);
+        assert!(felt_to_u128(&felt_ok).is_ok());
+
+        let felt_ok = Felt::from(123);
+        assert!(felt_to_u128(&felt_ok).is_ok());
+
+        let felt_ok = Felt::from(456789);
+        assert!(felt_to_u128(&felt_ok).is_ok());
+
+        let felt_ok = Felt::from(987654321);
+        assert!(felt_to_u128(&felt_ok).is_ok());
+
+        let felt_ok = Felt::from(123456789012345678u128);
+        assert!(felt_to_u128(&felt_ok).is_ok());
+    }
 }

crates/rpc-replay/tests/test_replay_block.rs

@@ -28,7 +28,8 @@ async fn test_replay_block() {
     let state_reader = AsyncRpcStateReader::new(rpc_client.clone(), previous_block_id);
     let mut state = CachedState::from(state_reader);
 
-    let block_context = build_block_context(ChainId::Sepolia, &block_with_txs, StarknetVersion::V0_13_1);
+    let block_context = build_block_context(ChainId::Sepolia, &block_with_txs, StarknetVersion::V0_13_1)
+        .expect("Failed to build block context");
 
     let traces = rpc_client
         .starknet_rpc()