Logic fixes for rounds with partial echo (#67)

* Allow unwraps for tests in lib.rs instead of a separate config file

* Add proper support for rounds where echo broadcasts are only sent to a subset of nodes

manul/Cargo.toml

@@ -29,12 +29,14 @@ serde_json = { version = "1", default-features = false, features = ["alloc"], op
 
 [dev-dependencies]
 impls = "1"
+rand_core = { version = "0.6.4", default-features = false, features = ["getrandom"] }
 rand = { version = "0.8", default-features = false }
 serde_asn1_der = "0.8"
 criterion = "0.5"
 serde-persistent-deserializer = "0.3"
 postcard = { version = "1", default-features = false, features = ["alloc"] }
 serde_json = { version = "1", default-features = false, features = ["alloc"] }
+tracing-subscriber = { version = "0.3", features = ["env-filter"] }
 
 [features]
 testing = ["rand", "postcard", "serde_json", "serde-persistent-deserializer"]

manul/src/combinators/chain.rs

@@ -382,6 +382,20 @@ where
         }
     }
 
+    fn expecting_messages_from(&self) -> &BTreeSet<Id> {
+        match &self.state {
+            ChainState::Protocol1 { round, .. } => round.as_ref().expecting_messages_from(),
+            ChainState::Protocol2(round) => round.as_ref().expecting_messages_from(),
+        }
+    }
+
+    fn echo_round_participation(&self) -> EchoRoundParticipation<Id> {
+        match &self.state {
+            ChainState::Protocol1 { round, .. } => round.as_ref().echo_round_participation(),
+            ChainState::Protocol2(round) => round.as_ref().echo_round_participation(),
+        }
+    }
+
     fn make_direct_message(
         &self,
         rng: &mut dyn CryptoRngCore,
@@ -514,11 +528,4 @@ where
             },
         }
     }
-
-    fn expecting_messages_from(&self) -> &BTreeSet<Id> {
-        match &self.state {
-            ChainState::Protocol1 { round, .. } => round.as_ref().expecting_messages_from(),
-            ChainState::Protocol2(round) => round.as_ref().expecting_messages_from(),
-        }
-    }
 }

manul/src/combinators/misbehave.rs

@@ -29,8 +29,9 @@ use core::fmt::Debug;
 use rand_core::CryptoRngCore;
 
 use crate::protocol::{
-    Artifact, BoxedRng, BoxedRound, Deserializer, DirectMessage, EchoBroadcast, EntryPoint, FinalizeError,
-    FinalizeOutcome, LocalError, NormalBroadcast, ObjectSafeRound, PartyId, Payload, ReceiveError, RoundId, Serializer,
+    Artifact, BoxedRng, BoxedRound, Deserializer, DirectMessage, EchoBroadcast, EchoRoundParticipation, EntryPoint,
+    FinalizeError, FinalizeOutcome, LocalError, NormalBroadcast, ObjectSafeRound, PartyId, Payload, ReceiveError,
+    RoundId, Serializer,
 };
 
 /// A trait describing required properties for a behavior type.
@@ -173,6 +174,14 @@ where
         self.round.as_ref().message_destinations()
     }
 
+    fn expecting_messages_from(&self) -> &BTreeSet<Id> {
+        self.round.as_ref().expecting_messages_from()
+    }
+
+    fn echo_round_participation(&self) -> EchoRoundParticipation<Id> {
+        self.round.as_ref().echo_round_participation()
+    }
+
     fn make_direct_message(
         &self,
         rng: &mut dyn CryptoRngCore,
@@ -284,8 +293,4 @@ where
             Err(err) => Err(err),
         }
     }
-
-    fn expecting_messages_from(&self) -> &BTreeSet<Id> {
-        self.round.as_ref().expecting_messages_from()
-    }
 }

manul/src/lib.rs

@@ -3,8 +3,6 @@
 #![doc = include_str!("../../README.md")]
 #![warn(
     clippy::mod_module_files,
-    clippy::unwrap_used,
-    clippy::indexing_slicing,
     missing_docs,
     missing_copy_implementations,
     rust_2018_idioms,
@@ -13,6 +11,7 @@
     unused_qualifications,
     missing_debug_implementations
 )]
+#![cfg_attr(not(test), warn(clippy::unwrap_used, clippy::indexing_slicing,))]
 
 extern crate alloc;
 
@@ -23,3 +22,6 @@ pub(crate) mod utils;
 
 #[cfg(any(test, feature = "testing"))]
 pub mod testing;
+
+#[cfg(test)]
+mod tests;

manul/src/protocol.rs

@@ -24,7 +24,8 @@ pub use errors::{
 pub use message::{DirectMessage, EchoBroadcast, NormalBroadcast, ProtocolMessagePart};
 pub use object_safe::BoxedRound;
 pub use round::{
-    Artifact, CorrectnessProof, EntryPoint, FinalizeOutcome, PartyId, Payload, Protocol, ProtocolError, Round, RoundId,
+    Artifact, CorrectnessProof, EchoRoundParticipation, EntryPoint, FinalizeOutcome, PartyId, Payload, Protocol,
+    ProtocolError, Round, RoundId,
 };
 pub use serialization::{Deserializer, Serializer};
 

manul/src/protocol/object_safe.rs

@@ -10,7 +10,7 @@ use rand_core::{CryptoRng, CryptoRngCore, RngCore};
 use super::{
     errors::{FinalizeError, LocalError, ReceiveError},
     message::{DirectMessage, EchoBroadcast, NormalBroadcast},
-    round::{Artifact, FinalizeOutcome, PartyId, Payload, Protocol, Round, RoundId},
+    round::{Artifact, EchoRoundParticipation, FinalizeOutcome, PartyId, Payload, Protocol, Round, RoundId},
     serialization::{Deserializer, Serializer},
 };
 
@@ -48,6 +48,10 @@ pub(crate) trait ObjectSafeRound<Id: PartyId>: 'static + Debug + Send + Sync {
 
     fn message_destinations(&self) -> &BTreeSet<Id>;
 
+    fn expecting_messages_from(&self) -> &BTreeSet<Id>;
+
+    fn echo_round_participation(&self) -> EchoRoundParticipation<Id>;
+
     fn make_direct_message(
         &self,
         rng: &mut dyn CryptoRngCore,
@@ -87,8 +91,6 @@ pub(crate) trait ObjectSafeRound<Id: PartyId>: 'static + Debug + Send + Sync {
         artifacts: BTreeMap<Id, Artifact>,
     ) -> Result<FinalizeOutcome<Id, Self::Protocol>, FinalizeError<Self::Protocol>>;
 
-    fn expecting_messages_from(&self) -> &BTreeSet<Id>;
-
     /// Returns the type ID of the implementing type.
     fn get_type_id(&self) -> core::any::TypeId {
         core::any::TypeId::of::<Self>()
@@ -135,6 +137,14 @@ where
         self.round.message_destinations()
     }
 
+    fn expecting_messages_from(&self) -> &BTreeSet<Id> {
+        self.round.expecting_messages_from()
+    }
+
+    fn echo_round_participation(&self) -> EchoRoundParticipation<Id> {
+        self.round.echo_round_participation()
+    }
+
     fn make_direct_message(
         &self,
         rng: &mut dyn CryptoRngCore,
@@ -195,10 +205,6 @@ where
         let mut boxed_rng = BoxedRng(rng);
         self.round.finalize(&mut boxed_rng, payloads, artifacts)
     }
-
-    fn expecting_messages_from(&self) -> &BTreeSet<Id> {
-        self.round.expecting_messages_from()
-    }
 }
 
 // We do not want to expose `ObjectSafeRound` to the user, so it is hidden in a struct.

manul/src/protocol/round.rs

@@ -369,6 +369,25 @@ pub trait PartyId: 'static + Debug + Clone + Ord + Send + Sync + Serialize + for
 
 impl<T> PartyId for T where T: 'static + Debug + Clone + Ord + Send + Sync + Serialize + for<'de> Deserialize<'de> {}
 
+/// The specific way the node participates in the echo round (if any).
+#[derive(Debug, Clone)]
+pub enum EchoRoundParticipation<Id> {
+    /// The default behavior: sends broadcasts and receives echoed messages, or does neither.
+    ///
+    /// That is, this node will be a part of the echo round if [`Round::make_echo_broadcast`] generates a message.
+    Default,
+
+    /// This node sends broadcasts that will be echoed, but does not receive any.
+    Send,
+
+    /// This node receives broadcasts that it needs to echo, but does not send any itself.
+    Receive {
+        /// The other participants of the echo round
+        /// (that is, the nodes to which echoed messages will be sent).
+        echo_targets: BTreeSet<Id>,
+    },
+}
+
 /**
 A type representing a single round of a protocol.
 
@@ -401,6 +420,21 @@ pub trait Round<Id: PartyId>: 'static + Debug + Send + Sync {
     ///   for each element of the returned set.
     fn message_destinations(&self) -> &BTreeSet<Id>;
 
+    /// Returns the set of node IDs from which this round expects messages.
+    ///
+    /// The execution layer will not call [`finalize`](`Self::finalize`) until all these nodes have responded
+    /// (and the corresponding [`receive_message`](`Self::receive_message`) finished successfully).
+    fn expecting_messages_from(&self) -> &BTreeSet<Id>;
+
+    /// Returns the specific way the node participates in the echo round following this round.
+    ///
+    /// Returns [`EchoRoundParticipation::Default`] by default; this works fine when every node
+    /// sends messages to every other one, or do not send or receive any echo broadcasts.
+    /// Otherwise, review the options in [`EchoRoundParticipation`] and pick the appropriate one.
+    fn echo_round_participation(&self) -> EchoRoundParticipation<Id> {
+        EchoRoundParticipation::Default
+    }
+
     /// Returns the direct message to the given destination and (maybe) an accompanying artifact.
     ///
     /// Return [`DirectMessage::none`] if this round does not send direct messages.
@@ -473,10 +507,4 @@ pub trait Round<Id: PartyId>: 'static + Debug + Send + Sync {
         payloads: BTreeMap<Id, Payload>,
         artifacts: BTreeMap<Id, Artifact>,
     ) -> Result<FinalizeOutcome<Id, Self::Protocol>, FinalizeError<Self::Protocol>>;
-
-    /// Returns the set of node IDs from which this round expects messages.
-    ///
-    /// The execution layer will not call [`finalize`](`Self::finalize`) until all these nodes have responded
-    /// (and the corresponding [`receive_message`](`Self::receive_message`) finished successfully).
-    fn expecting_messages_from(&self) -> &BTreeSet<Id>;
 }

manul/src/session/echo.rs

@@ -12,7 +12,7 @@ use tracing::debug;
 
 use super::{
     message::{MessageVerificationError, SignedMessagePart},
-    session::SessionParameters,
+    session::{EchoRoundInfo, SessionParameters},
     LocalError,
 };
 use crate::{
@@ -77,8 +77,7 @@ pub(crate) struct EchoRoundMessage<SP: SessionParameters> {
 pub struct EchoRound<P: Protocol, SP: SessionParameters> {
     verifier: SP::Verifier,
     echo_broadcasts: BTreeMap<SP::Verifier, SignedMessagePart<EchoBroadcast>>,
-    destinations: BTreeSet<SP::Verifier>,
-    expected_echos: BTreeSet<SP::Verifier>,
+    echo_round_info: EchoRoundInfo<SP::Verifier>,
     main_round: BoxedRound<SP::Verifier, P>,
     payloads: BTreeMap<SP::Verifier, Payload>,
     artifacts: BTreeMap<SP::Verifier, Artifact>,
@@ -93,25 +92,19 @@ where
         verifier: SP::Verifier,
         my_echo_broadcast: SignedMessagePart<EchoBroadcast>,
         echo_broadcasts: BTreeMap<SP::Verifier, SignedMessagePart<EchoBroadcast>>,
+        echo_round_info: EchoRoundInfo<SP::Verifier>,
         main_round: BoxedRound<SP::Verifier, P>,
         payloads: BTreeMap<SP::Verifier, Payload>,
         artifacts: BTreeMap<SP::Verifier, Artifact>,
     ) -> Self {
-        let destinations = echo_broadcasts.keys().cloned().collect::<BTreeSet<_>>();
-
-        // Add our own echo message because we expect it to be sent back from other nodes.
-        let mut expected_echos = destinations.clone();
-        expected_echos.insert(verifier.clone());
-
         let mut echo_broadcasts = echo_broadcasts;
         echo_broadcasts.insert(verifier.clone(), my_echo_broadcast);
 
-        debug!("{:?}: initialized echo round with {:?}", verifier, destinations);
+        debug!("{:?}: initialized echo round with {:?}", verifier, echo_round_info);
         Self {
             verifier,
             echo_broadcasts,
-            destinations,
-            expected_echos,
+            echo_round_info,
             main_round,
             payloads,
             artifacts,
@@ -154,7 +147,7 @@ where
     }
 
     fn message_destinations(&self) -> &BTreeSet<SP::Verifier> {
-        &self.destinations
+        &self.echo_round_info.message_destinations
     }
 
     fn make_normal_broadcast(
@@ -180,7 +173,7 @@ where
     }
 
     fn expecting_messages_from(&self) -> &BTreeSet<SP::Verifier> {
-        &self.destinations
+        &self.echo_round_info.expecting_messages_from
     }
 
     fn receive_message(
@@ -199,16 +192,14 @@ where
 
         let message = normal_broadcast.deserialize::<EchoRoundMessage<SP>>(deserializer)?;
 
-        // Check that the received message contains entries from `destinations` sans `from`
+        // Check that the received message contains entries from `expected_echos`.
         // It is an unprovable fault.
 
-        let mut expected_keys = self.expected_echos.clone();
-        if !expected_keys.remove(from) {
-            return Err(ReceiveError::local(format!(
-                "The message sender {from:?} is missing from the expected senders {:?}",
-                self.destinations
-            )));
-        }
+        let mut expected_keys = self.echo_round_info.expected_echos.clone();
+
+        // We don't expect the node to send its echo the second time.
+        expected_keys.remove(from);
+
         let message_keys = message.echo_broadcasts.keys().cloned().collect::<BTreeSet<_>>();
 
         let missing_keys = expected_keys.difference(&message_keys).collect::<Vec<_>>();