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

Add pivot updator for optimism #7586

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

marcindsobczak
Copy link
Contributor

@marcindsobczak marcindsobczak commented Oct 11, 2024

Changes

  • adding UnsafePivotUpdator which is working similar as general PivotUpdator except using head block hash (minus 64 blocks) instead of finalized block hash.

In PivotUpdator we are using finalized block hash, because it can't be reorganized so is a safe starting point. Optimism is not providing finalized block hash until fully synced, so we need to use unsafe head block hash, which potentially can be reorganized and break the node. For more safety we are using head block minus 64. On Optimism reorganizations aren't common, so it can be a justified risk to significantly improve UX in most cases

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

@LukaszRozmej
Copy link
Member

Wouldn't it be safer to get Head - N blocks? (Whatever that N would be)

@marcindsobczak
Copy link
Contributor Author

@LukaszRozmej it would be safer, but then we need to refactor and complicate the logic way more

Right now we are getting block hash and we are asking peers for a block by hash. Then we have block number and that's it.

If we want to have head - N, then we need to ask peers again about block head - N to get it's hash

@marcindsobczak
Copy link
Contributor Author

Ok after discussion we need to use head - N, so working on it

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.

2 participants