Skip to content

Conversation

@geoknee
Copy link
Contributor

@geoknee geoknee commented Jan 7, 2026

I expect this to fail, but it passes against ethereum-optimism/op-geth#732.

Merge plan:

  • initial review of this draft PR
  • merge Fix eth_simulateV1 after Jovian fork op-geth#732
  • push a commit to this PR updating the op-geth reference in go.mod (can't do this now easily, since the op-geth PR is from an external fork)
  • final review and merge this AT

@codecov
Copy link

codecov bot commented Jan 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.58%. Comparing base (e5f61e5) to head (109120e).
⚠️ Report is 5 commits behind head on develop.

❗ There is a different number of reports uploaded between BASE (e5f61e5) and HEAD (109120e). Click for more details.

HEAD has 8 uploads less than BASE
Flag BASE (e5f61e5) HEAD (109120e)
cannon-go-tests-64 3 1
contracts-bedrock-tests 6 0
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #18724      +/-   ##
===========================================
- Coverage    73.82%   66.58%   -7.25%     
===========================================
  Files          189       55     -134     
  Lines        11252     4031    -7221     
===========================================
- Hits          8307     2684    -5623     
+ Misses        2799     1203    -1596     
+ Partials       146      144       -2     
Flag Coverage Δ
cannon-go-tests-64 66.58% <ø> (-0.82%) ⬇️
contracts-bedrock-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.
see 139 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Add ReturnFullTransactions to simulation params and require the RPC call
to succeed. Assert exactly one block and one transaction are returned
and verify the transaction is a dynamic fee tx (type 0x2).
@geoknee geoknee requested a review from joshklop January 7, 2026 17:32
Copy link
Contributor

@joshklop joshklop left a comment

Choose a reason for hiding this comment

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

lgtm, passes for me locally. I think this threads the needle well: adding coverage where it's needed (ensuring eth_simulateV1 continues to function) without duplicating coverage already present in op-geth/op-reth.

Decode blobGasUsed with hexutil and assert it's nonzero to ensure
eth_simulateV1 can estimate DA size. Also fix a comment typo (bock ->
block).
Call eth_simulateV1 with block "0x0" (genesis) and require an error to
ensure the method cannot be used on the genesis block move test under
"base" directory
@geoknee geoknee marked this pull request as ready for review January 8, 2026 13:46
@geoknee geoknee requested a review from a team as a code owner January 8, 2026 13:46
@geoknee geoknee requested review from falcorocks and joshklop January 8, 2026 13:46
@geoknee geoknee changed the title op-acceptance-tests: Add EL eth_simulate acceptance test and init op-acceptance-tests: Add EL eth_simulate acceptance test Jan 8, 2026
)

replace github.com/ethereum/go-ethereum => github.com/ethereum-optimism/op-geth v1.101603.6-rc.1
replace github.com/ethereum/go-ethereum => github.com/ethereum-optimism/op-geth v1.101604.1-0.20260108133432-e4826126d221
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this make develop non-release ready?

Copy link
Contributor Author

@geoknee geoknee Jan 8, 2026

Choose a reason for hiding this comment

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

Let's keep this convo open and not merge the PR until we have an RC to point to.

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.

3 participants