A0-1128 DataProvider provides Option<Data> (#239)

Cardinal-Cryptography · Jul 15, 2022 · 704926e · 704926e
1 parent bd76102
commit 704926e
Show file tree

Hide file tree

Showing 22 changed files with 113 additions and 76 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/README.md b/README.md
@@ -60,7 +60,7 @@ More details are available [in the book][reference-link-implementation-details].
 - Import AlephBFT in your crate
   ```toml
   [dependencies]
-  aleph-bft = "^0.16"
+  aleph-bft = "^0.17"
   ```
 - The main entry point is the `run_session` function, which returns a Future that runs the
   consensus algorithm.

diff --git a/consensus/Cargo.toml b/consensus/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "aleph-bft"
-version = "0.16.0"
+version = "0.17.0"
 edition = "2021"
 authors = ["Cardinal Cryptography"]
 categories = ["algorithms", "data-structures", "cryptography", "database"]
@@ -14,7 +14,7 @@ description = "AlephBFT is an asynchronous and Byzantine fault tolerant consensu
 
 [dependencies]
 aleph-bft-rmc = { path = "../rmc", version = "0.3" }
-aleph-bft-types = { path = "../types", version = "0.5" }
+aleph-bft-types = { path = "../types", version = "0.6" }
 async-trait = "0.1"
 codec = { package = "parity-scale-codec", version = "3.0", default-features = false, features = ["derive"] }
 derivative = "2.2.0"

diff --git a/consensus/src/alerts/mod.rs b/consensus/src/alerts/mod.rs
@@ -84,7 +84,7 @@ impl<H: Hasher, D: Data, S: Signature> Alert<H, D, S> {
         // Only legit units might end up in the DAG, we can ignore the fork proof.
         self.legit_units
             .iter()
-            .map(|uu| uu.as_signable().data().clone())
+            .filter_map(|uu| uu.as_signable().data().clone())
             .collect()
     }
 }
@@ -524,7 +524,7 @@ mod tests {
         n_members: NodeCount,
         node_id: NodeIndex,
         round: Round,
-        variant: u32,
+        variant: Option<u32>,
     ) -> FullUnit<Hasher64, Data> {
         FullUnit::new(
             PreUnit::new(
@@ -544,8 +544,8 @@ mod tests {
         round: Round,
         n_members: NodeCount,
     ) -> TestForkProof {
-        let unit_0 = full_unit(n_members, node_id, round, 0);
-        let unit_1 = full_unit(n_members, node_id, round, 1);
+        let unit_0 = full_unit(n_members, node_id, round, Some(0));
+        let unit_1 = full_unit(n_members, node_id, round, Some(1));
         let signed_unit_0 = Signed::sign(unit_0, keychain).await.into_unchecked();
         let signed_unit_1 = Signed::sign(unit_1, keychain).await.into_unchecked();
         (signed_unit_0, signed_unit_1)
@@ -655,9 +655,12 @@ mod tests {
                 session_id: 0,
             },
         );
-        let valid_unit = Signed::sign(full_unit(n_members, alerter_index, 0, 0), &alerter_keychain)
-            .await
-            .into_unchecked();
+        let valid_unit = Signed::sign(
+            full_unit(n_members, alerter_index, 0, Some(0)),
+            &alerter_keychain,
+        )
+        .await
+        .into_unchecked();
         let wrong_fork_proof = (valid_unit.clone(), valid_unit);
         let wrong_alert = Alert::new(forker_index, wrong_fork_proof.clone(), vec![]);
         let signed_wrong_alert = Signed::sign(wrong_alert, &forker_keychain)
@@ -916,8 +919,8 @@ mod tests {
             },
         );
         let fork_proof = {
-            let unit_0 = full_unit(n_members, NodeIndex(6), 0, 0);
-            let unit_1 = full_unit(n_members, NodeIndex(5), 0, 0);
+            let unit_0 = full_unit(n_members, NodeIndex(6), 0, Some(0));
+            let unit_1 = full_unit(n_members, NodeIndex(5), 0, Some(0));
             let signed_unit_0 = Signed::sign(unit_0, &keychains[6]).await.into_unchecked();
             let signed_unit_1 = Signed::sign(unit_1, &keychains[5]).await.into_unchecked();
             (signed_unit_0, signed_unit_1)
@@ -940,8 +943,8 @@ mod tests {
             },
         );
         let fork_proof = {
-            let unit_0 = full_unit(n_members, forker_index, 0, 0);
-            let unit_1 = full_unit(n_members, forker_index, 1, 0);
+            let unit_0 = full_unit(n_members, forker_index, 0, Some(0));
+            let unit_1 = full_unit(n_members, forker_index, 1, Some(0));
             let signed_unit_0 = Signed::sign(unit_0, &forker_keychain)
                 .await
                 .into_unchecked();
@@ -985,8 +988,8 @@ mod tests {
         let fork_proof = if good_commitment {
             make_fork_proof(forker_index, &keychains[forker_index.0], 0, n_members).await
         } else {
-            let unit_0 = full_unit(n_members, forker_index, 0, 0);
-            let unit_1 = full_unit(n_members, forker_index, 1, 1);
+            let unit_0 = full_unit(n_members, forker_index, 0, Some(0));
+            let unit_1 = full_unit(n_members, forker_index, 1, Some(1));
             let signed_unit_0 = Signed::sign(unit_0, &keychains[forker_index.0])
                 .await
                 .into_unchecked();

diff --git a/consensus/src/member.rs b/consensus/src/member.rs
@@ -51,13 +51,13 @@ pub(crate) enum UnitMessage<H: Hasher, D: Data, S: Signature> {
 impl<H: Hasher, D: Data, S: Signature> UnitMessage<H, D, S> {
     pub(crate) fn included_data(&self) -> Vec<D> {
         match self {
-            Self::NewUnit(uu) => vec![uu.as_signable().data().clone()],
+            Self::NewUnit(uu) => uu.as_signable().included_data(),
             Self::RequestCoord(_, _) => Vec::new(),
-            Self::ResponseCoord(uu) => vec![uu.as_signable().data().clone()],
+            Self::ResponseCoord(uu) => uu.as_signable().included_data(),
             Self::RequestParents(_, _) => Vec::new(),
             Self::ResponseParents(_, units) => units
                 .iter()
-                .map(|uu| uu.as_signable().data().clone())
+                .flat_map(|uu| uu.as_signable().included_data())
                 .collect(),
             UnitMessage::RequestNewest(_, _) => Vec::new(),
             UnitMessage::ResponseNewest(response) => response.as_signable().included_data(),

diff --git a/consensus/src/network.rs b/consensus/src/network.rs
@@ -177,7 +177,7 @@ mod tests {
             combined_hash: 0.using_encoded(Hasher64::hash),
         };
         let pu = PreUnit::new(creator, round, control_hash);
-        let signable = FullUnit::new(pu, data, 0);
+        let signable = FullUnit::new(pu, Some(data), 0);
         Signed::sign(signable, &Keychain::new(0.into(), creator))
             .await
             .into_unchecked()
@@ -197,7 +197,7 @@ mod tests {
         use UnitMessage::NewUnit;
 
         let uu = test_unchecked_unit(5.into(), 43, 1729).await;
-        let included_data = vec![*uu.as_signable().data()];
+        let included_data = uu.as_signable().included_data();
         let nd = TestNetworkData::new(Units(NewUnit(uu.clone())));
         let decoded = TestNetworkData::decode(&mut &nd.encode()[..]);
         assert!(decoded.is_ok(), "Bug in encode/decode for NewUnit");
@@ -244,7 +244,7 @@ mod tests {
         use UnitMessage::ResponseCoord;
 
         let uu = test_unchecked_unit(5.into(), 43, 1729).await;
-        let included_data = vec![*uu.as_signable().data()];
+        let included_data = uu.as_signable().included_data();
         let nd = TestNetworkData::new(Units(ResponseCoord(uu.clone())));
         let decoded = TestNetworkData::decode(&mut &nd.encode()[..]);
         assert!(decoded.is_ok(), "Bug in encode/decode for ResponseCoord");
@@ -294,11 +294,13 @@ mod tests {
         let p1 = test_unchecked_unit(5.into(), 43, 1729).await;
         let p2 = test_unchecked_unit(13.into(), 43, 1729).await;
         let p3 = test_unchecked_unit(17.into(), 43, 1729).await;
-        let included_data = vec![
-            *p1.as_signable().data(),
-            *p2.as_signable().data(),
-            *p3.as_signable().data(),
-        ];
+        let included_data: Vec<Data> = p1
+            .as_signable()
+            .included_data()
+            .into_iter()
+            .chain(p2.as_signable().included_data().into_iter())
+            .chain(p3.as_signable().included_data().into_iter())
+            .collect();
         let parents = vec![p1, p2, p3];
 
         let nd = TestNetworkData::new(Units(ResponseParents(h, parents.clone())));
@@ -338,7 +340,8 @@ mod tests {
         let f2 = test_unchecked_unit(forker, 10, 1).await;
         let lu1 = test_unchecked_unit(forker, 11, 0).await;
         let lu2 = test_unchecked_unit(forker, 12, 0).await;
-        let included_data = vec![*lu1.as_signable().data(), *lu2.as_signable().data()];
+        let mut included_data = lu1.as_signable().included_data();
+        included_data.extend(lu2.as_signable().included_data());
         let sender: NodeIndex = 7.into();
         let alert = crate::alerts::Alert::new(sender, (f1, f2), vec![lu1, lu2]);
 

diff --git a/consensus/src/runway/collection.rs b/consensus/src/runway/collection.rs
@@ -67,7 +67,7 @@ impl<H: Hasher, D: Data, S: Signature> NewestUnitResponse<H, D, S> {
     /// The data included in this message, i.e. contents of the unit if any.
     pub fn included_data(&self) -> Vec<D> {
         match &self.unit {
-            Some(u) => vec![u.as_signable().data().clone()],
+            Some(u) => u.as_signable().included_data(),
             None => Vec::new(),
         }
     }
@@ -363,7 +363,7 @@ mod tests {
         session_id: SessionId,
         keychain: &Keychain,
     ) -> UncheckedSignedUnit {
-        let full_unit = FullUnit::new(pu, 0, session_id);
+        let full_unit = FullUnit::new(pu, Some(0), session_id);
         let signed_unit = Signed::sign(full_unit, keychain).await;
         signed_unit.into()
     }

diff --git a/consensus/src/runway/mod.rs b/consensus/src/runway/mod.rs
@@ -620,7 +620,7 @@ where
     fn on_ordered_batch(&mut self, batch: Vec<H::Hash>) {
         let data_iter: Vec<_> = batch
             .iter()
-            .map(|h| {
+            .filter_map(|h| {
                 self.store
                     .unit_by_hash(h)
                     .expect("Ordered units must be in store")

diff --git a/consensus/src/testing/alerts.rs b/consensus/src/testing/alerts.rs
@@ -114,7 +114,7 @@ impl TestCase {
                     self.keychain(NodeIndex(0)).node_count(),
                 )),
             ),
-            variant,
+            Some(variant),
             0,
         )
     }

diff --git a/consensus/src/testing/byzantine.rs b/consensus/src/testing/byzantine.rs
@@ -109,7 +109,7 @@ impl<'a> MaliciousMember<'a> {
             let control_hash = ControlHash::<Hasher64>::new(&parents);
             let new_preunit = PreUnit::<Hasher64>::new(self.node_ix, round, control_hash);
             if round != self.forking_round {
-                let full_unit = FullUnit::new(new_preunit, 0, self.session_id);
+                let full_unit = FullUnit::new(new_preunit, Some(0), self.session_id);
                 let signed_unit = Signed::sign(full_unit, self.keychain).await;
                 self.on_unit_received(signed_unit.clone());
                 self.send_legit_unit(signed_unit);
@@ -118,7 +118,7 @@ impl<'a> MaliciousMember<'a> {
                 debug!(target: "malicious-member", "Creating forks for round {}.", round);
                 let mut variants = Vec::new();
                 for data in 0u32..2u32 {
-                    let full_unit = FullUnit::new(new_preunit.clone(), data, self.session_id);
+                    let full_unit = FullUnit::new(new_preunit.clone(), Some(data), self.session_id);
                     let signed = Signed::sign(full_unit, self.keychain).await;
                     variants.push(signed);
                 }

diff --git a/consensus/src/testing/creation.rs b/consensus/src/testing/creation.rs
@@ -17,7 +17,7 @@ type FullUnit = GenericFullUnit<Hasher64, Data>;
 type NotificationOut = GenericNotificationOut<Hasher64>;
 
 fn preunit_to_unit(preunit: PreUnit) -> Unit {
-    FullUnit::new(preunit, 0, 0).unit()
+    FullUnit::new(preunit, Some(0), 0).unit()
 }
 
 struct TestController {

diff --git a/consensus/src/units/mod.rs b/consensus/src/units/mod.rs
@@ -115,7 +115,7 @@ impl<H: Hasher> PreUnit<H> {
 #[derivative(PartialEq, Eq, Hash)]
 pub struct FullUnit<H: Hasher, D: Data> {
     pre_unit: PreUnit<H>,
-    data: D,
+    data: Option<D>,
     session_id: SessionId,
     #[codec(skip)]
     #[derivative(PartialEq = "ignore")]
@@ -136,7 +136,7 @@ impl<H: Hasher, D: Data> Clone for FullUnit<H, D> {
 }
 
 impl<H: Hasher, D: Data> FullUnit<H, D> {
-    pub(crate) fn new(pre_unit: PreUnit<H>, data: D, session_id: SessionId) -> Self {
+    pub(crate) fn new(pre_unit: PreUnit<H>, data: Option<D>, session_id: SessionId) -> Self {
         FullUnit {
             pre_unit,
             data,
@@ -159,9 +159,12 @@ impl<H: Hasher, D: Data> FullUnit<H, D> {
     pub(crate) fn coord(&self) -> UnitCoord {
         self.pre_unit.coord
     }
-    pub(crate) fn data(&self) -> &D {
+    pub(crate) fn data(&self) -> &Option<D> {
         &self.data
     }
+    pub(crate) fn included_data(&self) -> Vec<D> {
+        self.data.iter().cloned().collect()
+    }
     pub(crate) fn session_id(&self) -> SessionId {
         self.session_id
     }
@@ -238,7 +241,12 @@ mod tests {
     fn test_full_unit_hash_is_correct() {
         let ch = ControlHash::<Hasher64>::new(&vec![].into());
         let pre_unit = PreUnit::new(NodeIndex(5), 6, ch);
-        let full_unit = FullUnit::new(pre_unit, 7, 8);
+        let full_unit = FullUnit::new(pre_unit, Some(7), 8);
+        let hash = full_unit.using_encoded(Hasher64::hash);
+        assert_eq!(full_unit.hash(), hash);
+        let ch = ControlHash::<Hasher64>::new(&vec![].into());
+        let pre_unit = PreUnit::new(NodeIndex(5), 6, ch);
+        let full_unit = FullUnit::new(pre_unit, None, 8);
         let hash = full_unit.using_encoded(Hasher64::hash);
         assert_eq!(full_unit.hash(), hash);
     }
@@ -256,7 +264,14 @@ mod tests {
     fn test_full_unit_codec() {
         let ch = ControlHash::<Hasher64>::new(&vec![].into());
         let pre_unit = PreUnit::new(NodeIndex(5), 6, ch);
-        let full_unit = FullUnit::new(pre_unit, 7, 8);
+        let full_unit = FullUnit::new(pre_unit, Some(7), 8);
+        full_unit.hash();
+        let encoded = full_unit.encode();
+        let decoded = FullUnit::decode(&mut encoded.as_slice()).expect("should decode correctly");
+        assert_eq!(decoded, full_unit);
+        let ch = ControlHash::<Hasher64>::new(&vec![].into());
+        let pre_unit = PreUnit::new(NodeIndex(5), 6, ch);
+        let full_unit = FullUnit::new(pre_unit, None, 8);
         full_unit.hash();
         let encoded = full_unit.encode();
         let decoded = FullUnit::decode(&mut encoded.as_slice()).expect("should decode correctly");

diff --git a/consensus/src/units/store.rs b/consensus/src/units/store.rs
@@ -218,7 +218,7 @@ mod tests {
             round,
             ControlHash::new(&NodeMap::with_size(count)),
         );
-        let full_unit = FullUnit::new(preunit, 0, session_id);
+        let full_unit = FullUnit::new(preunit, Some(0), session_id);
         Signed::sign(full_unit, keychain).await
     }
 

diff --git a/consensus/src/units/testing.rs b/consensus/src/units/testing.rs
@@ -30,7 +30,7 @@ pub fn create_units<'a, C: Iterator<Item = &'a Creator>>(
 }
 
 pub fn preunit_to_unit(preunit: PreUnit, session_id: SessionId) -> Unit {
-    FullUnit::new(preunit, 0, session_id).unit()
+    FullUnit::new(preunit, Some(0), session_id).unit()
 }
 
 impl Creator {
@@ -46,7 +46,7 @@ pub async fn preunit_to_unchecked_signed_unit(
     session_id: SessionId,
     keychain: &Keychain,
 ) -> UncheckedSignedUnit {
-    let full_unit = FullUnit::new(pu, 0, session_id);
+    let full_unit = FullUnit::new(pu, Some(0), session_id);
     let signed_unit = Signed::sign(full_unit, keychain).await;
     signed_unit.into()
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -114,7 +114,7 @@ impl TestCase { @@
                         self.keychain(NodeIndex(0)).node_count(),
                     )),
                 ),
-                variant,
+                Some(variant),
 ,
             )
         }
@@ Expand Down @@