chore: add the epoch of the buffered message to the buffered future m…

…essage error This is to enable logging the epoch after buffering a message.
wireapp · Jan 27, 2025 · 91846bb · 91846bb
1 parent d82b3dd
commit 91846bb
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 12 deletions.
diff --git a/crypto-ffi/src/generic/mod.rs b/crypto-ffi/src/generic/mod.rs
@@ -209,7 +209,7 @@ impl From<CryptoError> for CoreCryptoError {
         match value {
             CryptoError::ConversationAlreadyExists(id) => MlsError::ConversationAlreadyExists(id).into(),
             CryptoError::DuplicateMessage => MlsError::DuplicateMessage.into(),
-            CryptoError::BufferedFutureMessage => MlsError::BufferedFutureMessage.into(),
+            CryptoError::BufferedFutureMessage { .. } => MlsError::BufferedFutureMessage.into(),
             CryptoError::WrongEpoch => MlsError::WrongEpoch.into(),
             CryptoError::MessageEpochTooOld => MlsError::MessageEpochTooOld.into(),
             CryptoError::SelfCommitIgnored => MlsError::SelfCommitIgnored.into(),

diff --git a/crypto-ffi/src/wasm/mod.rs b/crypto-ffi/src/wasm/mod.rs
@@ -211,7 +211,7 @@ impl From<CryptoError> for InternalError {
         match value {
             CryptoError::ConversationAlreadyExists(id) => MlsError::ConversationAlreadyExists(id).into(),
             CryptoError::DuplicateMessage => MlsError::DuplicateMessage.into(),
-            CryptoError::BufferedFutureMessage => MlsError::BufferedFutureMessage.into(),
+            CryptoError::BufferedFutureMessage { .. } => MlsError::BufferedFutureMessage.into(),
             CryptoError::WrongEpoch => MlsError::WrongEpoch.into(),
             CryptoError::MessageEpochTooOld => MlsError::MessageEpochTooOld.into(),
             CryptoError::SelfCommitIgnored => MlsError::SelfCommitIgnored.into(),

diff --git a/crypto/src/error.rs b/crypto/src/error.rs
@@ -139,7 +139,10 @@ pub enum CryptoError {
     WrongEpoch,
     /// Incoming message is for a future epoch. We will buffer it until the commit for that epoch arrives
     #[error("Incoming message is for a future epoch. We will buffer it until the commit for that epoch arrives")]
-    BufferedFutureMessage,
+    BufferedFutureMessage {
+        /// The epoch of the buffered message
+        message_epoch: u64,
+    },
     /// Proteus Error Wrapper
     #[error(transparent)]
     ProteusError(#[from] ProteusError),

diff --git a/crypto/src/mls/buffer_external_commit.rs b/crypto/src/mls/buffer_external_commit.rs
@@ -192,9 +192,15 @@ mod tests {
 
                     // Since Alice missed Bob's commit she should buffer this message
                     let decrypt = alice_central.context.decrypt_message(&id, msg1).await;
-                    assert!(matches!(decrypt.unwrap_err(), CryptoError::BufferedFutureMessage));
+                    assert!(matches!(
+                        decrypt.unwrap_err(),
+                        CryptoError::BufferedFutureMessage { .. }
+                    ));
                     let decrypt = alice_central.context.decrypt_message(&id, msg2).await;
-                    assert!(matches!(decrypt.unwrap_err(), CryptoError::BufferedFutureMessage));
+                    assert!(matches!(
+                        decrypt.unwrap_err(),
+                        CryptoError::BufferedFutureMessage { .. }
+                    ));
                     assert_eq!(alice_central.context.count_entities().await.pending_messages, 2);
 
                     let gi = bob_central.get_group_info(&id).await;

diff --git a/crypto/src/mls/conversation/buffer_messages.rs b/crypto/src/mls/conversation/buffer_messages.rs
@@ -216,10 +216,16 @@ mod tests {
                         .map(|m| m.to_bytes().unwrap());
                     for m in messages {
                         let decrypt = bob_central.context.decrypt_message(&id, m).await;
-                        assert!(matches!(decrypt.unwrap_err(), CryptoError::BufferedFutureMessage));
+                        assert!(matches!(
+                            decrypt.unwrap_err(),
+                            CryptoError::BufferedFutureMessage { .. }
+                        ));
                     }
                     let decrypt = bob_central.context.decrypt_message(&id, app_msg).await;
-                    assert!(matches!(decrypt.unwrap_err(), CryptoError::BufferedFutureMessage));
+                    assert!(matches!(
+                        decrypt.unwrap_err(),
+                        CryptoError::BufferedFutureMessage { .. }
+                    ));
 
                     // Bob should have buffered the messages
                     assert_eq!(bob_central.context.count_entities().await.pending_messages, 4);
@@ -343,10 +349,16 @@ mod tests {
                             .map(|m| m.to_bytes().unwrap());
                         for m in messages {
                             let decrypt = alice_central.context.decrypt_message(&id, m).await;
-                            assert!(matches!(decrypt.unwrap_err(), CryptoError::BufferedFutureMessage));
+                            assert!(matches!(
+                                decrypt.unwrap_err(),
+                                CryptoError::BufferedFutureMessage { .. }
+                            ));
                         }
                         let decrypt = alice_central.context.decrypt_message(&id, app_msg).await;
-                        assert!(matches!(decrypt.unwrap_err(), CryptoError::BufferedFutureMessage));
+                        assert!(matches!(
+                            decrypt.unwrap_err(),
+                            CryptoError::BufferedFutureMessage { .. }
+                        ));
 
                         // Alice should have buffered the messages
                         assert_eq!(alice_central.context.count_entities().await.pending_messages, 4);

diff --git a/crypto/src/mls/conversation/commit.rs b/crypto/src/mls/conversation/commit.rs
@@ -1112,7 +1112,10 @@ mod tests {
 
                     // fails when a commit is skipped
                     let out_of_order = bob_central.context.decrypt_message(&id, &commit2).await;
-                    assert!(matches!(out_of_order.unwrap_err(), CryptoError::BufferedFutureMessage));
+                    assert!(matches!(
+                        out_of_order.unwrap_err(),
+                        CryptoError::BufferedFutureMessage { .. }
+                    ));
 
                     // works in the right order though
                     // NB: here 'commit2' has been buffered so it is also applied when we decrypt commit1

diff --git a/crypto/src/mls/conversation/decrypt.rs b/crypto/src/mls/conversation/decrypt.rs
@@ -347,7 +347,9 @@ impl MlsConversation {
                     } else if msg_epoch == group_epoch + 1 {
                         // limit to next epoch otherwise if we were buffering a commit for epoch + 2
                         // we would fail when trying to decrypt it in [MlsCentral::commit_accepted]
-                        CryptoError::BufferedFutureMessage
+                        CryptoError::BufferedFutureMessage {
+                            message_epoch: msg_epoch,
+                        }
                     } else if msg_epoch < group_epoch {
                         match content_type {
                             ContentType::Application => CryptoError::StaleMessage,
@@ -1291,7 +1293,10 @@ mod tests {
 
                     // which Bob cannot decrypt because of Post CompromiseSecurity
                     let decrypt = bob_central.context.decrypt_message(&id, &encrypted).await;
-                    assert!(matches!(decrypt.unwrap_err(), CryptoError::BufferedFutureMessage));
+                    assert!(matches!(
+                        decrypt.unwrap_err(),
+                        CryptoError::BufferedFutureMessage { .. }
+                    ));
 
                     let decrypted_commit = bob_central
                         .context