fix(eligibility): calculate target hash for witnessing eligibility

Author: drcpu-github

node/src/actors/chain_manager/mining.rs

@@ -382,7 +382,7 @@ impl ChainManager {
                 collateral_age
             };
             let (target_hash, probability) = if protocol_version >= V2_0 {
-                let eligibility = self
+                let (eligibility, target_hash, probability) = self
                     .chain_state
                     .stakes
                     .witnessing_eligibility(own_pkh, current_epoch, num_witnesses, round)
@@ -397,8 +397,8 @@ impl ChainManager {
                         return Ok(());
                     }
                 }
-                // Using a target hash of zero for V2_X essentially disables preliminary VRF checks
-                (Hash::min(), 0f64)
+
+                (target_hash, probability)
             } else {
                 calculate_reppoe_threshold(
                     rep_eng,
@@ -474,36 +474,31 @@ impl ChainManager {
                         )
                     })
                         .map(move |(vrf_proof, vrf_proof_hash)| {
-                            // This is where eligibility is verified for several protocol versions
-                            if protocol_version >= V2_0 {
-                                Ok(vrf_proof)
-                            }  else {
-                                // invalid: vrf_hash > target_hash
-                                let proof_invalid = vrf_proof_hash > target_hash;
+                            // invalid: vrf_hash > target_hash
+                            let proof_invalid = vrf_proof_hash > target_hash;
 
-                                log::debug!(
+                            log::debug!(
                                 "{} witnesses and {} backup witnesses",
                                 num_witnesses,
                                 num_backup_witnesses
                             );
-                                log::debug!(
+                            log::debug!(
                                 "Probability to be eligible for this data request: {:.6}%",
                                 probability * 100.0
                             );
-                                log::trace!("[DR] Target hash: {}", target_hash);
-                                log::trace!("[DR] Our proof:   {}", vrf_proof_hash);
-                                if proof_invalid {
-                                    log::debug!("No eligibility for data request {}", dr_pointer);
-                                    Err(())
-                                } else {
-                                    log::info!(
+                            log::trace!("[DR] Target hash: {}", target_hash);
+                            log::trace!("[DR] Our proof:   {}", vrf_proof_hash);
+                            if proof_invalid {
+                                log::debug!("No eligibility for data request {}", dr_pointer);
+                                Err(())
+                            } else {
+                                log::info!(
                                     "{} Discovered eligibility for mining a data request {} for epoch #{}",
                                     Yellow.bold().paint("[Mining]"),
                                     Yellow.bold().paint(dr_pointer.to_string()),
                                     Yellow.bold().paint(current_epoch.to_string())
                                 );
-                                    Ok(vrf_proof)
-                                }
+                                Ok(vrf_proof)
                             }
                         })
                 )

validations/src/eligibility/current.rs

@@ -4,7 +4,7 @@ use std::{
     ops::{Add, Div, Mul, Rem, Sub},
 };
 
-use witnet_data_structures::{staking::prelude::*, wit::PrecisionLoss};
+use witnet_data_structures::{chain::Hash, staking::prelude::*, wit::PrecisionLoss};
 
 const MINING_REPLICATION_FACTOR: usize = 4;
 const WITNESSING_MAX_ROUNDS: usize = 4;
@@ -71,7 +71,7 @@ where
         epoch: Epoch,
         witnesses: u16,
         round: u16,
-    ) -> StakesResult<Eligible, Address, Coins, Epoch>
+    ) -> StakesResult<(Eligible, Hash, f64), Address, Coins, Epoch>
     where
         ISK: Into<Address>;
 
@@ -87,10 +87,10 @@ where
     where
         ISK: Into<Address>,
     {
-        matches!(
-            self.witnessing_eligibility(validator, epoch, witnesses, round),
-            Ok(Eligible::Yes)
-        )
+        match self.witnessing_eligibility(validator, epoch, witnesses, round) {
+            Ok((eligible, _, _)) => matches!(eligible, Eligible::Yes),
+            Err(_) => false,
+        }
     }
 }
 
@@ -139,7 +139,7 @@ where
         + From<u64>
         + Sum
         + Display,
-    u64: From<Coins> + From<Power>,
+    u64: From<Coins> + From<Power> + Mul<Power, Output = u64> + Div<Power, Output = u64>,
 {
     fn mining_eligibility<ISK>(
         &self,
@@ -180,7 +180,7 @@ where
         epoch: Epoch,
         witnesses: u16,
         round: u16,
-    ) -> StakesResult<Eligible, Address, Coins, Epoch>
+    ) -> StakesResult<(Eligible, Hash, f64), Address, Coins, Epoch>
     where
         ISK: Into<Address>,
     {
@@ -189,44 +189,51 @@ where
             Err(e) => {
                 // Early exit if the stake key does not exist
                 return match e {
-                    StakesError::EntryNotFound { .. } => Ok(IneligibilityReason::NotStaking.into()),
+                    StakesError::EntryNotFound { .. } => {
+                        Ok((IneligibilityReason::NotStaking.into(), Hash::min(), 0.0))
+                    }
                     e => Err(e),
                 };
             }
         };
 
-        let mut rank = self.rank(Capability::Mining, epoch);
-        let rf = 2usize.pow(u32::from(round)) * witnesses as usize;
+        let mut rank = self.rank(Capability::Witnessing, epoch);
+        let (_, max_power) = rank.next().unwrap_or_default();
 
         // Requirement no. 2 from the WIP:
-        //  "the witnessing power of the block proposer is in the `rf / stakers`th quantile among the witnessing powers
-        //  of all the stakers"
-        let stakers = self.stakes_count();
-        let quantile = stakers / MINING_REPLICATION_FACTOR;
-        // TODO: verify if defaulting to 0 makes sense
-        let (_, threshold) = rank.nth(quantile).unwrap_or_default();
-        if power <= threshold {
-            return Ok(IneligibilityReason::InsufficientPower.into());
+        //  "the mining power of the block proposer is in the `rf / stakers`th quantile among the witnessing powers of all
+        //  the stakers"
+        let rf = 2usize.pow(u32::from(round)) * witnesses as usize;
+        let (_, threshold_power) = rank.nth(rf - 2).unwrap_or_default();
+        if power <= threshold_power {
+            return Ok((
+                IneligibilityReason::InsufficientPower.into(),
+                Hash::min(),
+                0.0,
+            ));
         }
 
         // Requirement no. 3 from the WIP:
         //  "the big-endian value of the VRF output is less than
-        //  `max_rounds * own_power / (max_power * (rf - max_rounds) - rf *  threshold_power)`"
-        // TODO: verify if defaulting to 0 makes sense
-        let (_, max_power) = rank.next().unwrap_or_default();
-        let stakers = self.stakes_count();
-        let quantile = stakers / rf;
-        // TODO: verify if defaulting to 0 makes sense
-        let (_, threshold_power) = rank.nth(quantile).unwrap_or_default();
+        //  `max_rounds * own_power / (max_power * (rf - max_rounds) - rf * threshold_power)`"
         let dividend = Power::from(WITNESSING_MAX_ROUNDS as u64) * power;
         let divisor = max_power * Power::from((rf - WITNESSING_MAX_ROUNDS) as u64)
             - Power::from(rf as u64) * threshold_power;
-        let threshold = dividend / divisor;
-        if power <= threshold {
-            return Ok(IneligibilityReason::InsufficientPower.into());
-        }
+        let target_hash = if divisor == Power::from(0) {
+            Hash::with_first_u32(u32::MAX)
+        } else {
+            Hash::with_first_u32(
+                (((u64::MAX / divisor).saturating_mul(dividend.into())) >> 32)
+                    .try_into()
+                    .unwrap(),
+            )
+        };
 
-        Ok(Eligible::Yes)
+        Ok((
+            Eligible::Yes,
+            target_hash,
+            (u64::from(dividend) / divisor) as f64,
+        ))
     }
 }
 
@@ -274,16 +281,20 @@ mod tests {
         let stakes = <Stakes<String, _, _, _>>::with_minimum(100u64);
         let isk = "validator";
 
-        assert_eq!(
-            stakes.witnessing_eligibility(isk, 0, 10, 0),
-            Ok(Eligible::No(IneligibilityReason::NotStaking))
-        );
+        match stakes.witnessing_eligibility(isk, 0, 10, 0) {
+            Ok((eligible, _, _)) => {
+                assert_eq!(eligible, Eligible::No(IneligibilityReason::NotStaking));
+            }
+            Err(_) => assert!(false),
+        }
         assert!(!stakes.witnessing_eligibility_bool(isk, 0, 10, 0));
 
-        assert_eq!(
-            stakes.witnessing_eligibility(isk, 100, 10, 0),
-            Ok(Eligible::No(IneligibilityReason::NotStaking))
-        );
+        match stakes.witnessing_eligibility(isk, 100, 10, 0) {
+            Ok((eligible, _, _)) => {
+                assert_eq!(eligible, Eligible::No(IneligibilityReason::NotStaking));
+            }
+            Err(_) => assert!(false),
+        }
         assert!(!stakes.witnessing_eligibility_bool(isk, 100, 10, 0));
     }
 
@@ -294,16 +305,58 @@ mod tests {
 
         stakes.add_stake(isk, 1_000, 0).unwrap();
 
-        assert_eq!(
-            stakes.witnessing_eligibility(isk, 0, 10, 0),
-            Ok(Eligible::No(IneligibilityReason::InsufficientPower))
-        );
+        match stakes.witnessing_eligibility(isk, 0, 10, 0) {
+            Ok((eligible, _, _)) => {
+                assert_eq!(
+                    eligible,
+                    Eligible::No(IneligibilityReason::InsufficientPower)
+                );
+            }
+            Err(_) => assert!(false),
+        }
         assert!(!stakes.witnessing_eligibility_bool(isk, 0, 10, 0));
 
-        assert_eq!(
-            stakes.witnessing_eligibility(isk, 100, 10, 0),
-            Ok(Eligible::Yes)
-        );
+        match stakes.witnessing_eligibility(isk, 100, 10, 0) {
+            Ok((eligible, _, _)) => {
+                assert_eq!(eligible, Eligible::Yes);
+            }
+            Err(_) => assert!(false),
+        }
         assert!(stakes.witnessing_eligibility_bool(isk, 100, 10, 0));
     }
+
+    #[test]
+    fn test_witnessing_eligibility_target_hash() {
+        let mut stakes = <Stakes<String, _, _, _>>::with_minimum(100u64);
+        let isk_1 = "validator_1";
+        let isk_2 = "validator_2";
+        let isk_3 = "validator_3";
+        let isk_4 = "validator_4";
+
+        stakes.add_stake(isk_1, 10_000_000_000, 0).unwrap();
+        stakes.add_stake(isk_2, 20_000_000_000, 0).unwrap();
+        stakes.add_stake(isk_3, 30_000_000_000, 0).unwrap();
+        stakes.add_stake(isk_4, 40_000_000_000, 0).unwrap();
+
+        match stakes.witnessing_eligibility(isk_1, 0, 2, 0) {
+            // TODO: verify target hash
+            Ok((eligible, _target_hash, _)) => {
+                assert_eq!(
+                    eligible,
+                    Eligible::No(IneligibilityReason::InsufficientPower)
+                );
+            }
+            Err(_) => assert!(false),
+        }
+        assert!(!stakes.witnessing_eligibility_bool(isk_1, 0, 10, 0));
+
+        match stakes.witnessing_eligibility(isk_1, 100, 2, 0) {
+            // TODO: verify target hash
+            Ok((eligible, _target_hash, _)) => {
+                assert_eq!(eligible, Eligible::No(IneligibilityReason::InsufficientPower));
+            }
+            Err(_) => assert!(false),
+        }
+        assert!(stakes.witnessing_eligibility_bool(isk_1, 100, 10, 0));
+    }
 }

validations/src/validations.rs

@@ -684,22 +684,32 @@ pub fn validate_commit_transaction(
     let proof_pkh = co_tx.body.proof.proof.pkh();
 
     // Check if the commit transaction is from an eligible validator
-    if protocol_version >= ProtocolVersion::V2_0 {
-        let eligibility = stakes.witnessing_eligibility(
+    let target_hash_wit2 = if protocol_version >= ProtocolVersion::V2_0 {
+        let target_hash = match stakes.witnessing_eligibility(
             proof_pkh,
             epoch,
             dr_state.data_request.witnesses,
             dr_state.info.current_commit_round,
-        );
-        if eligibility == Ok(Eligible::No(InsufficientPower))
-            || eligibility == Ok(Eligible::No(NotStaking))
-        {
-            return Err(TransactionError::ValidatorNotEligible {
-                validator: proof_pkh,
+        ) {
+            Ok((eligibility, target_hash, _)) => {
+                if eligibility == Eligible::No(InsufficientPower)
+                    || eligibility == Eligible::No(NotStaking)
+                {
+                    return Err(TransactionError::ValidatorNotEligible {
+                        validator: proof_pkh,
+                    }
+                    .into());
+                }
+
+                target_hash
             }
-            .into());
-        }
-    }
+            Err(e) => return Err(e.into()),
+        };
+
+        target_hash
+    } else {
+        Hash::min()
+    };
 
     // Commitment's output is only for change propose, so it only has to be one output and the
     // address has to be the same than the address which creates the commitment
@@ -780,7 +790,7 @@ pub fn validate_commit_transaction(
 
         target_hash
     } else {
-        Hash::max()
+        target_hash_wit2
     };
     add_dr_vrf_signature_to_verify(
         signatures_to_verify,