AZ-686: Factor validation out of runway (#184)

* Factor unit validation out of runway * Add validator tests * Fix embarassing rebase failures * Version bump * Improve logging * Typo * Require failing signature verification of out of bounds indices
Cardinal-Cryptography · Mar 29, 2022 · 5d70721 · 5d70721
1 parent 5661bdd
commit 5d70721
Show file tree

Hide file tree

Showing 10 changed files with 446 additions and 147 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/consensus/Cargo.toml b/consensus/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "aleph-bft"
-version = "0.9.0"
+version = "0.9.1"
 edition = "2018"
 authors = ["Cardinal Cryptography"]
 categories = ["algorithms", "data-structures", "cryptography", "database"]

diff --git a/consensus/src/creation/creator.rs b/consensus/src/creation/creator.rs
@@ -4,15 +4,15 @@ use crate::{
 };
 use log::trace;
 
-pub(super) struct Creator<H: Hasher> {
+pub struct Creator<H: Hasher> {
     node_id: NodeIndex,
     n_members: NodeCount,
     candidates_by_round: Vec<NodeMap<H::Hash>>,
     n_candidates_by_round: Vec<NodeCount>, // len of this - 1 is the highest round number of all known units
 }
 
 impl<H: Hasher> Creator<H> {
-    pub(super) fn new(node_id: NodeIndex, n_members: NodeCount) -> Self {
+    pub fn new(node_id: NodeIndex, n_members: NodeCount) -> Self {
         Creator {
             node_id,
             n_members,
@@ -21,7 +21,7 @@ impl<H: Hasher> Creator<H> {
         }
     }
 
-    pub(super) fn current_round(&self) -> Round {
+    pub fn current_round(&self) -> Round {
         (self.n_candidates_by_round.len() - 1) as Round
     }
 
@@ -38,7 +38,7 @@ impl<H: Hasher> Creator<H> {
     /// Returns `None` if a unit cannot be created.
     /// To create a new unit, we need to have at least floor(2*N/3) + 1 parents available in previous round.
     /// Additionally, our unit from previous round must be available.
-    pub(super) fn create_unit(&self, round: Round) -> Option<(PreUnit<H>, Vec<H::Hash>)> {
+    pub fn create_unit(&self, round: Round) -> Option<(PreUnit<H>, Vec<H::Hash>)> {
         if !self.can_create(round) {
             return None;
         }
@@ -58,7 +58,7 @@ impl<H: Hasher> Creator<H> {
         Some((new_preunit, parent_hashes))
     }
 
-    pub(super) fn add_unit(&mut self, unit: &Unit<H>) {
+    pub fn add_unit(&mut self, unit: &Unit<H>) {
         let round = unit.round();
         let pid = unit.creator();
         let hash = unit.hash();

diff --git a/consensus/src/creation/mod.rs b/consensus/src/creation/mod.rs
@@ -11,7 +11,7 @@ use std::time::Duration;
 
 mod creator;
 
-use creator::Creator;
+pub use creator::Creator;
 
 /// The configuration needed for the process creating new units.
 pub struct Config {

diff --git a/consensus/src/member.rs b/consensus/src/member.rs
@@ -1,10 +1,13 @@
 use crate::{
     config::Config,
     network,
-    runway::{self, Request, Response, RunwayIO, RunwayNotificationIn, RunwayNotificationOut},
+    runway::{
+        self, NewestUnitResponse, Request, Response, RunwayIO, RunwayNotificationIn,
+        RunwayNotificationOut,
+    },
     units::{UncheckedSignedUnit, UnitCoord},
     Data, DataProvider, FinalizationHandler, Hasher, MultiKeychain, Network, NodeCount, NodeIndex,
-    Receiver, Recipient, Sender, Signable, Signature, SpawnHandle, UncheckedSigned,
+    Receiver, Recipient, Sender, Signature, SpawnHandle, UncheckedSigned,
 };
 use codec::{Decode, Encode};
 use futures::{
@@ -24,28 +27,6 @@ use std::{
     time,
 };
 
-#[derive(Debug, Encode, Decode, Clone)]
-pub(crate) struct NewestUnitResponse<H: Hasher, D: Data, S: Signature> {
-    pub(crate) requester: NodeIndex,
-    pub(crate) responder: NodeIndex,
-    pub(crate) unit: Option<UncheckedSignedUnit<H, D, S>>,
-    pub(crate) salt: u64,
-}
-
-impl<H: Hasher, D: Data, S: Signature> Signable for NewestUnitResponse<H, D, S> {
-    type Hash = Vec<u8>;
-
-    fn hash(&self) -> Self::Hash {
-        self.encode()
-    }
-}
-
-impl<H: Hasher, D: Data, S: Signature> crate::Index for NewestUnitResponse<H, D, S> {
-    fn index(&self) -> NodeIndex {
-        self.responder
-    }
-}
-
 /// A message concerning units, either about new units or some requests for them.
 #[derive(Debug, Encode, Decode, Clone)]
 pub(crate) enum UnitMessage<H: Hasher, D: Data, S: Signature> {
@@ -77,12 +58,7 @@ impl<H: Hasher, D: Data, S: Signature> UnitMessage<H, D, S> {
                 .map(|uu| uu.as_signable().data().clone())
                 .collect(),
             UnitMessage::RequestNewest(_, _) => Vec::new(),
-            UnitMessage::ResponseNewest(response) => response
-                .as_signable()
-                .unit
-                .iter()
-                .map(|uu| uu.as_signable().data().clone())
-                .collect(),
+            UnitMessage::ResponseNewest(response) => response.as_signable().included_data(),
         }
     }
 }
@@ -336,7 +312,7 @@ where
                     self.send_unit_message(message, Recipient::Node(recipient))
                 }
                 Response::NewestUnit(response) => {
-                    let requester = response.as_signable().requester;
+                    let requester = response.as_signable().requester();
                     let message = UnitMessage::ResponseNewest(response);
                     self.send_unit_message(message, Recipient::Node(requester))
                 }