Old contact requests result in empty chat messages

[0x-r4bbit]

Okay so here's a funny scenario:

1. I have two accounts A and B each logged-in to their own desktop instances
2. B sends A three contact requests, A does only respond to the latest one
3. After the request was accepted, the chat of A<->B shows 2 empty chat messages from B

This is most likely because a contact request comes as a message signal and is probably interpreted as a chat message as well.
Then, desktop probably doesn't render render the requests textmessage. I assume it doesn't do that because the contenttype of the message isn't "text" but "unknown".

This needs to be checked, I'm just thinking out loud here.

Either way, I think we should not render previous contact request messages as chat message in the first place.
Here's a screenshot from A's perspective after accepting the request:

[0x-r4bbit]

Another thing I've noticed, those old contact requests (even the one that was accepted, is still rendered in the activity center and can still be interacted with.

For example, I can click "accept" on an old request. It will result in the following logs:

DBG 2023-02-03 12:59:10.218+01:00 NewBE_callPrivateRPC                       topics="rpc" tid=58625 file=core.nim:27 rpc_method=wakuext_acceptLatestContactRequestForContact
DBG 2023-02-03 12:59:10.251+01:00 NewBE_callPrivateRPC                       topics="rpc" tid=58625 file=core.nim:27 rpc_method=wakuext_firstUnseenMessageID
ERR 2023-02-03 12:59:10.254+01:00 error: requested community doesn't exists  topics="community-service" tid=58625 file=service.nim:559 communityId=
DBG 2023-02-03 12:59:10.257+01:00 NewBE_callPrivateRPC                       topics="rpc" tid=58625 file=core.nim:27 rpc_method=wakuext_unreadActivityCenterNotificationsCount

I do not know how communities are related to that at all.
We should probably make old requests "inactive" or something so they can't be interacted with anymore

[caybro]

We are missing the correct messageType mapping:

--- ui/imports/utils/Constants.qml
+++ ui/imports/utils/Constants.qml
@@ -398,6 +398,7 @@ QtObject {
         readonly property int gapType: 10
         readonly property int editType: 11
         readonly property int discordMessageType: 12
+        readonly property int contactIdentityVerification: 13
     }

and function convertContentType(value) in other places but you're right, this should most likely be not shown at all in the chat

[0x-r4bbit]

Thanks for confirming my hypothesis!

[caybro]

@MishkaRogachev any idea? :)

[MishkaRogachev]

@MishkaRogachev any idea? :)

Unfortunately, i did not touch contact requests for a long time, but it can be tails of the current CR refactoring from mobile team. Ill take a closer look a bit later

[qoqobolo]

Hi there! Can I ask you guys something here?
This step caught my eye:

2. B sends A three contact requests

Q: Are there any plans to change this behavior by limiting the possible number of sent requests to one?
AFAIK, by design, if you send a request once to a chat key, it should get the Pending status for the sender, which prevents them from sending more requests to the same chat key.

TL;DR

Why this worries me:

1. It looks like a possible spam vector (for desktop clients mostly).
   On mobile we have this issue User B is able to spam User A with contact requests In case if User A declines CR of User B status-mobile#14947
   And here is the behavior John suggests to prevent possible spam in the form of annoying repetitive requests if they are canceled by the recipient: The contact request is not being resent if previous was declined status-mobile#14798 (comment)
   One more scenario: if the request is canceled by the sender. In this case, it's also canceled on the receiver's side. You can send another one, but the receiver will also have only one request (-> the user is protected from spammy repeated requests in the Activity center).

2. There is a mismatch between mobile and desktop behavior in this case.
   If you send multiple contact requests from desktop to mobile, only the first request (and the first message) will be displayed on mobile in the Activity center. After accepting the request, all requests will be displayed in the chat history and will remain in Pending status forever (except for the first one). This inconsistency could also be fixed by adding the Pending status when the request is sent for the first time.

These are not urgent questions. It's just that we are close to the first release cut, so we are already thinking about such things :)
And please correct me if I misunderstand the scenario itself or if this is already covered somewhere.

[0x-r4bbit]

Q: Are there any plans to change this behavior by limiting the possible number of sent requests to one?
AFAIK, by design, if you send a request once to a chat key, it should get the Pending status for the sender, which prevents them from sending more requests to the same chat key.

@qoqobolo so I think the way it is done by Desktop atm, is that, when you have a pending request, you can cancel it and send a new one. The receiver will then get multiple requests. Senders will also not get notified when a request was rejected AFAIK.

I'm aware that this is a potential spam vector, but I think we went for it either way.

One more scenario: if the request is canceled by the sender. In this case, it's also canceled on the receiver's side. You can send another one, but the receiver will also have only one request (-> the user is protected from spammy repeated requests in the Activity center).

I'm not sure if this is actually the case right now. Whether or not a sender cancels a request is not something that the receiver will get notified about.

There is a mismatch between mobile and desktop behavior in this case.
If you send multiple contact requests from desktop to mobile, only the first request (and the first message) will be displayed on mobile in the Activity center. After accepting the request, all requests will be displayed in the chat history and will remain in Pending status forever (except for the first one). This inconsistency could also be fixed by adding the Pending status when the request is sent for the first time.

I think this is an important point.

@MishkaRogachev do you know from the top of your head what's the implementation parity between mobile and desktop for these cases?

[qoqobolo]

@0x-r4bbit hmm wait, in what way exactly did you send those three requests on your screenshot? :)

when you have a pending request, you can cancel it and send a new one.

I mean yes, you can cancel it in settings, but there is another more fast way to send a new one: you just remove the chat key and paste it again without canceling. This doesn't look right to me, because this is what makes it so easy to quickly send a large number of requests.

Screen.Recording.2023-02-06.at.16.02.38.mov

So in terms of spam, I just wanted to bring your attention and make sure you're aware of this way to send multiple contact requests. Let me know if it's better to forward this to the QA team for further discussion on whether this is correct, logging an issue, etc, I don't mean to shower you with these questions if you are not working specifically on this functionality :)

In terms of parity between mobile and desktop, I will also ask Andrea what is the desirable behavior from our side in this case.

[0x-r4bbit]

but there is another more fast way to send a new one: you just remove the chat key and paste it again without canceling. This doesn't look right to me, because this is what makes it so easy to quickly send a large number of requests.

That's exactly how I've done it.
We could forbid that though.

[qoqobolo]

We could forbid that though.

Yes, Pedro confirmed that we should do this

I'll create an issue.
Thanks for discussing it with me!

[jrainville]

Closing this as we can no longer send multiple contact requests, so the original issue can't be reproduced. Also, there have a been a lot of fixes to the displaying of the contact request messages in chat