Send hashes in the echo round instead of the full messages

CHANGELOG.md

@@ -42,6 +42,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - An error message in `ProtocolMessagePart::assert_is_none()`. ([#86])
+- Output size mismatch in `TestHasher`. ([#90])
+- Reduced the size of echo round message by sending hashes instead of full messages. ([#90])
 
 
 [#75]: https://github.com/entropyxyz/manul/pull/75
@@ -54,6 +56,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#86]: https://github.com/entropyxyz/manul/pull/86
 [#87]: https://github.com/entropyxyz/manul/pull/87
 [#88]: https://github.com/entropyxyz/manul/pull/88
+[#90]: https://github.com/entropyxyz/manul/pull/90
 [#91]: https://github.com/entropyxyz/manul/pull/91
 
 

manul/src/dev/session_parameters.rs

@@ -62,7 +62,7 @@ impl<D: digest::Digest> signature::DigestVerifier<D, TestSignature> for TestVeri
 #[derive(Debug, Clone, Copy, Default)]
 pub struct TestHasher {
     cursor: usize,
-    buffer: [u8; 32],
+    buffer: digest::Output<Self>,
 }
 
 impl digest::HashMarker for TestHasher {}
@@ -72,7 +72,7 @@ impl digest::Update for TestHasher {
         // A very simple algorithm for testing, just xor the data in buffer-sized chunks.
         for byte in data {
             *self.buffer.get_mut(self.cursor).expect("index within bounds") ^= byte;
-            self.cursor = (self.cursor + 1) % 32;
+            self.cursor = (self.cursor + 1) % self.buffer.len();
         }
     }
 }
@@ -84,7 +84,7 @@ impl digest::FixedOutput for TestHasher {
 }
 
 impl digest::OutputSizeUser for TestHasher {
-    type OutputSize = typenum::U8;
+    type OutputSize = typenum::U32;
 }
 
 /// An implementation of [`SessionParameters`] using the testing signer/verifier types.

manul/src/protocol.rs

@@ -30,4 +30,5 @@ pub use round::{
 pub use serialization::{Deserializer, Serializer};
 
 pub(crate) use errors::ReceiveErrorType;
+pub(crate) use message::ProtocolMessagePartHashable;
 pub(crate) use object_safe::{BoxedRng, ObjectSafeRound};

manul/src/protocol/message.rs

@@ -1,5 +1,6 @@
 use alloc::string::{String, ToString};
 
+use digest::Digest;
 use serde::{Deserialize, Serialize};
 
 use super::{
@@ -121,6 +122,42 @@ pub trait ProtocolMessagePart: ProtocolMessageWrapper {
     }
 }
 
+#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
+pub(crate) enum PartKind {
+    EchoBroadcast,
+    NormalBroadcast,
+    DirectMessage,
+}
+
+pub(crate) trait HasPartKind {
+    const KIND: PartKind;
+}
+
+// We don't want to expose this functionality to the user, so it is separate from `ProtocolMessagePart` trait.
+pub(crate) trait ProtocolMessagePartHashable: ProtocolMessagePart + HasPartKind {
+    fn hash<D: Digest>(&self) -> digest::Output<D> {
+        let mut digest = D::new_with_prefix(b"ProtocolMessagePart");
+        match Self::KIND {
+            PartKind::EchoBroadcast => digest.update([0u8]),
+            PartKind::NormalBroadcast => digest.update([1u8]),
+            PartKind::DirectMessage => digest.update([2u8]),
+        }
+        match self.maybe_message().as_ref() {
+            None => digest.update([0u8]),
+            Some(payload) => {
+                let payload_len =
+                    u64::try_from(payload.as_ref().len()).expect("payload length does not exceed 18 exabytes");
+                digest.update([1u8]);
+                digest.update(payload_len.to_be_bytes());
+                digest.update(payload);
+            }
+        };
+        digest.finalize()
+    }
+}
+
+impl<T: ProtocolMessagePart + HasPartKind> ProtocolMessagePartHashable for T {}
+
 /// A serialized direct message.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct DirectMessage(Option<MessagePayload>);
@@ -135,6 +172,10 @@ impl ProtocolMessageWrapper for DirectMessage {
     }
 }
 
+impl HasPartKind for DirectMessage {
+    const KIND: PartKind = PartKind::DirectMessage;
+}
+
 impl ProtocolMessagePart for DirectMessage {
     type Error = DirectMessageError;
 }
@@ -153,6 +194,10 @@ impl ProtocolMessageWrapper for EchoBroadcast {
     }
 }
 
+impl HasPartKind for EchoBroadcast {
+    const KIND: PartKind = PartKind::EchoBroadcast;
+}
+
 impl ProtocolMessagePart for EchoBroadcast {
     type Error = EchoBroadcastError;
 }
@@ -171,6 +216,10 @@ impl ProtocolMessageWrapper for NormalBroadcast {
     }
 }
 
+impl HasPartKind for NormalBroadcast {
+    const KIND: PartKind = PartKind::NormalBroadcast;
+}
+
 impl ProtocolMessagePart for NormalBroadcast {
     type Error = NormalBroadcastError;
 }

manul/src/session/echo.rs

@@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize};
 use tracing::debug;
 
 use super::{
-    message::{MessageVerificationError, SignedMessagePart},
+    message::{MessageVerificationError, SignedMessageHash, SignedMessagePart},
     session::{EchoRoundInfo, SessionParameters},
     LocalError,
 };
@@ -40,9 +40,8 @@ pub(crate) enum EchoRoundError<Id> {
     /// This is the fault of the sender of that specific broadcast.
     MismatchedBroadcasts {
         guilty_party: Id,
-        error: MismatchedBroadcastsError,
         we_received: SignedMessagePart<EchoBroadcast>,
-        echoed_to_us: SignedMessagePart<EchoBroadcast>,
+        echoed_to_us: SignedMessageHash,
     },
 }
 
@@ -57,17 +56,10 @@ impl<Id> EchoRoundError<Id> {
     }
 }
 
-#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
-pub(crate) enum MismatchedBroadcastsError {
-    /// The originally received message and the echoed one had different payloads.
-    DifferentPayloads,
-    /// The originally received message and the echoed one had different signatures.
-    DifferentSignatures,
-}
-
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub(crate) struct EchoRoundMessage<SP: SessionParameters> {
-    pub(super) echo_broadcasts: SerializableMap<SP::Verifier, SignedMessagePart<EchoBroadcast>>,
+    /// Signatures of echo broadcasts from respective nodes.
+    pub(super) message_hashes: SerializableMap<SP::Verifier, SignedMessageHash>,
 }
 
 /// Each protocol round can contain one `EchoRound` with "echo messages" that are sent to all
@@ -90,16 +82,12 @@ where
 {
     pub fn new(
         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 mut echo_broadcasts = echo_broadcasts;
-        echo_broadcasts.insert(verifier.clone(), my_echo_broadcast);
-
         debug!("{:?}: initialized echo round with {:?}", verifier, echo_round_info);
         Self {
             verifier,
@@ -166,9 +154,13 @@ where
             )));
         }
 
-        let message = EchoRoundMessage::<SP> {
-            echo_broadcasts: echo_broadcasts.into(),
-        };
+        let message_hashes = echo_broadcasts
+            .iter()
+            .map(|(id, echo_broadcast)| (id.clone(), echo_broadcast.to_signed_hash::<SP>()))
+            .collect::<BTreeMap<_, _>>()
+            .into();
+
+        let message = EchoRoundMessage::<SP> { message_hashes };
         NormalBroadcast::new(serializer, message)
     }
 
@@ -199,7 +191,7 @@ where
         // 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 message_keys = message.message_hashes.keys().cloned().collect::<BTreeSet<_>>();
 
         let missing_keys = expected_keys.difference(&message_keys).collect::<Vec<_>>();
         if !missing_keys.is_empty() {
@@ -221,7 +213,7 @@ where
         // If there's a difference, it's a provable fault,
         // since we have both messages signed by `from`.
 
-        for (sender, echo) in message.echo_broadcasts.iter() {
+        for (sender, echo) in message.message_hashes.iter() {
             // We expect the key to be there since
             // `message.echo_broadcasts.keys()` is within `self.destinations`
             // which was constructed as `self.echo_broadcasts.keys()`.
@@ -230,10 +222,6 @@ where
                 .get(sender)
                 .expect("the key is present by construction");
 
-            if echo == previously_received_echo {
-                continue;
-            }
-
             let verified_echo = match echo.clone().verify::<SP>(sender) {
                 Ok(echo) => echo,
                 Err(MessageVerificationError::Local(error)) => return Err(error.into()),
@@ -253,28 +241,17 @@ where
                 return Err(EchoRoundError::InvalidEcho(sender.clone()).into());
             }
 
-            // `sender` sent us and `from` messages with different payloads.
+            // `sender` sent us and `from` messages with different payloads,
+            // but with correct signatures and the same metadata.
             // Provable fault of `sender`.
-            if verified_echo.payload() != previously_received_echo.payload() {
+            if !verified_echo.is_hash_of::<SP, _>(previously_received_echo) {
                 return Err(EchoRoundError::MismatchedBroadcasts {
                     guilty_party: sender.clone(),
-                    error: MismatchedBroadcastsError::DifferentPayloads,
                     we_received: previously_received_echo.clone(),
                     echoed_to_us: echo.clone(),
                 }
                 .into());
             }
-
-            // At this point, we know that the echoed broadcast is not identical to what we initially received,
-            // but somehow they both have the correct metadata, and correct signatures.
-            // Something strange is going on.
-            return Err(EchoRoundError::MismatchedBroadcasts {
-                guilty_party: sender.clone(),
-                error: MismatchedBroadcastsError::DifferentSignatures,
-                we_received: previously_received_echo.clone(),
-                echoed_to_us: echo.clone(),
-            }
-            .into());
         }
 
         Ok(Payload::empty())