-
Notifications
You must be signed in to change notification settings - Fork 879
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
Qbft migration protocol schedule #3069
Qbft migration protocol schedule #3069
Conversation
Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>
… capabilities. Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>
Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>
Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>
Signed-off-by: Jason Frame <jasonwframe@gmail.com>
besu/src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
...rc/test/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilderTest.java
Outdated
Show resolved
Hide resolved
besu/src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Jason Frame <jasonwframe@gmail.com>
Signed-off-by: Jason Frame <jasonwframe@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have way through looking at it, so just adding a comment for now
besu/src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java
Outdated
Show resolved
Hide resolved
besu/src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One suggestion would be to move the logic inside createProtocolSchedule()
into a separate class (CombinedProtocolSchedule
or something else). This can make it easier to test and also make the code in ConsensusScheduleBesuControllerBuilder
easier to understand.
Signed-off-by: Jason Frame <jasonwframe@gmail.com>
…lls combined protocol schedule factory Signed-off-by: Jason Frame <jasonwframe@gmail.com>
Signed-off-by: Jason Frame <jasonwframe@gmail.com>
Signed-off-by: Jason Frame <jasonwframe@gmail.com>
Signed-off-by: Jason Frame <jasonwframe@gmail.com>
…besu into qbft_migration_protocol_schedule
Kudos, SonarCloud Quality Gate passed! |
Signed-off-by: Jason Frame <jasonwframe@gmail.com>
PR description
Create a combined protocol schedule for the QBFT migration that combines all the milestones from the delegate besu controllers protocol schedules.
fixes #2998
Fixed Issue(s)
Changelog