From dea21f440a9f630c4b8324fcde0b016d6c3fc02b Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Wed, 8 Feb 2023 14:58:10 +0000 Subject: [PATCH] Linting and some fixes on contact requests handling --- VERSION | 2 +- protocol/contact.go | 7 +- protocol/contact_test.go | 4 +- protocol/messenger_activity_center_test.go | 58 ++-------------- protocol/messenger_contact_requests_test.go | 2 +- protocol/messenger_edit_message_test.go | 77 --------------------- protocol/messenger_handler.go | 18 +++-- protocol/messenger_test.go | 2 - wakuv2/waku.go | 6 +- 9 files changed, 32 insertions(+), 144 deletions(-) diff --git a/VERSION b/VERSION index 746da2ab8..156fe0ca7 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.129.3 +0.130.0 diff --git a/protocol/contact.go b/protocol/contact.go index a82cf7ca6..19d13124c 100644 --- a/protocol/contact.go +++ b/protocol/contact.go @@ -431,8 +431,11 @@ func (c *Contact) ContactRequestPropagatedStateReceived(state *protobuf.ContactR response.processed = true c.ContactRequestLocalClock = expectedLocalClock c.ContactRequestLocalState = ContactRequestStateNone - // We set they remote state, as this was an implicit retraction - // potentially + // We set the remote state, as this was an implicit retraction + // potentially, for example this could happen if they + // sent a retraction earier, but we never received it, + // or one of our paired devices has retracted the contact request + // but we never synced with them. c.ContactRequestRemoteState = ContactRequestStateNone } diff --git a/protocol/contact_test.go b/protocol/contact_test.go index e8325468b..aeb80fb07 100644 --- a/protocol/contact_test.go +++ b/protocol/contact_test.go @@ -617,7 +617,9 @@ func TestContactContactRequestPropagatedStateReceivedOutOfDateLocalStateOnTheirS func TestContactContactRequestPropagatedStateReceivedOutOfDateLocalStateOnOurSide(t *testing.T) { // We receive a message with expected contact request state == none - // and clock > our clock. We consider this a retraction, unless we are in the dismissed state, since that should be only changed by a trusted device + // and clock > our clock. We consider this a retraction, unless we are + // in the dismissed state, since that should be only changed by a + // trusted device c := &Contact{} c.ContactRequestLocalState = ContactRequestStateSent diff --git a/protocol/messenger_activity_center_test.go b/protocol/messenger_activity_center_test.go index 248030d08..6fad52b0e 100644 --- a/protocol/messenger_activity_center_test.go +++ b/protocol/messenger_activity_center_test.go @@ -58,50 +58,6 @@ func (s *MessengerActivityCenterMessageSuite) newMessenger() *Messenger { return messenger } -func (s *MessengerActivityCenterMessageSuite) TestDismissOneToOneMessage() { - theirMessenger := s.newMessenger() - _, err := theirMessenger.Start() - s.Require().NoError(err) - - theirChat := CreateOneToOneChat("Their 1TO1", &s.privateKey.PublicKey, s.m.transport) - err = theirMessenger.SaveChat(theirChat) - s.Require().NoError(err) - - inputMessage := buildTestMessage(*theirChat) - sendResponse, err := theirMessenger.SendChatMessage(context.Background(), inputMessage) - s.NoError(err) - s.Require().Len(sendResponse.Messages(), 1) - - response, err := WaitOnMessengerResponse( - s.m, - func(r *MessengerResponse) bool { return len(r.messages) > 0 }, - "no messages", - ) - s.Require().NoError(err) - s.Require().Len(response.Chats(), 1) - s.Require().Len(response.Messages(), 1) - s.Require().Len(response.ActivityCenterNotifications(), 1) - - // Dismiss all - s.Require().NoError(s.m.DismissAllActivityCenterNotifications(context.Background())) - - // Send another message - inputMessage = buildTestMessage(*theirChat) - sendResponse, err = theirMessenger.SendChatMessage(context.Background(), inputMessage) - s.NoError(err) - s.Require().Len(sendResponse.Messages(), 1) - - response, err = WaitOnMessengerResponse( - s.m, - func(r *MessengerResponse) bool { return len(r.messages) > 0 }, - "no messages", - ) - s.Require().NoError(err) - s.Require().Len(response.Chats(), 1) - s.Require().Len(response.Messages(), 1) - s.Require().Len(response.ActivityCenterNotifications(), 1) -} - func (s *MessengerActivityCenterMessageSuite) TestDeleteOneToOneChat() { theirMessenger := s.newMessenger() _, err := theirMessenger.Start() @@ -111,8 +67,11 @@ func (s *MessengerActivityCenterMessageSuite) TestDeleteOneToOneChat() { err = theirMessenger.SaveChat(theirChat) s.Require().NoError(err) - inputMessage := buildTestMessage(*theirChat) - sendResponse, err := theirMessenger.SendChatMessage(context.Background(), inputMessage) + r := &requests.SendContactRequest{ + ID: types.Hex2Bytes(s.m.myHexIdentity()), + Message: "hello", + } + sendResponse, err := theirMessenger.SendContactRequest(context.Background(), r) s.NoError(err) s.Require().Len(sendResponse.Messages(), 1) @@ -143,19 +102,16 @@ func (s *MessengerActivityCenterMessageSuite) TestDeleteOneToOneChat() { s.Require().NoError(err) // Send another message - inputMessage = buildTestMessage(*theirChat) + inputMessage := buildTestMessage(*theirChat) sendResponse, err = theirMessenger.SendChatMessage(context.Background(), inputMessage) s.NoError(err) s.Require().Len(sendResponse.Messages(), 1) response, err = WaitOnMessengerResponse( s.m, - func(r *MessengerResponse) bool { return len(r.ActivityCenterNotifications()) > 0 }, + func(r *MessengerResponse) bool { return len(r.Chats()) > 0 }, "no messages", ) s.Require().NoError(err) s.Require().Len(response.Chats(), 1) - s.Require().Len(response.Messages(), 1) - s.Require().Len(response.ActivityCenterNotifications(), 1) - } diff --git a/protocol/messenger_contact_requests_test.go b/protocol/messenger_contact_requests_test.go index 6eb837c66..f15dd8db8 100644 --- a/protocol/messenger_contact_requests_test.go +++ b/protocol/messenger_contact_requests_test.go @@ -1359,7 +1359,7 @@ func (s *MessengerContactRequestSuite) TestAliceRecoverStateSendContactRequest() _, err = WaitOnMessengerResponse( bob, func(r *MessengerResponse) bool { - return len(r.Contacts) > 0 && len(r.Messages()) > 0 && len(r.ActivityCenterNotifications()) > 0 + return len(r.Contacts) > 0 }, "no messages", ) diff --git a/protocol/messenger_edit_message_test.go b/protocol/messenger_edit_message_test.go index 3be8b3b01..639050554 100644 --- a/protocol/messenger_edit_message_test.go +++ b/protocol/messenger_edit_message_test.go @@ -130,83 +130,6 @@ func (s *MessengerEditMessageSuite) TestEditMessage() { s.Require().Equal(ErrInvalidEditOrDeleteAuthor, err) } -func (s *MessengerEditMessageSuite) TestEditMessageActivityCenter() { - theirMessenger := s.newMessenger() - _, err := theirMessenger.Start() - s.Require().NoError(err) - - theirChat := CreateOneToOneChat("Their 1TO1", &s.privateKey.PublicKey, s.m.transport) - err = theirMessenger.SaveChat(theirChat) - s.Require().NoError(err) - - inputMessage := buildTestMessage(*theirChat) - sendResponse, err := theirMessenger.SendChatMessage(context.Background(), inputMessage) - s.NoError(err) - s.Require().Len(sendResponse.Messages(), 1) - - response, err := WaitOnMessengerResponse( - s.m, - func(r *MessengerResponse) bool { return len(r.messages) > 0 }, - "no messages", - ) - s.Require().NoError(err) - s.Require().Len(response.Chats(), 1) - s.Require().Len(response.Messages(), 1) - s.Require().Len(response.ActivityCenterNotifications(), 1) - - // Check we can fetch the notification - paginationResponse, err := s.m.ActivityCenterNotifications("", 10) - s.Require().NoError(err) - s.Require().Len(paginationResponse.Notifications, 1) - - ogMessage := sendResponse.Messages()[0] - - messageID, err := types.DecodeHex(ogMessage.ID) - s.Require().NoError(err) - - editedText := "edited text" - editedMessage := &requests.EditMessage{ - ID: messageID, - Text: editedText, - } - - sendResponse, err = theirMessenger.EditMessage(context.Background(), editedMessage) - - s.Require().NoError(err) - s.Require().Len(sendResponse.Messages(), 1) - s.Require().NotEmpty(sendResponse.Messages()[0].EditedAt) - s.Require().Equal(sendResponse.Messages()[0].Text, editedText) - s.Require().Len(sendResponse.Chats(), 1) - s.Require().NotNil(sendResponse.Chats()[0].LastMessage) - s.Require().NotEmpty(sendResponse.Chats()[0].LastMessage.EditedAt) - - response, err = WaitOnMessengerResponse( - s.m, - func(r *MessengerResponse) bool { return len(r.messages) > 0 }, - "no messages", - ) - s.Require().NoError(err) - - s.Require().Len(response.Chats(), 1) - s.Require().Len(response.Messages(), 1) - s.Require().NotEmpty(response.Messages()[0].EditedAt) - s.Require().False(response.Messages()[0].New) - - // Check we can fetch the notification - paginationResponse, err = s.m.ActivityCenterNotifications("", 10) - s.Require().NoError(err) - s.Require().Len(paginationResponse.Notifications, 1) - - // Main instance user attempts to edit the message it received from theirMessenger - editedMessage = &requests.EditMessage{ - ID: messageID, - Text: "edited-again text", - } - _, err = s.m.EditMessage(context.Background(), editedMessage) - - s.Require().Equal(ErrInvalidEditOrDeleteAuthor, err) -} - func (s *MessengerEditMessageSuite) TestEditMessageEdgeCases() { theirMessenger := s.newMessenger() _, err := theirMessenger.Start() diff --git a/protocol/messenger_handler.go b/protocol/messenger_handler.go index d32a49672..b9bb1cf0a 100644 --- a/protocol/messenger_handler.go +++ b/protocol/messenger_handler.go @@ -732,9 +732,9 @@ func (m *Messenger) handleAcceptContactRequest( originalRequest *common.Message, message protobuf.AcceptContactRequest) (ContactRequestProcessingResponse, error) { - m.logger.Info("received contact request", zap.Uint64("clock-sent", message.Clock), zap.Uint64("current-clock", contact.ContactRequestRemoteClock), zap.Uint64("current-state", uint64(contact.ContactRequestRemoteState))) + m.logger.Debug("received contact request", zap.Uint64("clock-sent", message.Clock), zap.Uint64("current-clock", contact.ContactRequestRemoteClock), zap.Uint64("current-state", uint64(contact.ContactRequestRemoteState))) if contact.ContactRequestRemoteClock > message.Clock { - m.logger.Info("not handling accept since clock lower") + m.logger.Debug("not handling accept since clock lower") return ContactRequestProcessingResponse{}, nil } @@ -825,7 +825,7 @@ func (m *Messenger) handleRetractContactRequest(contact *Contact, message protob m.logger.Debug("handling retracted contact request", zap.Uint64("clock", message.Clock)) r := contact.ContactRequestRetracted(message.Clock) if !r.processed { - m.logger.Info("not handling retract since clock lower") + m.logger.Debug("not handling retract since clock lower") return nil } @@ -846,7 +846,9 @@ func (m *Messenger) HandleRetractContactRequest(state *ReceivedMessageState, mes if err != nil { return err } - state.ModifiedContacts.Store(contact.ID, true) + if contact.ID != m.myHexIdentity() { + state.ModifiedContacts.Store(contact.ID, true) + } return nil } @@ -874,11 +876,13 @@ func (m *Messenger) HandleContactUpdate(state *ReceivedMessageState, message pro chat.Active = false } - logger.Info("Handling contact update") + logger.Debug("Handling contact update") if message.ContactRequestPropagatedState != nil { + logger.Debug("handling contact request propagated state", zap.Any("state before update", contact.ContactRequestPropagatedState())) result := contact.ContactRequestPropagatedStateReceived(message.ContactRequestPropagatedState) if result.sendBackState { + logger.Debug("sending back state") // This is a bit dangerous, since it might trigger a ping-pong of contact updates // also it should backoff/debounce _, err = m.sendContactUpdate(context.Background(), contact.ID, "", "", "", m.dispatchMessage) @@ -888,19 +892,21 @@ func (m *Messenger) HandleContactUpdate(state *ReceivedMessageState, message pro } if result.newContactRequestReceived { + logger.Debug("creating contact request notification") err = m.createContactRequestNotification(contact, state, nil) if err != nil { return err } } + logger.Debug("handled propagated state", zap.Any("state after update", contact.ContactRequestPropagatedState())) state.ModifiedContacts.Store(contact.ID, true) state.AllContacts.Store(contact.ID, contact) } if contact.LastUpdated < message.Clock { - logger.Info("Updating contact") + logger.Debug("Updating contact") if contact.EnsName != message.EnsName { contact.EnsName = message.EnsName contact.ENSVerified = false diff --git a/protocol/messenger_test.go b/protocol/messenger_test.go index 5fd913c4b..9932bb360 100644 --- a/protocol/messenger_test.go +++ b/protocol/messenger_test.go @@ -852,7 +852,6 @@ func (s *MessengerSuite) TestRetrieveTheirPrivateChatExisting() { s.Require().NoError(err) s.Require().Len(response.Chats(), 1) - s.Require().Len(response.ActivityCenterNotifications(), 1) actualChat := response.Chats()[0] // It updates the unviewed messages count s.Require().Equal(uint(2), actualChat.UnviewedMessagesCount) @@ -891,7 +890,6 @@ func (s *MessengerSuite) TestRetrieveTheirPrivateChatNonExisting() { s.Require().NoError(err) s.Require().Len(response.Chats(), 1) - s.Require().Len(response.ActivityCenterNotifications(), 1) actualChat := response.Chats()[0] // It updates the unviewed messages count s.Require().Equal(uint(1), actualChat.UnviewedMessagesCount) diff --git a/wakuv2/waku.go b/wakuv2/waku.go index f11953828..59a7a1d66 100644 --- a/wakuv2/waku.go +++ b/wakuv2/waku.go @@ -1221,9 +1221,9 @@ func (w *Waku) Stop() error { } func (w *Waku) OnNewEnvelopes(envelope *protocol.Envelope, msgType common.MessageType) ([]common.EnvelopeError, error) { - if envelope == nil { - return nil, errors.New("nil envelope error") - } + if envelope == nil { + return nil, errors.New("nil envelope error") + } recvMessage := common.NewReceivedMessage(envelope, msgType) envelopeErrors := make([]common.EnvelopeError, 0)