Skip to content

Commit

Permalink
Remove unsupporting validators after upgrade
Browse files Browse the repository at this point in the history
Produce upgraded blocks after gathering enough support
  • Loading branch information
paberr committed Nov 18, 2024
1 parent 5a48af3 commit 477cfbd
Show file tree
Hide file tree
Showing 14 changed files with 152 additions and 13 deletions.
7 changes: 6 additions & 1 deletion blockchain/src/block_production/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,12 +253,15 @@ impl BlockProducer {
round: u32,
// Extra data for this block.
extra_data: Vec<u8>,
// Version for this block.
version: Option<u16>,
) -> Result<MacroBlock, BlockProducerError> {
self.next_macro_block_proposal_with_rng(
blockchain,
timestamp,
round,
extra_data,
version,
&mut rand::thread_rng(),
)
}
Expand All @@ -277,6 +280,8 @@ impl BlockProducer {
round: u32,
// Extra data for this block.
extra_data: Vec<u8>,
// Version for this block (or current default).
version: Option<u16>,
// The rng seed. We need this parameterized in order to have determinism when running unit tests.
rng: &mut R,
) -> Result<MacroBlock, BlockProducerError> {
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 16 additions & 1 deletion blockchain/src/blockchain/history_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![]);
}
Expand All @@ -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(),
});
}
}
}
}
Expand Down
18 changes: 15 additions & 3 deletions blockchain/src/blockchain/inherents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<Inherent> {
// 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
}
}
5 changes: 5 additions & 0 deletions blockchain/tests/block_production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ fn it_can_produce_macro_blocks() {
bc.timestamp() + Policy::BLOCK_SEPARATION_TIME,
0u32,
vec![],
None,
)
.unwrap();

Expand Down Expand Up @@ -218,6 +219,7 @@ fn it_can_produce_macro_block_after_punishment() {
bc.timestamp() + Policy::BLOCK_SEPARATION_TIME,
0u32,
vec![],
None,
)
.unwrap();

Expand Down Expand Up @@ -268,6 +270,7 @@ fn it_can_produce_macro_block_after_punishment() {
bc.timestamp() + Policy::BLOCK_SEPARATION_TIME,
0u32,
vec![],
None,
)
.unwrap();

Expand Down Expand Up @@ -322,6 +325,7 @@ fn it_can_produce_election_blocks() {
bc.timestamp() + Policy::BLOCK_SEPARATION_TIME,
0u32,
vec![0x42],
None,
)
.unwrap();

Expand Down Expand Up @@ -368,6 +372,7 @@ fn it_can_produce_a_chain_with_txns() {
blockchain.timestamp() + Policy::BLOCK_SEPARATION_TIME,
0u32,
vec![],
None,
)
.unwrap();

Expand Down
17 changes: 16 additions & 1 deletion primitives/account/src/account/staking_contract/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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)
}
}
}

Expand Down Expand Up @@ -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)
}
Expand Down
19 changes: 19 additions & 0 deletions primitives/src/policy.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<Blake2bHash>, 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
Expand Down
6 changes: 4 additions & 2 deletions primitives/transaction/src/historic_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,9 @@ impl HistoricTransaction {
new_epoch_slot_range,
}),
}),
Inherent::FinalizeBatch => {}
Inherent::FinalizeEpoch => {}
Inherent::FinalizeBatch
| Inherent::FinalizeEpoch
| Inherent::VersionUpgrade { .. } => {}
}
}

Expand All @@ -219,6 +220,7 @@ impl HistoricTransaction {
Inherent::Jail { .. } => Some(()),
Inherent::FinalizeBatch => None,
Inherent::FinalizeEpoch => None,
Inherent::VersionUpgrade { .. } => None,
})
.count()
+ equivocation_locator.len()
Expand Down
5 changes: 4 additions & 1 deletion primitives/transaction/src/inherent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ pub enum Inherent {
FinalizeBatch,
/// Emitted only on epoch finalization.
FinalizeEpoch,
/// Emitted on version upgrades.
VersionUpgrade { new_version: u16 },
}

impl Inherent {
Expand All @@ -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,
}
}
}
Expand Down
1 change: 1 addition & 0 deletions test-utils/src/block_production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ impl TemporaryBlockProducer {
blockchain.timestamp() + Policy::BLOCK_SEPARATION_TIME,
0,
extra_data,
None,
)
.unwrap();

Expand Down
1 change: 1 addition & 0 deletions test-utils/src/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ pub fn produce_macro_blocks_with_txns(
blockchain.timestamp() + Policy::BLOCK_SEPARATION_TIME,
0u32,
vec![],
None,
)
.unwrap();

Expand Down
1 change: 1 addition & 0 deletions test-utils/src/blockchain_with_rng.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub fn produce_macro_blocks_with_rng<R: Rng + CryptoRng>(
blockchain.timestamp() + next_block_height * 1000,
0u32,
vec![],
None,
rng,
)
.unwrap();
Expand Down
1 change: 1 addition & 0 deletions test-utils/src/performance/blockchain-push/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ fn main() {
blockchain.timestamp() + Policy::BLOCK_SEPARATION_TIME,
0u32,
vec![],
None,
)
.unwrap();

Expand Down
43 changes: 42 additions & 1 deletion validator/src/tendermint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(…)`.
Expand Down
24 changes: 21 additions & 3 deletions validator/tests/tendermint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
};

Expand All @@ -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()
};

Expand Down

0 comments on commit 477cfbd

Please sign in to comment.