Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion rs/consensus/dkg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -995,7 +995,6 @@ mod tests {
for dkg_id in summary.dkg.configs.keys() {
assert_eq!(dkg_id.target_subnet, NiDkgTargetSubnet::Local);
}
assert_eq!(summary.dkg.transcripts_for_remote_subnets.as_ref(), None);
// Verify that the remote_dkg_attempts are set to `Completed`.
assert_eq!(
summary.dkg.remote_dkg_attempts.get(&target_id),
Expand Down
6 changes: 1 addition & 5 deletions rs/consensus/src/consensus/batch_delivery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ pub(crate) fn deliver_batches_with_result_processor(

/// This function creates responses to the system calls that are redirected to
/// consensus. There are two types of calls being handled here:
/// - Initial NiDKG transcript creation, where a response may come from summary or data payloads.
/// - Initial NiDKG transcript creation, where a response may come from data payloads.
/// - Canister threshold signature creation, where a response may come from from data payloads.
/// - CanisterHttpResponse handling, where a response to a canister http request may come from data payloads.
fn generate_responses_to_subnet_calls(
Expand All @@ -331,10 +331,6 @@ fn generate_responses_to_subnet_calls(
"New DKG summary with config ids created: {:?}",
summary_payload.dkg.configs.keys().collect::<Vec<_>>()
);
if let Some(transcripts) = summary_payload.dkg.transcripts_for_remote_subnets.as_ref() {
consensus_responses
.append(&mut generate_responses_to_remote_dkgs(transcripts, log));
}
CanisterHttpSpent::default()
}
BlockPayload::Data(data_payload) => {
Expand Down
4 changes: 2 additions & 2 deletions rs/protobuf/def/types/v1/dkg.proto
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ message PostSplitArgs {

// next id: 17
message Summary {
reserved 5, 6, 8;
reserved 5, 6, 8, 10;
reserved "transcripts_for_new_subnets";
reserved "transcripts_for_remote_subnets";
uint64 registry_version = 1;
uint64 interval_length = 2;
uint64 next_interval_length = 3;
uint64 height = 4;
repeated NiDkgConfig configs = 7;
repeated RemoteDkgAttemptCount remote_dkg_attempts = 9;
repeated CallbackIdedNiDkgTranscript transcripts_for_remote_subnets = 10;
repeated NiDkgTranscript current_transcripts = 11;
repeated NiDkgTranscript next_transcripts = 12;
oneof subnet_splitting_status {
Expand Down
2 changes: 0 additions & 2 deletions rs/protobuf/src/gen/types/types.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,6 @@ pub struct Summary {
pub configs: ::prost::alloc::vec::Vec<NiDkgConfig>,
#[prost(message, repeated, tag = "9")]
pub remote_dkg_attempts: ::prost::alloc::vec::Vec<RemoteDkgAttemptCount>,
#[prost(message, repeated, tag = "10")]
pub transcripts_for_remote_subnets: ::prost::alloc::vec::Vec<CallbackIdedNiDkgTranscript>,
#[prost(message, repeated, tag = "11")]
pub current_transcripts: ::prost::alloc::vec::Vec<NiDkgTranscript>,
#[prost(message, repeated, tag = "12")]
Expand Down
37 changes: 6 additions & 31 deletions rs/types/types/src/consensus/dkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,6 @@ pub struct DkgSummary {
/// corresponding to this tag.
#[serde_as(as = "Vec<(_, _)>")]
next_transcripts: BTreeMap<NiDkgTag, NiDkgTranscript>,
/// Transcripts that are computed for remote subnets.
pub transcripts_for_remote_subnets: BackwardsCompatible<Vec<RemoteTranscriptResult>, false>,
/// The length of the current interval in rounds (following the start
/// block).
pub interval_length: Height,
Expand Down Expand Up @@ -301,7 +299,6 @@ impl DkgSummary {
.collect(),
current_transcripts,
next_transcripts,
transcripts_for_remote_subnets: BackwardsCompatible::empty(),
registry_version,
interval_length,
next_interval_length,
Expand Down Expand Up @@ -453,22 +450,12 @@ impl From<&DkgSummary> for pb::Summary {
interval_length: summary.interval_length.get(),
next_interval_length: summary.next_interval_length.get(),
height: summary.height.get(),
transcripts_for_remote_subnets: summary
.transcripts_for_remote_subnets
.as_ref()
.map(|t| build_callback_ided_transcripts_vec(t.as_slice()))
// `None` -> empty vector
.unwrap_or_default(),
// Relay the marker instead of only ever setting it for our own summaries: `prost`
// drops unknown fields, so a replica version that decodes a summary coming from a
// version which no longer maintains the field and re-encodes it would otherwise strip
// the marker, turning `None` back into `Some(vec![])` downstream and thereby changing
// the hash of that summary.
transcripts_for_remote_subnets_removed: summary
.transcripts_for_remote_subnets
.as_ref()
.is_none()
.then_some(true),
// The marker must keep being set even though this replica version no longer reads it:
// replica versions that still carry the field derive it from this marker, and would
// otherwise decode the empty repeated field above into `Some(vec![])` and hash its
// length prefix, where this version hashes nothing. It may only stop being set once no
// replica version that reads it is deployed any more.
transcripts_for_remote_subnets_removed: Some(true),
remote_dkg_attempts: build_remote_dkg_attempts_vec(&summary.remote_dkg_attempts),
subnet_splitting_status: summary
.subnet_splitting_status
Expand Down Expand Up @@ -627,18 +614,6 @@ impl TryFrom<pb::Summary> for DkgSummary {
interval_length: Height::from(summary.interval_length),
next_interval_length: Height::from(summary.next_interval_length),
height: Height::from(summary.height),
transcripts_for_remote_subnets: BackwardsCompatible::try_from_proto_with(
// A set marker means the summary was produced by a replica version that no longer
// maintains the field, in which case the repeated field must be ignored entirely,
// including for hashing. Without the marker the repeated field is authoritative,
// even when empty: an empty vector still contributes its length prefix to the hash
// preimage, exactly as it did before the field became `BackwardsCompatible`.
(!summary
.transcripts_for_remote_subnets_removed
.unwrap_or_default())
.then_some(summary.transcripts_for_remote_subnets),
|t| build_transcripts_vec_from_pb(t).map_err(ProxyDecodeError::Other),
)?,
remote_dkg_attempts: build_remote_dkg_attempts_map(&summary.remote_dkg_attempts),
subnet_splitting_status: BackwardsCompatible::try_from_proto(
summary.subnet_splitting_status,
Expand Down
Loading