Skip to content

Comments

Bedrock: share more test utils, cleanup derive package structure, move payload/attributes types to eth package#2761

Merged
mergify[bot] merged 6 commits intodevelopfrom
bedrock-test-utils
Jun 13, 2022
Merged

Bedrock: share more test utils, cleanup derive package structure, move payload/attributes types to eth package#2761
mergify[bot] merged 6 commits intodevelopfrom
bedrock-test-utils

Conversation

@protolambda
Copy link
Contributor

No changes or additions in functionality in this PR. These changes aim to clean up the structure/naming of the derive package in preparation of the upcoming batch derivation changes, and usage in Cannon.

Changes:

  • Move ExecutionPayload, PayloadAttributes, and related API response types to the eth package.
    • This enables us to use them in the test utils, and pull the test utils into every package (except package eth itself), without circular dependencies
    • Cannon can implement the Engine interface that the driver code uses, to substitute the external engine with an in-memory EVM (with pre-image-oracle based state db) to run the fraud proof with.
  • Split up payload_attributes.go: it didn't even contain the payload attributes type, and it was a mixed bag of things.
  • The GenerateDepositLog function is now MarshalDepositLogEvent, since it nicely mirrors UnmarshalDepositLogEvent (slight rename from previous UnmarshalLogEvent) and we need it to be exposed for testing in other packages. This can't be part of the test util package, since the test-util package does not pull in anything from derive (since it's used for testing derive itself)
  • Move random data generators from test files to test utils package
  • Move testID (to easily create eth.BlockID with in tests) to test utils package
  • Minor renames / typo fixes / related doc updates.

@changeset-bot
Copy link

changeset-bot bot commented Jun 10, 2022

⚠️ No Changeset found

Latest commit: edef482

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mergify
Copy link
Contributor

mergify bot commented Jun 10, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

@protolambda protolambda requested a review from norswap June 10, 2022 22:07
Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

Restructure looks good to me.
Only weird thing are some of the imports. I'm fine addressing them now w/out needing re-review or addressing them in a follow up PR.

Copy link
Contributor

@norswap norswap 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 to me, definitely a step in the right direction!

@mergify
Copy link
Contributor

mergify bot commented Jun 13, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented Jun 13, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot merged commit 98e83d6 into develop Jun 13, 2022
@mergify mergify bot deleted the bedrock-test-utils branch June 13, 2022 16:25
@mergify mergify bot removed the on-merge-train label Jun 13, 2022
maurelian pushed a commit that referenced this pull request Jun 13, 2022
…e payload/attributes types to eth package (#2761)

* feat(bedrock): move ExecutionPayload, PayloadAttributes and other engine types from l2 to eth package

* chore(bedrock): clean up derive package, move test util out or polish for public use, split payload_attributes and improve file names

* chore(bedrock): move more test utils out of derive and driver packages into shared package

* bedrock: fix go import newlines/order

* testutils: describe TestID usage

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
maurelian pushed a commit that referenced this pull request Jun 14, 2022
…e payload/attributes types to eth package (#2761)

* feat(bedrock): move ExecutionPayload, PayloadAttributes and other engine types from l2 to eth package

* chore(bedrock): clean up derive package, move test util out or polish for public use, split payload_attributes and improve file names

* chore(bedrock): move more test utils out of derive and driver packages into shared package

* bedrock: fix go import newlines/order

* testutils: describe TestID usage

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@mslipper mslipper mentioned this pull request Jun 18, 2022
theochap added a commit that referenced this pull request Dec 10, 2025
…t rpc call (#2761)

## Description

It seems the `opp2p_disconnect` rpc call returns too early. This method
ensures that we're checking the peers are effectively not connected to
each other in the `opp2p_peers` call.
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