fix(gas-oracle): handle Optimism Ecotone blob fees #352
Conversation
|
Thanks for this. In the code comments where the gas price calculations are implemented, please add relevant links to the OpStack repo. |
tesseract/evm/src/gas_oracle.rs
Outdated
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_optimism_total_gas_calculation() { |
There was a problem hiding this comment.
It would be much better to test this using OpSepolia directly
There was a problem hiding this comment.
Hi @Wizdave97,
I've added the requested comments and references in the code, including links to the OpStack repository for additional context. Thanks again for the feedback—it's much appreciated!
Regarding testing against OpSepolia, I've currently limited the tests to unit-level logic simulations. Due to time constraints and other pending PRs I need to address, I haven't yet integrated direct end-to-end testing on OpSepolia. However, I definitely see value in adding those tests and will plan to revisit this when I have more bandwidth. It would be great to ensure full coverage and validate the changes in a live environment as well.
Will try to do this asap
There was a problem hiding this comment.
Hi @Wizdave97, I've now integrated direct testing against OpSepolia as requested. The tests run successfully against the OpSepolia endpoint, verifying both the L2 data cost calculations and the gas estimation logic in a live environment. Additionally, I've refined the code comments and references to the OpStack/Ecotone specifications for clarity. Let me know if there's anything else you'd like addressed!
sylvaincormier
force-pushed
the
fix/optimism-gas-oracle
branch
from
f4ad1d1 to
f6827d0
Compare
- Add OptimismGasComponents struct to track L1/L2 fees - Implement proper L1 data posting cost calculation - Consider both base fee and blob fee with scalars - Fix gas price overestimation issue Also: - Added inline code comments and OpStack references as requested by maintainers - Implemented direct testing against OpSepolia network
sylvaincormier
force-pushed
the
fix/optimism-gas-oracle
branch
from
f6827d0 to
dbbc135
Compare
tesseract/evm/src/gas_oracle.rs
Outdated
| id if is_op_stack(id) => { | ||
| let ovm_gas_price_oracle = OVM_gasPriceOracle::new(H160(OP_GAS_ORACLE), client.clone()); | ||
|
|
||
| // Get L1 gas used first |
There was a problem hiding this comment.
This logic is already encapsulated by the get_l1_fee method, so we should just keep that
https://github.com/ethereum-optimism/optimism/blob/3db0492cb4ad37441192f3bc94259cc9df9bb4fc/packages/contracts-bedrock/src/L2/GasPriceOracle.sol#L57
I would suggest investigating if we are using the resulting values correctly, look into the optimism javascript sdk to see how the gas cost is estimated and replicate that here.
There was a problem hiding this comment.
There was a problem hiding this comment.
If the implementations are identical you can report back.
There was a problem hiding this comment.
Hi @Wizdave97, thanks for pointing this out! I've removed the manual l1_gas_used calculation and now rely directly on get_l1_fee as per the contract implementation. Additionally, I've compared the updated logic with the Optimism SDK, and it aligns with their approach. Let me know if there's anything else you'd like adjusted.
- Dynamically calculate L1 fee based on transaction size - Fix incorrect handling of L1 gas used scaling - Add detailed logging for debug purposes
|
Closing because the current implementation of the gas estimate is identical to what's available in the optimism-sdk Tesseract estimates are approximately correct as shown from the samples in the doc The issue was fixed by this PR #337 |
Overview
Fixes gas price overestimation issue in the Optimism gas oracle following the Ecotone upgrade which introduced blob fees.
Changes
Testing
Impact
This fix ensures accurate gas estimations for Optimism transactions by properly accounting for:
Fixes #336