Conversation
jking-aus
left a comment
jking-aus
left a comment
There was a problem hiding this comment.
nice catch zac -- good extension to tests as well
| .retain(|&round, _value| round >= self.current_round); | ||
|
|
||
| // We are waiting for consensus on a round change, do not start the round yet | ||
| if matches!(self.state, InstanceState::SentRoundChange) { |
There was a problem hiding this comment.
What happens to the caller(s) in this case?
There was a problem hiding this comment.
If we hit this state, that means that we have received some round change messages but have not received a quorum yet, so nothing happens until the node reaches a round change quorum and can start the next round.
There was a problem hiding this comment.
I guess I'm just wondering about the API. In this case, we call start_roundbut nothing happens and it's not communicated to the caller.
There was a problem hiding this comment.
Ahh got it good point. I dont think it really matters. The caller can assume that when it makes some call to qbft, if there is a message that has to be sent it will receive it. In this case, we have no message to send since there is no progress to be made. here it just spins. I will think more on this.
There was a problem hiding this comment.
It somehow feels strange to call start_round, nothing happens, and there's no error. Does the caller find out about it in another way? Does it call it again later?
There was a problem hiding this comment.
Yep I get what you are saying. Calling start_round makes it seem like the round will start. Maybe try_start_round is better? Im hesitant to call this an error because it is functioning correctly, it just cant make any progress. Maybe it is better try to put this logic into the proposal code.
The caller will keep calling it until there is a quorum of round change messages and it can make progress, or right away when it is round 0. There should be errors added so that the caller can handle clear errors such as incorrect messages, but in this case there is really nothing for the caller to do. All that can be done is to wait for more round change messages to reach quorum.
3 participants
Issue Addressed
#36
Proposed Changes
Few changes here
Update
Todo