consensus: fix round number when handling RoundStepNewRound timeout#9252
consensus: fix round number when handling RoundStepNewRound timeout#9252cmwaters merged 2 commits intotendermint:mainfrom fatcat22:fix/from-main-round-of-newround-timeout
Conversation
cmwaters
left a comment
There was a problem hiding this comment.
LGTM
@milosevic any idea why it was like this in the first place?
|
Is there a test case that reveals this problem? |
|
I can double-check, but this timeout is scheduled when the node enters a height. This means that when it is triggered, the round to start is always 0. |
You are right, sorry about my mistake. I was brosing the master branch at first, for after reading the development-procedure document, I supposed that master branch is the default dev branch. (Am I wrong anain, or the doc is out of date?) Anyway, I still advise that using |
|
No problems, and to nothing to be sorry about, you presented an evidence that this behavior is not clearly documented in the code. |
|
We migrated from |
There was a problem hiding this comment.
I still think we should make the change here, even if the it doesn't have an immediate effect. I wrote the code that used this method in master and assumed that it actually used the round number value. It introduced a bug just by using the code in the way one would reasonably assume it worked.
Closes #9229
PR checklist
CHANGELOG_PENDING.mdupdated, or no changelog entry neededdocs/) and code comments, or nodocumentation updates needed