fix(chat): use clock for messages ordering

fixes: #8153
This commit is contained in:
Patryk Osmaczko 2022-11-10 12:41:37 +01:00 committed by Boris Melnik
parent b10b6617cf
commit 263b1c01c6
5 changed files with 20 additions and 46 deletions

View File

@ -86,7 +86,7 @@ proc createMessageItemFromDto(self: Module, message: MessageDto, chatDetails: Ch
message.containsContactMentions(), message.containsContactMentions(),
message.seen, message.seen,
timestamp = message.whisperTimestamp, timestamp = message.whisperTimestamp,
localTimestamp = message.timestamp, clock = message.clock,
ContentType(message.contentType), ContentType(message.contentType),
message.messageType, message.messageType,
message.contactRequestState, message.contactRequestState,

View File

@ -88,7 +88,7 @@ proc createFetchMoreMessagesItem(self: Module): Item =
messageContainsMentions = false, messageContainsMentions = false,
seen = true, seen = true,
timestamp = 0, timestamp = 0,
localTimestamp = 0, clock = 0,
ContentType.FetchMoreMessagesButton, ContentType.FetchMoreMessagesButton,
messageType = -1, messageType = -1,
contactRequestState = 0, contactRequestState = 0,
@ -129,7 +129,7 @@ proc createChatIdentifierItem(self: Module): Item =
messageContainsMentions = false, messageContainsMentions = false,
seen = true, seen = true,
timestamp = 0, timestamp = 0,
localTimestamp = 0, clock = 0,
ContentType.ChatIdentifier, ContentType.ChatIdentifier,
messageType = -1, messageType = -1,
contactRequestState = 0, contactRequestState = 0,
@ -197,7 +197,7 @@ method newMessagesLoaded*(self: Module, messages: seq[MessageDto], reactions: se
m.containsContactMentions(), m.containsContactMentions(),
m.seen, m.seen,
timestamp = m.whisperTimestamp, timestamp = m.whisperTimestamp,
localTimestamp = m.timestamp, clock = m.clock,
m.contentType.ContentType, m.contentType.ContentType,
m.messageType, m.messageType,
m.contactRequestState, m.contactRequestState,
@ -291,7 +291,7 @@ method messageAdded*(self: Module, message: MessageDto) =
message.containsContactMentions(), message.containsContactMentions(),
message.seen, message.seen,
timestamp = message.whisperTimestamp, timestamp = message.whisperTimestamp,
localTimestamp = message.timestamp, clock = message.clock,
message.contentType.ContentType, message.contentType.ContentType,
message.messageType, message.messageType,
message.contactRequestState, message.contactRequestState,
@ -312,7 +312,7 @@ method messageAdded*(self: Module, message: MessageDto) =
message.discordMessage message.discordMessage
) )
self.view.model().insertItemBasedOnTimestamp(item) self.view.model().insertItemBasedOnClock(item)
method onSendingMessageSuccess*(self: Module, message: MessageDto) = method onSendingMessageSuccess*(self: Module, message: MessageDto) =
self.messageAdded(message) self.messageAdded(message)

View File

@ -178,7 +178,7 @@ proc buildPinnedMessageItem(self: Module, messageId: string, actionInitiatedBy:
m.containsContactMentions(), m.containsContactMentions(),
m.seen, m.seen,
timestamp = m.whisperTimestamp, timestamp = m.whisperTimestamp,
localTimestamp = m.timestamp, clock = m.clock,
m.contentType.ContentType, m.contentType.ContentType,
m.messageType, m.messageType,
m.contactRequestState, m.contactRequestState,

View File

@ -6,31 +6,6 @@ import ../../../app_service/service/message/dto/message
export types.ContentType export types.ContentType
import message_reaction_model, message_reaction_item, message_transaction_parameters_item import message_reaction_model, message_reaction_item, message_transaction_parameters_item
## An explanation why we keep track of `timestamp` and `localTimestamp` here
##
## introduction of those two actually fixes the following issues:
## https://github.com/status-im/status-desktop/issues/6004
## https://github.com/status-im/status-desktop/issues/7058
##
## We should always refer to `whisperTimestamp` as it is set for a message by the network
## in order they are sent (that solves the issue #6004), but another issue #7058 is happening
## cause `whisperTimestamp` has one second accuracy (which is a very big timeframe for messages).
## That further means that all messsages sent by user A within 1000ms will be received with the
## same `whisperTimestamp` value on the side of user B, in that case to differ the order of
## those message we're using localy set `timestamp` on the sender side which is received unchanged
## on the receiver side.
## Now a question why don't we use only locally set `timestamp` may araise... the answer is...
## because of issue #6004, cause it can be that users A and B send a message in almost the same
## time in that case message sent by user A will be immediatelly added to the message list, while
## message sent by user B will arrive like a less then a second later and in that case user A may
## see user B message before or after his message and the same for user B, depends on local time
## of those 2 users which is set for `timestamp` time in the moment they sent a message.
##
## If we anyhow find a way to have here accutacy higher than 1 second, then we can go on only
## with `whisperTimestamp`
## https://github.com/status-im/status-go/blob/3f987cc565091327f017bfe674c08ed01e301d00/protocol/messenger.go#L3726
type type
Item* = ref object Item* = ref object
id: string id: string
@ -52,7 +27,7 @@ type
gapFrom: int64 gapFrom: int64
gapTo: int64 gapTo: int64
timestamp: int64 timestamp: int64
localTimestamp: int64 clock: int64 # lamport timestamp - used for ordering
contentType: ContentType contentType: ContentType
messageType: int messageType: int
contactRequestState: int contactRequestState: int
@ -83,8 +58,8 @@ proc initItem*(
image: string, image: string,
messageContainsMentions, messageContainsMentions,
seen: bool, seen: bool,
timestamp: int64, # whisper timestamp, with 1s accuracy (even accuracy looks like 1ms, last 3 digits are always 0) timestamp: int64,
localTimestamp: int64, # local timestamp obtained when a message is being sent, with 1ms accuracy clock: int64,
contentType: ContentType, contentType: ContentType,
messageType: int, messageType: int,
contactRequestState: int, contactRequestState: int,
@ -113,7 +88,7 @@ proc initItem*(
result.messageImage = image result.messageImage = image
result.messageContainsMentions = messageContainsMentions result.messageContainsMentions = messageContainsMentions
result.timestamp = timestamp result.timestamp = timestamp
result.localTimestamp = localTimestamp result.clock = clock
result.contentType = contentType result.contentType = contentType
result.messageType = messageType result.messageType = messageType
result.contactRequestState = contactRequestState result.contactRequestState = contactRequestState
@ -165,7 +140,6 @@ proc `$`*(self: Item): string =
messageText:{self.messageText}, messageText:{self.messageText},
messageContainsMentions:{self.messageContainsMentions}, messageContainsMentions:{self.messageContainsMentions},
timestamp:{$self.timestamp}, timestamp:{$self.timestamp},
localTimestamp:{$self.localTimestamp}
contentType:{$self.contentType.int}, contentType:{$self.contentType.int},
messageType:{$self.messageType}, messageType:{$self.messageType},
contactRequestState:{$self.contactRequestState}, contactRequestState:{$self.contactRequestState},
@ -265,8 +239,8 @@ proc seen*(self: Item): bool {.inline.} =
proc timestamp*(self: Item): int64 {.inline.} = proc timestamp*(self: Item): int64 {.inline.} =
self.timestamp self.timestamp
proc localTimestamp*(self: Item): int64 {.inline.} = proc clock*(self: Item): int64 {.inline.} =
self.localTimestamp self.clock
proc contentType*(self: Item): ContentType {.inline.} = proc contentType*(self: Item): ContentType {.inline.} =
self.contentType self.contentType
@ -344,7 +318,7 @@ proc toJsonNode*(self: Item): JsonNode =
"gapFrom": self.gapFrom, "gapFrom": self.gapFrom,
"gapTo": self.gapTo, "gapTo": self.gapTo,
"timestamp": self.timestamp, "timestamp": self.timestamp,
"localTimestamp": self.localTimestamp, "clock": self.clock,
"contentType": self.contentType.int, "contentType": self.contentType.int,
"messageType": self.messageType, "messageType": self.messageType,
"contactRequestState": self.contactRequestState, "contactRequestState": self.contactRequestState,

View File

@ -244,12 +244,12 @@ QtObject:
if(self.items[i].responseToMessageWithId == messageId): if(self.items[i].responseToMessageWithId == messageId):
result.add(self.items[i].id) result.add(self.items[i].id)
proc findIndexBasedOnTimestampToInsertTo(self: Model, timestamp: int64, localTimestamp: int64): int = proc findIndexBasedOnClockToInsertTo(self: Model, clock: int64, id: string): int =
for i in 0 ..< self.items.len: for i in 0 ..< self.items.len:
if timestamp > self.items[i].timestamp: if clock > self.items[i].clock:
return i return i
elif timestamp == self.items[i].timestamp: elif clock == self.items[i].clock: # break ties by message id
if localTimestamp > self.items[i].localTimestamp: if id > self.items[i].id:
return i return i
return 0 return 0
@ -323,14 +323,14 @@ QtObject:
self.updateItemAtIndex(1) self.updateItemAtIndex(1)
self.countChanged() self.countChanged()
proc insertItemBasedOnTimestamp*(self: Model, item: Item) = proc insertItemBasedOnClock*(self: Model, item: Item) =
if(self.findIndexForMessageId(item.id) != -1): if(self.findIndexForMessageId(item.id) != -1):
return return
let parentModelIndex = newQModelIndex() let parentModelIndex = newQModelIndex()
defer: parentModelIndex.delete defer: parentModelIndex.delete
let position = self.findIndexBasedOnTimestampToInsertTo(item.timestamp, item.localTimestamp) let position = self.findIndexBasedOnClockToInsertTo(item.clock, item.id)
self.beginInsertRows(parentModelIndex, position, position) self.beginInsertRows(parentModelIndex, position, position)
self.items.insert(item, position) self.items.insert(item, position)