pos: update validator raw hash validation

proof_of_stake/Cargo.toml

@@ -18,5 +18,6 @@ borsh = "0.9.1"
 thiserror = "1.0.30"
 # A fork with state machine testing
 proptest = {git = "https://github.com/heliaxdev/proptest", branch = "tomas/sm", optional = true}
+derivative = "2.2.0"
 
 [dev-dependencies]

proof_of_stake/src/types.rs

@@ -354,6 +354,12 @@ pub enum SlashType {
 )]
 pub struct BasisPoints(u64);
 
+/// Derive Tendermint raw hash from the public key
+pub trait PublicKeyTmRawHash {
+    /// Derive Tendermint raw hash from the public key
+    fn tm_raw_hash(&self) -> String;
+}
+
 impl VotingPower {
     /// Convert token amount into a voting power.
     pub fn from_tokens(tokens: impl Into<u64>, params: &PosParams) -> Self {

proof_of_stake/src/validation.rs

@@ -8,21 +8,22 @@ use std::hash::Hash;
 use std::ops::{Add, AddAssign, Neg, Sub, SubAssign};
 
 use borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
+use derivative::Derivative;
 use thiserror::Error;
 
 use crate::btree_set::BTreeSetShims;
 use crate::epoched::DynEpochOffset;
 use crate::parameters::PosParams;
 use crate::types::{
-    BondId, Bonds, Epoch, Slashes, TotalVotingPowers, Unbonds,
-    ValidatorConsensusKeys, ValidatorSets, ValidatorState, ValidatorStates,
-    ValidatorTotalDeltas, ValidatorVotingPowers, VotingPower, VotingPowerDelta,
-    WeightedValidator,
+    BondId, Bonds, Epoch, PublicKeyTmRawHash, Slashes, TotalVotingPowers,
+    Unbonds, ValidatorConsensusKeys, ValidatorSets, ValidatorState,
+    ValidatorStates, ValidatorTotalDeltas, ValidatorVotingPowers, VotingPower,
+    VotingPowerDelta, WeightedValidator,
 };
 
 #[allow(missing_docs)]
 #[derive(Error, Debug)]
-pub enum Error<Address, TokenChange>
+pub enum Error<Address, TokenChange, PublicKey>
 where
     Address: Display
         + Debug
@@ -36,6 +37,7 @@ where
         + BorshSchema
         + BorshDeserialize,
     TokenChange: Debug + Display,
+    PublicKey: Debug,
 {
     #[error("Unexpectedly missing state value for validator {0}")]
     ValidatorStateIsRequired(Address),
@@ -158,7 +160,7 @@ where
     #[error("Invalid address raw hash update")]
     InvalidRawHashUpdate,
     #[error("Invalid new validator {0}, some fields are missing: {1:?}.")]
-    InvalidNewValidator(Address, NewValidator),
+    InvalidNewValidator(Address, NewValidator<PublicKey>),
     #[error("New validator {0} has not been added to the validator set.")]
     NewValidatorMissingInValidatorSet(Address),
     #[error("Validator set has not been updated for new validators.")]
@@ -241,8 +243,8 @@ where
     ValidatorAddressRawHash {
         /// Raw hash value
         raw_hash: String,
-        /// The address and raw hash derived from it
-        data: Data<(Address, String)>,
+        /// The validator's address
+        data: Data<Address>,
     },
 }
 
@@ -291,14 +293,16 @@ where
 
 /// A new validator account initialized in a transaction, which is used to check
 /// that all the validator's required fields have been written.
-#[derive(Clone, Debug, Default)]
-pub struct NewValidator {
+#[derive(Clone, Debug, Derivative)]
+// https://mcarton.github.io/rust-derivative/latest/Default.html#custom-bound
+#[derivative(Default(bound = ""))]
+pub struct NewValidator<PublicKey> {
     has_state: bool,
-    has_consensus_key: bool,
+    has_consensus_key: Option<PublicKey>,
     has_total_deltas: bool,
     has_voting_power: bool,
     has_staking_reward_address: bool,
-    has_address_raw_hash: bool,
+    has_address_raw_hash: Option<String>,
     voting_power: VotingPower,
 }
 
@@ -309,7 +313,7 @@ pub fn validate<Address, TokenAmount, TokenChange, PublicKey>(
     params: &PosParams,
     changes: Vec<DataUpdate<Address, TokenAmount, TokenChange, PublicKey>>,
     current_epoch: impl Into<Epoch>,
-) -> Vec<Error<Address, TokenChange>>
+) -> Vec<Error<Address, TokenChange, PublicKey>>
 where
     Address: Display
         + Debug
@@ -362,7 +366,8 @@ where
         + BorshDeserialize
         + BorshSerialize
         + BorshSchema
-        + PartialEq,
+        + PartialEq
+        + PublicKeyTmRawHash,
 {
     let current_epoch = current_epoch.into();
     use DataUpdate::*;
@@ -408,7 +413,7 @@ where
         VotingPowerDelta,
     > = HashMap::default();
 
-    let mut new_validators: HashMap<Address, NewValidator> = HashMap::default();
+    let mut new_validators: HashMap<Address, NewValidator<PublicKey>> = HashMap::default();
 
     for change in changes {
         match change {
@@ -493,16 +498,19 @@ where
                         }
                         // The value must be known at pipeline epoch
                         match post.get(pipeline_epoch) {
-                            Some(_) => {}
+                            Some(consensus_key) => {
+                                let validator = new_validators
+                                    .entry(address.clone())
+                                    .or_default();
+                                validator.has_consensus_key =
+                                    Some(consensus_key.clone());
+                            }
                             _ => errors.push(
                                 Error::MissingNewValidatorConsensusKey(
                                     pipeline_epoch.into(),
                                 ),
                             ),
                         }
-                        let validator =
-                            new_validators.entry(address.clone()).or_default();
-                        validator.has_consensus_key = true;
                     }
                     (Some(pre), Some(post)) => {
                         if post.last_update() != current_epoch {
@@ -1231,16 +1239,10 @@ where
             },
             ValidatorAddressRawHash { raw_hash, data } => {
                 match (data.pre, data.post) {
-                    (None, Some((address, expected_raw_hash))) => {
-                        if raw_hash != expected_raw_hash {
-                            errors.push(Error::InvalidAddressRawHash(
-                                raw_hash,
-                                expected_raw_hash,
-                            ))
-                        }
+                    (None, Some(address)) => {
                         let validator =
                             new_validators.entry(address.clone()).or_default();
-                        validator.has_address_raw_hash = true;
+                        validator.has_address_raw_hash = Some(raw_hash);
                     }
                     (pre, post) if pre != post => {
                         errors.push(Error::InvalidRawHashUpdate)
@@ -1641,17 +1643,30 @@ where
                     } = &new_validator;
                     // The new validator must have set all the required fields
                     if !(*has_state
-                        && *has_consensus_key
                         && *has_total_deltas
                         && *has_voting_power
-                        && *has_staking_reward_address
-                        && *has_address_raw_hash)
+                        && *has_staking_reward_address)
                     {
                         errors.push(Error::InvalidNewValidator(
                             address.clone(),
                             new_validator.clone(),
                         ))
                     }
+                    match (has_address_raw_hash, has_consensus_key) {
+                        (Some(raw_hash), Some(consensus_key)) => {
+                            let expected_raw_hash = consensus_key.tm_raw_hash();
+                            if raw_hash != &expected_raw_hash {
+                                errors.push(Error::InvalidAddressRawHash(
+                                    raw_hash.clone(),
+                                    expected_raw_hash,
+                                ))
+                            }
+                        }
+                        _ => errors.push(Error::InvalidNewValidator(
+                            address.clone(),
+                            new_validator.clone(),
+                        )),
+                    }
                     let weighted_validator = WeightedValidator {
                         voting_power: *voting_power,
                         address: address.clone(),

shared/src/ledger/pos/vp.rs

@@ -215,17 +215,6 @@ where
                     .ctx
                     .read_post(key)?
                     .and_then(|bytes| Address::try_from_slice(&bytes[..]).ok());
-                // Find the raw hashes of the addresses
-                let pre = pre.map(|pre| {
-                    let raw_hash =
-                        pre.raw_hash().map(String::from).unwrap_or_default();
-                    (pre, raw_hash)
-                });
-                let post = post.map(|post| {
-                    let raw_hash =
-                        post.raw_hash().map(String::from).unwrap_or_default();
-                    (post, raw_hash)
-                });
                 changes.push(ValidatorAddressRawHash {
                     raw_hash: raw_hash.to_string(),
                     data: Data { pre, post },

shared/src/types/key/common.rs

@@ -4,13 +4,15 @@ use std::fmt::Display;
 use std::str::FromStr;
 
 use borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
+use namada_proof_of_stake::types::PublicKeyTmRawHash;
 #[cfg(feature = "rand")]
 use rand::{CryptoRng, RngCore};
 use serde::{Deserialize, Serialize};
 
 use super::{
-    ed25519, ParsePublicKeyError, ParseSecretKeyError, ParseSignatureError,
-    RefTo, SchemeType, SigScheme as SigSchemeTrait, VerifySigError,
+    ed25519, tm_consensus_key_raw_hash, ParsePublicKeyError,
+    ParseSecretKeyError, ParseSignatureError, RefTo, SchemeType,
+    SigScheme as SigSchemeTrait, VerifySigError,
 };
 
 /// Public key
@@ -275,3 +277,9 @@ impl super::SigScheme for SigScheme {
         }
     }
 }
+
+impl PublicKeyTmRawHash for PublicKey {
+    fn tm_raw_hash(&self) -> String {
+        tm_consensus_key_raw_hash(self)
+    }
+}

shared/src/types/key/mod.rs

@@ -395,6 +395,12 @@ pub mod testing {
         })
     }
 
+    /// Generate an arbitrary [`common::SecretKey`].
+    pub fn arb_common_keypair() -> impl Strategy<Value = common::SecretKey> {
+        arb_keypair::<ed25519::SigScheme>()
+            .prop_map(|keypair| keypair.try_to_sk().unwrap())
+    }
+
     /// Generate a new random [`super::SecretKey`].
     pub fn gen_keypair<S: SigScheme>() -> S::SecretKey {
         let mut rng: ThreadRng = thread_rng();