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

Just extend by factor if commit is from current view #762

Merged
merged 3 commits into from
Sep 19, 2022

Conversation

vncoelho
Copy link
Member

@vncoelho vncoelho commented Sep 16, 2022

Extend by factor with a commit from previous view will only de-synchronize nodes of current view, instead of acting as a factor for ensuring coherence between nodes.

Copy link
Contributor

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

Looks like we've had this behavior for some time already, seems to work fine:
nspcc-dev/dbft@57394a1
nspcc-dev/dbft#16

@vncoelho
Copy link
Member Author

vncoelho commented Sep 16, 2022

It is good that you noticed that @roman-khimov, and also good to know that you already modified in the neo-go version.
It would had been nice if you had communicated it here at the time, in order to keep nodes with same protocol.

In our vision, this change is needed to keep consistent with the mechanism that it is made for.

@roman-khimov
Copy link
Contributor

It would had been nice if you had communicated it here at the time, in order to keep nodes with same protocol.

Vitor, you're absolutely right and in general we do report on things we try/implement (like neo-project/neo#2057 or neo-project/neo#2058 or many other non-dBFT things), it's just this particular change was made quite a while ago, we were not entirely sure on it and we didn't track these deviations properly back then.

But now it's more than two years old, kinda tested, should be fine to have it here too.

@vncoelho
Copy link
Member Author

@roman-khimov, sounds good, you guys are doing a great job!

@vncoelho vncoelho merged commit 67300b1 into master Sep 19, 2022
@vncoelho vncoelho deleted the vncoelho-patch-3 branch September 19, 2022 11:13
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.

None yet

3 participants