diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs index ce74e4ffcc..6cee47baec 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -1,24 +1,21 @@ //! Implementation of the `FinalizeBlock` ABCI++ method for the Shell -use namada::ledger::pos::types::into_tm_voting_power; -use namada::ledger::protocol; -use namada::ledger::governance::storage as gov_storage; -use namada::ledger::governance::utils::{ - compute_tally, get_proposal_votes, ProposalEvent, -}; -use namada::ledger::governance::vp::ADDRESS as gov_address; use namada::ledger::inflation::{self, RewardsController}; use namada::ledger::parameters::storage as params_storage; -use namada::ledger::pos::types::{self, decimal_mult_u64, VoteInfo}; +use namada::ledger::pos::types::{ + self, decimal_mult_u64, into_tm_voting_power, VoteInfo, +}; use namada::ledger::pos::{ consensus_validator_set_accumulator_key, staking_token_address, }; +use namada::ledger::protocol; use namada::types::address::Address; #[cfg(feature = "abcipp")] use namada::types::key::tm_raw_hash_to_string; use namada::types::storage::{BlockHash, Epoch, Header}; use namada::types::token::{total_supply_key, Amount}; use rust_decimal::prelude::Decimal; + use super::governance::execute_governance_proposals; use super::*; use crate::facade::tendermint_proto::abci::Misbehavior as Evidence; @@ -302,7 +299,7 @@ where } None => { #[cfg(feature = "abcipp")] - if req.votes.len() == 0 && req.proposer_address.len() > 0 { + if req.votes.is_empty() && !req.proposer_address.is_empty() { // Get proposer address from storage based on the consensus // key hash let tm_raw_hash_string = @@ -414,7 +411,7 @@ where &mut self, current_epoch: Epoch, proposer_address: &Address, - votes: &Vec, + votes: &[VoteInfo], ) { let last_epoch = current_epoch - 1; // Get input values needed for the PD controller for PoS and MASP. @@ -426,7 +423,7 @@ where // block of the previous epoch), which also gives the final // accumulator value updates self.storage - .log_block_rewards(last_epoch, &proposer_address, votes) + .log_block_rewards(last_epoch, proposer_address, votes) .unwrap(); // TODO: review if the appropriate epoch is being used (last vs now) @@ -541,16 +538,12 @@ where let last_epoch = types::Epoch::from(last_epoch.0); // TODO: think about changing the reward to Decimal - let mut reward_tokens_remaining = pos_minted_tokens.clone(); + let mut reward_tokens_remaining = pos_minted_tokens; for (address, value) in accumulators.iter() { - dbg!(reward_tokens_remaining.clone()); // Get reward token amount for this validator let fractional_claim = value / Decimal::from(num_blocks_in_last_epoch); - let reward = decimal_mult_u64( - fractional_claim, - u64::from(pos_minted_tokens), - ); + let reward = decimal_mult_u64(fractional_claim, pos_minted_tokens); // Read epoched validator data and rewards products let validator_deltas = @@ -560,17 +553,15 @@ where let mut rewards_products = self .storage .read_validator_rewards_products(address) - .unwrap_or(std::collections::HashMap::new()); + .unwrap_or_default(); let mut delegation_rewards_products = self .storage .read_validator_delegation_rewards_products(address) - .unwrap_or(std::collections::HashMap::new()); + .unwrap_or_default(); // Get validator data at the last epoch - let stake = validator_deltas - .get(last_epoch) - .map(|sum| Decimal::from(sum)) - .unwrap(); + let stake = + validator_deltas.get(last_epoch).map(Decimal::from).unwrap(); let last_product = *rewards_products.get(&last_epoch).unwrap_or(&Decimal::ONE); let last_delegation_product = *delegation_rewards_products @@ -586,16 +577,23 @@ where + (Decimal::ONE - commission_rate) * Decimal::from(reward) / stake); rewards_products.insert(current_epoch, new_product); - delegation_rewards_products.insert(current_epoch, new_delegation_product); - self.storage.write_validator_rewards_products(address, &rewards_products); - self.storage.write_validator_delegation_rewards_products(address, &delegation_rewards_products); + delegation_rewards_products + .insert(current_epoch, new_delegation_product); + self.storage + .write_validator_rewards_products(address, &rewards_products); + self.storage.write_validator_delegation_rewards_products( + address, + &delegation_rewards_products, + ); reward_tokens_remaining -= reward; - // TODO: Figure out how to deal with round-off to a whole number of tokens. May be tricky. - // TODO: Storing reward products as a Decimal suggests that no round-off should be done here, - // TODO: perhaps only upon withdrawal. But by truncating at withdrawal, may leave tokens in - // TDOD: the PoS account that are not accounted for. Is this an issue? + // TODO: Figure out how to deal with round-off to a whole number of + // tokens. May be tricky. TODO: Storing reward products + // as a Decimal suggests that no round-off should be done here, + // TODO: perhaps only upon withdrawal. But by truncating at + // withdrawal, may leave tokens in TDOD: the PoS account + // that are not accounted for. Is this an issue? } if reward_tokens_remaining > 0 { diff --git a/apps/src/lib/node/ledger/shims/abcipp_shim.rs b/apps/src/lib/node/ledger/shims/abcipp_shim.rs index 336d06cf17..71121362bf 100644 --- a/apps/src/lib/node/ledger/shims/abcipp_shim.rs +++ b/apps/src/lib/node/ledger/shims/abcipp_shim.rs @@ -4,9 +4,9 @@ use std::path::PathBuf; use std::pin::Pin; use std::task::{Context, Poll}; +use futures::future::FutureExt; #[cfg(not(feature = "abcipp"))] use namada::ledger::pos::namada_proof_of_stake::PosBase; -use futures::future::FutureExt; #[cfg(not(feature = "abcipp"))] use namada::types::hash::Hash; #[cfg(not(feature = "abcipp"))] @@ -93,7 +93,7 @@ impl AbcippShim { let resp = match req { Req::ProcessProposal(proposal) => { #[cfg(not(feature = "abcipp"))] - if proposal.proposer_address.len() > 0 { + if !proposal.proposer_address.is_empty() { let tm_raw_hash_string = tm_raw_hash_to_string( proposal.proposer_address.clone(), ); diff --git a/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs b/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs index 3a5afe3de8..048e93bbdf 100644 --- a/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs +++ b/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs @@ -194,9 +194,9 @@ pub mod shim { pub mod request { use std::convert::TryFrom; + use namada::ledger::pos::types::VoteInfo; #[cfg(not(feature = "abcipp"))] use namada::tendermint_proto::abci::RequestBeginBlock; - use namada::ledger::pos::types::VoteInfo; use namada::types::hash::Hash; use namada::types::storage::{BlockHash, Header}; use namada::types::time::DateTimeUtc; diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 7573ba9b83..9d8f76eae7 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -34,17 +34,15 @@ use epoched::{ use parameters::PosParams; use rust_decimal::Decimal; use thiserror::Error; -use types::{ - ActiveValidator, Bonds, CommissionRates, Epoch, GenesisValidator, Slash, - SlashType, Slashes, TotalDeltas, Unbond, Unbonds, ValidatorConsensusKeys, - ValidatorDeltas, ValidatorSet, ValidatorSetUpdate, ValidatorSets, - ValidatorState, ValidatorStates, RewardsProducts, -}; use crate::btree_set::BTreeSetShims; use crate::rewards::PosRewardsCalculator; use crate::types::{ - decimal_mult_i128, decimal_mult_u64, Bond, BondId, VoteInfo, WeightedValidator, + decimal_mult_i128, decimal_mult_u64, ActiveValidator, Bond, BondId, Bonds, + CommissionRates, Epoch, GenesisValidator, RewardsProducts, Slash, + SlashType, Slashes, TotalDeltas, Unbond, Unbonds, ValidatorConsensusKeys, + ValidatorDeltas, ValidatorSet, ValidatorSetUpdate, ValidatorSets, + ValidatorState, ValidatorStates, VoteInfo, WeightedValidator, }; /// Read-only part of the PoS system @@ -707,9 +705,14 @@ pub trait PosBase { key: &Self::Address, ) -> Option; /// Read PoS validator's last known epoch with rewards products - fn read_validator_last_known_product_epoch(&self, key: &Self::Address) -> Epoch; + fn read_validator_last_known_product_epoch( + &self, + key: &Self::Address, + ) -> Epoch; /// Read PoS consensus validator's rewards accumulator - fn read_consensus_validator_rewards_accumulator(&self) -> Option>; + fn read_consensus_validator_rewards_accumulator( + &self, + ) -> Option>; /// Read PoS validator set (active and inactive). fn read_validator_set(&self) -> ValidatorSets; /// Read PoS total deltas of all validators (active and inactive). @@ -1008,7 +1011,7 @@ pub trait PosBase { &mut self, epoch: impl Into, proposer_address: &Self::Address, - votes: &Vec, + votes: &[VoteInfo], ) -> Result<(), InflationError> { // TODO: all values collected here need to be consistent with the same // block that the voting info corresponds to, which is the @@ -1038,8 +1041,11 @@ pub trait PosBase { let mut signer_set: HashSet = HashSet::new(); let mut total_signing_stake: u64 = 0; for vote in votes.iter() { - if !vote.signed_last_block { continue; } - let tm_raw_hash_string = hex::encode_upper(vote.validator_address.clone()); + if !vote.signed_last_block { + continue; + } + let tm_raw_hash_string = + hex::encode_upper(vote.validator_address.clone()); let native_address = self .read_validator_address_raw_hash(tm_raw_hash_string) .expect( @@ -1048,8 +1054,10 @@ pub trait PosBase { ); signer_set.insert(native_address.clone()); - // vote.validator_vp is updating at a constant delay relative to the validator deltas - // use validator deltas in namada protocol to get voting power instead + // vote.validator_vp is updating at a constant delay relative to the + // validator deltas. + // Use validator deltas in namada protocol to get voting power + // instead let deltas = self.read_validator_deltas(&native_address).unwrap(); let stake: Self::TokenChange = deltas.get(epoch).unwrap(); let stake: u64 = Into::::into(stake).try_into().unwrap(); @@ -1075,7 +1083,7 @@ pub trait PosBase { // update the reward accumulators let mut validator_accumulators = self .read_consensus_validator_rewards_accumulator() - .unwrap_or_else(|| HashMap::::new()); + .unwrap_or_default(); for validator in validators.active.iter() { let mut rewards_frac = Decimal::default(); let stake: Decimal = validator.bonded_stake.into(); @@ -1095,8 +1103,11 @@ pub trait PosBase { let active_val_frac = stake / active_val_stake; rewards_frac += coeffs.active_val_coeff * active_val_frac; - let prev_val = *validator_accumulators.get(&validator.address).unwrap_or(&Decimal::ZERO); - validator_accumulators.insert(validator.address.clone(), prev_val + rewards_frac); + let prev_val = *validator_accumulators + .get(&validator.address) + .unwrap_or(&Decimal::ZERO); + validator_accumulators + .insert(validator.address.clone(), prev_val + rewards_frac); } // Write the updated map of reward accumulators back to storage diff --git a/proof_of_stake/src/rewards.rs b/proof_of_stake/src/rewards.rs index ff44beeffc..9b021af613 100644 --- a/proof_of_stake/src/rewards.rs +++ b/proof_of_stake/src/rewards.rs @@ -1,6 +1,7 @@ //! PoS rewards use rust_decimal::Decimal; +use rust_decimal_macros::dec; use thiserror::Error; /// Errors during rewards calculation @@ -55,7 +56,7 @@ impl PosRewardsCalculator { pub fn get_reward_coeffs(&self) -> Result { // TODO: think about possibility of u64 overflow let votes_needed = self.get_min_required_votes(); - if self.signing_stake < votes_needed.into() { + if self.signing_stake < votes_needed { return Err(RewardsError::InsufficentVotes); } @@ -81,8 +82,4 @@ impl PosRewardsCalculator { fn get_min_required_votes(&self) -> u64 { ((2 * self.total_stake) + 3 - 1) / 3 } - - - - } diff --git a/shared/src/ledger/inflation.rs b/shared/src/ledger/inflation.rs index dfecc23105..491e4c2719 100644 --- a/shared/src/ledger/inflation.rs +++ b/shared/src/ledger/inflation.rs @@ -20,6 +20,7 @@ pub enum RewardsType { } /// Holds the PD controller values that should be updated in storage +#[allow(missing_docs)] pub struct ValsToUpdate { pub locked_ratio: Decimal, pub inflation: u64, @@ -41,6 +42,7 @@ pub struct RewardsController { impl RewardsController { /// Initialize a new PD controller + #[allow(clippy::too_many_arguments)] pub fn new( locked_tokens: token::Amount, total_tokens: token::Amount, @@ -95,12 +97,10 @@ impl RewardsController { let last_inflation_amount = Decimal::from(*last_inflation_amount); let inflation = if last_inflation_amount + control_val > max_inflation { max_inflation + } else if last_inflation_amount + control_val > dec!(0.0) { + last_inflation_amount + control_val } else { - if last_inflation_amount + control_val > dec!(0.0) { - last_inflation_amount + control_val - } else { - dec!(0.0) - } + dec!(0.0) }; let inflation: u64 = inflation.to_u64().unwrap(); diff --git a/shared/src/ledger/pos/storage.rs b/shared/src/ledger/pos/storage.rs index a6af069548..46cb8bd905 100644 --- a/shared/src/ledger/pos/storage.rs +++ b/shared/src/ledger/pos/storage.rs @@ -180,9 +180,7 @@ pub fn validator_self_rewards_product_key(validator: &Address) -> Key { } /// Is storage key for validator's self rewards products? -pub fn is_validator_self_rewards_product_key( - key: &Key, -) -> Option<&Address> { +pub fn is_validator_self_rewards_product_key(key: &Key) -> Option<&Address> { match &key.segments[..] { [ DbKeySeg::AddressSeg(addr), @@ -582,8 +580,9 @@ where &self, key: &Self::Address, ) -> rust_decimal::Decimal { - let (value, _gas) = - self.read(&validator_max_commission_rate_change_key(key)).unwrap(); + let (value, _gas) = self + .read(&validator_max_commission_rate_change_key(key)) + .unwrap(); decode(value.unwrap()).unwrap() } @@ -606,8 +605,13 @@ where value.map(|value| decode(value).unwrap()) } - fn read_validator_last_known_product_epoch(&self, key: &Self::Address) -> Epoch { - let (value, _gas) = self.read(&validator_delegation_rewards_product_key(key)).unwrap(); + fn read_validator_last_known_product_epoch( + &self, + key: &Self::Address, + ) -> Epoch { + let (value, _gas) = self + .read(&validator_delegation_rewards_product_key(key)) + .unwrap(); decode(value.unwrap()).unwrap() } @@ -680,15 +684,18 @@ where key: &Self::Address, value: &RewardsProducts, ) { - self.write(&validator_delegation_rewards_product_key(key), encode(value)) - .unwrap(); + self.write( + &validator_delegation_rewards_product_key(key), + encode(value), + ) + .unwrap(); } fn write_validator_last_known_product_epoch( - &mut self, - key: &Self::Address, - value: &Epoch, - ) { + &mut self, + key: &Self::Address, + value: &Epoch, + ) { self.write(&validator_last_known_product_epoch_key(key), encode(value)) .unwrap(); } diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index ff82295911..e885216a34 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -1076,7 +1076,7 @@ dependencies = [ "subtle-encoding", "tendermint", "tendermint-light-client-verifier", - "tendermint-proto", + "tendermint-proto 0.23.6", "tendermint-testgen", "time", "tracing", @@ -1092,7 +1092,7 @@ dependencies = [ "prost", "prost-types", "serde", - "tendermint-proto", + "tendermint-proto 0.23.6", ] [[package]] @@ -1390,7 +1390,7 @@ dependencies = [ "sparse-merkle-tree", "tempfile", "tendermint", - "tendermint-proto", + "tendermint-proto 0.23.6", "thiserror", "tonic-build", "tracing", @@ -1418,10 +1418,11 @@ version = "0.9.0" dependencies = [ "borsh", "derivative", + "hex", "proptest", "rust_decimal", "rust_decimal_macros", - "tendermint-proto", + "tendermint-proto 0.23.5", "thiserror", ] @@ -2361,7 +2362,7 @@ dependencies = [ "signature", "subtle", "subtle-encoding", - "tendermint-proto", + "tendermint-proto 0.23.6", "time", "zeroize", ] @@ -2378,6 +2379,23 @@ dependencies = [ "time", ] +[[package]] +name = "tendermint-proto" +version = "0.23.5" +source = "git+https://github.com/heliaxdev/tendermint-rs?rev=95c52476bc37927218374f94ac8e2a19bd35bec9#95c52476bc37927218374f94ac8e2a19bd35bec9" +dependencies = [ + "bytes", + "flex-error", + "num-derive", + "num-traits", + "prost", + "prost-types", + "serde", + "serde_bytes", + "subtle-encoding", + "time", +] + [[package]] name = "tendermint-proto" version = "0.23.6" diff --git a/wasm_for_tests/wasm_source/Cargo.lock b/wasm_for_tests/wasm_source/Cargo.lock index 2ddcecd325..7c36baa251 100644 --- a/wasm_for_tests/wasm_source/Cargo.lock +++ b/wasm_for_tests/wasm_source/Cargo.lock @@ -1076,7 +1076,7 @@ dependencies = [ "subtle-encoding", "tendermint", "tendermint-light-client-verifier", - "tendermint-proto", + "tendermint-proto 0.23.6", "tendermint-testgen", "time", "tracing", @@ -1092,7 +1092,7 @@ dependencies = [ "prost", "prost-types", "serde", - "tendermint-proto", + "tendermint-proto 0.23.6", ] [[package]] @@ -1390,7 +1390,7 @@ dependencies = [ "sparse-merkle-tree", "tempfile", "tendermint", - "tendermint-proto", + "tendermint-proto 0.23.6", "thiserror", "tonic-build", "tracing", @@ -1418,9 +1418,11 @@ version = "0.9.0" dependencies = [ "borsh", "derivative", + "hex", "proptest", "rust_decimal", "rust_decimal_macros", + "tendermint-proto 0.23.5", "thiserror", ] @@ -2354,7 +2356,7 @@ dependencies = [ "signature", "subtle", "subtle-encoding", - "tendermint-proto", + "tendermint-proto 0.23.6", "time", "zeroize", ] @@ -2371,6 +2373,23 @@ dependencies = [ "time", ] +[[package]] +name = "tendermint-proto" +version = "0.23.5" +source = "git+https://github.com/heliaxdev/tendermint-rs?rev=95c52476bc37927218374f94ac8e2a19bd35bec9#95c52476bc37927218374f94ac8e2a19bd35bec9" +dependencies = [ + "bytes", + "flex-error", + "num-derive", + "num-traits", + "prost", + "prost-types", + "serde", + "serde_bytes", + "subtle-encoding", + "time", +] + [[package]] name = "tendermint-proto" version = "0.23.6"