This repository has been archived by the owner on Sep 26, 2019. It is now read-only.
Ibft2: Replace NewRound with extended Proposal #872
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
jframe
reviewed
Feb 18, 2019
| return false; | ||
| } | ||
| } else { | ||
| if (!roundChangeCertificate.isPresent()) { |
There was a problem hiding this comment.
be nice if this block was split out into function for validating the future proposal
| @@ -149,19 +148,17 @@ public void whenSufficientRoundChangeMessagesAreReceivedForNewRoundLocalNodeCrea | |||
| final RoundChange rc3 = peers.getNonProposing(2).injectRoundChange(targetRound, empty()); | |||
| final RoundChange rc4 = peers.getProposer().injectRoundChange(targetRound, empty()); | |||
|
|
|||
| final NewRound expectedNewRound = | |||
| localNodeMessageFactory.createNewRound( | |||
| final Proposal expectedNewRound = | |||
There was a problem hiding this comment.
nit: this should be renamed to expectedProposal
There was a problem hiding this comment.
done (plus lots of others).
| @@ -55,7 +73,15 @@ public static Proposal decode(final BytesValue data) { | |||
| final SignedData<ProposalPayload> payload = SignedData.readSignedProposalPayloadFrom(rlpIn); | |||
| final Block proposedBlock = | |||
| Block.readFrom(rlpIn, IbftBlockHashing::calculateDataHashForCommittedSeal); | |||
|
|
|||
| RoundChangeCertificate roundChangeCertificate = null; | |||
There was a problem hiding this comment.
This is a bit nasty, be nicer if reading the round change certificate was split out into a seperate function then you could quite easily return Optional without the need for the variable initialiser.
consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java
Show resolved
Hide resolved
* PAN-2270: Rename password hash command * Fixing test * PR review
…to the returned CompletableFuture. (PegaSysEng#884)
…ing (PegaSysEng#876) * Reduce logging for invalid peer discovery packets. The message is enough to locate the source of the rejection. * Reduce ECIESHandshaker logging to trace since it documents a normal flow through the handshake process.
* o * add test * clean up * scaffolding * update * update * comments * add test * update * update ii * format * update ii * fix * verifyBroadcastBlockInvocation * test * update * update to difficulty calculation * remove BlockBroadcasterTest from this pr * update * update * update II
jframe
approved these changes
Feb 20, 2019
saltiniroberto
approved these changes
Feb 20, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The NewRound message has been removed, and the Proposal has been extended to include the RoundChangeCertificate (for non-0 rounds).
This has required updates to:
Unit and integration tests have been summarily updated.