Skip to content

op-e2e: Refactor GeneratedTransaction to avoid tests needing to calculate identifiers and payloads.#14447

Merged
ajsutton merged 5 commits intodevelopfrom
aj/better-tx-refs
Feb 21, 2025
Merged

op-e2e: Refactor GeneratedTransaction to avoid tests needing to calculate identifiers and payloads.#14447
ajsutton merged 5 commits intodevelopfrom
aj/better-tx-refs

Conversation

@ajsutton
Copy link
Copy Markdown
Contributor

Description

Refactors the interop DSL so that transaction creators return a GeneratedTransaction and it can capture its receipt when it is included in a block. This allows it to calculate the identifier and payload even before the block it is included in is actually sealed.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 19, 2025

Codecov Report

Attention: Patch coverage is 8.33333% with 11 lines in your changes missing coverage. Please review.

Project coverage is 41.87%. Comparing base (0e4b867) to head (74ece5c).
Report is 10 commits behind head on develop.

Files with missing lines Patch % Lines
op-program/client/l2/engineapi/l2_engine_api.go 0.00% 8 Missing ⚠️
op-program/client/l2/engineapi/block_processor.go 0.00% 3 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (0e4b867) and HEAD (74ece5c). Click for more details.

HEAD has 14 uploads less than BASE
Flag BASE (0e4b867) HEAD (74ece5c)
2 1
cannon-go-tests-32 6 0
cannon-go-tests-64 6 0
contracts-bedrock-tests 1 0
Additional details and impacted files
@@             Coverage Diff              @@
##           develop   #14447       +/-   ##
============================================
- Coverage    78.11%   41.87%   -36.25%     
============================================
  Files          178      847      +669     
  Lines        10667    77868    +67201     
============================================
+ Hits          8333    32605    +24272     
- Misses        2143    42395    +40252     
- Partials       191     2868     +2677     
Flag Coverage Δ
cannon-go-tests-32 ?
cannon-go-tests-64 ?
contracts-bedrock-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
op-program/client/l2/engine_backend.go 78.16% <100.00%> (+78.16%) ⬆️
op-program/client/l2/engineapi/block_processor.go 56.14% <0.00%> (ø)
op-program/client/l2/engineapi/l2_engine_api.go 32.43% <0.00%> (ø)

... and 1014 files with indirect coverage changes

@ajsutton ajsutton requested a review from Inphi February 20, 2025 02:42
Copy link
Copy Markdown
Contributor

@Inphi Inphi left a comment

Choose a reason for hiding this comment

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

When I remove the skip, the cross safety checks at proofs_test.go L406 as now passing. Which suggests that the cyclic dependencies are now considered as valid. There hasn't been any recent changes to the supervisor so I think the test is broken somewhere.

@ajsutton
Copy link
Copy Markdown
Contributor Author

Well crap - what did I change last minute that broke that again... it was failing like I expected. :) Good spot thank you.

Copy link
Copy Markdown
Contributor Author

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

Fix the problem and it's now failing to advance the cross unsafe head as it does on develop.

@ajsutton ajsutton enabled auto-merge February 21, 2025 03:02
@ajsutton ajsutton added this pull request to the merge queue Feb 21, 2025
Merged via the queue into develop with commit 6934b39 Feb 21, 2025
46 checks passed
@ajsutton ajsutton deleted the aj/better-tx-refs branch February 21, 2025 03:26
Rjected pushed a commit to paradigmxyz/optimism that referenced this pull request Feb 25, 2025
…late identifiers and payloads. (ethereum-optimism#14447)

* op-e2e: Refactor GeneratedTransaction to avoid tests needing to calculate identifiers and payloads.

* op-e2e: Use a single method for inbox executing transactions.

* op-e2e: Include transaction on correct chains.

* Simplify further.

* Skip test again.
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