Merge branch 'tomas/fix-init-chain-validator-set' (#1686)

* origin/tomas/fix-init-chain-validator-set:
  improve error handling for `set_initial_validators`
  changelog: add #1686
  app/ledger/finalize_block: refactor validator set update for re-use
  app/ledger/init_chain: get genesis validator set using the new fn
  pos: add a function to get genesis validator consensus set for TM

.changelog/unreleased/bug-fixes/1686-delete-prefix.md

@@ -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))

apps/src/lib/node/ledger/shell/finalize_block.rs

@@ -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;
@@ -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>
@@ -640,45 +638,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

apps/src/lib/node/ledger/shell/init_chain.rs

@@ -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::{
@@ -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;
@@ -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
@@ -467,7 +463,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
@@ -476,20 +472,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: {}.",
@@ -510,21 +501,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)
     }
 }
 

apps/src/lib/node/ledger/shell/mod.rs

@@ -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,
 };
@@ -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>>

proof_of_stake/src/lib.rs

@@ -2529,6 +2529,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()
+}
+
 /// 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.