Only resend proposals once after external commit (#4103)

* Only resend proposals once after external commit

* Add CHANGELOG entry

* Log when duplicate proposals are found

* Regenerate nix packages

changelog.d/3-bug-fixes/filter-duplicates-when-resending-props

@@ -0,0 +1 @@
+Only resend proposals once after external commit

services/galley/default.nix

@@ -44,6 +44,7 @@
 , galley-types
 , gitignoreSource
 , gundeck-types
+, hex
 , HsOpenSSL
 , http-api-data
 , http-client
@@ -164,6 +165,7 @@ mkDerivation {
     extra
     galley-types
     gundeck-types
+    hex
     HsOpenSSL
     http-client
     http-client-openssl

services/galley/galley.cabal

@@ -312,6 +312,7 @@ library
     , extra                  >=1.3
     , galley-types           >=0.65.0
     , gundeck-types          >=1.35.2
+    , hex
     , HsOpenSSL              >=0.11
     , http-client            >=0.7
     , http-client-openssl    >=0.2

services/galley/src/Galley/API/MLS/Commit/ExternalCommit.hs

@@ -168,15 +168,15 @@ processExternalCommit senderIdentity lConvOrSub ciphersuite epoch action updateP
     lConvOrSub' <- for lConvOrSub incrementEpoch
 
     -- fetch backend remove proposals of the previous epoch
-    indicesInRemoveProposals <-
-      -- skip remove proposals of already removed by the external commit
-      (\\ toList action.remove)
-        <$> getPendingBackendRemoveProposals groupId epoch
+    indices0 <- getPendingBackendRemoveProposals groupId epoch
+
+    -- skip proposals for clients already removed by the external commit
+    let indices = maybe id Set.delete action.remove indices0
 
     -- requeue backend remove proposals for the current epoch
     createAndSendRemoveProposals
       lConvOrSub'
-      indicesInRemoveProposals
+      indices
       (cidQualifiedUser senderIdentity)
       (tUnqualified lConvOrSub').members
 

services/galley/src/Galley/API/MLS/Util.hs

@@ -18,8 +18,10 @@
 module Galley.API.MLS.Util where
 
 import Control.Comonad
+import Data.Hex
 import Data.Id
 import Data.Qualified
+import Data.Set qualified as Set
 import Data.Text qualified as T
 import Galley.Data.Conversation.Types hiding (Conversation)
 import Galley.Data.Conversation.Types qualified as Data
@@ -77,21 +79,30 @@ getPendingBackendRemoveProposals ::
   ) =>
   GroupId ->
   Epoch ->
-  Sem r [LeafIndex]
+  Sem r (Set LeafIndex)
 getPendingBackendRemoveProposals gid epoch = do
   proposals <- getAllPendingProposals gid epoch
-  catMaybes
-    <$> for
-      proposals
-      ( \case
-          (Just ProposalOriginBackend, proposal) -> case value proposal of
-            RemoveProposal i -> pure (Just i)
-            _ -> pure Nothing
-          (Just ProposalOriginClient, _) -> pure Nothing
-          (Nothing, _) -> do
-            TinyLog.warn $ Log.msg ("found pending proposal without origin, ignoring" :: ByteString)
-            pure Nothing
-      )
+  indexList <-
+    catMaybes
+      <$> for
+        proposals
+        ( \case
+            (Just ProposalOriginBackend, proposal) -> case proposal.value of
+              RemoveProposal i -> pure (Just i)
+              _ -> pure Nothing
+            (Just ProposalOriginClient, _) -> pure Nothing
+            (Nothing, _) -> do
+              TinyLog.warn $ Log.msg ("found pending proposal without origin, ignoring" :: ByteString)
+              pure Nothing
+        )
+
+  let indexSet = Set.fromList indexList
+  when (length indexList /= length indexSet) $ do
+    TinyLog.warn $
+      Log.msg ("found duplicate proposals" :: ByteString)
+        . Log.field "groupId" ("0x" <> hex (unGroupId gid))
+        . Log.field "epoch" (epochNumber epoch)
+  pure indexSet
 
 withCommitLock ::
   forall r a.