Skip to content

fix(op-revm): return error when enveloped_tx is missing#3143

Merged
rakita merged 3 commits intobluealloy:mainfrom
Keemosty12:enveloped
Nov 13, 2025
Merged

fix(op-revm): return error when enveloped_tx is missing#3143
rakita merged 3 commits intobluealloy:mainfrom
Keemosty12:enveloped

Conversation

@Keemosty12
Copy link
Contributor

Fixes a missing case from #3055 where tx_cost_with_tx in l1block.rs:272 still uses .expect() with the same enveloped_tx() call.

/// Internally calls [`L1BlockInfo::tx_cost`].
#[track_caller]
pub fn tx_cost_with_tx(&mut self, tx: impl OpTxTr, spec: OpSpecId) -> U256 {
pub fn tx_cost_with_tx(&mut self, tx: impl OpTxTr, spec: OpSpecId) -> Result<U256, String> {
Copy link
Member

Choose a reason for hiding this comment

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

Better to return Option here, and leave for caller to create the error message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @rakita, updated, pls check, let me know if there's anything else needs tweaking. Appreciate your time.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 13, 2025

CodSpeed Performance Report

Merging #3143 will not alter performance

Comparing Keemosty12:enveloped (319fc71) with main (7299531)

Summary

✅ 173 untouched

.enveloped_tx()
.expect("all not deposit tx have enveloped tx")
.clone();
let enveloped_tx = tx.enveloped_tx().cloned()?;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let enveloped_tx = tx.enveloped_tx().cloned()?;
let enveloped_tx = tx.enveloped_tx()?;

probably dont need clone here, not sure why was cloned in first place

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, removed, thanks!

Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

lgtm

@rakita rakita merged commit 84d41bc into bluealloy:main Nov 13, 2025
30 checks passed
This was referenced Nov 13, 2025
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 21, 2026
…vm#3143)

* fix(op-revm): return error when enveloped_tx is missing

* fmt

* Remove unnecessary clone
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants