supervisor and testing: interop: reorg-testing: reorg a chain with invalid exec msg

[nonsense]

Description

This PR is adding 3 changes - reorg tests, happy interop init/exec msg test and modification to DecodeExecutingMessageLog behavior.

1. Reorg test

Adding tests for the reorg functionality of the supervisor when it detects an invalid exec msg:

1. We make an unsafe block that includes an invalid exec msg on chain A (the identifier of the exec msg points to unknown chain)
2. We wait for the supervisor to trigger a reorg on the executing side (chain A), without change to the initiating side (chain B).

This implies the sequencer is malicious. To cover this scenario, we use the op-test-sequencer to force in the bad tx.

2. Modification to DecodeExecutingMessageLog behavior

DepSet now includes a reserved ChainIndex for an unknown chain. We make sure that no chain is using that index when hydrating/initialising the set from config.

Conversion of ChainID to ChainIndex within DecodeExecutingMessageLog remains, during parsing of ExecutingMessage (EM) but if we see an unknown chain, we convert ChainID to the reserved chain index for unknown chain, essentially losing the value within our internal database. Later that EM is discarded the same way we discard other EMs for other validation errors.

This way the supervisor correctly triggers a reorg of the chain, and issues a similar error as other validation problems (such as invalid block number, or invalid log index, etc.)

3. Happy-path interop init/exec msg test

Confirming the changes in behavior in point 2. are not affecting correctly formatted EMs

Metadata

Fixes: #15500

[wiz-inc-a178a98b5d]

Wiz Scan Summary

Scanner	Findings
Vulnerabilities
Sensitive Data
Secrets
IaC Misconfigurations
Total

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (83b7e9a) to head (298445e).
Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop   #15662       +/-   ##
============================================
- Coverage    81.40%        0   -81.41%     
============================================
  Files          161        0      -161     
  Lines         8812        0     -8812     
============================================
- Hits          7173        0     -7173     
+ Misses        1506        0     -1506     
+ Partials       133        0      -133     

Flag	Coverage Δ
cannon-go-tests-64	?
contracts-bedrock-tests	?

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

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

[tynes]

An identifier that has a chain id not in the dep set should 100% trigger a reorg, it is a bug if it does not

[nonsense]

@tynes thanks for the clarification, I will investigate further why no reorg is triggered and see how to fix this!

For reference, the relevant part of the code is: https://github.com/ethereum-optimism/optimism/pull/15662/files#diff-9a44438696e317ca53bc4a82b8bfc9fc1b6de6da787d3230aa4464ebf3ca79e8R157-R160

[axelKingsley]

Thank you for diving into this! I don't think we want to handle this in the Chain Processor, but rather the indexer should be permissive of adding Executing Messages with bad Chain IDs (it errors currently). It should allow those messages in, and then rely on the cross package to fail the interop invariants and issue invalidation/replacement like it does in other cases.

[nonsense]

Following the merge of the parent PR, rebased this one onto develop and its ready for review / merge.

[protolambda]

Good to see new testing, but I think this could really use some DSL for a more maintainable/readable test. I'm open to merging it like this, if you can create a github issue to track the needed DSL.

[nonsense]

Good to see new testing, but I think this could really use some DSL for a more maintainable/readable test. I'm open to merging it like this, if you can create a github issue to track the needed DSL.

Agree that tests can be more readable, but I'd rather we have a few more tests, before introducing more DSL, so that we avoid having DSL that is used once, and only introduces indirection.

Follow up issue: #15843

[ajsutton]

Agree that tests can be more readable, but I'd rather we have a few more tests, before introducing more DSL, so that we avoid having DSL that is used once, and only introduces indirection.

I'd strongly suggest introducing DSL from the very start. Good DSL pays off even if it's only used once by making the test easier to read, and it's pretty rare for things to be refactored into a good DSL later - the verbose code just winds up being duplicated a lot (see the op-e2e tests...). It doesn't have to be complex DSL, but just enough to make the test high level and read more declaratively with the detail of how things actually work in the DSL.

[nonsense]

I added a follow up ticket with a few TODOs / improvements to all of the new tests - #15843. I'd rather these fixes happen outside of this PR, in order to keep the scope limited to the fix with the depset check within the supervisor.

fixed