feat(consenus): [CON-1565] splitting cup share making/validating/aggregating by kpop-dfinity · Pull Request #8623 · dfinity/ic

kpop-dfinity · 2026-01-30T16:33:11Z

Background

During subnet splitting, when replicas detect that a subnet should be split at a summary height H, they will immediately start producing CUP shares at height H + dkg_interval_length, according to the post-split subnet memberships, i.e.
half of the node will produce the shares as members of the original subnet, and the other half as the member of the new subnet.

Implement the logic of subnet splitting CUP share creation, validation, and aggregation.

CUP shares creation

When a replica detects that a "subnet splitting" block (denoted by a new field, subnet-splitting-status in the DkgSummary struct) has been finalized and the state at the block's height has been committed, it will create a CUP in the following way:

Create a new summary block, on the fly, at the next CUP height, with dkg transcript taken from the registry;
Create a dummy random beacon at the next CUP height,
Check if node belongs to the high threshold committee, based on the transcript from the newly created summary block, if not, return None,
Create a CUP, with the block and random beacon created in steps 1. and 2., respectively, and sign it using the key material from the block.

CUP shares validation

Validation works in the following way:

Check if the hash of the block in the CUP share matches the recreated summary block's hash
Check if the random beacon in the CUP share matches the recreated random beacon
Check if the signature can be verified using the key material from the recreated summary block
Check if the state hash matches our local state's hash

Note

As the field subnet_splitting_status is not yet population, the new code branches are never executed.

* validator * share aggregator

eichhorl · 2026-02-04T09:23:58Z

+            height,
+            initial_dkg_attempts,
+        } = self;
+


Don't we need to do something like this?

Suggested change

if let Some(status) = subnet_splitting_status {

status.hash(state);

}

pierugo-dfinity · 2026-02-06T09:54:43Z

+        assert!(
+            !self.records.is_empty(),
+            "Cannot setup a registry without records."
+        );


I think self.records[0] two lines above would have panicked before reaching this assert.

eichhorl · 2026-02-04T13:55:57Z

 use std::sync::Arc;

-/// CatchUpPackage maker is responsible for creating beacon shares
+/// [`CatchUpPackage`] maker is responsible for creating beacon shares


Suggested change

/// [`CatchUpPackage`] maker is responsible for creating beacon shares

/// [`CatchUpPackage`] maker is responsible for creating catch up package shares

eichhorl · 2026-02-05T17:00:04Z

+    SubnetSplittingError(String),
 }

 /// Reasons for why a dkg payload might be invalid.
 #[derive(PartialEq, Debug)]
+#[allow(clippy::large_enum_variant)]
 pub enum InvalidDkgPayloadReason {
    CryptoError(CryptoError),
    DkgVerifyDealingError(DkgVerifyDealingError),
    MismatchedDkgSummary(DkgSummary, DkgSummary),
    MissingDkgConfigForDealing,
    DkgStartHeightDoesNotMatchParentBlock,
    DkgSummaryAtNonStartHeight(Height),
+    SubnetSplittingNotEnabled,


What does "not enabled" mean? I don't think these variants are used anywhere (yet?).

eichhorl · 2026-02-05T17:09:45Z

+    subnet_id: SubnetId,
+    registry: &dyn RegistryClient,
+    registry_version: RegistryVersion,
+    subnet_splitting_status: Option<SubnetSplittingStatus>,


I assume the Option part will disappear eventually? Do I understand correctly that calling this function with SubnetSplittingStatus::Scheduled { .. } doesn't make sense? Maybe we could change the argument to Option<SubnetId> where None corresponds to SubnetSplittingStatus::NotScheduled and Some(id) corresponds to SubnetSplittingStatus::Done { id }?

Or we could use the new CatchUpPackageType enum from below

eichhorl · 2026-02-06T08:24:26Z


+pub struct DependenciesBuilder {
+    pool_config: ArtifactPoolConfig,
+    records: Vec<(u64, SubnetId, SubnetRecord)>,


Suggested change

records: Vec<(u64, SubnetId, SubnetRecord)>,

subnet_records: Vec<(u64, SubnetId, SubnetRecord)>,

eichhorl · 2026-02-06T08:26:27Z

+        if let Some(last_version) = last_version {
+            add_subnet_list_record(
+                &registry_data_provider,
+                last_version,
+                Vec::from_iter(subnet_ids),
+            );
+        }


Why do we need to do this twice if last_version != version (see line 174)?

eichhorl · 2026-02-06T09:42:33Z

+        cup_block: &Block,
+        cup_type: CatchUpPackageType,
+    ) -> Result<NiDkgId, String> {
+        // TODO: can we always take the transcript from the block?


Intuitively I would say yes, because the summary block is the first block of the current interval

eichhorl · 2026-02-06T10:04:32Z

+        }
+
+        let cup_content =
+            CatchUpContent::from_share_content(shares[0].content.clone(), block.clone());


Is this clone necessary?

Suggested change

CatchUpContent::from_share_content(shares[0].content.clone(), block.clone());

CatchUpContent::from_share_content(shares[0].content.clone(), block);

eichhorl · 2026-02-06T10:05:49Z

+    fn aggregate_catch_up_package_shares_for_summary_block(
+        &self,
+        pool: &PoolReader<'_>,
+        summary_block: &Block,


Does this work?

Suggested change

summary_block: &Block,

summary_block: Block,

eichhorl · 2026-02-06T10:07:40Z

+            .get_catch_up_package_shares(block.height())
+            .collect::<Vec<_>>();
+
+        if shares.len() < threshold {


I guess despite this check we might still not have enough shares below, because get_catch_up_package_shares above might return shares created by the other half of the subnet?

Or would the signature verification fail when validating those shares?

eichhorl · 2026-02-06T10:12:52Z

+                    InvalidArtifactReason::InvalidHeightInSplittingCatchUpPackageShare.into(),
+                );
+            }
+            CatchUpPackageType::PostSplit { .. } | CatchUpPackageType::Normal => {


When do we ever expect to see the CatchUpPackageType::PostSplit { .. } case here?

kpop-dfinity added 4 commits January 28, 2026 09:12

dkg types

e5cee2a

Merge branch 'master' into kpop/types_only

bc9d509

Ref

764f751

* cup maker

e95109f

* validator * share aggregator

kpop-dfinity added the CI_ALL_BAZEL_TARGETS Runs all bazel targets label Jan 30, 2026

github-actions Bot added the feat label Jan 30, 2026

kpop-dfinity added 5 commits February 2, 2026 08:52

fix clippy

e563e18

Merge branch 'kpop/types_only' into kpop/cup_maker

f0a4121

rev

a0df55c

clean up

771bc09

protobuf lint

a9a46c7

kpop-dfinity changed the title ~~feat(subnet-splitting): splitting cup share making/validating/aggregating~~ feat(consenus):[CON-1565] splitting cup share making/validating/aggregating Feb 2, 2026

kpop-dfinity changed the title ~~feat(consenus):[CON-1565] splitting cup share making/validating/aggregating~~ feat(consenus): [CON-1565] splitting cup share making/validating/aggregating Feb 3, 2026

kpop-dfinity marked this pull request as ready for review February 3, 2026 13:51

kpop-dfinity requested review from a team as code owners February 3, 2026 13:51

github-actions Bot added @consensus @ic-interface-owners labels Feb 3, 2026

eichhorl reviewed Feb 4, 2026

View reviewed changes

pierugo-dfinity reviewed Feb 6, 2026

View reviewed changes

eichhorl reviewed Feb 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(consenus): [CON-1565] splitting cup share making/validating/aggregating#8623

feat(consenus): [CON-1565] splitting cup share making/validating/aggregating#8623
kpop-dfinity wants to merge 9 commits into
masterfrom
kpop/cup_maker

kpop-dfinity commented Jan 30, 2026 •

edited

Loading

Uh oh!

eichhorl Feb 4, 2026 •

edited

Loading

Uh oh!

pierugo-dfinity Feb 6, 2026

Uh oh!

eichhorl Feb 4, 2026

Uh oh!

eichhorl Feb 5, 2026

Uh oh!

eichhorl Feb 5, 2026

Uh oh!

eichhorl Feb 6, 2026

Uh oh!

eichhorl Feb 6, 2026

Uh oh!

eichhorl Feb 6, 2026

Uh oh!

eichhorl Feb 6, 2026

Uh oh!

eichhorl Feb 6, 2026

Uh oh!

eichhorl Feb 6, 2026

Uh oh!

eichhorl Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

+if let Some(status) = subnet_splitting_status {
+    status.hash(state);
+}

	/// [`CatchUpPackage`] maker is responsible for creating beacon shares
	/// [`CatchUpPackage`] maker is responsible for creating catch up package shares

	records: Vec<(u64, SubnetId, SubnetRecord)>,
	subnet_records: Vec<(u64, SubnetId, SubnetRecord)>,

	CatchUpContent::from_share_content(shares[0].content.clone(), block.clone());
	CatchUpContent::from_share_content(shares[0].content.clone(), block);

Uh oh!

Conversation

kpop-dfinity commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

CUP shares creation

CUP shares validation

Note

Uh oh!

eichhorl Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kpop-dfinity commented Jan 30, 2026 •

edited

Loading

eichhorl Feb 4, 2026 •

edited

Loading