Skip to content

feat(subnet-splitting): implement subnet splitting's halting logic - #10936

Merged
pierugo-dfinity merged 18 commits into
masterfrom
pierugo/subnet-splitting/status
Aug 3, 2026
Merged

feat(subnet-splitting): implement subnet splitting's halting logic#10936
pierugo-dfinity merged 18 commits into
masterfrom
pierugo/subnet-splitting/status

Conversation

@pierugo-dfinity

@pierugo-dfinity pierugo-dfinity commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

This PR implements the halting logic of subnet splitting by adapting Consensus' get_status function, used by block making, block validation, and batch delivery.

On a Scheduled subnet split summary, replicas will halt as they are about to skip the whole interval and create CUPs for the next summary height directly.

On a PostSplit subnet split summary, the split is done. As a replica's subnet ID is determined at startup, the source subnet will have it correct and can continue creating blocks. Though for a short amount of time, the destination subnet will still have the configured source subnet ID while the summary indicates the destination subnet ID. In that case, do not create new blocks (i.e. halt) until the orchestrator observes this PostSplit CUP and restarts the replica (with the correct destination subnet ID). After they restarted, replicas' subnet ID will match the one in the summary and should create blocks.

Note: as the two subnets are still connected under the same P2P network after the split but have different halting conditions, it is expected to receive artifact invalidation during that time: the destination subnet will receive non-empty blocks from the source subnet.
Note 2: again, because the two subnets are still connected under the same P2P network after the split, the source subnet will broadcast certifications/certification shares for heights above the post-split summary. These should be ignored by the destination subnet: coming later in a separate PR.

P.S.: the PR also finds the last summary block at one single place and passes it around during block making and validation, allowing to remove some error variants and avoid panics.

@github-actions github-actions Bot added the feat label Jul 28, 2026
Base automatically changed from pierugo/add-subnet-splitting-status-field to master July 30, 2026 09:38
@pierugo-dfinity
pierugo-dfinity force-pushed the pierugo/subnet-splitting/status branch from 6af0506 to 46227a8 Compare July 30, 2026 10:04
@pierugo-dfinity
pierugo-dfinity marked this pull request as ready for review July 30, 2026 12:11
@pierugo-dfinity
pierugo-dfinity requested a review from a team as a code owner July 30, 2026 12:11
@zeropath-ai

zeropath-ai Bot commented Jul 30, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to fb258f5.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rs/consensus/dkg/src/lib.rs
    Modify test calls to use parent.clone() and include dkg_summary_block in payload comparisons
► rs/consensus/dkg/src/payload_builder.rs
    Add last_summary_block parameter; pass and use in payload creation
► rs/consensus/dkg/src/payload_validator.rs
    Add last_summary_block parameter; adjust usage in validation
► rs/consensus/idkg/src/payload_builder.rs
    Add prev_summary_block parameter; switch to using summary_block variable in data payload creation
► rs/consensus/idkg/src/payload_builder/errors.rs
    Remove ConsensusSummaryBlockNotFound error variant
► rs/consensus/idkg/src/payload_verifier.rs
    Add last_summary_block parameter; thread through validation to include summary block
► rs/consensus/idkg/src/payload_verifier.rs
    Adjust validate_summary_payload and validate_data_payload signatures to accept last_summary_block
► rs/consensus/src/consensus/batch_delivery.rs
    Retrieve dkg summary block for finalization height; use in status checks
► rs/consensus/src/consensus/block_maker.rs
    Fetch last_summary_block for parent; pass to block making routines; log if missing
► rs/consensus/src/consensus/catchup_package_maker.rs
    Pass start_block to halting checks
► rs/consensus/src/consensus/malicious_consensus.rs
    Fetch last_summary_block for status checks and pass into block making flow
► rs/consensus/src/consensus/notary.rs
    Adjust halting check to include optional last_summary_block in status decision
► rs/consensus/src/consensus/status.rs
    Additional handling for last_summary_block in get_status and should_halt; compute subnet-splitting status based on summary block; introduce any(...) helper; update tests to reflect new behavior
► rs/consensus/src/consensus/validator.rs
    Add retrieval of last_summary_block; adjust status checks; update error enum case for consensus status retrieval; propagate last_summary_block through validation calls
► rs/test_utilities/artifact_pool/src/consensus_pool.rs
    Populate last_summary_block for payload builder in test environment
► rs/types/types/src/consensus/dkg.rs
    Remove MissingDkgStartBlock error variant from DkgPayloadCreationError

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements consensus halting behavior needed for subnet splitting by extending consensus status evaluation to consider subnet-splitting state from the governing DKG summary, and by passing the last summary block through block making/validation paths to avoid repeated lookups and panics.

Changes:

  • Extend consensus::status::{get_status, should_halt} to include subnet-splitting-based halting (Scheduled/PostSplit), and refactor boolean aggregation.
  • Thread the governing DKG summary block through consensus block making, validation, and batch delivery; remove error variants/panics that previously came from re-looking up the summary block.
  • Update/expand tests to cover subnet splitting halting scenarios and adapt existing DKG/iDKG validation paths to the new signatures.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
rs/types/types/src/consensus/dkg.rs Removes a DKG payload creation error variant now that callers provide the governing summary block.
rs/test_utilities/artifact_pool/src/consensus_pool.rs Updates test payload builder wiring to fetch and pass the last DKG summary block.
rs/consensus/src/consensus/validator.rs Fetches governing summary once and passes it into status + iDKG/DKG validation; improves failure classification.
rs/consensus/src/consensus/status.rs Adds subnet-splitting halting logic and refactors halting decision computation; expands tests.
rs/consensus/src/consensus/notary.rs Adapts should_halt call site to new signature.
rs/consensus/src/consensus/malicious_consensus.rs Threads last DKG summary block into block construction path.
rs/consensus/src/consensus/catchup_package_maker.rs Adapts should_halt call site and uses start (summary) block for halting checks.
rs/consensus/src/consensus/block_maker.rs Fetches governing summary once, passes through payload builders, and logs when proposing splitting summaries.
rs/consensus/src/consensus/batch_delivery.rs Retrieves summary block earlier and passes it into status checks before delivering non-summary batches.
rs/consensus/idkg/src/payload_verifier.rs Accepts governing summary block from caller instead of re-looking it up (removes panic path).
rs/consensus/idkg/src/payload_builder/errors.rs Removes an iDKG error variant tied to missing summary lookup.
rs/consensus/idkg/src/payload_builder.rs Threads the prior summary block through iDKG payload building rather than looking it up internally.
rs/consensus/dkg/src/payload_validator.rs Accepts governing summary block from caller instead of re-looking it up (removes panic path) and updates tests.
rs/consensus/dkg/src/payload_builder.rs Requires caller-provided governing summary block, avoiding internal lookup/error variant.
rs/consensus/dkg/src/lib.rs Updates DKG tests for new validator signature requiring the governing summary block.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rs/consensus/src/consensus/status.rs
Comment thread rs/consensus/src/consensus/block_maker.rs
Comment thread rs/consensus/src/consensus/status.rs
Comment thread rs/consensus/src/consensus/status.rs Outdated
Comment thread rs/consensus/src/consensus/validator.rs Outdated
Comment thread rs/consensus/src/consensus/status.rs Outdated
Comment thread rs/consensus/src/consensus/notary.rs
Comment thread rs/consensus/src/consensus/status.rs Outdated
Comment thread rs/consensus/src/consensus/status.rs Outdated
Comment thread rs/consensus/src/consensus/status.rs Outdated
@pierugo-dfinity
pierugo-dfinity added this pull request to the merge queue Aug 3, 2026
Merged via the queue into master with commit 198de6d Aug 3, 2026
38 checks passed
@pierugo-dfinity
pierugo-dfinity deleted the pierugo/subnet-splitting/status branch August 3, 2026 08:32
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.

3 participants