Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix: Make calc_sort_timestamp() a continuous function of message ti…
…mestamp

This also simplifies the SQL query in `calc_sort_timestamp()` and prepares for creation of a db
index for it so that it's fast. Currently it doesn't uses indexes effectively; if a chat has many
messages, it's slow, i.e. O(n).

This as well fixes ordering of delayed encrypted outgoing messages; before, they could be sorted
above "Messages are end-to-end encrypted."
  • Loading branch information
iequidoo authored and link2xt committed Sep 13, 2025
commit c23ff467b301022ce31e4a0c00311f90c64910aa
60 changes: 37 additions & 23 deletions src/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,10 @@ impl ChatId {
let chat = Chat::load_from_db(context, chat_id).await?;

if chat.is_encrypted(context).await? {
chat_id.add_encrypted_msg(context, timestamp).await?;
let respect_delayed_msgs = true;
chat_id
.add_encrypted_msg(context, timestamp, respect_delayed_msgs)
.await?;
}

info!(
Expand Down Expand Up @@ -459,15 +462,23 @@ impl ChatId {
}

/// Adds message "Messages are end-to-end encrypted".
async fn add_encrypted_msg(self, context: &Context, timestamp_sort: i64) -> Result<()> {
async fn add_encrypted_msg(
self,
context: &Context,
timestamp_sent: i64,
respect_delayed_msgs: bool,
) -> Result<()> {
let text = stock_str::messages_e2e_encrypted(context).await;
add_info_msg_with_cmd(
context,
self,
&text,
SystemMessage::ChatE2ee,
timestamp_sort,
None,
// Create a time window for delayed encrypted messages so that they are sorted under
// "Messages are end-to-end encrypted." This way time still monotonically increases and
// there's no magic "N years ago" which should be adjusted in the future.
timestamp_sent / if respect_delayed_msgs { 2 } else { 1 },
Some(timestamp_sent),
None,
None,
None,
Expand Down Expand Up @@ -1220,37 +1231,35 @@ impl ChatId {
)
.await?
} else if received {
// Received messages shouldn't mingle with just sent ones and appear somewhere in the
// middle of the chat, so we go after the newest non fresh message.
//
// But if a received outgoing message is older than some seen message, better sort the
// received message purely by timestamp. We could place it just before that seen
// message, but anyway the user may not notice it.
// Received incoming messages shouldn't mingle with just sent ones and appear somewhere
// in the middle of the chat, so we go after the newest non fresh message. Received
// outgoing messages are allowed to mingle with seen messages though to avoid seen
// replies appearing before messages sent from another device (cases like the user
// sharing the account with others or bots are rare, so let them break sometimes).
//
// NB: Received outgoing messages may break sorting of fresh incoming ones, but this
// shouldn't happen frequently. Seen incoming messages don't really break sorting of
// fresh ones, they rather mean that older incoming messages are actually seen as well.
context
.sql
.query_row_optional(
"SELECT MAX(timestamp), MAX(IIF(state=?,timestamp_sent,0))
"SELECT MAX(timestamp)
FROM msgs
WHERE chat_id=? AND hidden=0 AND state>?
HAVING COUNT(*) > 0",
(MessageState::InSeen, self, MessageState::InFresh),
(
self,
match incoming {
true => MessageState::InFresh,
false => MessageState::InSeen,
},
),
|row| {
let ts: i64 = row.get(0)?;
let ts_sent_seen: i64 = row.get(1)?;
Ok((ts, ts_sent_seen))
Ok(ts)
},
)
.await?
.and_then(|(ts, ts_sent_seen)| {
match incoming || ts_sent_seen <= message_timestamp {
true => Some(ts),
false => None,
}
})
} else {
None
};
Expand Down Expand Up @@ -2425,7 +2434,10 @@ impl ChatIdBlocked {
&& !chat.param.exists(Param::Devicetalk)
&& !chat.param.exists(Param::Selftalk)
{
chat_id.add_encrypted_msg(context, smeared_time).await?;
let respect_delayed_msgs = true;
chat_id
.add_encrypted_msg(context, smeared_time, respect_delayed_msgs)
.await?;
}

Ok(Self {
Expand Down Expand Up @@ -3487,8 +3499,10 @@ pub(crate) async fn create_group_ex(
chatlist_events::emit_chatlist_item_changed(context, chat_id);

if is_encrypted {
// Add "Messages are end-to-end encrypted." message.
chat_id.add_encrypted_msg(context, timestamp).await?;
let respect_delayed_msgs = false;
chat_id
.add_encrypted_msg(context, timestamp, respect_delayed_msgs)
.await?;
}

if !context.get_config_bool(Config::Bot).await?
Expand Down
18 changes: 15 additions & 3 deletions src/tests/verified_chats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,11 @@ async fn test_degrade_verified_oneonone_chat() -> Result<()> {
/// This test tests that the messages are still in the right order.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_old_message_4() -> Result<()> {
let alice = TestContext::new_alice().await;
let mut tcm = TestContextManager::new();
let alice = &tcm.alice().await;
let bob = &tcm.bob().await;
let msg_incoming = receive_imf(
&alice,
alice,
b"From: Bob <[email protected]>\n\
To: [email protected]\n\
Message-ID: <[email protected]>\n\
Expand All @@ -232,7 +234,7 @@ async fn test_old_message_4() -> Result<()> {
.unwrap();

let msg_sent = receive_imf(
&alice,
alice,
b"From: [email protected]\n\
To: Bob <[email protected]>\n\
Message-ID: <[email protected]>\n\
Expand All @@ -247,6 +249,16 @@ async fn test_old_message_4() -> Result<()> {
// The "Happy birthday" message should be shown first, and then the "Thanks" message
assert!(msg_sent.sort_timestamp < msg_incoming.sort_timestamp);

// And now the same for encrypted messages.
let msg_incoming = tcm.send_recv(bob, alice, "Thanks, Alice!").await;
message::markseen_msgs(alice, vec![msg_incoming.id]).await?;
let raw = include_bytes!("../../test-data/message/thunderbird_with_autocrypt.eml");
let msg_sent = receive_imf(alice, raw, true).await?.unwrap();
assert_eq!(msg_sent.chat_id, msg_incoming.chat_id);
assert!(msg_sent.sort_timestamp < msg_incoming.timestamp_sort);
alice
.golden_test_chat(msg_sent.chat_id, "test_old_message_4")
.await;
Ok(())
}

Expand Down
6 changes: 6 additions & 0 deletions test-data/golden/test_old_message_4
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Single#Chat#11: [email protected] [KEY [email protected]]
--------------------------------------------------------------------------------
Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO]
Msg#14🔒: Me (Contact#Contact#Self): Test – This is encrypted, signed, and has an Autocrypt Header without prefer-encrypt=mutual. √
Msg#13🔒: (Contact#Contact#11): Thanks, Alice! [SEEN]
--------------------------------------------------------------------------------
2 changes: 1 addition & 1 deletion test-data/golden/two_group_securejoins
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
Group#Chat#11: Group [3 member(s)]
--------------------------------------------------------------------------------
Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO]
Msg#13: info (Contact#Contact#Info): [email protected] invited you to join this group.

Waiting for the device of [email protected] to reply… [NOTICED][INFO]
Msg#15: info (Contact#Contact#Info): [email protected] replied, waiting for being added to the group… [NOTICED][INFO]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reply is already encrypted, so now the message order looks more correct as to me. If an old delayed message arrives, even if it's outgoing, e.g. from another device, it also will be sorted correctly

Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO]
Msg#17🔒: (Contact#Contact#10): Member Me added by [email protected]. [FRESH][INFO]
--------------------------------------------------------------------------------