Skip to content

Commit

Permalink
pr_comments: implement suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
fborello-lambda committed Dec 30, 2024
1 parent 88b97fc commit c7aa3b7
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 64 deletions.
13 changes: 1 addition & 12 deletions crates/l2/Makefile
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
.PHONY: help init down clean restart cli update-cli-contracts init-local-l1 init-l1 down-local-l1 restart-local-l1 rm-db-l1 clean-contract-deps restart-contract-deps deploy-l1 init-l2 down-l2 restart-l2 init-prover rm-db-l2 ci_test test
.DEFAULT_GOAL := help

L2_GENESIS_FILE_PATH=../../test_data/genesis-l2.json
L1_GENESIS_FILE_PATH=../../test_data/genesis-l1.json

# Basic
.PHONY: help init down clean restart

help: ## 📚 Show help for each of the Makefile recipes
@grep -E '^[a-zA-Z0-9_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'

Expand All @@ -18,8 +17,6 @@ clean: clean-contract-deps ## 🧹 Cleans the localnet
restart: restart-local-l1 deploy-l1 restart-l2 ## 🔄 Restarts the localnet

# CLI
.PHONY: cli update-cli-contracts

cli: ## 🛠️ Installs the L2 Lambda ethrex CLI
cargo install --path ${ethrex_PATH}/cmd/ethrex_l2/ --force

Expand Down Expand Up @@ -53,8 +50,6 @@ L1_AUTH_PORT=8551
L2_AUTH_PORT=8552

# Local L1
.PHONY: init-local-l1 init-l1 down-local-l1 restart-local-l1 rm-db-l1

init-local-l1: ## 🚀 Initializes an L1 Lambda ethrex Client with Docker (Used with make init)
docker compose -f ${ethrex_DEV_DOCKER_COMPOSE_PATH} up -d

Expand All @@ -76,8 +71,6 @@ rm-db-l1: ## 🛑 Removes the DB used by the L1
cargo run --release --manifest-path ../../Cargo.toml --bin ethrex -- removedb --datadir ${ethrex_L1_DEV_LIBMDBX}

# Contracts
PHONY: clean-contract-deps restart-contract-deps deploy-l1

clean-contract-deps: ## 🧹 Cleans the dependencies for the L1 contracts.
rm -rf contracts/solc_out
rm -rf contracts/lib
Expand All @@ -88,8 +81,6 @@ deploy-l1: ## 📜 Deploys the L1 contracts
DEPLOYER_CONTRACTS_PATH=contracts cargo run --release --bin ethrex_l2_l1_deployer --manifest-path ${ethrex_L2_CONTRACTS_PATH}/Cargo.toml

# L2
PHONY: init-l2 down-l2 restart-l2 init-prover rm-db-l2

init-l2: ## 🚀 Initializes an L2 Lambda ethrex Client
cargo run --release --manifest-path ../../Cargo.toml --bin ethrex --features l2 -- \
--network ${L2_GENESIS_FILE_PATH} \
Expand Down Expand Up @@ -123,8 +114,6 @@ rm-db-l2: ## 🛑 Removes the DB used by the L2
cargo run --release --manifest-path ../../Cargo.toml --bin ethrex -- removedb --datadir ${ethrex_L2_DEV_LIBMDBX}

# Testing
PHONY: ci_test test

ci_test: ## 🚧 Runs the L2's integration test, used by the github's CI
docker compose -f ${ethrex_L2_DOCKER_COMPOSE_PATH} down
docker compose -f ${ethrex_L2_DOCKER_COMPOSE_PATH} up -d --build
Expand Down
6 changes: 2 additions & 4 deletions crates/l2/contracts/deployer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,8 @@ async fn main() -> Result<(), DeployError> {
)
.await?;

let sp1_contract_verifier_address = match sp1_verifier_address {
Some(address) => address,
None => setup_result.sp1_contract_verifier_address,
};
let sp1_contract_verifier_address =
sp1_verifier_address.unwrap_or(setup_result.sp1_contract_verifier_address);

initialize_contracts(
setup_result.deployer_address,
Expand Down
2 changes: 2 additions & 0 deletions crates/l2/proposer/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub enum L1WatcherError {
FailedToRetrieveChainConfig(String),
#[error("L1Watcher failed to get config: {0}")]
FailedToGetConfig(#[from] ConfigError),
#[error("{0}")]
Custom(String),
}

#[derive(Debug, thiserror::Error)]
Expand Down
6 changes: 1 addition & 5 deletions crates/l2/proposer/l1_watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,7 @@ impl L1Watcher {

let gas_price = self.l2_client.get_gas_price().await?;
// Avoid panicking when using as_u64()
let gas_price = if gas_price > u64::MAX.into() {
u64::MAX
} else {
gas_price.as_u64()
};
let gas_price: u64 = gas_price.try_into()?;

let mut mint_transaction = self
.eth_client
Expand Down
15 changes: 6 additions & 9 deletions crates/l2/proposer/prover_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ impl Risc0Proof {
ProverServerError::Custom(format!("Failed to hex::decode(selector): {e}"))
})?
}
Err(_) => vec![32; 0],
Err(_) => vec![0_u8; 32],
};

let mut image_id: [u32; 8] = [0; 8];
let mut image_id = [0_u32; 8];
for (i, b) in image_id.iter_mut().enumerate() {
*b = *self.prover_id.get(i).ok_or(ProverServerError::Custom(
"Failed to get image_id in handle_proof_submission()".to_owned(),
Expand Down Expand Up @@ -618,16 +618,15 @@ impl ProverServer {
})?;

let calldata_len: u64 = (calldata.len()).try_into().map_err(|err| {
ProverServerError::Custom(format!("calldata length does not fit in u64: {}", err))
ProverServerError::Custom(format!("calldata length does not fit in u64: {err}"))
})?;

let leading_zeros_after_block_proof: u64 =
calculate_padding(calldata_len + 64 + block_proof_len)?
.try_into()
.map_err(|err| {
ProverServerError::Custom(format!(
"calculate_padding length does not fit in u64: {}",
err
"calculate_padding length does not fit in u64: {err}"
))
})?;

Expand All @@ -646,17 +645,15 @@ impl ProverServer {
.try_into()
.map_err(|err| {
ProverServerError::Custom(format!(
"public_values length does not fit in u64: {}",
err
"public_values length does not fit in u64: {err}"
))
})?;

let leading_zeros_after_public_values: u64 = calculate_padding(offset + public_values_len)?
.try_into()
.map_err(|err| {
ProverServerError::Custom(format!(
"calculate_padding length does not fit in u64: {}",
err
"calculate_padding length does not fit in u64: {err}"
))
})?;

Expand Down
8 changes: 4 additions & 4 deletions crates/l2/prover/zkvm/interface/risc0/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ fn main() {
let receipts = execute_block(&block, &mut state).expect("failed to execute block");
validate_gas_used(&receipts, &block.header).expect("invalid gas used");

let cumulative_gas_used = match receipts.last() {
Some(last_receipt) => last_receipt.cumulative_gas_used,
None => 0_u64,
};
let cumulative_gas_used = receipts
.last()
.map(|last_receipt| last_receipt.cumulative_gas_used)
.unwrap_or_default();

env::write(&cumulative_gas_used);

Expand Down
8 changes: 4 additions & 4 deletions crates/l2/prover/zkvm/interface/sp1/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ pub fn main() {
let receipts = execute_block(&block, &mut state).expect("failed to execute block");
validate_gas_used(&receipts, &block.header).expect("invalid gas used");

let cumulative_gas_used = match receipts.last() {
Some(last_receipt) => last_receipt.cumulative_gas_used,
None => 0_u64,
};
let cumulative_gas_used = receipts
.last()
.map(|last_receipt| last_receipt.cumulative_gas_used)
.unwrap_or_default();

sp1_zkvm::io::commit(&cumulative_gas_used);

Expand Down
46 changes: 20 additions & 26 deletions crates/l2/utils/eth_client/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::{fmt, time::Duration};
use std::{
fmt::{self, format},
time::Duration,
};

use crate::utils::config::eth::EthConfig;
use bytes::Bytes;
Expand Down Expand Up @@ -404,15 +407,12 @@ impl EthClient {
.to_owned(),
))?,
);

let string_len = if string_length > usize::MAX.into() {
return Err(EthClientError::Custom(
let string_len: usize = string_length.try_into().map_err(|_| {
EthClientError::Custom(
"Failed to convert string_length to usize in estimate_gas"
.to_owned(),
));
} else {
string_length.as_usize()
};
)
})?;
let string_data = abi_decoded_error_data.get(68..68 + string_len).ok_or(
EthClientError::Custom(
"Failed to slice index abi_decoded_error_data in estimate_gas"
Expand Down Expand Up @@ -724,7 +724,7 @@ impl EthClient {
overrides: Overrides,
bump_retries: u64,
) -> Result<EIP1559Transaction, EthClientError> {
let get_gas_price;
let get_gas_price: u64;
let mut tx = EIP1559Transaction {
to: TxKind::Call(to),
chain_id: if let Some(chain_id) = overrides.chain_id {
Expand All @@ -741,11 +741,9 @@ impl EthClient {
gas_price
} else {
let gas_price = self.get_gas_price().await?;
get_gas_price = if gas_price > u64::MAX.into() {
u64::MAX
} else {
gas_price.as_u64()
};
get_gas_price = gas_price
.try_into()
.map_err(|e| EthClientError::Custom(format!("{e}")))?;
get_gas_price
},
max_fee_per_gas: if let Some(gas_price) = overrides.gas_price {
Expand Down Expand Up @@ -813,7 +811,7 @@ impl EthClient {
) -> Result<WrappedEIP4844Transaction, EthClientError> {
let blob_versioned_hashes = blobs_bundle.generate_versioned_hashes();

let get_gas_price;
let get_gas_price: u64;
let tx = EIP4844Transaction {
to,
chain_id: if let Some(chain_id) = overrides.chain_id {
Expand All @@ -830,11 +828,9 @@ impl EthClient {
gas_price
} else {
let gas_price = self.get_gas_price().await?;
get_gas_price = if gas_price > u64::MAX.into() {
u64::MAX
} else {
gas_price.as_u64()
};
get_gas_price = gas_price
.try_into()
.map_err(|e| EthClientError::Custom(format!("{e}")))?;
get_gas_price
},
max_fee_per_gas: if let Some(gas_price) = overrides.gas_price {
Expand Down Expand Up @@ -903,7 +899,7 @@ impl EthClient {
overrides: Overrides,
bump_retries: u64,
) -> Result<PrivilegedL2Transaction, EthClientError> {
let get_gas_price;
let get_gas_price: u64;
let mut tx = PrivilegedL2Transaction {
tx_type,
to: TxKind::Call(to),
Expand All @@ -921,11 +917,9 @@ impl EthClient {
gas_price
} else {
let gas_price = self.get_gas_price().await?;
get_gas_price = if gas_price > u64::MAX.into() {
u64::MAX
} else {
gas_price.as_u64()
};
get_gas_price = gas_price
.try_into()
.map_err(|e| EthClientError::Custom(format!("{e}")))?;
get_gas_price
},
max_fee_per_gas: if let Some(gas_price) = overrides.gas_price {
Expand Down

0 comments on commit c7aa3b7

Please sign in to comment.