From 349c9650e3db37f8476302167312d327d774601b Mon Sep 17 00:00:00 2001 From: link2xt Date: Wed, 16 Sep 2026 18:56:47 +0000 Subject: [PATCH] fix: do not send a sync message when changing "configured_addr" Sending transport is not synchronized since cd42efb36df4fdaa59c3048a2d27c3c55696c109 (PR https://github.com/chatmail/core/pull/8510) so there is no need to send a sync message when the sending transport is changed. --- .../tests/test_multitransport.py | 4 -- src/config.rs | 22 +----- src/transport/transport_tests.rs | 67 +++++++++---------- 3 files changed, 33 insertions(+), 60 deletions(-) diff --git a/deltachat-rpc-client/tests/test_multitransport.py b/deltachat-rpc-client/tests/test_multitransport.py index e0e04eb2b..055c0b787 100644 --- a/deltachat-rpc-client/tests/test_multitransport.py +++ b/deltachat-rpc-client/tests/test_multitransport.py @@ -228,10 +228,6 @@ def test_transport_sync_new_as_primary(acf, log) -> None: log.section("ac1 changes the primary transport") ac1.set_config("configured_addr", transport2["addr"]) - ac1.wait_for_event(EventType.TRANSPORTS_MODIFIED) - - ac1_clone.wait_for_event(EventType.TRANSPORTS_MODIFIED) - assert ac1_clone.get_config("configured_addr") == transport1["addr"] log.section("ac1_clone receives a message via the new transport") ac1_chat = ac1.create_chat(bob) diff --git a/src/config.rs b/src/config.rs index b509a8c20..966484f7b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -18,8 +18,8 @@ use crate::events::EventType; use crate::log::LogExt; use crate::mimefactory::RECOMMENDED_FILE_SIZE; use crate::sync::{self, Sync::*, SyncData}; -use crate::tools::{get_abs_path, time}; -use crate::transport::{add_pseudo_transport, send_sync_transports, transport_addrs}; +use crate::tools::get_abs_path; +use crate::transport::{add_pseudo_transport, transport_addrs}; use crate::{constants, stats}; /// The available configuration keys. @@ -781,28 +781,10 @@ impl Context { (addr,), )?; - // The timestamp must strictly increase because - // other devices ignore the row update otherwise, - // and contacts only adopt the re-signed key - // if its signature timestamp increases. - transaction - .execute( - "UPDATE transports - SET add_timestamp=MAX(?, add_timestamp+1) - WHERE addr=?", - (time(), addr), - ) - .context( - "Failed to update add_timestamp for the new sending transport", - )?; - Ok(()) }) .await?; - // Invalidate the cache so the sync message - // cannot read a stale sending address. self.sql.uncache_raw_config("configured_addr").await; - send_sync_transports(self).await?; } } _ => { diff --git a/src/transport/transport_tests.rs b/src/transport/transport_tests.rs index 5681a3e08..dc7e97853 100644 --- a/src/transport/transport_tests.rs +++ b/src/transport/transport_tests.rs @@ -226,10 +226,11 @@ async fn test_delete_transport() -> Result<()> { Ok(()) } -/// Tests that promoting a transport bumps its `add_timestamp` on other devices -/// even if it was added within the same second. +/// Tests that selecting sending transport by setting "configured_addr" does not +/// send the sync message, is not synchronized between devices even if sync message is forced, +/// and does not bump sending transport `add_timestamp`. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_promote_transport_same_second() -> Result<()> { +async fn test_no_configured_addr_synchronization() -> Result<()> { let mut tcm = TestContextManager::new(); let alice = &tcm.alice().await; let alice2 = &tcm.alice().await; @@ -238,11 +239,35 @@ async fn test_promote_transport_same_second() -> Result<()> { a.set_config_bool(Config::BccSelf, true).await?; } - add_dummy_transport(alice, "alice@otherprovider.com").await?; + let addr = "alice@otherprovider.com"; + add_dummy_transport(alice, addr).await?; send_sync_transports(alice).await?; - sync_and_check_recipients(alice, alice2, "alice@otherprovider.com alice@example.org").await; + sync_and_check_recipients(alice, alice2, &format!("{addr} alice@example.org")).await; - promote_transport_and_sync(alice, alice2, "alice@otherprovider.com").await + // Selects `addr` on `alice` as the sending transport + // and syncs the transport update to `alice2`, + // whose own sending transport must stay unchanged. + let old_timestamp = add_timestamp(alice2, addr).await; + let alice2_primary = alice2.get_config(Config::ConfiguredAddr).await?; + alice.set_config(Config::ConfiguredAddr, Some(addr)).await?; + assert_eq!(add_timestamp(alice, addr).await, old_timestamp); + + send_sync_transports(alice).await?; + alice.send_sync_msg().await?.unwrap(); + let sync_msg = alice.pop_sent_msg().await; + assert_eq!(sync_msg.recipients, format!("alice@example.org {addr}")); + // The sync message comes from the new sending address, + // which must not make `alice2` adopt it as its own sending address. + assert!(sync_msg.payload.contains(&format!("From: <{addr}>"))); + alice2.recv_msg_trash(&sync_msg).await; + + // add_timestamp must not change. + assert_eq!(add_timestamp(alice2, addr).await, old_timestamp); + assert_eq!( + alice2.get_config(Config::ConfiguredAddr).await?, + alice2_primary + ); + Ok(()) } /// Tests that `sync_transports()` requests an IO restart @@ -263,36 +288,6 @@ async fn test_sync_transports_requests_io_restart() -> Result<()> { Ok(()) } -/// Promotes `addr` on `alice` and syncs the transport update to `alice2`, -/// whose own primary transport must stay unchanged. -async fn promote_transport_and_sync( - alice: &TestContext, - alice2: &TestContext, - addr: &str, -) -> Result<()> { - let old_timestamp = add_timestamp(alice2, addr).await; - let alice2_primary = alice2.get_config(Config::ConfiguredAddr).await?; - alice.set_config(Config::ConfiguredAddr, Some(addr)).await?; - assert!(add_timestamp(alice, addr).await > old_timestamp); - - alice.send_sync_msg().await?.unwrap(); - let sync_msg = alice.pop_sent_msg().await; - assert_eq!(sync_msg.recipients, format!("alice@example.org {addr}")); - // The sync message comes from the new primary, - // which must not make `alice2` adopt it as its own primary. - assert!(sync_msg.payload.contains(&format!("From: <{addr}>"))); - alice2.recv_msg_trash(&sync_msg).await; - - // add_timestamp must monotonically increase because - // other devices ignore the change otherwise. - assert!(add_timestamp(alice2, addr).await > old_timestamp); - assert_eq!( - alice2.get_config(Config::ConfiguredAddr).await?, - alice2_primary - ); - Ok(()) -} - async fn add_timestamp(t: &TestContext, addr: &str) -> i64 { t.sql .query_get_value("SELECT add_timestamp FROM transports WHERE addr=?", (addr,))