From 0220689964fbecf2e3d293ddd03a4775df874dc1 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 29 May 2025 12:55:37 +0200 Subject: [PATCH] refactor: Wrap EncryptionInfo in Arc It's >100 bytes large and often optional, so it makes sense to put it on the heap to reduce the size of structs with such optional fields, and the stack size of functions with such optional parameters. It's also cloned in a couple places in the UI crate, so it probably makes sense to just always refcount it. This started as a clippy suggestion to box PendingEdit inside AggregationKind::Edit. --- Cargo.toml | 2 +- .../event_cache/store/integration_tests.rs | 6 ++-- .../src/deserialized_responses.rs | 29 ++++++++++--------- crates/matrix-sdk-crypto/src/machine/mod.rs | 12 ++++---- .../src/machine/tests/mod.rs | 6 ++-- .../src/timeline/controller/aggregations.rs | 4 +-- .../controller/decryption_retry_task.rs | 4 +-- .../src/timeline/event_handler.rs | 6 ++-- .../src/timeline/event_item/mod.rs | 7 +++-- .../src/timeline/event_item/remote.rs | 4 +-- .../matrix-sdk-ui/src/timeline/tests/edit.rs | 8 ++--- .../src/timeline/tests/encryption.rs | 9 ++++-- .../matrix-sdk-ui/src/timeline/tests/mod.rs | 7 +++-- crates/matrix-sdk-ui/src/timeline/traits.rs | 6 ++-- crates/matrix-sdk/src/event_handler/mod.rs | 2 +- crates/matrix-sdk/src/room/mod.rs | 2 +- 16 files changed, 62 insertions(+), 52 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4df15c91c..330c14011 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -79,7 +79,7 @@ ruma = { git = "https://github.com/ruma/ruma", rev = "b4941a991919685345646a230b ruma-common = { git = "https://github.com/ruma/ruma", rev = "b4941a991919685345646a230b05b985338ec3f8" } sentry = "0.36.0" sentry-tracing = "0.36.0" -serde = "1.0.217" +serde = { version = "1.0.217", features = ["rc"] } serde_html_form = "0.2.7" serde_json = "1.0.138" sha2 = "0.10.8" diff --git a/crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs b/crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs index 42fd9658a..b30837c68 100644 --- a/crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs +++ b/crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs @@ -14,6 +14,8 @@ //! Trait and macro of integration tests for `EventCacheStore` implementations. +use std::sync::Arc; + use assert_matches::assert_matches; use matrix_sdk_common::{ deserialized_responses::{ @@ -55,7 +57,7 @@ pub fn make_test_event_with_event_id( content: &str, event_id: Option<&EventId>, ) -> TimelineEvent { - let encryption_info = EncryptionInfo { + let encryption_info = Arc::new(EncryptionInfo { sender: (*ALICE).into(), sender_device: None, algorithm_info: AlgorithmInfo::MegolmV1AesSha2 { @@ -64,7 +66,7 @@ pub fn make_test_event_with_event_id( session_id: Some("mysessionid9".to_owned()), }, verification_state: VerificationState::Verified, - }; + }); let mut builder = EventFactory::new().text_msg(content).room(room_id).sender(*ALICE); if let Some(event_id) = event_id { diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index 0fe4d5d5e..3ad74f868 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{collections::BTreeMap, fmt}; +use std::{collections::BTreeMap, fmt, sync::Arc}; #[cfg(doc)] use ruma::events::AnyTimelineEvent; @@ -462,7 +462,7 @@ impl TimelineEvent { /// If the event was a decrypted event that was successfully decrypted, get /// its encryption info. Otherwise, `None`. - pub fn encryption_info(&self) -> Option<&EncryptionInfo> { + pub fn encryption_info(&self) -> Option<&Arc> { self.kind.encryption_info() } @@ -569,7 +569,7 @@ impl TimelineEventKind { /// If the event was a decrypted event that was successfully decrypted, get /// its encryption info. Otherwise, `None`. - pub fn encryption_info(&self) -> Option<&EncryptionInfo> { + pub fn encryption_info(&self) -> Option<&Arc> { match self { TimelineEventKind::Decrypted(d) => Some(&d.encryption_info), TimelineEventKind::UnableToDecrypt { .. } | TimelineEventKind::PlainText { .. } => None, @@ -649,7 +649,7 @@ pub struct DecryptedRoomEvent { pub event: Raw, /// The encryption info about the event. - pub encryption_info: EncryptionInfo, + pub encryption_info: Arc, /// The encryption info about the events bundled in the `unsigned` /// object. @@ -706,7 +706,7 @@ impl UnsignedEventLocation { #[derive(Debug, Clone, Serialize, Deserialize)] pub enum UnsignedDecryptionResult { /// The event was successfully decrypted. - Decrypted(EncryptionInfo), + Decrypted(Arc), /// The event failed to be decrypted. UnableToDecrypt(UnableToDecryptInfo), } @@ -714,7 +714,7 @@ pub enum UnsignedDecryptionResult { impl UnsignedDecryptionResult { /// Returns the encryption info for this bundled event if it was /// successfully decrypted. - pub fn encryption_info(&self) -> Option<&EncryptionInfo> { + pub fn encryption_info(&self) -> Option<&Arc> { match self { Self::Decrypted(info) => Some(info), Self::UnableToDecrypt(_) => None, @@ -918,9 +918,10 @@ struct SyncTimelineEventDeserializationHelperV0 { /// The actual event. event: Raw, - /// The encryption info about the event. Will be `None` if the event - /// was not encrypted. - encryption_info: Option, + /// The encryption info about the event. + /// + /// Will be `None` if the event was not encrypted. + encryption_info: Option>, /// The push actions associated with this event. #[serde(default)] @@ -966,7 +967,7 @@ impl From for TimelineEvent { #[cfg(test)] mod tests { - use std::collections::BTreeMap; + use std::{collections::BTreeMap, sync::Arc}; use assert_matches::assert_matches; use insta::{assert_json_snapshot, with_settings}; @@ -1118,7 +1119,7 @@ mod tests { let room_event = TimelineEvent { kind: TimelineEventKind::Decrypted(DecryptedRoomEvent { event: Raw::new(&example_event()).unwrap().cast(), - encryption_info: EncryptionInfo { + encryption_info: Arc::new(EncryptionInfo { sender: user_id!("@sender:example.com").to_owned(), sender_device: None, algorithm_info: AlgorithmInfo::MegolmV1AesSha2 { @@ -1127,7 +1128,7 @@ mod tests { session_id: Some("xyz".to_owned()), }, verification_state: VerificationState::Verified, - }, + }), unsigned_encryption_info: Some(BTreeMap::from([( UnsignedEventLocation::RelationsReplace, UnsignedDecryptionResult::UnableToDecrypt(UnableToDecryptInfo { @@ -1490,7 +1491,7 @@ mod tests { let room_event = TimelineEvent { kind: TimelineEventKind::Decrypted(DecryptedRoomEvent { event: Raw::new(&example_event()).unwrap().cast(), - encryption_info: EncryptionInfo { + encryption_info: Arc::new(EncryptionInfo { sender: user_id!("@sender:example.com").to_owned(), sender_device: Some(device_id!("ABCDEFGHIJ").to_owned()), algorithm_info: AlgorithmInfo::MegolmV1AesSha2 { @@ -1508,7 +1509,7 @@ mod tests { session_id: Some("mysessionid112".to_owned()), }, verification_state: VerificationState::Verified, - }, + }), unsigned_encryption_info: Some(BTreeMap::from([( UnsignedEventLocation::RelationsThreadLatestEvent, UnsignedDecryptionResult::UnableToDecrypt(UnableToDecryptInfo { diff --git a/crates/matrix-sdk-crypto/src/machine/mod.rs b/crates/matrix-sdk-crypto/src/machine/mod.rs index 6802c3c34..aade09f2b 100644 --- a/crates/matrix-sdk-crypto/src/machine/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/mod.rs @@ -1760,13 +1760,13 @@ impl OlmMachine { &self, session: &InboundGroupSession, sender: &UserId, - ) -> MegolmResult { + ) -> MegolmResult> { let (verification_state, device_id) = self.get_or_update_verification_state(session, sender).await?; let sender = sender.to_owned(); - Ok(EncryptionInfo { + Ok(Arc::new(EncryptionInfo { sender, sender_device: device_id, algorithm_info: AlgorithmInfo::MegolmV1AesSha2 { @@ -1779,7 +1779,7 @@ impl OlmMachine { session_id: Some(session.session_id().to_owned()), }, verification_state, - }) + })) } async fn decrypt_megolm_events( @@ -1788,7 +1788,7 @@ impl OlmMachine { event: &EncryptedEvent, content: &SupportedEventEncryptionSchemes<'_>, decryption_settings: &DecryptionSettings, - ) -> MegolmResult<(JsonObject, EncryptionInfo)> { + ) -> MegolmResult<(JsonObject, Arc)> { let session = self.get_inbound_group_session_or_error(room_id, content.session_id()).await?; @@ -2200,7 +2200,7 @@ impl OlmMachine { &self, event: &Raw, room_id: &RoomId, - ) -> MegolmResult { + ) -> MegolmResult> { let event = event.deserialize()?; let content: SupportedEventEncryptionSchemes<'_> = match &event.content.scheme { @@ -2233,7 +2233,7 @@ impl OlmMachine { room_id: &RoomId, session_id: &str, sender: &UserId, - ) -> MegolmResult { + ) -> MegolmResult> { let session = self.get_inbound_group_session_or_error(room_id, session_id).await?; self.get_encryption_info(&session, sender).await } diff --git a/crates/matrix-sdk-crypto/src/machine/tests/mod.rs b/crates/matrix-sdk-crypto/src/machine/tests/mod.rs index dc4b60bca..f88d30237 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/mod.rs @@ -443,12 +443,12 @@ async fn test_session_encryption_info_can_be_fetched() { // Then the expected info is returned assert_eq!(encryption_info.sender, alice_id()); - assert_eq!(encryption_info.sender_device.unwrap(), alice_device_id()); + assert_eq!(encryption_info.sender_device.as_deref(), Some(alice_device_id())); assert_matches!( - encryption_info.algorithm_info, + &encryption_info.algorithm_info, AlgorithmInfo::MegolmV1AesSha2 { curve25519_key, .. } ); - assert_eq!(curve25519_key, alice_session.sender_key().to_string()); + assert_eq!(*curve25519_key, alice_session.sender_key().to_string()); assert_eq!( encryption_info.verification_state, VerificationState::Unverified(VerificationLevel::UnsignedDevice) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/aggregations.rs b/crates/matrix-sdk-ui/src/timeline/controller/aggregations.rs index 3534ec38c..751800dcf 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/aggregations.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/aggregations.rs @@ -37,7 +37,7 @@ //! to cater for the first use case, and to never lose any aggregations in the //! second use case. -use std::{borrow::Cow, collections::HashMap}; +use std::{borrow::Cow, collections::HashMap, sync::Arc}; use as_variant::as_variant; use matrix_sdk::deserialized_responses::EncryptionInfo; @@ -82,7 +82,7 @@ pub(in crate::timeline) struct PendingEdit { pub edit_json: Option>, /// The encryption info for this edit. - pub encryption_info: Option, + pub encryption_info: Option>, } /// Which kind of aggregation (related event) is this? diff --git a/crates/matrix-sdk-ui/src/timeline/controller/decryption_retry_task.rs b/crates/matrix-sdk-ui/src/timeline/controller/decryption_retry_task.rs index b82972ab3..cc5168e2c 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/decryption_retry_task.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/decryption_retry_task.rs @@ -506,7 +506,7 @@ mod tests { read_receipts: Default::default(), is_own: false, is_highlighted: false, - encryption_info: Some(EncryptionInfo { + encryption_info: Some(Arc::new(EncryptionInfo { sender: owned_user_id!("@u:s.co"), sender_device: None, algorithm_info: AlgorithmInfo::MegolmV1AesSha2 { @@ -515,7 +515,7 @@ mod tests { session_id: Some(session_id.to_owned()), }, verification_state: VerificationState::Verified, - }), + })), original_json: None, latest_edit_json: None, origin: RemoteEventOrigin::Sync, diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index f19f35623..b98defa24 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -85,7 +85,7 @@ pub(super) enum Flow { /// Where should this be added in the timeline. position: TimelineItemPosition, /// Information about the encryption for this event. - encryption_info: Option, + encryption_info: Option>, }, } @@ -150,7 +150,7 @@ pub(super) struct RemoteEventContext<'a> { event_id: &'a EventId, raw_event: &'a Raw, relations: BundledMessageLikeRelations, - bundled_edit_encryption_info: Option, + bundled_edit_encryption_info: Option>, } /// An action that we want to cause on the timeline. @@ -195,7 +195,7 @@ impl TimelineAction { raw_event: &Raw, room_data_provider: &P, unable_to_decrypt_info: Option, - bundled_edit_encryption_info: Option, + bundled_edit_encryption_info: Option>, timeline_items: &Vector>, meta: &mut TimelineMetadata, ) -> Option { diff --git a/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs b/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs index 465994712..c742cb4f1 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs @@ -398,7 +398,7 @@ impl EventTimelineItem { pub fn encryption_info(&self) -> Option<&EncryptionInfo> { match &self.kind { EventTimelineItemKind::Local(_) => None, - EventTimelineItemKind::Remote(remote_event) => remote_event.encryption_info.as_ref(), + EventTimelineItemKind::Remote(remote_event) => remote_event.encryption_info.as_deref(), } } @@ -521,7 +521,10 @@ impl EventTimelineItem { } /// Clone the current event item, and update its `encryption_info`. - pub(super) fn with_encryption_info(&self, encryption_info: Option) -> Self { + pub(super) fn with_encryption_info( + &self, + encryption_info: Option>, + ) -> Self { let mut new = self.clone(); if let EventTimelineItemKind::Remote(r) = &mut new.kind { r.encryption_info = encryption_info; diff --git a/crates/matrix-sdk-ui/src/timeline/event_item/remote.rs b/crates/matrix-sdk-ui/src/timeline/event_item/remote.rs index 8e0d6a684..e6a8d0600 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_item/remote.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_item/remote.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::fmt; +use std::{fmt, sync::Arc}; use indexmap::IndexMap; use matrix_sdk::deserialized_responses::EncryptionInfo; @@ -46,7 +46,7 @@ pub(in crate::timeline) struct RemoteEventTimelineItem { pub is_highlighted: bool, /// Encryption information. - pub encryption_info: Option, + pub encryption_info: Option>, /// JSON of the original event. /// diff --git a/crates/matrix-sdk-ui/src/timeline/tests/edit.rs b/crates/matrix-sdk-ui/src/timeline/tests/edit.rs index 90b44ff6d..cc716eada 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/edit.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/edit.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::BTreeMap; +use std::{collections::BTreeMap, sync::Arc}; use assert_matches2::assert_let; use eyeball_im::VectorDiff; @@ -167,7 +167,7 @@ async fn test_edit_updates_encryption_info() { .room(room_id) .into_raw_timeline(); - let mut encryption_info = EncryptionInfo { + let mut encryption_info = Arc::new(EncryptionInfo { sender: (*ALICE).into(), sender_device: None, algorithm_info: AlgorithmInfo::MegolmV1AesSha2 { @@ -176,7 +176,7 @@ async fn test_edit_updates_encryption_info() { session_id: Some("mysessionid6333".to_owned()), }, verification_state: VerificationState::Verified, - }; + }); let original_event: TimelineEvent = DecryptedRoomEvent { event: original_event.cast(), @@ -205,7 +205,7 @@ async fn test_edit_updates_encryption_info() { .room(room_id) .edit(original_event_id, MessageType::text_plain("!!edited!! **better** message").into()) .into_raw_timeline(); - encryption_info.verification_state = + Arc::make_mut(&mut encryption_info).verification_state = VerificationState::Unverified(VerificationLevel::UnverifiedIdentity); let edit_event: TimelineEvent = DecryptedRoomEvent { event: edit_event.cast(), diff --git a/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs b/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs index 881ae785d..0e712afb7 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs @@ -690,8 +690,11 @@ async fn test_retry_fetching_encryption_info() { assert_pending!(stream); } -fn make_encryption_info(session_id: &str, verification_state: VerificationState) -> EncryptionInfo { - EncryptionInfo { +fn make_encryption_info( + session_id: &str, + verification_state: VerificationState, +) -> Arc { + Arc::new(EncryptionInfo { sender: BOB.to_owned(), sender_device: Some(owned_device_id!("BOBDEVICE")), algorithm_info: AlgorithmInfo::MegolmV1AesSha2 { @@ -700,7 +703,7 @@ fn make_encryption_info(session_id: &str, verification_state: VerificationState) session_id: Some(session_id.to_owned()), }, verification_state, - } + }) } #[async_test] diff --git a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs index 8b6440101..08d9b2dff 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs @@ -264,7 +264,7 @@ struct TestRoomDataProvider { /// The [`EncryptionInfo`] describing the Megolm sessions that were used to /// encrypt events. - pub encryption_info: HashMap, + pub encryption_info: HashMap>, } impl TestRoomDataProvider { @@ -272,6 +272,7 @@ impl TestRoomDataProvider { self.initial_user_receipts = initial_user_receipts; self } + fn with_fully_read_marker(mut self, event_id: OwnedEventId) -> Self { self.fully_read_marker = Some(event_id); self @@ -280,7 +281,7 @@ impl TestRoomDataProvider { fn with_encryption_info( mut self, session_id: &str, - encryption_info: EncryptionInfo, + encryption_info: Arc, ) -> TestRoomDataProvider { self.encryption_info.insert(session_id.to_owned(), encryption_info); self @@ -425,7 +426,7 @@ impl RoomDataProvider for TestRoomDataProvider { &self, session_id: &str, _sender: &UserId, - ) -> Option { + ) -> Option> { self.encryption_info.get(session_id).cloned() } diff --git a/crates/matrix-sdk-ui/src/timeline/traits.rs b/crates/matrix-sdk-ui/src/timeline/traits.rs index bd985d7d3..11fc3bc2b 100644 --- a/crates/matrix-sdk-ui/src/timeline/traits.rs +++ b/crates/matrix-sdk-ui/src/timeline/traits.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::future::Future; +use std::{future::Future, sync::Arc}; use eyeball::Subscriber; use indexmap::IndexMap; @@ -142,7 +142,7 @@ pub(super) trait RoomDataProvider: &self, session_id: &str, sender: &UserId, - ) -> impl Future> + SendOutsideWasm; + ) -> impl Future>> + SendOutsideWasm; async fn relations(&self, event_id: OwnedEventId, opts: RelationsOptions) -> Result; } @@ -286,7 +286,7 @@ impl RoomDataProvider for Room { &self, session_id: &str, sender: &UserId, - ) -> Option { + ) -> Option> { // Pass directly on to `Room::get_encryption_info` self.get_encryption_info(session_id, sender).await } diff --git a/crates/matrix-sdk/src/event_handler/mod.rs b/crates/matrix-sdk/src/event_handler/mod.rs index 22dfe5f6e..b50ae5a53 100644 --- a/crates/matrix-sdk/src/event_handler/mod.rs +++ b/crates/matrix-sdk/src/event_handler/mod.rs @@ -401,7 +401,7 @@ impl Client { }; let raw_event = item.raw().json(); - let encryption_info = item.encryption_info(); + let encryption_info = item.encryption_info().map(|i| &**i); let push_actions = item.push_actions.as_deref().unwrap_or(&[]); // Event handlers for possibly-redacted timeline events diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index 6dac8d4b5..368190dbe 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -1523,7 +1523,7 @@ impl Room { &self, session_id: &str, sender: &UserId, - ) -> Option { + ) -> Option> { let machine = self.client.olm_machine().await; let machine = machine.as_ref()?; machine.get_session_encryption_info(self.room_id(), session_id, sender).await.ok()