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

Reject pre-pivot payloads #4216

Closed
wants to merge 3 commits into from
Closed

Conversation

marcindsobczak
Copy link
Contributor

Changes:

  • return syncing when received payload is pre-pivot
    This PR is fixing issue with not downloading old blocks if pivot is post-merge

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Testing

Requires testing

  • Yes
  • No

In case you checked yes, did you write tests??

  • Yes
  • No


if (block.Header.Number <= _syncConfig.PivotNumberParsed)
{
if (_logger.IsTrace) _logger.Trace($"Pre-pivot block, ignored and returned Syncing. Result of {requestStr}.");
Copy link
Contributor

Choose a reason for hiding this comment

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

hmmm maybe trace -> info ?

Copy link
Contributor

@MarekM25 MarekM25 Jun 28, 2022

Choose a reason for hiding this comment

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

@marcindsobczak it would be good to a simple unit test

@marcindsobczak marcindsobczak marked this pull request as ready for review June 29, 2022 11:51
@LukaszRozmej
Copy link
Member

Closing in favor of #4224

@rubo rubo deleted the merge/fix_old_bodies branch December 20, 2023 21:29
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

3 participants