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

IBFT 2.0 #650

Merged
merged 160 commits into from
Aug 3, 2022
Merged

IBFT 2.0 #650

merged 160 commits into from
Aug 3, 2022

Conversation

dbrajovic
Copy link
Contributor

@dbrajovic dbrajovic commented Jul 22, 2022

Description

This PR provides the integration logic with the go-ibft package, and introduces IBFT 2.0 into Polygon Edge.

Changes include

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

Breaking changes

The IBFT 2.0 upgrade introduces liveness fixes, which are by default breaking changes, as it alters the way messages are constructed.

This breaking change is breaking in a sense that an entire cluster running on pre IBFT 2.0 needs to be restarted to run on post IBFT 2.0 (all nodes in the cluster need to be running the latest version). The blockchain history remains intact.

This means that nodes running IBFT 2.0 version of Edge will not work with nodes not running IBFT 2.0 version of Edge (older versions) - all nodes in the cluster need to run IBFT 2.0 of Edge in order to work.

Checklist

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have added the relevant labels
  • I have updated the official documentation
  • I have added sufficient documentation in code

Testing

  • I have tested this code with the official test suite
  • I have tested this code manually

Manual tests

  • Loadbot testing
  • Manual voting / unvoting
  • Time period tests

Additional comments

Fixes EDGE-614

Kourin1996 and others added 30 commits June 15, 2022 08:56
* implement watchSync

* Fix some linter errors and resolve conflicts

* Rename newStatus channel

* wait for a new status on end watch sync

* Add guard to check ih bestPeer is not nil

* Rename isValidator to match module encapsulation

* Call watchSync on every newStatus

* Close stream after featching blocks

* Fix linter issues
* implement watchSync

* Fix some linter errors and resolve conflicts

* Fix current failed tests

* Add mocks for sycer tests

* Add syncer unit tests

* Add PeerMap unit test

* Add SyncPeerService unit test

* Fix mockBlockchain in syncer

* Add bufcon in vendor

* Add client unittests in syncer

* Rename newStatus channel

* wait for a new status on end watch sync

* Add guard to check ih bestPeer is not nil

* Rename isValidator to match module encapsulation

* Call watchSync on every newStatus

* Add TestPeerConnectionUpdateEventCh

* Add unit tests for GetBlocks of SyncPeerClient

* Fix failed test

* Fix lint error

* Close stream after featching blocks

* Fix linter issues

* Fix failed tests

Co-authored-by: AleksaOpacic <[email protected]>
@zivkovicmilos zivkovicmilos marked this pull request as ready for review July 27, 2022 12:30
@codecov
Copy link

codecov bot commented Jul 27, 2022

Codecov Report

Merging #650 (6e4c0a9) into develop (31f8abc) will decrease coverage by 2.62%.
The diff coverage is 9.83%.

@@             Coverage Diff             @@
##           develop     #650      +/-   ##
===========================================
- Coverage    51.27%   48.65%   -2.63%     
===========================================
  Files          109      112       +3     
  Lines        15825    15364     -461     
===========================================
- Hits          8115     7476     -639     
- Misses        7028     7254     +226     
+ Partials       682      634      -48     
Impacted Files Coverage Δ
archive/restore.go 65.59% <0.00%> (ø)
blockchain/blockchain.go 51.62% <0.00%> (ø)
consensus/ibft/consensus.go 0.00% <0.00%> (ø)
consensus/ibft/consensus_backend.go 0.00% <0.00%> (ø)
consensus/ibft/hooks.go 23.68% <ø> (-31.58%) ⬇️
consensus/ibft/messages.go 0.00% <0.00%> (ø)
consensus/ibft/operator_service.go 54.80% <0.00%> (-1.08%) ⬇️
consensus/ibft/pos.go 0.00% <0.00%> (ø)
consensus/ibft/state.go 69.56% <ø> (-8.15%) ⬇️
consensus/ibft/transport.go 0.00% <0.00%> (ø)
... and 21 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

consensus/dev/dev.go Outdated Show resolved Hide resolved
archive/restore.go Outdated Show resolved Hide resolved
consensus/ibft/consensus_backend.go Show resolved Hide resolved
consensus/ibft/consensus_backend.go Show resolved Hide resolved
syncer/syncer.go Outdated Show resolved Hide resolved
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. It has cleaned up IBFT completely

Copy link
Contributor

@0xAleksaOpacic 0xAleksaOpacic left a comment

Choose a reason for hiding this comment

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

LGTM, Let's merge this :D

@zivkovicmilos zivkovicmilos merged commit 39a6abd into develop Aug 3, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Aug 3, 2022
@zivkovicmilos zivkovicmilos deleted the feature/backend-m2 branch August 4, 2022 08:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking change Functionality that contains breaking changes feature New update to Polygon Edge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants