feat(node): always solve data requests included in a pre-wit/2 block using pre-wit/2 logic

drcpu-github · aesedepece · commit b155e53b72c9 · 2025-01-23T13:30:30.000+01:00
diff --git a/data_structures/src/chain/mod.rs b/data_structures/src/chain/mod.rs
@@ -4836,6 +4836,7 @@ pub enum SignaturesToVerify {
         dr_hash: Hash,
         target_hash: Hash,
         withdrawer: Option<PublicKeyHash>,
+        inclusion_epoch: Option<Epoch>,
     },
     VrfBlock {
         proof: BlockEligibilityClaim,
diff --git a/node/src/actors/chain_manager/mining.rs b/node/src/actors/chain_manager/mining.rs
@@ -320,7 +320,6 @@ impl ChainManager {
             .unwrap_or_default();
         // Retrieve the withdrawer address of the first stake entry in which we are the validator
         let first_withdrawer_address = stake_entries.first().map(|stake| stake.key.withdrawer);
-        let protocol_version = get_protocol_version(self.current_epoch);
         let mut available_stake = totalize_stakes(stake_entries).unwrap_or_default();
 
         // Data Request mining
@@ -358,14 +357,18 @@ impl ChainManager {
                 .data_request_state(&dr_pointer)
                 .map(|dr_state| (dr_pointer, dr_state.clone()))
         }) {
+            // The protocol version under which a data request needs to be resolved is decided
+            // based on the epoch of the block in which the data request was included.
+            let protocol_version = get_protocol_version(Some(dr_state.epoch));
+
             // It's possible a data request requesting too many witnesses are in our local data
             // request pool (e.g., when we failed to validate a proposed block). Do not attempt
             // to solve this data request.
             let validator_count = self.chain_state.stakes.validator_count();
             if data_request_has_too_many_witnesses(
                 &dr_state.data_request,
                 validator_count,
-                Some(current_epoch),
+                Some(dr_state.epoch),
             ) {
                 continue;
             }
@@ -846,7 +849,7 @@ impl ChainManager {
                         let too_many_witnesses = data_request_has_too_many_witnesses(
                             &dr_state.data_request,
                             validator_count,
-                            Some(block_epoch),
+                            Some(dr_state.epoch),
                         );
 
                         // The result of `RunTally` will be published as tally
@@ -883,7 +886,7 @@ impl ChainManager {
                             collateral_minimum,
                             tally_bytes_on_encode_error(),
                             &active_wips_inside_move,
-                            get_protocol_version(Some(block_epoch)),
+                            get_protocol_version(Some(dr_state.epoch)),
                         );
 
                         log::info!(
@@ -1381,7 +1384,7 @@ mod tests {
         PublicKey as Secp256k1_PublicKey, SecretKey as Secp256k1_SecretKey,
     };
     use witnet_crypto::signature::{sign, verify};
-    use witnet_data_structures::{chain::*, strum::IntoEnumIterator, transaction::*, vrf::VrfCtx};
+    use witnet_data_structures::{chain::*, transaction::*, vrf::VrfCtx};
     use witnet_protected::Protected;
     use witnet_validations::validations::validate_block_signature;
 
@@ -1555,9 +1558,7 @@ mod tests {
         let mut signatures_to_verify = vec![];
         assert!(validate_block_signature(&block, &mut signatures_to_verify).is_ok());
 
-        for protocol in ProtocolVersion::iter() {
-            assert!(verify_signatures(signatures_to_verify.clone(), vrf, protocol).is_ok());
-        }
+        assert!(verify_signatures(signatures_to_verify.clone(), vrf).is_ok());
     }
 
     static MILLION_TX_OUTPUT: &str =
diff --git a/node/src/actors/chain_manager/mod.rs b/node/src/actors/chain_manager/mod.rs
@@ -1348,7 +1348,6 @@ impl ChainManager {
 
     fn add_temp_superblock_votes(&mut self, ctx: &mut Context<Self>) {
         let consensus_constants = self.consensus_constants();
-        let protocol = ProtocolVersion::from_epoch_opt(self.current_epoch);
 
         let superblock_period = u32::from(consensus_constants.superblock_period);
 
@@ -1360,12 +1359,9 @@ impl ChainManager {
             }
 
             // Validate secp256k1 signature
-            signature_mngr::verify_signatures(
-                vec![SignaturesToVerify::SuperBlockVote {
-                    superblock_vote: superblock_vote.clone(),
-                }],
-                protocol,
-            )
+            signature_mngr::verify_signatures(vec![SignaturesToVerify::SuperBlockVote {
+                superblock_vote: superblock_vote.clone(),
+            }])
             .map(|res| {
                 res.map_err(|e| {
                     log::error!("Verify superblock vote signature: {}", e);
@@ -1396,7 +1392,6 @@ impl ChainManager {
             self.sm_state
         );
         let consensus_constants = self.consensus_constants();
-        let protocol = ProtocolVersion::from_epoch_opt(self.current_epoch);
 
         let superblock_period = u32::from(consensus_constants.superblock_period);
 
@@ -1410,12 +1405,9 @@ impl ChainManager {
         }
 
         // Validate secp256k1 signature
-        signature_mngr::verify_signatures(
-            vec![SignaturesToVerify::SuperBlockVote {
-                superblock_vote: superblock_vote.clone(),
-            }],
-            protocol,
-        )
+        signature_mngr::verify_signatures(vec![SignaturesToVerify::SuperBlockVote {
+            superblock_vote: superblock_vote.clone(),
+        }])
         .into_actor(self)
         .map_err(|e, _act, _ctx| {
             log::error!("Verify superblock vote signature: {}", e);
@@ -1578,7 +1570,24 @@ impl ChainManager {
                 // than or equal to the current epoch
                 block_epoch: current_epoch,
             };
-            let protocol_version = ProtocolVersion::from_epoch(current_epoch);
+            // For commit transactions, we want to verify their validity using the protocol version at
+            // the time of the data request inclusion. For simplicity's sake when validating signatures,
+            // we use the protocol version of the block epoch.
+            // For all other transactions which can pass through the add_transaction function and which
+            // need the protocol version, we can always use the protocol version of the block epoch.
+            let protocol_version = if let Transaction::Commit(co_tx) = &msg.transaction {
+                if let Some(dr_state) = self
+                    .chain_state
+                    .data_request_pool
+                    .data_request_state(&co_tx.body.dr_pointer)
+                {
+                    ProtocolVersion::from_epoch(dr_state.epoch)
+                } else {
+                    ProtocolVersion::from_epoch(current_epoch)
+                }
+            } else {
+                ProtocolVersion::from_epoch(current_epoch)
+            };
             let collateral_age = self
                 .consensus_constants_wit2
                 .get_collateral_age(&active_wips);
@@ -1612,7 +1621,7 @@ impl ChainManager {
             ))
             .into_actor(self)
             .and_then(move |fee, act, _ctx| {
-                signature_mngr::verify_signatures(signatures_to_verify, protocol_version)
+                signature_mngr::verify_signatures(signatures_to_verify)
                     .map(move |res| res.map(|()| fee))
                     .into_actor(act)
             })
@@ -2147,7 +2156,7 @@ impl ChainManager {
             // Short-circuit if validation failed
             res?;
 
-            signature_mngr::verify_signatures(signatures_to_verify, protocol_version).await
+            signature_mngr::verify_signatures(signatures_to_verify).await
         }
         .into_actor(self)
         .and_then(move |(), act, _ctx| {
@@ -2166,13 +2175,12 @@ impl ChainManager {
                 &active_wips,
                 None,
                 &act.chain_state.stakes,
-                protocol_version,
             );
             async move {
                 // Short-circuit if validation failed
                 let diff = res?;
 
-                signature_mngr::verify_signatures(signatures_to_verify, protocol_version)
+                signature_mngr::verify_signatures(signatures_to_verify)
                     .await
                     .map(|()| diff)
             }
@@ -2935,7 +2943,7 @@ pub fn process_validations(
             replication_factor,
         )?;
         log::trace!("Verifying {} block signatures", signatures_to_verify.len());
-        verify_signatures(signatures_to_verify, vrf_ctx, protocol_version)?;
+        verify_signatures(signatures_to_verify, vrf_ctx)?;
     }
 
     let mut signatures_to_verify = vec![];
@@ -2953,15 +2961,14 @@ pub fn process_validations(
         active_wips,
         transaction_visitor,
         stakes,
-        protocol_version,
     )?;
 
     if !resynchronizing {
         log::trace!(
             "Verifying {} transaction signatures",
             signatures_to_verify.len()
         );
-        verify_signatures(signatures_to_verify, vrf_ctx, protocol_version)?;
+        verify_signatures(signatures_to_verify, vrf_ctx)?;
     }
 
     Ok(utxo_dif)
@@ -3044,6 +3051,10 @@ fn process_wit2_stakes_changes(
 
         let mut total_commit_reward = 0;
         for co_tx in &block.txns.commit_txns {
+            if data_request_is_pre_wit2(data_request_pool, &Transaction::Commit(co_tx.clone())) {
+                continue;
+            }
+
             let commit_pkh = co_tx.body.proof.proof.pkh();
             total_commit_reward +=
                 if let Some(dr_output) = data_request_pool.get_dr_output(&co_tx.body.dr_pointer) {
@@ -3081,6 +3092,10 @@ fn process_wit2_stakes_changes(
         // Add reveal rewards
         let mut total_reveal_reward = 0;
         for re_tx in &block.txns.reveal_txns {
+            if data_request_is_pre_wit2(data_request_pool, &Transaction::Reveal(re_tx.clone())) {
+                continue;
+            }
+
             total_reveal_reward +=
                 if let Some(dr_output) = data_request_pool.get_dr_output(&re_tx.body.dr_pointer) {
                     dr_output.commit_and_reveal_fee
@@ -3101,6 +3116,10 @@ fn process_wit2_stakes_changes(
         }
 
         for ta_tx in &block.txns.tally_txns {
+            if data_request_is_pre_wit2(data_request_pool, &Transaction::Tally(ta_tx.clone())) {
+                continue;
+            }
+
             let (collateral, reward) =
                 if let Some(dr_output) = data_request_pool.get_dr_output(&ta_tx.dr_pointer) {
                     (dr_output.collateral, dr_output.witness_reward)
@@ -3194,6 +3213,10 @@ fn process_wit2_stakes_changes(
 
         // Reset witnessing power
         for co_tx in &block.txns.commit_txns {
+            if data_request_is_pre_wit2(data_request_pool, &Transaction::Commit(co_tx.clone())) {
+                continue;
+            }
+
             let commit_pkh = co_tx.body.proof.proof.pkh();
             log::debug!(
                 "Resetting witnessing age for {} to {}",
@@ -3209,6 +3232,10 @@ fn process_wit2_stakes_changes(
         // Collateral was already reserved, so not returning it results in losing it
         // Reset the age for witnessing power to 10 epochs in the future
         for ta_tx in &block.txns.tally_txns {
+            if data_request_is_pre_wit2(data_request_pool, &Transaction::Tally(ta_tx.clone())) {
+                continue;
+            }
+
             let liar_pkhs: Vec<PublicKeyHash> = ta_tx
                 .out_of_consensus
                 .iter()
@@ -3353,6 +3380,40 @@ fn update_pools(
     rep_info
 }
 
+fn data_request_is_pre_wit2(
+    data_request_pool: &DataRequestPool,
+    transaction: &Transaction,
+) -> bool {
+    // Data requests which were included in a block prior to the activation of wit/2 do
+    // not need to be processed using the wit/2 business logic.
+    match transaction {
+        Transaction::Commit(co_tx) => {
+            if let Some(dr_state) = data_request_pool.data_request_state(&co_tx.body.dr_pointer) {
+                if get_protocol_version(Some(dr_state.epoch)) < ProtocolVersion::V2_0 {
+                    return true;
+                }
+            }
+        }
+        Transaction::Reveal(re_tx) => {
+            if let Some(dr_state) = data_request_pool.data_request_state(&re_tx.body.dr_pointer) {
+                if get_protocol_version(Some(dr_state.epoch)) < ProtocolVersion::V2_0 {
+                    return true;
+                }
+            }
+        }
+        Transaction::Tally(ta_tx) => {
+            if let Some(dr_state) = data_request_pool.data_request_state(&ta_tx.dr_pointer) {
+                if get_protocol_version(Some(dr_state.epoch)) < ProtocolVersion::V2_0 {
+                    return true;
+                }
+            }
+        }
+        _ => (),
+    }
+
+    false
+}
+
 fn separate_honest_errors_and_liars<K, I>(rep_info: I) -> (Vec<K>, Vec<K>, Vec<(K, u32)>)
 where
     I: IntoIterator<Item = (K, RequestResult)>,
@@ -4793,6 +4854,7 @@ mod tests {
         }
 
         // Build pools
+        let epoch = 200;
         let mut data_request_pool = DataRequestPool::default();
         let data_request_state = DataRequestState {
             data_request: DataRequestOutput {
@@ -4810,6 +4872,7 @@ mod tests {
                 ..Default::default()
             },
             stage: DataRequestStage::TALLY,
+            epoch,
             ..Default::default()
         };
         data_request_pool.data_request_pool =
@@ -4818,7 +4881,6 @@ mod tests {
         let unspent_outputs_pool = UnspentOutputsPool::default();
 
         // Test stake changes with a block with a tally with four honests
-        let epoch = 200;
         let tally_transactions = vec![TallyTransaction::new(
             Hash::from(vec![1; 32]), // dr_pointer
             vec![],                  // tally
@@ -4827,7 +4889,7 @@ mod tests {
             vec![],                  // error_committers
         )];
         let block_one_succesful_tally = build_block_with_tally_transactions(
-            epoch,
+            epoch + 3,
             &mut data_request_pool,
             tally_transactions,
             &stakes,
diff --git a/node/src/signature_mngr.rs b/node/src/signature_mngr.rs
@@ -21,7 +21,6 @@ use witnet_data_structures::{
         Bn256KeyedSignature, Bn256PublicKey, Bn256SecretKey, ExtendedSecretKey, Hash, Hashable,
         KeyedSignature, PublicKey, PublicKeyHash, SecretKey, Signature, SignaturesToVerify,
     },
-    proto::versioning::ProtocolVersion,
     transaction::MemoizedHashable,
     vrf::{VrfCtx, VrfMessage, VrfProof},
 };
@@ -145,14 +144,9 @@ pub async fn vrf_prove(message: VrfMessage) -> Result<(VrfProof, Hash), failure:
 }
 
 /// Verify signatures async
-pub async fn verify_signatures(
-    message: Vec<SignaturesToVerify>,
-    protocol: ProtocolVersion,
-) -> Result<(), failure::Error> {
+pub async fn verify_signatures(message: Vec<SignaturesToVerify>) -> Result<(), failure::Error> {
     let addr = SignatureManagerAdapter::from_registry();
-    addr.send(VerifySignatures(message, protocol))
-        .flatten_err()
-        .await
+    addr.send(VerifySignatures(message)).flatten_err().await
 }
 
 #[derive(Debug, Default)]
@@ -194,7 +188,7 @@ struct GetBn256KeyPair;
 
 struct VrfProve(VrfMessage);
 
-struct VerifySignatures(Vec<SignaturesToVerify>, ProtocolVersion);
+struct VerifySignatures(Vec<SignaturesToVerify>);
 
 async fn persist_master_key(master_key: ExtendedSK) -> Result<(), failure::Error> {
     let master_key = ExtendedSecretKey::from(master_key);
@@ -468,13 +462,8 @@ impl Handler<VrfProve> for SignatureManager {
 impl Handler<VerifySignatures> for SignatureManager {
     type Result = <VerifySignatures as Message>::Result;
 
-    fn handle(
-        &mut self,
-        VerifySignatures(signatures, protocol): VerifySignatures,
-        _ctx: &mut Self::Context,
-    ) -> Self::Result {
-        validations::verify_signatures(signatures, self.vrf_ctx.as_mut().unwrap(), protocol)
-            .map(|_| ())
+    fn handle(&mut self, msg: VerifySignatures, _ctx: &mut Self::Context) -> Self::Result {
+        validations::verify_signatures(msg.0, self.vrf_ctx.as_mut().unwrap()).map(|_| ())
     }
 }
 
diff --git a/validations/src/tests/mod.rs b/validations/src/tests/mod.rs
diff --git a/validations/src/validations.rs b/validations/src/validations.rs
