From 7f45cb27bc1eef3a82b10a5a174d13211f6350bb Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 11 Jul 2023 21:00:36 -0400 Subject: [PATCH] Revert "Merge branch 'bengt/pos-cli-queries' (#1656)" This reverts commit 3dd72457ad6f992561d9301b2b7063691187c121, reversing changes made to 119210b1ad9b07184f618e31ff659fae026fc4f5. --- .../improvements/1656-pos-cli-queries.md | 2 - apps/src/bin/namada-client/cli.rs | 16 -- apps/src/lib/cli.rs | 68 +------- apps/src/lib/client/rpc.rs | 156 ++---------------- proof_of_stake/src/lib.rs | 40 +---- shared/src/ledger/args.rs | 11 -- shared/src/ledger/queries/vp/pos.rs | 111 ++++++------- wasm/checksums.json | 2 +- wasm/wasm_source/src/tx_bond.rs | 6 +- wasm/wasm_source/src/tx_unbond.rs | 4 +- 10 files changed, 84 insertions(+), 332 deletions(-) delete mode 100644 .changelog/unreleased/improvements/1656-pos-cli-queries.md diff --git a/.changelog/unreleased/improvements/1656-pos-cli-queries.md b/.changelog/unreleased/improvements/1656-pos-cli-queries.md deleted file mode 100644 index a114c6f2f6..0000000000 --- a/.changelog/unreleased/improvements/1656-pos-cli-queries.md +++ /dev/null @@ -1,2 +0,0 @@ -- Added a client query for `validator-state` and improved the slashes query to - show more info. ([\#1656](https://github.com/anoma/namada/pull/1656)) \ No newline at end of file diff --git a/apps/src/bin/namada-client/cli.rs b/apps/src/bin/namada-client/cli.rs index 60ad4a04c1..dbdb4bc516 100644 --- a/apps/src/bin/namada-client/cli.rs +++ b/apps/src/bin/namada-client/cli.rs @@ -325,22 +325,6 @@ pub async fn main() -> Result<()> { let args = args.to_sdk(&mut ctx); rpc::query_bonded_stake(&client, args).await; } - Sub::QueryValidatorState(QueryValidatorState(mut args)) => { - let client = HttpClient::new(utils::take_config_address( - &mut args.query.ledger_address, - )) - .unwrap(); - wait_until_node_is_synched(&client) - .await - .proceed_or_else(error)?; - let args = args.to_sdk(&mut ctx); - rpc::query_and_print_validator_state( - &client, - &mut ctx.wallet, - args, - ) - .await; - } Sub::QueryCommissionRate(QueryCommissionRate(mut args)) => { let client = HttpClient::new(utils::take_config_address( &mut args.query.ledger_address, diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index e55f79a930..40f459d10a 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -381,7 +381,6 @@ pub mod cmds { QueryProposal(QueryProposal), QueryProposalResult(QueryProposalResult), QueryProtocolParameters(QueryProtocolParameters), - QueryValidatorState(QueryValidatorState), } #[allow(clippy::large_enum_variant)] @@ -1454,27 +1453,6 @@ pub mod cmds { } } - #[derive(Clone, Debug)] - pub struct QueryValidatorState( - pub args::QueryValidatorState, - ); - - impl SubCmd for QueryValidatorState { - const CMD: &'static str = "validator-state"; - - fn parse(matches: &ArgMatches) -> Option { - matches.subcommand_matches(Self::CMD).map(|matches| { - QueryValidatorState(args::QueryValidatorState::parse(matches)) - }) - } - - fn def() -> App { - App::new(Self::CMD) - .about("Query the state of a PoS validator.") - .add_args::>() - } - } - #[derive(Clone, Debug)] pub struct QueryTransfers(pub args::QueryTransfers); @@ -1510,7 +1488,7 @@ pub mod cmds { fn def() -> App { App::new(Self::CMD) - .about("Query a validator's commission rate.") + .about("Query commission rate.") .add_args::>() } } @@ -4064,44 +4042,8 @@ pub mod args { "The validator's address whose bonded stake to query.", )) .arg(EPOCH.def().help( - "The epoch at which to query (corresponding to the last \ - committed block, if not specified).", - )) - } - } - - impl CliToSdk> for QueryValidatorState { - fn to_sdk(self, ctx: &mut Context) -> QueryValidatorState { - QueryValidatorState:: { - query: self.query.to_sdk(ctx), - validator: ctx.get(&self.validator), - epoch: self.epoch, - } - } - } - - impl Args for QueryValidatorState { - fn parse(matches: &ArgMatches) -> Self { - let query = Query::parse(matches); - let validator = VALIDATOR.parse(matches); - let epoch = EPOCH.parse(matches); - Self { - query, - validator, - epoch, - } - } - - fn def(app: App) -> App { - app.add_args::>() - .arg( - VALIDATOR.def().help( - "The validator's address whose state is queried.", - ), - ) - .arg(EPOCH.def().help( - "The epoch at which to query (corresponding to the last \ - committed block, if not specified).", + "The epoch at which to query (last committed, if not \ + specified).", )) } } @@ -4211,8 +4153,8 @@ pub mod args { "The validator's address whose commission rate to query.", )) .arg(EPOCH.def().help( - "The epoch at which to query (corresponding to the last \ - committed block, if not specified).", + "The epoch at which to query (last committed, if not \ + specified).", )) } } diff --git a/apps/src/lib/client/rpc.rs b/apps/src/lib/client/rpc.rs index 6f2be4a9c5..2b0b71650e 100644 --- a/apps/src/lib/client/rpc.rs +++ b/apps/src/lib/client/rpc.rs @@ -1,7 +1,7 @@ //! Client RPC queries use std::cmp::Ordering; -use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; +use std::collections::{HashMap, HashSet}; use std::fs::File; use std::io::{self, Write}; use std::iter::Iterator; @@ -36,7 +36,7 @@ use namada::ledger::rpc::{ }; use namada::ledger::storage::ConversionState; use namada::ledger::wallet::{AddressVpType, Wallet}; -use namada::proof_of_stake::types::{ValidatorState, WeightedValidator}; +use namada::proof_of_stake::types::WeightedValidator; use namada::types::address::{masp, Address}; use namada::types::control_flow::ProceedOrElse; use namada::types::governance::{ @@ -1567,14 +1567,14 @@ pub async fn query_bonded_stake( } None => { let consensus = - unwrap_client_response::>( + unwrap_client_response::>( RPC.vp() .pos() .consensus_validator_set(client, &Some(epoch)) .await, ); let below_capacity = - unwrap_client_response::>( + unwrap_client_response::>( RPC.vp() .pos() .below_capacity_validator_set(client, &Some(epoch)) @@ -1586,7 +1586,7 @@ pub async fn query_bonded_stake( let mut w = stdout.lock(); writeln!(w, "Consensus validators:").unwrap(); - for val in consensus.into_iter().rev() { + for val in consensus { writeln!( w, " {}: {}", @@ -1597,7 +1597,7 @@ pub async fn query_bonded_stake( } if !below_capacity.is_empty() { writeln!(w, "Below capacity validators:").unwrap(); - for val in below_capacity.into_iter().rev() { + for val in &below_capacity { writeln!( w, " {}: {}", @@ -1634,60 +1634,6 @@ pub async fn query_commission_rate< ) } -/// Query and return validator's state -pub async fn query_validator_state< - C: namada::ledger::queries::Client + Sync, ->( - client: &C, - validator: &Address, - epoch: Option, -) -> Option { - unwrap_client_response::>( - RPC.vp() - .pos() - .validator_state(client, validator, &epoch) - .await, - ) -} - -/// Query a validator's state information -pub async fn query_and_print_validator_state< - C: namada::ledger::queries::Client + Sync, ->( - client: &C, - _wallet: &mut Wallet, - args: args::QueryValidatorState, -) { - let validator = args.validator; - let state: Option = - query_validator_state(client, &validator, args.epoch).await; - - match state { - Some(state) => match state { - ValidatorState::Consensus => { - println!("Validator {validator} is in the consensus set") - } - ValidatorState::BelowCapacity => { - println!("Validator {validator} is in the below-capacity set") - } - ValidatorState::BelowThreshold => { - println!("Validator {validator} is in the below-threshold set") - } - ValidatorState::Inactive => { - println!("Validator {validator} is inactive") - } - ValidatorState::Jailed => { - println!("Validator {validator} is jailed") - } - }, - None => println!( - "Validator {validator} is either not a validator, or an epoch \ - before the current epoch has been queried (and the validator \ - state information is no longer stored)" - ), - } -} - /// Query PoS validator's commission rate information pub async fn query_and_print_commission_rate< C: namada::ledger::queries::Client + Sync, @@ -1728,6 +1674,11 @@ pub async fn query_slashes( _wallet: &mut Wallet, args: args::QuerySlashes, ) { + let params_key = pos::params_key(); + let params = query_storage_value::(client, ¶ms_key) + .await + .expect("Parameter should be defined."); + match args.validator { Some(validator) => { let validator = validator; @@ -1736,54 +1687,18 @@ pub async fn query_slashes( RPC.vp().pos().validator_slashes(client, &validator).await, ); if !slashes.is_empty() { - println!("Processed slashes:"); let stdout = io::stdout(); let mut w = stdout.lock(); for slash in slashes { writeln!( w, - "Infraction epoch {}, block height {}, type {}, rate \ - {}", - slash.epoch, - slash.block_height, - slash.r#type, - slash.rate + "Slash epoch {}, type {}, rate {}", + slash.epoch, slash.r#type, slash.rate ) .unwrap(); } } else { - println!( - "No processed slashes found for {}", - validator.encode() - ) - } - // Find enqueued slashes to be processed in the future for the given - // validator - let enqueued_slashes: HashMap< - Address, - BTreeMap>, - > = unwrap_client_response::< - C, - HashMap>>, - >(RPC.vp().pos().enqueued_slashes(client).await); - let enqueued_slashes = enqueued_slashes.get(&validator).cloned(); - if let Some(enqueued) = enqueued_slashes { - println!("\nEnqueued slashes for future processing"); - for (epoch, slashes) in enqueued { - println!("To be processed in epoch {}", epoch); - for slash in slashes { - let stdout = io::stdout(); - let mut w = stdout.lock(); - writeln!( - w, - "Infraction epoch {}, block height {}, type {}", - slash.epoch, slash.block_height, slash.r#type, - ) - .unwrap(); - } - } - } else { - println!("No enqueued slashes found for {}", validator.encode()) + println!("No slashes found for {}", validator.encode()) } } None => { @@ -1795,16 +1710,15 @@ pub async fn query_slashes( if !all_slashes.is_empty() { let stdout = io::stdout(); let mut w = stdout.lock(); - println!("Processed slashes:"); for (validator, slashes) in all_slashes.into_iter() { for slash in slashes { writeln!( w, - "Infraction epoch {}, block height {}, rate {}, \ - type {}, validator {}", + "Slash epoch {}, block height {}, rate {}, type \ + {}, validator {}", slash.epoch, slash.block_height, - slash.rate, + slash.r#type.get_slash_rate(¶ms), slash.r#type, validator, ) @@ -1812,41 +1726,7 @@ pub async fn query_slashes( } } } else { - println!("No processed slashes found") - } - - // Find enqueued slashes to be processed in the future for the given - // validator - let enqueued_slashes: HashMap< - Address, - BTreeMap>, - > = unwrap_client_response::< - C, - HashMap>>, - >(RPC.vp().pos().enqueued_slashes(client).await); - if !enqueued_slashes.is_empty() { - println!("\nEnqueued slashes for future processing"); - for (validator, slashes_by_epoch) in enqueued_slashes { - for (epoch, slashes) in slashes_by_epoch { - println!("\nTo be processed in epoch {}", epoch); - for slash in slashes { - let stdout = io::stdout(); - let mut w = stdout.lock(); - writeln!( - w, - "Infraction epoch {}, block height {}, type \ - {}, validator {}", - slash.epoch, - slash.block_height, - slash.r#type, - validator - ) - .unwrap(); - } - } - } - } else { - println!("\nNo enqueued slashes found for future processing") + println!("No slashes found") } } } diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 538eb105f2..805d79443c 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -756,7 +756,7 @@ where pub fn read_consensus_validator_set_addresses_with_stake( storage: &S, epoch: namada_core::types::storage::Epoch, -) -> storage_api::Result> +) -> storage_api::Result> where S: StorageRead, { @@ -800,7 +800,7 @@ where pub fn read_below_capacity_validator_set_addresses_with_stake( storage: &S, epoch: namada_core::types::storage::Epoch, -) -> storage_api::Result> +) -> storage_api::Result> where S: StorageRead, { @@ -2827,42 +2827,6 @@ where } } -/// Collect the details of all of the enqueued slashes to be processed in future -/// epochs into a nested map -pub fn find_all_enqueued_slashes( - storage: &S, - epoch: Epoch, -) -> storage_api::Result>>> -where - S: StorageRead, -{ - let mut enqueued = HashMap::>>::new(); - for res in enqueued_slashes_handle().get_data_handler().iter(storage)? { - let ( - NestedSubKey::Data { - key: processing_epoch, - nested_sub_key: - NestedSubKey::Data { - key: address, - nested_sub_key: _, - }, - }, - slash, - ) = res?; - if processing_epoch <= epoch { - continue; - } - - let slashes = enqueued - .entry(address) - .or_default() - .entry(processing_epoch) - .or_default(); - slashes.push(slash); - } - Ok(enqueued) -} - /// Find all slashes and the associated validators in the PoS system pub fn find_all_slashes( storage: &S, diff --git a/shared/src/ledger/args.rs b/shared/src/ledger/args.rs index 47117ba594..35081d3283 100644 --- a/shared/src/ledger/args.rs +++ b/shared/src/ledger/args.rs @@ -356,17 +356,6 @@ pub struct QueryBondedStake { pub epoch: Option, } -/// Query the state of a validator (its validator set or if it is jailed) -#[derive(Clone, Debug)] -pub struct QueryValidatorState { - /// Common query args - pub query: Query, - /// Address of a validator - pub validator: C::Address, - /// Epoch in which to find the validator state - pub epoch: Option, -} - #[derive(Clone, Debug)] /// Commission rate change args pub struct CommissionRateChange { diff --git a/shared/src/ledger/queries/vp/pos.rs b/shared/src/ledger/queries/vp/pos.rs index 177b31ed87..d872aa5002 100644 --- a/shared/src/ledger/queries/vp/pos.rs +++ b/shared/src/ledger/queries/vp/pos.rs @@ -1,23 +1,21 @@ //! Queries router and handlers for PoS validity predicate -use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; +use std::collections::{HashMap, HashSet}; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; use namada_core::ledger::storage_api::collections::lazy_map; use namada_core::ledger::storage_api::OptionExt; use namada_proof_of_stake::types::{ BondId, BondsAndUnbondsDetail, BondsAndUnbondsDetails, CommissionPair, - Slash, ValidatorState, WeightedValidator, + Slash, WeightedValidator, }; use namada_proof_of_stake::{ - self, bond_amount, bond_handle, find_all_enqueued_slashes, - find_all_slashes, find_delegation_validators, find_delegations, - read_all_validator_addresses, - read_below_capacity_validator_set_addresses_with_stake, - read_consensus_validator_set_addresses_with_stake, read_pos_params, - read_total_stake, read_validator_max_commission_rate_change, - read_validator_stake, unbond_handle, validator_commission_rate_handle, - validator_slashes_handle, validator_state_handle, + self, below_capacity_validator_set_handle, bond_amount, bond_handle, + consensus_validator_set_handle, find_all_slashes, + find_delegation_validators, find_delegations, read_all_validator_addresses, + read_pos_params, read_total_stake, + read_validator_max_commission_rate_change, read_validator_stake, + unbond_handle, validator_commission_rate_handle, validator_slashes_handle, }; use crate::ledger::queries::types::RequestCtx; @@ -45,17 +43,14 @@ router! {POS, ( "commission" / [validator: Address] / [epoch: opt Epoch] ) -> Option = validator_commission, - - ( "state" / [validator: Address] / [epoch: opt Epoch] ) - -> Option = validator_state, }, ( "validator_set" ) = { ( "consensus" / [epoch: opt Epoch] ) - -> BTreeSet = consensus_validator_set, + -> HashSet = consensus_validator_set, ( "below_capacity" / [epoch: opt Epoch] ) - -> BTreeSet = below_capacity_validator_set, + -> HashSet = below_capacity_validator_set, // TODO: add "below_threshold" }, @@ -87,9 +82,6 @@ router! {POS, ( "bonds_and_unbonds" / [source: opt Address] / [validator: opt Address] ) -> BondsAndUnbondsDetails = bonds_and_unbonds, - ( "enqueued_slashes" ) - -> HashMap>> = enqueued_slashes, - ( "all_slashes" ) -> HashMap> = slashes, ( "is_delegator" / [addr: Address ] / [epoch: opt Epoch] ) -> bool = is_delegator, @@ -211,26 +203,6 @@ where } } -/// Get the validator state -fn validator_state( - ctx: RequestCtx<'_, D, H>, - validator: Address, - epoch: Option, -) -> storage_api::Result> -where - D: 'static + DB + for<'iter> DBIter<'iter> + Sync, - H: 'static + StorageHasher + Sync, -{ - let epoch = epoch.unwrap_or(ctx.wl_storage.storage.last_epoch); - let params = read_pos_params(ctx.wl_storage)?; - let state = validator_state_handle(&validator).get( - ctx.wl_storage, - epoch, - ¶ms, - )?; - Ok(state) -} - /// Get the total stake of a validator at the given epoch or current when /// `None`. The total stake is a sum of validator's self-bonds and delegations /// to their address. @@ -254,29 +226,64 @@ where fn consensus_validator_set( ctx: RequestCtx<'_, D, H>, epoch: Option, -) -> storage_api::Result> +) -> storage_api::Result> where D: 'static + DB + for<'iter> DBIter<'iter> + Sync, H: 'static + StorageHasher + Sync, { let epoch = epoch.unwrap_or(ctx.wl_storage.storage.last_epoch); - read_consensus_validator_set_addresses_with_stake(ctx.wl_storage, epoch) + consensus_validator_set_handle() + .at(&epoch) + .iter(ctx.wl_storage)? + .map(|next_result| { + next_result.map( + |( + lazy_map::NestedSubKey::Data { + key: bonded_stake, + nested_sub_key: _position, + }, + address, + )| { + WeightedValidator { + bonded_stake, + address, + } + }, + ) + }) + .collect() } /// Get all the validator in the below-capacity set with their bonded stake. fn below_capacity_validator_set( ctx: RequestCtx<'_, D, H>, epoch: Option, -) -> storage_api::Result> +) -> storage_api::Result> where D: 'static + DB + for<'iter> DBIter<'iter> + Sync, H: 'static + StorageHasher + Sync, { let epoch = epoch.unwrap_or(ctx.wl_storage.storage.last_epoch); - read_below_capacity_validator_set_addresses_with_stake( - ctx.wl_storage, - epoch, - ) + below_capacity_validator_set_handle() + .at(&epoch) + .iter(ctx.wl_storage)? + .map(|next_result| { + next_result.map( + |( + lazy_map::NestedSubKey::Data { + key: bonded_stake, + nested_sub_key: _position, + }, + address, + )| { + WeightedValidator { + bonded_stake: bonded_stake.into(), + address, + } + }, + ) + }) + .collect() } /// Get the total stake in PoS system at the given epoch or current when `None`. @@ -489,19 +496,7 @@ where find_all_slashes(ctx.wl_storage) } -/// Enqueued slashes -fn enqueued_slashes( - ctx: RequestCtx<'_, D, H>, -) -> storage_api::Result>>> -where - D: 'static + DB + for<'iter> DBIter<'iter> + Sync, - H: 'static + StorageHasher + Sync, -{ - let current_epoch = ctx.wl_storage.storage.last_epoch; - find_all_enqueued_slashes(ctx.wl_storage, current_epoch) -} - -/// Native validator address by looking up the Tendermint address +/// All slashes fn validator_by_tm_addr( ctx: RequestCtx<'_, D, H>, tm_addr: String, diff --git a/wasm/checksums.json b/wasm/checksums.json index c2d3d7e014..8397ffb0d6 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -19,4 +19,4 @@ "vp_token.wasm": "vp_token.ec4f3914d074168f7ecbd43d5587e014381d409b3df4db4f63eaf73d3a56cdd6.wasm", "vp_user.wasm": "vp_user.fd9810232a2ec79ff3f9f8fc70a6427ce9d07a9ef51c1468a785247a9b08b337.wasm", "vp_validator.wasm": "vp_validator.8ce4c52a53aa451459e37ec560aa56ac4083d8829b0c29168c448e1e9d764c22.wasm" -} +} \ No newline at end of file diff --git a/wasm/wasm_source/src/tx_bond.rs b/wasm/wasm_source/src/tx_bond.rs index 9340592bb0..df55270ceb 100644 --- a/wasm/wasm_source/src/tx_bond.rs +++ b/wasm/wasm_source/src/tx_bond.rs @@ -15,7 +15,7 @@ fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { #[cfg(test)] mod tests { - use std::collections::BTreeSet; + use std::collections::HashSet; use namada::ledger::pos::{GenesisValidator, PosParams, PosVP}; use namada::proof_of_stake::{ @@ -132,7 +132,7 @@ mod tests { // Read some data before the tx is executed let mut epoched_total_stake_pre: Vec = Vec::new(); let mut epoched_validator_stake_pre: Vec = Vec::new(); - let mut epoched_validator_set_pre: Vec> = + let mut epoched_validator_set_pre: Vec> = Vec::new(); for epoch in 0..=pos_params.unbonding_len { @@ -163,7 +163,7 @@ mod tests { // Read the data after the tx is executed. let mut epoched_total_stake_post: Vec = Vec::new(); let mut epoched_validator_stake_post: Vec = Vec::new(); - let mut epoched_validator_set_post: Vec> = + let mut epoched_validator_set_post: Vec> = Vec::new(); println!("\nFILLING POST STATE\n"); diff --git a/wasm/wasm_source/src/tx_unbond.rs b/wasm/wasm_source/src/tx_unbond.rs index fc69294b2b..7fb1de8f4f 100644 --- a/wasm/wasm_source/src/tx_unbond.rs +++ b/wasm/wasm_source/src/tx_unbond.rs @@ -15,7 +15,7 @@ fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { #[cfg(test)] mod tests { - use std::collections::BTreeSet; + use std::collections::HashSet; use namada::ledger::pos::{GenesisValidator, PosParams, PosVP}; use namada::proof_of_stake::types::WeightedValidator; @@ -164,7 +164,7 @@ mod tests { let mut epoched_total_stake_pre: Vec = Vec::new(); let mut epoched_validator_stake_pre: Vec = Vec::new(); let mut epoched_bonds_pre: Vec> = Vec::new(); - let mut epoched_validator_set_pre: Vec> = + let mut epoched_validator_set_pre: Vec> = Vec::new(); for epoch in 0..=pos_params.unbonding_len {