Implement Version Upgrade Mechanism #3096

paberr · 2024-11-18T21:26:59Z

What's in this pull request?

This PR implements the mechanism to carry out breaking changes (version upgrades) after launch.

It mostly follows the specification in the Albatross paper with one exception:
We will not only check for the proportion of stake supporting the upgrade, but also for the proportion of slots supporting the upgrade. This makes sure we don't propose an upgrade in an epoch where less than 2f+1 elected validators support the upgrade.

⚠️ All changes in this PR that affect the consensus pre-version upgrade are backwards compatible in that the current nodes enforce stricter rules. All consensus breaking changes would only ever be performed post-version upgrade (e.g., deactivating validators).

Mechanism Overview

The version upgrade mechanism introduces the following key changes, discussed in a bit more detail below:

Modified block validity (i.e., when upgrades are possible)
Tools for checking supporting stake/slots
Validators producing blocks with new version
New version upgrade inherent (deactivating unsupporting validators)
Moving validator selection in election blocks from before the accounts commit to after the accounts commit

Block validity

The version number is stored in every block. We allow version upgrades only in election blocks.
Instead of checking against a constant version from the Policy, block-level verification of the version is now split into two parts:

Block::verify_header: Making sure that the version is always below or equal to the maximum supported version Policy::max_supported_version(network_id). The maximum supported version depends on the network, so that we can roll out changes in the testnet first.
Block:: verify_immediate_successor and Block::verify_macro_successor: Making sure that the version number can only increase in election blocks and only in steps of one. For all other scenarios, the version needs to match the predecessor's version. We test this in both functions as these are called in different scenarios.

Helper functions

We only perform version upgrades when at least Policy:: UPGRADE_MIN_SUPPORT% of the active stake signals support for the upgrade and when at least Policy::TWO_F_PLUS_ONE slots of the currently elected validators signals support.
We add helper functions to determine the supporting stake and slots.

The get_supporting_stake function is computationally expensive in that it iterates over all validators in the staking contract. This number is bounded by the validator deposit and we only call the function when proposing an election block and not being on the maximum version already.

Currently, the support for a version is signalled by encoding the supported version in the first two bytes of the validator's signal data (in big-endian). This can still be changed later and be implemented version-specific in the Policy::supports_upgrade function.

We also add a function to deactivate unsupportive stake, which is discussed in the section about the new inherent.

Producing an upgrade

Validators who updated their client and have a maximum supported version higher than the current blockchain's version (determined from the head block) will determine support for this upgrade whenever they are the ones to produce a macro proposal. Only when the conditions (supporting_stake >= Policy:: UPGRADE_MIN_SUPPORT * active stake && supporting_slots > Policy::TWO_F_PLUS_ONE) are met, the proposal will contain the increased version number. If the proposal is accepted, the upgrade is successfully active from this block on (including this election block).

There is no explicit check for validators to only vote for a block if they support the upgrade. If they updated their client and have a new maximum supported version, the proposal will be deemed valid and voted for. Validators not supporting the upgrade won't update their clients and will reject the proposal due to an invalid version.

Version upgrade inherent

The mechanism adds a new inherent VersionUpgrade that also has the new version. It has the side effect of deactivating all validators that did not explicitly signal support for the new version with immediate effect. This also means that these unsupporting validators should not be selected for the next epoch (thus preventing possible stalls).

Full nodes will apply this side-effect through trie-diffs. The inherent is not stored in the history and thus does not require any special handling.

Switching validator selection and accounts commit

In the current model, we first select the validators for the next epoch (based on the current staking contract) and then apply the accounts changes of an election block. The only accounts changes possible in an election block right now are: reward distribution and preparing the bitsets in the staking contract for the next epoch.

Since none of these have an effect on the validator selection, we switch the order of these two. This is required so that unsupporting validators are deactivated before the validator selection.

Tests

This PR includes unit tests in the following crates:

validator: Testing that the validator will propose an election block with a new version if the conditions are met.
block: Testing the new validity rules.
staking_contract: Testing correct computation of stake/slot support and correct handling of inherent.
blockchain: Testing that the blockchain correctly emits the new inherent.

Pull request checklist

All tests pass. The project builds and runs.
I have resolved any merge conflicts.
I have resolved all clippy and rustfmt warnings.

Add function for supporting slots

Produce upgraded blocks after gathering enough support

Implement upgrade test Add block tests Add accounts tree tests and fix block number Test creation of inherent

hrxi

I wonder if we should somehow support the case of someone not wanting an upgrade. In that case, they should probably not signal support for it — however, this means that they'll get deactivated on a version upgrade. This puts them in a bad spot. Could we maybe add a "I have technical support for this upgrade but I don't like it" flag that allows a validator to vote against an upgrade while not getting deactivated if it does get activated?

hrxi · 2025-02-12T13:48:55Z

primitives/src/policy.rs

+
+            _ => 1,
+        }
+    }


This should probably be a member variable of Policy instead.

hrxi · 2025-02-13T15:04:38Z

validator/src/tendermint.rs

        // Create the proposal.
+        // TODO: Version


hrxi · 2025-02-13T15:06:39Z

validator/src/tendermint.rs

+
+            // We propose an upgraded block version only if:
+            // - `Policy::UPGRADE_MIN_SUPPORT` of the stake supports the upgrade.
+            // - `Policy::TWO_F_PLUS_ONE` slots support the upgrade.


Maybe also only upgrade when 80% of the slots support the upgrade? Otherwise, a few slots can successfully reject this proposal.

hrxi · 2025-02-13T15:08:21Z

primitives/src/policy.rs

+    pub const UPGRADE_MIN_SUPPORT: u8 = 80;
+
+    /// This function is used to determine if a validator signalled for a specific upgrade.
+    /// For now, this is checking the first two bytes of the signal data.


Suggested change

/// For now, this is checking the first two bytes of the signal data.

/// This is checking the first two bytes of the signal data.

"For now" suggests to me that we're currently already planning something else.

hrxi · 2025-02-13T15:11:24Z

validator/src/tendermint.rs

@@ -228,11 +228,62 @@ where
            return Err(ProtocolError::Abort);
        }

+        // Check if we want to upgrade the version.
+        // This assumes we only ever upgrade to the latest version
+        // and don't queue multiple upgrades.


This is not a problem, I think. We can fix this if we ever want to do multiple upgrades at once.

I disagree. I think this is a problem.
In the scenario of a proposed upgrade never being adopted there is no good way of having another update being adopted. Basically this means that every update must be accepted before another can be accepted or to introduce ambiguity.

What version would another update have? In order for it to be able to be adopted it needs to be current_version + 1, but that is already occupied by the first (not adopted) upgrade. This ambiguity is not good I think.

After one update is rejected, the next update (with version + 2) can include the mechanism that updates with + 2 can be accepted as well. We're not setting anything in stone here, I think. Just like our current code doesn't support updates at all but we can still allow the update with version + 1 to work.

hrxi · 2025-02-13T15:12:29Z

blockchain/tests/inherents.rs

+        validators: None,
+        next_batch_initial_punished_set,
+        ..Default::default()
+    };


Most of this struct can probably be omitted since you specify ..Default::default() anyway.

jsdanielh · 2025-02-14T00:31:53Z

blockchain/src/blockchain/inherents.rs

-        Inherent::FinalizeEpoch
+        let mut inherents = vec![Inherent::FinalizeEpoch];
+        // Add version upgrade inherent if needed.
+        if version != self.state.current_version() {


What about:

Suggested change

if version != self.state.current_version() {

if version > self.state.current_version() {

?

jsdanielh · 2025-02-14T00:32:16Z

blockchain/src/blockchain/slots.rs

+        let txn = txn.or_new(&self.db);
+        let staking_contract = self
+            .get_staking_contract_if_complete(Some(&txn))
+            .expect("We should always have the staking contract.");


Suggested change

.expect("We should always have the staking contract.");

.expect("We should always have the staking contract");

jsdanielh · 2025-02-14T00:37:11Z

primitives/Cargo.toml

@@ -61,7 +61,7 @@ coin = ["hex", "nimiq-serde", "regex", "thiserror"]
 key-nibbles = ["hex", "nimiq-keys", "nimiq-database-value", "nimiq-database-value-derive", "nimiq-serde"]
 networks = ["thiserror"]
 parallel = ["rayon", "ark-ec/parallel"]
-policy = ["nimiq-keys", "nimiq-utils", "parking_lot"]
+policy = ["nimiq-keys", "nimiq-utils", "parking_lot", "networks"]


Suggested change

policy = ["nimiq-keys", "nimiq-utils", "parking_lot", "networks"]

policy = [ "networks", "nimiq-keys", "nimiq-utils", "parking_lot"]

jsdanielh · 2025-02-14T00:43:24Z

primitives/block/src/block.rs

@@ -406,13 +406,20 @@ impl Block {
            return Err(BlockError::NetworkMismatch);
        }

-        // Check the version
-        if self.version() != Policy::VERSION {
+        error!(


Why is this an error log?

paberr added 4 commits November 18, 2024 14:32

Add helper functions for upgrades

d4595c4

Add function for supporting slots

Allow version upgrades in election blocks

4632185

Make validator selection dependent on accounts

5a48af3

Remove unsupporting validators after upgrade

477cfbd

Produce upgraded blocks after gathering enough support

paberr added the enhancement New feature or request label Nov 18, 2024

paberr self-assigned this Nov 18, 2024

paberr requested a review from hrxi November 18, 2024 21:42

paberr force-pushed the pb/upgrade branch 2 times, most recently from eb68def to cad6edb Compare November 19, 2024 00:54

Make max supported version dependent on network

42076ca

Implement upgrade test Add block tests Add accounts tree tests and fix block number Test creation of inherent

paberr force-pushed the pb/upgrade branch from cad6edb to 42076ca Compare November 19, 2024 01:59

paberr requested review from styppo and nibhar November 19, 2024 02:10

hrxi reviewed Feb 13, 2025

View reviewed changes

jsdanielh reviewed Feb 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Version Upgrade Mechanism #3096

Implement Version Upgrade Mechanism #3096

paberr commented Nov 18, 2024

hrxi left a comment

hrxi Feb 12, 2025

hrxi Feb 13, 2025

hrxi Feb 13, 2025

hrxi Feb 13, 2025

hrxi Feb 13, 2025

nibhar Feb 13, 2025

hrxi Feb 14, 2025

hrxi Feb 13, 2025

jsdanielh Feb 14, 2025

jsdanielh Feb 14, 2025

jsdanielh Feb 14, 2025

jsdanielh Feb 14, 2025

	/// For now, this is checking the first two bytes of the signal data.
	/// This is checking the first two bytes of the signal data.

	if version != self.state.current_version() {
	if version > self.state.current_version() {

	.expect("We should always have the staking contract.");
	.expect("We should always have the staking contract");

	policy = ["nimiq-keys", "nimiq-utils", "parking_lot", "networks"]
	policy = [ "networks", "nimiq-keys", "nimiq-utils", "parking_lot"]

Implement Version Upgrade Mechanism #3096

Are you sure you want to change the base?

Implement Version Upgrade Mechanism #3096

Conversation

paberr commented Nov 18, 2024

What's in this pull request?

Mechanism Overview

Block validity

Helper functions

Producing an upgrade

Version upgrade inherent

Switching validator selection and accounts commit

Tests

Pull request checklist

hrxi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment