From 477cfbdf659a843bd7d6cee56382f5f97efb2f51 Mon Sep 17 00:00:00 2001 From: Pascal Berrang Date: Sun, 17 Nov 2024 09:42:32 -0600 Subject: [PATCH] Remove unsupporting validators after upgrade Produce upgraded blocks after gathering enough support --- blockchain/src/block_production/mod.rs | 7 ++- blockchain/src/blockchain/history_sync.rs | 17 +++++++- blockchain/src/blockchain/inherents.rs | 18 ++++++-- blockchain/tests/block_production.rs | 5 +++ .../src/account/staking_contract/traits.rs | 17 +++++++- primitives/src/policy.rs | 19 ++++++++ .../transaction/src/historic_transaction.rs | 6 ++- primitives/transaction/src/inherent.rs | 5 ++- test-utils/src/block_production.rs | 1 + test-utils/src/blockchain.rs | 1 + test-utils/src/blockchain_with_rng.rs | 1 + .../src/performance/blockchain-push/main.rs | 1 + validator/src/tendermint.rs | 43 ++++++++++++++++++- validator/tests/tendermint.rs | 24 +++++++++-- 14 files changed, 152 insertions(+), 13 deletions(-) diff --git a/blockchain/src/block_production/mod.rs b/blockchain/src/block_production/mod.rs index 397c68c2a1..3fc43b6bad 100644 --- a/blockchain/src/block_production/mod.rs +++ b/blockchain/src/block_production/mod.rs @@ -253,12 +253,15 @@ impl BlockProducer { round: u32, // Extra data for this block. extra_data: Vec, + // Version for this block. + version: Option, ) -> Result { self.next_macro_block_proposal_with_rng( blockchain, timestamp, round, extra_data, + version, &mut rand::thread_rng(), ) } @@ -277,6 +280,8 @@ impl BlockProducer { round: u32, // Extra data for this block. extra_data: Vec, + // Version for this block (or current default). + version: Option, // The rng seed. We need this parameterized in order to have determinism when running unit tests. rng: &mut R, ) -> Result { @@ -325,7 +330,7 @@ impl BlockProducer { // state. let mut header = MacroHeader { network, - version: blockchain.state().current_version(), + version: version.unwrap_or_else(|| blockchain.state().current_version()), block_number, round, timestamp, diff --git a/blockchain/src/blockchain/history_sync.rs b/blockchain/src/blockchain/history_sync.rs index 8ef8eeb214..bf2d4a78aa 100644 --- a/blockchain/src/blockchain/history_sync.rs +++ b/blockchain/src/blockchain/history_sync.rs @@ -287,7 +287,7 @@ impl Blockchain { "Inserting final macro block" ); block_numbers.push(block.block_number()); - block_timestamps.push(0); // FIXME + block_timestamps.push(block.timestamp()); block_transactions.push(vec![]); block_inherents.push(vec![]); } @@ -303,10 +303,25 @@ impl Blockchain { .push(Inherent::FinalizeBatch); if Policy::is_election_block_at(*block_number) { + assert_eq!( + *block_number, + block.block_number(), + "Only the last block can be an election block" + ); block_inherents .get_mut(i) .unwrap() .push(Inherent::FinalizeEpoch); + + // If the version changed, add the upgrade inherent. + if this.state.current_version() < block.version() { + block_inherents + .get_mut(i) + .unwrap() + .push(Inherent::VersionUpgrade { + new_version: block.version(), + }); + } } } } diff --git a/blockchain/src/blockchain/inherents.rs b/blockchain/src/blockchain/inherents.rs index 3aafb888ff..917ed9b6d4 100644 --- a/blockchain/src/blockchain/inherents.rs +++ b/blockchain/src/blockchain/inherents.rs @@ -26,7 +26,7 @@ impl Blockchain { if Policy::is_election_block_at(macro_block.block_number()) { // On election the previous epoch needs to be finalized. // We can rely on `state` here, since we cannot revert macro blocks. - inherents.push(self.finalize_previous_epoch()); + inherents.append(&mut self.finalize_previous_epoch(macro_block.header.version)); } inherents @@ -321,8 +321,20 @@ impl Blockchain { } /// Creates the inherent to finalize an epoch. The inherent is for updating the StakingContract. - pub fn finalize_previous_epoch(&self) -> Inherent { + pub fn finalize_previous_epoch(&self, version: u16) -> Vec { // Create the FinalizeEpoch inherent. - Inherent::FinalizeEpoch + let mut inherents = vec![Inherent::FinalizeEpoch]; + // Add version upgrade inherent if needed. + if version != self.state.current_version() { + assert_eq!( + version, + self.state.current_version(), + "Version should only be upgraded in steps of one." + ); + inherents.push(Inherent::VersionUpgrade { + new_version: version, + }) + } + inherents } } diff --git a/blockchain/tests/block_production.rs b/blockchain/tests/block_production.rs index ddff3395eb..c44a2eb044 100644 --- a/blockchain/tests/block_production.rs +++ b/blockchain/tests/block_production.rs @@ -190,6 +190,7 @@ fn it_can_produce_macro_blocks() { bc.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0u32, vec![], + None, ) .unwrap(); @@ -218,6 +219,7 @@ fn it_can_produce_macro_block_after_punishment() { bc.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0u32, vec![], + None, ) .unwrap(); @@ -268,6 +270,7 @@ fn it_can_produce_macro_block_after_punishment() { bc.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0u32, vec![], + None, ) .unwrap(); @@ -322,6 +325,7 @@ fn it_can_produce_election_blocks() { bc.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0u32, vec![0x42], + None, ) .unwrap(); @@ -368,6 +372,7 @@ fn it_can_produce_a_chain_with_txns() { blockchain.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0u32, vec![], + None, ) .unwrap(); diff --git a/primitives/account/src/account/staking_contract/traits.rs b/primitives/account/src/account/staking_contract/traits.rs index f06f408a8f..0838e818a5 100644 --- a/primitives/account/src/account/staking_contract/traits.rs +++ b/primitives/account/src/account/staking_contract/traits.rs @@ -2,6 +2,7 @@ use nimiq_keys::Address; use nimiq_primitives::{ account::{AccountError, AccountType}, coin::Coin, + policy::Policy, }; use nimiq_serde::Deserialize; use nimiq_transaction::{ @@ -805,6 +806,20 @@ impl AccountInherentInteraction for StakingContract { Ok(None) } Inherent::Reward { .. } => Err(AccountError::InvalidForTarget), + Inherent::VersionUpgrade { new_version } => { + // Deactivate unsupporting validators. + let mut tx_logger = TransactionLog::empty(); + let mut store = StakingContractStoreWrite::new(&mut data_store); + self.deactivate_unsupporting_validators( + &mut store, + |data| Policy::supports_upgrade(data, *new_version), + block_state.number, + &mut tx_logger, + )?; + inherent_logger.push_tx_logger(tx_logger); + + Ok(None) + } } } @@ -878,7 +893,7 @@ impl AccountInherentInteraction for StakingContract { Ok(()) } - Inherent::FinalizeBatch | Inherent::FinalizeEpoch => { + Inherent::FinalizeBatch | Inherent::FinalizeEpoch | Inherent::VersionUpgrade { .. } => { // We should not be able to revert finalized epochs or batches! Err(AccountError::InvalidForTarget) } diff --git a/primitives/src/policy.rs b/primitives/src/policy.rs index 4939004d14..058d2d317e 100644 --- a/primitives/src/policy.rs +++ b/primitives/src/policy.rs @@ -1,5 +1,6 @@ use std::cmp; +use nimiq_hash::Blake2bHash; use nimiq_keys::Address; use nimiq_utils::math::powi; use once_cell::sync::OnceCell; @@ -156,6 +157,24 @@ impl Policy { /// 25 MB. pub const HISTORY_CHUNKS_MAX_SIZE: u64 = 25 * 1024 * 1024; + /// Percentage of minimum supporting stake to perform upgrade. + /// A value of `80` means 80% of the stake needs to support the upgrade. + 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. + /// However, the check could also be version specific in the future. + pub fn supports_upgrade(signal_data: Option, version: u16) -> bool { + signal_data + .map(|data| { + // Check if the first two bytes in big-endian match the requested version. + let supported_version = + u16::from_be_bytes(data.as_slice()[..2].try_into().unwrap()); + supported_version == version + }) + .unwrap_or(false) + } + #[inline] fn get_blocks_per_epoch(&self) -> u32 { self.blocks_per_batch * self.batches_per_epoch as u32 diff --git a/primitives/transaction/src/historic_transaction.rs b/primitives/transaction/src/historic_transaction.rs index 0d68b2e1a2..6e7a1fb43d 100644 --- a/primitives/transaction/src/historic_transaction.rs +++ b/primitives/transaction/src/historic_transaction.rs @@ -197,8 +197,9 @@ impl HistoricTransaction { new_epoch_slot_range, }), }), - Inherent::FinalizeBatch => {} - Inherent::FinalizeEpoch => {} + Inherent::FinalizeBatch + | Inherent::FinalizeEpoch + | Inherent::VersionUpgrade { .. } => {} } } @@ -219,6 +220,7 @@ impl HistoricTransaction { Inherent::Jail { .. } => Some(()), Inherent::FinalizeBatch => None, Inherent::FinalizeEpoch => None, + Inherent::VersionUpgrade { .. } => None, }) .count() + equivocation_locator.len() diff --git a/primitives/transaction/src/inherent.rs b/primitives/transaction/src/inherent.rs index 35857092c8..22ef791464 100644 --- a/primitives/transaction/src/inherent.rs +++ b/primitives/transaction/src/inherent.rs @@ -39,6 +39,8 @@ pub enum Inherent { FinalizeBatch, /// Emitted only on epoch finalization. FinalizeEpoch, + /// Emitted on version upgrades. + VersionUpgrade { new_version: u16 }, } impl Inherent { @@ -48,7 +50,8 @@ impl Inherent { Inherent::Penalize { .. } | Inherent::Jail { .. } | Inherent::FinalizeBatch - | Inherent::FinalizeEpoch => &Policy::STAKING_CONTRACT_ADDRESS, + | Inherent::FinalizeEpoch + | Inherent::VersionUpgrade { .. } => &Policy::STAKING_CONTRACT_ADDRESS, } } } diff --git a/test-utils/src/block_production.rs b/test-utils/src/block_production.rs index 03635d7ba8..aa93cfe347 100644 --- a/test-utils/src/block_production.rs +++ b/test-utils/src/block_production.rs @@ -183,6 +183,7 @@ impl TemporaryBlockProducer { blockchain.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0, extra_data, + None, ) .unwrap(); diff --git a/test-utils/src/blockchain.rs b/test-utils/src/blockchain.rs index 27b1d3fc62..605e084335 100644 --- a/test-utils/src/blockchain.rs +++ b/test-utils/src/blockchain.rs @@ -87,6 +87,7 @@ pub fn produce_macro_blocks_with_txns( blockchain.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0u32, vec![], + None, ) .unwrap(); diff --git a/test-utils/src/blockchain_with_rng.rs b/test-utils/src/blockchain_with_rng.rs index e88385dcca..7bed519173 100644 --- a/test-utils/src/blockchain_with_rng.rs +++ b/test-utils/src/blockchain_with_rng.rs @@ -28,6 +28,7 @@ pub fn produce_macro_blocks_with_rng( blockchain.timestamp() + next_block_height * 1000, 0u32, vec![], + None, rng, ) .unwrap(); diff --git a/test-utils/src/performance/blockchain-push/main.rs b/test-utils/src/performance/blockchain-push/main.rs index abe0c86897..bec32f2318 100644 --- a/test-utils/src/performance/blockchain-push/main.rs +++ b/test-utils/src/performance/blockchain-push/main.rs @@ -89,6 +89,7 @@ fn main() { blockchain.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0u32, vec![], + None, ) .unwrap(); diff --git a/validator/src/tendermint.rs b/validator/src/tendermint.rs index 11efc3c75f..5304c44ad7 100644 --- a/validator/src/tendermint.rs +++ b/validator/src/tendermint.rs @@ -228,11 +228,52 @@ 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. + let version = if blockchain.state().current_version() + 1 == Policy::MAX_SUPPORTED_VERSION { + let staking_contract = blockchain + .get_staking_contract_if_complete(None) + .expect("Staking Contract must be complete to create a macro proposal"); + let data_store = blockchain.get_staking_contract_store(); + let txn = blockchain.read_transaction(); + let data_store_read = &data_store.read(&txn); + + // Calculate support for upgrade. + let support_check = + |data| Policy::supports_upgrade(data, Policy::MAX_SUPPORTED_VERSION); + let supporting_stake = + staking_contract.get_supporting_stake(data_store_read, support_check); + let total_stake = staking_contract.balance; + let supporting_slots = staking_contract.get_supporting_slots( + data_store_read, + &blockchain + .current_validators() + .expect("There need to be validators present"), + support_check, + ); + + // 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. + if supporting_slots >= Policy::TWO_F_PLUS_ONE + && u64::from(supporting_stake) * 100 / u64::from(total_stake) + >= Policy::UPGRADE_MIN_SUPPORT as u64 + { + Some(Policy::MAX_SUPPORTED_VERSION) + } else { + None + } + } else { + None + }; + // Create the proposal. + // TODO: Version let time = blockchain.time.now(); let block = self .block_producer - .next_macro_block_proposal(&blockchain, time, round, vec![]) + .next_macro_block_proposal(&blockchain, time, round, vec![], version) .map_err(|_| ProtocolError::Abort)?; // Always `Some(…)` because the above function always sets it to `Some(…)`. diff --git a/validator/tests/tendermint.rs b/validator/tests/tendermint.rs index d35925cf5e..d4b27783f7 100644 --- a/validator/tests/tendermint.rs +++ b/validator/tests/tendermint.rs @@ -44,7 +44,13 @@ async fn it_verifies_inferior_chain_proposals() { let b = blockchain1.read(); temp_producer1 .producer - .next_macro_block_proposal(&b, b.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0, vec![]) + .next_macro_block_proposal( + &b, + b.timestamp() + Policy::BLOCK_SEPARATION_TIME, + 0, + vec![], + None, + ) .unwrap() }; // Use the skip block to rebranch the micro blocks away, effectively leaving the proposal on an inferior chain. @@ -74,7 +80,13 @@ async fn it_verifies_inferior_chain_proposals() { let b = blockchain1.read(); temp_producer1 .producer - .next_macro_block_proposal(&b, b.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0, vec![]) + .next_macro_block_proposal( + &b, + b.timestamp() + Policy::BLOCK_SEPARATION_TIME, + 0, + vec![], + None, + ) .unwrap() }; @@ -100,7 +112,13 @@ async fn it_verifies_inferior_chain_proposals() { let b = blockchain1.read(); temp_producer1 .producer - .next_macro_block_proposal(&b, b.timestamp() + Policy::BLOCK_SEPARATION_TIME, 0, vec![]) + .next_macro_block_proposal( + &b, + b.timestamp() + Policy::BLOCK_SEPARATION_TIME, + 0, + vec![], + None, + ) .unwrap() };