Make initial unit collection more independent

Author: timorleph

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "aleph-bft"
-version = "0.43.0"
+version = "0.43.1"
 edition = "2021"
 authors = ["Cardinal Cryptography"]
 categories = ["algorithms", "data-structures", "cryptography", "database"]

consensus/Cargo.toml

@@ -1,7 +1,7 @@
 use crate::{
+    collection::{NewestUnitResponse, Salt},
     dag::Request as ReconstructionRequest,
     network::UnitMessage,
-    runway::{NewestUnitResponse, Salt},
     units::UncheckedSignedUnit,
     Data, Hasher, NodeIndex, Recipient, Signature, UncheckedSigned,
 };
@@ -57,21 +57,6 @@ impl<T> Addressed<T> {
     }
 }
 
-/// Possible requests for information from other nodes.
-#[derive(Eq, PartialEq, Debug, Clone)]
-pub enum DisseminationRequest<H: Hasher> {
-    /// A request for unit information in normal operation.
-    Unit(ReconstructionRequest<H>),
-    /// Request for what the specified node thinks about our newest unit.
-    NewestUnit(NodeIndex, Salt),
-}
-
-impl<H: Hasher> From<ReconstructionRequest<H>> for DisseminationRequest<H> {
-    fn from(request: ReconstructionRequest<H>) -> Self {
-        DisseminationRequest::Unit(request)
-    }
-}
-
 /// Responses to requests.
 #[derive(Eq, PartialEq, Debug, Clone)]
 pub enum DisseminationResponse<H: Hasher, D: Data, S: Signature> {
@@ -89,9 +74,11 @@ pub enum DisseminationMessage<H: Hasher, D: Data, S: Signature> {
     /// Unit, either broadcast or in response to a coord request.
     Unit(UncheckedSignedUnit<H, D, S>),
     /// Request coming from the specified node for something.
-    Request(NodeIndex, DisseminationRequest<H>),
+    Request(NodeIndex, ReconstructionRequest<H>),
     /// Response to a parent request.
     ParentsResponse(H::Hash, Vec<UncheckedSignedUnit<H, D, S>>),
+    /// Initial unit collection request.
+    NewestUnitRequest(NodeIndex, Salt),
     /// Response to initial unit collection.
     NewestUnitResponse(UncheckedSigned<NewestUnitResponse<H, D, S>, S>),
 }
@@ -104,15 +91,13 @@ impl<H: Hasher, D: Data, S: Signature> From<UnitMessage<H, D, S>>
         match message {
             UnitMessage::Unit(u) => Unit(u),
             UnitMessage::CoordRequest(node_id, coord) => {
-                Request(node_id, ReconstructionRequest::Coord(coord).into())
+                Request(node_id, ReconstructionRequest::Coord(coord))
             }
             UnitMessage::ParentsRequest(node_id, hash) => {
-                Request(node_id, ReconstructionRequest::ParentsOf(hash).into())
+                Request(node_id, ReconstructionRequest::ParentsOf(hash))
             }
             UnitMessage::ParentsResponse(h, units) => ParentsResponse(h, units),
-            UnitMessage::NewestRequest(node_id, salt) => {
-                Request(node_id, DisseminationRequest::NewestUnit(node_id, salt))
-            }
+            UnitMessage::NewestRequest(node_id, salt) => NewestUnitRequest(node_id, salt),
             UnitMessage::NewestResponse(response) => NewestUnitResponse(response),
         }
     }
@@ -125,17 +110,14 @@ impl<H: Hasher, D: Data, S: Signature> From<DisseminationMessage<H, D, S>>
         use DisseminationMessage::*;
         match message {
             Unit(u) => UnitMessage::Unit(u),
-            Request(node_id, DisseminationRequest::Unit(ReconstructionRequest::Coord(coord))) => {
+            Request(node_id, ReconstructionRequest::Coord(coord)) => {
                 UnitMessage::CoordRequest(node_id, coord)
             }
-            Request(
-                node_id,
-                DisseminationRequest::Unit(ReconstructionRequest::ParentsOf(hash)),
-            ) => UnitMessage::ParentsRequest(node_id, hash),
-            ParentsResponse(h, units) => UnitMessage::ParentsResponse(h, units),
-            Request(node_id, DisseminationRequest::NewestUnit(_, salt)) => {
-                UnitMessage::NewestRequest(node_id, salt)
+            Request(node_id, ReconstructionRequest::ParentsOf(hash)) => {
+                UnitMessage::ParentsRequest(node_id, hash)
             }
+            ParentsResponse(h, units) => UnitMessage::ParentsResponse(h, units),
+            NewestUnitRequest(node_id, salt) => UnitMessage::NewestRequest(node_id, salt),
             NewestUnitResponse(response) => UnitMessage::NewestResponse(response),
         }
     }

consensus/src/runway/collection.rs consensus/src/collection/mod.rsconsensus/src/runway/collection.rs renamed to consensus/src/collection/mod.rs

@@ -1,7 +1,7 @@
 use crate::{
+    collection::{NewestUnitResponse, Salt},
     dag::{DagUnit, Request},
-    dissemination::{DisseminationRequest, DisseminationResponse},
-    runway::{NewestUnitResponse, Salt},
+    dissemination::DisseminationResponse,
     units::{UnitCoord, UnitStore, UnitWithParents, WrappedUnit},
     Data, Hasher, MultiKeychain, NodeIndex, Signed,
 };
@@ -91,18 +91,25 @@ impl<H: Hasher, D: Data, MK: MultiKeychain> Responder<H, D, MK> {
     /// aren't able to help.
     pub fn handle_request(
         &self,
-        request: DisseminationRequest<H>,
+        request: Request<H>,
         units: &UnitStore<DagUnit<H, D, MK>>,
     ) -> Result<DisseminationResponse<H, D, MK::Signature>, Error<H>> {
-        use DisseminationRequest::*;
+        use Request::*;
         match request {
-            Unit(unit_request) => match unit_request {
-                Request::Coord(coord) => self.on_request_coord(coord, units),
-                Request::ParentsOf(hash) => self.on_request_parents(hash, units),
-            },
-            NewestUnit(node_id, salt) => Ok(self.on_request_newest(node_id, salt, units)),
+            Coord(coord) => self.on_request_coord(coord, units),
+            ParentsOf(hash) => self.on_request_parents(hash, units),
         }
     }
+
+    /// Handle an incoming request for the newest unit of a given node we know of.
+    pub fn handle_newest_unit_request(
+        &self,
+        requester: NodeIndex,
+        salt: Salt,
+        units: &UnitStore<DagUnit<H, D, MK>>,
+    ) -> DisseminationResponse<H, D, MK::Signature> {
+        self.on_request_newest(requester, salt, units)
+    }
 }
 
 #[cfg(test)]
@@ -111,7 +118,7 @@ mod test {
         dag::Request,
         dissemination::{
             responder::{Error, Responder},
-            DisseminationRequest, DisseminationResponse,
+            DisseminationResponse,
         },
         units::{
             random_full_parent_reconstrusted_units_up_to, TestingDagUnit, Unit, UnitCoord,
@@ -142,7 +149,7 @@ mod test {
     fn empty_fails_to_respond_to_coords() {
         let (responder, store, _) = setup();
         let coord = UnitCoord::new(0, NodeIndex(1));
-        let request = Request::Coord(coord).into();
+        let request = Request::Coord(coord);
         match responder.handle_request(request, &store) {
             Ok(response) => panic!("Unexpected response: {:?}.", response),
             Err(err) => assert_eq!(err, Error::NoCanonicalAt(coord)),
@@ -160,7 +167,7 @@ mod test {
                 .last()
                 .expect("the round has at least one unit")
                 .hash();
-        let request = Request::ParentsOf(hash).into();
+        let request = Request::ParentsOf(hash);
         match responder.handle_request(request, &store) {
             Ok(response) => panic!("Unexpected response: {:?}.", response),
             Err(err) => assert_eq!(err, Error::UnknownUnit(hash)),
@@ -171,10 +178,7 @@ mod test {
     fn empty_newest_responds_with_no_units() {
         let (responder, store, keychains) = setup();
         let requester = NodeIndex(1);
-        let request = DisseminationRequest::NewestUnit(requester, rand::random());
-        let response = responder
-            .handle_request(request, &store)
-            .expect("newest unit requests always get a response");
+        let response = responder.handle_newest_unit_request(requester, rand::random(), &store);
         match response {
             DisseminationResponse::NewestUnit(newest_unit_response) => {
                 let checked_newest_unit_response = newest_unit_response
@@ -205,7 +209,7 @@ mod test {
                 store.insert(unit.clone());
             }
         }
-        let request = Request::Coord(coord).into();
+        let request = Request::Coord(coord);
         let response = responder
             .handle_request(request, &store)
             .expect("should successfully respond");
@@ -237,7 +241,7 @@ mod test {
                 store.insert(unit.clone());
             }
         }
-        let request = Request::Coord(coord).into();
+        let request = Request::Coord(coord);
         match responder.handle_request(request, &store) {
             Ok(response) => panic!("Unexpected response: {:?}.", response),
             Err(err) => assert_eq!(err, Error::NoCanonicalAt(coord)),
@@ -261,7 +265,7 @@ mod test {
             .last()
             .expect("the round has at least one unit")
             .clone();
-        let request = Request::ParentsOf(requested_unit.hash()).into();
+        let request = Request::ParentsOf(requested_unit.hash());
         let response = responder
             .handle_request(request, &store)
             .expect("should successfully respond");
@@ -295,7 +299,7 @@ mod test {
                 .last()
                 .expect("the round has at least one unit")
                 .hash();
-        let request = Request::ParentsOf(hash).into();
+        let request = Request::ParentsOf(hash);
         match responder.handle_request(request, &store) {
             Ok(response) => panic!("Unexpected response: {:?}.", response),
             Err(err) => assert_eq!(err, Error::UnknownUnit(hash)),
@@ -314,10 +318,7 @@ mod test {
             }
         }
         let requester = NodeIndex(1);
-        let request = DisseminationRequest::NewestUnit(requester, rand::random());
-        let response = responder
-            .handle_request(request, &store)
-            .expect("newest unit requests always get a response");
+        let response = responder.handle_newest_unit_request(requester, rand::random(), &store);
         match response {
             DisseminationResponse::NewestUnit(newest_unit_response) => {
                 newest_unit_response

consensus/src/dissemination/mod.rs

@@ -1,4 +1,4 @@
-use crate::{dag::Request, runway::Salt, units::UncheckedSignedUnit, Data, Hasher, Signature};
+use crate::{dag::Request, units::UncheckedSignedUnit, Data, Hasher, Signature};
 
 /// Task that needs to be performed to ensure successful unit dissemination, either requesting or broadcasting a unit.
 #[derive(Eq, PartialEq, Debug, Clone)]
@@ -8,6 +8,4 @@ pub enum DisseminationTask<H: Hasher, D: Data, S: Signature> {
     /// Broadcast a unit.
     /// TODO(A0-4567): This should soon only contain the hash.
     Broadcast(UncheckedSignedUnit<H, D, S>),
-    /// TODO(A0-4569): Only here temporarily, will soon be factored out.
-    RequestNewest(Salt),
 }

consensus/src/dissemination/responder.rs

@@ -3,6 +3,7 @@
 //! gives appropriate access to the set of available data that we need to make consensus on.
 
 mod alerts;
+mod collection;
 mod config;
 mod creation;
 mod dag;