AZ-318 Compatibility with old justification format. (#204)

* Compatibility with old justification format.

* Commented code.

* Clippy.

* Added tests

* Review comments.

* Fix comment in tests.

* Remove v1 modules.

finality-aleph/src/crypto.rs

@@ -18,6 +18,12 @@ pub enum Error {
 #[derive(PartialEq, Eq, Clone, Debug, Decode, Encode)]
 pub struct Signature(AuthoritySignature);
 
+impl From<AuthoritySignature> for Signature {
+    fn from(authority_signature: AuthoritySignature) -> Signature {
+        Signature(authority_signature)
+    }
+}
+
 /// Ties an authority identification and a cryptography keystore together for use in
 /// signing that requires an authority.
 #[derive(Clone)]
@@ -172,6 +178,19 @@ impl MultiKeychain for KeyBox {
     }
 }
 
+/// Old format of signatures, needed for backwards compatibility.
+#[derive(PartialEq, Eq, Clone, Debug, Decode, Encode)]
+pub(crate) struct SignatureV1 {
+    pub(crate) _id: NodeIndex,
+    pub(crate) sgn: AuthoritySignature,
+}
+
+impl From<SignatureV1> for Signature {
+    fn from(sig_v1: SignatureV1) -> Signature {
+        Signature(sig_v1.sgn)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

finality-aleph/src/import.rs

@@ -1,11 +1,12 @@
 use crate::{
-    justification::{AlephJustification, JustificationNotification},
+    justification::{
+        backwards_compatible_decode, JustificationDecoding, JustificationNotification,
+    },
     metrics::{Checkpoint, Metrics},
 };
 use aleph_primitives::ALEPH_ENGINE_ID;
-use codec::Decode;
 use futures::channel::mpsc::{TrySendError, UnboundedSender};
-use log::debug;
+use log::{debug, warn};
 use sc_client_api::backend::Backend;
 use sc_consensus::{
     BlockCheckParams, BlockImport, BlockImportParams, ImportResult, JustificationImport,
@@ -30,6 +31,7 @@ where
     _phantom: PhantomData<Be>,
 }
 
+#[derive(Debug)]
 enum SendJustificationError<Block>
 where
     Block: BlockT,
@@ -64,16 +66,23 @@ where
         number: NumberFor<Block>,
         justification: Justification,
     ) -> Result<(), SendJustificationError<Block>> {
-        debug!(target: "afa", "Importing justification for block #{:?}", number);
+        debug!(target: "afa", "Importing justification for block {:?}", number);
         if justification.0 != ALEPH_ENGINE_ID {
             return Err(SendJustificationError::Consensus(Box::new(
                 ConsensusError::ClientImport("Aleph can import only Aleph justifications.".into()),
             )));
         }
         let justification_raw = justification.1;
-
-        let aleph_justification = AlephJustification::decode(&mut &*justification_raw)
-            .map_err(|_| SendJustificationError::Decode)?;
+        let aleph_justification = match backwards_compatible_decode(justification_raw) {
+            JustificationDecoding::V1(just) => {
+                debug!(target: "afa", "Justification for block {:?} decoded correctly as V1", number);
+                just.into()
+            }
+            JustificationDecoding::V2(just) => just,
+            JustificationDecoding::Err => {
+                return Err(SendJustificationError::Decode);
+            }
+        };
 
         self.justification_tx
             .unbounded_send(JustificationNotification {
@@ -146,13 +155,12 @@ where
         if let Some(justification) =
             justifications.and_then(|just| just.into_justification(ALEPH_ENGINE_ID))
         {
-            debug!(target: "afa", "Got justification along imported block #{:?}", number);
+            debug!(target: "afa", "Got justification along imported block {:?}", number);
 
-            if self
-                .send_justification(post_hash, number, (ALEPH_ENGINE_ID, justification))
-                .is_err()
+            if let Err(e) =
+                self.send_justification(post_hash, number, (ALEPH_ENGINE_ID, justification))
             {
-                debug!(target: "afa", "Some issue with justification");
+                warn!(target: "afa", "Error while receiving justification for block {:?}: {:?}", post_hash, e);
             }
         }
 
@@ -192,6 +200,7 @@ where
                 )),
                 SendJustificationError::Consensus(e) => *e,
                 SendJustificationError::Decode => {
+                    warn!(target: "afa", "Justification for block {:?} decoded incorrectly", number);
                     ConsensusError::ClientImport(String::from("Could not decode justification"))
                 }
             })

finality-aleph/src/justification.rs

@@ -1,13 +1,13 @@
 use crate::{
-    crypto::{AuthorityVerifier, Signature},
+    crypto::{AuthorityVerifier, Signature, SignatureV1},
     finalization::finalize_block,
     last_block_of_session,
     metrics::Checkpoint,
     network, session_id_from_block_num, Metrics, SessionId, SessionMap, SessionPeriod,
 };
-use aleph_bft::SignatureSet;
+use aleph_bft::{PartialMultisignature, SignatureSet};
 use aleph_primitives::ALEPH_ENGINE_ID;
-use codec::{Decode, Encode};
+use codec::{Decode, DecodeAll, Encode};
 use futures::{channel::mpsc, StreamExt};
 use futures_timer::Delay;
 use log::{debug, error, warn};
@@ -23,7 +23,7 @@ use std::{
 use tokio::time::timeout;
 
 /// A proof of block finality, currently in the form of a sufficiently long list of signatures.
-#[derive(Clone, Encode, Decode, Debug)]
+#[derive(Clone, Encode, Decode, Debug, PartialEq)]
 pub struct AlephJustification {
     pub(crate) signature: SignatureSet<Signature>,
 }
@@ -245,3 +245,41 @@ where
         ))
     }
 }
+
+/// Old format of justifications, needed for backwards compatibility.
+#[derive(Clone, Encode, Decode, Debug, PartialEq)]
+pub(crate) struct AlephJustificationV1 {
+    pub(crate) signature: SignatureSet<SignatureV1>,
+}
+
+impl From<AlephJustificationV1> for AlephJustification {
+    fn from(just_v1: AlephJustificationV1) -> AlephJustification {
+        let size = just_v1.signature.size();
+        let just_drop_id: SignatureSet<Signature> = just_v1
+            .signature
+            .into_iter()
+            .fold(SignatureSet::with_size(size), |sig_set, (id, sgn)| {
+                sig_set.add_signature(&sgn.into(), id)
+            });
+        AlephJustification {
+            signature: just_drop_id,
+        }
+    }
+}
+
+#[derive(Clone, Debug, PartialEq)]
+pub(crate) enum JustificationDecoding {
+    V1(AlephJustificationV1),
+    V2(AlephJustification),
+    Err,
+}
+
+pub(crate) fn backwards_compatible_decode(justification_raw: Vec<u8>) -> JustificationDecoding {
+    if let Ok(justification) = AlephJustification::decode_all(&justification_raw) {
+        JustificationDecoding::V2(justification)
+    } else if let Ok(justification) = AlephJustificationV1::decode_all(&justification_raw) {
+        JustificationDecoding::V1(justification)
+    } else {
+        JustificationDecoding::Err
+    }
+}