Fix: Don't create extra mutual state messages when accepting the CR again (#4488)

* Fix: don't create extra system messages when accepting the CR again

* chore: add one more test for cr flows repeated

* chore: use deprecation.AddChatsCount in contact requests tests

protocol/messenger_contact_requests_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/suite"
 	"go.uber.org/zap"
 
+	"github.com/status-im/status-go/deprecation"
 	"github.com/status-im/status-go/eth-node/crypto"
 	"github.com/status-im/status-go/eth-node/types"
 	"github.com/status-im/status-go/multiaccounts/settings"
@@ -486,6 +487,32 @@ func (s *MessengerContactRequestSuite) TestReceiveAcceptAndRetractContactRequest
 	s.retractContactRequest(contactID, theirMessenger)
 }
 
+// The scenario tested is as follow:
+//  1. Repeat 5 times:
+//     2.1) Alice sends a contact request to Bob
+//     2.2) Bob accepts the contact request
+//     2.3) Alice removes bob from contacts
+func (s *MessengerContactRequestSuite) TestAcceptCRRemoveAndRepeat() {
+	theirMessenger := s.newMessenger()
+	_, err := theirMessenger.Start()
+	s.Require().NoError(err)
+	defer TearDownMessenger(&s.Suite, theirMessenger)
+
+	contactID := types.EncodeHex(crypto.FromECDSAPub(&theirMessenger.identity.PublicKey))
+
+	for i := 0; i < 5; i++ {
+		messageText := fmt.Sprintf("hello %d", i)
+		request := &requests.SendContactRequest{
+			ID:      contactID,
+			Message: messageText,
+		}
+		s.sendContactRequest(request, s.m)
+		contactRequest := s.receiveContactRequest(messageText, theirMessenger)
+		s.acceptContactRequest(contactRequest, s.m, theirMessenger)
+		s.retractContactRequest(contactID, theirMessenger)
+	}
+}
+
 // The scenario tested is as follow:
 // 1) Alice sends a contact request to Bob
 // 2) Bob declines the contact request
@@ -539,7 +566,68 @@ func (s *MessengerContactRequestSuite) TestAliceTriesToSpamBobWithContactRequest
 	)
 	s.Require().Error(err)
 	s.Require().ErrorContains(err, "no messages")
+}
 
+// The scenario tested is as follow:
+// 1) Alice sends a contact request to Bob
+// 2) Bob accepts the contact
+// 3) Bob accepts the contact request (again!)
+// 4) No extra mesages on Alice's side
+func (s *MessengerContactRequestSuite) TestAliceSeesOnlyOneAcceptFromBob() {
+	messageTextAlice := "You wanna play with fire, Bobby?!"
+	alice := s.m
+
+	bob := s.newMessenger()
+	_, err := bob.Start()
+	s.Require().NoError(err)
+	defer TearDownMessenger(&s.Suite, bob)
+
+	bobID := types.EncodeHex(crypto.FromECDSAPub(&bob.identity.PublicKey))
+
+	// Alice sends a contact request to Bob
+	request := &requests.SendContactRequest{
+		ID:      bobID,
+		Message: messageTextAlice,
+	}
+	s.sendContactRequest(request, alice)
+
+	contactRequest := s.receiveContactRequest(messageTextAlice, bob)
+	s.Require().NotNil(contactRequest)
+
+	// Bob accepts the contact request
+	s.acceptContactRequest(contactRequest, alice, bob)
+
+	// Accept contact request again
+	_, err = bob.AcceptContactRequest(context.Background(), &requests.AcceptContactRequest{ID: types.Hex2Bytes(contactRequest.ID)})
+	s.Require().NoError(err)
+
+	// Check we don't have extra messages on Alice's side
+	resp, err := WaitOnMessengerResponse(alice,
+		func(r *MessengerResponse) bool {
+			return len(r.ActivityCenterNotifications()) == 1 && len(r.Messages()) == 1
+		},
+		"contact request acceptance not received",
+	)
+	s.Require().NoError(err)
+	s.Require().NotNil(resp)
+
+	// Check activity center notification is of the right type
+	s.Require().Len(resp.ActivityCenterNotifications(), 1)
+	s.Require().Equal(ActivityCenterNotificationTypeContactRequest, resp.ActivityCenterNotifications()[0].Type)
+	s.Require().Equal(common.ContactRequestStateAccepted, resp.ActivityCenterNotifications()[0].Message.ContactRequestState)
+	s.Require().Equal(resp.ActivityCenterNotifications()[0].Read, true)
+	s.Require().Equal(resp.ActivityCenterNotifications()[0].Accepted, true)
+	s.Require().Equal(resp.ActivityCenterNotifications()[0].Dismissed, false)
+	s.Require().NotNil(resp.ActivityCenterNotifications()[0].Message)
+
+	// Make sure the message is updated, sender side
+	s.Require().Len(resp.Messages(), 1)
+
+	contactRequest = s.findFirstByContentType(resp.Messages(), protobuf.ChatMessage_CONTACT_REQUEST)
+	s.Require().NotNil(contactRequest)
+
+	s.Require().Equal(common.ContactRequestStateAccepted, contactRequest.ContactRequestState)
+	s.Require().Equal(request.Message, contactRequest.Text)
 }
 
 func (s *MessengerContactRequestSuite) TestReceiveAndAcceptContactRequestTwice() { //nolint: unused
@@ -1472,7 +1560,7 @@ func (s *MessengerContactRequestSuite) blockContactAndSync(alice1 *Messenger, al
 	s.Require().Equal(ContactRequestStateReceived, respContact.ContactRequestRemoteState)
 
 	// Check chats list
-	//s.Require().Len(alice2.Chats(), 1) // FIXME: Uncomment after https://github.com/status-im/status-go/issues/3800
+	s.Require().Len(alice2.Chats(), deprecation.AddChatsCount(2))
 }
 
 func (s *MessengerContactRequestSuite) unblockContactAndSync(alice1 *Messenger, alice2 *Messenger, bob *Messenger) {
@@ -1503,7 +1591,7 @@ func (s *MessengerContactRequestSuite) unblockContactAndSync(alice1 *Messenger,
 	s.Require().Equal(respContact.ContactRequestRemoteState, ContactRequestStateNone)
 
 	// Check chats list
-	//s.Require().Len(alice2.Chats(), 1) // FIXME: Uncomment after https://github.com/status-im/status-go/issues/3800
+	s.Require().Len(alice2.Chats(), deprecation.AddChatsCount(2))
 }
 
 func (s *MessengerContactRequestSuite) TestBlockedContactSyncing() {

protocol/messenger_handler.go

@@ -980,8 +980,12 @@ func (m *Messenger) handleAcceptContactRequestMessage(state *ReceivedMessageStat
 		return err
 	}
 
+	// We still want to handle acceptance of the CR even it was already accepted
+	previouslyAccepted := request != nil && request.ContactRequestState == common.ContactRequestStateAccepted
+
 	contact := state.CurrentMessageState.Contact
 
+	// The request message will be added to the response here
 	processingResponse, err := m.handleAcceptContactRequest(state.Response, contact, request, clock)
 	if err != nil {
 		return err
@@ -1010,25 +1014,28 @@ func (m *Messenger) handleAcceptContactRequestMessage(state *ReceivedMessageStat
 		}
 
 		// Add mutual state update message for incoming contact request
-		clock, timestamp := chat.NextClockAndTimestamp(m.transport)
+		if !previouslyAccepted {
+			clock, timestamp := chat.NextClockAndTimestamp(m.transport)
 
-		updateMessage, err := m.prepareMutualStateUpdateMessage(contact.ID, MutualStateUpdateTypeAdded, clock, timestamp, false)
+			updateMessage, err := m.prepareMutualStateUpdateMessage(contact.ID, MutualStateUpdateTypeAdded, clock, timestamp, false)
-		if err != nil {
+			if err != nil {
-			return err
+				return err
+			}
+
+			m.prepareMessage(updateMessage, m.httpServer)
+			err = m.persistence.SaveMessages([]*common.Message{updateMessage})
+			if err != nil {
+				return err
+			}
+			state.Response.AddMessage(updateMessage)
+
+			err = chat.UpdateFromMessage(updateMessage, m.getTimesource())
+			if err != nil {
+				return err
+			}
+			chat.UnviewedMessagesCount++
 		}
 
-		m.prepareMessage(updateMessage, m.httpServer)
-		err = m.persistence.SaveMessages([]*common.Message{updateMessage})
-		if err != nil {
-			return err
-		}
-		state.Response.AddMessage(updateMessage)
-
-		err = chat.UpdateFromMessage(updateMessage, m.getTimesource())
-		if err != nil {
-			return err
-		}
-		chat.UnviewedMessagesCount++
 		state.Response.AddChat(chat)
 		state.AllChats.Store(chat.ID, chat)
 	}