From a717150bdff8d337cfb00687b9bf7412c4d4e856 Mon Sep 17 00:00:00 2001 From: spaced4ndy <8711996+spaced4ndy@users.noreply.github.com> Date: Tue, 30 Jun 2026 18:26:54 +0400 Subject: [PATCH] wip --- src/Simplex/Chat/Library/Commands.hs | 46 +++++++++++++++----------- src/Simplex/Chat/Library/Subscriber.hs | 14 ++++++-- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index 1c49b0f05a..b468ebd680 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -2756,7 +2756,7 @@ processChatCommand cxt nm = \case -- TODO [relays] possible optimization is to read only required members + relays g@(Group gInfo members) <- withFastStore $ \db -> getGroup db cxt user groupId when (selfSelected gInfo) $ throwCmdError "can't change role for self" - let (invitedMems, currentMems, unchangedMems, maxRole, anyAdmin, anyPending, anyPrivilegedTarget, finalPrivilegedCount) = selectMembers members + let (invitedMems, currentMems, unchangedMems, maxRole, anyAdmin, anyPending, anyPrivilegedTarget, anyRosterChange, finalPrivilegedCount) = selectMembers members when (length invitedMems + length currentMems + length unchangedMems /= length memberIds) $ throwChatError CEGroupMemberNotFound when (length memberIds > 1 && (anyAdmin || newRole >= GRAdmin)) $ throwCmdError "can't change role of multiple members when admins selected, or new role is admin" @@ -2768,7 +2768,7 @@ processChatCommand cxt nm = \case when (useRelays' gInfo && isRosterRole newRole && finalPrivilegedCount > maxGroupRosterSize) $ throwCmdError $ "the number of members, moderators and admins would exceed the limit of " <> show maxGroupRosterSize (errs1, changed1) <- changeRoleInvitedMems user gInfo invitedMems - let doBumpRoster = useRelays' gInfo && memberRole' (membership gInfo) == GROwner && (isRosterRole newRole || anyPrivilegedTarget) + let doBumpRoster = useRelays' gInfo && memberRole' (membership gInfo) == GROwner && anyRosterChange -- roster (with the change projected in) before the delta, so a relay stores the blob at this version before forwarding the delta rosterVer <- if doBumpRoster then Just <$> broadcastRoster user gInfo (RDRoleChanged newRole currentMems) else pure Nothing (errs2, changed2, acis, msgSigned) <- changeRoleCurrentMems user g rosterVer currentMems @@ -2778,12 +2778,13 @@ processChatCommand cxt nm = \case pure $ CRMembersRoleUser {user, groupInfo = gInfo, members = changed1 <> changed2, toRole = newRole, msgSigned} -- same order is not guaranteed where selfSelected GroupInfo {membership} = elem (groupMemberId' membership) memberIds - -- anyPrivilegedTarget: a target currently moderator/admin; finalPrivilegedCount: - -- moderators + admins after the change (targets take newRole, others keep their role). - selectMembers :: [GroupMember] -> ([GroupMember], [GroupMember], [GroupMember], GroupMemberRole, Bool, Bool, Bool, Int) - selectMembers = foldr' addMember ([], [], [], GRObserver, False, False, False, 0) + -- anyPrivilegedTarget: a target currently moderator/admin (gates the owner-only check); anyRosterChange: + -- a current member's role change that alters the roster blob - the only case that bumps the version, since a + -- bump with no delta reads as a gap to subscribers; finalPrivilegedCount: moderators + admins after the change. + selectMembers :: [GroupMember] -> ([GroupMember], [GroupMember], [GroupMember], GroupMemberRole, Bool, Bool, Bool, Bool, Int) + selectMembers = foldr' addMember ([], [], [], GRObserver, False, False, False, False, 0) where - addMember m@GroupMember {groupMemberId, memberStatus, memberRole} (invited, current, unchanged, maxRole, anyAdmin, anyPending, anyPrivTarget, privCount) + addMember m@GroupMember {groupMemberId, memberStatus, memberRole} (invited, current, unchanged, maxRole, anyAdmin, anyPending, anyPrivTarget, anyRosterChange, privCount) | groupMemberId `elem` memberIds = let maxRole' = max maxRole memberRole anyAdmin' = anyAdmin || memberRole >= GRAdmin @@ -2791,10 +2792,11 @@ processChatCommand cxt nm = \case anyPrivTarget' = anyPrivTarget || isRosterRole memberRole privCount' = if isRosterRole newRole then privCount + 1 else privCount in if - | memberRole == newRole -> (invited, current, m : unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', privCount') - | memberStatus == GSMemInvited -> (m : invited, current, unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', privCount') - | otherwise -> (invited, m : current, unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', privCount') - | otherwise = (invited, current, unchanged, maxRole, anyAdmin, anyPending, anyPrivTarget, if isRosterRole memberRole then privCount + 1 else privCount) + | memberRole == newRole -> (invited, current, m : unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', anyRosterChange, privCount') + | memberStatus == GSMemInvited -> (m : invited, current, unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', anyRosterChange, privCount') + -- a current member's role actually changes here; it alters the roster iff the old or new role is on it + | otherwise -> (invited, m : current, unchanged, maxRole', anyAdmin', anyPending', anyPrivTarget', anyRosterChange || isRosterRole newRole || isRosterRole memberRole, privCount') + | otherwise = (invited, current, unchanged, maxRole, anyAdmin, anyPending, anyPrivTarget, anyRosterChange, if isRosterRole memberRole then privCount + 1 else privCount) changeRoleInvitedMems :: User -> GroupInfo -> [GroupMember] -> CM ([ChatError], [GroupMember]) changeRoleInvitedMems user gInfo memsToChange = do -- not batched, as we need to send different invitations to different connections anyway @@ -2886,7 +2888,7 @@ processChatCommand cxt nm = \case withGroupLock "removeMembers" groupId $ do -- TODO [relays] possible optimization is to read only required members + relays Group gInfo members <- withFastStore $ \db -> getGroup db cxt user groupId - let (count, invitedMems, pendingApprvMems, pendingRvwMems, currentMems, maxRole, anyAdmin, anyPrivilegedRemoved) = selectMembers gmIds members + let (count, invitedMems, pendingApprvMems, pendingRvwMems, currentMems, maxRole, anyAdmin, anyPrivilegedRemoved, anyRosterRemoved) = selectMembers gmIds members gmIds = S.fromList $ L.toList groupMemberIds memCount = length groupMemberIds when (count /= memCount) $ throwChatError CEGroupMemberNotFound @@ -2896,7 +2898,7 @@ processChatCommand cxt nm = \case throwCmdError "only the group owner can remove members, moderators and admins" (errs1, deleted1) <- deleteInvitedMems user invitedMems let recipients = filter memberCurrent members - let doBumpRoster = useRelays' gInfo && memberRole' (membership gInfo) == GROwner && anyPrivilegedRemoved + let doBumpRoster = useRelays' gInfo && memberRole' (membership gInfo) == GROwner && anyRosterRemoved -- roster (excluding the removed members) before the delta, so a relay stores the blob at this version before forwarding the delta rosterVer <- if doBumpRoster then Just <$> broadcastRoster user gInfo (RDRemoved currentMems) else pure Nothing (errs2, deleted2, acis2, signed2) <- deleteMemsSend user gInfo Nothing rosterVer recipients currentMems @@ -2919,20 +2921,24 @@ processChatCommand cxt nm = \case unless (null errs) $ toView $ CEvtChatErrors errs pure $ CRUserDeletedMembers user gInfo' deleted withMessages msgSigned -- same order is not guaranteed where - selectMembers :: S.Set GroupMemberId -> [GroupMember] -> (Int, [GroupMember], [GroupMember], [GroupMember], [GroupMember], GroupMemberRole, Bool, Bool) - selectMembers gmIds = foldl' addMember (0, [], [], [], [], GRObserver, False, False) + -- anyPrivilegedRemoved: any removed member is moderator/admin (gates the owner-only check); anyRosterRemoved: + -- a current roster member is removed - the only case that alters the blob and so bumps the version (pending/ + -- invited members aren't on the roster, and a bump with no delta reads as a gap to subscribers). + selectMembers :: S.Set GroupMemberId -> [GroupMember] -> (Int, [GroupMember], [GroupMember], [GroupMember], [GroupMember], GroupMemberRole, Bool, Bool, Bool) + selectMembers gmIds = foldl' addMember (0, [], [], [], [], GRObserver, False, False, False) where - addMember acc@(n, invited, pendingApprv, pendingRvw, current, maxRole, anyAdmin, anyPrivRemoved) m@GroupMember {groupMemberId, memberStatus, memberRole} + addMember acc@(n, invited, pendingApprv, pendingRvw, current, maxRole, anyAdmin, anyPrivRemoved, anyRosterRemoved) m@GroupMember {groupMemberId, memberStatus, memberRole} | groupMemberId `S.member` gmIds = let maxRole' = max maxRole memberRole anyAdmin' = anyAdmin || memberRole >= GRAdmin anyPrivRemoved' = anyPrivRemoved || isRosterRole memberRole n' = n + 1 in case memberStatus of - GSMemInvited -> (n', m : invited, pendingApprv, pendingRvw, current, maxRole', anyAdmin', anyPrivRemoved') - GSMemPendingApproval -> (n', invited, m : pendingApprv, pendingRvw, current, maxRole', anyAdmin', anyPrivRemoved') - GSMemPendingReview -> (n', invited, pendingApprv, m : pendingRvw, current, maxRole', anyAdmin', anyPrivRemoved') - _ -> (n', invited, pendingApprv, pendingRvw, m : current, maxRole', anyAdmin', anyPrivRemoved') + GSMemInvited -> (n', m : invited, pendingApprv, pendingRvw, current, maxRole', anyAdmin', anyPrivRemoved', anyRosterRemoved) + GSMemPendingApproval -> (n', invited, m : pendingApprv, pendingRvw, current, maxRole', anyAdmin', anyPrivRemoved', anyRosterRemoved) + GSMemPendingReview -> (n', invited, pendingApprv, m : pendingRvw, current, maxRole', anyAdmin', anyPrivRemoved', anyRosterRemoved) + -- removed current member: alters the roster blob iff it currently holds a roster role + _ -> (n', invited, pendingApprv, pendingRvw, m : current, maxRole', anyAdmin', anyPrivRemoved', anyRosterRemoved || isRosterRole memberRole) | otherwise = acc deleteInvitedMems :: User -> [GroupMember] -> CM ([ChatError], [GroupMember]) deleteInvitedMems user memsToDelete = do diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index 23f918dcab..c06c8686c7 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -3258,14 +3258,22 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = let fresh = maybe True (v >=) gate when fresh $ do setGroupRosterVersion db gInfo v - -- advance the complete frontier only when this delta is the next version (no gap) - when (maybe False (\(VersionRoster c) -> v == VersionRoster (c + 1)) prevComplete) $ + -- advance the frontier when this delta is the next version. One version can carry several deltas + -- (a multi-member role change), delivered in order, so seeing any one of them advances the frontier + -- past that whole version; a relay that drops some deltas within a version is not detected here. + when (v == nextCompleteVersion prevComplete) $ setCompleteRosterVersion db gInfo v pure (fresh, prevComplete) if accept then (requestRosterOnGap v prevComplete `catchAllErrors` eToView) >> action else messageWarning "x.grp.mem: roster version not newer than current, ignoring" $> Nothing where + -- the next contiguous version after the complete frontier. With no frontier the baseline is the first + -- roster version (VersionRoster 0, see broadcastRoster): a subscriber seeing v0 without a prior roster is + -- current, not gapped, as it cannot have missed an earlier version - so v0 neither requests nor stays behind. + nextCompleteVersion = \case + Just (VersionRoster c) -> VersionRoster (c + 1) + Nothing -> VersionRoster 0 -- a subscriber whose complete frontier (before this delta) lags more than one below it has missed versions: -- ask the relay that forwarded it (it holds >= v = the new gate) to re-serve the full roster, carrying the -- previous frontier so only a fuller snapshot is served. A stuck frontier re-asks on every following delta @@ -3279,7 +3287,7 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = void $ sendGroupMessage' user gInfo [relay] (XGrpRosterRequest prevComplete) _ -> pure () where - gap = maybe True (\(VersionRoster c) -> v > VersionRoster (c + 1)) prevComplete + gap = v > nextCompleteVersion prevComplete xGrpMemRole :: GroupInfo -> Maybe GroupMember -> GroupMember -> MemberId -> GroupMemberRole -> Maybe MemberKey -> Maybe VersionRoster -> RcvMessage -> UTCTime -> CM (Maybe DeliveryJobScope) xGrpMemRole gInfo@GroupInfo {membership} fwdRelay_ m@GroupMember {memberRole = senderRole} memId memRole memberKey_ rosterVer_ msg@RcvMessage {msgSigned} brokerTs