Apply review comments

Author: jpraynaud

mithril-common/src/chain_observer/cli_observer.rs

@@ -47,11 +47,13 @@ impl CardanoCliRunner {
         }
     }
 
-    fn random_out_file() -> PathBuf {
+    fn random_out_file() -> Result<PathBuf, Box<dyn Error + Sync + Send>> {
         let mut rng = rand_core::OsRng;
-        std::env::temp_dir()
-            .join("cardano-cli-runner")
-            .join(format!("{}", rng.next_u64()))
+        let dir = std::env::temp_dir().join("cardano-cli-runner");
+        if !dir.exists() {
+            fs::create_dir_all(&dir)?;
+        }
+        Ok(dir.join(format!("{}.out", rng.next_u64())))
     }
 
     fn command_for_utxo(&self, address: &str, out_file: PathBuf) -> Command {
@@ -136,14 +138,14 @@ impl CardanoCliRunner {
 #[async_trait]
 impl CliRunner for CardanoCliRunner {
     async fn launch_utxo(&self, address: &str) -> Result<String, Box<dyn Error + Sync + Send>> {
-        let out_file = Self::random_out_file();
+        let out_file = Self::random_out_file()?;
         let output = self
             .command_for_utxo(address, out_file.clone())
             .output()
             .await?;
 
         if output.status.success() {
-            Ok(std::str::from_utf8(&output.stdout)?.trim().to_string())
+            Ok(fs::read_to_string(out_file)?.trim().to_string())
         } else {
             let message = String::from_utf8_lossy(&output.stderr);
 

mithril-common/src/chain_observer/model.rs

@@ -22,18 +22,20 @@ pub enum TxDatumError {
 pub struct TxDatum(pub String);
 
 impl TxDatum {
-    /// Retrieves the ith field of the datum with given type
-    pub fn get_field_raw_value(
+    /// Retrieves the nth field of the datum with given type
+    pub fn get_nth_field_by_type(
         &self,
         type_name: &str,
         index: usize,
     ) -> Result<Value, Box<dyn StdError>> {
         let tx_datum_raw = &self.0;
+        // 1- Parse the Utxo raw data to a hashmap
         let v: HashMap<String, Value> = serde_json::from_str(tx_datum_raw).map_err(|e| {
             TxDatumError::InvalidContent(
                 format!("Error: {e:?}, tx datum was = '{tx_datum_raw}'").into(),
             )
         })?;
+        // 2- Convert the 'fields' entry to a vec of json objects
         let fields = v.get("fields").ok_or_else(|| {
             TxDatumError::InvalidContent(
                 format!("Error: missing 'fields' entry, tx datum was = '{tx_datum_raw}'").into(),
@@ -43,6 +45,7 @@ impl TxDatum {
                 format!("Error: 'fields' entry is not correctly structured, tx datum was = '{tx_datum_raw}'").into(),
             )
         })?;
+        // 3- Filter the vec (keep the ones that match the given type), and retrieve the nth entry of this filtered vec
         let field_value = fields
             .iter()
             .filter(|&field| field.get(type_name).is_some())
@@ -76,29 +79,29 @@ mod test {
         assert_eq!(
             "bytes0",
             tx_datum
-                .get_field_raw_value("bytes", 0)
+                .get_nth_field_by_type("bytes", 0)
                 .unwrap()
                 .as_str()
                 .unwrap()
         );
         assert_eq!(
             "bytes1",
             tx_datum
-                .get_field_raw_value("bytes", 1)
+                .get_nth_field_by_type("bytes", 1)
                 .unwrap()
                 .as_str()
                 .unwrap()
         );
         assert_eq!(
             "bytes2",
             tx_datum
-                .get_field_raw_value("bytes", 2)
+                .get_nth_field_by_type("bytes", 2)
                 .unwrap()
                 .as_str()
                 .unwrap()
         );
         tx_datum
-            .get_field_raw_value("bytes", 100)
+            .get_nth_field_by_type("bytes", 100)
             .expect_err("should have returned an error");
     }
 
@@ -108,29 +111,29 @@ mod test {
         assert_eq!(
             0,
             tx_datum
-                .get_field_raw_value("int", 0)
+                .get_nth_field_by_type("int", 0)
                 .unwrap()
                 .as_u64()
                 .unwrap()
         );
         assert_eq!(
             1,
             tx_datum
-                .get_field_raw_value("int", 1)
+                .get_nth_field_by_type("int", 1)
                 .unwrap()
                 .as_u64()
                 .unwrap()
         );
         assert_eq!(
             2,
             tx_datum
-                .get_field_raw_value("int", 2)
+                .get_nth_field_by_type("int", 2)
                 .unwrap()
                 .as_u64()
                 .unwrap()
         );
         tx_datum
-            .get_field_raw_value("int", 100)
+            .get_nth_field_by_type("int", 100)
             .expect_err("should have returned an error");
     }
 }

mithril-common/src/crypto_helper/era.rs

@@ -21,8 +21,7 @@ pub enum EraMarkersVerifierError {
     SignatureVerification(#[from] SignatureError),
 }
 
-/// A protocol Signer that is responsible for signing the
-/// [Certificate](https://mithril.network/doc/mithril/mithril-protocol/certificates#the-certificate-chain-design)
+/// A cryptographic signer that is responsible for signing the [EreMarker]s
 #[derive(Debug, Serialize, Deserialize)]
 pub struct EraMarkersSigner {
     pub(crate) secret_key: EraMarkersVerifierSecretKey,
@@ -84,7 +83,7 @@ impl EraMarkersSigner {
     }
 }
 
-/// An era merkers Verifier
+/// An era markers verifier that checks the authenticity of era markers stored on the chain
 #[derive(Debug, Serialize, Deserialize, Clone)]
 pub struct EraMarkersVerifier {
     pub(crate) verification_key: EraMarkersVerifierVerificationKey,

mithril-common/src/era/adapters/cardano_chain.rs

@@ -28,6 +28,10 @@ pub enum EraMarkersPayloadError {
     #[error("could not deserialize signature: {0}")]
     DeserializeSignature(GeneralError),
 
+    /// Error raised when the signature is missing
+    #[error("could not verify signature: signature is missing")]
+    MissingSignature,
+
     /// Error raised when the signature is invalid
     #[error("could not verify signature: {0}")]
     VerifySignature(GeneralError),
@@ -37,45 +41,57 @@ pub enum EraMarkersPayloadError {
     CreateSignature(GeneralError),
 }
 
+/// Era markers payload
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-struct EraMarkersPayload {
+pub struct EraMarkersPayload {
     markers: Vec<EraMarker>,
-    signature: HexEncodeEraMarkerSignature,
+    signature: Option<HexEncodeEraMarkerSignature>,
 }
 
 impl EraMarkersPayload {
-    fn message_to_sign(&self) -> Result<Vec<u8>, EraMarkersPayloadError> {
+    fn message_to_bytes(&self) -> Result<Vec<u8>, EraMarkersPayloadError> {
         serde_json::to_vec(&self.markers)
             .map_err(|e| EraMarkersPayloadError::SerializeMessage(e.into()))
     }
 
-    fn verify_signature(
+    fn deserialize_signature(&self) -> Result<EraMarkersVerifierSignature, EraMarkersPayloadError> {
+        EraMarkersVerifierSignature::from_bytes(
+            &Vec::from_hex(
+                self.signature
+                    .as_ref()
+                    .ok_or(EraMarkersPayloadError::MissingSignature)?,
+            )
+            .map_err(|e| EraMarkersPayloadError::DeserializeSignature(e.into()))?,
+        )
+        .map_err(|e| EraMarkersPayloadError::DeserializeSignature(e.into()))
+    }
+
+    /// Verify the signature an era markers payload
+    pub fn verify_signature(
         &self,
         verification_key: EraMarkersVerifierVerificationKey,
     ) -> Result<(), EraMarkersPayloadError> {
-        let signature = EraMarkersVerifierSignature::from_bytes(
-            &Vec::from_hex(&self.signature)
-                .map_err(|e| EraMarkersPayloadError::DeserializeSignature(e.into()))?,
-        )
-        .map_err(|e| EraMarkersPayloadError::DeserializeSignature(e.into()))?;
         let markers_verifier = EraMarkersVerifier::from_verification_key(verification_key);
+
         markers_verifier
-            .verify(&self.message_to_sign()?, &signature)
+            .verify(&self.message_to_bytes()?, &self.deserialize_signature()?)
             .map_err(|e| EraMarkersPayloadError::VerifySignature(e.into()))
     }
 
-    #[cfg(any(test, feature = "test_only"))]
-    fn sign(self, signer: &EraMarkersSigner) -> Result<Self, EraMarkersPayloadError> {
+    /// Sign an era markers payload
+    #[allow(dead_code)]
+    pub fn sign(self, signer: &EraMarkersSigner) -> Result<Self, EraMarkersPayloadError> {
         let signature = signer
             .sign(
                 &self
-                    .message_to_sign()
+                    .message_to_bytes()
                     .map_err(|e| EraMarkersPayloadError::CreateSignature(e.into()))?,
             )
             .encode_hex::<String>();
+
         Ok(Self {
             markers: self.markers,
-            signature,
+            signature: Some(signature),
         })
     }
 }
@@ -111,7 +127,7 @@ impl EraReaderAdapter for CardanoChainAdapter {
             .await?;
         let markers_list = tx_datums
             .into_iter()
-            .filter_map(|datum| datum.get_field_raw_value("bytes", 0).ok())
+            .filter_map(|datum| datum.get_nth_field_by_type("bytes", 0).ok())
             .filter_map(|field_value| field_value.as_str().map(|s| s.to_string()))
             .filter_map(|field_value_str| key_decode_hex(&field_value_str).ok())
             .filter_map(|era_markers_payload: EraMarkersPayload| {
@@ -121,6 +137,7 @@ impl EraReaderAdapter for CardanoChainAdapter {
                     .map(|_| era_markers_payload.markers)
             })
             .collect::<Vec<Vec<EraMarker>>>();
+
         Ok(markers_list.first().unwrap_or(&Vec::new()).to_owned())
     }
 }
@@ -154,14 +171,14 @@ mod test {
                 EraMarker::new("thales", Some(Epoch(1))),
                 EraMarker::new("pythagoras", None),
             ],
-            signature: "".to_string(),
+            signature: None,
         };
         let era_marker_payload_2 = EraMarkersPayload {
             markers: vec![
                 EraMarker::new("thales", Some(Epoch(1))),
                 EraMarker::new("pythagoras", Some(Epoch(2))),
             ],
-            signature: "".to_string(),
+            signature: None,
         };
         let mut fake_datums = dummy_tx_datums_from_markers_payload(vec![
             era_marker_payload_1,

mithril-test-lab/mithril-devnet/devnet-mkfiles.sh

@@ -1004,7 +1004,7 @@ done
 NODE_IX=0
 for NODE in ${POOL_NODES}; do
     NODE_ID=$(( $NODE_IX + 1))
-if [ `expr $NODE_IX % 2` == 0 || -z "${WITH_UNCERTIFIED_SIGNERS}" ]; then 
+if [ `expr $NODE_IX % 2` == 0 ] || [ -z "${WITH_UNCERTIFIED_SIGNERS}" ]; then 
     # 50% of signers with key certification
     cat >> ${NODE}/info.json <<EOF
 {