Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

op-node: Separate attributes processing from EngineQueue #10642

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

protolambda
Copy link
Contributor

@protolambda protolambda commented May 24, 2024

Description

This separate the attributes-processing (safe head consolidation / force-insertion) from the EngineQueue.

This PR depends on #10599, and is the next sub-step of 1a in the derivation refactor (see design doc) to remove the EngineQueue and make the derivation-pipeline return attributes, rather than tightly coupled to the execution engine.

Since attributes-matching is coupled to the attributes-handling, I moved this out of derive and into the same new attributes package, with some slight improvements.

The EngineQueue still does a L1-origin check for the derivation traversal, which I'll pull into its own isolated function in the next PR, and then we can finally remove the EngineQueue.

Also moves the sync-progress logging to the engine-controller, so it doesn't get duplicated by all the different engine-bindings callers.

Tests

The EngineQueue was missing tests for safe-head progression/consolidation. Now that this functionality has been encapsulated, the AttributesHandler covers the different ways attributes can be processed.

Metadata

Fix https://github.com/ethereum-optimism/protocol-quest/issues/271

added the do-not-merge label, so the base PRs can be merged first, to keep the diffs sane

@protolambda protolambda added the M-do-not-merge Meta: Do not merge label May 24, 2024
@protolambda protolambda self-assigned this May 24, 2024
Copy link
Contributor

semgrep-app bot commented May 24, 2024

Semgrep found 2 golang_fmt_errorf_no_params findings:

No fmt.Errorf invocations without fmt arguments allowed

Ignore this finding from golang_fmt_errorf_no_params.

Semgrep found 2 invalid-usage-of-modified-variable findings:

Variable unsafeInNode is likely modified and later used on error. In some cases this could result in panics due to a nil dereference

Ignore this finding from invalid-usage-of-modified-variable.

Semgrep found 1 math-random-used finding:

  • op-node/rollup/clsync/clsync_test.go

Do not use math/rand. Use crypto/rand instead.

Ignore this finding from math-random-used.

Semgrep found 1 todos_require_linear finding:

  • packages/core-utils/src/common/bn.ts

Please create a GitHub ticket for this TODO.

Ignore this finding from todos_require_linear.

Copy link
Contributor

@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.

LGTM.

@protolambda
Copy link
Contributor Author

Rebased on unsafe-blocks-refactor, which was rebased on new finality PR, which was rebased on develop.

Base automatically changed from unsafe-blocks-refactor to develop June 3, 2024 17:24
@protolambda
Copy link
Contributor Author

rebased on develop, dependency PR was merged

@protolambda protolambda removed the M-do-not-merge Meta: Do not merge label Jun 3, 2024
@protolambda protolambda added this pull request to the merge queue Jun 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 3, 2024
@protolambda protolambda added this pull request to the merge queue Jun 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 3, 2024
@protolambda
Copy link
Contributor Author

Rebased again, hoping for less flakes in CI, although won't affect merge queue

@protolambda protolambda added this pull request to the merge queue Jun 4, 2024
Merged via the queue into develop with commit 1eda12b Jun 4, 2024
64 checks passed
@protolambda protolambda deleted the attributes-handler branch June 4, 2024 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants