Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/unreleased/bug-fixes/1686-delete-prefix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- PoS: ensure that the size of genesis validator set
is limited by the `max_validator_slots` parameter.
([\#1686](https://github.com/anoma/namada/pull/1686))
44 changes: 3 additions & 41 deletions apps/src/lib/node/ledger/shell/finalize_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::collections::HashMap;

use data_encoding::HEXUPPER;
use namada::ledger::parameters::storage as params_storage;
use namada::ledger::pos::types::into_tm_voting_power;
use namada::ledger::pos::{namada_proof_of_stake, staking_token_address};
use namada::ledger::storage::EPOCH_SWITCH_BLOCKS_DELAY;
use namada::ledger::storage_api::token::credit_tokens;
Expand Down Expand Up @@ -32,7 +31,6 @@ use super::*;
use crate::facade::tendermint_proto::abci::{
Misbehavior as Evidence, VoteInfo,
};
use crate::facade::tendermint_proto::crypto::PublicKey as TendermintPublicKey;
use crate::node::ledger::shell::stats::InternalStats;

impl<D, H> Shell<D, H>
Expand Down Expand Up @@ -634,45 +632,9 @@ where
/// changes to the validator sets and consensus parameters
fn update_epoch(&mut self, response: &mut shim::response::FinalizeBlock) {
// Apply validator set update
let (current_epoch, _gas) = self.wl_storage.storage.get_current_epoch();
let pos_params =
namada_proof_of_stake::read_pos_params(&self.wl_storage)
.expect("Could not find the PoS parameters");
// TODO ABCI validator updates on block H affects the validator set
// on block H+2, do we need to update a block earlier?
response.validator_updates =
namada_proof_of_stake::validator_set_update_tendermint(
&self.wl_storage,
&pos_params,
current_epoch,
|update| {
let (consensus_key, power) = match update {
ValidatorSetUpdate::Consensus(ConsensusValidator {
consensus_key,
bonded_stake,
}) => {
let power: i64 = into_tm_voting_power(
pos_params.tm_votes_per_token,
bonded_stake,
);
(consensus_key, power)
}
ValidatorSetUpdate::Deactivated(consensus_key) => {
// Any validators that have been dropped from the
// consensus set must have voting power set to 0 to
// remove them from the conensus set
let power = 0_i64;
(consensus_key, power)
}
};
let pub_key = TendermintPublicKey {
sum: Some(key_to_tendermint(&consensus_key).unwrap()),
};
let pub_key = Some(pub_key);
ValidatorUpdate { pub_key, power }
},
)
.expect("Must be able to update validator sets");
response.validator_updates = self
.get_abci_validator_updates(false)
.expect("Must be able to update validator set");
}

/// Calculate the new inflation rate, mint the new tokens to the PoS
Expand Down
44 changes: 13 additions & 31 deletions apps/src/lib/node/ledger/shell/init_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ use std::hash::Hash;
use namada::core::ledger::testnet_pow;
use namada::ledger::eth_bridge::EthBridgeStatus;
use namada::ledger::parameters::{self, Parameters};
use namada::ledger::pos::{
into_tm_voting_power, staking_token_address, PosParams,
};
use namada::ledger::pos::{staking_token_address, PosParams};
use namada::ledger::storage::traits::StorageHasher;
use namada::ledger::storage::{DBIter, DB};
use namada::ledger::storage_api::token::{
Expand All @@ -23,8 +21,6 @@ use namada::types::time::{DateTimeUtc, TimeZone, Utc};
use namada::types::token;

use super::*;
use crate::facade::tendermint_proto::abci;
use crate::facade::tendermint_proto::crypto::PublicKey as TendermintPublicKey;
use crate::facade::tendermint_proto::google::protobuf;
use crate::facade::tower_abci::{request, response};
use crate::wasm_loader;
Expand Down Expand Up @@ -249,11 +245,11 @@ where
&implicit_vp_code_path,
);
// set the initial validators set
Ok(self.set_initial_validators(
self.set_initial_validators(
&staking_token,
genesis.validators,
&genesis.pos_params,
))
)
}

/// Initialize genesis established accounts
Expand Down Expand Up @@ -464,7 +460,7 @@ where
staking_token: &Address,
validators: Vec<genesis::Validator>,
pos_params: &PosParams,
) -> response::InitChain {
) -> Result<response::InitChain> {
let mut response = response::InitChain::default();
// PoS system depends on epoch being initialized. Write the total
// genesis staking token balance to storage after
Expand All @@ -473,20 +469,15 @@ where
pos::init_genesis_storage(
&mut self.wl_storage,
pos_params,
validators
.clone()
.into_iter()
.map(|validator| validator.pos_data),
validators.into_iter().map(|validator| validator.pos_data),
current_epoch,
);

let total_nam =
read_total_supply(&self.wl_storage, staking_token).unwrap();
let total_nam = read_total_supply(&self.wl_storage, staking_token)?;
// At this stage in the chain genesis, the PoS address balance is the
// same as the number of staked tokens
let total_staked_nam =
read_balance(&self.wl_storage, staking_token, &address::POS)
.unwrap();
read_balance(&self.wl_storage, staking_token, &address::POS)?;

tracing::info!(
"Genesis total native tokens: {}.",
Expand All @@ -507,21 +498,12 @@ where
ibc::init_genesis_storage(&mut self.wl_storage);

// Set the initial validator set
for validator in validators {
let mut abci_validator = abci::ValidatorUpdate::default();
let consensus_key: common::PublicKey =
validator.pos_data.consensus_key.clone();
let pub_key = TendermintPublicKey {
sum: Some(key_to_tendermint(&consensus_key).unwrap()),
};
abci_validator.pub_key = Some(pub_key);
abci_validator.power = into_tm_voting_power(
pos_params.tm_votes_per_token,
validator.pos_data.tokens,
);
response.validators.push(abci_validator);
}
response
response.validators = self
.get_abci_validator_updates(true)
.expect("Must be able to set genesis validator set");
debug_assert!(!response.validators.is_empty());

Ok(response)
}
}

Expand Down
54 changes: 54 additions & 0 deletions apps/src/lib/node/ledger/shell/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use namada::ledger::eth_bridge::{EthBridgeQueries, EthereumBridgeConfig};
use namada::ledger::events::log::EventLog;
use namada::ledger::events::Event;
use namada::ledger::gas::BlockGasMeter;
use namada::ledger::pos::into_tm_voting_power;
use namada::ledger::pos::namada_proof_of_stake::types::{
ConsensusValidator, ValidatorSetUpdate,
};
Expand Down Expand Up @@ -1374,6 +1375,59 @@ where
}
false
}

fn get_abci_validator_updates(
&self,
is_genesis: bool,
) -> storage_api::Result<Vec<namada::tendermint_proto::abci::ValidatorUpdate>>
{
use namada::ledger::pos::namada_proof_of_stake;

use crate::facade::tendermint_proto::crypto::PublicKey as TendermintPublicKey;

let (current_epoch, _gas) = self.wl_storage.storage.get_current_epoch();
let pos_params =
namada_proof_of_stake::read_pos_params(&self.wl_storage)
.expect("Could not find the PoS parameters");

let validator_set_update_fn = if is_genesis {
namada_proof_of_stake::genesis_validator_set_tendermint
} else {
namada_proof_of_stake::validator_set_update_tendermint
};

validator_set_update_fn(
&self.wl_storage,
&pos_params,
current_epoch,
|update| {
let (consensus_key, power) = match update {
ValidatorSetUpdate::Consensus(ConsensusValidator {
consensus_key,
bonded_stake,
}) => {
let power: i64 = into_tm_voting_power(
pos_params.tm_votes_per_token,
bonded_stake,
);
(consensus_key, power)
}
ValidatorSetUpdate::Deactivated(consensus_key) => {
// Any validators that have been dropped from the
// consensus set must have voting power set to 0 to
// remove them from the conensus set
let power = 0_i64;
(consensus_key, power)
}
};
let pub_key = TendermintPublicKey {
sum: Some(key_to_tendermint(&consensus_key).unwrap()),
};
let pub_key = Some(pub_key);
ValidatorUpdate { pub_key, power }
},
)
}
}

impl<'a, D, H> From<&'a mut Shell<D, H>>
Expand Down
35 changes: 35 additions & 0 deletions proof_of_stake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2455,6 +2455,41 @@ where
Ok((total, total_active))
}

/// Get the genesis consensus validators stake and consensus key for Tendermint,
/// converted from [`ValidatorSetUpdate`]s using the given function.
pub fn genesis_validator_set_tendermint<S, T>(
storage: &S,
params: &PosParams,
current_epoch: Epoch,
mut f: impl FnMut(ValidatorSetUpdate) -> T,
) -> storage_api::Result<Vec<T>>
where
S: StorageRead,
{
let consensus_validator_handle =
consensus_validator_set_handle().at(&current_epoch);
let iter = consensus_validator_handle.iter(storage)?;

iter.map(|validator| {
let (
NestedSubKey::Data {
key: new_stake,
nested_sub_key: _,
},
address,
) = validator?;
let consensus_key = validator_consensus_key_handle(&address)
.get(storage, current_epoch, params)?
.unwrap();
let converted = f(ValidatorSetUpdate::Consensus(ConsensusValidator {
consensus_key,
bonded_stake: new_stake,
}));
Ok(converted)
})
.collect()
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

So why was this not causing issues at genesis? Is it right that Tendermint thought more validator were in consensus and allowed to submit consensus votes than we wanted to allow according to our own consensus set?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we were giving tendermint it all the validators in genesis even when there were more than max_validator_slots - it doesn't break anything itself, but it's not correct and the ones that are not actually in consensus set in Namada state would have potentially stayed in tendermint consensus set indefinitely

/// Communicate imminent validator set updates to Tendermint. This function is
/// called two blocks before the start of a new epoch because Tendermint
/// validator updates become active two blocks after the updates are submitted.
Expand Down