Merge pull request #378 from mpretty-cyro/feature/sort-members-by-pubkey

Sort group member change control messages deterministically
pull/1061/head
Morgan Pretty 2 months ago committed by GitHub
commit c1dee3e9b7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -625,6 +625,28 @@ public extension ClosedGroup {
} }
} }
public extension Collection where Element == (String, Profile?) {
func sortedById(userSessionId: SessionId) -> [Element] {
return sorted { lhs, rhs in
guard lhs.0 != userSessionId.hexString else { return true }
guard rhs.0 != userSessionId.hexString else { return false }
return (lhs.0 < rhs.0)
}
}
}
public extension Collection where Element == String {
func sortedById(userSessionId: SessionId) -> [Element] {
return sorted { lhs, rhs in
guard lhs != userSessionId.hexString else { return true }
guard rhs != userSessionId.hexString else { return false }
return (lhs < rhs)
}
}
}
public extension [ClosedGroup.RemovableGroupData] { public extension [ClosedGroup.RemovableGroupData] {
static var allData: [ClosedGroup.RemovableGroupData] { ClosedGroup.RemovableGroupData.allCases } static var allData: [ClosedGroup.RemovableGroupData] { ClosedGroup.RemovableGroupData.allCases }
static var noData: [ClosedGroup.RemovableGroupData] { [] } static var noData: [ClosedGroup.RemovableGroupData] { [] }

@ -433,7 +433,7 @@ extension MessageReceiver {
.defaulting(to: []) .defaulting(to: [])
.reduce(into: [:]) { result, next in result[next.id] = next } .reduce(into: [:]) { result, next in result[next.id] = next }
let names: [String] = message.memberSessionIds let names: [String] = message.memberSessionIds
.sorted { lhs, rhs in lhs == userSessionId.hexString } .sortedById(userSessionId: userSessionId)
.map { id in .map { id in
profiles[id]?.displayName(for: .group) ?? profiles[id]?.displayName(for: .group) ??
Profile.truncated(id: id, truncating: .middle) Profile.truncated(id: id, truncating: .middle)

@ -23,6 +23,11 @@ extension MessageSender {
members: [(String, Profile?)], members: [(String, Profile?)],
using dependencies: Dependencies using dependencies: Dependencies
) -> AnyPublisher<SessionThread, Error> { ) -> AnyPublisher<SessionThread, Error> {
let userSessionId: SessionId = dependencies[cache: .general].sessionId
let sortedOtherMembers: [(String, Profile?)] = members
.filter { id, _ in id != userSessionId.hexString }
.sortedById(userSessionId: userSessionId)
return Just(()) return Just(())
.setFailureType(to: Error.self) .setFailureType(to: Error.self)
.flatMap { _ -> AnyPublisher<DisplayPictureManager.UploadResult?, Error> in .flatMap { _ -> AnyPublisher<DisplayPictureManager.UploadResult?, Error> in
@ -40,8 +45,6 @@ extension MessageSender {
} }
.flatMap { (displayPictureInfo: DisplayPictureManager.UploadResult?) -> AnyPublisher<PreparedGroupData, Error> in .flatMap { (displayPictureInfo: DisplayPictureManager.UploadResult?) -> AnyPublisher<PreparedGroupData, Error> in
dependencies[singleton: .storage].writePublisher { db -> PreparedGroupData in dependencies[singleton: .storage].writePublisher { db -> PreparedGroupData in
let userSessionId: SessionId = dependencies[cache: .general].sessionId
/// Create and cache the libSession entries /// Create and cache the libSession entries
let createdInfo: LibSession.CreatedGroupInfo = try LibSession.createGroup( let createdInfo: LibSession.CreatedGroupInfo = try LibSession.createGroup(
db, db,
@ -77,12 +80,10 @@ extension MessageSender {
body: ClosedGroup.MessageInfo body: ClosedGroup.MessageInfo
.addedUsers( .addedUsers(
hasCurrentUser: false, hasCurrentUser: false,
names: members names: sortedOtherMembers.map { id, profile in
.filter { id, _ in id != userSessionId.hexString } profile?.displayName(for: .group) ??
.map { id, profile in Profile.truncated(id: id, truncating: .middle)
profile?.displayName(for: .group) ?? },
Profile.truncated(id: id, truncating: .middle)
},
historyShared: false historyShared: false
) )
.infoString(using: dependencies), .infoString(using: dependencies),
@ -101,9 +102,7 @@ extension MessageSender {
destination: .closedGroup(groupPublicKey: createdInfo.group.id), destination: .closedGroup(groupPublicKey: createdInfo.group.id),
message: GroupUpdateMemberChangeMessage( message: GroupUpdateMemberChangeMessage(
changeType: .added, changeType: .added,
memberSessionIds: members memberSessionIds: sortedOtherMembers.map { id, _ in id },
.filter { id, _ -> Bool in id != userSessionId.hexString }
.map { id, _ in id },
historyShared: false, historyShared: false,
sentTimestampMs: UInt64(createdInfo.group.formationTimestamp * 1000), sentTimestampMs: UInt64(createdInfo.group.formationTimestamp * 1000),
authMethod: Authentication.groupAdmin( authMethod: Authentication.groupAdmin(
@ -553,7 +552,7 @@ extension MessageSender {
public static func addGroupMembers( public static func addGroupMembers(
groupSessionId: String, groupSessionId: String,
members: [(id: String, profile: Profile?)], members: [(String, Profile?)],
allowAccessToHistoricMessages: Bool, allowAccessToHistoricMessages: Bool,
using dependencies: Dependencies using dependencies: Dependencies
) -> AnyPublisher<Void, Error> { ) -> AnyPublisher<Void, Error> {
@ -568,6 +567,10 @@ extension MessageSender {
subaccountToken: [UInt8] subaccountToken: [UInt8]
) )
let userSessionId: SessionId = dependencies[cache: .general].sessionId
let sortedMembers: [(String, Profile?)] = members
.sortedById(userSessionId: userSessionId)
return dependencies[singleton: .storage] return dependencies[singleton: .storage]
.writePublisher { db -> ([MemberJobData], Network.PreparedRequest<Void>, Network.PreparedRequest<Void>?) in .writePublisher { db -> ([MemberJobData], Network.PreparedRequest<Void>, Network.PreparedRequest<Void>?) in
guard guard
@ -578,7 +581,6 @@ extension MessageSender {
.fetchOne(db) .fetchOne(db)
else { throw MessageSenderError.invalidClosedGroupUpdate } else { throw MessageSenderError.invalidClosedGroupUpdate }
let userSessionId: SessionId = dependencies[cache: .general].sessionId
let changeTimestampMs: Int64 = dependencies[cache: .snodeAPI].currentOffsetTimestampMs() let changeTimestampMs: Int64 = dependencies[cache: .snodeAPI].currentOffsetTimestampMs()
var maybeSupplementalKeyRequest: Network.PreparedRequest<Void>? var maybeSupplementalKeyRequest: Network.PreparedRequest<Void>?
@ -601,7 +603,7 @@ extension MessageSender {
let supplementData: Data = try LibSession.keySupplement( let supplementData: Data = try LibSession.keySupplement(
db, db,
groupSessionId: sessionId, groupSessionId: sessionId,
memberIds: members.map { $0.id }.asSet(), memberIds: members.map { id, _ in id }.asSet(),
using: dependencies using: dependencies
) )
@ -708,13 +710,11 @@ extension MessageSender {
variant: .infoGroupMembersUpdated, variant: .infoGroupMembersUpdated,
body: ClosedGroup.MessageInfo body: ClosedGroup.MessageInfo
.addedUsers( .addedUsers(
hasCurrentUser: members.map { $0.id }.contains(userSessionId.hexString), hasCurrentUser: members.contains { id, _ in id == userSessionId.hexString },
names: members names: sortedMembers.map { id, profile in
.sorted { lhs, rhs in lhs.id == userSessionId.hexString } profile?.displayName(for: .group) ??
.map { id, profile in Profile.truncated(id: id, truncating: .middle)
profile?.displayName(for: .group) ?? },
Profile.truncated(id: id, truncating: .middle)
},
historyShared: allowAccessToHistoricMessages historyShared: allowAccessToHistoricMessages
) )
.infoString(using: dependencies), .infoString(using: dependencies),
@ -737,7 +737,7 @@ extension MessageSender {
destination: .closedGroup(groupPublicKey: sessionId.hexString), destination: .closedGroup(groupPublicKey: sessionId.hexString),
message: GroupUpdateMemberChangeMessage( message: GroupUpdateMemberChangeMessage(
changeType: .added, changeType: .added,
memberSessionIds: members.map { $0.id }, memberSessionIds: sortedMembers.map { id, _ in id },
historyShared: allowAccessToHistoricMessages, historyShared: allowAccessToHistoricMessages,
sentTimestampMs: UInt64(changeTimestampMs), sentTimestampMs: UInt64(changeTimestampMs),
authMethod: Authentication.groupAdmin( authMethod: Authentication.groupAdmin(
@ -979,6 +979,9 @@ extension MessageSender {
dependencies[cache: .snodeAPI].currentOffsetTimestampMs() dependencies[cache: .snodeAPI].currentOffsetTimestampMs()
) )
let userSessionId: SessionId = dependencies[cache: .general].sessionId
let sortedMemberIds: [String] = memberIds.sortedById(userSessionId: userSessionId)
return dependencies[singleton: .storage] return dependencies[singleton: .storage]
.writePublisher { db in .writePublisher { db in
guard guard
@ -1028,7 +1031,6 @@ extension MessageSender {
/// Send the member changed message if desired /// Send the member changed message if desired
if sendMemberChangedMessage { if sendMemberChangedMessage {
let userSessionId: SessionId = dependencies[cache: .general].sessionId
let removedMemberProfiles: [String: Profile] = (try? Profile let removedMemberProfiles: [String: Profile] = (try? Profile
.filter(ids: memberIds) .filter(ids: memberIds)
.fetchAll(db)) .fetchAll(db))
@ -1045,12 +1047,10 @@ extension MessageSender {
body: ClosedGroup.MessageInfo body: ClosedGroup.MessageInfo
.removedUsers( .removedUsers(
hasCurrentUser: memberIds.contains(userSessionId.hexString), hasCurrentUser: memberIds.contains(userSessionId.hexString),
names: memberIds names: sortedMemberIds.map { id in
.sorted { lhs, rhs in lhs == userSessionId.hexString } removedMemberProfiles[id]?.displayName(for: .group) ??
.map { id in Profile.truncated(id: id, truncating: .middle)
removedMemberProfiles[id]?.displayName(for: .group) ?? }
Profile.truncated(id: id, truncating: .middle)
}
) )
.infoString(using: dependencies), .infoString(using: dependencies),
timestampMs: targetChangeTimestampMs, timestampMs: targetChangeTimestampMs,
@ -1072,7 +1072,7 @@ extension MessageSender {
destination: .closedGroup(groupPublicKey: sessionId.hexString), destination: .closedGroup(groupPublicKey: sessionId.hexString),
message: GroupUpdateMemberChangeMessage( message: GroupUpdateMemberChangeMessage(
changeType: .removed, changeType: .removed,
memberSessionIds: Array(memberIds), memberSessionIds: sortedMemberIds,
historyShared: false, historyShared: false,
sentTimestampMs: UInt64(targetChangeTimestampMs), sentTimestampMs: UInt64(targetChangeTimestampMs),
authMethod: Authentication.groupAdmin( authMethod: Authentication.groupAdmin(
@ -1098,10 +1098,12 @@ extension MessageSender {
public static func promoteGroupMembers( public static func promoteGroupMembers(
groupSessionId: SessionId, groupSessionId: SessionId,
members: [(id: String, profile: Profile?)], members: [(String, Profile?)],
isResend: Bool, isResend: Bool,
using dependencies: Dependencies using dependencies: Dependencies
) -> AnyPublisher<Void, Error> { ) -> AnyPublisher<Void, Error> {
let userSessionId: SessionId = dependencies[cache: .general].sessionId
return dependencies[singleton: .storage] return dependencies[singleton: .storage]
.writePublisher { db -> Set<String> in .writePublisher { db -> Set<String> in
guard guard
@ -1113,7 +1115,7 @@ extension MessageSender {
else { throw MessageSenderError.invalidClosedGroupUpdate } else { throw MessageSenderError.invalidClosedGroupUpdate }
/// Determine which members actually need to be promoted (rather than just resent promotions) /// Determine which members actually need to be promoted (rather than just resent promotions)
let memberIds: Set<String> = Set(members.map(\.id)) let memberIds: Set<String> = Set(members.map { id, _ in id })
let memberIdsRequiringPromotions: Set<String> = try GroupMember let memberIdsRequiringPromotions: Set<String> = try GroupMember
.select(.profileId) .select(.profileId)
.filter(GroupMember.Columns.groupId == groupSessionId.hexString) .filter(GroupMember.Columns.groupId == groupSessionId.hexString)
@ -1121,8 +1123,10 @@ extension MessageSender {
.filter(GroupMember.Columns.role == GroupMember.Role.standard) .filter(GroupMember.Columns.role == GroupMember.Role.standard)
.asRequest(of: String.self) .asRequest(of: String.self)
.fetchSet(db) .fetchSet(db)
let membersReceivingPromotions: [(id: String, profile: Profile?)] = members let membersReceivingPromotions: [(String, Profile?)] = members
.filter { id, _ in memberIdsRequiringPromotions.contains(id) } .filter { id, _ in memberIdsRequiringPromotions.contains(id) }
let sortedMembersReceivingPromotions: [(String, Profile?)] = membersReceivingPromotions
.sortedById(userSessionId: userSessionId)
/// Perform the config changes without triggering a config sync (we will do so manually after the process completes) /// Perform the config changes without triggering a config sync (we will do so manually after the process completes)
try dependencies.mutate(cache: .libSession) { cache in try dependencies.mutate(cache: .libSession) { cache in
@ -1173,7 +1177,6 @@ extension MessageSender {
/// that are getting promotions re-sent to them - we only want to send an admin changed message if there /// that are getting promotions re-sent to them - we only want to send an admin changed message if there
/// is a newly promoted member /// is a newly promoted member
if !isResend && !membersReceivingPromotions.isEmpty { if !isResend && !membersReceivingPromotions.isEmpty {
let userSessionId: SessionId = dependencies[cache: .general].sessionId
let changeTimestampMs: Int64 = dependencies[cache: .snodeAPI].currentOffsetTimestampMs() let changeTimestampMs: Int64 = dependencies[cache: .snodeAPI].currentOffsetTimestampMs()
let disappearingConfig: DisappearingMessagesConfiguration? = try? DisappearingMessagesConfiguration.fetchOne(db, id: groupSessionId.hexString) let disappearingConfig: DisappearingMessagesConfiguration? = try? DisappearingMessagesConfiguration.fetchOne(db, id: groupSessionId.hexString)
@ -1185,14 +1188,12 @@ extension MessageSender {
body: ClosedGroup.MessageInfo body: ClosedGroup.MessageInfo
.promotedUsers( .promotedUsers(
hasCurrentUser: membersReceivingPromotions hasCurrentUser: membersReceivingPromotions
.map { $0.id } .map { id, _ in id }
.contains(userSessionId.hexString), .contains(userSessionId.hexString),
names: membersReceivingPromotions names: sortedMembersReceivingPromotions.map { id, profile in
.sorted { lhs, rhs in lhs.id == userSessionId.hexString } profile?.displayName(for: .group) ??
.map { id, profile in Profile.truncated(id: id, truncating: .middle)
profile?.displayName(for: .group) ?? }
Profile.truncated(id: id, truncating: .middle)
}
) )
.infoString(using: dependencies), .infoString(using: dependencies),
timestampMs: changeTimestampMs, timestampMs: changeTimestampMs,
@ -1214,7 +1215,7 @@ extension MessageSender {
destination: .closedGroup(groupPublicKey: groupSessionId.hexString), destination: .closedGroup(groupPublicKey: groupSessionId.hexString),
message: GroupUpdateMemberChangeMessage( message: GroupUpdateMemberChangeMessage(
changeType: .promoted, changeType: .promoted,
memberSessionIds: membersReceivingPromotions.map { $0.id }, memberSessionIds: sortedMembersReceivingPromotions.map { id, _ in id },
historyShared: false, historyShared: false,
sentTimestampMs: UInt64(changeTimestampMs), sentTimestampMs: UInt64(changeTimestampMs),
authMethod: Authentication.groupAdmin( authMethod: Authentication.groupAdmin(

@ -1433,6 +1433,53 @@ class MessageSenderGroupsSpec: QuickSpec {
) )
}) })
} }
// MARK: ---- sorts the members in the control message deterministically
it("sorts the members in the control message deterministically") {
MessageSender.addGroupMembers(
groupSessionId: groupId.hexString,
members: [
("051234111111111111111111111111111111111111111111111111111111111112", nil),
("051111111111111111111111111111111111111111111111111111111111111112", nil),
("05\(TestConstants.publicKey)", nil)
],
allowAccessToHistoricMessages: false,
using: dependencies
).sinkUntilComplete()
expect(mockJobRunner)
.to(call(.exactly(times: 1), matchingParameters: .all) { jobRunner in
jobRunner.add(
.any,
job: Job(
variant: .messageSend,
behaviour: .runOnceAfterConfigSyncIgnoringPermanentFailure,
threadId: groupId.hexString,
details: MessageSendJob.Details(
destination: .closedGroup(groupPublicKey: groupId.hexString),
message: try GroupUpdateMemberChangeMessage(
changeType: .added,
memberSessionIds: [
"05\(TestConstants.publicKey)",
"051111111111111111111111111111111111111111111111111111111111111112",
"051234111111111111111111111111111111111111111111111111111111111112"
],
historyShared: false,
sentTimestampMs: UInt64(1234567890000),
authMethod: Authentication.groupAdmin(
groupSessionId: SessionId(.group, hex: groupId.hexString),
ed25519SecretKey: [1, 2, 3]
),
using: dependencies
),
requiredConfigSyncVariant: .groupMembers
)
),
dependantJob: nil,
canStartJob: false
)
})
}
} }
} }
} }

Loading…
Cancel
Save