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-e2e: only add a signer key to l1 #6291

Merged

Conversation

jyellick
Copy link
Contributor

@jyellick jyellick commented Jul 14, 2023

The current code adds the same signing key to all Ethereum clients (including the L2 nodes). I found this to be quite confusing when parsing the code, and, it's unnecessary for the L2s to have signing keys.

cc @ajsutton

@jyellick jyellick requested a review from a team as a code owner July 14, 2023 14:09
@changeset-bot
Copy link

changeset-bot bot commented Jul 14, 2023

⚠️ No Changeset found

Latest commit: f3ed61e

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

@netlify
Copy link

netlify bot commented Jul 14, 2023

Deploy Preview for opstack-docs canceled.

Name Link
🔨 Latest commit f3ed61e
🔍 Latest deploy log https://app.netlify.com/sites/opstack-docs/deploys/64b541734c2337000872d47b

@ajsutton
Copy link
Contributor

@jyellick Looks like CI is having a hard time. I don't really understand why yarn doesn't exist but there's been a bunch of changes and fixes around pnpm. Could you try rebasing this on top of latest develop and see if that helps please?

@ajsutton ajsutton self-requested a review July 17, 2023 01:47
The current code adds the same signing key to all Ethereum clients
(including the L2 nodes).  I found this to be quite confusing when
parsing the code, and, it's unnecessary for the L2s to have signing
keys.
@OptimismBot OptimismBot merged commit b707310 into ethereum-optimism:develop Jul 17, 2023
73 of 82 checks passed
@mergify
Copy link
Contributor

mergify bot commented Jul 17, 2023

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

@mergify
Copy link
Contributor

mergify bot commented Jul 17, 2023

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

@mergify mergify bot removed the on-merge-train label Jul 17, 2023
@jyellick jyellick deleted the op-e2e-only-add-key-for-l1 branch July 17, 2023 13:56
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.

None yet

4 participants