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: refactor L1BlockInfo::tx_estimated_size_fjord #1856

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
4 changes: 3 additions & 1 deletion crates/optimism/src/fast_lz.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/// Returns the length of the data after compression through FastLZ, based on
// https://github.com/Vectorized/solady/blob/5315d937d79b335c668896d7533ac603adac5315/js/solady.js
/// <https://github.com/Vectorized/solady/blob/5315d937d79b335c668896d7533ac603adac5315/js/solady.js>
/// The u32s match op-geth's Go port:
/// <https://github.com/ethereum-optimism/op-geth/blob/647c346e2bef36219cc7b47d76b1cb87e7ca29e4/core/types/rollup_cost.go#L411>
mattsse marked this conversation as resolved.
Show resolved Hide resolved
pub(crate) fn flz_compress_len(input: &[u8]) -> u32 {
let mut idx: u32 = 2;

Expand Down
24 changes: 18 additions & 6 deletions crates/optimism/src/l1block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ pub const BASE_FEE_RECIPIENT: Address = address!("420000000000000000000000000000
/// The address of the L1Block contract.
pub const L1_BLOCK_CONTRACT: Address = address!("4200000000000000000000000000000000000015");

/// <https://github.com/ethereum-optimism/op-geth/blob/647c346e2bef36219cc7b47d76b1cb87e7ca29e4/core/types/rollup_cost.go#L79>
const L1_COST_FASTLZ_COEF: u64 = 836_500;

/// <https://github.com/ethereum-optimism/op-geth/blob/647c346e2bef36219cc7b47d76b1cb87e7ca29e4/core/types/rollup_cost.go#L78>
/// Inverted to be used with `saturating_sub`.
const L1_COST_INTERCEPT: u64 = 42_585_600;

/// <https://github.com/ethereum-optimism/op-geth/blob/647c346e2bef36219cc7b47d76b1cb87e7ca29e4/core/types/rollup_cost.go#82>
const MIN_TX_SIZE_SCALED: u64 = 100 * 1_000_000;

/// L1 block info
///
/// We can extract L1 epoch data from each L2 block, by looking at the `setL1BlockValues`
Expand Down Expand Up @@ -161,12 +171,14 @@ impl L1BlockInfo {
// This value is computed based on the following formula:
// max(minTransactionSize, intercept + fastlzCoef*fastlzSize)
fn tx_estimated_size_fjord(&self, input: &[u8]) -> U256 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we change this to u64?

looks like we use this in other places where we can also operate on u64

Copy link
Contributor Author

@hai-rise hai-rise Nov 19, 2024

Choose a reason for hiding this comment

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

The call in calculate_tx_l1_cost_fjord multiples with a large l1_fee_scaled so U256 is needed.

The call in data_gas looks pretty safe, but is not in any hot path as data_gas is only called in calculate_tx_l1_cost_bedrock and calculate_tx_l1_cost_ecotone that don't pass in a FJORD-enabled spec.

I'd say it's not worth it as it forces callers to mentally track u64::MAX to not wrap in a U256 anyway. It's easier within tx_estimated_size_fjord as we know even a u32::MAX returned by flz_compress_len wouldn't overflow u64.

let fastlz_size = U256::from(flz_compress_len(input));

fastlz_size
.saturating_mul(U256::from(836_500))
.saturating_sub(U256::from(42_585_600))
.max(U256::from(100_000_000))
let fastlz_size = flz_compress_len(input) as u64;

U256::from(
fastlz_size
.saturating_mul(L1_COST_FASTLZ_COEF)
.saturating_sub(L1_COST_INTERCEPT)
.max(MIN_TX_SIZE_SCALED),
)
}

/// Calculate the gas cost of a transaction based on L1 block data posted on L2, depending on the [OptimismSpecId] passed.
Expand Down
Loading