feat(data_structures): enable protocol versions injection through config

also: make protocol versioning safer

config/src/config.rs

@@ -38,16 +38,20 @@
 //! // Default config for mainnet
 //! // Config::from_partial(&PartialConfig::default_mainnet());
 //! ```
-use std::convert::TryFrom;
 use std::{
-    collections::HashSet, fmt, marker::PhantomData, net::SocketAddr, path::PathBuf, time::Duration,
+    array::IntoIter, collections::HashSet, convert::TryFrom, fmt, marker::PhantomData,
+    net::SocketAddr, path::PathBuf, time::Duration,
 };
 
-use partial_struct::PartialStruct;
 use serde::{de, Deserialize, Deserializer, Serialize};
+
+use partial_struct::PartialStruct;
 use witnet_crypto::hash::HashFunction;
-use witnet_data_structures::chain::{ConsensusConstants, Environment, PartialConsensusConstants};
-use witnet_data_structures::witnessing::WitnessingConfig;
+use witnet_data_structures::{
+    chain::{ConsensusConstants, Environment, Epoch, PartialConsensusConstants},
+    proto::versioning::ProtocolVersion,
+    witnessing::WitnessingConfig,
+};
 use witnet_protected::ProtectedString;
 
 use crate::{
@@ -125,6 +129,11 @@ pub struct Config {
     #[partial_struct(ty = "PartialWitnessing")]
     #[partial_struct(serde(default))]
     pub witnessing: Witnessing,
+
+    /// Configuration related with protocol versions
+    #[partial_struct(skip)]
+    #[partial_struct(serde(default))]
+    pub protocol: Protocol,
 }
 
 /// Log-specific configuration.
@@ -420,6 +429,25 @@ pub struct Tapi {
     pub oppose_wip0027: bool,
 }
 
+/// Configuration related to protocol versions.
+#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
+pub struct Protocol {
+    pub v1_7: Option<Epoch>,
+    pub v1_8: Option<Epoch>,
+    pub v2_0: Option<Epoch>,
+}
+
+impl Protocol {
+    pub fn iter(&self) -> IntoIter<(ProtocolVersion, Option<Epoch>), 3> {
+        [
+            (ProtocolVersion::V1_7, self.v1_7),
+            (ProtocolVersion::V1_8, self.v1_8),
+            (ProtocolVersion::V2_0, self.v2_0),
+        ]
+        .into_iter()
+    }
+}
+
 fn to_partial_consensus_constants(c: &ConsensusConstants) -> PartialConsensusConstants {
     PartialConsensusConstants {
         checkpoint_zero_timestamp: Some(c.checkpoint_zero_timestamp),
@@ -450,6 +478,13 @@ fn to_partial_consensus_constants(c: &ConsensusConstants) -> PartialConsensusCon
     }
 }
 
+pub trait Partializable {
+    type Partial;
+
+    fn from_partial(config: &Self::Partial, defaults: &dyn Defaults) -> Self;
+    fn to_partial(&self) -> Self::Partial;
+}
+
 impl Config {
     pub fn from_partial(config: &PartialConfig) -> Self {
         let defaults: &dyn Defaults = match config.environment {
@@ -478,6 +513,7 @@ impl Config {
             mempool: Mempool::from_partial(&config.mempool, defaults),
             tapi: config.tapi.clone(),
             witnessing: Witnessing::from_partial(&config.witnessing, defaults),
+            protocol: Protocol::from_partial(&config.protocol, defaults),
         }
     }
 
@@ -496,6 +532,7 @@ impl Config {
             mempool: self.mempool.to_partial(),
             tapi: self.tapi.clone(),
             witnessing: self.witnessing.to_partial(),
+            protocol: self.protocol.to_partial(),
         }
     }
 }
@@ -1171,6 +1208,30 @@ impl Witnessing {
     }
 }
 
+impl Partializable for Protocol {
+    type Partial = Self;
+
+    fn from_partial(config: &Self::Partial, defaults: &dyn Defaults) -> Self {
+        let defaults = defaults.protocol_versions();
+
+        Protocol {
+            v1_7: config
+                .v1_7
+                .or(defaults.get(&ProtocolVersion::V1_7).copied()),
+            v1_8: config
+                .v1_8
+                .or(defaults.get(&ProtocolVersion::V1_8).copied()),
+            v2_0: config
+                .v2_0
+                .or(defaults.get(&ProtocolVersion::V2_0).copied()),
+        }
+    }
+
+    fn to_partial(&self) -> Self::Partial {
+        self.clone()
+    }
+}
+
 // Serialization helpers
 
 fn as_log_filter_string<S>(

config/src/defaults.rs

@@ -2,13 +2,18 @@
 //!
 //! This module contains per-environment default values for the Witnet
 //! protocol params.
-use std::collections::HashSet;
-use std::net::{IpAddr, Ipv4Addr, SocketAddr};
-use std::path::PathBuf;
-use std::time::Duration;
+use std::{
+    collections::{HashMap, HashSet},
+    net::{IpAddr, Ipv4Addr, SocketAddr},
+    path::PathBuf,
+    time::Duration,
+};
 
 use witnet_crypto::hash::HashFunction;
-use witnet_data_structures::chain::Hash;
+use witnet_data_structures::{
+    chain::{Epoch, Hash},
+    proto::versioning::ProtocolVersion,
+};
 use witnet_protected::ProtectedString;
 
 // When changing the defaults, remember to update the documentation!
@@ -475,6 +480,10 @@ pub trait Defaults {
     fn mempool_max_reinserted_transactions(&self) -> u32 {
         100
     }
+
+    fn protocol_versions(&self) -> HashMap<ProtocolVersion, Epoch> {
+        [(ProtocolVersion::V1_7, 0)].into_iter().collect()
+    }
 }
 
 /// Allow setting a reward to collateral percentage for a data request to be included in a block

data_structures/Cargo.toml

@@ -33,6 +33,8 @@ rand = "0.8.5"
 serde = { version = "1.0.104", features = ["derive"] }
 serde_cbor = "0.11.1"
 serde_json = "1.0.48"
+strum = "0.25.0"
+strum_macros = "0.25.3"
 vrf = "0.2.3"
 
 witnet_crypto = { path = "../crypto" }

data_structures/src/chain/mod.rs

@@ -1569,50 +1569,74 @@ pub struct KeyedSignature {
 }
 
 impl KeyedSignature {
-    pub fn from_recoverable_hex(string: &str, msg: &[u8]) -> Self {
-        // FIXME: make this safe by using a `Result` instead of unwrapping
-        let bytes = hex::decode(string).unwrap();
+    pub fn from_recoverable_hex(
+        string: &str,
+        msg: &[u8],
+    ) -> Result<Self, Secp256k1ConversionError> {
+        let bytes = hex::decode(string).map_err(|e| Secp256k1ConversionError::HexDecode {
+            hex: String::from(string),
+            inner: e,
+        })?;
 
         Self::from_recoverable_slice(&bytes, msg)
     }
-    pub fn from_recoverable(recoverable: &RecoverableSignature, message: &[u8]) -> Self {
-        // FIXME: make this safe by using a `Result` instead of unwrapping
-        let msg = secp256k1::Message::from_digest_slice(message).unwrap();
+    pub fn from_recoverable(
+        recoverable: &RecoverableSignature,
+        message: &[u8],
+    ) -> Result<Self, Secp256k1ConversionError> {
+        let msg = secp256k1::Message::from_digest_slice(message)
+            .map_err(|e| Secp256k1ConversionError::Secp256k1 { inner: e })?;
         let signature = recoverable.to_standard();
-        let public_key = recoverable.recover(&msg).unwrap();
+        let public_key = recoverable
+            .recover(&msg)
+            .map_err(|e| Secp256k1ConversionError::Secp256k1 { inner: e })?;
 
-        KeyedSignature {
+        Ok(KeyedSignature {
             signature: signature.into(),
             public_key: public_key.into(),
-        }
+        })
     }
 
     // Recovers a keyed signature from its serialized form and a known message.
-    pub fn from_recoverable_slice(compact: &[u8], message: &[u8]) -> Self {
-        // FIXME: make this safe by using a `Result` instead of unwrapping
-        let recid = RecoveryId::from_i32(compact[0] as i32).unwrap();
-        let recoverable = RecoverableSignature::from_compact(&compact[1..], recid).unwrap();
+    pub fn from_recoverable_slice(
+        compact: &[u8],
+        message: &[u8],
+    ) -> Result<Self, Secp256k1ConversionError> {
+        let recid = RecoveryId::from_i32(compact[0] as i32)
+            .map_err(|e| Secp256k1ConversionError::Secp256k1 { inner: e })?;
+        let recoverable = RecoverableSignature::from_compact(&compact[1..], recid)
+            .map_err(|e| Secp256k1ConversionError::Secp256k1 { inner: e })?;
 
         Self::from_recoverable(&recoverable, message)
     }
 
     /// Serializes a `KeyedSignature` into a compact encoding form that contains the public key recovery ID as a prefix.
-    pub fn to_recoverable_bytes(self, message: &[u8]) -> [u8; 65] {
-        // FIXME: make this safe by using a `Result` instead of unwrapping
+    pub fn to_recoverable_bytes(
+        self,
+        message: &[u8],
+    ) -> Result<[u8; 65], Secp256k1ConversionError> {
         let mut recoverable_bytes = [0; 65];
-        recoverable_bytes[1..].clone_from_slice(&self.signature.to_bytes().unwrap());
-
+        let bytes = self
+            .signature
+            .to_bytes()
+            .map_err(|e| Secp256k1ConversionError::Other {
+                inner: e.to_string(),
+            })?;
+        recoverable_bytes[1..].clone_from_slice(&bytes);
+
+        // Silly algorithm that tries recovery with different recovery IDs in an attempt to guess which one is correct,
+        // provided that our `KeyedSignature` misses that information in comparison with `RecoverableSignature`
         for i in 0..4 {
             recoverable_bytes[0] = i;
 
-            let recovered = KeyedSignature::from_recoverable_slice(&recoverable_bytes, message);
+            let recovered = KeyedSignature::from_recoverable_slice(&recoverable_bytes, message)?;
 
             if recovered.public_key == self.public_key {
                 break;
             }
         }
 
-        recoverable_bytes
+        Ok(recoverable_bytes)
     }
 }
 
@@ -4925,7 +4949,7 @@ mod tests {
         let secp = Secp256k1::new();
         let secret_key =
             Secp256k1_SecretKey::from_slice(&[0xcd; 32]).expect("32 bytes, within curve order");
-        let msg = Secp256k1_Message::from_slice(&data).unwrap();
+        let msg = Secp256k1_Message::from_digest_slice(&data).unwrap();
         let signature = secp.sign_ecdsa(&msg, &secret_key);
 
         let witnet_signature = Secp256k1Signature::from(signature);
@@ -4946,7 +4970,7 @@ mod tests {
         let secp = Secp256k1::new();
         let secret_key =
             Secp256k1_SecretKey::from_slice(&[0xcd; 32]).expect("32 bytes, within curve order");
-        let msg = Secp256k1_Message::from_slice(&data).unwrap();
+        let msg = Secp256k1_Message::from_digest_slice(&data).unwrap();
         let signature = secp.sign_ecdsa(&msg, &secret_key);
 
         let witnet_signature = Signature::from(signature);

data_structures/src/error.rs

@@ -1,7 +1,9 @@
 //! Error type definitions for the data structure module.
 
 use failure::Fail;
+use hex::FromHexError;
 use std::num::ParseIntError;
+use witnet_crypto::secp256k1;
 
 use crate::chain::{
     DataRequestOutput, Epoch, Hash, HashParseError, OutputPointer, PublicKeyHash, RADType,
@@ -484,7 +486,7 @@ pub enum OutputPointerParseError {
 }
 
 /// The error type for operations on a [`Secp256k1Signature`](Secp256k1Signature)
-#[derive(Debug, PartialEq, Eq, Fail)]
+#[derive(Debug, PartialEq, Fail)]
 pub enum Secp256k1ConversionError {
     #[fail(
         display = "Failed to convert `witnet_data_structures::Signature` into `secp256k1::Signature`"
@@ -503,6 +505,15 @@ pub enum Secp256k1ConversionError {
         display = "Failed to convert `witnet_data_structures::SecretKey` into `secp256k1::SecretKey`"
     )]
     FailSecretKeyConversion,
+    #[fail(
+        display = "Cannot decode a `witnet_data_structures::KeyedSignature` from the allegedly hex-encoded string '{}': {}",
+        hex, inner
+    )]
+    HexDecode { hex: String, inner: FromHexError },
+    #[fail(display = "{}", inner)]
+    Secp256k1 { inner: secp256k1::Error },
+    #[fail(display = "{}", inner)]
+    Other { inner: String },
 }
 
 /// The error type for operations on a [`DataRequestPool`](DataRequestPool)

data_structures/src/lib.rs

@@ -136,20 +136,31 @@ pub fn get_protocol_version(epoch: Option<Epoch>) -> ProtocolVersion {
     }
 }
 
-pub fn register_protocol_version(epoch: Epoch, protocol_version: ProtocolVersion) {
+/// Let the protocol versions controller know about the a protocol version, and its activation epoch.
+pub fn register_protocol_version(protocol_version: ProtocolVersion, epoch: Epoch) {
+    log::debug!(
+        "Registering protocol version {protocol_version}, which enters into force at epoch {epoch}"
+    );
     // This unwrap is safe as long as the lock is not poisoned.
     // The lock can only become poisoned when a writer panics.
     let mut protocol_info = PROTOCOL.write().unwrap();
     protocol_info.register(epoch, protocol_version);
 }
 
 /// Set the protocol version that we are running.
-/// #[cfg(not(test))]
 pub fn set_protocol_version(protocol_version: ProtocolVersion) {
+    // The lock can only become poisoned when a writer panics.
     let mut protocol = PROTOCOL.write().unwrap();
     protocol.current_version = protocol_version;
 }
 
+/// Refresh the protocol version, i.e. derive the current version from the current epoch, and update `current_version`
+/// accordingly.
+pub fn refresh_protocol_version(current_epoch: Epoch) {
+    let current_version = get_protocol_version(Some(current_epoch));
+    set_protocol_version(current_version)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -170,9 +181,9 @@ mod tests {
         assert_eq!(version, ProtocolVersion::V1_7);
 
         // Register the different protocol versions
-        register_protocol_version(100, ProtocolVersion::V1_7);
-        register_protocol_version(200, ProtocolVersion::V1_8);
-        register_protocol_version(300, ProtocolVersion::V2_0);
+        register_protocol_version(ProtocolVersion::V1_7, 100);
+        register_protocol_version(ProtocolVersion::V1_8, 200);
+        register_protocol_version(ProtocolVersion::V2_0, 300);
 
         // The initial protocol version should be the default one
         let version = get_protocol_version(Some(0));