Queue messages for SMTP before encryption by link2xt · Pull Request #8619 · chatmail/core

link2xt · 2026-08-22T13:45:16Z

Part of #8607, this removes the code that deletes the whole queue when the sending relay is changed, and makes queueing the mail independent of the sending address. Follow-up to #8345 in some way, this now uses QueuedMail for real.

There is a python test that was previously failing, deleting the current transport while having messages queued.

Large number of lines added is because of better types (new QueuedEncryption separate from mimefactory::Encryption) and duplicated smtp2 table with documentation.

Left for follow-up PRs:

Store side effects and the mail itself in a single transaction in chat::enqueue_mail. This needs refactoring location streaming code etc. that is unrelated.
Moving QueuedMail, QueuedEncryption and related functions from mimefactory to a separate module.
Cycling through transports in SMTP loop if connection to the first one fails and connecting to the last used one instead of ConfiguredAddr.

Note re performance of encryption: maybe it even makes UI more responsive because the message bubble can appear on the sending side faster as the message does not need to be encrypted yet.

Commit message

Headers like From and Autocrypt are now added late, right before sending the message over SMTP. This way we advertise the latest list of transports and use the correct From address in the encrypted part even for messages queued while being offline.

BCC-self recipients are also added late.
For unencrypted messages we only want to send a copy
to the sending address, but we don't know the sending address
when queueing the message.
Adding bcc-self recipients when dequeuing the message
also makes it possible to send copies to updated list of relays.

j-g00da · 2026-09-10T10:40:53Z

+) -> Result<(
+    Option<(QueuedMail, Option<QueueSideEffects>)>,
+    (QueuedMail, Option<QueueSideEffects>),
+)> {


nit: maybe (QueuedMail, Option<QueueSideEffects>) is worth an alias?

hpk42

Overall i think the PR is larger than it needs to be.

Conceptually there is one main issue IMO: For BCC-self-copies this PR re-resolves current relay list, but not for the recipient addresses. In bad network/partially-offline situations messages can be in the queue for long enough that a keyupdate arrived. Logically, recipient fingerprints should be derived to addresses at sending time, and not fixated at queuing time. At send-time we anyway read the public keys, so it wouldn't cause more IO. Messages then either have fingerprints as recipients, or addresses but never both at the same time. After sending to addresses in a chunk succeeded, they could be written to a new sent_finished column, so that the next address-derivation can skip them.

hpk42 · 2026-09-10T11:41:24Z

            // eg. are just freetext and/or do not follow any standard.
            headers.push((
                "Chat-Disposition-Notification-To",
                mail_builder::headers::raw::Raw::new(self.from_addr.clone()).into(),


This would need to be re-rendered on transport change as well but i instead suggest to relax the check in https://github.com/chatmail/core/blob/link2xt/late-encryption/src/mimeparser.rs#L955 and treat the header existence as a boolean, or compare to all relay addresses.
There also is another use of self.from_addr for the Sender header but i think it's not critical on the receiver side. Ideally there would be no self.from_addr left at all.

Opened #8681 now.
For Chat-Disposition-Notification-To and Sender, i have edited them in the first message #8572 while doing #8645 which is also split out of this PR in attempt to get rid of from_addr.

Sender is essentially a boolean flag saying "this is a bridge bot, show display name with ~ and save it in the message".

hpk42 · 2026-09-10T12:39:12Z

+            migration_version,
+        )
+        .await?;
+    }


Doesn't this orphan currently queued messages in the existing smtp table? At least they should be marked failed i think.

hpk42 · 2026-09-10T13:04:00Z

    };
+    let mut recipients = queued_mail.recipients.clone();
+    if queued_mail.bcc_self {
+        add_self_recipients(


this adds the self-recipients on every retry without dedup. After a partially sent chunked message, the remainder written back to the db further down already contains them.

I added sent_to column now.

hpk42 · 2026-09-10T14:14:07Z

+-- Replaced with smtp2.
+CREATE TABLE smtp (
+  id INTEGER PRIMARY KEY AUTOINCREMENT,
+  rfc724_mid TEXT NOT NULL,          -- Message-ID
+  mime TEXT NOT NULL,                -- SMTP payload
+  msg_id INTEGER NOT NULL,           -- ID of the message in `msgs` table
+  recipients TEXT NOT NULL,          -- List of recipients separated by space
+  retries INTEGER NOT NULL DEFAULT 0 -- Number of failed attempts to send the message
+);


why move this table and increase the diff?

All unused tables are at the end currently, below the -- Deprecated and unused tables. comment.

hpk42 · 2026-09-10T14:29:07Z

 }

+#[derive(Debug, Clone)]
+pub(crate) enum QueuedEncryption {


I don't think this new enum is much "better" as you say in the PR description. It's only about dropping addresses but e.g. keyupdates just used empty strings on main and render_queued_mail dropped the address unconditionally. So overall, it's more diff and more LOCs without much gain IMO.

I'm saying "better types" as in "better typed", i.e. having less invalid states. Or e.g. wrapping all Strings into newtypes or turning all booleans into enums with two variants is "better typed", does not necessarily mean it cannot be an overkill.

As for the usefulness of this, I initially implemented serialization of pairs of addresses and fingerprints, both saving to SQL and loading, only to later realize that the addresses are never used. Not having addresses in the type would have made it impossible.

I want to move QueuedMail and QueuedEncryption to a separate module out of mimefactory and keep it pub(crate). I did not do it in this PR because I don't want to move QueuedMail and make the diff more difficult to read, but otherwise i think it belongs more to SMTP level than to MIME level. mimefactory::Encryption is a private type now that is an implementation detail of mimefactory, it is never serialized and i think may even be removed/reduced if mimefactory is made more functional and not a large object that is mutating itself in multiple steps and is immediately rendered afterwards. Keeping mimefactory::Encryption public just to reuse the type in SMTP code and having less LOCs will make changing mimefactory harder and also will require understanding why there are always empty addresses next to keys for anyone who changes SMTP code.

hpk42 · 2026-09-10T14:40:25Z

-    let trans_fn = |t: &mut rusqlite::Transaction| {
-        let mut row_ids = Vec::<i64>::new();


Why did the transaction have to go? Removing it here seems not neccessary, and produces follow-up Todos.

hpk42 · 2026-09-10T14:44:12Z

+        let msg_id = message::insert_tombstone(context, &rfc724_mid).await?;
        let keys = chunk.iter().map(|r| r.public_key.clone()).collect();
-        let rendered_message = render_keyupdate_message(context, &rfc724_mid, keys).await?;
-        insert_into_smtp(context, &rfc724_mid, &envelope, rendered_message).await?;


why not let insert_into_smtp take a QueuedMail instead of the rendered string? Removing the helper and inlining it three times in different ways seems unnecessary.

Restored insert_into_smtp. It was lost to rebase because PR started when insert_into_smtp did not exist. It is now 3 lines long and has no SQL statement, so it just calls time(), creates a tombstone and then calls enqueue_mail. In two cases rfc724_mid could be created inside, but not in handle_securejoin_handshake, so no more common code can be moved inside.

It was incorrectly converting unused row ID to MsgId type and selecting already known msg_id.

This is similar to mimefactory::Encryption, but does not have email addresses for asymmetrically encrypted messages. Queued messages don't need email addresses for public keys. Addresses are only needed to render Autocrypt-Gossip headers.

Headers like From and Autocrypt are now added late, right before sending the message over SMTP. This way we advertise the latest list of transports and use the correct From address in the encrypted part even for messages queued while being offline. BCC-self recipients are also added late. For unencrypted messages we only want to send a copy to the sending address, but we don't know the sending address when queueing the message. Adding bcc-self recipients when dequeuing the message also makes it possible to send copies to updated list of relays.

link2xt force-pushed the link2xt/late-encryption branch 2 times, most recently from 026e72d to ce56911 Compare August 25, 2026 17:11

link2xt changed the base branch from main to link2xt/create_send_msg_jobs-row_id August 25, 2026 17:14

link2xt force-pushed the link2xt/late-encryption branch 2 times, most recently from d688b11 to 6809578 Compare August 25, 2026 20:23

link2xt force-pushed the link2xt/create_send_msg_jobs-row_id branch from e657a72 to a67a5b0 Compare August 26, 2026 14:36

Base automatically changed from link2xt/create_send_msg_jobs-row_id to main August 26, 2026 20:22

link2xt mentioned this pull request Aug 27, 2026

introduce asymetrically encrypted keyupdate messages informing contacts about relay changes #8621

Merged

link2xt force-pushed the link2xt/late-encryption branch 3 times, most recently from 756d28c to 3d72fe8 Compare September 1, 2026 17:25

link2xt mentioned this pull request Sep 1, 2026

Remove Original-Recipient and Final-Recipient fields from MDNs #8645

Merged

link2xt force-pushed the link2xt/late-encryption branch 8 times, most recently from b838513 to bdfd0d4 Compare September 9, 2026 05:43

link2xt changed the base branch from main to link2xt/refactor-ref-vec September 9, 2026 06:13

link2xt added this pull request to stack #8677 September 9, 2026 06:13

link2xt force-pushed the link2xt/late-encryption branch 2 times, most recently from 958c526 to fe41960 Compare September 9, 2026 07:55

Base automatically changed from link2xt/refactor-ref-vec to main September 9, 2026 08:34

link2xt force-pushed the link2xt/late-encryption branch 2 times, most recently from 8a42d9b to b9bba12 Compare September 9, 2026 08:37

link2xt removed this pull request from stack #8677 September 9, 2026 08:38

link2xt force-pushed the link2xt/late-encryption branch 2 times, most recently from 3e8fdaa to eb1c34c Compare September 9, 2026 10:11

link2xt force-pushed the link2xt/late-encryption branch from eb1c34c to 7eb324f Compare September 9, 2026 10:26

link2xt changed the title ~~WIP: Late encryption~~ Queue messages for SMTP before encryption Sep 9, 2026

link2xt marked this pull request as ready for review September 9, 2026 10:27

link2xt requested review from Hocuri and j-g00da September 10, 2026 09:02

j-g00da reviewed Sep 10, 2026

View reviewed changes

hpk42 reviewed Sep 10, 2026

View reviewed changes

link2xt added 2 commits September 10, 2026 21:52

test: cleanup get_smtp_rows_for_msg()

71972e8

It was incorrectly converting unused row ID to MsgId type and selecting already known msg_id.

refactor: add Encryption.is_encrypted()

6c25d8d

link2xt force-pushed the link2xt/late-encryption branch from 7eb324f to 84dd1f4 Compare September 11, 2026 02:51

link2xt added 4 commits September 11, 2026 03:26

refactor: separate QueuedEncryption

07bc5a4

This is similar to mimefactory::Encryption, but does not have email addresses for asymmetrically encrypted messages. Queued messages don't need email addresses for public keys. Addresses are only needed to render Autocrypt-Gossip headers.

fixup: address comments

34c7ca6

add smtp2.sent_to

8fded29

link2xt force-pushed the link2xt/late-encryption branch from 84dd1f4 to 8fded29 Compare September 11, 2026 03:26

link2xt added 3 commits September 11, 2026 03:55

docs fixup

33f1a77

render_queued_mail_with_context()

ad946de

restore insert_into_smtp

9726613

link2xt force-pushed the link2xt/late-encryption branch from f69e443 to 9726613 Compare September 11, 2026 14:11

		let trans_fn = \|t: &mut rusqlite::Transaction\| {
		let mut row_ids = Vec::<i64>::new();

Uh oh!

Uh oh!

Conversation

link2xt commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hpk42 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

link2xt Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

link2xt Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

link2xt commented Aug 22, 2026 •

edited

Loading

link2xt Sep 11, 2026 •

edited

Loading

link2xt Sep 11, 2026 •

edited

Loading