Add some basic tests

In its stead, remove `recode_avatar` high-level migration. It was added
5 years ago, and even if someone updates Delta Chat after such a long
time, large avatars are not a problem anymore since we don't send them
in unencrypted messages.

.github/workflows/ci.yml

@@ -20,7 +20,7 @@ permissions: {}
 
 env:
   RUSTFLAGS: -Dwarnings
-  RUST_VERSION: 1.98.1
+  RUST_VERSION: 1.99.0
 
   # Minimum Supported Rust Version
   MSRV: 1.89.0
@@ -30,6 +30,9 @@ jobs:
     name: Lint Rust
     runs-on: ubuntu-latest
     timeout-minutes: 60
+    env:
+      # Tests always unwind: match it so dependencies are only checked once.
+      CARGO_PROFILE_DEV_PANIC: unwind
     steps:
       - uses: actions/checkout@v7
         with:
@@ -48,8 +51,6 @@ jobs:
         run: cargo fmt --all -- --check
       - name: Run clippy
         run: scripts/clippy.sh
-      - name: Check with all features
-        run: cargo check --workspace --all-targets --all-features
       - name: Check with only default features
         run: cargo check --all-targets
 
@@ -139,12 +140,12 @@ jobs:
       - name: Tests
         env:
           RUST_BACKTRACE: 1
-        run: cargo nextest run --workspace --locked
+        run: cargo nextest run --workspace --exclude deltachat-jsonrpc-bindings --locked
 
       - name: Doc-Tests
         env:
           RUST_BACKTRACE: 1
-        run: cargo test --workspace --locked --doc
+        run: cargo test --workspace --exclude deltachat-jsonrpc-bindings --locked --doc
 
       - name: Test cargo vendor
         run: cargo vendor

Cargo.lock

@@ -194,9 +194,9 @@ dependencies = [
 
 [[package]]
 name = "astral-tokio-tar"
-version = "0.6.4"
+version = "0.7.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b18457efd137254e016bbde5e1d88df61c4e1a5ae2223746e56123bac6af2463"
+checksum = "6f2e989b33246fe9240d39accf4dd9a01e0b6c1f3ce9dd095e0a47fa02505523"
 dependencies = [
  "futures-core",
  "libc",
@@ -205,6 +205,7 @@ dependencies = [
  "rustix 1.1.4",
  "tokio",
  "tokio-stream",
+ "zerocopy 0.8.59",
 ]
 
 [[package]]
@@ -1405,7 +1406,6 @@ dependencies = [
  "tokio-stream",
  "tokio-util",
  "toml",
- "tracing",
  "url",
  "uuid",
  "walkdir",
@@ -4246,9 +4246,9 @@ dependencies = [
 
 [[package]]
 name = "pgp"
-version = "0.20.0"
+version = "0.21.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1cfa4743b28656065ff4c0ba09e46b357a65e8c00fc2341e89084b82f87cbdf1"
+checksum = "ae70f4d9325a391db30d115d6191a7bb67cec856aa0bba131290f0c4e09a532a"
 dependencies = [
  "aead",
  "aes",
@@ -4595,7 +4595,7 @@ version = "0.2.20"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "77957b295656769bb8ad2b6a6b09d897d94f05c41b069aede1fcdaa675eaea04"
 dependencies = [
- "zerocopy",
+ "zerocopy 0.7.35",
 ]
 
 [[package]]
@@ -5812,7 +5812,7 @@ dependencies = [
  "sha3",
  "signature",
  "typenum",
- "zerocopy",
+ "zerocopy 0.7.35",
 ]
 
 [[package]]
@@ -7582,7 +7582,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "1b9b4fd18abc82b8136838da5d50bae7bdea537c574d8dc1a34ed098d6c166f0"
 dependencies = [
  "byteorder",
- "zerocopy-derive",
+ "zerocopy-derive 0.7.35",
+]
+
+[[package]]
+name = "zerocopy"
+version = "0.8.59"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6df92bf3d9227be3d53173901ddbffac2babc27ae50f397776ffd6dc33f800cb"
+dependencies = [
+ "zerocopy-derive 0.8.59",
 ]
 
 [[package]]
@@ -7596,6 +7605,17 @@ dependencies = [
  "syn 2.0.118",
 ]
 
+[[package]]
+name = "zerocopy-derive"
+version = "0.8.59"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ac4f328cf2f05d084e496c3e9c3f33ed0a183656a16e1fcec4d464d8373aec82"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn 2.0.118",
+]
+
 [[package]]
 name = "zerofrom"
 version = "0.1.5"

Cargo.toml

@@ -77,7 +77,7 @@ num-derive = "0.4"
 num-traits = { workspace = true }
 parking_lot = "0.12.4"
 percent-encoding = "2.3"
-pgp = { version = "0.20.0", features = ["draft-pqc"], default-features = false }
+pgp = { version = "0.21.0", features = ["pqc"], default-features = false }
 pin-project = "1"
 qrcodegen = "1.7.0"
 quick-xml = { version = "0.41", features = ["escape-html"] }
@@ -100,11 +100,10 @@ thiserror = { workspace = true }
 tokio-io-timeout = "1.2.1"
 tokio-rustls = { version = "0.26.2", default-features = false, features = ["tls12", "brotli"] }
 tokio-stream = { version = "0.1.17", features = ["fs"] }
-astral-tokio-tar = { version = "0.6.3", default-features = false }
+astral-tokio-tar = { version = "0.7.0", default-features = false }
 tokio-util = { workspace = true }
 tokio = { workspace = true, features = ["fs", "rt-multi-thread", "macros"] }
 toml = "0.9"
-tracing = "0.1.41"
 url = "2"
 uuid = { version = "1", features = ["serde", "v4"] }
 walkdir = "2.5.0"

deltachat-ffi/deltachat.h

@@ -3543,28 +3543,6 @@ uint32_t         dc_chatlist_get_msg_id      (const dc_chatlist_t* chatlist, siz
 dc_lot_t*        dc_chatlist_get_summary     (const dc_chatlist_t* chatlist, size_t index, dc_chat_t* chat);
 
 
-/**
- * Create a chatlist summary item when the chatlist object is already unref()'d.
- *
- * This function is similar to dc_chatlist_get_summary(), however,
- * it takes the chat ID and the message ID as returned by dc_chatlist_get_chat_id() and dc_chatlist_get_msg_id()
- * as arguments. The chatlist object itself is not needed directly.
- *
- * This maybe useful if you convert the complete object into a different representation
- * as done e.g. in the node-bindings.
- * If you have access to the chatlist object in some way, using this function is not recommended,
- * use dc_chatlist_get_summary() in this case instead.
- *
- * @memberof dc_context_t
- * @param context The context object.
- * @param chat_id The chat ID to get a summary for.
- * @param msg_id The message ID to get a summary for.
- * @return The summary as an dc_lot_t object, see dc_chatlist_get_summary() for details.
- *     Must be freed using dc_lot_unref(). NULL is never returned.
- */
-dc_lot_t*        dc_chatlist_get_summary2    (dc_context_t* context, uint32_t chat_id, uint32_t msg_id);
-
-
 /**
  * Get info summary for a chat, in JSON format.
  *

deltachat-ffi/src/lib.rs

@@ -47,7 +47,6 @@ mod dc_array;
 mod lot;
 
 mod string;
-use deltachat::chatlist::Chatlist;
 
 use self::string::*;
 
@@ -2844,34 +2843,6 @@ pub unsafe extern "C" fn dc_chatlist_get_summary(
     Box::into_raw(Box::new(summary.into()))
 }
 
-#[unsafe(no_mangle)]
-pub unsafe extern "C" fn dc_chatlist_get_summary2(
-    context: *mut dc_context_t,
-    chat_id: u32,
-    msg_id: u32,
-) -> *mut dc_lot_t {
-    if context.is_null() {
-        eprintln!("ignoring careless call to dc_chatlist_get_summary2()");
-        return ptr::null_mut();
-    }
-    let ctx = unsafe { &*context };
-    let msg_id = if msg_id == 0 {
-        None
-    } else {
-        Some(MsgId::new(msg_id))
-    };
-    let summary = block_on(Chatlist::get_summary2(
-        ctx,
-        ChatId::new(chat_id),
-        msg_id,
-        None,
-    ))
-    .context("get_summary2 failed")
-    .log_err(ctx)
-    .unwrap_or_default();
-    Box::into_raw(Box::new(summary.into()))
-}
-
 // dc_chat_t
 
 /// FFI struct for [dc_chat_t]

deltachat-jsonrpc/src/api/types/chat_list.rs

@@ -31,8 +31,6 @@ pub enum ChatListItemFetchResult {
         summary_text1: String,
         summary_text2: String,
         summary_status: u32,
-        /// showing preview if last chat message is image
-        summary_preview_image: Option<String>,
 
         /// True if the chat is encrypted.
         /// This means that all messages in the chat are encrypted,
@@ -103,8 +101,6 @@ pub(crate) async fn get_chat_list_item_by_id(
     let summary_text1 = summary.prefix.map_or_else(String::new, |s| s.to_string());
     let summary_text2 = summary.text.to_owned();
 
-    let summary_preview_image = summary.thumbnail_path;
-
     let visibility = chat.get_visibility();
 
     let avatar_path = chat
@@ -157,7 +153,6 @@ pub(crate) async fn get_chat_list_item_by_id(
         summary_text1,
         summary_text2,
         summary_status: summary.state.to_u32().expect("impossible"), // idea and a function to transform the constant to strings? or return string enum
-        summary_preview_image,
         is_encrypted: chat.is_encrypted(ctx).await?,
         is_group: chat.get_type() == Chattype::Group,
         fresh_message_counter,

deltachat-rpc-client/src/deltachat_rpc_client/rpc.py

@@ -149,7 +149,10 @@ class Rpc:
     def close(self) -> None:
         """Terminate RPC server process and wait until the reader loop finishes."""
         self.closing = True
-        self.stop_io_for_all_accounts()
+        # JSON-RPC error may happen if RPC server process has crashed already.
+        # We still want to shutdown all threads in this case.
+        with contextlib.suppress(JsonRpcError):
+            self.stop_io_for_all_accounts()
         # Let `events_loop` stop cleanly on `closing` before the pipe goes away,
         # otherwise it might exit through an "RPC server closed" error instead.
         self.events_thread.join()

deltachat-rpc-client/tests/test_something.py

@@ -35,6 +35,19 @@ def test_sleep(rpc) -> None:
         assert sleep_5_future in pending
 
 
+def test_shutdown(rpc) -> None:
+    """Test RPC client shutdown if RPC server process is terminated.
+
+    This is a regression test, at the time of adding it RPC client
+    did not terminate all threads and python process did not exit.
+    Shutting down all threads is tested implicitly by pytest-timeout.
+    """
+    rpc.process.kill()
+    rpc.process.wait()
+    with pytest.raises(JsonRpcError):
+        rpc.get_system_info()
+
+
 def test_email_address_validity(rpc) -> None:
     valid_addresses = [
         "email@example.com",

deny.toml

@@ -113,6 +113,8 @@ skip = [
      { name = "windows_x86_64_gnu" },
      { name = "windows_x86_64_gnullvm" },
      { name = "windows_x86_64_msvc" },
+     { name = "zerocopy-derive", version = "0.7.35" },
+     { name = "zerocopy", version = "0.7.35" },
 ]
 
 

docs/schema.sql

@@ -456,6 +456,21 @@ CREATE TABLE smtp_status_updates (
     descr TEXT NOT NULL -- text to send along with the updates
 );
 
+-- Table to record the successful usage transports for sending.
+-- Sorting the table by rowid in descending order
+-- returns most recently successfully used transport first.
+CREATE TABLE smtp_success (
+    -- Sequentially increasing ID of the success.
+    -- Transport with the highest ID is to be used first.
+    id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
+
+    -- ID of the transport that was used to send a message.
+    transport_id INTEGER UNIQUE NOT NULL,
+
+    -- Delete `smtp_success` rows when the transport is deleted.
+    FOREIGN KEY(transport_id) REFERENCES transports(id) ON DELETE CASCADE
+) STRICT;
+
 -- Table of "sync items" to be grouped into sync messages
 -- and sent to own devices.
 CREATE TABLE multi_device_sync (

scripts/clippy.sh

@@ -6,4 +6,4 @@
 #
 # To automatically fix warnings, run
 #   scripts/clippy.sh --fix --allow-dirty
-cargo clippy --locked --workspace --all-targets --all-features "$@" -- -D warnings
+cargo clippy --locked --workspace --exclude deltachat-jsonrpc-bindings --all-targets --all-features "$@" -- -D warnings

src/accounts.rs

@@ -76,12 +76,8 @@ impl Accounts {
         Accounts::open(events, dir, writable).await
     }
 
-    /// Get the ID used to log events.
-    ///
-    /// Account manager logs events with ID 0
-    /// which is not used by any accounts.
-    fn get_id(&self) -> u32 {
-        0
+    fn log_info(&self, file: &str, line: u32, msg: String) {
+        self.emit_event(EventType::Info(format!("{file}:{line}: {msg}")));
     }
 
     /// Ensures the accounts directory and config file exist.
@@ -395,11 +391,6 @@ impl Accounts {
                 "Starting background fetch for {n_accounts} accounts."
             )),
         });
-        ::tracing::event!(
-            ::tracing::Level::INFO,
-            account_id = 0,
-            "Starting background fetch for {n_accounts} accounts."
-        );
         let mut set = JoinSet::new();
         for account in accounts {
             set.spawn(async move {
@@ -415,11 +406,6 @@ impl Accounts {
                 "Finished background fetch for {n_accounts} accounts."
             )),
         });
-        ::tracing::event!(
-            ::tracing::Level::INFO,
-            account_id = 0,
-            "Finished background fetch for {n_accounts} accounts."
-        );
     }
 
     /// Auxiliary function for [Accounts::background_fetch].
@@ -462,11 +448,6 @@ impl Accounts {
                 id: 0,
                 typ: EventType::Warning("Background fetch timed out.".to_string()),
             });
-            ::tracing::event!(
-                ::tracing::Level::WARN,
-                account_id = 0,
-                "Background fetch timed out."
-            );
         }
         events.emit(Event {
             id: 0,
@@ -549,7 +530,7 @@ impl Accounts {
         }
     }
 
-    /// Emits a single event.
+    /// Emits a single event with ID 0, which is not used by any accounts.
     pub fn emit_event(&self, event: EventType) {
         self.events.emit(Event { id: 0, typ: event })
     }

src/log.rs

@@ -3,6 +3,7 @@
 #![allow(missing_docs)]
 
 use crate::context::Context;
+use crate::events::EventType;
 
 mod stream;
 
@@ -12,15 +13,9 @@ macro_rules! info {
     ($ctx:expr,  $msg:expr) => {
         info!($ctx, $msg,)
     };
-    ($ctx:expr, $msg:expr, $($args:expr),* $(,)?) => {{
-        let formatted = format!($msg, $($args),*);
-        let full = format!("{file}:{line}: {msg}",
-                           file = file!(),
-                           line = line!(),
-                           msg = &formatted);
-        ::tracing::event!(::tracing::Level::INFO, account_id = $ctx.get_id(), "{}", &formatted);
-        $ctx.emit_event($crate::EventType::Info(full));
-    }};
+    ($ctx:expr, $msg:expr, $($args:expr),* $(,)?) => {
+        $ctx.log_info(file!(), line!(), format!($msg, $($args),*))
+    };
 }
 
 // Workaround for <https://github.com/rust-lang/rust/issues/133708>.
@@ -30,15 +25,9 @@ mod warn_macro_mod {
         ($ctx:expr, $msg:expr) => {
             warn_macro!($ctx, $msg,)
         };
-        ($ctx:expr, $msg:expr, $($args:expr),* $(,)?) => {{
-            let formatted = format!($msg, $($args),*);
-            let full = format!("{file}:{line}: {msg}",
-                               file = file!(),
-                               line = line!(),
-                               msg = &formatted);
-            ::tracing::event!(::tracing::Level::WARN, account_id = $ctx.get_id(), "{}", &formatted);
-            $ctx.emit_event($crate::EventType::Warning(full));
-        }};
+        ($ctx:expr, $msg:expr, $($args:expr),* $(,)?) => {
+            $ctx.log_warn(file!(), line!(), format!($msg, $($args),*))
+        };
     }
 
     pub(crate) use warn_macro;
@@ -50,15 +39,25 @@ macro_rules! error {
     ($ctx:expr, $msg:expr) => {
         error!($ctx, $msg,)
     };
-    ($ctx:expr, $msg:expr, $($args:expr),* $(,)?) => {{
-        let formatted = format!($msg, $($args),*);
-        ::tracing::event!(::tracing::Level::ERROR, account_id = $ctx.get_id(), "{}", &formatted);
-        $ctx.set_last_error(&formatted);
-        $ctx.emit_event($crate::EventType::Error(formatted));
-    }};
+    ($ctx:expr, $msg:expr, $($args:expr),* $(,)?) => {
+        $ctx.log_error(format!($msg, $($args),*))
+    };
 }
 
 impl Context {
+    pub(crate) fn log_info(&self, file: &str, line: u32, msg: String) {
+        self.emit_event(EventType::Info(format!("{file}:{line}: {msg}")));
+    }
+
+    pub(crate) fn log_warn(&self, file: &str, line: u32, msg: String) {
+        self.emit_event(EventType::Warning(format!("{file}:{line}: {msg}")));
+    }
+
+    pub(crate) fn log_error(&self, msg: String) {
+        self.set_last_error(&msg);
+        self.emit_event(EventType::Error(msg));
+    }
+
     /// Set last error string.
     /// Implemented as blocking as used from macros in different, not always async blocks.
     pub fn set_last_error(&self, error: &str) {
@@ -116,12 +115,6 @@ impl<T, E: std::fmt::Display> LogExt<T, E> for Result<T, E> {
             );
             // We can't use the warn!() macro here as the file!() and line!() macros
             // don't work with #[track_caller]
-            tracing::event!(
-                ::tracing::Level::WARN,
-                account_id = context.get_id(),
-                "{}",
-                &full
-            );
             context.emit_event(crate::EventType::Warning(full));
         };
         self

src/log/stream.rs

@@ -93,11 +93,6 @@ impl<S: SessionStream> AsyncRead for LoggingStream<S> {
                 "Read error on stream {peer_addr:?} after reading {} and writing {} bytes: {err}.",
                 this.metrics.total_read, this.metrics.total_written
             );
-            tracing::event!(
-                ::tracing::Level::WARN,
-                account_id = *this.account_id,
-                log_message
-            );
             this.events.emit(Event {
                 id: *this.account_id,
                 typ: EventType::Warning(log_message),

src/reaction.rs

@@ -1032,7 +1032,6 @@ Content-Disposition: reaction\n\
         assert_eq!(summary.timestamp, bob_msg1.get_timestamp()); // time refers to message, not to reaction
         assert_eq!(summary.state, MessageState::InFresh); // state refers to message, not to reaction
         assert!(summary.prefix.is_none());
-        assert!(summary.thumbnail_path.is_none());
         assert_summary(&alice, "BOB reacted 👍 to \"Party?\"").await;
 
         // Alice reacts to own message as well

src/smtp.rs

@@ -54,6 +54,39 @@ pub(crate) struct Smtp {
     pub(crate) last_send_error: Option<String>,
 }
 
+/// Returns transports with their IDs in the order in which they should be tried.
+async fn sorted_transports(context: &Context) -> Result<Vec<(u32, ConfiguredLoginParam)>> {
+    context
+        .sql
+        .query_map_vec(
+            "SELECT transports.id, configured_param FROM transports
+             LEFT JOIN smtp_success ON smtp_success.transport_id=transports.id
+             ORDER BY IFNULL(smtp_success.id, 0) DESC, transports.id ASC",
+            (),
+            |row| {
+                let id: u32 = row.get(0)?;
+                let json: String = row.get(1)?;
+                let param = ConfiguredLoginParam::from_json(&json)?;
+                Ok((id, param))
+            },
+        )
+        .await
+}
+
+/// Records successful use of SMTP transport so it is tried first next time we connect to SMTP.
+async fn record_success(context: &Context, transport_id: u32) -> Result<()> {
+    // INSERT OR REPLACE essentially replaces rowid of the row
+    // if the row exists already, so it becomes the highest rowid in the table.
+    context
+        .sql
+        .execute(
+            "INSERT OR REPLACE INTO smtp_success (transport_id) VALUES (?)",
+            (transport_id,),
+        )
+        .await?;
+    Ok(())
+}
+
 impl Smtp {
     /// Create a new Smtp instances.
     pub fn new() -> Self {
@@ -68,6 +101,8 @@ impl Smtp {
             // separate task to avoid waiting for reply or timeout.
             task::spawn(async move { transport.quit().await });
         }
+        self.transport_id = None;
+        self.from = None;
         self.last_success = None;
     }
 
@@ -99,13 +134,7 @@ impl Smtp {
 
         self.connectivity.set_connecting(context);
         let proxy_config = ProxyConfig::load(context).await?;
-        let transports = ConfiguredLoginParam::load_all(context).await?;
-
-        // Try to connect to the newest transport first. If sending is unreliable,
-        // user can configure a new transport and it will be the one used.
-        // Conversely, if user just added a new transport and sending got less reliable,
-        // user can restore old state by removing the just added transport.
-        for (transport_id, lp) in transports.into_iter().rev() {
+        for (transport_id, lp) in sorted_transports(context).await? {
             info!(context, "Trying to connect to transport {transport_id}.");
             match self
                 .connect(
@@ -325,6 +354,18 @@ pub(crate) async fn smtp_send(
         Ok(()) => SendResult::Success,
     };
 
+    if matches!(status, SendResult::Success) {
+        debug_assert!(smtp.transport_id.is_some());
+        if let Some(transport_id) = smtp.transport_id
+            && let Err(err) = record_success(context, transport_id).await
+        {
+            warn!(
+                context,
+                "Failed to record successful use of transport {transport_id} in smtp_success table: {err:#}."
+            );
+        }
+    }
+
     if let SendResult::Failure(err) = &status
         && let Some(msg_id) = msg_id
     {
@@ -856,3 +897,6 @@ pub(crate) async fn add_self_recipients(
 
     Ok(())
 }
+
+#[cfg(test)]
+mod smtp_tests;

src/smtp/smtp_tests.rs

@@ -0,0 +1,49 @@
+use anyhow::Result;
+
+use crate::test_utils::TestContextManager;
+use crate::transport;
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_smtp_candidates() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let t = &tcm.unconfigured().await;
+
+    transport::add_pseudo_transport(t, "foo@example.net").await?;
+    transport::add_pseudo_transport(t, "bar@example.net").await?;
+    transport::add_pseudo_transport(t, "baz@example.net").await?;
+
+    let transports = super::sorted_transports(t).await?;
+    let [
+        (transport_id1, ref transport1),
+        (transport_id2, ref transport2),
+        (transport_id3, ref transport3),
+    ] = transports[..]
+    else {
+        panic!("Unexpected number of transports");
+    };
+
+    // By default first added transport is used first.
+    assert_eq!(transport1.addr, "foo@example.net");
+    assert_eq!(transport2.addr, "bar@example.net");
+    assert_eq!(transport3.addr, "baz@example.net");
+
+    super::record_success(t, transport_id3).await?;
+    let transports2 = super::sorted_transports(t).await?;
+    assert_eq!(transports2[0].0, transport_id3);
+    assert_eq!(transports2[1].0, transport_id1);
+    assert_eq!(transports2[2].0, transport_id2);
+
+    super::record_success(t, transport_id2).await?;
+    let transports3 = super::sorted_transports(t).await?;
+    assert_eq!(transports3[0].0, transport_id2);
+    assert_eq!(transports3[1].0, transport_id3);
+    assert_eq!(transports3[2].0, transport_id1);
+
+    super::record_success(t, transport_id3).await?;
+    let transports4 = super::sorted_transports(t).await?;
+    assert_eq!(transports4[0].0, transport_id3);
+    assert_eq!(transports4[1].0, transport_id2);
+    assert_eq!(transports4[2].0, transport_id1);
+
+    Ok(())
+}

src/sql.rs

@@ -8,8 +8,7 @@ use anyhow::{Context as _, Result, bail};
 use rusqlite::{Connection, OpenFlags, Row, config::DbConfig, types::ValueRef};
 use tokio::sync::RwLock;
 
-use crate::blob::BlobObject;
-use crate::chat::ChatId;
+use crate::chat::{ChatId, add_device_msg};
 use crate::config::Config;
 use crate::context::Context;
 use crate::debug_logging::set_debug_logging_xdc;
@@ -214,32 +213,23 @@ impl Sql {
         // this should be done before updates that use high-level objects that
         // rely themselves on the low-level structure.
 
-        let recode_avatar = migrations::run(context, self)
+        let show_unencrypted_device_msg = migrations::run(context, self)
             .await
             .context("failed to run migrations")?;
 
         // (2) updates that require high-level objects
         // the structure is complete now and all objects are usable
 
-        if recode_avatar && let Some(avatar) = context.get_config(Config::Selfavatar).await? {
-            let mut blob = BlobObject::from_path(context, Path::new(&avatar))?;
-            match blob.recode_to_avatar_size(context).await {
-                Ok(()) => {
-                    if let Some(path) = blob.to_abs_path().to_str() {
-                        context
-                            .set_config_internal(Config::Selfavatar, Some(path))
-                            .await?;
-                    } else {
-                        warn!(context, "Setting selfavatar failed: non-UTF-8 filename");
-                    }
-                }
-                Err(e) => {
-                    warn!(context, "Migrations can't recode avatar, removing. {:#}", e);
-                    context
-                        .set_config_internal(Config::Selfavatar, None)
-                        .await?
-                }
-            }
+        if show_unencrypted_device_msg {
+            let txt = r#"To keep Delta Chat simpler and more secure, it no longer sends or receives messages that aren't end-to-end encrypted.
+
+Nothing is lost: your encrypted chats work as before, and old unencrypted chats remain readable.
+
+To keep sending and receiving unencrypted email, use a regular email app. You can find your email password in Delta Chat under "Settings → Advanced → Relays": tap (or right-click) your address and choose "Edit Relay".
+
+More details: https://..."#;
+            let mut msg = crate::message::Message::new_text(txt.to_string());
+            add_device_msg(context, Some("unencrypted-device-msg"), Some(&mut msg)).await?;
         }
 
         Ok(())
@@ -684,6 +674,43 @@ impl Sql {
     }
 }
 
+pub(crate) trait TransactionExt {
+    /// Used for executing `SELECT COUNT` statements only. Returns the resulting count.
+    fn count(&self, query: &str, params: impl rusqlite::Params + Send) -> Result<usize>;
+
+    /// Executes a query which is expected to return one row and one
+    /// column. If the query does not return any rows, returns `Ok(None)`.
+    fn query_get_value<T>(
+        &self,
+        query: &str,
+        params: impl rusqlite::Params + Send,
+    ) -> Result<Option<T>>
+    where
+        T: rusqlite::types::FromSql + Send + 'static;
+}
+
+impl TransactionExt for rusqlite::Transaction<'_> {
+    fn count(&self, query: &str, params: impl rusqlite::Params + Send) -> Result<usize> {
+        let count: isize = self.query_row(query, params, |row| row.get(0))?;
+        Ok(usize::try_from(count)?)
+    }
+
+    fn query_get_value<T>(
+        &self,
+        query: &str,
+        params: impl rusqlite::Params + Send,
+    ) -> Result<Option<T>>
+    where
+        T: rusqlite::types::FromSql + Send + 'static,
+    {
+        match self.query_row(query, params, |row| row.get::<_, T>(0)) {
+            Ok(res) => Ok(Some(res)),
+            Err(rusqlite::Error::QueryReturnedNoRows) => Ok(None),
+            Err(err) => Err(err.into()),
+        }
+    }
+}
+
 /// Creates a new SQLite connection.
 ///
 /// `path` is the database path.

src/sql/migrations.rs

@@ -16,6 +16,7 @@ use crate::key::DcKey;
 use crate::log::warn;
 
 use crate::sql::Sql;
+use crate::sql::TransactionExt as _;
 use crate::tools::{self, Time, inc_and_check, time_elapsed};
 use crate::transport::ConfiguredLoginParam;
 
@@ -713,6 +714,111 @@ pub(crate) async fn msgs_to_key_contacts(context: &Context) -> Result<()> {
     Ok(())
 }
 
+fn unencrypted_chats_migration(
+    context: &Context,
+    transaction: &mut rusqlite::Transaction<'_>,
+) -> Result<bool> {
+    let mut show_unencrypted_device_msg = false;
+
+    // Migrate:
+    // - all unencrypted (i.e. ad-hoc) groups to have 0 members
+    // - all chats of type Mailinglist into a group with 0 members
+    transaction.execute_batch(
+        "
+CREATE TEMP TABLE temp.legacy_unencrypted_chats(chat_id INTEGER PRIMARY KEY, type INTEGER) STRICT;
+
+INSERT INTO temp.legacy_unencrypted_chats(chat_id, type)
+SELECT id, type FROM chats
+WHERE ((type=120 AND grpid='') OR type=140) -- Ad-hoc groups and mailinglists
+  AND id>9;
+
+DELETE FROM chats_contacts
+WHERE chat_id IN (SELECT chat_id FROM temp.legacy_unencrypted_chats);
+
+UPDATE chats SET type=120
+WHERE id IN (SELECT chat_id FROM temp.legacy_unencrypted_chats);
+",
+    )?;
+
+    // Rewrite all address-contacts to have "Hidden" origin.
+    // We still need the contacts because we want to keep the messages, and every message needs a sender.
+    // Make sure that the address is available in the name, so that the user can still see it.
+    transaction.execute_batch(
+        "
+UPDATE contacts
+SET origin=8 -- Origin::Hidden
+WHERE fingerprint='' AND id>9;
+
+UPDATE contacts
+SET name=name || ' (' || addr || ')'
+WHERE fingerprint='' AND id>9 AND name!='';
+
+UPDATE contacts
+SET authname=authname || ' (' || addr || ')'
+WHERE fingerprint='' AND id>9 AND name='' AND authname!='';
+
+UPDATE contacts
+SET authname=addr
+WHERE fingerprint='' AND id>9 AND name='' AND authname='';
+
+-- Also update the names of the chats:
+UPDATE chats
+SET name = (
+    SELECT CASE
+        WHEN c.name != ''     THEN c.name
+        WHEN c.authname != '' THEN c.authname
+        ELSE c.addr
+    END
+    FROM chats_contacts cc
+    JOIN contacts c ON c.id = cc.contact_id
+    WHERE cc.chat_id = chats.id
+)
+WHERE type = 100 AND id > 9
+AND EXISTS (
+    SELECT 1 FROM chats_contacts cc
+    JOIN contacts c ON c.id = cc.contact_id
+    WHERE cc.chat_id = chats.id AND c.fingerprint = '' AND c.id > 9
+);
+",
+    )?;
+
+    let legacy_chats =
+        transaction.count("SELECT COUNT(*) FROM temp.legacy_unencrypted_chats", ())?;
+    let legacy_contacts = transaction.count(
+        "SELECT COUNT(*) FROM contacts WHERE fingerprint='' AND id>9",
+        (),
+    )?;
+    if legacy_chats > 0 || legacy_contacts > 0 {
+        // Set the gray letter avatar for all legacy chats:
+        let blob = crate::blob::BlobObject::create_and_deduplicate_from_bytes(
+            context,
+            include_bytes!("../../assets/icon-unencrypted.png"),
+            "icon-unencrypted.png",
+        )?;
+        let new_param = &format!("i={}", blob.as_name());
+        transaction.execute(
+            "UPDATE chats SET param=? WHERE id IN (SELECT chat_id FROM temp.legacy_unencrypted_chats)",
+            (new_param,),
+        )?;
+        transaction.execute(
+            "UPDATE contacts SET param=? WHERE fingerprint='' AND id>9",
+            (new_param,),
+        )?;
+
+        let force_encryption: Option<String> = transaction.query_get_value(
+            "SELECT value FROM config WHERE keyname='force_encryption'",
+            (),
+        )?;
+        if force_encryption == Some("0".to_string()) {
+            show_unencrypted_device_msg = true;
+        }
+    }
+
+    transaction.execute("DROP TABLE temp.legacy_unencrypted_chats", ())?;
+
+    Ok(show_unencrypted_device_msg)
+}
+
 impl Sql {
     async fn set_db_version(&self, version: i32) -> Result<()> {
         self.set_raw_config_int(VERSION_CFG, version).await?;
@@ -812,7 +918,7 @@ pub async fn run(context: &Context, sql: &Sql) -> Result<bool> {
     }
 
     let dbversion = dbversion_before_update;
-    let mut recode_avatar = false;
+    let mut show_unencrypted_device_msg = false;
 
     if dbversion < 1 {
         sql.execute_migration(
@@ -1187,7 +1293,7 @@ CREATE TABLE imap_sync (folder TEXT PRIMARY KEY, uidvalidity INTEGER DEFAULT 0,
             .await?;
     }
     if dbversion < 77 {
-        recode_avatar = true;
+        // removed
         sql.set_db_version(77).await?;
     }
     if dbversion < 78 {
@@ -2672,6 +2778,34 @@ CREATE TABLE smtp2 (
             .await?;
     }
 
+    inc_and_check(&mut migration_version, 168)?;
+    if dbversion < migration_version {
+        sql.execute_migration(
+            "
+CREATE TABLE smtp_success (
+    id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
+    transport_id INTEGER UNIQUE NOT NULL,
+    FOREIGN KEY(transport_id) REFERENCES transports(id) ON DELETE CASCADE
+) STRICT;
+",
+            migration_version,
+        )
+        .await?;
+    }
+
+    inc_and_check(&mut migration_version, 169)?;
+    if dbversion < migration_version {
+        sql.execute_migration_transaction(
+            |transaction| {
+                show_unencrypted_device_msg = unencrypted_chats_migration(context, transaction)?;
+
+                Ok(())
+            },
+            migration_version,
+        )
+        .await?;
+    }
+
     let new_version = sql
         .get_raw_config_int(VERSION_CFG)
         .await?
@@ -2686,7 +2820,7 @@ CREATE TABLE smtp2 (
     }
     info!(context, "Database version: v{new_version}.");
 
-    Ok(recode_avatar)
+    Ok(show_unencrypted_device_msg)
 }
 
 #[cfg(test)]

src/sql/migrations/migrations_tests.rs

@@ -1,8 +1,10 @@
 use super::*;
 use crate::chat;
+use crate::chat::Chat;
 use crate::chat::ChatId;
 use crate::config::Config;
 use crate::constants;
+use crate::constants::Chattype;
 use crate::contact::Contact;
 use crate::contact::ContactId;
 use crate::contact::Origin;
@@ -145,7 +147,8 @@ async fn test_key_contacts_migration_email1() -> Result<()> {
         .unwrap();
     let email_bob = Contact::get_by_id(&t, email_bob_id).await?;
     assert_eq!(email_bob.is_key_contact(), false);
-    assert_eq!(email_bob.origin, Origin::OutgoingTo);
+    // All email address contacts are hidden now:
+    assert_eq!(email_bob.origin, Origin::Hidden);
     assert_eq!(email_bob.e2ee_avail(&t).await?, false);
     assert_eq!(email_bob.fingerprint(), None);
 
@@ -178,7 +181,8 @@ async fn test_key_contacts_migration_email2() -> Result<()> {
         .unwrap();
     let email_bob = Contact::get_by_id(&t, email_bob_id).await?;
     assert_eq!(email_bob.is_key_contact(), false);
-    assert_eq!(email_bob.origin, Origin::OutgoingTo);
+    // All email address contacts are hidden now:
+    assert_eq!(email_bob.origin, Origin::Hidden);
     assert_eq!(email_bob.e2ee_avail(&t).await?, false);
     assert_eq!(email_bob.fingerprint(), None);
 
@@ -227,3 +231,98 @@ async fn test_key_contacts_migration_verified() -> Result<()> {
 
     Ok(())
 }
+
+/// Creates a context right before the unencrypted-chats migration (v169).
+async fn context_before_unencrypted_migration() -> TestContext {
+    STOP_MIGRATIONS_AT
+        .scope(168, async move { TestContext::new_alice().await })
+        .await
+}
+
+/// Adds legacy data: a 1:1 chat (chat 10), an ad-hoc group (chat 11),
+/// a mailing list (chat 12) and an address-contact (contact 10).
+async fn add_legacy_data(t: &TestContext) -> Result<()> {
+    t.sql
+        .call_write(|conn| {
+            conn.execute_batch(
+                r#"
+INSERT INTO contacts (id, name, addr, origin, fingerprint)
+    VALUES (10, 'Bob', 'bob@example.net', 16384, '');
+INSERT INTO chats (id, type, name, grpid) VALUES
+    (10, 100, 'Bob', ''),
+    (11, 120, 'Thread', ''),
+    (12, 140, 'List', 'list.example.org');
+INSERT INTO chats_contacts (chat_id, contact_id) VALUES
+    (10, 10), (11, 1), (11, 10), (12, 10);"#,
+            )?;
+            Ok(())
+        })
+        .await
+}
+
+/// Legacy unencrypted 1:1 chats get the email address put into the name,
+/// and get the "unencrypted" avatar.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_unencrypted_chats_migration_1to1_chat() -> Result<()> {
+    let t = &context_before_unencrypted_migration().await;
+    add_legacy_data(t).await?;
+    t.sql.run_migrations(t).await?;
+
+    let bob = Contact::get_by_id(t, ContactId::new(10)).await?;
+    assert_eq!(bob.get_display_name(), "Bob (bob@example.net)");
+    assert!(bob.get_profile_image(t).await?.unwrap().exists());
+
+    let chat = Chat::load_from_db(t, ChatId::new(10)).await?;
+    assert_eq!(chat.get_name(), "Bob (bob@example.net)");
+    assert!(chat.get_profile_image(t).await?.unwrap().exists());
+
+    Ok(())
+}
+
+/// Legacy ad-hoc groups and mailing lists become read-only groups without members
+/// and get the "unencrypted" avatar.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_unencrypted_chats_migration_readonly_groups() -> Result<()> {
+    let t = &context_before_unencrypted_migration().await;
+    add_legacy_data(t).await?;
+    t.sql.run_migrations(t).await?;
+
+    for chat_id in [11, 12] {
+        let chat_id = ChatId::new(chat_id);
+        let chat = Chat::load_from_db(t, chat_id).await?;
+        assert_eq!(chat.typ, Chattype::Group);
+        assert!(chat::get_chat_contacts(t, chat_id).await?.is_empty());
+        assert_eq!(chat.can_send(t).await?, false);
+        assert!(chat.get_profile_image(t).await?.unwrap().exists());
+    }
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_unencrypted_chats_migration_device_msg() -> Result<()> {
+    for (force_encryption, has_legacy_data, expect_device_msg) in [
+        (Some("0"), true, true),
+        (Some("1"), true, false),
+        (None, true, false),
+        (Some("0"), false, false),
+    ] {
+        let t = &context_before_unencrypted_migration().await;
+        t.sql
+            .set_raw_config("force_encryption", force_encryption)
+            .await?;
+        if has_legacy_data {
+            add_legacy_data(t).await?;
+        }
+        t.sql.run_migrations(t).await?;
+
+        let shown = chat::was_device_msg_ever_added(t, "unencrypted-device-msg").await?;
+        assert_eq!(shown, expect_device_msg);
+
+        // This assert will need to be removed once we remove the ForceEncryption config,
+        // but it is useful for now to check that the logic is implemented correctly:
+        let allow_unencrypted = !t.get_config_bool(Config::ForceEncryption).await?;
+        assert_eq!(shown, allow_unencrypted && has_legacy_data);
+    }
+    Ok(())
+}

src/summary.rs

@@ -56,9 +56,6 @@ pub struct Summary {
 
     /// Message state.
     pub state: MessageState,
-
-    /// Message preview image path
-    pub thumbnail_path: Option<String>,
 }
 
 impl Summary {
@@ -83,7 +80,6 @@ impl Summary {
                 text: msg_reacted(context, reaction_contact_id, &reaction, &summary).await,
                 timestamp: msg.get_timestamp(), // message timestamp (not reaction) to make timestamps more consistent with chats ordering
                 state: msg.state, // message state (not reaction) - indicating if it was me sending the last message
-                thumbnail_path: None,
             });
         }
         Self::new(context, msg, chat, contact).await
@@ -127,24 +123,11 @@ impl Summary {
             text = stock_str::reply_noun(context)
         }
 
-        let thumbnail_path = if msg.viewtype == Viewtype::Image
-            || msg.viewtype == Viewtype::Gif
-            || msg.viewtype == Viewtype::Sticker
-        {
-            msg.get_file(context)
-                .and_then(|path| path.to_str().map(|p| p.to_owned()))
-        } else if msg.viewtype == Viewtype::Webxdc {
-            Some("webxdc-icon://last-msg-id".to_string())
-        } else {
-            None
-        };
-
         Ok(Summary {
             prefix,
             text,
             timestamp: msg.get_timestamp(),
             state: msg.state,
-            thumbnail_path,
         })
     }
 

src/webxdc/webxdc_tests.rs

@@ -1664,7 +1664,6 @@ async fn test_webxdc_chatlist_summary() -> Result<()> {
     assert_eq!(chatlist.len(), 1);
     let summary = chatlist.get_summary(&t, 0, None).await?;
     assert_eq!(summary.text, "📱 nice app!".to_string());
-    assert_eq!(summary.thumbnail_path.unwrap(), "webxdc-icon://last-msg-id");
 
     Ok(())
 }