search

status-im / specs

Specifications for Status clients.
https://specs.status.im/
MIT License
14 stars 14 forks source link

Add spec for the contact request refactor #154

Open jrainville opened 3 years ago

jrainville commented 3 years ago

Specs Contacts, the contact requests and the way to persist them on the network.

jakubgs commented 2 years ago

Is this PR dead?

jrainville commented 2 years ago

@cammellos sorry for the late reply. I was on vacation when you did the changes so I missed it and before that, this feature was not a priority so I had pushed it back.

Anyway, I rebased the PR, fixed a typo and also reviewed your changes. It looks good, my only question would be what happens with the "contact persistence" part? I saw that you removed it. Is it because it was too implementation heavy or just because it is no longer wanted?

I can understand if it is the latter, since it would have been heavy on the network and also harder to implement. The contact signature is easier and also does solve the 1-1 chat issues we have. However, we will still have the issue of losing all your contacts on account import. Is it fixed using the normal sync instead (between devices)?

cammellos commented 2 years ago

@jrainville thanks for the review,

It looks good, my only question would be what happens with the "contact persistence" part? I saw that you removed it. Is it because it was too implementation heavy or just because it is no longer wanted?

We have now implemented contact backup, it hasn't been specced yet, but it's already in develop (and likely in desktop), and it works slightly differently, so I thought I'd remove it

jrainville commented 2 years ago

Turns out I can't approve my own PR 😅 Anyway, merge whenever you want.