diff --git a/Cargo.lock b/Cargo.lock index a6fdb340ba..110554bae3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -93,9 +93,9 @@ dependencies = [ [[package]] name = "aleph-bft" -version = "0.7.0" +version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec7dd13b71eafa058807cfce372c12c4bba794f9e1cd2616468d97188d09afdf" +checksum = "972cc3d0e242dd26caf8f6fa542d79e1cf61e03119b4d28ae682929c8a8eda08" dependencies = [ "async-trait", "bit-vec", @@ -215,9 +215,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.45" +version = "1.0.47" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee10e43ae4a853c0a3591d4e2ada1719e553be18199d9da9d4a83f5927c2f5c7" +checksum = "38d9ff5d688f1c13395289f67db01d4826b46dd694e7580accdc3e8430f2d98e" [[package]] name = "approx" @@ -554,9 +554,9 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitvec" -version = "0.19.5" +version = "0.19.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8942c8d352ae1838c9dda0b0ca2ab657696ef2232a20147cf1b30ae1a9cb4321" +checksum = "55f93d0ef3363c364d5976646a38f04cf67cfe1d4c8d160cdea02cab2c116b33" dependencies = [ "funty", "radium 0.5.3", @@ -912,7 +912,7 @@ checksum = "fa66045b9cb23c2e9c1520732030608b02ee07e5cfaa5a521ec15ded7fa24c90" dependencies = [ "glob", "libc", - "libloading 0.7.1", + "libloading 0.7.2", ] [[package]] @@ -978,9 +978,9 @@ checksum = "b3a71ab494c0b5b860bdc8407ae08978052417070c2ced38573a9157ad75b8ac" [[package]] name = "cpp_demangle" -version = "0.3.3" +version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ea47428dc9d2237f3c6bc134472edfd63ebba0af932e783506dcfd66f10d18a" +checksum = "931ab2a3e6330a07900b8e7ca4e106cdcbb93f2b9a52df55e54ee53d8305b55d" dependencies = [ "cfg-if 1.0.0", ] @@ -1444,9 +1444,9 @@ checksum = "ee2626afccd7561a06cf1367e2950c4718ea04565e20fb5029b6c7d8ad09abcf" [[package]] name = "ed25519" -version = "1.2.0" +version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4620d40f6d2601794401d6dd95a5cf69b6c157852539470eeda433a99b3c0efc" +checksum = "74e1069e39f1454367eb2de793ed062fac4c35c2934b76a81d90dd9abcd28816" dependencies = [ "signature", ] @@ -2943,9 +2943,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.107" +version = "0.2.108" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fbe5e23404da5b4f555ef85ebed98fb4083e55a00c317800bc2a50ede9f3d219" +checksum = "8521a1b57e76b1ec69af7599e75e38e7b7fad6610f037db8c79b127201b5d119" [[package]] name = "libloading" @@ -2959,9 +2959,9 @@ dependencies = [ [[package]] name = "libloading" -version = "0.7.1" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0cf036d15402bea3c5d4de17b3fce76b3e4a56ebc1f577be0e7a72f7c607cf0" +checksum = "afe203d669ec979b7128619bae5a63b7b42e9203c1b29146079ee05e2f604b52" dependencies = [ "cfg-if 1.0.0", "winapi 0.3.9", @@ -4031,9 +4031,9 @@ checksum = "a3e378b66a060d48947b590737b30a1be76706c8dd7b8ba0f2fe3989c68a853f" [[package]] name = "matrixmultiply" -version = "0.3.1" +version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a8a15b776d9dfaecd44b03c5828c2199cddff5247215858aac14624f8d6b741" +checksum = "add85d4dd35074e6fedc608f8c8f513a3548619a9024b751949ef0e8e45a4d84" dependencies = [ "rawpointer", ] @@ -4213,9 +4213,9 @@ dependencies = [ [[package]] name = "more-asserts" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0debeb9fcf88823ea64d64e4a815ab1643f33127d995978e099942ce38f25238" +checksum = "7843ec2de400bcbc6a6328c958dc38e5359da6e93e72e37bc5246bf1ae776389" [[package]] name = "multiaddr" @@ -4377,7 +4377,7 @@ version = "6.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e7413f999671bd4745a7b624bd370a569fb6bc574b23c83a3c5ed2e453f3d5e2" dependencies = [ - "bitvec 0.19.5", + "bitvec 0.19.6", "funty", "memchr", "version_check", @@ -4712,7 +4712,6 @@ source = "git+https://github.com/paritytech/substrate.git?branch=polkadot-v0.9.9 dependencies = [ "frame-support", "frame-system", - "log", "parity-scale-codec", "serde", "smallvec 1.7.0", @@ -6863,9 +6862,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.70" +version = "1.0.71" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e277c495ac6cd1a01a58d0a0c574568b4d1ddf14f59965c6a58b8d96400b54f3" +checksum = "063bf466a64011ac24040a49009724ee60a57da1b437617ceb32e53ad61bfb19" dependencies = [ "itoa", "ryu", diff --git a/finality-aleph/src/crypto.rs b/finality-aleph/src/crypto.rs index bd913627ce..184d4cc479 100644 --- a/finality-aleph/src/crypto.rs +++ b/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 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 for Signature { + fn from(sig_v1: SignatureV1) -> Signature { + Signature(sig_v1.sgn) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/finality-aleph/src/import.rs b/finality-aleph/src/import.rs index ae05cfb791..631635c817 100644 --- a/finality-aleph/src/import.rs +++ b/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, } +#[derive(Debug)] enum SendJustificationError where Block: BlockT, @@ -64,16 +66,23 @@ where number: NumberFor, justification: Justification, ) -> Result<(), SendJustificationError> { - 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")) } }) diff --git a/finality-aleph/src/justification.rs b/finality-aleph/src/justification.rs index 9fcee4ef63..a0c0c59dcf 100644 --- a/finality-aleph/src/justification.rs +++ b/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, } @@ -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, +} + +impl From for AlephJustification { + fn from(just_v1: AlephJustificationV1) -> AlephJustification { + let size = just_v1.signature.size(); + let just_drop_id: SignatureSet = 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) -> 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 + } +} diff --git a/finality-aleph/src/testing/justification.rs b/finality-aleph/src/testing/justification.rs new file mode 100644 index 0000000000..bae2973678 --- /dev/null +++ b/finality-aleph/src/testing/justification.rs @@ -0,0 +1,98 @@ +use crate::crypto::{Signature, SignatureV1}; +use crate::justification::{ + backwards_compatible_decode, AlephJustification, AlephJustificationV1, JustificationDecoding, +}; +use aleph_bft::{NodeCount, PartialMultisignature, SignatureSet}; +use aleph_primitives::AuthoritySignature; +use codec::Encode; + +#[test] +fn correctly_decodes_v1() { + let mut signature_set: SignatureSet = SignatureSet::with_size(7.into()); + for i in 0..7 { + let id = i.into(); + let signature_v1 = SignatureV1 { + _id: id, + sgn: Default::default(), + }; + signature_set = signature_set.add_signature(&signature_v1, id); + } + + let just_v1 = AlephJustificationV1 { + signature: signature_set, + }; + let encoded_just: Vec = just_v1.encode(); + let decoded = backwards_compatible_decode(encoded_just); + assert_eq!(decoded, JustificationDecoding::V1(just_v1)); +} + +#[test] +fn correctly_decodes_v2() { + let mut signature_set: SignatureSet = SignatureSet::with_size(7.into()); + for i in 0..7 { + let authority_signature: AuthoritySignature = Default::default(); + signature_set = signature_set.add_signature(&authority_signature.into(), i.into()); + } + + let just_v2 = AlephJustification { + signature: signature_set, + }; + let encoded_just: Vec = just_v2.encode(); + let decoded = backwards_compatible_decode(encoded_just); + assert_eq!(decoded, JustificationDecoding::V2(just_v2),); +} + +#[test] +fn correctly_decodes_legacy_v1_size4() { + // This is a justification for 4 nodes generated by the version at commit `a426d7a` + let raw: Vec = vec![ + 16, 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 70, 165, 218, 105, 238, 187, 137, 176, 148, 97, 251, 204, + 157, 166, 21, 31, 222, 144, 57, 47, 229, 130, 113, 221, 27, 138, 96, 189, 104, 39, 235, + 217, 107, 217, 184, 99, 252, 227, 142, 169, 60, 92, 64, 26, 128, 73, 40, 49, 208, 54, 173, + 47, 24, 229, 87, 93, 136, 157, 141, 173, 229, 156, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 148, 100, + 171, 132, 5, 223, 228, 210, 92, 49, 165, 58, 241, 100, 3, 208, 81, 194, 122, 36, 4, 31, + 108, 104, 227, 164, 204, 165, 181, 237, 168, 93, 81, 37, 243, 183, 37, 141, 233, 10, 232, + 189, 189, 95, 129, 126, 113, 239, 121, 8, 18, 43, 200, 200, 128, 211, 80, 34, 7, 104, 198, + 215, 213, 8, 1, 2, 0, 0, 0, 0, 0, 0, 0, 126, 125, 118, 133, 4, 152, 203, 42, 36, 177, 160, + 243, 211, 223, 249, 171, 206, 112, 228, 170, 54, 6, 223, 223, 83, 106, 27, 168, 40, 82, 48, + 28, 150, 76, 98, 250, 13, 97, 163, 152, 77, 30, 153, 206, 49, 210, 53, 218, 1, 52, 195, 97, + 58, 229, 250, 198, 35, 155, 118, 249, 180, 123, 12, 8, 0, + ]; + let decoded = backwards_compatible_decode(raw); + if let JustificationDecoding::V1(just) = decoded { + assert_eq!(just.signature.size(), NodeCount(4)); + } else { + panic!("decoded should be V1, and is {:?}", decoded); + } +} + +#[test] +fn correctly_decodes_legacy_v1_size6() { + // This is a justification for 6 nodes generated by the version at commit `a426d7a` + let raw: Vec = vec![ + 24, 1, 0, 0, 0, 0, 0, 0, 0, 0, 82, 120, 213, 50, 242, 152, 25, 224, 232, 243, 218, 52, 111, + 133, 171, 153, 160, 41, 16, 239, 33, 1, 252, 229, 53, 252, 155, 199, 63, 150, 6, 227, 44, + 130, 28, 24, 26, 202, 30, 197, 67, 119, 144, 44, 69, 39, 117, 53, 239, 104, 165, 208, 143, + 204, 4, 165, 6, 165, 27, 134, 7, 44, 172, 7, 1, 1, 0, 0, 0, 0, 0, 0, 0, 173, 204, 199, 231, + 18, 118, 216, 71, 19, 249, 239, 86, 196, 86, 173, 38, 113, 87, 118, 112, 26, 70, 125, 228, + 180, 101, 172, 159, 79, 8, 106, 247, 42, 255, 178, 0, 141, 194, 242, 81, 93, 1, 230, 89, + 247, 247, 233, 237, 136, 9, 254, 103, 74, 37, 43, 124, 226, 59, 146, 242, 143, 208, 49, 13, + 1, 2, 0, 0, 0, 0, 0, 0, 0, 162, 194, 14, 148, 20, 248, 49, 230, 200, 102, 179, 223, 186, + 103, 28, 58, 59, 67, 195, 77, 22, 20, 213, 92, 85, 61, 133, 57, 55, 123, 221, 193, 121, 80, + 18, 68, 92, 5, 2, 56, 55, 43, 1, 214, 145, 131, 146, 103, 245, 135, 25, 251, 212, 85, 230, + 223, 143, 44, 190, 102, 121, 121, 67, 12, 1, 3, 0, 0, 0, 0, 0, 0, 0, 176, 17, 161, 159, 68, + 184, 2, 127, 122, 162, 2, 213, 232, 113, 111, 86, 35, 196, 150, 186, 221, 188, 14, 245, 41, + 21, 206, 174, 134, 142, 191, 212, 20, 102, 99, 111, 110, 48, 75, 214, 163, 173, 107, 251, + 82, 24, 43, 131, 210, 160, 59, 88, 111, 150, 236, 25, 128, 36, 179, 255, 56, 189, 99, 13, + 1, 4, 0, 0, 0, 0, 0, 0, 0, 140, 68, 206, 82, 199, 166, 235, 142, 114, 218, 219, 235, 206, + 76, 253, 180, 143, 213, 7, 39, 49, 154, 83, 142, 250, 26, 74, 37, 95, 106, 51, 179, 185, + 75, 63, 244, 63, 1, 179, 125, 53, 110, 220, 13, 126, 46, 124, 173, 98, 164, 194, 175, 52, + 108, 43, 68, 94, 254, 77, 39, 172, 255, 145, 10, 0, + ]; + let decoded = backwards_compatible_decode(raw); + if let JustificationDecoding::V1(just) = decoded { + assert_eq!(just.signature.size(), NodeCount(6)); + } else { + panic!("decoded should be V1, and is {:?}", decoded); + } +} diff --git a/finality-aleph/src/testing/mod.rs b/finality-aleph/src/testing/mod.rs index 827cf55471..1808ee781a 100644 --- a/finality-aleph/src/testing/mod.rs +++ b/finality-aleph/src/testing/mod.rs @@ -1,4 +1,6 @@ #[cfg(test)] mod data_io; #[cfg(test)] +mod justification; +#[cfg(test)] mod network;