Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Delayed Prevote Prototype #1460

Closed
wants to merge 41 commits into from

Conversation

staheri14
Copy link
Contributor

No description provided.

@staheri14 staheri14 self-assigned this Aug 21, 2024
@@ -942,6 +946,8 @@ func (cs *State) handleTimeout(ti timeoutInfo, rs cstypes.RoundState) {

cs.enterPrevote(ti.Height, ti.Round)

case cstypes.RoundStepDelaydPrevote:
cs.enterPrevote(ti.Height, ti.Round)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you call doPrevotehere?
If you call enterPrevote here again, the defer in method enterPrevote will be called twice. Not catastrophic, but there'll be a few duplicated events that may confuse external observers (e.g., relayers)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sergio-mena for your comment, you are right, the duplicate events needs to be considered carefully which is not yet the case in the current prototype implementation. But, will certainly address this in the full version.

@staheri14
Copy link
Contributor Author

staheri14 commented Sep 16, 2024

Closing this as the results from delayed precommit experiments [see this] were promising, hence pursuing delayed prevotes wouldn't be necessary at this point.

@staheri14 staheri14 closed this Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants