Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

[IBFT] Block difficulty is one #217

Closed
wants to merge 10 commits into from
Closed

Conversation

dbrajovic
Copy link
Contributor

@dbrajovic dbrajovic commented Nov 18, 2021

Description

Originally starting as the idea in (now closed) PR#182, this PR is the third (and final) part of the series, addressing the block difficulty in IBFT. The first two being PR#201 and PR#208.

Essentially, it conforms to the standard described in the original IBFT consensus

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Breaking changes

The breaking change is introduced here.

A previously running node restarted with this build might still be able to produce blocks, but the danger of it failing to verify headers (during bulk sync) at some point is never absent.

Newly added nodes to the cluster will not be able to move past genesis and can never sync with the chain.

Checklist

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have tested this code
  • I have updated the README and other relevant documents (guides...)
  • I have added sufficient documentation both in code, as well as in the READMEs

@dbrajovic dbrajovic self-assigned this Nov 19, 2021
@dbrajovic dbrajovic marked this pull request as ready for review November 19, 2021 08:24
Copy link
Contributor

@zivkovicmilos zivkovicmilos 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 💯

I've left some minor comments.

Thank you for leaving the chronological description of the changes in the PR, now people can follow along 🙏

Copy link
Contributor

@Kourin1996 Kourin1996 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, apart from the reviews by Milos 👍

@lazartravica lazartravica added the don't merge Please don't merge this functionality temporarily label Nov 28, 2021
@mrwillis
Copy link
Contributor

mrwillis commented Dec 3, 2021

hi @dbrajovic @lazartravica, will this be added as a flag or something like --ibft-difficulty in develop given that it's breaking? Is there any actual benefits to using this other than conforming to the spec?

@dbrajovic
Copy link
Contributor Author

hi @dbrajovic @lazartravica, will this be added as a flag or something like --ibft-difficulty in develop given that it's breaking? Is there any actual benefits to using this other than conforming to the spec?

The original idea was just conforming to the spec - no additional benefits other than that. If this gets released after the point the chain cannot be reset for you, we'll probably include a forks-in-time support

@zivkovicmilos
Copy link
Contributor

Closing and archiving under archive/bug/block-difficulty-is-one:

https://github.com/0xPolygon/polygon-sdk/releases/tag/archive%2Fbug%2Fblock-difficulty-is-one

@zivkovicmilos zivkovicmilos deleted the bug/block-difficulty-is-one branch January 14, 2022 13:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
don't merge Please don't merge this functionality temporarily
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants