-
Notifications
You must be signed in to change notification settings - Fork 85
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 threshold for light client state update #1461
Conversation
Is it feasible to add a regression test for this? |
Any suggestion for this? I don't know where to put. |
Not sure. It's kind of a "magic", it's not that obvious that the correct computation is Would it make sense to add a |
Also to be clear, I don't intend to delay merging of this PR. It's just a suggestion to reduce tech-debt and future problems. |
I agree with @sveitser, single responsibility principle when possible. |
Good idea! Just implemented this. |
hotshot-state-prover/src/service.rs
Outdated
@@ -79,6 +79,12 @@ pub struct StateProverConfig { | |||
pub stake_table_capacity: usize, | |||
} | |||
|
|||
#[inline] | |||
/// A helper function to compute the quorum threshold given a total amount of stake. | |||
pub fn compute_quorum_threshold(total_stake: U256) -> U256 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should call this one_honest_threshold
, not quorum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right: 0a6b8b1
No issue related.
This PR:
use_mock_contract
option.This PR does not:
Key places to review: