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

refactor(consensus): Refactor code in NiDkg to prepare VetKD implementation #3522

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Sawchord
Copy link
Contributor

@Sawchord Sawchord commented Jan 20, 2025

This PR makes changes to DKG in order to prepare the implementation of CON-1413.
In particular, it makes two changes:

  1. Summary::current_transcript now returns an Option, rather than panicking.
    With the introduction of the NiDkgTag::HighThresholdForKey(_), we can no longer enumerate all variants, and we should not assume that the Summary contains all the tags we assume. In fact, when generating a vetkey, we will end up in situations where next_transcripts contains tags that current_transctips doesn't have. This PR simply adds unwraps at all call sites, since the code should not panic, if called with NiDkgTag::LowThreshold or NiDkgTag::HighThreshold.
  2. Reimplement into_next_transcripts and move it into dkg crate
    The current implementation is slightly wrong, since it enumerates the tags from the current_transcripts, which makes the assumption that the tags of next_transcripts is a subset of the tags of current_transcripts. This is not correct. When we add a new vetkey, we generate a Config for it at put it in theSummary. This will generate a NiDkgTag::HighThresholdForKey(_) transcript for it and put it into next_transcripts of the next block. So this transcript would never become a current_transcript with the old code. The new code simply copies next_transcripts over to the new summary as current_transcripts and then looks for old current_trascripts whose tags are missing and copies them over too.
    Also the code was moved from ic-types into the dkg crate, since it was only used there, and this way we can add some logging.

@Sawchord Sawchord marked this pull request as ready for review January 20, 2025 16:08
@Sawchord Sawchord requested a review from a team as a code owner January 20, 2025 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant