Skip to content

op-e2e,interop: Test safe block replacement of cascading invalid blocks#14949

Merged
3 commits merged intodevelopfrom
ts/test-safe-block-replacement
Mar 21, 2025
Merged

op-e2e,interop: Test safe block replacement of cascading invalid blocks#14949
3 commits merged intodevelopfrom
ts/test-safe-block-replacement

Conversation

@ghost
Copy link

@ghost ghost commented Mar 19, 2025

This tests that if a safe block is cross-invalid because one of its dependencies is cross-invalid:

  1. Both blocks are replaced
  2. Both replacement blocks become cross-safe

@ghost ghost marked this pull request as ready for review March 19, 2025 16:55
@ghost ghost requested review from a team as code owners March 19, 2025 16:55
@ghost ghost requested a review from protolambda March 19, 2025 16:55
@codecov
Copy link

codecov bot commented Mar 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 42.37%. Comparing base (ef21b9b) to head (aa3dd3e).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14949      +/-   ##
===========================================
- Coverage    46.46%   42.37%   -4.10%     
===========================================
  Files         1149      977     -172     
  Lines        98730    88526   -10204     
===========================================
- Hits         45877    37512    -8365     
+ Misses       49563    47887    -1676     
+ Partials      3290     3127     -163     
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.

see 178 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.

Copy link
Contributor

@axelKingsley axelKingsley left a comment

Choose a reason for hiding this comment

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

Looks good, I follow along:

  • A1 exists and is fine
  • B1 exists and is fine
  • A2 makes a valid reference onto B2
  • B2 has that reference, but makes a different invalid reference

A2 and B2 are both invalid, and we can see they are replaced by the heads advancing, the block hashes changing, and the tx not being included.

If you wanted, you could stop the Sync calls part way through to confirm that one chain has done the replacement while the other hasn't yet. Not sure it's needed tho.

Should also get @Inphi 's opinion to make sure this test does what they originally meant.

Copy link
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.

Thank you so much for highlighting the issue with the test and fixing it.
As a bonus, I can now confirm that the fault proof program works in this case. I'll follow up on updating FP tests in another PR.

@Inphi
Copy link
Contributor

Inphi commented Mar 19, 2025

just one more thing, I wanna confirm that is indeed a non-issue in the flip case. Could you please add a similar test case where the invalid executing message is included on chain A rather than chain B?

@Inphi Inphi self-requested a review March 19, 2025 18:45
@ghost ghost force-pushed the ts/test-safe-block-replacement branch from b84c5da to aa3dd3e Compare March 21, 2025 06:15
@ghost ghost added this pull request to the merge queue Mar 21, 2025
Merged via the queue into develop with commit 6a436fe Mar 21, 2025
51 checks passed
@ghost ghost deleted the ts/test-safe-block-replacement branch March 21, 2025 07:40
This pull request was closed.
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