Skip to content
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

allow zero as gas prices #416

Merged
merged 14 commits into from
Nov 5, 2024
5 changes: 4 additions & 1 deletion crates/bin/prove_block/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,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};
Expand Down Expand Up @@ -64,6 +65,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(
Expand Down Expand Up @@ -157,7 +160,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<_> =
Expand Down
70 changes: 62 additions & 8 deletions crates/rpc-replay/src/block_context.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,32 @@
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);
}

// If felt_to_u128 conversion is ok, it won't fail cause we're catching the zero above
Ok(NonZeroU128::new(felt_to_u128(price)?).unwrap())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't handle the error gracefully?

}

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 {
Expand All @@ -26,10 +39,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,
};
Expand All @@ -50,5 +63,46 @@ 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);
}
Comment on lines +68 to +109
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Could we add a test that checks that, if we have correct values, the block context is updated accordingly?

}
2 changes: 1 addition & 1 deletion crates/rpc-replay/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
pub mod block_context;
pub mod rpc_state_reader;
pub mod transactions;
pub(crate) mod utils;
pub mod utils;
18 changes: 10 additions & 8 deletions crates/rpc-replay/src/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(
Expand Down Expand Up @@ -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)?),
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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),
Expand All @@ -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,
Expand Down
50 changes: 48 additions & 2 deletions crates/rpc-replay/src/utils.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -10,7 +11,52 @@ 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,
}

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());
}
}
2 changes: 1 addition & 1 deletion crates/rpc-replay/tests/test_replay_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ 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).unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle error


let traces = rpc_client
.starknet_rpc()
Expand Down
Loading