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

RecoveryRequest triggers PrepareRequest from Primary even if the time is not yet came to send it #74

Closed
roman-khimov opened this issue Apr 19, 2023 · 1 comment · Fixed by #75
Assignees
Labels
bug Something isn't working

Comments

@roman-khimov
Copy link
Member

roman-khimov commented Apr 19, 2023

Consider a network operated by @aprasolova. She has seven consensus nodes and tries to play with one of them, substituting CN+IR setup with a single IR using internal CN. We have a network with TimePerBlock: 15s, but she accidentally sets it to 1s on the node she works with. What happens next is whole network creating blocks each ~1-3s which is obvious for the case when we have a primary node using 1s timeout, but less so when it's a backup. When it's a backup it sends a recovery request after ~2s which triggers

                        } else if d.IsPrimary() {
                                d.sendPrepareRequest()

in onRecoveryMessage on a primary and then everyone is rushing to complete a block.

Can @aprasolova be blamed for misconfiguration? YES, that's a huge and obvious mistake. But what she can't be blamed for is a single-node setting affecting whole network. This MUST NOT happen at the dBFT/CN level.

I'm wondering what would the C# node reaction be to a similar RecoveryRequest.

@roman-khimov roman-khimov added the bug Something isn't working label Apr 19, 2023
AnnaShaleva added a commit that referenced this issue Apr 21, 2023
We have timer, and we should follow it. Close #74.

Signed-off-by: Anna Shaleva <anna@nspcc.ru>
AnnaShaleva added a commit that referenced this issue Apr 21, 2023
We have timer, and we should follow it. Close #74.

Signed-off-by: Anna Shaleva <anna@nspcc.ru>
AnnaShaleva added a commit that referenced this issue Apr 21, 2023
We have timer, and we should follow it. Close #74.

Signed-off-by: Anna Shaleva <anna@nspcc.ru>
@vncoelho
Copy link

Sure, @roman-khimov,

In fact, that line could be removed.
It would be also good to keep in mind a mechanism that often check network parameters between nodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants