fix(node): fix DR eligibility for (allegedly) all protocol versions

fix #2527

data_structures/src/data_request.rs

@@ -4,21 +4,20 @@ use std::{
 };
 
 use itertools::izip;
-
 use serde::{Deserialize, Serialize};
 
-use crate::proto::versioning::ProtocolVersion;
+use witnet_crypto::hash::calculate_sha256;
+
 use crate::{
     chain::{
-        tapi::ActiveWips, DataRequestInfo, DataRequestOutput, DataRequestStage, DataRequestState,
-        Epoch, Hash, Hashable, PublicKeyHash, ValueTransferOutput,
+        DataRequestInfo, DataRequestOutput, DataRequestStage, DataRequestState, Epoch,
+        Hash, Hashable, PublicKeyHash, tapi::ActiveWips, ValueTransferOutput,
     },
     error::{DataRequestError, TransactionError},
-    get_protocol_version,
+    proto::versioning::{ProtocolVersion, VersionedHashable},
     radon_report::{RadonReport, Stage, TypeLike},
     transaction::{CommitTransaction, DRTransaction, RevealTransaction, TallyTransaction},
 };
-use witnet_crypto::hash::calculate_sha256;
 
 /// Pool of active data requests
 #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
@@ -137,7 +136,7 @@ impl DataRequestPool {
         data_request: DRTransaction,
         block_hash: &Hash,
     ) -> Result<(), failure::Error> {
-        let dr_hash = data_request.hash();
+        let dr_hash = data_request.versioned_hash(ProtocolVersion::from_epoch(epoch));
         if data_request.signatures.is_empty() {
             return Err(TransactionError::SignatureNotFound.into());
         }
@@ -157,11 +156,13 @@ impl DataRequestPool {
     /// Add a commit to the corresponding data request
     fn add_commit(
         &mut self,
+        epoch: Epoch,
         pkh: PublicKeyHash,
         commit: CommitTransaction,
         block_hash: &Hash,
     ) -> Result<(), failure::Error> {
-        let tx_hash = commit.hash();
+        let tx_hash = commit.versioned_hash(ProtocolVersion::from_epoch(epoch));
+        log::debug!("Adding commit tx {}: {:?}", tx_hash, commit);
         // For a commit output, we need to get the corresponding data request input
         let dr_pointer = commit.body.dr_pointer;
 
@@ -182,11 +183,12 @@ impl DataRequestPool {
     /// Add a reveal transaction
     fn add_reveal(
         &mut self,
+        epoch: Epoch,
         pkh: PublicKeyHash,
         reveal: RevealTransaction,
         block_hash: &Hash,
     ) -> Result<(), failure::Error> {
-        let tx_hash = reveal.hash();
+        let tx_hash = reveal.versioned_hash(ProtocolVersion::from_epoch(epoch));
         // For a commit output, we need to get the corresponding data request input
         let dr_pointer = reveal.body.dr_pointer;
         // The data request must be from a previous block, and must not be timelocked.
@@ -338,20 +340,22 @@ impl DataRequestPool {
     pub fn process_commit(
         &mut self,
         commit_transaction: &CommitTransaction,
+        epoch: Epoch,
         block_hash: &Hash,
     ) -> Result<(), failure::Error> {
         let pkh = PublicKeyHash::from_public_key(&commit_transaction.signatures[0].public_key);
-        self.add_commit(pkh, commit_transaction.clone(), block_hash)
+        self.add_commit(epoch, pkh, commit_transaction.clone(), block_hash)
     }
 
     /// New reveals are added to their respective data requests, updating the stage to tally
     pub fn process_reveal(
         &mut self,
         reveal_transaction: &RevealTransaction,
+        epoch: Epoch,
         block_hash: &Hash,
     ) -> Result<(), failure::Error> {
         let pkh = PublicKeyHash::from_public_key(&reveal_transaction.signatures[0].public_key);
-        self.add_reveal(pkh, reveal_transaction.clone(), block_hash)
+        self.add_reveal(epoch, pkh, reveal_transaction.clone(), block_hash)
     }
 
     /// New data requests are inserted and wait for commitments
@@ -554,7 +558,7 @@ pub fn data_request_has_too_many_witnesses(
     validator_count: usize,
     epoch: Option<Epoch>,
 ) -> bool {
-    if get_protocol_version(epoch) < ProtocolVersion::V2_0 {
+    if ProtocolVersion::from_epoch_opt(epoch) < ProtocolVersion::V2_0 {
         false
     } else {
         usize::from(dr_output.witnesses) > validator_count / 4
@@ -917,7 +921,7 @@ mod tests {
             vec![KeyedSignature::default()],
         );
 
-        p.process_commit(&commit_transaction, &fake_block_hash)
+        p.process_commit(&commit_transaction, epoch, &fake_block_hash)
             .unwrap();
 
         // And we can also get all the commit pointers from the data request
@@ -967,7 +971,7 @@ mod tests {
             vec![KeyedSignature::default()],
         );
 
-        p.process_reveal(&reveal_transaction, &fake_block_hash)
+        p.process_reveal(&reveal_transaction, 0, &fake_block_hash)
             .unwrap();
 
         assert_eq!(
@@ -1048,7 +1052,7 @@ mod tests {
 
     #[test]
     fn test_from_reveal_to_tally_3_stages_uncompleted() {
-        let (_epoch, fake_block_hash, mut p, dr_pointer) = add_data_requests_with_3_reveal_stages();
+        let (epoch, fake_block_hash, mut p, dr_pointer) = add_data_requests_with_3_reveal_stages();
 
         let commit_transaction = CommitTransaction::new(
             CommitTransactionBody::without_collateral(
@@ -1075,9 +1079,9 @@ mod tests {
             }],
         );
 
-        p.process_commit(&commit_transaction, &fake_block_hash)
+        p.process_commit(&commit_transaction, epoch, &fake_block_hash)
             .unwrap();
-        p.process_commit(&commit_transaction2, &fake_block_hash)
+        p.process_commit(&commit_transaction2, epoch, &fake_block_hash)
             .unwrap();
 
         // Update stages
@@ -1098,7 +1102,7 @@ mod tests {
             vec![KeyedSignature::default()],
         );
 
-        p.process_reveal(&reveal_transaction, &fake_block_hash)
+        p.process_reveal(&reveal_transaction, 0, &fake_block_hash)
             .unwrap();
 
         // Update stages
@@ -1139,7 +1143,7 @@ mod tests {
 
     #[test]
     fn test_from_reveal_to_tally_3_stages_completed() {
-        let (_epoch, fake_block_hash, mut p, dr_pointer) = add_data_requests_with_3_reveal_stages();
+        let (epoch, fake_block_hash, mut p, dr_pointer) = add_data_requests_with_3_reveal_stages();
 
         let commit_transaction = CommitTransaction::new(
             CommitTransactionBody::without_collateral(
@@ -1166,9 +1170,9 @@ mod tests {
             }],
         );
 
-        p.process_commit(&commit_transaction, &fake_block_hash)
+        p.process_commit(&commit_transaction, epoch, &fake_block_hash)
             .unwrap();
-        p.process_commit(&commit_transaction2, &fake_block_hash)
+        p.process_commit(&commit_transaction2, epoch, &fake_block_hash)
             .unwrap();
 
         // Update stages
@@ -1189,7 +1193,7 @@ mod tests {
             vec![KeyedSignature::default()],
         );
 
-        p.process_reveal(&reveal_transaction, &fake_block_hash)
+        p.process_reveal(&reveal_transaction, epoch, &fake_block_hash)
             .unwrap();
 
         // Update stages
@@ -1211,7 +1215,7 @@ mod tests {
                 public_key: pk2,
             }],
         );
-        p.process_reveal(&reveal_transaction2, &fake_block_hash)
+        p.process_reveal(&reveal_transaction2, epoch, &fake_block_hash)
             .unwrap();
 
         // Update stages
@@ -1225,7 +1229,7 @@ mod tests {
 
     #[test]
     fn test_from_reveal_to_tally_3_stages_zero_reveals() {
-        let (_epoch, fake_block_hash, mut p, dr_pointer) = add_data_requests_with_3_reveal_stages();
+        let (epoch, fake_block_hash, mut p, dr_pointer) = add_data_requests_with_3_reveal_stages();
 
         let commit_transaction = CommitTransaction::new(
             CommitTransactionBody::without_collateral(
@@ -1252,9 +1256,9 @@ mod tests {
             }],
         );
 
-        p.process_commit(&commit_transaction, &fake_block_hash)
+        p.process_commit(&commit_transaction, epoch, &fake_block_hash)
             .unwrap();
-        p.process_commit(&commit_transaction2, &fake_block_hash)
+        p.process_commit(&commit_transaction2, epoch, &fake_block_hash)
             .unwrap();
 
         // Update stages
@@ -1315,7 +1319,7 @@ mod tests {
     #[test]
     fn my_claims() {
         // Test the `add_own_reveal` function
-        let (_epoch, fake_block_hash, mut p, dr_pointer) = add_data_requests();
+        let (epoch, fake_block_hash, mut p, dr_pointer) = add_data_requests();
 
         let commit_transaction = CommitTransaction::new(
             CommitTransactionBody::without_collateral(
@@ -1340,7 +1344,7 @@ mod tests {
             Some(&reveal_transaction)
         );
 
-        p.process_commit(&commit_transaction, &fake_block_hash)
+        p.process_commit(&commit_transaction, epoch, &fake_block_hash)
             .unwrap();
 
         // Still in commit stage until we update

data_structures/src/error.rs

@@ -47,7 +47,7 @@ pub enum TransactionError {
     OutputNotFound { output: OutputPointer },
     #[fail(display = "Data Request not found: {}", hash)]
     DataRequestNotFound { hash: Hash },
-    #[fail(display = "Commit transaction has a invalid Proof of Eligibility")]
+    #[fail(display = "Commit transaction has an invalid Proof of Eligibility")]
     InvalidDataRequestPoe,
     #[fail(
         display = "Validator {} is not eligible to commit to a data request",

data_structures/src/proto/versioning.rs

@@ -97,14 +97,22 @@ pub enum ProtocolVersion {
 }
 
 impl ProtocolVersion {
+    #[inline]
     pub fn guess() -> Self {
-        get_protocol_version(None)
+        Self::from_epoch_opt(None)
     }
 
+    #[inline]
     pub fn from_epoch(epoch: Epoch) -> Self {
-        get_protocol_version(Some(epoch))
+        Self::from_epoch_opt(Some(epoch))
+    }
+
+    #[inline]
+    pub fn from_epoch_opt(epoch: Option<Epoch>) -> Self {
+        get_protocol_version(epoch)
     }
 
+
     pub fn next(&self) -> Self {
         match self {
             ProtocolVersion::V1_7 => ProtocolVersion::V1_8,

data_structures/src/vrf.rs

@@ -217,20 +217,21 @@ impl DataRequestEligibilityClaim {
     }
 
     /// Verify a data request eligibility claim for a given vrf message and data request hash
+    /// FIXME: this assumes protocol version 1_7
     pub fn verify(
         &self,
         vrf: &mut VrfCtx,
         vrf_input: CheckpointVRF,
         dr_hash: Hash,
     ) -> Result<Hash, failure::Error> {
-        self.proof
-            .verify(vrf, &VrfMessage::data_request_v1(vrf_input, dr_hash))
-            .map(|v| {
-                let mut sha256 = [0; 32];
-                sha256.copy_from_slice(&v);
+        let vrf_mesage = VrfMessage::data_request_v1(vrf_input, dr_hash);
+        log::debug!("Verifying DR eligibility claim for dr {}.\nProof: {}\nVrf input: {:?}\nVrf message: {}", dr_hash, hex::encode(&self.proof.proof), vrf_input, hex::encode(&vrf_mesage.0));
+        self.proof.verify(vrf, &vrf_mesage).map(|v| {
+            let mut sha256 = [0; 32];
+            sha256.copy_from_slice(&v);
 
-                Hash::SHA256(sha256)
-            })
+            Hash::SHA256(sha256)
+        })
     }
 }
 
@@ -317,4 +318,17 @@ mod tests {
             .unwrap();
         assert!(proof.verify(vrf, vrf_input, dr_pointer2).is_err());
     }
+
+    #[test]
+    pub fn vrf_message_versions() {
+        let vrf_input = CheckpointVRF::default();
+        let dr_hash = Hash::default();
+        let withdrawer = PublicKeyHash::default();
+
+        let VrfMessage(v1_bytes) = VrfMessage::data_request_v1(vrf_input, dr_hash);
+        let VrfMessage(v2_bytes) =
+            VrfMessage::data_request_v2(vrf_input, dr_hash, Some(withdrawer));
+
+        assert_eq!(v1_bytes, v2_bytes[..(v2_bytes.len() - 24)]);
+    }
 }

node/src/actors/chain_manager/mining.rs

@@ -469,9 +469,9 @@ impl ChainManager {
             // distinguish eligibility for stake entries belonging to multiple withdrawers but
             // operated by a single validator acting as a delegatee.
             let vrf_message = if protocol_version >= V2_0 {
-                VrfMessage::data_request_v1(dr_vrf_input, dr_pointer)
-            } else {
                 VrfMessage::data_request_v2(dr_vrf_input, dr_pointer, first_withdrawer_address)
+            } else {
+                VrfMessage::data_request_v1(dr_vrf_input, dr_pointer)
             };
 
             signature_mngr::vrf_prove(vrf_message)