chore: code review comments applied

- sync clock set for all settings prop participating in syncing
- if other sql error happens we return immediately

VERSION

@@ -1 +1 @@
-0.117.1
+0.117.2

multiaccounts/settings/database.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"sync"
+	"time"
 
 	"github.com/status-im/status-go/appdatabase"
 	"github.com/status-im/status-go/eth-node/types"
@@ -144,6 +145,14 @@ INSERT INTO settings (
 		return err
 	}
 
+	if s.DisplayName != "" {
+		now := time.Now().Unix()
+		err = db.SetSettingLastSynced(DisplayName, uint64(now))
+		if err != nil {
+			return err
+		}
+	}
+
 	return nodecfg.SaveConfigWithTx(tx, &n)
 }
 

protocol/messenger.go

@@ -151,7 +151,6 @@ type Messenger struct {
 	downloadHistoryArchiveTasksWaitGroup sync.WaitGroup
 	verificationDatabase                 *verification.Persistence
 	savedAddressesManager                *wallet.SavedAddressesManager
-	backedupMessagesHandler              backupHandler
 
 	// TODO(samyoul) Determine if/how the remaining usage of this mutex can be removed
 	mutex                     sync.Mutex
@@ -480,9 +479,6 @@ func NewMessenger(
 		},
 		logger:                logger,
 		savedAddressesManager: savedAddressesManager,
-		backedupMessagesHandler: backupHandler{
-			postponeHandling: true,
-		},
 	}
 
 	if c.outputMessagesCSV {
@@ -688,7 +684,6 @@ func (m *Messenger) Start() (*MessengerResponse, error) {
 	m.broadcastLatestUserStatus()
 	m.timeoutAutomaticStatusUpdates()
 	m.startBackupLoop()
-	m.startWaitingForTheLatestBackedupMessageLoop()
 	err = m.startAutoMessageLoop()
 	if err != nil {
 		return nil, err

protocol/messenger_backup.go

@@ -310,9 +310,6 @@ func (m *Messenger) backupProfile(ctx context.Context, clock uint64) ([]*protobu
 	if err != nil {
 		return nil, err
 	}
-	if displayNameClock == 0 {
-		displayNameClock = clock
-	}
 
 	keyUID := m.account.KeyUID
 	images, err := m.multiAccounts.GetIdentityImages(keyUID)

protocol/messenger_backup_handler.go

@@ -1,8 +1,7 @@
 package protocol
 
 import (
-	"sync"
+	"database/sql"
-	"time"
 
 	"go.uber.org/zap"
 
@@ -13,73 +12,14 @@ import (
 )
 
 const (
-	// timeToPostponeBackedUpMessagesHandling - the idea is to wait for 30 secs in loading state with the hope that at least
-	// one message from the set of last backed up messages will arrive, that we can know which set of messages to work with
-	timeToPostponeBackedUpMessagesHandling = 30 * time.Second
-
 	SyncWakuSectionKeyProfile     = "profile"
 	SyncWakuSectionKeyContacts    = "contacts"
 	SyncWakuSectionKeyCommunities = "communities"
 	SyncWakuSectionKeySettings    = "settings"
 )
 
-type backupHandler struct {
-	messagesToProceed      []protobuf.Backup
-	lastKnownTime          uint64
-	postponeHandling       bool
-	postponeTasksWaitGroup sync.WaitGroup
-}
-
-func (bh *backupHandler) addMessage(message protobuf.Backup) {
-	if message.Clock < bh.lastKnownTime {
-		return
-	}
-	if message.Clock > bh.lastKnownTime {
-		bh.messagesToProceed = nil
-		bh.lastKnownTime = message.Clock
-	}
-
-	bh.messagesToProceed = append(bh.messagesToProceed, message)
-}
-
-func (m *Messenger) startWaitingForTheLatestBackedupMessageLoop() {
-	ticker := time.NewTicker(timeToPostponeBackedUpMessagesHandling)
-	m.backedupMessagesHandler.postponeTasksWaitGroup.Add(1)
-	go func() {
-		defer m.backedupMessagesHandler.postponeTasksWaitGroup.Done()
-		for {
-			select {
-			case <-ticker.C:
-				if !m.backedupMessagesHandler.postponeHandling {
-					return
-				}
-				m.backedupMessagesHandler.postponeHandling = false
-
-				for _, msg := range m.backedupMessagesHandler.messagesToProceed {
-					messageState := m.buildMessageState()
-					errors := m.HandleBackup(messageState, msg)
-					if len(errors) > 0 {
-						for _, err := range errors {
-							m.logger.Warn("failed to handle postponed Backup messages", zap.Error(err))
-						}
-					}
-				}
-
-				m.backedupMessagesHandler.messagesToProceed = nil
-			case <-m.quit:
-				ticker.Stop()
-				return
-			}
-		}
-	}()
-}
-
 func (m *Messenger) HandleBackup(state *ReceivedMessageState, message protobuf.Backup) []error {
 	var errors []error
-	if m.backedupMessagesHandler.postponeHandling {
-		m.backedupMessagesHandler.addMessage(message)
-		return errors
-	}
 
 	err := m.handleBackedUpProfile(message.Profile, message.Clock)
 	if err != nil {
@@ -128,18 +68,28 @@ func (m *Messenger) handleBackedUpProfile(message *protobuf.BackedUpProfile, bac
 		return err
 	}
 
+	contentSet := false
 	response := wakusync.WakuBackedUpDataResponse{
 		Profile: &wakusync.BackedUpProfile{},
 	}
 
 	if dbDisplayNameClock < message.DisplayNameClock {
-		err := m.SetDisplayName(message.DisplayName, false)
+		err = m.SetDisplayName(message.DisplayName, false)
-		response.AddDisplayName(message.DisplayName, err == nil)
+		if err != nil {
+			return err
+		}
+		contentSet = true
+		response.AddDisplayName(message.DisplayName)
 	}
 
 	syncWithBackedUpImages := false
 	dbImages, err := m.multiAccounts.GetIdentityImages(message.KeyUid)
 	if err != nil {
+		if err != sql.ErrNoRows {
+			return err
+		}
+		// if images are deleted and no images were backed up, then we need to delete them on other devices,
+		// that's why we don't return in case of `sql.ErrNoRows`
 		syncWithBackedUpImages = true
 	}
 	if len(dbImages) == 0 {
@@ -155,7 +105,11 @@ func (m *Messenger) handleBackedUpProfile(message *protobuf.BackedUpProfile, bac
 	if syncWithBackedUpImages {
 		if len(message.Pictures) == 0 {
 			err = m.multiAccounts.DeleteIdentityImage(message.KeyUid)
-			response.AddImages(nil, err == nil)
+			if err != nil {
+				return err
+			}
+			contentSet = true
+			response.AddImages(nil)
 		} else {
 			idImages := make([]images.IdentityImage, len(message.Pictures))
 			for i, pic := range message.Pictures {
@@ -171,11 +125,15 @@ func (m *Messenger) handleBackedUpProfile(message *protobuf.BackedUpProfile, bac
 				idImages[i] = img
 			}
 			err = m.multiAccounts.StoreIdentityImages(message.KeyUid, idImages, false)
-			response.AddImages(idImages, err == nil)
+			if err != nil {
+				return err
+			}
+			contentSet = true
+			response.AddImages(idImages)
 		}
 	}
 
-	if m.config.messengerSignalsHandler != nil {
+	if m.config.messengerSignalsHandler != nil && contentSet {
 		m.config.messengerSignalsHandler.SendWakuBackedUpProfile(&response)
 	}
 
@@ -193,6 +151,7 @@ func (m *Messenger) handleBackedUpSettings(message *protobuf.SyncSetting) error
 		return nil
 	}
 
+	if settingField != nil {
 		response := wakusync.WakuBackedUpDataResponse{
 			Setting: settingField,
 		}
@@ -200,6 +159,7 @@ func (m *Messenger) handleBackedUpSettings(message *protobuf.SyncSetting) error
 		if m.config.messengerSignalsHandler != nil {
 			m.config.messengerSignalsHandler.SendWakuBackedUpSettings(&response)
 		}
+	}
 
 	return nil
 }

protocol/messenger_backup_test.go

@@ -122,9 +122,6 @@ func (s *MessengerBackupSuite) TestBackupContacts() {
 		"contacts not backed up",
 	)
 	s.Require().NoError(err)
-
-	bob2.backedupMessagesHandler.postponeTasksWaitGroup.Wait()
-
 	s.Require().Len(bob2.AddedContacts(), 2)
 
 	actualContacts = bob2.AddedContacts()
@@ -202,8 +199,6 @@ func (s *MessengerBackupSuite) TestBackupProfile() {
 	)
 	s.Require().NoError(err)
 
-	bob2.backedupMessagesHandler.postponeTasksWaitGroup.Wait()
-
 	// Check bob2
 	storedBob2DisplayName, err = bob2.settings.DisplayName()
 	s.Require().NoError(err)
@@ -303,8 +298,6 @@ func (s *MessengerBackupSuite) TestBackupSettings() {
 	)
 	s.Require().NoError(err)
 
-	bob2.backedupMessagesHandler.postponeTasksWaitGroup.Wait()
-
 	// Check bob2
 	storedBob2DisplayName, err = bob2.settings.DisplayName()
 	s.Require().NoError(err)
@@ -372,8 +365,6 @@ func (s *MessengerBackupSuite) TestBackupContactsGreaterThanBatch() {
 	)
 	s.Require().NoError(err)
 
-	bob2.backedupMessagesHandler.postponeTasksWaitGroup.Wait()
-
 	s.Require().Less(BackupContactsPerBatch*2, len(bob2.Contacts()))
 	s.Require().Len(bob2.AddedContacts(), BackupContactsPerBatch*2)
 }
@@ -448,8 +439,6 @@ func (s *MessengerBackupSuite) TestBackupRemovedContact() {
 	// Bob 2 should remove the contact
 	s.Require().NoError(err)
 
-	bob2.backedupMessagesHandler.postponeTasksWaitGroup.Wait()
-
 	s.Require().Len(bob2.AddedContacts(), 1)
 	s.Require().Equal(contactID1, bob2.AddedContacts()[0].ID)
 
@@ -502,8 +491,6 @@ func (s *MessengerBackupSuite) TestBackupLocalNickname() {
 	)
 	s.Require().NoError(err)
 
-	bob2.backedupMessagesHandler.postponeTasksWaitGroup.Wait()
-
 	for _, c := range bob2.Contacts() {
 		if c.ID == contactID1 {
 			actualContact = c
@@ -557,8 +544,6 @@ func (s *MessengerBackupSuite) TestBackupBlockedContacts() {
 	)
 	s.Require().NoError(err)
 
-	bob2.backedupMessagesHandler.postponeTasksWaitGroup.Wait()
-
 	s.Require().Len(bob2.AddedContacts(), 1)
 
 	actualContacts := bob2.AddedContacts()
@@ -633,8 +618,6 @@ func (s *MessengerBackupSuite) TestBackupCommunities() {
 
 	s.Require().NoError(err)
 
-	bob2.backedupMessagesHandler.postponeTasksWaitGroup.Wait()
-
 	communities, err = bob2.JoinedCommunities()
 	s.Require().NoError(err)
 	s.Require().Len(communities, 1)

protocol/wakusync/profile_response.go

@@ -6,17 +6,13 @@ import (
 
 type BackedUpProfile struct {
 	DisplayName string                 `json:"displayName,omitempty"`
-	DisplayNameStored bool                   `json:"displayNameStored,omitempty"`
 	Images      []images.IdentityImage `json:"images,omitempty,omitempty"`
-	ImagesStored      bool                   `json:"imagesStored,omitempty"`
 }
 
-func (sfwr *WakuBackedUpDataResponse) AddDisplayName(displayName string, stored bool) {
+func (sfwr *WakuBackedUpDataResponse) AddDisplayName(displayName string) {
 	sfwr.Profile.DisplayName = displayName
-	sfwr.Profile.DisplayNameStored = stored
 }
 
-func (sfwr *WakuBackedUpDataResponse) AddImages(images []images.IdentityImage, stored bool) {
+func (sfwr *WakuBackedUpDataResponse) AddImages(images []images.IdentityImage) {
-	sfwr.Profile.ImagesStored = stored
 	sfwr.Profile.Images = images
 }