Waku backup - deleted contact is restored when recovering an account from recovery phrase

[yevh-berdnyk]

Bug Report

Problem

Now I can reproduce this issue with only one account which is added as a contact. Probably that's because that account was created on the old version of Status desktop. However this issue is reproducible in each build

Expected behavior

Deleted contact is not restored

Actual behavior

Deleted contact is restored - userDesktop is listed in contacts tab

Reproduction

1. On the first device restore this account: process fence december tourist trim follow pepper level useless oblige tank defy, let's call them userDesktop
2. On the second device create a new user, userA
3. UserA adds userDesktop as a contact and userDesktop accepts this contact request
4. UserA performs backup (Profile -> Sync Settings -> Backup Settings -> Perform Backup)
5. UserA removes userDesktop from contacts
6. UserA performs backup once again
7. Remove all the data from the second device and restore userA's account
8. Check contacts list

Additional Information

• Status version: nightly
• Operating System: Android, iOS
• The user who should be added as a contact was created on this desktop version:

Logs

mobile_user_geth.log.zip
mobile_user_logcat.log.zip

[ilmotta]

@jrainville, @osmaczko do you think this bug could be related to timing due to the asynchronous nature of backups? Do you have any context about how backups are scheduled and propagated? It seems from the reproduction steps that the order of backups is not respected (sometimes at least).

[osmaczko]

@jrainville, @osmaczko do you think this bug could be related to timing due to the asynchronous nature of backups? Do you have any context about how backups are scheduled and propagated? It seems from the reproduction steps that the order of backups is not respected (sometimes at least).

My guess is that there's either a missing clock check or the newer message hasn't been processed. I'm not super familiar with how backups are implemented, so I'll defer to @saledjenic, who worked on that some time ago.

[jrainville]

@jrainville, @osmaczko do you think this bug could be related to timing due to the asynchronous nature of backups? Do you have any context about how backups are scheduled and propagated? It seems from the reproduction steps that the order of backups is not respected (sometimes at least).

My guess is that there's either a missing clock check or the newer message hasn't been processed. I'm not super familiar with how backups are implemented, so I'll defer to @saledjenic, who worked on that some time ago.

Indeed, it's either a missing clock, or the second backup message doesn't get fetched for some reason, but it sounds like the is reproducible a lot, so it's probably more a clock issue.

The other possibility is if we never implemented the contact removal flow for backups? I don't know by heart how contact backups work, but if when we get the backup of contacts, we don't actually reset the table, then maybe we just don't look for removals. We check for new contacts and add them, but we'd forget to check if there are removed ones.

[saledjenic]

@ilmotta @osmaczko @jrainville could you check if this PR fixes the issue?
status-im/status-go#5925

[ilmotta]

@ilmotta @osmaczko @jrainville could you check if this PR fixes the issue? status-im/status-go#5925

Thank you @saledjenic. We will try to get the status-go PR tested this week 👍🏼

[ilmotta]

@ilmotta @osmaczko @jrainville could you check if this PR fixes the issue? status-im/status-go#5925

Thank you @saledjenic. We will try to get the status-go PR tested this week 👍🏼

@saledjenic, sorry for the late notice, we will come back to this issue and test the status-go PR after the release 2.31 is done. As suggested here status-im/status-go#5925 (review) it would be valuable if the status-go PR had tests to demonstrate the fix works and to protect the code from future regressions. Thank you for working on this fix.

[churik]

I don't think that we have a capacity for it in 2.32, given the desired release cut date.
Deferring the issue, please be free to move it bak if you think there is a room for it @ilmotta

[ilmotta]

I don't think that we have a capacity for it in 2.32, given the desired release cut date. Deferring the issue, please be free to move it bak if you think there is a room for it @ilmotta

Agreed @churik, let's make this one happen in 2.33.