From fc069a9b00e8dbf228e0d3737d0c274c25c20ca4 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Thu, 15 Jan 2026 03:36:01 -0300 Subject: [PATCH] feat: Move primary transport checked update into "remove transports" transaction This guarantees that the primary transport is never removed while being in use. At least this way it's obvious that there are no such corner cases. --- .../tests/test_multitransport.py | 3 -- src/receive_imf.rs | 48 ++----------------- src/sync.rs | 16 +++++-- src/transport.rs | 38 +++++++++++++-- 4 files changed, 53 insertions(+), 52 deletions(-) diff --git a/deltachat-rpc-client/tests/test_multitransport.py b/deltachat-rpc-client/tests/test_multitransport.py index 2e52cb0d5..8477d6bdc 100644 --- a/deltachat-rpc-client/tests/test_multitransport.py +++ b/deltachat-rpc-client/tests/test_multitransport.py @@ -168,9 +168,6 @@ def test_transport_synchronization(acfactory, log) -> None: log.section("ac1 changes the primary transport") ac1.set_config("configured_addr", transport3["addr"]) - # One event for updated `add_timestamp` of the new primary transport, - # one event for the `configured_addr` update. - ac1_clone.wait_for_event(EventType.TRANSPORTS_MODIFIED) ac1_clone.wait_for_event(EventType.TRANSPORTS_MODIFIED) [transport1, transport3] = ac1_clone.list_transports() assert ac1_clone.get_config("configured_addr") == addr3 diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 0415d76b4..7ed8d6624 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -819,50 +819,12 @@ pub(crate) async fn receive_imf_inner( if from_id == ContactId::SELF { if mime_parser.was_encrypted() { context - .execute_sync_items(sync_items, mime_parser.timestamp_sent) + .execute_sync_items( + sync_items, + mime_parser.timestamp_sent, + &mime_parser.from.addr, + ) .await; - - // Receiving encrypted message from self updates primary transport. - let from_addr = &mime_parser.from.addr; - - let transport_changed = context - .sql - .transaction(|transaction| { - let transport_exists = transaction.query_row( - "SELECT COUNT(*) FROM transports WHERE addr=?", - (from_addr,), - |row| { - let count: i64 = row.get(0)?; - Ok(count > 0) - }, - )?; - - let transport_changed = if transport_exists { - transaction.execute( - " -UPDATE config SET value=? WHERE keyname='configured_addr' AND value!=?1 - ", - (from_addr,), - )? > 0 - } else { - warn!( - context, - "Received sync message from unknown address {from_addr:?}." - ); - false - }; - Ok(transport_changed) - }) - .await?; - if transport_changed { - info!(context, "Primary transport changed to {from_addr:?}."); - context.sql.uncache_raw_config("configured_addr").await; - - // Regenerate User ID in V4 keys. - context.self_public_key.lock().await.take(); - - context.emit_event(EventType::TransportsModified); - } } else { warn!(context, "Sync items are not encrypted."); } diff --git a/src/sync.rs b/src/sync.rs index 8b050456d..a2aa453e3 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -307,7 +307,12 @@ impl Context { /// If an error is returned, the caller shall not try over because some sync items could be /// already executed. Sync items are considered independent and executed in the given order but /// regardless of whether executing of the previous items succeeded. - pub(crate) async fn execute_sync_items(&self, items: &SyncItems, timestamp_sent: i64) { + pub(crate) async fn execute_sync_items( + &self, + items: &SyncItems, + timestamp_sent: i64, + from: &str, + ) { info!(self, "executing {} sync item(s)", items.items.len()); for item in &items.items { // Limit the timestamp to ensure it is not in the future. @@ -327,7 +332,7 @@ impl Context { SyncData::Transports { transports, removed_transports, - } => sync_transports(self, transports, removed_transports).await, + } => sync_transports(self, from, transports, removed_transports).await, }, SyncDataOrUnknown::Unknown(data) => { warn!(self, "Ignored unknown sync item: {data}."); @@ -636,7 +641,12 @@ mod tests { .to_string(), ) ?; - t.execute_sync_items(&sync_items, timestamp_sent).await; + t.execute_sync_items( + &sync_items, + timestamp_sent, + &t.get_config(Config::Addr).await?.unwrap(), + ) + .await; assert!( Contact::lookup_id_by_addr(&t, "bob@example.net", Origin::Unknown) diff --git a/src/transport.rs b/src/transport.rs index fafe19377..0f280dc4b 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -21,6 +21,7 @@ use crate::constants::{DC_LP_AUTH_FLAGS, DC_LP_AUTH_OAUTH2}; use crate::context::Context; use crate::ensure_and_debug_assert; use crate::events::EventType; +use crate::log::warn; use crate::login_param::EnteredLoginParam; use crate::net::load_connection_timestamp; use crate::provider::{Protocol, Provider, Socket, UsernamePattern, get_provider_by_id}; @@ -774,6 +775,7 @@ pub(crate) async fn send_sync_transports(context: &Context) -> Result<()> { /// Process received data for transport synchronization. pub(crate) async fn sync_transports( context: &Context, + from_addr: &str, transports: &[TransportData], removed_transports: &[RemovedTransportData], ) -> Result<()> { @@ -788,7 +790,7 @@ pub(crate) async fn sync_transports( modified |= save_transport(context, entered, configured, *timestamp, *is_published).await?; } - context + let primary_changed = context .sql .transaction(|transaction| { for RemovedTransportData { addr, timestamp } in removed_transports { @@ -806,13 +808,43 @@ pub(crate) async fn sync_transports( (addr, timestamp), )?; } - Ok(()) + + let transport_exists = transaction.query_row( + "SELECT COUNT(*) FROM transports WHERE addr=?", + (from_addr,), + |row| { + let count: i64 = row.get(0)?; + Ok(count > 0) + }, + )?; + + let primary_changed = if transport_exists { + transaction.execute( + " +UPDATE config SET value=? WHERE keyname='configured_addr' AND value!=?1 + ", + (from_addr,), + )? > 0 + } else { + warn!( + context, + "Received sync message from unknown address {from_addr:?}." + ); + false + }; + Ok(primary_changed) }) .await?; if modified { - context.self_public_key.lock().await.take(); tokio::task::spawn(restart_io_if_running_boxed(context.clone())); + } + if primary_changed { + info!(context, "Primary transport changed to {from_addr:?}."); + context.sql.uncache_raw_config("configured_addr").await; + } + if modified || primary_changed { + context.self_public_key.lock().await.take(); context.emit_event(EventType::TransportsModified); } Ok(())