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.

Sign up for GitHub

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

Jump to bottom

fix: Optimism execution #789

Merged
merged 6 commits into from Oct 12, 2023
Merged

fix: Optimism execution #789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c52acd6
use l2 base fee
Wollac Oct 8, 2023
d5dd7cd
use optimism handler when activated
Wollac Oct 8, 2023
fd616f2
fix panic for eth execution with optimism flag
Wollac Oct 10, 2023
e7b6b1d
use is_none
Wollac Oct 10, 2023
56804fa
address review comments
Wollac Oct 11, 2023
7951fab
Merge branch 'main' into fix/optimism-execution
Wollac Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 20 additions & 23 deletions crates/revm/src/evm_impl.rs
View file
Open in desktop
Expand Up @@ -216,34 +216,23 @@ impl<'a, GSPEC: Spec + 'static, DB: Database> Transact<DB::Error> for EVMImpl<'a
let tx_data = env.tx.data.clone();
let tx_gas_limit = env.tx.gas_limit;

// the L1-cost fee is only computed for Optimism non-deposit transactions.
#[cfg(feature = "optimism")]
let tx_l1_cost = {
let is_deposit = env.tx.optimism.source_hash.is_some();

let tx_l1_cost = if env.cfg.optimism && env.tx.optimism.source_hash.is_none() {
let l1_block_info =
optimism::L1BlockInfo::try_fetch(self.data.db, self.data.env.cfg.optimism)
.map_err(EVMError::Database)?;

// Perform this calculation optimistically to avoid cloning the enveloped tx.
let tx_l1_cost = l1_block_info.as_ref().map(|l1_block_info| {
env.tx
.optimism
.enveloped_tx
.as_ref()
.map(|enveloped_tx| {
l1_block_info.calculate_tx_l1_cost::<GSPEC>(enveloped_tx, is_deposit)
})
.unwrap_or(U256::ZERO)
});
// storage l1 block info for later use.
self.data.l1_block_info = l1_block_info;
optimism::L1BlockInfo::try_fetch(self.data.db).map_err(EVMError::Database)?;

//
let Some(tx_l1_cost) = tx_l1_cost else {
panic!("[OPTIMISM] L1 Block Info could not be loaded from the DB.")
let Some(enveloped_tx) = &env.tx.optimism.enveloped_tx else {
panic!("[OPTIMISM] Failed to load enveloped transaction.");
};
let tx_l1_cost = l1_block_info.calculate_tx_l1_cost::<GSPEC>(enveloped_tx);

// storage l1 block info for later use.
self.data.l1_block_info = Some(l1_block_info);

tx_l1_cost
} else {
U256::ZERO
};

let initial_gas_spend = initial_tx_gas::<GSPEC>(
Expand Down Expand Up @@ -444,6 +433,14 @@ impl<'a, GSPEC: Spec + 'static, DB: Database> EVMImpl<'a, GSPEC, DB> {
} else {
InstructionTables::Plain(Arc::new(make_instruction_table::<GSPEC, Self>()))
};
#[cfg(feature = "optimism")]
let handler = if env.cfg.optimism {
Handler::optimism::<GSPEC>()
} else {
Handler::mainnet::<GSPEC>()
};
#[cfg(not(feature = "optimism"))]
let handler = Handler::mainnet::<GSPEC>();

Self {
data: EVMData {
Expand All @@ -457,7 +454,7 @@ impl<'a, GSPEC: Spec + 'static, DB: Database> EVMImpl<'a, GSPEC, DB> {
},
inspector,
instruction_table,
handler: Handler::mainnet::<GSPEC>(),
handler,
_phantomdata: PhantomData {},
}
}
Expand Down
9 changes: 6 additions & 3 deletions crates/revm/src/handler/optimism.rs
View file
Open in desktop
Expand Up @@ -112,7 +112,7 @@ pub fn reward_beneficiary<SPEC: Spec, DB: Database>(
panic!("[OPTIMISM] Failed to load enveloped transaction.");
};

let l1_cost = l1_block_info.calculate_tx_l1_cost::<SPEC>(enveloped_tx, is_deposit);
let l1_cost = l1_block_info.calculate_tx_l1_cost::<SPEC>(enveloped_tx);

// Send the L1 cost of the transaction to the L1 Fee Vault.
let Ok((l1_fee_vault_account, _)) = data
Expand All @@ -132,8 +132,11 @@ pub fn reward_beneficiary<SPEC: Spec, DB: Database>(
panic!("[OPTIMISM] Failed to load Base Fee Vault account");
};
base_fee_vault_account.mark_touch();
base_fee_vault_account.info.balance +=
l1_block_info.l1_base_fee.mul(U256::from(gas.spend()));
base_fee_vault_account.info.balance += data
.env
.block
.basefee
.mul(U256::from(gas.spend() - gas_refund));
}
Ok(())
}
Expand Down
54 changes: 24 additions & 30 deletions crates/revm/src/optimism.rs
View file
Open in desktop
Expand Up @@ -41,23 +41,17 @@ pub struct L1BlockInfo {
}

impl L1BlockInfo {
pub fn try_fetch<DB: Database>(
db: &mut DB,
is_optimism: bool,
) -> Result<Option<L1BlockInfo>, DB::Error> {
is_optimism
.then(|| {
let l1_base_fee = db.storage(L1_BLOCK_CONTRACT, L1_BASE_FEE_SLOT)?;
let l1_fee_overhead = db.storage(L1_BLOCK_CONTRACT, L1_OVERHEAD_SLOT)?;
let l1_fee_scalar = db.storage(L1_BLOCK_CONTRACT, L1_SCALAR_SLOT)?;

Ok(L1BlockInfo {
l1_base_fee,
l1_fee_overhead,
l1_fee_scalar,
})
})
.map_or(Ok(None), |v| v.map(Some))
/// Try to fetch the L1 block info from the database.
pub fn try_fetch<DB: Database>(db: &mut DB) -> Result<L1BlockInfo, DB::Error> {
let l1_base_fee = db.storage(L1_BLOCK_CONTRACT, L1_BASE_FEE_SLOT)?;
let l1_fee_overhead = db.storage(L1_BLOCK_CONTRACT, L1_OVERHEAD_SLOT)?;
let l1_fee_scalar = db.storage(L1_BLOCK_CONTRACT, L1_SCALAR_SLOT)?;

Ok(L1BlockInfo {
l1_base_fee,
l1_fee_overhead,
l1_fee_scalar,
})
}

/// Calculate the data gas for posting the transaction on L1. Calldata costs 16 gas per non-zero
Expand All @@ -83,19 +77,18 @@ impl L1BlockInfo {
}

/// Calculate the gas cost of a transaction based on L1 block data posted on L2
pub fn calculate_tx_l1_cost<SPEC: Spec>(&self, input: &Bytes, is_deposit: bool) -> U256 {
let rollup_data_gas_cost = self.data_gas::<SPEC>(input);

if is_deposit || rollup_data_gas_cost == U256::ZERO {
pub fn calculate_tx_l1_cost<SPEC: Spec>(&self, input: &Bytes) -> U256 {
// If the input is not a deposit transaction, the default value is zero.
if input.is_empty() || input.first() == Some(&0x7F) {
return U256::ZERO;
}

let rollup_data_gas_cost = self.data_gas::<SPEC>(input);
rollup_data_gas_cost
.saturating_add(self.l1_fee_overhead)
.saturating_mul(self.l1_base_fee)
.saturating_mul(self.l1_fee_scalar)
.checked_div(U256::from(1_000_000))
.unwrap_or_default()
/ U256::from(1_000_000)
Comment on lines -97 to +91
Copy link
Contributor

Choose a reason for hiding this comment

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

Would prefer if we kept this as a checked div just in case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checked_div only checks if the divisor is 0. Since it is always hardcoded to one million, I don't think checked div would make any difference.
However, checked operations might make sense before multiplication and addition, as there should be no overflows there.

}
}

Expand Down Expand Up @@ -160,17 +153,18 @@ mod tests {
l1_fee_scalar: U256::from(1_000),
};

// The gas cost here should be zero since the tx is a deposit
let input = bytes!("FACADE");
let gas_cost = l1_block_info.calculate_tx_l1_cost::<BedrockSpec>(&input, true);
assert_eq!(gas_cost, U256::ZERO);

let gas_cost = l1_block_info.calculate_tx_l1_cost::<RegolithSpec>(&input, false);
let gas_cost = l1_block_info.calculate_tx_l1_cost::<RegolithSpec>(&input);
assert_eq!(gas_cost, U256::from(1048));

// Zero rollup data gas cost should result in zero for non-deposits
// Zero rollup data gas cost should result in zero
let input = bytes!("");
let gas_cost = l1_block_info.calculate_tx_l1_cost::<RegolithSpec>(&input, false);
let gas_cost = l1_block_info.calculate_tx_l1_cost::<RegolithSpec>(&input);
assert_eq!(gas_cost, U256::ZERO);

// Deposit transactions with the EIP-2718 type of 0x7F should result in zero
let input = bytes!("7FFACADE");
let gas_cost = l1_block_info.calculate_tx_l1_cost::<RegolithSpec>(&input);
assert_eq!(gas_cost, U256::ZERO);
}
}