Ibft Height Manager #418
Ibft Height Manager #418
Conversation
The IbftHeightManager is responsible for all things 'meta-round' related, such things as: * Handling RoundTimeout and starting a new round * Handling NewRound messages and starting a new round * Ensuring RoundChange messages are sent at the correct time with appropriate content. * Collating RoundChange messages and starting a new round using the best prepared certificate in the collection.
|
|
||
| private void actionOrBufferMessage( | ||
| final SignedData<? extends Payload> msg, | ||
| final Action inRoundHandler, |
There was a problem hiding this comment.
Be nicer to have stronger typing here for the inRoundHandler function reference so say something like Consumer<SignedData<T>> inRoundHandler where T is <T extends Payload>
There was a problem hiding this comment.
That will work, the goal here was to have the lambdas fully defined in the calling function - i.e. this function no longer actually needs the msg (other than for its RoundNumber) - as the supplied lambda already has the message contained within it.
Have replaced 'Action' with the inbuilt Runnable.
By having the lambda contain the message, this function is no longer templated, which is a secondary nicety.
.../src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java
Outdated
Show resolved
Hide resolved
.../src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| if (newRoundMessageValidator.validateNewRoundMessage(msg)) { | ||
|
|
|
|
||
| private void actionOrBufferMessage( | ||
| final SignedData<? extends Payload> msg, | ||
| final Runnable inRoundHandler, |
There was a problem hiding this comment.
Don't love Runnable as it's documented to be for actions run in a thread. I can't find a suitable alternative in java.util.function that matches the type signature. Do we know of anything?
There was a problem hiding this comment.
I'd argue the docs are providing purpose without appreciating the concept of something which can be "done" - i.e. its a consumer without parameter, and its a supplier without a result.
Anyway - its been changed to a Biconsumer.
| roundChangeCache | ||
| .entrySet() | ||
| .removeIf(entry -> isAnEarlierOrEqualRound(entry.getKey(), completedRoundIdentifier)); | ||
| .removeIf(entry -> isAnEarlierRound(entry.getKey(), completedRoundIdentifier)); |
There was a problem hiding this comment.
not part of your change, but can simplify with .keySet().removeIf(roundIdentifier-> isAnEarlierRound(roundIdentifier, ...
| when(messageValidator.validateCommmitMessage(any())).thenReturn(true); | ||
| when(messageValidator.validatePrepareMessage(any())).thenReturn(true); | ||
|
|
||
| // when(finalState.getValidators()).thenReturn(validators); |
| @@ -27,7 +27,7 @@ public IbftStateMachine(final IbftBlockCreatorFactory blockCreatorFactory) { | |||
| /** | |||
| * Attempt to consume the event and update the maintained state | |||
| * | |||
| * @param event the external action that has occurred | |||
| * @param event the external Action that has occurred | |||
There was a problem hiding this comment.
This got capitalised in an auto refactor from the Action type?
| final Runnable inRoundHandler, | ||
| private <T extends Payload> void actionOrBufferMessage( | ||
| final SignedData<T> msg, | ||
| final Consumer<SignedData<T>> inRoundHandler, | ||
| final Consumer<RoundState> buffer) { |
There was a problem hiding this comment.
with inRoundHandler accepting the message but the buffer lambda having the msg already inside it, feels mismatched. Need to make them both take msg or neither. I'm leaning towards both and making buffer be BiConsumer<RoundState, SignedData>
The IbftHeightManager is responsible for all things 'meta-round'
related, such things as:
appropriate content.
best prepared certificate in the collection.