Simon/documenting dkg #254
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #254 +/- ##
=======================================
Coverage 87.91% 87.91%
=======================================
Files 51 51
Lines 9787 9787
Branches 9787 9787
=======================================
Hits 8604 8604
Misses 733 733
Partials 450 450 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
netrome
left a comment
netrome
left a comment
There was a problem hiding this comment.
Great to have this documented. I have some thoughts and questions. Only blocker is that we don't define the lambda (lagrange coefficient?) before using it making it unclear to me as a reader what it means. There could be more things. I don't understand what the hashes we send in step 12 and 13 help with either.
| ## Types of Thresholds | ||
|
|
||
| There are two types of thresholds one has to be aware of: the **asynchronous distributed systems threshold** a.k.a. the **BFT threshold**, and the **cryptography threshold** a.k.a. the **reconstruction threshold**. | ||
|
|
||
| The BFT threshold states that the maximum number of faulty nodes a distributed system ($\mathsf{MaxFaulty}$) can tolerate while still reaching consensus is at most one-third of the total number of participants $N$. More specifically: |
There was a problem hiding this comment.
Hmm, I like that we're getting into the threshold types here but I wonder if it's relevant for the DKG algorithm. MaxFaulty is unused so I think we could move this part of the discussion to an appendix or similar since it's still interesting. For the purpose of understanding the DKG we only need to know the MaxMalicious parameter which in this case is the reconstruction lower bound - 1 as per our conversation yesterday.
Also, I think we take the reconstruction bound as input in the code but we talk abut max malicious here. Is that correct? I wonder if we should consider migrating to use max malicious as input to the dkg protocol in the future (which is a bit of a tricky change to do safely).
There was a problem hiding this comment.
For the purpose of understanding the DKG we only need to know the MaxMalicious parameter which in this case is the reconstruction lower bound - 1 as per our conversation yesterday.
I see that the MaxFaulty is also essential to the understanding of the algorithm. I suggest I keep this section but I definitely expand things in #98 and #18
There was a problem hiding this comment.
Also, I think we take the reconstruction bound as input in the code but we talk abut max malicious here. Is that correct? I wonder if we should consider migrating to use max malicious as input to the dkg protocol in the future (which is a bit of a tricky change to do safely).
Could you pinpoint the spot in the implementation where we talk about reconstruction bound?
Co-authored-by: Mårten Blankfors <[email protected]>
gilcu3
left a comment
gilcu3
left a comment
There was a problem hiding this comment.
Same as in the EdDSA docs, to properly review this I need to check that it coincides bit by bit with the implementation. To make that task doable for me (and for future crate users) we need to include the step numbers in the code. After that is done I can then go ahead and make a proper review.
| /// H(id, `context_string`, g^{secret} , R) | ||
| /// H(`domain_separator`, id, g^{secret} , R) | ||
| fn challenge<C: Ciphersuite>( | ||
| session_id: &HashOutput, | ||
| domain_separator: u32, | ||
| session_id: &HashOutput, | ||
| id: Scalar<C>, |
There was a problem hiding this comment.
I find it strange that the domain separator is of type u32. Shouldn't it be [u8; 4]?
There was a problem hiding this comment.
What's the reasoning behind this?
There was a problem hiding this comment.
we use it only as bytes later for example. And one of the important characteristics of domain separators is exactly that they have fixed width, so why have them as u32?
Closes #99