Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid getting/storing ourselves as contact #3719

Closed
igor-sirotin opened this issue Jul 5, 2023 · 1 comment
Closed

Avoid getting/storing ourselves as contact #3719

igor-sirotin opened this issue Jul 5, 2023 · 1 comment

Comments

@igor-sirotin
Copy link
Collaborator

igor-sirotin commented Jul 5, 2023 •
edited
Loading

Problem

As found out in #3667, we were loading (and if not found, storing) ourselves to ContactsMap, which shouldn't be done.
I've added required conditions in #3627, but also tracked down all places we were doing this.

To gather this info I've added callstack logging (04675d8) and a python script to analyse output of make test.

Show me the python script 
import json
import re


class SetEncoder(json.JSONEncoder):
  def default(self, obj):
      if isinstance(obj, set):
          return list(obj)
      return json.JSONEncoder.default(self, obj)

file_name = 'vendor/status-go/tests_5.log'
file1 = open(file_name, 'r')
lines = file1.readlines()
file1.close()

full_dict = {}
loads_count = 0
stacks_dict = {}

for index, line in enumerate(lines):

  # FIXME: Fix logger format difference
  #        Example at lines: {47540, 47556, 47570, 47630, 47644, 47658, 47770, 47779, 47803, 47806, 47807}
  #        diff = set(loads.keys()) ^ set(loads2.keys())
  match_all = re.match(r'.*contacts map: loading own identity.*', line)
  if match_all is None:
      continue

  loads_count += 1
  m = re.match(r'.*contacts map: loading own identity.*"stack": (?P<stack>.*)}', line)
  if m is None:
      continue

  stack_json = m.group('stack')
  stack = json.loads(stack_json)
  full_dict[index] = stack

  stack_key = []
  for stack_item in stack[1:4]:
      stack_key.append(stack_item['file'])
  stack_key_string = json.dumps(stack_key)

  if stack_key_string not in stacks_dict:
      stacks_dict[stack_key_string] = {'count': 1, 'last': stack, 'message_types': set()}
  else:
      stacks_dict[stack_key_string]['count'] += 1
      stacks_dict[stack_key_string]['last'] = stack
      # Analyze previous line for "processing message"
      prev_line = lines[index - 1]
      m2 = re.match(r'.*processing message.*{"type": "(?P<message_type>.*)", "senderID".*', prev_line)
      if m2 is not None:
          message_type = m2.group('message_type')
          stacks_dict[stack_key_string]['message_types'].add(message_type)

print("Total lines found:   {}".format(len(lines)))
print("Lost records:        {}".format(loads_count - len(full_dict.keys())))
print("Processed records:   {}".format(len(full_dict.keys())))
print("Unique stacks found: {}".format(len(stacks_dict.keys())))

file = open("stacks_2.json", "w")
file.write(json.dumps(stacks_dict, cls=SetEncoder))
file.close()

According to my investigation, there're 4 such places:

1. When receiving a message from ourselves

... with one of following types:

PIN_MESSAGE
COMMUNITY_ADMIN_MESSAGE
UNKNOWN
COMMUNITY_DESCRIPTION
PAIR_INSTALLATION
DELETE_MESSAGE
CONTACT_CODE_ADVERTISEMENT
CHAT_MESSAGE
SYNC_COMMUNITY_SETTINGS
BACKUP
SYNC_ACTIVITY_CENTER_NOTIFICATION
SYNC_INSTALLATION_COMMUNITY
STATUS_UPDATE

status-go/protocol/messenger.go

Line 3441 in 04675d8

contact, contactFound := messageState.AllContacts.Load(senderID)

encountered 364 times during tests

Full call stack 
    {
      "file": "/opt/homebrew/Cellar/go/1.20.3/libexec/src/runtime/debug/stack.go:24 +0x64",
      "function": "runtime/debug.Stack()"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_maps.go:122 +0xb8",
      "function": "github.com/status-im/status-go/protocol.(*contactMap).Load(0x140012b2f00, {0x14004f58120, 0x84})"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger.go:3441 +0xf9c",
      "function": "github.com/status-im/status-go/protocol.(*Messenger).handleRetrievedMessages(0x14002076000, 0x30000007d?, 0x1)"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger.go:3038 +0x4c",
      "function": "github.com/status-im/status-go/protocol.(*Messenger).RetrieveAll(0x14002076000)"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_testing_utils.go:15 +0x40",
      "function": "github.com/status-im/status-go/protocol.WaitOnMessengerResponse.func1()"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/vendor/github.com/cenkalti/backoff/v3/retry.go:52 +0xc8",
      "function": "github.com/cenkalti/backoff/v3.RetryNotifyWithTimer(0x140029cd238, {0x1030c7328, 0x1400144a360}, 0x0, {0x0?, 0x0?})"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/vendor/github.com/cenkalti/backoff/v3/retry.go:31",
      "function": "github.com/cenkalti/backoff/v3.RetryNotify(...)"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/vendor/github.com/cenkalti/backoff/v3/retry.go:25",
      "function": "github.com/cenkalti/backoff/v3.Retry(...)"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/tt/backoff.go:24 +0x198",
      "function": "github.com/status-im/status-go/protocol/tt.RetryWithBackOff(0x140009c84e0?, {0x0, 0x0, 0x10216e02c?})"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_testing_utils.go:13 +0x88",
      "function": "github.com/status-im/status-go/protocol.WaitOnMessengerResponse(0x14002076000, 0x1030b44f8, {0x1023541d5, 0xb})"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/sync_device_test.go:234 +0x418",
      "function": "github.com/status-im/status-go/server/pairing.(*SyncDeviceSuite).acceptContactRequest(0x140009aa630, 0x140016bd340, 0x14002076000, 0x140013e3b80)"
    },
    {
      "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/sync_device_test.go:552 +0x56c",
      "function": "github.com/status-im/status-go/server/pairing.(*SyncDeviceSuite).TestPairingThreeDevices(0x140009aa630)"
    }

2. SyncDevices -> sendContactUpdate with myID

status-go/protocol/messenger.go

Line 2393 in 04675d8

if _, err = m.sendContactUpdate(ctx, myID, displayName, ensName, photoPath, rawMessageHandler); err != nil {

encountered 8 times during tests

Full call stack 
{
  "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_maps.go:122 +0xb8",
  "function": "github.com/status-im/status-go/protocol.(*contactMap).Load(0x140051279b0, {0x1400126acf0, 0x84})"
},
{
  "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_contacts.go:1031 +0x78",
  "function": "github.com/status-im/status-go/protocol.(*Messenger).sendContactUpdate(0x14000c9f080, {0x1030d31f0, 0x140000440d8}, {0x1400126acf0, 0x84}, {0x0, 0x0}, {0x14000cd98a8, 0x16}, {0x0, ...}, ...)"
},
{
  "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger.go:2393 +0x12c",
  "function": "github.com/status-im/status-go/protocol.(*Messenger).SyncDevices(0x14000c9f080, {0x1030d31f0, 0x140000440d8}, {0x14000cd98a8, 0x16}, {0x0, 0x0}, 0x140010bf0b8)"
},
{
  "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/raw_message_handler.go:60 +0x1fc",
  "function": "github.com/status-im/status-go/server/pairing.(*SyncRawMessageHandler).PrepareRawMessage(0x140011062a8, {0x140012620f0, 0x42}, {0x1400509c060, 0x7})"
},
{
  "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/payload_mounter.go:173 +0x38",
  "function": "github.com/status-im/status-go/server/pairing.(*RawMessageLoader).Load(0x14005023560)"
},
{
  "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/payload_mounter.go:45 +0x30",
  "function": "github.com/status-im/status-go/server/pairing.(*BasePayloadMounter).Mount(0x1400502b100)"
},
{
  "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/handlers.go:104 +0x148",
  "function": "github.com/status-im/status-go/server/pairing.handlePairingSyncDeviceSend.func1({0x1030d18a0, 0x14004f4f6c0}, 0x140010bf998?)"
},
{
  "file": "/opt/homebrew/Cellar/go/1.20.3/libexec/src/net/http/server.go:2122 +0x38",
  "function": "net/http.HandlerFunc.ServeHTTP(0x140050234a0?, {0x1030d18a0?, 0x14004f4f6c0?}, 0x14002bf2700?)"
},
{
  "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/handlers.go:191 +0x1b0",
  "function": "github.com/status-im/status-go/server/pairing.middlewareChallenge.func1({0x1030d18a0, 0x14004f4f6c0}, 0x3b?)"
},
{
  "file": "/opt/homebrew/Cellar/go/1.20.3/libexec/src/net/http/server.go:2122 +0x38",
  "function": "net/http.HandlerFunc.ServeHTTP(0x140010bfad8?, {0x1030d18a0?, 0x14004f4f6c0?}, 0x10?)"
},
{
  "file": "/opt/homebrew/Cellar/go/1.20.3/libexec/src/net/http/server.go:2500 +0x140",
  "function": "net/http.(*ServeMux).ServeHTTP(0x0?, {0x1030d18a0, 0x14004f4f6c0}, 0x14002bf2700)"
},
{
  "file": "/opt/homebrew/Cellar/go/1.20.3/libexec/src/net/http/server.go:2936 +0x2d8",
  "function": "net/http.serverHandler.ServeHTTP({0x14000e27f80?}, {0x1030d18a0, 0x14004f4f6c0}, 0x14002bf2700)"
},
{
  "file": "/opt/homebrew/Cellar/go/1.20.3/libexec/src/net/http/server.go:1995 +0x560",
  "function": "net/http.(*conn).serve(0x1400249d830, {0x1030d3260, 0x140050237d0})"
},
{
  "file": "/opt/homebrew/Cellar/go/1.20.3/libexec/src/net/http/server.go:3089 +0x520",
  "function": "created by net/http.(*Server).Serve"
}

3. updateAcceptedContactRequest

status-go/protocol/messenger_contacts.go

Line 286 in 04675d8

contact, ok := m.allContacts.Load(contactRequest.From)

encountered 1 time during tests

Full call stack ```json { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_maps.go:122 +0xb8", "function": "github.com/status-im/status-go/protocol.(*contactMap).Load(0x140028a19e0, {0x14003481b90, 0x84})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_contacts.go:286 +0x364", "function": "github.com/status-im/status-go/protocol.(*Messenger).updateAcceptedContactRequest(0x14000ffe580, 0x0, {0x140034819e0, 0x86})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_handler.go:3229 +0x54", "function": "github.com/status-im/status-go/protocol.(*Messenger).HandleSyncContactRequestDecision(0x102337708?, 0x140016e4660, {0x18925520d0e, {0x140034819e0, 0x86}, 0x0, {}, {0x0, 0x0, 0x0}, ...})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_sync_raw_messages.go:192 +0x1fac", "function": "github.com/status-im/status-go/protocol.(*Messenger).HandleSyncRawMessages(0x14000ffe580, {0x14000646800, 0xf, 0x24?})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/raw_message_handler.go:125 +0x428", "function": "github.com/status-im/status-go/server/pairing.(*SyncRawMessageHandler).HandleRawMessage(0x14001584fa8?, 0x1021618d0?, 0x14002aca000?, {0x14005194dd0?, 0x800?}, {0x14005194dc0, 0x7}, 0x14000e27b60)" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/payload_receiver.go:258 +0x4c", "function": "github.com/status-im/status-go/server/pairing.(*RawMessageStorer).Store(0x140005b5d50?)" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/payload_receiver.go:66 +0x98", "function": "github.com/status-im/status-go/server/pairing.(*BasePayloadReceiver).Receive(0x140006b7600, {0x14001076000?, 0x102f152e0?, 0x14002bb2f80?})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/client.go:387 +0x1a8", "function": "github.com/status-im/status-go/server/pairing.(*ReceiverClient).receiveSyncDeviceData(0x140006b78c0)" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/client.go:474 +0x60", "function": "github.com/status-im/status-go/server/pairing.StartUpReceivingClient(0x1030ca438?, {0x1400098c540?, 0x1ace?}, {0x140002d7100?, 0x0?})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/sync_device_test.go:187 +0x5cc", "function": "github.com/status-im/status-go/server/pairing.(*SyncDeviceSuite).pairAccounts(0x140009aa630, 0x140013d7c20, {0x14000cf8150, 0x70}, 0x140013d7e00, {0x14000cf81c0, 0x70})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/sync_device_test.go:568 +0x6b4", "function": "github.com/status-im/status-go/server/pairing.(*SyncDeviceSuite).TestPairingThreeDevices(0x140009aa630)" }, ```

4. HandleSyncRawMessages -> saveDataAndPrepareResponse

status-go/protocol/messenger.go

Line 4499 in 04675d8

contact, ok := m.allContacts.Load(chat.ID)

encountered 1 time during tests

Full call stack ```json [ { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_maps.go:122 +0xb8", "function": "github.com/status-im/status-go/protocol.(*contactMap).Load(0x140028a19e0, {0x14000672120, 0x84})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger.go:4499 +0x134", "function": "github.com/status-im/status-go/protocol.(*Messenger).saveDataAndPrepareResponse(0x14000ffe580, 0x140016e4660)" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/protocol/messenger_sync_raw_messages.go:306 +0x2b4c", "function": "github.com/status-im/status-go/protocol.(*Messenger).HandleSyncRawMessages(0x14000ffe580, {0x14000646800, 0xf, 0x24?})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/raw_message_handler.go:125 +0x428", "function": "github.com/status-im/status-go/server/pairing.(*SyncRawMessageHandler).HandleRawMessage(0x14001584fa8?, 0x1021618d0?, 0x14002aca000?, {0x14005194dd0?, 0x800?}, {0x14005194dc0, 0x7}, 0x14000e27b60)" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/payload_receiver.go:258 +0x4c", "function": "github.com/status-im/status-go/server/pairing.(*RawMessageStorer).Store(0x140005b5d50?)" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/payload_receiver.go:66 +0x98", "function": "github.com/status-im/status-go/server/pairing.(*BasePayloadReceiver).Receive(0x140006b7600, {0x14001076000?, 0x102f152e0?, 0x14002bb2f80?})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/client.go:387 +0x1a8", "function": "github.com/status-im/status-go/server/pairing.(*ReceiverClient).receiveSyncDeviceData(0x140006b78c0)" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/client.go:474 +0x60", "function": "github.com/status-im/status-go/server/pairing.StartUpReceivingClient(0x1030ca438?, {0x1400098c540?, 0x1ace?}, {0x140002d7100?, 0x0?})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/sync_device_test.go:187 +0x5cc", "function": "github.com/status-im/status-go/server/pairing.(*SyncDeviceSuite).pairAccounts(0x140009aa630, 0x140013d7c20, {0x14000cf8150, 0x70}, 0x140013d7e00, {0x14000cf81c0, 0x70})" }, { "file": "/Users/igorsirotin/Repositories/Status/status-desktop/vendor/status-go/server/pairing/sync_device_test.go:568 +0x6b4", "function": "github.com/status-im/status-go/server/pairing.(*SyncDeviceSuite).TestPairingThreeDevices(0x140009aa630)" }, ] ```
@igor-sirotin igor-sirotin mentioned this issue Jul 5, 2023
Merged
@igor-sirotin igor-sirotin changed the title When we try to store ourselves as contact Avoid storing ourselves as contact Jul 5, 2023
@igor-sirotin igor-sirotin mentioned this issue Aug 3, 2023
Merged
@igor-sirotin igor-sirotin mentioned this issue Oct 16, 2023
Closed
@igor-sirotin igor-sirotin changed the title Avoid storing ourselves as contact Avoid getting/storing ourselves as contact Oct 29, 2023
@cammellos
Copy link
Contributor

This is complete I believe, feel free to reopen if necessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@cammellos @igor-sirotin