QBFT migration: Create migrating mining coordinator #3097
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hyperledger#2999 Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
…ordinator Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
…ct instance the ControllerBuilder has previous built that we're looking for. Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
siladu
commented
Nov 23, 2021
consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftForksSchedule.java
Outdated
Show resolved
Hide resolved
siladu
commented
Nov 26, 2021
| import org.apache.logging.log4j.Logger; | ||
| import org.apache.tuweni.bytes.Bytes; | ||
|
|
||
| public class SchedulableMiningCoordinator implements MiningCoordinator, BlockAddedObserver { |
There was a problem hiding this comment.
I need to think of a better name...
There was a problem hiding this comment.
Options...
MigratingMiningCoordinator
SwitchableMiningCoordinator
ConsensusScheduleMiningCoordinator
ForksScheduleMiningCoordinator (trying to avoid Fork but maybe ForksSchedule should be renamed too?)
There was a problem hiding this comment.
Perhaps MigratingMiningCoordinator as that's what it's used for. But otherwise don't have any good suggestions
jframe
reviewed
Nov 26, 2021
besu/src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java
Outdated
Show resolved
Hide resolved
consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftForksSchedule.java
Outdated
Show resolved
Hide resolved
...va/org/hyperledger/besu/consensus/common/bft/blockcreation/SchedulableMiningCoordinator.java
Outdated
Show resolved
Hide resolved
...va/org/hyperledger/besu/consensus/common/bft/blockcreation/SchedulableMiningCoordinator.java
Outdated
Show resolved
Hide resolved
...va/org/hyperledger/besu/consensus/common/bft/blockcreation/SchedulableMiningCoordinator.java
Outdated
Show resolved
Hide resolved
.../main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinator.java
Outdated
Show resolved
Hide resolved
…ordinator Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
This prevents local testing of the migration, but ensures there are no other regressions. Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
jframe
approved these changes
Nov 30, 2021
jframe
reviewed
Nov 30, 2021
besu/src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java
Show resolved
Hide resolved
…ordinator Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
…ordinator Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
1 task
…ordinator Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
|
Kudos, SonarCloud Quality Gate passed! |
siladu
changed the title
garyschulte
added a commit
that referenced
this pull request
Dec 14, 2021
* add more logging for get blocks form peers task (#3047) * add more logging for get blocks form peers task Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net> * (internal) Refactor PrivacyReorgTest to use mock enclave (#3103) * init Signed-off-by: Frank Li <b439988l@gmail.com> * working test Signed-off-by: Frank Li <b439988l@gmail.com> * WIP | 3/4 tests working Signed-off-by: Frank Li <b439988l@gmail.com> * use constant rather than random (#3116) Signed-off-by: Frank Li <b439988l@gmail.com> * Updating gradle to version 7.3 (#3109) check the [release notes](https://docs.gradle.org/7.3/release-notes.html) to see what is new (jdk17 support among other things). Signed-off-by: Jiri Peinlich <jiri.peinlich@gmail.com> Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net> * Include invalid value in Enode error message (#3112) * Include invalid value in Enode error message. Makes it much easier for users to identify their mistake if the CLI argument after `--bootnodes <some-valid-enode>` has a typo. PicoCLI will treat any unrecognised option as another bootnode so it's very confusing to get an invalid enode message when the enode is fine and it's the next argument that's incorrect. Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net> Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net> * Reduce log level down to debug on EthPeer Permissioning (#3114) Signed-off-by: Antony Denyer <git@antonydenyer.co.uk> * Rename BftForksSchedule to ForksSchedule and prepare for some more generic use cases (#3108) * Rename BftForksSchedule to ForkSchedule * Make ForksSchedule generic beyond BftConfigOptions to allow it to work for MiningCoordinator * Extract BftForksScheduleFactory from ForksSchedule * Move ForksSchedule to common package Signed-off-by: Simon Dudley <simon.dudley@consensys.net> * Jiri maintainer (#3100) * adds Jiri Peinlich as maintainer Signed-off-by: Justin Florentine <justin+github@florentine.us> Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net> * Merge: engine_getPayload json-rpc endpoint (#3107) Signed-off-by: garyschulte <garyschulte@gmail.com> * Update bls12-381 native library (#2992) * Update bls12-381 native library Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * QBFT migration: Create migrating mining coordinator (#3097) Create MigratingMiningCoordinator, a schedulable mining coordinator that will switch between mining coordinators based on the consensus schedule. #2999 Signed-off-by: Simon Dudley <simon.dudley@consensys.net> * Remove duplicate code (TODO) in FlexiblePrivacyPrecompiledContract (#3132) Signed-off-by: Romeet Puhar <Romeet27@outlook.com> Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net> * Change the ForkSchedule to take a single argument (#3133) Signed-off-by: Jason Frame <jasonwframe@gmail.com> * QBFT migration: Create a migrating protocol context (#3117) Signed-off-by: Jason Frame <jasonwframe@gmail.com> * baseFee as Wei instead of long (#3065) * [Config] convert baseFee from long to wei Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> * [MINOR] updating enclave AT code to remove orion references (#3143) * cleanup orion messages Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net> * Add Frank Li as a maintainer (#3120) Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net> Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net> * Tls configuration (#3105) * Add support for configuring TLS protocols and ciphersuites Two new command line options --rpc-http-tls-protocols --rpc-http-tls-ciphersuites New test cases added for valid and invalid TLS protocols and ciphersuites Signed-off-by: Jiri Peinlich <jiri.peinlich@gmail.com> * Fixing tests and cleaning up the code Signed-off-by: Jiri Peinlich <jiri.peinlich@gmail.com> * Adding changelog note Signed-off-by: Jiri Peinlich <jiri.peinlich@gmail.com> * Making TLSv1.3 part of the default Signed-off-by: Jiri Peinlich <jiri.peinlich@gmail.com> Co-authored-by: cusden <ian.cusden@gmail.com> Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net> * Migrating mining coordinator duplicate event (#3098) Unsubscribe the active delegate mining coordinator from BlockAddedEvents The MigratingMiningCoordinator acts as the observer on its behalf Signed-off-by: Simon Dudley <simon.dudley@consensys.net> * Update log4j (#3151) Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net> * Eip 4399 (#3111) Signed-off-by: Justin Florentine <justin+github@florentine.us> * merge kintsugi-v3 changes back to main (#3152) * merge kintsugi-v3 changes back to main Signed-off-by: garyschulte <garyschulte@gmail.com> * uprev to 20.10.4-SNAPSHOT (#3159) * uprev to 20.10.4-SNAPSHOT Co-authored-by: Danno Ferrin <danno.ferrin@shemnon.com> Signed-off-by: Justin Florentine <justin+github@florentine.us> * Merge pull request from GHSA-7pg2-p5vj-xp5h Shift operation expect the shift amount to be an unsigned int 256, even though only 8 bits provides meaningful value. Besu optimized this by casting this to an int, but it was signed, resulting in a bad shift amount. To ensure that such errors don't cause issues again test the binary data types as a potentially negative along the major int types (8,16,32,64,128,256). Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com> * fixes for devnet-3 compare UInt256.ZERO in RandomOperation remove MergeBlockImporter and defer to MainnetBlockImporter Signed-off-by: garyschulte <garyschulte@gmail.com> Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Co-authored-by: Frank Li <39414003+frankisawesome@users.noreply.github.com> Co-authored-by: Jiri Peinlich <jiri.peinlich@gmail.com> Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net> Co-authored-by: Adrian Sutton <adrian.sutton@consensys.net> Co-authored-by: Antony Denyer <git@antonydenyer.co.uk> Co-authored-by: Simon Dudley <simon.dudley@consensys.net> Co-authored-by: Justin Florentine <justin+github@florentine.us> Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net> Co-authored-by: RP27 <38156169+RP27@users.noreply.github.com> Co-authored-by: Jason Frame <jasonwframe@gmail.com> Co-authored-by: fab-10 <91944855+fab-10@users.noreply.github.com> Co-authored-by: cusden <ian.cusden@gmail.com> Co-authored-by: Danno Ferrin <danno.ferrin@shemnon.com> Co-authored-by: Danno Ferrin <danno.ferrin@gmail.com>
eum602
pushed a commit
to lacchain/besu
that referenced
this pull request
Nov 3, 2023
Create MigratingMiningCoordinator, a schedulable mining coordinator that will switch between mining coordinators based on the consensus schedule. hyperledger#2999 Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First PR for #2999
TODO:
Make it work for NoopMiningCoordinator (IbftLegacy)Duplicate block event due to SchedulableMiningCoordinator being an observer as well as delegate MiningCoordinatorto be handled in separate PR: Migrating mining coordinator duplicate event #3098Note, despite writing some code for it in this PR, we still need to resolve #3003 as a separate issue.
To discuss:
Is BftForkSchedule the best choice for the mining schedule? If even more classes will use it in the future then I think yes.If yes to above then should BftForkSchedule be renamed to ForksSchedule since it's used with which is more general than BftMiningCoordinator?IBFT1 -> QBFT doesn't work because IBFT1 extraData causes problems when QbftBesuControllerBuilder calls BftBlockInterface.validatorsInBlock. We'll need to resolve this before we can say it works with IBFT1