Skip to content

Commit

Permalink
refactor: CON-1434 Serialize current and next transcripts into `pb::N…
Browse files Browse the repository at this point in the history
…iDkgTranscript` (#3039)

As the `NiDkgId` (and therefore the `NiDkgTag`) is already part of the
`NiDkgTranscript`, serializing the `NiDkgTag` separately as part of
`pb::TaggedNiDkgTranscript` is redundant.

This PR instead serializes the current and next transcripts directly as
`pb::NiDkgTranscript`.

#2838 introduced the new block payload
proto fields of type `pb::NiDkgTranscript`, and implemented the
deserialization function to consider both new and old fields. This
ensures the backwards/forwards compatibility of the change made by this
PR.

During upgrade:
- The old summary still contains `pb::TaggedNiDkgTranscript`s, but they
can still be read as the deserialization function supports both types.
- Newly created summary blocks will instead contain
`pb::NiDkgTranscript`s directly.

During downgrade:
- As the deserialization function supporting both types already exists
in the previous version, Summary blocks created by the version of this
PR (using `pb::NiDkgTranscript`) will be deserialized successfully.
- Newly created summary blocks on the old version will then again
contain `pb::TaggedNiDkgTranscript`s instead.

Once this change is deployed to all mainnet subnets, we may remove the
deprecated `pb::TaggedNiDkgTranscript` fields entirely.


Additionally, this change forces us to fix a state machine setup
function in `rs/state_machine_tests/src/lib.rs`, which incorrectly
marked both genesis NiDkg transcripts as "high threshold". Therefore,
this test setup function broke the (now explicit) invariant that for
each (tag, transcript) in `block.dkg_transcript_map` it holds that `tag
== transcript.tag`.
  • Loading branch information
eichhorl authored Jan 21, 2025
1 parent f52c098 commit f2a9a3b
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 19 deletions.
8 changes: 6 additions & 2 deletions rs/state_machine_tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,9 +422,13 @@ fn make_nodes_registry(
.build();

// Insert initial DKG transcripts
let mut high_threshold_transcript = ni_dkg_transcript.clone();
high_threshold_transcript.dkg_id.dkg_tag = NiDkgTag::HighThreshold;
let mut low_threshold_transcript = ni_dkg_transcript;
low_threshold_transcript.dkg_id.dkg_tag = NiDkgTag::LowThreshold;
let cup_contents = CatchUpPackageContents {
initial_ni_dkg_transcript_high_threshold: Some(ni_dkg_transcript.clone().into()),
initial_ni_dkg_transcript_low_threshold: Some(ni_dkg_transcript.into()),
initial_ni_dkg_transcript_high_threshold: Some(high_threshold_transcript.into()),
initial_ni_dkg_transcript_low_threshold: Some(low_threshold_transcript.into()),
..Default::default()
};
registry_data_provider
Expand Down
25 changes: 8 additions & 17 deletions rs/types/types/src/consensus/dkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,19 +293,12 @@ impl Summary {
}
}

fn build_tagged_transcripts_vec(
fn build_transcripts_vec(
transcripts: &BTreeMap<NiDkgTag, NiDkgTranscript>,
) -> Vec<pb::TaggedNiDkgTranscript> {
) -> Vec<pb::NiDkgTranscript> {
transcripts
.iter()
.map(|(tag, transcript)| pb::TaggedNiDkgTranscript {
tag: pb::NiDkgTag::from(tag) as i32,
transcript: Some(pb::NiDkgTranscript::from(transcript)),
key_id: match tag {
NiDkgTag::HighThresholdForKey(k) => Some(pb::MasterPublicKeyId::from(k)),
_ => None,
},
})
.values()
.map(pb::NiDkgTranscript::from)
.collect()
}

Expand Down Expand Up @@ -355,12 +348,10 @@ impl From<&Summary> for pb::Summary {
.values()
.map(pb::NiDkgConfig::from)
.collect(),
current_transcripts_deprecated: build_tagged_transcripts_vec(
&summary.current_transcripts,
),
current_transcripts_new: Vec::new(),
next_transcripts_deprecated: build_tagged_transcripts_vec(&summary.next_transcripts),
next_transcripts_new: Vec::new(),
current_transcripts_new: build_transcripts_vec(&summary.current_transcripts),
current_transcripts_deprecated: Vec::new(),
next_transcripts_new: build_transcripts_vec(&summary.next_transcripts),
next_transcripts_deprecated: Vec::new(),
interval_length: summary.interval_length.get(),
next_interval_length: summary.next_interval_length.get(),
height: summary.height.get(),
Expand Down

0 comments on commit f2a9a3b

Please sign in to comment.