From 2ca0fe9d5749f774a8f1f2406e147e3b82063b5e Mon Sep 17 00:00:00 2001
From: Luis Rubio <l.rubiorod@gmail.com>
Date: Fri, 17 Sep 2021 12:54:37 +0200
Subject: [PATCH] feat: apply suggestions

---
 data_structures/src/data_request.rs        | 23 ++++++++++++----------
 data_structures/src/mainnet_validations.rs |  8 +++++---
 node/src/actors/chain_manager/handlers.rs  |  8 ++++----
 node/src/actors/chain_manager/mining.rs    |  3 +++
 4 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/data_structures/src/data_request.rs b/data_structures/src/data_request.rs
index 478574d2c1..6dcc07ed44 100644
--- a/data_structures/src/data_request.rs
+++ b/data_structures/src/data_request.rs
@@ -67,12 +67,9 @@ impl DataRequestPool {
         self.data_request_pool.get(dr_pointer).map(|dr_state| {
             let mut reveals: Vec<&RevealTransaction> = dr_state.info.reveals.values().collect();
             if active_wips.wip0019() {
+                // As specified in (7) in WIP-0019
                 reveals.sort_unstable_by_key(|reveal| {
-                    let mut bytes_to_hash = vec![];
-                    bytes_to_hash.extend(reveal.body.pkh.hash);
-                    bytes_to_hash.extend(dr_pointer.as_ref());
-
-                    calculate_sha256(bytes_to_hash.as_ref()).0
+                    concatenate_and_hash(&reveal.body.pkh.hash, dr_pointer.as_ref())
                 });
             } else {
                 reveals.sort_unstable_by_key(|reveal| reveal.body.pkh);
@@ -100,12 +97,9 @@ impl DataRequestPool {
                         dr_state.info.reveals.values().cloned().collect();
 
                     if active_wips.wip0019() {
+                        // As specified in (7) in WIP-0019
                         reveals.sort_unstable_by_key(|reveal| {
-                            let mut bytes_to_hash = vec![];
-                            bytes_to_hash.extend(reveal.body.pkh.hash);
-                            bytes_to_hash.extend(dr_pointer.as_ref());
-
-                            calculate_sha256(bytes_to_hash.as_ref()).0
+                            concatenate_and_hash(&reveal.body.pkh.hash, dr_pointer.as_ref())
                         });
                     } else {
                         reveals.sort_unstable_by_key(|reveal| reveal.body.pkh);
@@ -424,6 +418,15 @@ impl DataRequestPool {
     }
 }
 
+/// Concatenate 2 bytes sequences and hash
+fn concatenate_and_hash(a: &[u8], b: &[u8]) -> [u8; 32] {
+    let mut bytes_to_hash = vec![];
+    bytes_to_hash.extend(a);
+    bytes_to_hash.extend(b);
+
+    calculate_sha256(bytes_to_hash.as_ref()).0
+}
+
 /// Return the change that should be returned to the creator of the data request if
 /// some witness fails to commit, fails to reveal, or reports a value out of consensus.
 /// If the change is 0, the change `ValueTransferOutput` should not be created
diff --git a/data_structures/src/mainnet_validations.rs b/data_structures/src/mainnet_validations.rs
index 788c76aed7..6f9e66bff6 100644
--- a/data_structures/src/mainnet_validations.rs
+++ b/data_structures/src/mainnet_validations.rs
@@ -73,10 +73,12 @@ pub fn current_active_wips() -> ActiveWips {
 /// Auxiliary function that returns the current active wips and the WIPs in voting process as actived
 /// It is only used for testing
 pub fn all_wips_active() -> ActiveWips {
-    let mut h = current_active_wips();
-    h.active_wips.insert("WIP0017-0018-0019".to_string(), 0);
+    let mut active_wips = current_active_wips();
+    active_wips
+        .active_wips
+        .insert("WIP0017-0018-0019".to_string(), 0);
 
-    h
+    active_wips
 }
 
 impl TapiEngine {
diff --git a/node/src/actors/chain_manager/handlers.rs b/node/src/actors/chain_manager/handlers.rs
index c16787a0c2..6630d131e5 100644
--- a/node/src/actors/chain_manager/handlers.rs
+++ b/node/src/actors/chain_manager/handlers.rs
@@ -1277,26 +1277,26 @@ impl Handler<BuildDrt> for ChainManager {
             block_epoch: self.current_epoch.unwrap(),
         };
 
-        let mut dro = msg.dro;
+        let mut dr_output = msg.dro;
         // TODO: Remove after WIP-0019 activation
         // Before wip-0019 activation, RadType::HttpGet enum is serialized with a 0.
         // With the new update, position 0 is RadType::Unknown, so to keep backward compatibility,
         // we need to convert HttpGet retrievals to Unknown.
         if !active_wips.wip0019() {
-            for retrieval in &mut dro.data_request.retrieve {
+            for retrieval in &mut dr_output.data_request.retrieve {
                 if retrieval.kind == RADType::HttpGet {
                     retrieval.kind = RADType::Unknown;
                 }
             }
         }
 
-        if let Err(e) = validate_rad_request(&dro.data_request, &active_wips) {
+        if let Err(e) = validate_rad_request(&dr_output.data_request, &active_wips) {
             return Box::pin(actix::fut::err(e));
         }
         let timestamp = u64::try_from(get_timestamp()).unwrap();
         let max_dr_weight = self.consensus_constants().max_dr_weight;
         match transaction_factory::build_drt(
-            dro,
+            dr_output,
             msg.fee,
             &mut self.chain_state.own_utxos,
             self.own_pkh.unwrap(),
diff --git a/node/src/actors/chain_manager/mining.rs b/node/src/actors/chain_manager/mining.rs
index f17bad7b8e..3db144cc58 100644
--- a/node/src/actors/chain_manager/mining.rs
+++ b/node/src/actors/chain_manager/mining.rs
@@ -143,6 +143,9 @@ impl ChainManager {
             active_wips: self.chain_state.tapi_engine.wip_activation.clone(),
             block_epoch: current_epoch,
         };
+
+        // TODO: Remove after WIP-0019 activation
+        // It is only used to include the active_wips in the extra validation of the rad_requests
         let active_wips = Arc::new(active_wips);
         let active_wips2 = active_wips.clone();