feat(subnet-splitting): implement `PostSplit` CUPs making & validation by pierugo-dfinity · Pull Request #10980 · dfinity/ic

pierugo-dfinity · 2026-07-31T07:42:05Z

This PR implements the core logic behind subnet splitting in Consensus. It affects the following components, in suggested order of review:

DKG summary payload builder
CUP-shares maker
CUP-shares validator
CUP-shares aggregator
CUP validator
Bouncer

DKG summary payload builder

When a subnet split is detected in the registry, the validation context's registry version is frozen until reaching the next summary, at which point it is bumped to precisely the version it was scheduled. The payload builder thus fills the subnet_splitting_status field with Scheduled if there is a scheduled split at the validation context's registry version. Otherwise, it fills it with NotScheduled. It should never fill it with PostSplit, this will be the role of the CUP-shares maker.

CUP-shares maker

Instead of unconditionally creating a CUP for finalized summary blocks, the CUP-shares maker now looks at the block's subnet_splitting_status field. If it is Scheduled, then it will not create a CUP for it. It will instead create a CUP for a "post-split" block for H+500 which is not notarized/finalized like usual, but is instead computed locally (it is deterministic) and its hash is gossiped as part of the CUP share.
CUP-shares makers will create such shares based on the subnet they will take part in after the split. This means that there will be two distinct sets of CUP shares gossiped around, one for each resulting subnet. Each set of CUP shares contains the relevant catchup information for the new subnet: the DKG transcripts taken from the registry (not the ones from the original summary), the state hash and oldest registry version in use from the split state at the height of the original summary, and a dummy random beacon.

CUP-shares validator

As soon as the Scheduled summary is reached, half the CUP-shares maker will create shares for a CUP A and the other half for a CUP B. The role of the CUP-shares validator will be to invalidate the CUP shares intended for the other subnet and validate the ones intended for its own subnet. To do so, it recreates the post-split block, state hash, etc and only validates shares which match these. The other subnet's CUP shares will be invalidated as the block, beacon and state hash will be different.

CUP-shares aggregator

This component is not changed by much but its logic still had to be slightly rewritten to consider these new post-split CUPs.

CUP validator

Similar to the CUP-shares validator, here as soon as enough shares for the two CUPs are gossiped, they will be aggregated and the two CUPs will be gossiped around. The role of the CUP validator will be to invalidate the CUP intended for the other subnet and validate the one intended for its own subnet. To do so, we look at the CUP's block's subnet_splitting_status as it contains the subnet the CUP is intended for (PostSplit { new_subnet_id }).

Bouncer

Finally, the bouncer needs to tell P2P to start downloading CUP shares also for the next summary height since this is the height at which the post-split CUP will be created.

Remaining changes

Follow-ups will adapt

DKG key manager to preemptively load the post-split transcripts as soon as the Scheduled summary is reached and actually be able to sign the post-split CUP shares
Batch delivery to tell DSM to actually split the state
The orchestrator to restart destination replicas and update their ReplicaConfig.subnet_id

This reverts commit 7bee851.

Copilot

Pull request overview

This PR adds support for “PostSplit” catch-up packages (CUPs) to enable subnet splitting: it records subnet-splitting status in DKG summaries, creates post-split CUP shares/CUPs with on-the-fly summary + dummy random beacon, and extends validation/aggregation/priority logic so replicas can accept the correct CUP artifacts during a split.

Changes:

Extend DKG summary construction to carry SubnetSplittingStatus (incl. PostSplit) and add utilities to compute post-split subnet assignment from the registry.
Introduce CatchUpPackageType and update CUP share creation, share aggregation, and artifact validation to handle post-split CUPs (including dummy random beacons and height/state-hash handling).
Update consensus “bouncer”/pool-reader helpers and add/adjust tests across consensus components to cover subnet-splitting CUP behavior.

Reviewed changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
rs/types/types/src/consensus/dkg.rs	Thread `SubnetSplittingStatus` through `DkgSummary` construction; add creation error variant for subnet-splitting status retrieval.
rs/types/types/src/consensus/catchup.rs	Introduce `CatchUpPackageType` (`Normal` vs `PostSplit { new_subnet_id }`) for CUP maker behavior.
rs/test_utilities/consensus/src/fake.rs	Update `Fake` `DkgSummary` construction to pass default `SubnetSplittingStatus`.
rs/replica/setup_ic_network/src/lib.rs	Pass `registry_client` and `replica_config` to consensus components that now require them.
rs/replay/src/validator.rs	Pass `registry` and `replica_cfg` through to validator construction paths.
rs/registry/helpers/src/node.rs	Add `NodeRegistry::get_subnet_id_from_node_id` helper plus unit tests.
rs/consensus/utils/src/subnet_splitting.rs	Add helpers to detect scheduled splits and compute post-split subnet assignment; expand tests.
rs/consensus/utils/src/pool_reader.rs	Add `get_next_summary_height()` and tests to distinguish summary height progression from CUP height.
rs/consensus/utils/src/lib.rs	Add `aggregate_with_threshold()` to aggregate shares when committee threshold can’t be derived from pool membership (post-split CUPs).
rs/consensus/tests/payload.rs	Update test consensus wiring to pass `registry_client` and `replica_config` where required.
rs/consensus/tests/framework/runner.rs	Update consensus runner wiring to pass `registry_client` and `replica_config` where required.
rs/consensus/src/consensus/validator.rs	Extend CUP/CUP-share validation for subnet splitting: correct subnet filtering post-split, post-split block/beacon handling, and associated tests.
rs/consensus/src/consensus/share_aggregator.rs	Aggregate post-split CUP shares by deriving transcript/threshold from the post-split summary block; log aggregation events; update tests.
rs/consensus/src/consensus/priority.rs	Adjust bouncer logic to fetch CUP shares up to the next summary height and exempt CUP shares from the “validator-CUP gap”; add tests.
rs/consensus/src/consensus/catchup_package_maker.rs	Implement post-split CUP share creation (post-split summary + dummy beacon), factoring out CUP-type detection and helper constructors; expand tests.
rs/consensus/src/consensus/block_maker.rs	Minor test flow adjustment around when summary blocks are produced in subnet-splitting-related tests.
rs/consensus/src/consensus.rs	Wire new dependencies (`registry_client`, `replica_config`) into consensus subcomponents (CUP maker, share aggregator, etc.).
rs/consensus/idkg/src/payload_builder.rs	Update test DKG summary construction call site to pass default `SubnetSplittingStatus`.
rs/consensus/dkg/src/remote.rs	Update test DKG summary construction call site to pass default `SubnetSplittingStatus`.
rs/consensus/dkg/src/payload_validator.rs	Update test/component construction paths to pass `registry` and `replica_config` where required.
rs/consensus/dkg/src/payload_builder.rs	Record subnet-splitting status in created summaries; add post-split summary construction from registry CUP contents; add tests.
rs/consensus/dkg/src/lib.rs	Update DKG tests to pass `registry`/`replica_config` into `DkgKeyManager` construction helper(s).
rs/consensus/dkg/src/dkg_key_manager.rs	Load post-split transcripts (from post-split summary) instead of “Scheduled” summary transcripts during a split; add tests.
rs/consensus/dkg/Cargo.toml	Add `rstest` dependency for DKG test expansions.
rs/consensus/dkg/BUILD.bazel	Add `rstest` to Bazel test deps.
Cargo.lock	Add `rstest` to the resolved dependency set for affected crates.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

rs/consensus/src/consensus/priority.rs:115

This now requests every advertised CUP share between the current CUP and the next summary, although only shares at already-finalized heights or exactly the synthetic post-split height can be useful. Shares for intermediate future/data heights fail with FinalizedBlockNotFound and remain unvalidated, allowing peers to consume pool/network resources across the whole DKG interval. Preserve the old finalized-height condition and add only height == next_summary_height for the post-split case.

            } else if height <= next_summary_height {

The post-split transcript loading in the DKG key manager is being split out into `pierugo/subnet-splitting/dkg-key-manager`, which is stacked on top of this branch.

Copilot

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

rs/consensus/utils/src/subnet_splitting.rs:122

The contained subnet ID is omitted from this error's display text, so failures only report that a membership change occurred without identifying the unexpected assignment. Include {0} to make production diagnostics actionable.

    #[error("The node changed subnets during subnet splitting")]
    DisallowedMembershipChange(SubnetId),

Copilot

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.

zeropath-ai · 2026-08-19T07:48:06Z

✅ No security or compliance issues detected. Reviewed everything up to ae301cb.

Security Overview

🔎 Scanned files: 19 changed file(s)
🔗 Scan Link: https://zeropath.com/app/repositories/f5e9d2bb-29a4-4b64-902a-391d65fd8b2d?scanId=bf722e50-0e47-492e-8c55-0a6d06de4f79&codeScanTypes=PrScan&tab=issues

Detected Code Changes

The diff is too large to display a summary of code changes.

zeropath-ai · 2026-08-19T07:48:15Z

✅ No security or compliance issues detected. Reviewed everything up to ae301cb.

Security Overview

🔎 Scanned files: 19 changed file(s)
🔗 Scan Link: https://zeropath.com/app/repositories/f975f1a5-0a47-4a3b-b7f0-a55609e90390?scanId=65d7f05c-7c78-4681-bae1-be412b37d0f1&codeScanTypes=PrScan&tab=issues

Detected Code Changes

The diff is too large to display a summary of code changes.

eichhorl · 2026-08-20T08:01:55Z

-                Arc::clone(&registry_client),
+                registry_client.clone(),


I think the convention is actually Arc::clone, but we don't seem to follow it so no action required.

eichhorl · 2026-08-20T08:45:50Z

+        debug!(
+            self.log,
+            "Proposing a CatchUpPackageShare (type: {cup_type:?}) at height {height}"
+        );


I don't think debug logs show up anywhere

eichhorl · 2026-08-20T08:47:46Z

+        // Skip if this node has already made a share
+        if pool
+            .get_catch_up_package_shares(height)
+            .any(|share| share.signature.signer == my_node_id)
+        {
+            return None;
+        }


Maybe we can still do this check more towards the top of the function

eichhorl · 2026-08-20T11:48:11Z

+            self.replica_config.node_id,
+            &start_block,
+        )
+        .inspect_err(|err| warn!(self.log, "Failed to get the catch up package type: {err}"))


Adding every_n_seconds would avoid flooding the logs, if this ever were to happen. Same for the logs below

eichhorl · 2026-08-20T11:55:14Z

+                    state_manager,
+                    ..
+                } = DependenciesBuilder::multiple_subnets(pool_config, records)
+                    .with_replica_config(ReplicaConfig {
+                        node_id: NODE_5,
+                        subnet_id: SOURCE_SUBNET_ID,
+                    })
+                    .build();
+
+                let fake_state_hash = CryptoHashOfState::from(CryptoHash(vec![1, 2, 3]));
+                state_manager
+                    .get_mut()
+                    .expect_get_state_hash_at()
+                    .return_const(Ok(fake_state_hash.clone()));
+
+                let message_routing = FakeMessageRouting::new();
+                *message_routing.next_batch_height.write().unwrap() = Height::from(2);
+                let message_routing = Arc::new(message_routing);
+
+                let cup_maker = CatchUpPackageMaker::new(
+                    ReplicaConfig {
+                        node_id: NODE_5,
+                        subnet_id: SOURCE_SUBNET_ID,
+                    },


Suggested change

state_manager,

..

} = DependenciesBuilder::multiple_subnets(pool_config, records)

.with_replica_config(ReplicaConfig {

node_id: NODE_5,

subnet_id: SOURCE_SUBNET_ID,

})

.build();

let fake_state_hash = CryptoHashOfState::from(CryptoHash(vec![1, 2, 3]));

state_manager

.get_mut()

.expect_get_state_hash_at()

.return_const(Ok(fake_state_hash.clone()));

let message_routing = FakeMessageRouting::new();

*message_routing.next_batch_height.write().unwrap() = Height::from(2);

let message_routing = Arc::new(message_routing);

let cup_maker = CatchUpPackageMaker::new(

ReplicaConfig {

node_id: NODE_5,

subnet_id: SOURCE_SUBNET_ID,

},

state_manager,

replica_config,

..

} = DependenciesBuilder::multiple_subnets(pool_config, records)

.with_replica_config(ReplicaConfig {

node_id: NODE_5,

subnet_id: SOURCE_SUBNET_ID,

})

.build();

let fake_state_hash = CryptoHashOfState::from(CryptoHash(vec![1, 2, 3]));

state_manager

.get_mut()

.expect_get_state_hash_at()

.return_const(Ok(fake_state_hash.clone()));

let message_routing = FakeMessageRouting::new();

*message_routing.next_batch_height.write().unwrap() = Height::from(2);

let message_routing = Arc::new(message_routing);

let cup_maker = CatchUpPackageMaker::new(

replica_config,

eichhorl · 2026-08-20T12:00:38Z

+    debug_assert!(matches!(
+        last_summary.subnet_splitting_status(),
+        SubnetSplittingStatus::Scheduled(..)
+    ));


Should we return an error in this case?

eichhorl · 2026-08-20T12:02:51Z

+    FailedToGetSubnetIdFromTheRegistry(RegistryVersion, RegistryClientError),
+    #[error("The node is unassigned at registry version {0}")]
+    Unassigned(RegistryVersion),
+    #[error("The node changed subnets during subnet splitting")]


Suggested change

#[error("The node changed subnets during subnet splitting")]

#[error("The node changed to subnet {0} during the split, which is neither the source nor destination subnet")]

eichhorl · 2026-08-20T12:20:58Z

+                    warn!(
+                        self.log,


Suggested change

warn!(

self.log,

warn!(

every_n_seconds => ...,

self.log,

eichhorl · 2026-08-20T12:30:02Z

+        let shares = pool
+            .get_catch_up_package_shares(block.height())
+            .map(|share| Signed {
+                content: CatchUpContent::from_share_content(share.content, block.clone()),


Should we keep the check that the hash in the share actually matches the block (that check was previously hidden in get_block)

eichhorl · 2026-08-20T12:59:26Z

+    let subnet_splitting_status = get_subnet_splitting_status(
+        subnet_id,
+        registry_client,
+        registry_version,
+        validation_context.registry_version,
+    )?;
+


If the DKG interval is smaller than ACCEPTABLE_NOTARIZATION_CERTIFICATION_GAP, could it happen that we create valid summary at the "post-split CUP height" the usual way?

github-actions Bot added the feat label Jul 31, 2026

pierugo-dfinity changed the base branch from master to pierugo/subnet-splitting/freeze-registry-version July 31, 2026 07:42

pierugo-dfinity force-pushed the pierugo/subnet-splitting/cup-maker branch from 5097309 to eef64e2 Compare July 31, 2026 09:24

pierugo-dfinity added 4 commits August 3, 2026 08:48

feat: freeze registry version until summary

3b9ff82

fix: clippy

ccbf2bc

refactor: pass last summary directly as argument

d1a8601

docs

da13866

feat: DKG key manager + CUP maker + validator + aggregator

e8bb30d

pierugo-dfinity force-pushed the pierugo/subnet-splitting/cup-maker branch from eef64e2 to e8bb30d Compare August 4, 2026 11:54

ws

21e197b

Base automatically changed from pierugo/subnet-splitting/freeze-registry-version to master August 6, 2026 15:14

pierugo-dfinity added 13 commits August 7, 2026 17:55

Merge branch 'master' into pierugo/subnet-splitting/cup-maker

fb292d6

fix: merge

fd920a6

test: more tests

085c40e

refactor: code cleaning

78da8b9

fix: clippy

4a0a74b

fix: Leo's old comments + docs

af85746

test: stop pinning brittle hash

04dfaa9

Merge branch 'master' into pierugo/subnet-splitting/cup-maker

8b65a10

refactor: load post-split summary once and in lieu of scheduled summary

b732a90

style: re-use replica_config

7a4f360

test: stronger checks in unit test

9af60da

test both subnets

7e68f73

fix: update refs of IDKG summary

7bee851

pierugo-dfinity force-pushed the pierugo/subnet-splitting/cup-maker branch from 8d7e105 to 7bee851 Compare August 14, 2026 16:09

pierugo-dfinity added 3 commits August 17, 2026 08:50

Revert "fix: update refs of IDKG summary"

08e69e5

This reverts commit 7bee851.

feat: do not support splitting a chain-key enabled subnet

8c87cba

feat: wire in status to summary

d62e752

pierugo-dfinity requested a lite review from Copilot August 18, 2026 11:24

Copilot started reviewing on behalf of pierugo-dfinity August 18, 2026 11:25 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Comment thread rs/types/types/src/consensus/catchup.rs Outdated

Comment thread rs/consensus/src/consensus/catchup_package_maker.rs

typos

01be326

pierugo-dfinity requested a balanced review from Copilot August 18, 2026 14:39

Copilot started reviewing on behalf of pierugo-dfinity August 18, 2026 14:40 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Comment thread rs/consensus/dkg/src/dkg_key_manager.rs Outdated

pierugo-dfinity added 4 commits August 18, 2026 15:42

debug

4a9cd11

fix: retain post-split transcripts

03857ee

refactor: simplify share_aggregator

cd51408

refactor: move DKG key manager changes to their own branch

cd9530e

The post-split transcript loading in the DKG key manager is being split out into `pierugo/subnet-splitting/dkg-key-manager`, which is stacked on top of this branch.

pierugo-dfinity requested a balanced review from Copilot August 18, 2026 19:19

Copilot started reviewing on behalf of pierugo-dfinity August 18, 2026 19:19 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Comment thread rs/consensus/src/consensus/validator.rs

fix: ensure CUP's block is a summary

ae301cb

pierugo-dfinity requested a balanced review from Copilot August 18, 2026 19:34

Copilot started reviewing on behalf of pierugo-dfinity August 18, 2026 19:35 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Comment thread rs/consensus/src/consensus/priority.rs

pierugo-dfinity requested a balanced review from Copilot August 18, 2026 19:51

Copilot started reviewing on behalf of pierugo-dfinity August 18, 2026 19:52 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

pierugo-dfinity marked this pull request as ready for review August 19, 2026 07:47

pierugo-dfinity requested review from a team as code owners August 19, 2026 07:47

github-actions Bot added @node @core-protocol labels Aug 19, 2026

eichhorl reviewed Aug 20, 2026

View reviewed changes

Bownairo approved these changes Aug 20, 2026

View reviewed changes

	#[error("The node changed subnets during subnet splitting")]
	#[error("The node changed to subnet {0} during the split, which is neither the source nor destination subnet")]

Uh oh!

Conversation

pierugo-dfinity commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DKG summary payload builder

CUP-shares maker

CUP-shares validator

CUP-shares aggregator

CUP validator

Bouncer

Remaining changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

zeropath-ai Bot commented Aug 19, 2026

Uh oh!

zeropath-ai Bot commented Aug 19, 2026

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!

4 participants

pierugo-dfinity commented Jul 31, 2026 •

edited

Loading