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

chore(l2): forbid as conversions #1347

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions crates/common/types/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,18 @@ pub enum TxType {
Privileged = 0x7e,
}

impl From<TxType> for u8 {
fn from(val: TxType) -> Self {
match val {
TxType::Legacy => 0x00,
TxType::EIP2930 => 0x01,
TxType::EIP1559 => 0x02,
TxType::EIP4844 => 0x03,
TxType::Privileged => 0x7e,
}
}
}

pub trait Signable {
fn sign(&self, private_key: &SecretKey) -> Self
where
Expand Down
2 changes: 2 additions & 0 deletions crates/l2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,5 @@ path = "./l2.rs"
[lints.clippy]
unwrap_used = "deny"
expect_used = "deny"
as_conversions = "deny"
unnecessary_cast = "warn"
4 changes: 4 additions & 0 deletions crates/l2/proposer/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ pub enum CommitterError {
InvalidWithdrawalTransaction,
#[error("Blob estimation failed: {0}")]
BlobEstimationError(#[from] BlobEstimationError),
#[error("length does not fit in u16")]
TryIntoError(#[from] std::num::TryFromIntError),
}

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -126,4 +128,6 @@ pub enum StateDiffError {
BytecodeAndBytecodeHashSet,
#[error("Empty account diff")]
EmptyAccountDiff,
#[error("The length of the vector is too big to fit in u16: {0}")]
LengthTooBig(#[from] core::num::TryFromIntError),
}
18 changes: 12 additions & 6 deletions crates/l2/proposer/l1_committer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl Committer {
.send_commitment(
block_to_commit.header.number,
withdrawal_logs_merkle_root,
deposit_logs_hash,
deposit_logs_hash?,
blobs_bundle,
)
.await
Expand Down Expand Up @@ -206,11 +206,15 @@ impl Committer {
deposits
}

pub fn get_deposit_hash(&self, deposit_hashes: Vec<H256>) -> H256 {
if !deposit_hashes.is_empty() {
pub fn get_deposit_hash(&self, deposit_hashes: Vec<H256>) -> Result<H256, CommitterError> {
Ok(if !deposit_hashes.is_empty() {
let deposit_hashes_len: u16 = deposit_hashes
.len()
.try_into()
.map_err(CommitterError::from)?;
H256::from_slice(
[
&(deposit_hashes.len() as u16).to_be_bytes(),
&deposit_hashes_len.to_be_bytes(),
&keccak(
deposit_hashes
.iter()
Expand All @@ -225,7 +229,7 @@ impl Committer {
)
} else {
H256::zero()
}
})
}
/// Prepare the state diff for the block.
pub fn prepare_state_diff(
Expand Down Expand Up @@ -265,7 +269,9 @@ impl Committer {
.clone()
.ok_or(CommitterError::FailedToRetrieveDataFromStorage)?
.nonce
- prev_nonce) as u16,
- prev_nonce)
.try_into()
.map_err(CommitterError::from)?,
storage: account_update.added_storage.clone().into_iter().collect(),
bytecode: account_update.code.clone(),
bytecode_hash: None,
Expand Down
7 changes: 6 additions & 1 deletion crates/l2/proposer/prover_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,12 @@ impl ProverServer {
calldata.extend(journal_digest.as_bytes());

// extend with size of seal
calldata.extend(H256::from_low_u64_be(seal.len() as u64).as_bytes());
calldata.extend(
H256::from_low_u64_be(seal.len().try_into().map_err(|err| {
ProverServerError::Custom(format!("Seal length does not fit in u64: {}", err))
})?)
.as_bytes(),
);
// extend with seal
calldata.extend(seal);
// extend with zero padding
Expand Down
48 changes: 40 additions & 8 deletions crates/l2/proposer/state_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,32 @@ impl Default for StateDiff {
}
}

impl From<AccountStateDiffType> for u8 {
fn from(value: AccountStateDiffType) -> Self {
match value {
AccountStateDiffType::NewBalance => 1,
AccountStateDiffType::NonceDiff => 2,
AccountStateDiffType::Storage => 4,
AccountStateDiffType::Bytecode => 8,
AccountStateDiffType::BytecodeHash => 16,
}
}
}

impl StateDiff {
pub fn encode(&self) -> Result<Bytes, StateDiffError> {
if self.version != 1 {
return Err(StateDiffError::UnsupportedVersion(self.version));
}
let modified_accounts_len: u16 = self
.modified_accounts
.len()
.try_into()
.map_err(StateDiffError::from)?;

let mut encoded: Vec<u8> = Vec::new();
encoded.push(self.version);
encoded.extend((self.modified_accounts.len() as u16).to_be_bytes());
encoded.extend(modified_accounts_len.to_be_bytes());

for (address, diff) in &self.modified_accounts {
let (r#type, diff_encoded) = diff.encode()?;
Expand Down Expand Up @@ -119,20 +136,28 @@ impl AccountStateDiff {
let mut encoded: Vec<u8> = Vec::new();

if let Some(new_balance) = self.new_balance {
r#type += AccountStateDiffType::NewBalance as u8;
let r_type: u8 = AccountStateDiffType::NewBalance.into();
r#type += r_type;
let buf = &mut [0u8; 32];
new_balance.to_big_endian(buf);
encoded.extend_from_slice(buf);
}

if self.nonce_diff != 0 {
r#type += AccountStateDiffType::NonceDiff as u8;
let r_type: u8 = AccountStateDiffType::NonceDiff.into();
r#type += r_type;
encoded.extend(self.nonce_diff.to_be_bytes());
}

if !self.storage.is_empty() {
r#type += AccountStateDiffType::Storage as u8;
encoded.extend((self.storage.len() as u16).to_be_bytes());
let r_type: u8 = AccountStateDiffType::Storage.into();
let storage_len: u16 = self
.storage
.len()
.try_into()
.map_err(StateDiffError::from)?;
r#type += r_type;
encoded.extend(storage_len.to_be_bytes());
for (key, value) in &self.storage {
encoded.extend_from_slice(&key.0);
let buf = &mut [0u8; 32];
Expand All @@ -142,13 +167,20 @@ impl AccountStateDiff {
}

if let Some(bytecode) = &self.bytecode {
r#type += AccountStateDiffType::Bytecode as u8;
encoded.extend((bytecode.len() as u16).to_be_bytes());
let r_type: u8 = AccountStateDiffType::Bytecode.into();
let bytecode_len: u16 = self
.storage
.len()
.try_into()
.map_err(StateDiffError::from)?;
r#type += r_type;
encoded.extend(bytecode_len.to_be_bytes());
encoded.extend(bytecode);
}

if let Some(bytecode_hash) = &self.bytecode_hash {
r#type += AccountStateDiffType::BytecodeHash as u8;
let r_type: u8 = AccountStateDiffType::BytecodeHash.into();
r#type += r_type;
encoded.extend(&bytecode_hash.0);
}

Expand Down
6 changes: 5 additions & 1 deletion crates/l2/utils/eth_client/eth_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,11 @@ impl EthClient {
let encoded_from = deployer.encode_to_vec();
// FIXME: We'll probably need to use nonce - 1 since it was updated above.
let encoded_nonce = self.get_nonce(deployer).await?.encode_to_vec();
let mut encoded = vec![(0xc0 + encoded_from.len() + encoded_nonce.len()) as u8];
let mut encoded = vec![(0xc0 + encoded_from.len() + encoded_nonce.len())
.try_into()
.map_err(|err| {
EthClientError::Custom(format!("Failed to encode deployed_address {}", err))
})?];
encoded.extend(encoded_from.clone());
encoded.extend(encoded_nonce.clone());
let deployed_address = Address::from(keccak(encoded));
Expand Down
50 changes: 23 additions & 27 deletions crates/l2/utils/eth_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ impl EthClient {
let signed_tx = tx.sign(private_key);

let mut encoded_tx = signed_tx.encode_to_vec();
encoded_tx.insert(0, TxType::EIP1559 as u8);
encoded_tx.insert(0, TxType::EIP1559.into());

self.send_raw_transaction(encoded_tx.as_slice()).await
}
Expand All @@ -139,7 +139,7 @@ impl EthClient {
wrapped_tx.tx.sign_inplace(private_key);

let mut encoded_tx = wrapped_tx.encode_to_vec();
encoded_tx.insert(0, TxType::EIP4844 as u8);
encoded_tx.insert(0, TxType::EIP4844.into());

self.send_raw_transaction(encoded_tx.as_slice()).await
}
Expand Down Expand Up @@ -246,7 +246,7 @@ impl EthClient {
})?;
// Sometimes the penalty is a 100%
// Increase max fee per gas by 110% (set it to 210% of the original)
self.bump_eip1559(tx, 1.1);
self.bump_eip1559(tx, 110);
let wrapped_tx = &mut WrappedTransaction::EIP1559(tx.clone());
self.estimate_gas_for_wrapped_tx(wrapped_tx, from).await?;

Expand All @@ -259,11 +259,10 @@ impl EthClient {
}

/// Increase max fee per gas by percentage% (set it to (100+percentage)% of the original)
pub fn bump_eip1559(&self, tx: &mut EIP1559Transaction, percentage: f64) {
pub fn bump_eip1559(&self, tx: &mut EIP1559Transaction, percentage: u64) {
// TODO: handle as conversions
tx.max_fee_per_gas = (tx.max_fee_per_gas as f64 * (1.0 + percentage)).round() as u64;
tx.max_priority_fee_per_gas +=
(tx.max_priority_fee_per_gas as f64 * (1.0 + percentage)).round() as u64;
tx.max_fee_per_gas = (tx.max_fee_per_gas * (100 + percentage)) / 100;
tx.max_priority_fee_per_gas += (tx.max_priority_fee_per_gas * (100 + percentage)) / 100;
}

pub async fn bump_and_resend_eip4844(
Expand All @@ -276,7 +275,7 @@ impl EthClient {
})?;
// Sometimes the penalty is a 100%
// Increase max fee per gas by 110% (set it to 210% of the original)
self.bump_eip4844(wrapped_tx, 1.1);
self.bump_eip4844(wrapped_tx, 110);
let wrapped_eip4844 = &mut WrappedTransaction::EIP4844(wrapped_tx.clone());
self.estimate_gas_for_wrapped_tx(wrapped_eip4844, from)
.await?;
Expand All @@ -291,14 +290,12 @@ impl EthClient {
}

/// Increase max fee per gas by percentage% (set it to (100+percentage)% of the original)
pub fn bump_eip4844(&self, wrapped_tx: &mut WrappedEIP4844Transaction, percentage: f64) {
pub fn bump_eip4844(&self, wrapped_tx: &mut WrappedEIP4844Transaction, percentage: u64) {
// TODO: handle as conversions
wrapped_tx.tx.max_fee_per_gas =
(wrapped_tx.tx.max_fee_per_gas as f64 * (1.0 + percentage)).round() as u64;
wrapped_tx.tx.max_priority_fee_per_gas =
(wrapped_tx.tx.max_priority_fee_per_gas as f64 * (1.0 + percentage)).round() as u64;

let factor = ((1.0 + percentage) * 10.0).ceil() as u64;
wrapped_tx.tx.max_fee_per_gas = (wrapped_tx.tx.max_fee_per_gas * (100 + percentage)) / 100;
wrapped_tx.tx.max_priority_fee_per_gas +=
(wrapped_tx.tx.max_priority_fee_per_gas * (100 + percentage)) / 100;
let factor = 1 + (percentage / 100) * 10;
wrapped_tx.tx.max_fee_per_blob_gas = wrapped_tx
.tx
.max_fee_per_blob_gas
Expand All @@ -316,7 +313,7 @@ impl EthClient {
})?;
// Sometimes the penalty is a 100%
// Increase max fee per gas by 110% (set it to 210% of the original)
self.bump_privileged_l2(tx, 1.1);
self.bump_privileged_l2(tx, 110);
let wrapped_tx = &mut WrappedTransaction::L2(tx.clone());
self.estimate_gas_for_wrapped_tx(wrapped_tx, from).await?;
if let WrappedTransaction::L2(l2_tx) = wrapped_tx {
Expand All @@ -328,11 +325,10 @@ impl EthClient {
}

/// Increase max fee per gas by percentage% (set it to (100+percentage)% of the original)
pub fn bump_privileged_l2(&self, tx: &mut PrivilegedL2Transaction, percentage: f64) {
pub fn bump_privileged_l2(&self, tx: &mut PrivilegedL2Transaction, percentage: u64) {
// TODO: handle as conversions
tx.max_fee_per_gas = (tx.max_fee_per_gas as f64 * (1.0 + percentage)).round() as u64;
tx.max_priority_fee_per_gas +=
(tx.max_priority_fee_per_gas as f64 * (1.0 + percentage)).round() as u64;
tx.max_fee_per_gas = (tx.max_fee_per_gas * (100 + percentage)) / 100;
tx.max_priority_fee_per_gas += (tx.max_priority_fee_per_gas * (100 + percentage)) / 100;
}

pub async fn send_privileged_l2_transaction(
Expand All @@ -343,7 +339,7 @@ impl EthClient {
let signed_tx = tx.sign(private_key);

let mut encoded_tx = signed_tx.encode_to_vec();
encoded_tx.insert(0, TxType::Privileged as u8);
encoded_tx.insert(0, TxType::Privileged.into());

self.send_raw_transaction(encoded_tx.as_slice()).await
}
Expand Down Expand Up @@ -672,13 +668,13 @@ impl EthClient {
if error.contains("transaction underpriced") {
match wrapped_tx {
WrappedTransaction::EIP4844(wrapped_eip4844_transaction) => {
self.bump_eip4844(wrapped_eip4844_transaction, 1.1);
self.bump_eip4844(wrapped_eip4844_transaction, 110);
}
WrappedTransaction::EIP1559(eip1559_transaction) => {
self.bump_eip1559(eip1559_transaction, 1.1);
self.bump_eip1559(eip1559_transaction, 110);
}
WrappedTransaction::L2(privileged_l2_transaction) => {
self.bump_privileged_l2(privileged_l2_transaction, 1.1);
self.bump_privileged_l2(privileged_l2_transaction, 110);
}
};
continue;
Expand Down Expand Up @@ -754,7 +750,7 @@ impl EthClient {
if error.contains("replacement transaction underpriced") {
warn!("Bumping gas while building: already known");
retry += 1;
self.bump_eip1559(&mut tx, 1.1);
self.bump_eip1559(&mut tx, 110);
continue;
}
return Err(e);
Expand Down Expand Up @@ -840,7 +836,7 @@ impl EthClient {
if error.contains("already known") {
warn!("Bumping gas while building: already known");
retry += 1;
self.bump_eip4844(&mut wrapped_eip4844, 1.1);
self.bump_eip4844(&mut wrapped_eip4844, 110);
continue;
}
return Err(e);
Expand Down Expand Up @@ -922,7 +918,7 @@ impl EthClient {
if error.contains("already known") {
warn!("Bumping gas while building: already known");
retry += 1;
self.bump_privileged_l2(&mut tx, 1.1);
self.bump_privileged_l2(&mut tx, 110);
continue;
}
return Err(e);
Expand Down
Loading