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: attributes-handler with events #10947

Merged
merged 2 commits into from
Jun 23, 2024
Merged

op-node: attributes-handler with events #10947

merged 2 commits into from
Jun 23, 2024

Conversation

protolambda
Copy link
Contributor

@protolambda protolambda commented Jun 19, 2024

Description

One more step towards separating every components with events.

This turns the AttributesHandler into a Deriver, processing and emitting events to interact with the other things around it.

Depends on #10971

Tests

  • Attributes handler unit tests are cleaner now, abstract away engine-controller internals
  • Updated some op-e2e tests to deal with the move away from immediate attributes-processing coupled to pipeline-steps.
    After we have events, there's no single synchronous notion of a derivation-step anymore. Thus in the e2e action tests I instead fixed up tests to run the derivation till a certain event, rather than an arbitrary step that leaks internal behavior of the derivation progression.

Metadata

Fix #10852

@protolambda protolambda added the M-do-not-merge Meta: Do not merge label Jun 19, 2024
Copy link
Contributor

semgrep-app bot commented Jun 19, 2024

Semgrep found 1 sol-style-return-arg-fmt finding:

  • packages/contracts-bedrock/src/cannon/libraries/MIPSInstructions.sol

Named return arguments to functions must be appended with an underscore (_)

Ignore this finding from sol-style-return-arg-fmt.

Semgrep found 1 sol-style-malformed-revert finding:

  • packages/contracts-bedrock/src/cannon/MIPS.sol

Malformed revert statement style.

Ignore this finding from sol-style-malformed-revert.

Semgrep found 1 sol-style-input-arg-fmt finding:

  • packages/contracts-bedrock/src/cannon/libraries/MIPSInstructions.sol

Inputs to functions must be prepended with an underscore (_)

Ignore this finding from sol-style-input-arg-fmt.

Base automatically changed from cl-sync-events to develop June 19, 2024 20:25
@protolambda protolambda changed the base branch from develop to derivation-pipeline-events June 20, 2024 06:10
@protolambda protolambda changed the title op-node: attributes-handler with events [work in progress draft] op-node: attributes-handler with events Jun 20, 2024
@protolambda protolambda marked this pull request as ready for review June 20, 2024 06:20
@protolambda protolambda requested review from a team and ajsutton as code owners June 20, 2024 06:20
@protolambda
Copy link
Contributor Author

Wait for dependency PR and plasma-test updates before merging

Base automatically changed from derivation-pipeline-events to develop June 20, 2024 06:29
@protolambda
Copy link
Contributor Author

Rebased on develop.

Also, looks like there's something wrong with op-program refactor, it's running out of events to process. Some state-change is not feeding back into the program for continuation. Will split it out of this PR and debug it tomorrow.

Copy link
Contributor

semgrep-app bot commented Jun 20, 2024

Semgrep found 1 sol-style-malformed-revert finding:

  • packages/contracts-bedrock/src/cannon/MIPS.sol

Malformed revert statement style.

Ignore this finding from sol-style-malformed-revert.

@protolambda protolambda force-pushed the attrib-events branch 2 times, most recently from 96fb214 to 03fcfc2 Compare June 21, 2024 23:23
Base automatically changed from op-program-events to develop June 21, 2024 23:48
@protolambda protolambda removed the M-do-not-merge Meta: Do not merge label Jun 23, 2024
@protolambda
Copy link
Contributor Author

Ready for review

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. Just a suggestion around the e2e tests.

op-e2e/actions/sync_test.go Show resolved Hide resolved
@protolambda protolambda added this pull request to the merge queue Jun 23, 2024
Merged via the queue into develop with commit e449dd2 Jun 23, 2024
58 checks passed
@protolambda protolambda deleted the attrib-events branch June 23, 2024 22:43
rdovgan pushed a commit to rdovgan/optimism that referenced this pull request Jun 24, 2024
* op-node: event handling on block attributes

todo

* op-node: update plasma step to no longer hardcode pipeline stepping
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.

Interop: Event-handling update of AttributesHandler
2 participants