feat: Fast upgrades: upgrade payload section in blocks by frankdavid · Pull Request #11655 · dfinity/ic

frankdavid · 2026-09-22T20:20:05Z

Add the upgrade section to block payloads and wire it into the payload builder infrastructure (with a placeholder builder for now).

Add upgrade_payload_bytes to the Block proto and upgrade to BatchPayload/BatchMessages
Add an Upgrade section to BatchPayloadSectionBuilder
Add the ic-consensus-upgrade crate with a placeholder UpgradePayloadBuilder

First step of the Phase-2 rolling-reboot consensus protocol, which adds the data types and the artifact mechanism. * Add `UpgradePermitAction` (`Request` / `Authorize` / `Return`). `UpgradePermitAuthorizationRequest` is the signed content, `UpgradePermitAuthorizationShare` is the gossiped artifact. * Add the in-memory `UpgradePermitAuthPoolImpl` which is modeled after other pools. * Add the `ic-consensus-upgrade` crate with `UpgradePermitAuthPoolManager`. It signs shares for requests in finalized blocks and validates gossiped shares. * Register the new proto file with the generator and add `UpgradePermitAuthorizationRequest` (signing) and `UpgradePermitAuthorizationShare` (pool-ID hashing) domain separators.

Add the upgrade section to block payloads and wire it into the payload builder infrastructure (with a placeholder builder for now). - Add `upgrade_payload_bytes` to the `Block` proto and upgrade to `BatchPayload`/`BatchMessages` - Add an `Upgrade` section to `BatchPayloadSectionBuilder` - Add the `ic-consensus-upgrade` crate with a placeholder `UpgradePayloadBuilder`

zeropath-ai · 2026-09-22T20:22:24Z

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

Security Overview

🔎 Scanned files: 30 changed file(s)
🔗 Scan Link: https://zeropath.com/app/repositories/f5e9d2bb-29a4-4b64-902a-391d65fd8b2d?scanId=1a46c624-e386-4255-ae50-6780cf920d6d&codeScanTypes=PrScan&tab=issues&detectionTypes=traditional,businessLogic,customRule,IAC,SECRETS,CICD,EOL,uncategorized

Detected Code Changes

Change Type	Relevant files
Enhancement	► rs/consensus/upgrade/BUILD.bazel Add upgrade module to build dependencies ► rs/consensus/upgrade/Cargo.toml Add ic-consensus-upgrade dependency ► rs/consensus/upgrade/src/lib.rs Introduce upgrade payload builder module ► rs/consensus/upgrade/src/payload_builder.rs Implement UpgradePayloadBuilderImpl with build/validate methods
Enhancement	► rs/interfaces/src/upgrade.rs Define InvalidUpgradePayloadReason enum ► rs/interfaces/src/consensus.rs Add InvalidUpgradePayload to InvalidPayloadReason
Enhancement	► rs/protobuf/def/types/v1/consensus.proto Add upgrade_payload_bytes field to Block message ► rs/protobuf/src/gen/types/types.v1.rs Update Block proto struct to include upgrade_payload_bytes
Enhancement	► rs/types/types/src/batch.rs Add upgrade field to BatchPayload ► rs/types/types/src/consensus.rs Include upgrade_payload_bytes in Block conversions
Enhancement	► rs/consensus/src/consensus/payload.rs Add Upgrade variant to BatchPayloadSectionBuilder and related handling ► rs/consensus/src/consensus/payload_builder.rs Update to include Upgrade payload section in builder pipeline ► rs/consensus/src/consensus.rs Extend constructor to accept upgrade_payload_builder and wire through tests
Enhancement	► rs/consensus/src/consensus/payload_builder.rs (existing file changes reflected above)
Enhancement	► rs/consensus/BUILD.bazel (existing) updated to include upgrade in various targets (references across BUILD files)
Enhancement	► rs/replica/setup_ic_network/BUILD.bazel Include upgrade in consensus dependencies ► rs/replica/setup_ic_network/Cargo.toml Add ic-consensus-upgrade dependency
Enhancement	► rs/replica/setup_ic_network/src/lib.rs Import UpgradePayloadBuilderImpl and instantiate in start_consensus
Enhancement	► rs/replica/setup_ic_network/src/lib.rs Wire upgrade_payload_builder into start_consensus call sites
Enhancement	► rs/consensus/tests/framework/runner.rs Pass upgrade_payload_builder through framework dependencies
Enhancement	► rs/consensus/tests/framework/types.rs Add upgrade_payload_builder to ConsensusDependencies
Enhancement	► rs/consensus/tests/payload.rs Create upgrade_payload_builder and pass to test setup ► rs/consensus/tests/payload.rs Include upgrade in consensus payload tests
Enhancement	► rs/consensus/tests/payload.rs Adjust test expectations to account for upgrade payload ordering and size calculations
Enhancement	► rs/consensus/upgrade/BUILD.bazel (new file) ► rs/consensus/upgrade/Cargo.toml (new file) ► rs/consensus/upgrade/src/lib.rs (new file) ► rs/consensus/upgrade/src/payload_builder.rs (new file)
Enhancement	► rs/interfaces/src/upgrade.rs (new) and related interface changes across upgrade-related codepaths

alin-at-dfinity

The change overall LGTM, but I'll leave it to Leo or Pierugo to approve, particularly since I'm not sure what the changed hashes in the crypto_hash_stability tests imply.

alin-at-dfinity · 2026-09-23T15:19:40Z

        assert_eq!(
            hex::encode(hash.get_ref().0.as_slice()),
-            "764535296841f3db421a928cfadff3460be406d0182da64034eee623a9a97e99",
+            "c20a87578beb94df369dabfefc30c0d47d170c75d68236aae3b16335c0f21c4a",


@pierugo-dfinity, @eichhorl, is this (and the similar hash changes below) OK? Does it require any other changes / staged rollout / whatever?

The comment on mod crypto_hash_stability simply says "hashes [...] must remain constant across code changes".

It's a red herring. As Leo mentioned, the better alternative would be to use a summary block for building the CUP instead of a data block here and below.

Moved to summary block

Do you want to split the block and use summary for CUP tests and Data for others (only block_proposal_stability atm)?

I think it's fine for now (a proposal for a summary block is just as legitimate). If that's not too much of a burden, maybe you could rename fn test_block to fn test_summary_block such that it's clearer for next time (we're already modifying the surrounding code).

pierugo-dfinity · 2026-09-24T11:24:31Z

        logger: ReplicaLogger,
    ) -> Self {
        let section_builder = vec![
+            BatchPayloadSectionBuilder::Upgrade(upgrade_payload_builder),


Why put it first when it is last everywhere else?

Just for test stability. Otherwise the test would need more changes. The section_builder order should not matter for behavior but if you prefer a particular order, I can change it (and then also change the tests accordingly.)

I think it is cleaner to keep it last here as well, if the plumbing is expected when adding a new section (which is rare) and isn't too demanding.

pierugo-dfinity · 2026-09-24T11:33:35Z

+        // TODO: implement proper validation
+        UpgradePayload::deserialize(payload)
+            .map(|_| ())
+            .map_err(|e| {
+                ValidationError::InvalidArtifact(InvalidPayloadReason::InvalidUpgradePayload(
+                    InvalidUpgradePayloadReason::DecodeFailed(format!("{e:?}")),
+                ))
+            })


Shouldn't we actually validate that the payload is empty? Otherwise a malicious node could fill that section.

Execution doesn't care about it, so it wouldn't really matter, right? The next PR is bringing the real logic, so unless you can think of a way how this could be abused, I'd just leave it for now.

I suppose the idea is that a malicious block maker could fill the upgrade payload with garbage, slowing down the subnet (this up to 4 MB payload would have to be gossiped to everyone for no good reason).

OTOH, this is only a temporary situation, until we actually start putting useful content in the payload.

fill the upgrade payload with garbage, slowing down the subnet

Yes exactly. I think it's best to always keep the builder and the validator in sync. And rejecting obvious malicious behaviour like here.

pierugo-dfinity · 2026-09-24T11:49:40Z

        assert_eq!(
            hex::encode(hash.get_ref().0.as_slice()),
-            "764535296841f3db421a928cfadff3460be406d0182da64034eee623a9a97e99",
+            "c20a87578beb94df369dabfefc30c0d47d170c75d68236aae3b16335c0f21c4a",


It's a red herring. As Leo mentioned, the better alternative would be to use a summary block for building the CUP instead of a data block here and below.

Co-authored-by: Pierugo Pace <pierugo.pace@dfinity.org>

…st-upgrades-block-payload

…rades-block-payload # Conflicts: # rs/types/types/src/crypto/hash/tests.rs

frankdavid added 10 commits September 11, 2026 16:59

lint

f8f1910

Revert some stuff that will be added in a later PR to keep PR small

6a82887

"against"

f2a3e65

Hash tests

3b60c88

Add UpgradePayload wrapper

85f87cf

default_batch_payload_is_zero_bytes for upgrade

433eb15

Do not include upgrade_payload_bytes in the Block yet (will come later)

71384a7

Revert upgrade.rs

4b590b9

frankdavid added this pull request to stack #11656 September 22, 2026 20:20

frankdavid requested a review from a team as a code owner September 22, 2026 20:20

github-actions Bot added the feat label Sep 22, 2026

Revert BatchPayload for now

8424007

github-actions Bot added the @core-protocol label Sep 22, 2026

alin-at-dfinity reviewed Sep 23, 2026

View reviewed changes

frankdavid added 3 commits September 23, 2026 17:26

Address review comments

0bde3a7

Address review comments

705835a

Address review comments

9ade04f

pierugo-dfinity reviewed Sep 24, 2026

View reviewed changes

frankdavid and others added 3 commits September 24, 2026 14:00

Update rs/types/types/src/consensus/upgrade.rs

547b366

Co-authored-by: Pierugo Pace <pierugo.pace@dfinity.org>

Address review comments + remove unnecessary derives

6bfa154

Merge branch 'frankdavid/fast-upgrades-consensus1' into frankdavid/fa…

49ec513

…st-upgrades-block-payload

frankdavid removed this pull request from stack #11656 September 24, 2026 13:33

Base automatically changed from frankdavid/fast-upgrades-consensus1 to master September 24, 2026 14:41

frankdavid and others added 3 commits September 24, 2026 17:29

Address review comments

ccd9cda

Merge remote-tracking branch 'origin/master' into frankdavid/fast-upg…

76047db

…rades-block-payload # Conflicts: # rs/types/types/src/crypto/hash/tests.rs

Automatically fixing code for linting and formatting issues

e49ead3

frankdavid requested a review from pierugo-dfinity September 24, 2026 15:36

frankdavid requested a review from alin-at-dfinity September 24, 2026 15:36

Missing deps

5324358

Conversation

frankdavid commented Sep 22, 2026

Uh oh!

zeropath-ai Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alin-at-dfinity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

pierugo-dfinity Sep 25, 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!

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!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zeropath-ai Bot commented Sep 22, 2026 •

edited

Loading

pierugo-dfinity Sep 25, 2026 •

edited

Loading