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

[Fix] Correct quorum threshold calculation in test function is_round_reached() #3384

Open
wants to merge 4 commits into
base: staging
Choose a base branch
from

Conversation

ungaro
Copy link

@ungaro ungaro commented Aug 24, 2024

Motivation

This PR updates the quorum threshold calculation in the is_round_reached function to ensure correct Byzantine fault tolerance. The current implementation uses a simple majority calculation, which is insufficient for Byzantine fault tolerance in certain cases.

Current implementation:

let quorum_threshold = self.validators.len() / 2 + 1;

Proposed implementation:

let quorum_threshold = self.validators.len() - (self.validators.len() - 1) / 3;

The new calculation is based on the principle that in a Byzantine fault-tolerant system with N validators, we must be able to tolerate f faulty validators, where N = 3f + 1 + k (0 <= k < 3). The correct quorum threshold is N - f, which our new calculation achieves.

Quorum Threshold Comparison

Comparison between current implementation (n / 2 + 1) and proposed implementation (n - (n - 1) / 3) for various ranges of validator counts (n).

Range: 1 <= n < 10

n Current (n/2 + 1) Proposed (n - (n-1)/3) Correct?
1 1 1 Yes
2 2 2 Yes
3 2 3 No
4 3 3 Yes
5 3 4 No
6 4 5 No
7 4 5 No
8 5 6 No
9 5 7 No

Range: 99 < n < 110

n Current (n/2 + 1) Proposed (n - (n-1)/3) Correct?
100 51 67 No
101 51 68 No
102 52 68 No
103 52 69 No
104 53 70 No
105 53 70 No
106 54 71 No
107 54 72 No
108 55 72 No
109 55 73 No

Range: 999 < n < 1010

n Current (n/2 + 1) Proposed (n - (n-1)/3) Correct?
1000 501 667 No
1001 501 668 No
1002 502 668 No
1003 502 669 No
1004 503 670 No
1005 503 670 No
1006 504 671 No
1007 504 672 No
1008 505 672 No
1009 505 673 No

Key Observations:

  1. For n < 4, both implementations give the same result, but the current implementation is correct only by coincidence.

  2. Starting from n = 3, the proposed implementation consistently requires a higher quorum threshold, which is necessary for Byzantine fault tolerance.

  3. As n increases, the difference between the two implementations becomes more pronounced. The current implementation significantly underestimates the required quorum for larger validator sets.

  4. The current implementation always returns a value close to 50% of n, which is insufficient for Byzantine fault tolerance.

  5. The proposed implementation correctly calculates the Byzantine fault-tolerant quorum (approximately 2/3 of n) for all values of n.

This comparison demonstrates that the proposed implementation provides the correct quorum threshold for Byzantine fault tolerance across all ranges of validator counts, while the current implementation fails to do so in almost all cases where n > 2.

@ungaro
Copy link
Author

ungaro commented Aug 24, 2024

would like to get a review, thanks.
@bendyarm @d0cd

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