Skip to content

l2geth: verifier sync in parallel with dtl#2335

Closed
tonykogias wants to merge 1 commit intoethereum-optimism:developfrom
tonykogias:sync-l2geth-in-parallel-with-dtl
Closed

l2geth: verifier sync in parallel with dtl#2335
tonykogias wants to merge 1 commit intoethereum-optimism:developfrom
tonykogias:sync-l2geth-in-parallel-with-dtl

Conversation

@tonykogias
Copy link
Contributor

Have verifier node sync in parallel with dtl to save time.

Metadata

@changeset-bot
Copy link

changeset-bot bot commented Mar 17, 2022

🦋 Changeset detected

Latest commit: 311149b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/l2geth Patch

Not sure what this means? Click here to learn what changesets are.

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

@tynes
Copy link
Contributor

tynes commented Mar 17, 2022

Will need to run this to be sure that it works as expected

@mergify mergify bot requested a review from mslipper March 31, 2022 19:02
@mslipper mslipper added the C-protocol-critical Category: Modifies protocol-critical code label Apr 1, 2022
@mergify mergify bot requested a review from tuxcanfly April 4, 2022 18:46
@github-actions
Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Apr 19, 2022
Copy link
Contributor

@Inphi Inphi left a comment

Choose a reason for hiding this comment

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

LGTM!

@Inphi Inphi removed the Stale label Apr 19, 2022
if !status.Syncing {
tStatus.Stop()
break
if !cfg.IsVerifier {
Copy link
Contributor

Choose a reason for hiding this comment

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

This will only run for the sequencer which I'm not sure is solving the problem. It's the replicas that we want to speed up, I think it should be safe to skip waiting if it's a verifier and backend L2. We would need to test running a verifier with backend L1 before I felt safe merging it

@smartcontracts
Copy link
Contributor

Gonna reopen this, I think it'd be useful and it's annoying that geth currently doesn't sync at the same time as the DTL.

@smartcontracts
Copy link
Contributor

Just needs a rebase, then I can go ahead and cut a canary release + test this on my own

@smartcontracts
Copy link
Contributor

Am currently syncing this on an L1 syncing node

@tonykogias
Copy link
Contributor Author

I close this PR and re-opened it here #2656 after rebasing @tynes @smartcontracts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cannon Area: cannon C-protocol-critical Category: Modifies protocol-critical code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Have L2Geth sync in parallel with the DTL

5 participants