From 9b670ff453c8d45a2f0c138653ee7d6a0763df55 Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Tue, 13 Sep 2022 17:07:34 +0100 Subject: [PATCH] Correctly abridge events after changes in joined users --- protocol/common/message_sender.go | 4 +- protocol/v1/membership_update_message.go | 85 +++--- protocol/v1/membership_update_message_test.go | 244 +++++++++--------- 3 files changed, 182 insertions(+), 151 deletions(-) diff --git a/protocol/common/message_sender.go b/protocol/common/message_sender.go index 732cc2c2a..5411aac27 100644 --- a/protocol/common/message_sender.go +++ b/protocol/common/message_sender.go @@ -481,14 +481,14 @@ func (s *MessageSender) EncodeMembershipUpdate( } // EncodeAbridgedMembershipUpdate takes a group and an optional chat message and returns the protobuf representation to be sent on the wire. -// Only the events relevant to the sender are encoded +// Only the events relevant to the current group are encoded func (s *MessageSender) EncodeAbridgedMembershipUpdate( group *v1protocol.Group, chatEntity ChatEntity, ) ([]byte, error) { message := v1protocol.MembershipUpdateMessage{ ChatID: group.ChatID(), - Events: group.AbridgedEvents(&s.identity.PublicKey), + Events: group.AbridgedEvents(), } return s.encodeMembershipUpdate(message, chatEntity) } diff --git a/protocol/v1/membership_update_message.go b/protocol/v1/membership_update_message.go index fdf987cf1..9de189e39 100644 --- a/protocol/v1/membership_update_message.go +++ b/protocol/v1/membership_update_message.go @@ -356,15 +356,23 @@ func isInSlice(m string, set []string) bool { } // AbridgedEvents returns the minimum set of events for a user to publish a post -func (g Group) AbridgedEvents(publicKey *ecdsa.PublicKey) []MembershipUpdateEvent { +// The events we want to keep: +// 1) Chat created +// 2) Latest color changed +// 3) Latest image changed +// 4) For each admin, the latest admins added event that contains them +// 5) For each member, the latest members added event that contains them +// 4 & 5, might bring removed admins or removed members, for those, we also need to +// keep the event that removes them +func (g Group) AbridgedEvents() []MembershipUpdateEvent { var events []MembershipUpdateEvent var nameChangedEventFound bool var colorChangedEventFound bool var imageChangedEventFound bool - var joinedEventFound bool - memberID := publicKeyToString(publicKey) - var addedEventFound bool - nextInChain := memberID + removedMembers := make(map[string]*MembershipUpdateEvent) + addedMembers := make(map[string]bool) + extraMembers := make(map[string]bool) + admins := make(map[string]bool) // Iterate in reverse for i := len(g.events) - 1; i >= 0; i-- { event := g.events[i] @@ -389,45 +397,60 @@ func (g Group) AbridgedEvents(publicKey *ecdsa.PublicKey) []MembershipUpdateEven } events = append(events, event) imageChangedEventFound = true + case protobuf.MembershipUpdateEvent_MEMBERS_ADDED: - // If we already have an added event - // or the user is not in slice, ignore - if addedEventFound { - continue + var shouldAddEvent bool + for _, m := range event.Members { + // If it's adding a current user, and we don't have a more + // recent event + // if it's an admin, we track it + if admins[m] || (g.members.Has(m) && !addedMembers[m]) { + addedMembers[m] = true + shouldAddEvent = true + } } - - areWeTheTarget := isInSlice(nextInChain, event.Members) - - // If it's us, and we have been added by the creator, no more work to do, this is authoritative - if areWeTheTarget && g.events[0].From == event.From { - addedEventFound = true - events = append(events, event) - - } else if areWeTheTarget { - // if it's us and we haven't been added by the creator, we follow the history of whoever invited us - nextInChain = event.From + if shouldAddEvent { + // Append the event and check the not current members that are also + // added + for _, m := range event.Members { + if !g.members.Has(m) && !admins[m] { + extraMembers[m] = true + } + } events = append(events, event) } - case protobuf.MembershipUpdateEvent_MEMBER_JOINED: - if joinedEventFound || event.From != memberID { - continue - } - joinedEventFound = true + case protobuf.MembershipUpdateEvent_ADMIN_REMOVED: + // We add it always for now events = append(events, event) - case protobuf.MembershipUpdateEvent_ADMINS_ADDED: - if isInSlice(nextInChain, event.Members) { + // We track admins in full + admins[event.Members[0]] = true + events = append(events, event) + case protobuf.MembershipUpdateEvent_MEMBER_REMOVED: + // Save member removed events, as we might need it + // to remove members who have been added but subsequently left + if removedMembers[event.Members[0]] == nil || removedMembers[event.Members[0]].ClockValue < event.ClockValue { + removedMembers[event.Members[0]] = &event + } + + case protobuf.MembershipUpdateEvent_MEMBER_JOINED: + if g.members.Has(event.From) { events = append(events, event) } + } - } - // Reverse events - for i, j := 0, len(events)-1; i < j; i, j = i+1, j-1 { - events[i], events[j] = events[j], events[i] + for m := range extraMembers { + if removedMembers[m] != nil { + events = append(events, *removedMembers[m]) + } } + sort.Slice(events, func(i, j int) bool { + return events[i].ClockValue < events[j].ClockValue + }) + return events } diff --git a/protocol/v1/membership_update_message_test.go b/protocol/v1/membership_update_message_test.go index f30f3ee09..a4173e84e 100644 --- a/protocol/v1/membership_update_message_test.go +++ b/protocol/v1/membership_update_message_test.go @@ -371,7 +371,45 @@ func TestMembershipUpdateEventEqual(t *testing.T) { require.False(t, u1.Equal(u2)) } -func TestAbridgedEvents(t *testing.T) { +func TestAbridgedEventsNameChanged(t *testing.T) { + var clock uint64 = 0 + creator, err := crypto.GenerateKey() + require.NoError(t, err) + creatorID := publicKeyToString(&creator.PublicKey) + + g, err := NewGroupWithCreator("name-0", "#fa6565", clock, creator) + require.NoError(t, err) + clock++ + + // Full events is only a single one + require.Len(t, g.Events(), 1) + // same as abridged + require.Len(t, g.AbridgedEvents(), 1) + + // We change name of the chat + nameChangedEvent1 := NewNameChangedEvent("name-1", clock) + nameChangedEvent1.From = creatorID + nameChangedEvent1.ChatID = g.chatID + err = g.ProcessEvent(nameChangedEvent1) + require.NoError(t, err) + clock++ + + // We change name of the chat again + nameChangedEvent2 := NewNameChangedEvent("name-2", clock) + nameChangedEvent2.From = creatorID + nameChangedEvent2.ChatID = g.chatID + err = g.ProcessEvent(nameChangedEvent2) + require.NoError(t, err) + clock++ + + // Full events is 3 events + require.Len(t, g.Events(), 3) + // While abridged should exclude the first name-1 event + require.Len(t, g.AbridgedEvents(), 2) + require.Equal(t, g.AbridgedEvents()[1].Name, "name-2") +} + +func TestAbridgedEventsMembers(t *testing.T) { var clock uint64 = 0 creator, err := crypto.GenerateKey() require.NoError(t, err) @@ -400,141 +438,111 @@ func TestAbridgedEvents(t *testing.T) { // Full events is only a single one require.Len(t, g.Events(), 1) // same as abridged - require.Len(t, g.AbridgedEvents(&creator.PublicKey), 1) + require.Len(t, g.AbridgedEvents(), 1) - // We change name of the chat - nameChangedEvent1 := NewNameChangedEvent("name-1", clock) - nameChangedEvent1.From = creatorID - nameChangedEvent1.ChatID = g.chatID - err = g.ProcessEvent(nameChangedEvent1) + // Add three new members + event := NewMembersAddedEvent([]string{member1ID, member2ID, member3ID}, clock) + event.From = creatorID + event.ChatID = g.chatID + err = g.ProcessEvent(event) require.NoError(t, err) clock++ - // We change name of the chat again - nameChangedEvent2 := NewNameChangedEvent("name-2", clock) - nameChangedEvent2.From = creatorID - nameChangedEvent2.ChatID = g.chatID - err = g.ProcessEvent(nameChangedEvent2) + require.Len(t, g.Events(), 2) + // All the events are relevant here, so it should be the same + require.Len(t, g.AbridgedEvents(), 2) + + // We remove one of the users + event = NewMemberRemovedEvent(member3ID, clock) + event.From = creatorID + event.ChatID = g.chatID + err = g.ProcessEvent(event) require.NoError(t, err) clock++ - // Full events is 3 events require.Len(t, g.Events(), 3) - // While abridged should exclude the first name-1 event - require.Len(t, g.AbridgedEvents(&creator.PublicKey), 2) - require.Equal(t, g.AbridgedEvents(&creator.PublicKey)[1].Name, "name-2") + // All the events are relevant here, so it should be the same + require.Len(t, g.AbridgedEvents(), 3) // Add a new member - newMemberEvent1 := NewMembersAddedEvent([]string{member1ID}, clock) - newMemberEvent1.From = creatorID - newMemberEvent1.ChatID = g.chatID - err = g.ProcessEvent(newMemberEvent1) + event = NewMembersAddedEvent([]string{member4ID}, clock) + event.From = creatorID + event.ChatID = g.chatID + err = g.ProcessEvent(event) require.NoError(t, err) clock++ - // Full events is 4 events require.Len(t, g.Events(), 4) - // While abridged, given we are the creator, we only take 2 events and ignore - // the member created event - require.Len(t, g.AbridgedEvents(&creator.PublicKey), 2) - require.Equal(t, g.AbridgedEvents(&creator.PublicKey)[1].Name, "name-2") + // All the events are relevant here, so it should be the same + require.Len(t, g.AbridgedEvents(), 4) - // While abridged, given we are the new member, we take 3 events - // that are relevant to us - require.Len(t, g.AbridgedEvents(&member1.PublicKey), 3) - require.Equal(t, g.AbridgedEvents(&member1.PublicKey)[1].Name, "name-2") - require.Equal(t, g.AbridgedEvents(&member1.PublicKey)[2].Members, []string{member1ID}) - - // We join the chat - joinedEvent1 := NewMemberJoinedEvent(clock) - joinedEvent1.From = member1ID - joinedEvent1.ChatID = g.chatID - err = g.ProcessEvent(joinedEvent1) + // We remove the member just added + event = NewMemberRemovedEvent(member4ID, clock) + event.From = creatorID + event.ChatID = g.chatID + err = g.ProcessEvent(event) require.NoError(t, err) clock++ - // Full events is 5 events require.Len(t, g.Events(), 5) - // While abridged, given we are the creator, we only take 2 events and ignore - // the member created event - require.Len(t, g.AbridgedEvents(&creator.PublicKey), 2) - require.Equal(t, g.AbridgedEvents(&creator.PublicKey)[1].Name, "name-2") + // The previous two events, should be removed, because they have no impact + // on the chat history + abridgedEvents := g.AbridgedEvents() + require.Len(t, abridgedEvents, 3) - // While abridged, given we are the new member, we take 4 events - // that are relevant to us - require.Len(t, g.AbridgedEvents(&member1.PublicKey), 4) - - // Next is the tricky case, a user that has been invited by someone - // made an admin. We need to follow the history of admins so - // that whoever receives the message can see that Creator-> Invited A -> Made A admin -> A Invited B - - // Creator makes member1 Admin - addedAdminEvent1 := NewAdminsAddedEvent([]string{member1ID}, clock) - addedAdminEvent1.From = creatorID - addedAdminEvent1.ChatID = g.chatID - err = g.ProcessEvent(addedAdminEvent1) - require.NoError(t, err) - clock++ - - // member1 adds member2 - newMemberEvent2 := NewMembersAddedEvent([]string{member2ID}, clock) - newMemberEvent2.From = member1ID - newMemberEvent2.ChatID = g.chatID - err = g.ProcessEvent(newMemberEvent2) - require.NoError(t, err) - clock++ - - // member1 makes member2 admin - addedAdminEvent2 := NewAdminsAddedEvent([]string{member2ID}, clock) - addedAdminEvent2.From = member1ID - addedAdminEvent2.ChatID = g.chatID - err = g.ProcessEvent(addedAdminEvent2) - require.NoError(t, err) - clock++ - - // member2 adds member3 - newMemberEvent3 := NewMembersAddedEvent([]string{member3ID}, clock) - newMemberEvent3.From = member2ID - newMemberEvent3.ChatID = g.chatID - err = g.ProcessEvent(newMemberEvent3) - require.NoError(t, err) - clock++ - - // member1 makes member3 admin - addedAdminEvent3 := NewAdminsAddedEvent([]string{member3ID}, clock) - addedAdminEvent3.From = member1ID - addedAdminEvent3.ChatID = g.chatID - err = g.ProcessEvent(addedAdminEvent3) - require.NoError(t, err) - clock++ - - // member3 adds member4 - newMemberEvent4 := NewMembersAddedEvent([]string{member4ID}, clock) - newMemberEvent4.From = member3ID - newMemberEvent4.ChatID = g.chatID - err = g.ProcessEvent(newMemberEvent4) - require.NoError(t, err) - - // Now we check that the history has been correctly followed - // Full events is 4 events - require.Len(t, g.Events(), 11) - // While abridged, given we are the creator, we only take 2 events and ignore - // the member created event - require.Len(t, g.AbridgedEvents(&creator.PublicKey), 2) - require.Equal(t, g.AbridgedEvents(&creator.PublicKey)[1].Name, "name-2") - - // While abridged, given we are the new member, we take 3 events - // that are relevant to us - require.Len(t, g.AbridgedEvents(&member4.PublicKey), 9) - - // We build a group from the abridged events - - group, err := NewGroupWithEvents(g.chatID, g.AbridgedEvents(&member4.PublicKey)) - require.NoError(t, err) - - // Make sure the chatID, name is the same - require.Equal(t, g.name, group.name) - require.Equal(t, g.chatID, group.chatID) - // Make sure that user 4 is a member - require.True(t, group.IsMember(member4ID)) + require.Equal(t, uint64(0), abridgedEvents[0].ClockValue) + require.Equal(t, uint64(1), abridgedEvents[1].ClockValue) + require.Equal(t, uint64(2), abridgedEvents[2].ClockValue) +} + +func TestAbridgedEventsAdmins(t *testing.T) { + var clock uint64 = 0 + creator, err := crypto.GenerateKey() + require.NoError(t, err) + creatorID := publicKeyToString(&creator.PublicKey) + + member1, err := crypto.GenerateKey() + require.NoError(t, err) + member1ID := publicKeyToString(&member1.PublicKey) + + member2, err := crypto.GenerateKey() + require.NoError(t, err) + member2ID := publicKeyToString(&member2.PublicKey) + + member3, err := crypto.GenerateKey() + require.NoError(t, err) + member3ID := publicKeyToString(&member3.PublicKey) + + g, err := NewGroupWithCreator("name-0", "#fa6565", clock, creator) + require.NoError(t, err) + clock++ + + // Full events is only a single one + require.Len(t, g.Events(), 1) + // same as abridged + require.Len(t, g.AbridgedEvents(), 1) + + // Add three new members + event := NewMembersAddedEvent([]string{member1ID, member2ID, member3ID}, clock) + event.From = creatorID + event.ChatID = g.chatID + err = g.ProcessEvent(event) + require.NoError(t, err) + clock++ + + require.Len(t, g.Events(), 2) + // All the events are relevant here, so it should be the same + require.Len(t, g.AbridgedEvents(), 2) + + // Make two of them admins + event = NewAdminsAddedEvent([]string{member1ID, member2ID}, clock) + event.From = creatorID + event.ChatID = g.chatID + err = g.ProcessEvent(event) + require.NoError(t, err) + clock++ + + require.Len(t, g.Events(), 3) + // All the events are relevant here, so it should be the same + require.Len(t, g.AbridgedEvents(), 3) }