Skip to content

Commit

Permalink
chore(l2): trying to implement no_panic crate (#1389)
Browse files Browse the repository at this point in the history
**Motivation**

As explained by issue #1369, implementing the `no_panic` crate would
help prevent some hidden panics.

**Description**

I've tried to implement it for the non-async functions, facing the
following problems:
- For functions interacting with `TcpStream` and the `Database` it
throws a "panic compilation error".
- The `keccak` function used and `c-kzg` throws the "panic compilation
error".
- `no_panic` seems to be a crate that aims to avoid panics in low level
lib crates.

- Found a potential panic in `fake_exponential`, it's solved by the
`fake_exponential_checked` implementation.
- Some extra changes regarding usability were added.
- Remove `pub` keyword if not needed.

See if we can close the issue #1369
  • Loading branch information
fborello-lambda authored Dec 10, 2024
1 parent 88c6a8c commit 006a7f2
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 44 deletions.
42 changes: 42 additions & 0 deletions crates/common/types/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,48 @@ pub fn fake_exponential(factor: u64, numerator: u64, denominator: u64) -> u64 {
output / denominator
}

#[derive(Debug, thiserror::Error)]
pub enum FakeExponentialError {
#[error("Denominator cannot be zero.")]
DenominatorIsZero,
#[error("Checked div failed is None.")]
CheckedDiv,
#[error("Checked mul failed is None.")]
CheckedMul,
}

pub fn fake_exponential_checked(
factor: u64,
numerator: u64,
denominator: u64,
) -> Result<u64, FakeExponentialError> {
let mut i = 1_u64;
let mut output = 0_u64;
let mut numerator_accum = factor * denominator;
if denominator == 0 {
return Err(FakeExponentialError::DenominatorIsZero);
}

while numerator_accum > 0 {
output = output.saturating_add(numerator_accum);

let denominator_i = denominator
.checked_mul(i)
.ok_or(FakeExponentialError::CheckedMul)?;

if denominator_i == 0 {
return Err(FakeExponentialError::DenominatorIsZero);
}

numerator_accum = numerator_accum * numerator / denominator_i;
i += 1;
}

output
.checked_div(denominator)
.ok_or(FakeExponentialError::CheckedDiv)
}

// Calculates the base fee for the current block based on its gas_limit and parent's gas and fee
// Returns None if the block gas limit is not valid in relation to its parent's gas limit
pub fn calculate_base_fee_per_gas(
Expand Down
1 change: 0 additions & 1 deletion crates/l2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ keccak-hash = "0.10.0"
envy = "0.4.2"
thiserror.workspace = true
zkvm_interface = { path = "./prover/zkvm/interface/", default-features = false }

# risc0
risc0-zkvm = { version = "1.2.0" }

Expand Down
4 changes: 3 additions & 1 deletion crates/l2/proposer/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::sync::mpsc::SendError;
use crate::utils::merkle_tree::MerkleError;
use crate::utils::{config::errors::ConfigError, eth_client::errors::EthClientError};
use ethereum_types::FromStrRadixErr;
use ethrex_core::types::BlobsBundleError;
use ethrex_core::types::{BlobsBundleError, FakeExponentialError};
use ethrex_dev::utils::engine_client::errors::EngineClientError;
use ethrex_storage::error::StoreError;
use ethrex_vm::EvmError;
Expand Down Expand Up @@ -115,6 +115,8 @@ pub enum BlobEstimationError {
CalculationError,
#[error("Blob gas estimation resulted in an infinite or undefined value. Outside valid or expected ranges")]
NonFiniteResult,
#[error("{0}")]
FakeExponentialError(#[from] FakeExponentialError),
}

#[derive(Debug, thiserror::Error)]
Expand Down
26 changes: 13 additions & 13 deletions crates/l2/proposer/l1_committer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
use bytes::Bytes;
use ethrex_core::{
types::{
blobs_bundle, fake_exponential, BlobsBundle, Block, PrivilegedL2Transaction,
blobs_bundle, fake_exponential_checked, BlobsBundle, Block, PrivilegedL2Transaction,
PrivilegedTxType, Transaction, TxKind, BLOB_BASE_FEE_UPDATE_FRACTION,
MIN_BASE_FEE_PER_BLOB_GAS,
},
Expand Down Expand Up @@ -159,7 +159,7 @@ impl Committer {
}
}

pub fn get_block_withdrawals(
fn get_block_withdrawals(
&self,
block: &Block,
) -> Result<Vec<(H256, PrivilegedL2Transaction)>, CommitterError> {
Expand All @@ -180,7 +180,7 @@ impl Committer {
Ok(withdrawals)
}

pub fn get_withdrawals_merkle_root(
fn get_withdrawals_merkle_root(
&self,
withdrawals_hashes: Vec<H256>,
) -> Result<H256, CommitterError> {
Expand All @@ -191,7 +191,7 @@ impl Committer {
}
}

pub fn get_block_deposits(&self, block: &Block) -> Vec<PrivilegedL2Transaction> {
fn get_block_deposits(&self, block: &Block) -> Vec<PrivilegedL2Transaction> {
let deposits = block
.body
.transactions
Expand All @@ -209,7 +209,7 @@ impl Committer {
deposits
}

pub fn get_deposit_hash(&self, deposit_hashes: Vec<H256>) -> Result<H256, CommitterError> {
fn get_deposit_hash(&self, deposit_hashes: Vec<H256>) -> Result<H256, CommitterError> {
if !deposit_hashes.is_empty() {
let deposit_hashes_len: u16 = deposit_hashes
.len()
Expand All @@ -236,8 +236,9 @@ impl Committer {
Ok(H256::zero())
}
}

/// Prepare the state diff for the block.
pub fn prepare_state_diff(
fn prepare_state_diff(
&self,
block: &Block,
store: Store,
Expand Down Expand Up @@ -314,18 +315,15 @@ impl Committer {
}

/// Generate the blob bundle necessary for the EIP-4844 transaction.
pub fn generate_blobs_bundle(
&self,
state_diff: &StateDiff,
) -> Result<BlobsBundle, CommitterError> {
fn generate_blobs_bundle(&self, state_diff: &StateDiff) -> Result<BlobsBundle, CommitterError> {
let blob_data = state_diff.encode().map_err(CommitterError::from)?;

let blob = blobs_bundle::blob_from_bytes(blob_data).map_err(CommitterError::from)?;

BlobsBundle::create_from_blobs(&vec![blob]).map_err(CommitterError::from)
}

pub async fn send_commitment(
async fn send_commitment(
&self,
block_number: u64,
withdrawal_logs_merkle_root: H256,
Expand Down Expand Up @@ -438,11 +436,13 @@ async fn estimate_blob_gas(
};

// If the blob's market is in high demand, the equation may give a really big number.
let blob_gas = fake_exponential(
// This function doesn't panic, it performs checked/saturating operations.
let blob_gas = fake_exponential_checked(
MIN_BASE_FEE_PER_BLOB_GAS,
total_blob_gas,
BLOB_BASE_FEE_UPDATE_FRACTION,
);
)
.map_err(BlobEstimationError::FakeExponentialError)?;

let gas_with_headroom = (blob_gas * (100 + headroom)) / 100;

Expand Down
59 changes: 41 additions & 18 deletions crates/l2/proposer/prover_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use keccak_hash::keccak;
use secp256k1::SecretKey;
use serde::{Deserialize, Serialize};
use std::{
io::{BufReader, BufWriter},
io::{BufReader, BufWriter, Write},
net::{IpAddr, Shutdown, TcpListener, TcpStream},
sync::mpsc::{self, Receiver},
thread,
Expand Down Expand Up @@ -78,6 +78,34 @@ pub enum ProofData {
SubmitAck { block_number: u64 },
}

impl ProofData {
/// Builder function for creating a Request
pub fn request() -> Self {
ProofData::Request
}

/// Builder function for creating a Response
pub fn response(block_number: Option<u64>, input: Option<ProverInputData>) -> Self {
ProofData::Response {
block_number,
input,
}
}

/// Builder function for creating a Submit
pub fn submit(block_number: u64, receipt: (risc0_zkvm::Receipt, Vec<u32>)) -> Self {
ProofData::Submit {
block_number,
receipt: Box::new(receipt),
}
}

/// Builder function for creating a SubmitAck
pub fn submit_ack(block_number: u64) -> Self {
ProofData::SubmitAck { block_number }
}
}

pub async fn start_prover_server(store: Store) -> Result<(), ConfigError> {
let server_config = ProverServerConfig::from_env()?;
let eth_config = EthConfig::from_env()?;
Expand Down Expand Up @@ -219,10 +247,7 @@ impl ProverServer {
let data: Result<ProofData, _> = serde_json::de::from_reader(buf_reader);
match data {
Ok(ProofData::Request) => {
if let Err(e) = self
.handle_request(&mut stream, last_verified_block + 1)
.await
{
if let Err(e) = self.handle_request(&stream, last_verified_block + 1).await {
warn!("Failed to handle request: {e}");
}
}
Expand Down Expand Up @@ -252,7 +277,7 @@ impl ProverServer {

async fn handle_request(
&self,
stream: &mut TcpStream,
stream: &TcpStream,
block_number: u64,
) -> Result<(), ProverServerError> {
debug!("Request received");
Expand All @@ -263,18 +288,12 @@ impl ProverServer {
.ok_or(ProverServerError::StorageDataIsNone)?;

let response = if block_number > latest_block_number {
let response = ProofData::Response {
block_number: None,
input: None,
};
let response = ProofData::response(None, None);
warn!("Didn't send response");
response
} else {
let input = self.create_prover_input(block_number)?;
let response = ProofData::Response {
block_number: Some(block_number),
input: Some(input),
};
let response = ProofData::response(Some(block_number), Some(input));
info!("Sent Response for block_number: {block_number}");
response
};
Expand All @@ -291,10 +310,14 @@ impl ProverServer {
) -> Result<(), ProverServerError> {
debug!("Submit received for BlockNumber: {block_number}");

let response = ProofData::SubmitAck { block_number };
let writer = BufWriter::new(stream);
serde_json::to_writer(writer, &response)
.map_err(|e| ProverServerError::ConnectionError(e.into()))
let response = ProofData::submit_ack(block_number);
let json_string = serde_json::to_string(&response)
.map_err(|e| ProverServerError::Custom(format!("serde_json::to_string(): {e}")))?;
stream
.write_all(json_string.as_bytes())
.map_err(ProverServerError::ConnectionError)?;

Ok(())
}

async fn handle_proof_submission(
Expand Down
16 changes: 5 additions & 11 deletions crates/l2/prover/src/prover_client.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
use ethrex_l2::{
proposer::prover_server::ProofData, utils::config::prover_client::ProverClientConfig,
};
use std::{
io::{BufReader, BufWriter},
net::TcpStream,
time::Duration,
};

use tokio::time::sleep;
use tracing::{debug, error, info, warn};

use zkvm_interface::io::ProgramInput;

use ethrex_l2::{
proposer::prover_server::ProofData, utils::config::prover_client::ProverClientConfig,
};

use super::prover::Prover;

pub async fn start_proof_data_client(config: ProverClientConfig) {
Expand Down Expand Up @@ -61,7 +58,7 @@ impl ProverClient {

fn request_new_input(&self) -> Result<(u64, ProgramInput), String> {
// Request the input with the correct block_number
let request = ProofData::Request;
let request = ProofData::request();
let response = connect_to_prover_server_wr(&self.prover_server_endpoint, &request)
.map_err(|e| format!("Failed to get Response: {e}"))?;

Expand Down Expand Up @@ -93,10 +90,7 @@ impl ProverClient {
receipt: risc0_zkvm::Receipt,
prover_id: Vec<u32>,
) -> Result<(), String> {
let submit = ProofData::Submit {
block_number,
receipt: Box::new((receipt, prover_id)),
};
let submit = ProofData::submit(block_number, (receipt, prover_id));
let submit_ack = connect_to_prover_server_wr(&self.prover_server_endpoint, &submit)
.map_err(|e| format!("Failed to get SubmitAck: {e}"))?;

Expand Down

0 comments on commit 006a7f2

Please sign in to comment.