diff --git a/protocol/messenger_response.go b/protocol/messenger_response.go index 7676f2f3d..adb4c645e 100644 --- a/protocol/messenger_response.go +++ b/protocol/messenger_response.go @@ -3,6 +3,8 @@ package protocol import ( "encoding/json" + "golang.org/x/exp/maps" + ensservice "github.com/status-im/status-go/services/ens" "github.com/status-im/status-go/services/browsers" @@ -510,11 +512,7 @@ func (r *MessengerResponse) AddSavedAddress(er *wallet.SavedAddress) { } func (r *MessengerResponse) SavedAddresses() []*wallet.SavedAddress { - var ers []*wallet.SavedAddress - for _, er := range r.savedAddresses { - ers = append(ers, er) - } - return ers + return maps.Values(r.savedAddresses) } func (r *MessengerResponse) AddEnsUsernameDetail(detail *ensservice.UsernameDetail) { diff --git a/protocol/messenger_saved_address.go b/protocol/messenger_saved_address.go index fec211325..9eef49a7d 100644 --- a/protocol/messenger_saved_address.go +++ b/protocol/messenger_saved_address.go @@ -15,15 +15,16 @@ import ( ) func (m *Messenger) UpsertSavedAddress(ctx context.Context, sa wallet.SavedAddress) error { - updatedClock, err := m.savedAddressesManager.UpdateMetadataAndUpsertSavedAddress(sa) + sa.UpdateClock, _ = m.getLastClockWithRelatedChat() + err := m.savedAddressesManager.UpdateMetadataAndUpsertSavedAddress(sa) if err != nil { return err } - return m.syncNewSavedAddress(ctx, &sa, updatedClock, m.dispatchMessage) + return m.syncNewSavedAddress(ctx, &sa, sa.UpdateClock, m.dispatchMessage) } func (m *Messenger) DeleteSavedAddress(ctx context.Context, address gethcommon.Address, isTest bool) error { - updateClock := uint64(time.Now().Unix()) + updateClock, _ := m.getLastClockWithRelatedChat() _, err := m.savedAddressesManager.DeleteSavedAddress(address, isTest, updateClock) if err != nil { return err @@ -31,7 +32,7 @@ func (m *Messenger) DeleteSavedAddress(ctx context.Context, address gethcommon.A return m.syncDeletedSavedAddress(ctx, address, isTest, updateClock, m.dispatchMessage) } -func (m *Messenger) GetSavedAddresses(ctx context.Context) ([]wallet.SavedAddress, error) { +func (m *Messenger) GetSavedAddresses(ctx context.Context) ([]*wallet.SavedAddress, error) { return m.savedAddressesManager.GetSavedAddresses() } @@ -89,13 +90,13 @@ func (m *Messenger) syncDeletedSavedAddress(ctx context.Context, address gethcom }, rawMessageHandler) } -func (m *Messenger) syncSavedAddress(ctx context.Context, savedAddress wallet.SavedAddress, rawMessageHandler RawMessageHandler) (err error) { +func (m *Messenger) syncSavedAddress(ctx context.Context, savedAddress *wallet.SavedAddress, rawMessageHandler RawMessageHandler) (err error) { if savedAddress.Removed { if err = m.syncDeletedSavedAddress(ctx, savedAddress.Address, savedAddress.IsTest, savedAddress.UpdateClock, rawMessageHandler); err != nil { return err } } else { - if err = m.syncNewSavedAddress(ctx, &savedAddress, savedAddress.UpdateClock, rawMessageHandler); err != nil { + if err = m.syncNewSavedAddress(ctx, savedAddress, savedAddress.UpdateClock, rawMessageHandler); err != nil { return err } } @@ -105,12 +106,14 @@ func (m *Messenger) syncSavedAddress(ctx context.Context, savedAddress wallet.Sa func (m *Messenger) HandleSyncSavedAddress(state *ReceivedMessageState, syncMessage *protobuf.SyncSavedAddress, statusMessage *v1protocol.StatusMessage) (err error) { address := gethcommon.BytesToAddress(syncMessage.Address) if syncMessage.Removed { - _, err = m.savedAddressesManager.DeleteSavedAddress( + deleted, err := m.savedAddressesManager.DeleteSavedAddress( address, syncMessage.IsTest, syncMessage.UpdateClock) if err != nil { return err } - state.Response.AddSavedAddress(&wallet.SavedAddress{Address: address, ENSName: syncMessage.Ens, IsTest: syncMessage.IsTest, Removed: true}) + if deleted { + state.Response.AddSavedAddress(&wallet.SavedAddress{Address: address, ENSName: syncMessage.Ens, IsTest: syncMessage.IsTest, Removed: true}) + } } else { sa := wallet.SavedAddress{ Address: address, @@ -120,12 +123,15 @@ func (m *Messenger) HandleSyncSavedAddress(state *ReceivedMessageState, syncMess IsTest: syncMessage.IsTest, ColorID: multiAccCommon.CustomizationColor(syncMessage.Color), } + sa.UpdateClock = syncMessage.UpdateClock - _, err = m.savedAddressesManager.AddSavedAddressIfNewerUpdate(sa, syncMessage.UpdateClock) + added, err := m.savedAddressesManager.AddSavedAddressIfNewerUpdate(sa) if err != nil { return err } - state.Response.AddSavedAddress(&sa) + if added { + state.Response.AddSavedAddress(&sa) + } } return } diff --git a/protocol/messenger_sync_saved_addresses_test.go b/protocol/messenger_sync_saved_addresses_test.go index aa04e0731..50e963faf 100644 --- a/protocol/messenger_sync_saved_addresses_test.go +++ b/protocol/messenger_sync_saved_addresses_test.go @@ -3,6 +3,8 @@ package protocol import ( "context" "crypto/ecdsa" + "reflect" + "sort" "testing" "github.com/stretchr/testify/suite" @@ -45,14 +47,14 @@ func (s *MessengerSyncSavedAddressesSuite) SetupTest() { s.shh = gethbridge.NewGethWakuWrapper(shh) s.Require().NoError(shh.Start()) - s.main = s.newMessenger(s.shh) + s.main = s.newMessenger(s.logger.Named("main")) s.privateKey = s.main.identity // Start the main messenger in order to receive installations _, err := s.main.Start() s.Require().NoError(err) // Create new device and add main account to - s.other, err = newMessengerWithKey(s.shh, s.main.identity, s.logger, nil) + s.other, err = newMessengerWithKey(s.shh, s.main.identity, s.logger.Named("other"), nil) s.Require().NoError(err) // Pair devices (main and other) @@ -82,11 +84,11 @@ func (s *MessengerSyncSavedAddressesSuite) TearDownTest() { TearDownMessenger(&s.Suite, s.main) } -func (s *MessengerSyncSavedAddressesSuite) newMessenger(shh types.Waku) *Messenger { +func (s *MessengerSyncSavedAddressesSuite) newMessenger(logger *zap.Logger) *Messenger { privateKey, err := crypto.GenerateKey() s.Require().NoError(err) - messenger, err := newMessengerWithKey(s.shh, privateKey, s.logger, nil) + messenger, err := newMessengerWithKey(s.shh, privateKey, logger, nil) s.Require().NoError(err) return messenger @@ -104,6 +106,9 @@ func contains[T comparable](container []T, element T, isEqual func(T, T) bool) b } func haveSameElements[T comparable](a []T, b []T, isEqual func(T, T) bool) bool { + if len(a) != len(b) { + return false + } for _, v := range a { if !contains(b, v, isEqual) { return false @@ -112,7 +117,7 @@ func haveSameElements[T comparable](a []T, b []T, isEqual func(T, T) bool) bool return true } -func savedAddressDataIsEqual(a, b wallet.SavedAddress) bool { +func savedAddressDataIsEqual(a, b *wallet.SavedAddress) bool { return a.Address == b.Address && a.IsTest == b.IsTest && a.Name == b.Name && a.ENSName == b.ENSName && a.ChainShortNames == b.ChainShortNames && a.ColorID == b.ColorID } @@ -134,15 +139,9 @@ func (s *MessengerSyncSavedAddressesSuite) TestSyncExistingSavedAddresses() { IsTest: isTestChain2, } - savedAddressesManager := s.main.savedAddressesManager - - _, err := savedAddressesManager.UpdateMetadataAndUpsertSavedAddress(sa1) + err := s.main.UpsertSavedAddress(context.Background(), sa1) s.Require().NoError(err) - _, err = savedAddressesManager.UpdateMetadataAndUpsertSavedAddress(sa2) - s.Require().NoError(err) - - // Trigger's a sync between devices - err = s.main.SyncDevices(context.Background(), "ens-name", "profile-image", nil) + err = s.main.UpsertSavedAddress(context.Background(), sa2) s.Require().NoError(err) // Wait and check that saved addresses are synced @@ -151,7 +150,7 @@ func (s *MessengerSyncSavedAddressesSuite) TestSyncExistingSavedAddresses() { func(r *MessengerResponse) bool { if len(r.SavedAddresses()) == 2 { sas := r.SavedAddresses() - s.Require().True(haveSameElements([]wallet.SavedAddress{sa1, sa2}, []wallet.SavedAddress{*sas[0], *sas[1]}, savedAddressDataIsEqual)) + s.Require().True(haveSameElements([]*wallet.SavedAddress{&sa1, &sa2}, []*wallet.SavedAddress{sas[0], sas[1]}, savedAddressDataIsEqual)) return true } return false @@ -163,7 +162,7 @@ func (s *MessengerSyncSavedAddressesSuite) TestSyncExistingSavedAddresses() { savedAddresses, err := s.other.savedAddressesManager.GetSavedAddresses() s.Require().NoError(err) s.Require().Equal(2, len(savedAddresses)) - s.Require().True(haveSameElements([]wallet.SavedAddress{sa1, sa2}, savedAddresses, savedAddressDataIsEqual)) + s.Require().True(haveSameElements([]*wallet.SavedAddress{&sa1, &sa2}, savedAddresses, savedAddressDataIsEqual)) } func (s *MessengerSyncSavedAddressesSuite) TestSyncSavedAddresses() { @@ -193,7 +192,7 @@ func (s *MessengerSyncSavedAddressesSuite) TestSyncSavedAddresses() { func(r *MessengerResponse) bool { if len(r.SavedAddresses()) == 2 { sas := r.SavedAddresses() - s.Require().True(haveSameElements([]wallet.SavedAddress{sa1, sa2}, []wallet.SavedAddress{*sas[0], *sas[1]}, savedAddressDataIsEqual)) + s.Require().True(haveSameElements([]*wallet.SavedAddress{&sa1, &sa2}, []*wallet.SavedAddress{sas[0], sas[1]}, savedAddressDataIsEqual)) return true } return false @@ -205,25 +204,30 @@ func (s *MessengerSyncSavedAddressesSuite) TestSyncSavedAddresses() { savedAddresses, err := s.other.savedAddressesManager.GetSavedAddresses() s.Require().NoError(err) s.Require().Equal(2, len(savedAddresses)) - s.Require().True(haveSameElements([]wallet.SavedAddress{sa1, sa2}, savedAddresses, savedAddressDataIsEqual)) + s.Require().True(haveSameElements([]*wallet.SavedAddress{&sa1, &sa2}, savedAddresses, savedAddressDataIsEqual)) } -func (s *MessengerSyncSavedAddressesSuite) testSyncDeletesOfSavedAddressesWithTestModes(testModeMain bool, testModeOther bool) { - var isTestChain1 bool = true - var isTestChain2 bool = false - var testAddress1 = common.Address{1} - var testAddress2 = common.Address{2} +func (s *MessengerSyncSavedAddressesSuite) requireSavedAddressesEqual(a, b []*wallet.SavedAddress) { + sort.Slice(a, func(i, j int) bool { + return a[i].Address.Hex() < a[j].Address.Hex() + }) + sort.Slice(b, func(i, j int) bool { + return b[i].Address.Hex() < b[j].Address.Hex() + }) + s.Require().True(reflect.DeepEqual(a, b)) +} - // Add saved addresses to main device - sa1 := wallet.SavedAddress{ - Address: testAddress1, +func (s *MessengerSyncSavedAddressesSuite) testSyncDeletesOfSavedAddresses(testModeMain bool, testModeOther bool) { + + sa1 := &wallet.SavedAddress{ + Address: common.Address{1}, Name: "TestC1A1", - IsTest: isTestChain1, + IsTest: true, } - sa2 := wallet.SavedAddress{ - Address: testAddress2, + sa2 := &wallet.SavedAddress{ + Address: common.Address{2}, Name: "TestC1A2", - IsTest: isTestChain2, + IsTest: false, } err := s.main.settings.SaveSettingField(settings.TestNetworksEnabled, testModeMain) @@ -231,92 +235,142 @@ func (s *MessengerSyncSavedAddressesSuite) testSyncDeletesOfSavedAddressesWithTe err = s.other.settings.SaveSettingField(settings.TestNetworksEnabled, testModeOther) s.Require().NoError(err) + // Add saved addresses to main device + err = s.main.UpsertSavedAddress(context.Background(), *sa1) s.Require().NoError(err) - err = s.main.UpsertSavedAddress(context.Background(), sa1) - s.Require().NoError(err) - err = s.main.UpsertSavedAddress(context.Background(), sa2) + err = s.main.UpsertSavedAddress(context.Background(), *sa2) s.Require().NoError(err) // Wait and check that saved addresses are synced - _, err = WaitOnMessengerResponse( - s.other, - func(r *MessengerResponse) bool { - if len(r.SavedAddresses()) == 2 { - sas := r.SavedAddresses() - s.Require().True(haveSameElements([]wallet.SavedAddress{sa1, sa2}, []wallet.SavedAddress{*sas[0], *sas[1]}, savedAddressDataIsEqual)) - return true - } - return false - }, - "expected to receive two changes", - ) - s.Require().NoError(err) + { + response, err := WaitOnMessengerResponse( + s.other, + func(r *MessengerResponse) bool { + return len(r.SavedAddresses()) == 2 + }, + "expected to receive two changes", + ) + s.Require().NoError(err) - savedAddresses, err := s.other.savedAddressesManager.GetSavedAddresses() - s.Require().NoError(err) - s.Require().Equal(2, len(savedAddresses)) + otherSavedAddresses := response.SavedAddresses() + s.Require().Len(otherSavedAddresses, 2) - // Delete saved addresses with test mode = true and sync with the other device - err = s.main.DeleteSavedAddress(context.Background(), sa1.Address, sa1.IsTest) - s.Require().NoError(err) + // Check that the UpdateClock was bumped + s.Require().GreaterOrEqual(otherSavedAddresses[0].CreatedAt, int64(0)) + s.Require().GreaterOrEqual(otherSavedAddresses[1].CreatedAt, int64(0)) + s.Require().Greater(otherSavedAddresses[0].UpdateClock, uint64(0)) + s.Require().Greater(otherSavedAddresses[1].UpdateClock, uint64(0)) - // Wait and check that saved addresses are synced - _, err = WaitOnMessengerResponse( - s.other, - func(r *MessengerResponse) bool { - if len(r.SavedAddresses()) == 1 { - sa := r.SavedAddresses()[0] - // We expect the deleted event to report address, ens, isTest - s.Require().Equal(sa1.Address, sa.Address) - s.Require().Equal(sa1.IsTest, sa.IsTest) - s.Require().Equal("", sa.Name) - return true - } - return false - }, - "expected to receive one change", - ) - s.Require().NoError(err) + // Reset the UpdateClock to 0 for comparison + otherSavedAddresses[0].CreatedAt = 0 + otherSavedAddresses[1].CreatedAt = 0 + otherSavedAddresses[0].UpdateClock = 0 + otherSavedAddresses[1].UpdateClock = 0 + s.requireSavedAddressesEqual([]*wallet.SavedAddress{sa1, sa2}, otherSavedAddresses) - savedAddresses, err = s.other.savedAddressesManager.GetSavedAddresses() - s.Require().NoError(err) - s.Require().True(haveSameElements([]wallet.SavedAddress{sa2}, savedAddresses, savedAddressDataIsEqual)) + // Ensure the messenger actually has the saved addresses, not just the response + savedAddresses, err := s.other.savedAddressesManager.GetSavedAddresses() + s.Require().NoError(err) + s.Require().Len(savedAddresses, 2) - // Delete saved addresses with test mode = false and sync with the other device - err = s.main.DeleteSavedAddress(context.Background(), sa2.Address, sa2.IsTest) - s.Require().NoError(err) + // Reset the UpdateClock to 0 for comparison + savedAddresses[0].CreatedAt = 0 + savedAddresses[1].CreatedAt = 0 + savedAddresses[0].UpdateClock = 0 + savedAddresses[1].UpdateClock = 0 + s.requireSavedAddressesEqual([]*wallet.SavedAddress{sa1, sa2}, savedAddresses) + } - // Wait and check that saved addresses are synced - _, err = WaitOnMessengerResponse( - s.other, - func(r *MessengerResponse) bool { - if len(r.SavedAddresses()) == 1 { - sa := r.SavedAddresses()[0] - // We expect the deleted event to report address, ens, isTest - s.Require().Equal(sa2.Address, sa.Address) - s.Require().Equal(sa2.IsTest, sa.IsTest) - s.Require().Equal("", sa.Name) - return true - } - return false - }, - "expected to receive one change", - ) - s.Require().NoError(err) + // Delete saved address 1 (test mode = true) and sync with the other device + { + err = s.main.DeleteSavedAddress(context.Background(), sa1.Address, sa1.IsTest) + s.Require().NoError(err) - savedAddresses, err = s.other.savedAddressesManager.GetSavedAddresses() - s.Require().NoError(err) - s.Require().Equal(0, len(savedAddresses)) + // Ensure the removal + savedAddresses, err := s.main.savedAddressesManager.GetSavedAddresses() + s.Require().NoError(err) + s.Require().Len(savedAddresses, 1) + sa2.CreatedAt = savedAddresses[0].CreatedAt // force same value + sa2.UpdateClock = savedAddresses[0].UpdateClock // force same value + s.Require().Equal(sa2, savedAddresses[0]) + + // Wait other device to receive the change + response, err := WaitOnMessengerResponse( + s.other, + func(r *MessengerResponse) bool { + return len(r.SavedAddresses()) == 1 + }, + "saved address removal wasn't received", + ) + s.Require().NoError(err) + + // We expect the delete event to report address, ens, isTest + sa := response.SavedAddresses()[0] + s.Require().Equal(sa1.Address, sa.Address) + s.Require().Equal(sa1.IsTest, sa.IsTest) + s.Require().Equal("", sa.Name) + + // Ensure the messenger doesn't return the removed address + savedAddresses, err = s.other.savedAddressesManager.GetSavedAddresses() + s.Require().NoError(err) + s.Require().Len(savedAddresses, 1) + savedAddresses[0].CreatedAt = sa2.CreatedAt // force same value + s.Require().Equal(sa2, savedAddresses[0]) + } + + // Delete saved address 2 (test mode = false) and sync with the other device + { + err = s.main.DeleteSavedAddress(context.Background(), sa2.Address, sa2.IsTest) + s.Require().NoError(err) + + // Ensure the removal + savedAddresses, err := s.main.savedAddressesManager.GetSavedAddresses() + s.Require().NoError(err) + s.Require().Len(savedAddresses, 0) + + // Wait other device to receive the change + response, err := WaitOnMessengerResponse( + s.other, + func(r *MessengerResponse) bool { + return len(r.SavedAddresses()) == 1 + }, + "expected to receive one change", + ) + s.Require().NoError(err) + + sa := response.SavedAddresses()[0] + // We expect the deleted event to report address, ens, isTest + s.Require().Equal(sa2.Address, sa.Address) + s.Require().Equal(sa2.IsTest, sa.IsTest) + s.Require().Equal("", sa.Name) + + savedAddresses, err = s.other.savedAddressesManager.GetSavedAddresses() + s.Require().NoError(err) + s.Require().Len(savedAddresses, 0) + } } -func (s *MessengerSyncSavedAddressesSuite) TestSyncDeletesOfSavedAddressesSameTestModeOnBothDevices() { - testModeMain := true - testModeOther := testModeMain - s.testSyncDeletesOfSavedAddressesWithTestModes(testModeMain, testModeOther) -} +func (s *MessengerSyncSavedAddressesSuite) TestSyncDeletesOfSavedAddresses() { + testCases := []struct { + Name string + TestModeMain bool + TestModeOther bool + }{ + { + Name: "same test mode on both devices", + TestModeMain: true, + TestModeOther: true, + }, + { + Name: "different test mode on devices", + TestModeMain: true, + TestModeOther: false, + }, + } -func (s *MessengerSyncSavedAddressesSuite) TestSyncDeletesOfSavedAddressesDifferentTestModeOnDevices() { - testModeMain := true - testModeOther := !testModeMain - s.testSyncDeletesOfSavedAddressesWithTestModes(testModeMain, testModeOther) + for _, tc := range testCases { + s.Run(tc.Name, func() { + s.testSyncDeletesOfSavedAddresses(tc.TestModeMain, tc.TestModeOther) + }) + } } diff --git a/services/ext/api.go b/services/ext/api.go index 2c4c66fa2..444eeb455 100644 --- a/services/ext/api.go +++ b/services/ext/api.go @@ -1076,7 +1076,7 @@ func (api *PublicAPI) DeleteSavedAddress(ctx context.Context, address ethcommon. return api.service.messenger.DeleteSavedAddress(ctx, address, isTest) } -func (api *PublicAPI) GetSavedAddresses(ctx context.Context) ([]wallet.SavedAddress, error) { +func (api *PublicAPI) GetSavedAddresses(ctx context.Context) ([]*wallet.SavedAddress, error) { return api.service.messenger.GetSavedAddresses(ctx) } diff --git a/services/wallet/saved_addresses.go b/services/wallet/saved_addresses.go index ea395d014..2465681de 100644 --- a/services/wallet/saved_addresses.go +++ b/services/wallet/saved_addresses.go @@ -42,10 +42,10 @@ func NewSavedAddressesManager(db *sql.DB) *SavedAddressesManager { const rawQueryColumnsOrder = "address, name, removed, update_clock, chain_short_names, ens_name, is_test, created_at, color" // getSavedAddressesFromDBRows retrieves all data based on SELECT Query using rawQueryColumnsOrder -func getSavedAddressesFromDBRows(rows *sql.Rows) ([]SavedAddress, error) { - var addresses []SavedAddress +func getSavedAddressesFromDBRows(rows *sql.Rows) ([]*SavedAddress, error) { + var addresses []*SavedAddress for rows.Next() { - sa := SavedAddress{} + sa := &SavedAddress{} // based on rawQueryColumnsOrder err := rows.Scan( &sa.Address, @@ -68,7 +68,7 @@ func getSavedAddressesFromDBRows(rows *sql.Rows) ([]SavedAddress, error) { return addresses, nil } -func (sam *SavedAddressesManager) getSavedAddresses(condition string) ([]SavedAddress, error) { +func (sam *SavedAddressesManager) getSavedAddresses(condition string) ([]*SavedAddress, error) { var whereCondition string if condition != "" { whereCondition = fmt.Sprintf("WHERE %s", condition) @@ -84,12 +84,12 @@ func (sam *SavedAddressesManager) getSavedAddresses(condition string) ([]SavedAd return addresses, err } -func (sam *SavedAddressesManager) GetSavedAddresses() ([]SavedAddress, error) { +func (sam *SavedAddressesManager) GetSavedAddresses() ([]*SavedAddress, error) { return sam.getSavedAddresses("removed != 1") } // GetRawSavedAddresses provides access to the soft-delete and sync metadata -func (sam *SavedAddressesManager) GetRawSavedAddresses() ([]SavedAddress, error) { +func (sam *SavedAddressesManager) GetRawSavedAddresses() ([]*SavedAddress, error) { return sam.getSavedAddresses("") } @@ -153,13 +153,8 @@ func (sam *SavedAddressesManager) upsertSavedAddress(sa SavedAddress, tx *sql.Tx return err } -func (sam *SavedAddressesManager) UpdateMetadataAndUpsertSavedAddress(sa SavedAddress) (updatedClock uint64, err error) { - sa.UpdateClock = uint64(time.Now().Unix()) - err = sam.upsertSavedAddress(sa, nil) - if err != nil { - return 0, err - } - return sa.UpdateClock, nil +func (sam *SavedAddressesManager) UpdateMetadataAndUpsertSavedAddress(sa SavedAddress) error { + return sam.upsertSavedAddress(sa, nil) } func (sam *SavedAddressesManager) startTransactionAndCheckIfNewerChange(address common.Address, isTest bool, updateClock uint64) (newer bool, tx *sql.Tx, err error) { @@ -180,8 +175,8 @@ func (sam *SavedAddressesManager) startTransactionAndCheckIfNewerChange(address return dbUpdateClock < updateClock, tx, nil } -func (sam *SavedAddressesManager) AddSavedAddressIfNewerUpdate(sa SavedAddress, updateClock uint64) (insertedOrUpdated bool, err error) { - newer, tx, err := sam.startTransactionAndCheckIfNewerChange(sa.Address, sa.IsTest, updateClock) +func (sam *SavedAddressesManager) AddSavedAddressIfNewerUpdate(sa SavedAddress) (insertedOrUpdated bool, err error) { + newer, tx, err := sam.startTransactionAndCheckIfNewerChange(sa.Address, sa.IsTest, sa.UpdateClock) defer func() { if err == nil { err = tx.Commit() @@ -193,7 +188,6 @@ func (sam *SavedAddressesManager) AddSavedAddressIfNewerUpdate(sa SavedAddress, return false, err } - sa.UpdateClock = updateClock err = sam.upsertSavedAddress(sa, tx) if err != nil { return false, err diff --git a/services/wallet/saved_addresses_test.go b/services/wallet/saved_addresses_test.go index 9f24d2e83..ba7bcf2b9 100644 --- a/services/wallet/saved_addresses_test.go +++ b/services/wallet/saved_addresses_test.go @@ -45,7 +45,7 @@ func TestSavedAddressesAdd(t *testing.T) { IsTest: false, } - _, err = manager.UpdateMetadataAndUpsertSavedAddress(sa) + err = manager.UpdateMetadataAndUpsertSavedAddress(sa) require.NoError(t, err) rst, err = manager.GetRawSavedAddresses() @@ -77,7 +77,7 @@ func haveSameElements[T comparable](a []T, b []T, isEqual func(T, T) bool) bool return true } -func savedAddressDataIsEqual(a, b SavedAddress) bool { +func savedAddressDataIsEqual(a, b *SavedAddress) bool { return a.Address == b.Address && a.Name == b.Name && a.ChainShortNames == b.ChainShortNames && a.ENSName == b.ENSName && a.IsTest == b.IsTest && a.ColorID == b.ColorID } @@ -117,10 +117,12 @@ func TestSavedAddressesMetadata(t *testing.T) { Name: "Simple", ColorID: multiAccCommon.CustomizationColorBlue, IsTest: false, + savedAddressMeta: savedAddressMeta{ + UpdateClock: 42, + }, } - var sa2UpdatedClock uint64 - sa2UpdatedClock, err = manager.UpdateMetadataAndUpsertSavedAddress(sa2) + err = manager.UpdateMetadataAndUpsertSavedAddress(sa2) require.NoError(t, err) dbSavedAddresses, err = manager.GetRawSavedAddresses() @@ -129,7 +131,7 @@ func TestSavedAddressesMetadata(t *testing.T) { // The order is not guaranteed check raw entry to decide rawIndex := 0 simpleIndex := 1 - if dbSavedAddresses[0] != sa1 { + if dbSavedAddresses[0].Address == sa1.Address { rawIndex = 1 simpleIndex = 0 } @@ -141,18 +143,20 @@ func TestSavedAddressesMetadata(t *testing.T) { // Check the default values require.False(t, dbSavedAddresses[rawIndex].Removed) - require.Equal(t, dbSavedAddresses[rawIndex].UpdateClock, sa2UpdatedClock) + require.Equal(t, dbSavedAddresses[rawIndex].UpdateClock, sa2.UpdateClock) require.Greater(t, dbSavedAddresses[rawIndex].UpdateClock, uint64(0)) sa2Older := sa2 sa2Older.IsTest = false + sa2Older.UpdateClock = dbSavedAddresses[rawIndex].UpdateClock - 1 sa2Newer := sa2 sa2Newer.IsTest = false + sa2Newer.UpdateClock = dbSavedAddresses[rawIndex].UpdateClock + 1 // Try to add an older entry updated := false - updated, err = manager.AddSavedAddressIfNewerUpdate(sa2Older, dbSavedAddresses[rawIndex].UpdateClock-1) + updated, err = manager.AddSavedAddressIfNewerUpdate(sa2Older) require.NoError(t, err) require.False(t, updated) @@ -161,18 +165,18 @@ func TestSavedAddressesMetadata(t *testing.T) { rawIndex = 0 simpleIndex = 1 - if dbSavedAddresses[0] != sa1 { + if dbSavedAddresses[0].Address == sa1.Address { rawIndex = 1 simpleIndex = 0 } require.Equal(t, 2, len(dbSavedAddresses)) - require.True(t, haveSameElements([]SavedAddress{sa1, sa2}, dbSavedAddresses, savedAddressDataIsEqual)) + require.True(t, haveSameElements([]*SavedAddress{&sa1, &sa2}, dbSavedAddresses, savedAddressDataIsEqual)) require.Equal(t, sa1.savedAddressMeta, dbSavedAddresses[simpleIndex].savedAddressMeta) // Try to update sa2 with a newer entry updatedClock := dbSavedAddresses[rawIndex].UpdateClock + 1 - updated, err = manager.AddSavedAddressIfNewerUpdate(sa2Newer, updatedClock) + updated, err = manager.AddSavedAddressIfNewerUpdate(sa2Newer) require.NoError(t, err) require.True(t, updated) @@ -180,7 +184,7 @@ func TestSavedAddressesMetadata(t *testing.T) { require.NoError(t, err) require.Equal(t, 2, len(dbSavedAddresses)) - require.True(t, haveSameElements([]SavedAddress{sa1, sa2Newer}, dbSavedAddresses, savedAddressDataIsEqual)) + require.True(t, haveSameElements([]*SavedAddress{&sa1, &sa2Newer}, dbSavedAddresses, savedAddressDataIsEqual)) require.Equal(t, updatedClock, dbSavedAddresses[rawIndex].UpdateClock) // Try to delete the sa2 newer entry @@ -254,7 +258,7 @@ func TestSavedAddressesGet(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, len(dbSavedAddresses)) - require.True(t, savedAddressDataIsEqual(sa, dbSavedAddresses[0])) + require.True(t, savedAddressDataIsEqual(&sa, dbSavedAddresses[0])) dbSavedAddresses, err = manager.GetSavedAddresses() require.NoError(t, err) @@ -277,7 +281,7 @@ func TestSavedAddressesDelete(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, len(rst)) - require.True(t, savedAddressDataIsEqual(sa0, rst[0])) + require.True(t, savedAddressDataIsEqual(&sa0, rst[0])) // Modify IsTest flag, insert sa1 := sa0 @@ -294,7 +298,7 @@ func TestSavedAddressesDelete(t *testing.T) { rst, err = manager.GetSavedAddresses() require.NoError(t, err) require.Equal(t, 1, len(rst)) - require.True(t, savedAddressDataIsEqual(sa1, rst[0])) + require.True(t, savedAddressDataIsEqual(&sa1, rst[0])) // Test that we still have both addresses rst, err = manager.GetRawSavedAddresses() @@ -325,7 +329,7 @@ func testInsertSameAddressWithOneChange(t *testing.T, member int) { require.NoError(t, err) require.Equal(t, 1, len(rst)) - require.True(t, savedAddressDataIsEqual(sa, rst[0])) + require.True(t, savedAddressDataIsEqual(&sa, rst[0])) sa2 := sa @@ -350,12 +354,12 @@ func testInsertSameAddressWithOneChange(t *testing.T, member int) { // guaranteed to be the same as insertions, so swap indices if first record does not match firstIndex := 0 secondIndex := 1 - if rst[firstIndex] != sa { + if rst[firstIndex].Address == sa.Address { firstIndex = 1 secondIndex = 0 } - require.True(t, savedAddressDataIsEqual(sa, rst[firstIndex])) - require.True(t, savedAddressDataIsEqual(sa2, rst[secondIndex])) + require.True(t, savedAddressDataIsEqual(&sa, rst[firstIndex])) + require.True(t, savedAddressDataIsEqual(&sa2, rst[secondIndex])) } func TestSavedAddressesAddDifferentIsTest(t *testing.T) { @@ -380,7 +384,7 @@ func TestSavedAddressesAddSame(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, len(rst)) - require.True(t, savedAddressDataIsEqual(sa, rst[0])) + require.True(t, savedAddressDataIsEqual(&sa, rst[0])) sa2 := sa err = manager.upsertSavedAddress(sa2, nil)