The `UserIdentity::is_verified()` method in the matrix-sdk-crypto crate
before version 0.7.2 doesn't take into account the verification status
of the user's own identity while performing the check and may as a result
return a value contrary to what is implied by its name and documentation.
This patch fixes this and adds a regression test.
The method itself is not used internally and as such has not a larger
impact.
Co-authored-by: Denis Kasak <dkasak@termina.org.uk>
Signed-off-by: Damir Jelić <poljar@termina.org.uk>
… and specify a version such that publishing of the ui crate becomes possible.
We still use a git dependency for FFI crate builds because there were some
likely important breaking changes that haven't been released.
This is required to publish `matrix-sdk-ui`.
In migrate_data_for_v6, we incorrectly copied the keys in inbound_group_sessions verbatim into inbound_group_sessions2. What we should have done is re-encrypt them using the new table name, so we fix that up with a new migration here.
This caused the bug because we were looking for sessions to mark as backed up by calculating their key (from room_id and session_id) but that key did not exist, because the old sessions were stored under the incorrect keys. So no sessions were marked as backed up, and we repeatedly tried to re-mark them.
Before this patch, we needed to clone the inner `timeline_queue` and turn it into a concrete `Vec<SyncTimelineEvent>`, just to iterate on the elements,
and because returning an iterator from a trait method is impractical. This now changes it to return the actual concrete type of `timeline_queue`, so
we don't need the extra allocations.
Ideally, matrix-sdk and matrix-sdk-base would be merged, so we don't need to use a trait at all here.
* Bring back original (slower) build style under the `sequentially` flag to work around new build style hanging on older machines
* Apply suggestions from code review
Signed-off-by: Benjamin Bouvier <public@benj.me>
---------
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <public@benj.me>
The room_info variable in this function contains the latest_event, and later it's set as the Room's room info (`inner.inner`) field.
Calling `set_latest_event` manually here will cause an update of the `RoomInfo` subscriber, while we're not done processing the full
room, and a room info update will happen anyways later (when the entire room is processed), so this one is spurious and will only
show a partial update of the fields.
- We can look up the session meta only once, it's not useful to do it once per event as it's
loop-invariant.
- We can look at the events backwards and stop after the first room membership event we see
in that order.
* maybe trigger backup at end of sync
* add test for backup upload
* missing e2e cfg
* add sliding sync tests
* fix borrow
* fix indent
* fix naming from copy/paste
* Remove extracted function
* fix clippy
* style: inline a few variables only used once into their use sites
---------
Co-authored-by: Benjamin Bouvier <public@benj.me>
This patch improves `test_media_content` to ensure that, in case of
multiple medias, only the expected ones are removed. Previously, the
test wasn't testing _other_ medias that should be kept in case of
removals.
This patch continues to improve `test_media_content` to ensure that the
content of the media are the expected ones.
Finally, this patch updates the `MemoryStore` implementation to make
tests happy.
This patch rewrites `MemoryStore::add_media_content`,
`::get_media_content`, `::remove_media_content` and
`::remove_media_content_for_uri` to (i) work on `mxc://` URI instead
of “unique key”, and (ii) to handle removal correctly thanks to the new
`RingBuffer::remove` method.
If `use_cache` is true and the cache exists, let's return everything in
one go instead of declaring a `content` variable. It makes the code
easier to read and to understand.
This is a rather over simplistic media cache implementation for the
`MemoryStore`.
It's based on a `RingBuffer` of size 20.
`remove_media_content` pops all medias until the correct one is met (if
it exists). `remove_media_content_for_uri` removes all medias, it
ignores the URI.
A `From` implementation is part of the public API
and this conversion does not make sense outside of the module.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch updates `Timeline::send_image` and `Timeline::send_video` so
that `thumbnail_url` is now an `Option<String>`.
The idea is to allow sending an image or a video without a thumbnail.
This patch removes `RUNTIME.block_on` inside `Client::get_media_file`,
`::upload_media`, `::get_media_content` and `::get_media_thumbnail`, and
makes those methods async.
Useful if we want to know in what room
we just sent a VerificationRequest,
with UserIdentity::request_verification
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
… and fix build swift errors.
- The XCFramework was being built with dylibs which aren't supported on iOS
- The tests build was attempting to generate uniffi from a moved file
`notifications` were stored in the `StateChanges` struct, which made me think that they're then persisted in the database. It's not the case, they
were just stored there by convenience. This commit changes it to a parameter that's passed, as it's not too invasive and clearer that this is only
transient data.
`itertools::format` is known to cause issues when its display implementation is being
used multiple times, as it consumes the iterator it was given (and that can only happen once,
unless caching it). This is bad, as our production apps may have multiple subscribers they will
run into a panic.
The fix is to reify the debug string before it's logged, so the tracing consumer will only see
a string, and not the display implementation that would panic on the second use.
Currently, querying for inbound group sessions which need backing up is very
inefficient: we have to search through the whole list.
Here, we change the way they are stored so that we can maintain an index of the
ones that need a backup.
Fixes: https://github.com/vector-im/element-web/issues/26488
Fixes: https://github.com/matrix-org/matrix-rust-sdk/issues/2877
---
* indexeddb: Update storage for inbound_group_sessions
Currently, querying for inbound group sessions which need backing up is very
inefficient: we have to search through the whole list.
Here, we change the way they are stored so that we can maintain an index of the
ones that need a backup.
* Rename functions for clarity
* Remove spurious log line
This was a bit verbose
* Rename constants for i_g_s store names
* improve log messages
* add a warning
* Rename `InboundGroupSessionIndexedDbObject.data`
* formatting
Otherwise the matrix-sdk crate can be used with an older version of
ruma-client-api which uses different types for a sliding sync type.
The sliding sync breaking change was allowed to be part of a patch
release because sliding sync is an unstable feature in Ruma.
Somehow `fmt::pretty` manages to be both overcomplicated and not flexible
enough.
The driver here is that I want to put the event "fields" on a separate line to
the message.
This PR fixes the `EventTimelineItem.is_editable()` function for polls.
Before this changes it always returned false.
Now it consider poll's preconditions for editing:
- The poll has no votes yet
- The poll hasn't an end event
Sometimes, we called this `keys query`, sometimes `/keys/query`, and sometimes,
just for variety, `keys/query`. Generally in Matrix we talk about `/keys/query`
so let's standardise on that.
Until https://github.com/matrix-org/matrix-rust-sdk/pull/2862,
`GroupSessionManager::encrypt_session_for` called `Device::encrypt` (via
`Device::maybe_encrypt_room_key`.
`Device::encrypt` is a thin wrapper for `ReadOnlyDevice::encrypt`, but it also
has an `instrument` annotation.
https://github.com/matrix-org/matrix-rust-sdk/pull/2862 short-cuts
`Device::encrypt` and calls `ReadOnlyDevice::encrypt` directly: that was
functionally fine but of course means that we no longer benefit from the
`instrument` annotation.
This PR rectifies the situation by pushing the annotation down to
`ReadOnlyDevice::encrypt`. It also adds some documentation for that function,
since we are using it in more places now.
(Longer-term, I think we should probably aim to get rid of `Device::encrypt`
altogether, but that's a refactor I don't want to take on today.)
This is useful if we ever decide to switch to X3DH for the session
establishment. It also refactors a bit the /keys/claim response handling
method.
Co-authored-by: Jonas Platte <jplatte@matrix.org>
The mocks in some notification tests mock any PUT/DELETE request without
the request to hit a certain path. These mocks then might respond to
unintended new requests the client might make.
The tests also assert a bunch of things manually instead of using
expectations when building the mock and calling server.verify().
The meaning of "null" and "undefined" were conflated by Ruma, previously. This updates to the latest Ruma, which changed the `avatar` type from
`Option` to `JsOption` and thus allowed us to distinguish both cases: `null` means the avatar has been unset, `undefined` means it's not changed
since the previous request.
Encryption doesn't usually require access to the user identity, so we can skip loading that from the store.
Unfortunately that means a fair bit of refectoring, in the form of replacing Device with ReadOnlyDevice.
This reduces the time taken to encrypt a message in a large room from about 3 seconds to 1 second.
This is a deprecated flag for element call using livekit. It was used to enable/disable matrix signalling event encryption. This is not relevant for embedded mode at all since there the hosting client is taking care of encryption.
---
* Remove E2EEenabled flag from the rust sdk.
This is a deprecated flag for element call using livekit. It is replaced
by the `password` and the `perParticipantE2EE` flag.
If `perParticipantE2EE` is set to `false`
and there is now password encryption is disabled implicitly.
Signed-off-by: Timo K <toger5@hotmail.de>
* `cargo +nightly fmt`
Signed-off-by: Timo K <toger5@hotmail.de>
* change test based on review
Signed-off-by: Timo K <toger5@hotmail.de>
* Apply suggestions from code review
---------
Signed-off-by: Timo K <toger5@hotmail.de>
Co-authored-by: Benjamin Bouvier <public@benj.me>
Somehow `fmt::pretty` manages to be both overcomplicated and not flexible
enough.
The driver here is that I want to put the event "fields" on a separate line to
the message.
Two sets of improvements to `get_missing_sessions` here:
* Currently, for any users who have an empty device list, we call `KeyQueryManager::wait_if_user_key_query_pending`, which waits up to 5 seconds for a `/keys/query` response. (Arguably, we should be waiting longer: it is not unusual for such a request to take a while.)
The problem is that that user's server may have been blacklisted, in which case we won't even be trying to do `/keys/query` resquests for that user, so we'll be waiting for something that never happens.
To fix this, let's check the failures list when deciding if we should wait for a user's devices.
* Separately, but closely related: for each user thus affected, we do the wait *in series*. This is a bit silly: there is no point waiting 50 times. We can parallelise the work.
Fixes#2793.
---
* Move `get_user_devices` to `IdentityManager`
... since it's going to need to interact with the `FailuresCache` which is
stored there.
* `get_user_devices_for_encryption`: don't wait for failed servers
* `get_missing_sessions`: wait for users in parallel
Rather than waiting for each pending user in series, do all the waits in
parallel.
* Changelog
* Update crates/matrix-sdk-crypto/src/identities/manager.rs
* Address review comments
Includes the following bugfixes:
- Fix the name of the fallback text field for extensible events in
`RoomMessageEventContentWithoutRelation::make_reply_to_raw()`
- Allow to deserialize `(New)ConditionalPushRule` with a missing `conditions`
field
- Fix deserialization of `claim_keys` responses without a `failures` field
It doesn't matter at the moment as the only test using `test_json::MEMBERS`
does not rely on the event being valid, but it shows this error
nonetheless:
```
2023-11-10T08:54:29.920782Z DEBUG receive_members{room_id="!hIMjEx205EXNyjVPCV:localhost"}: matrix_sdk_base::client: Failed to deserialize member event: missing field `room_id` at line 1 column 297 event_id="$151800140517rfvjc:localhost"
```
and https://spec.matrix.org/v1.8/client-server-api/#get_matrixclientv3roomsroomidmembers
says it is a required key.
- Log URI instead of separate homeserver, path
- Always log query parameters (not just for sliding sync)
- Only log request size for requests that could have a body based on the
HTTP verb
Otherwise, it's possible for the `NotificationClient` to be destroyed *after* the ffi `Client`, and then the hack introduced in the previous commit won't work.
A detached task was spawned to react upon session changes, and that task captured a clone of the current `Client`.
This caused a leak of the `Client`, because that task would never get aborted, and would not stop by itself.
The fix here consists in having `Client::set_delegate` return a task handle that needs to be stashed by the FFI
users, and cancelled when the Client gets out of scope. This fixes the leak, by removing the last reference onto
the Client.
Then, when dropping the Client, we have to drop the Stores in it. These stores may be sqlite-based stores, which
make use of deadpool. Deadpool has a sync wrapper that will call `block_on` in a `drop` method, and as such it
requires to be in the scope of a tokio runtime to run properly. To avoid breaking all abstractions and giving
access to the inners of the `Client`, the hack used here to properly be in a runtime when dropping the stores is
to replace the inner sdk `Client` in the FFI `Client::drop` method (and replace it with a dummy client that is
minimally configured and will use in-memory stores).
RoomInfo is often passed around by value, including in futures where
holding types with a large stack size is especially problematic¹.
It might make sense to move the actual data of (Base)RoomInfo into
an inner struct that is always held inside a Box or Arc, but this change
should have most of the benefits of that while being a bit simpler.
¹ https://github.com/rust-lang/rust/issues/69826
* crypto(fix): don't hold the cache lock while waiting on a user key query
Fixes#2802. The lock was only useful to sync the database and the in-memory cache for the users awaiting a key query request.
So it's possible to slightly tweak the API by moving the method from `SyncedKeyQueryManager` to non-synced `KeyQueryManager`, and require a
`StoreCacheGuard` (i.e. the owned lock, so we can manually drop it when we feel like so).
I've looked at all the other methods, and they do require the cache for writing into it and the store.
At the limit we could also move `SyncedKeyQueryManager::users_for_key_query`
into `KeyQueryManager`, but the lock in there is hold for a very short-time, so it shouldn't be an issue.
* Add test for the key query deadlock while waiting for the response.
* Update crates/matrix-sdk-crypto/src/machine.rs
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
`Account::create_outbound_session` no longer takes an entire list of keys;
rather it takes a single key and it is up to the caller to pick a key out of
the list.
This in turn means that `SessionCreationError` loses one of its reason codes.
* Add `perParticipantE2EE` to element call url
* add ffi
* nightly fmt
* refactor to use enum + test
* rename to PerParticipantKeys
* cleanup (spelling + formatting)
Signed-off-by: Timo K <toger5@hotmail.de>
The event_type passed to `encrypt_room_event_raw` must be the one of the cleartext event, not `m.room.encrypted`. The returned event has the expected type.
In the previous situation, running the tests with `cargo test` would sometimes fail because despite appending the number of milliseconds since
the start of epoch to the user names, some user names would clash across different tests, leading to unexpected results. This fixes it by using
an actual RNG in there, so the names don't ever clash.
Instead make futures modules part of the API. It is extremely uncommon
to actually refer to a type that implements `Future` by its name / path,
so it's better if these don't show up in documentation pages that are
already large and somewhat cluttered.
There was a bug previously, that the cross-signing bootstrapping could include a signature for a device key that hadn't been uploaded yet.
This fixes it by returning a third (!) request in `bootstrap_cross_signing`, that must be sent as the second in order, and will upload
keys, if required.
Also tweaks documentation. Fixes#2749.
The store cache can be filled lazily when it's actually needed. It's not needed in this
function here, and may be needed in another function the caller of this function may call
later. No need to preemptively fill the cache here.
These functions are a bit confusing, so I wrote some comments. I also factored out
`get_user_signing_key_from_response`, to reduce duplication and (IMHO) improve clarity.
Hashes computed by `compute_session_hash` may be stored in the crypto store, and may or may not
be encrypted; make them slightly more secure by using a more secure hash. Also use sha2 for
the logged hashes that are useful for debugging OIDC issues.
… allows the FFI type MessageLikeEventContent to hold custom msgtypes,
which is useful for notification.
Also allows sending custom msgtypes, as long as no fields other than the
msgtype itself and body are needed.
This PR adds a ffi binding for sending voice messages in the legacy format (before extensible events). It also makes minor changes in the `matrix-sdk` crate to accomodate this change.
* Add send_voice_message api
* Update ruma-events dependency
* Fix attachment info mapping
* Remove unstable dependency in attachment.rs
* Bump ruma-events
* Fix matrix-sdk Cargo.toml
* Fix formatting issues
* Refactor Voice case
* Remove clone from AttachmentInfo
* Remove duplicate code
* Remove unused imports
* Fix formatting issue
* Rename update function
This includes the requests that are sent to the matrix client (i.e.
a request to send a matrix event) as well as requests that are sent to
the widget.
The difference between this and `Action` is that `Action` is rather
"low-level" (i.e. it has a generic `SendToWidget(String)` variant),
whereas this API is a high-level API that we will use internally to
conveniently issue and `await` for outgoing responses.
This code path could never be taken:
- the only way to set the cross_process_token_refresh_manager is from calling the same function
- this function is guarded against a lock, so it's not reentrant
- this function's only internally called, during `restore_session` or after a successful login:
both will set a session, hence those functions would fail beforehands if another session had
been set earlier.
#2587 introduced a breaking change in the serialized format of `RoomInfo`, changing the `event`
field format in particular, without a migration. As a result, this lead to users state stores
being invalidated for containing incorrect data, and thus being logged out.
This mitigates the issue by making sure that the LatestEvent type is either deserialized as its
newer format (aka `SerializedLatestEvent`) or its older format (aka `SyncTimelineEvent`). This
way, we maintain compatibility with the previous format. We always serialize to the new one, so
we'd only run this migration once.
- `content` is now of type `RoomMessageEventContentWithoutRelation`,
the relation was previously always overwritten if set
- `add_mentions` is now inferred from `content.mentions`
This adds a new `StoreTransaction` type, that wraps a `StoreCache` and a `Store`. The idea is that it will allow write access to the `Store` (and maintains the cache at the same time), while the `Store::cache` method will only give read-only access to the store's content.
Another new data type is introduced, `PendingChanges`, that reflects `Changes` but for fields that are properly maintained in the `StoreCache` and that one can write in the `StoreTransaction`. In the future, it wouldn't be possible to use `save_pending_changes` from the outside of a `StoreTransaction` context.
The layering is the following:
- `Store` wraps the `DynCryptoStore`, contains a reference to a `StoreCache`.
- When read-only access is sufficient, one can get a handle to the cache with `Store::cache()`.
- When a write happens, then one can create a `StoreTransaction` (later, only one at a time will be allowed, by putting the `StoreCache` behind a `RwLock`; this has been deferred to not make the PR grow too much).
- Any field in the `StoreCache` will get a method to get a reference to the cached thing: it will either load from the DB if not cached, or return the previously cached value.
- Any field that can be written to will get a method to get a mutable reference in the `StoreTransaction`: it will either load from the cache into a `PendingChanges` scratch pad, or return the scratchpad temporary value.
- When a `StoreTransaction::commit()` happens, fields are backpropagated into the DB *and* the cache.
Then, this `StoreTransaction` is used to update a `ReadOnlyAccount` in multiple places (and usage of `ReadOnlyAccount` is minimized so as not to require a transaction or cache function call as much as possible). With this, the read-only account only exists transiently, and it's only stored long-term in the cache.
Followup PRs include:
- making the `ReadOnlyAccount` not cloneable
- remove inner mutability from the `ReadOnlyAccount`
- add a `RwLock` on the `StoreTransaction`
Part of https://github.com/matrix-org/matrix-rust-sdk/issues/2624 + https://github.com/matrix-org/matrix-rust-sdk/issues/2000.
---
* crypto: Replace some uses of `ReadOnlyAccount` with `StaticAccountData` and identify tests
* crypto: introduce `StoreTransaction` to modify a `ReadOnlyAccount`
* crypto: introduce `save_pending_changes`, aka `save_changes` v2
* crypto: Start using `StoreTransaction`s to save the account, get rid of `Store::save_account` + `Account::save`
* crypto: use `StoreTransaction` to save an account in `keys_for_upload`
* crypto: use `StoreTransaction` and the cache in more places
* crypto: remove `Account` from the `Changes` \o/
* crypto: remove last (test-only) callers of `Store::account()`
* crypto: move `ReadOnlyAccount` inside the cache only
* crypto: use `ReadOnlyAccount` and `Account` in fewer places
whenever we can use `StaticAccountData` in place.
* crypto: make tests rely less on OlmMachine
* crypto: Don't put the `ReadOnlyAccount` behind a RwLock just yet
+ clippy
Stores may race when performing writes, since in `save_changes` some data is pickled, and live across await points (it could be stale after an
await point). To prevent multiple threads racing when calling into `save_changes`, a new lock is introduced there.
* support creating emotes
adds an emote param to message_event_content_from_markdown and
message_event_content_from_html so that clients can create
m.room.message events with msgtype emote (on the assumption
that the msgtype returned in the RoomMessageEventContentWithoutRelation
is immutable, and so can't be overridden by the client)
* lint
* switch to separate methods per review feedback
This patch adds a `sender_profile: Option<MinimalRoomMemberEvent>`
field. Thus, `cache_latest_events` now receives an
`Option<&StateChanges>` and an `Option<&Store>`. The `StateChanges`
is used to read the most recent sender profile, otherwise the sender
profile will be fetch from the storage.
This patch adds a new `LatestEvent` type. It wraps the
`SyncTimelineEvent` that was used before. The idea is to add more fields
onto this `LatestEvent` struct, but this patch starts easy by using
`LatestEvent` everywhere where it's required.
This patch updates `all_rooms` to require the `m.room.member` state set
to `$LAZY`. That way, it's more likely to get the `m.room.member` event
associated to the latest event without needing to sync all the members.
This is to enable entirely local stacks of Element X to work correctly. It's mostly seamless (no ffi changes) because it ties into `ClientBuilder.insecure_server_name_no_tls()`.
---------
Co-authored-by: Benjamin Bouvier <public@benj.me>
Before this commit, a call to get_notification would fail if decrypting failed on the first attempt, because `Room::decrypt_event` will hard-fail if the key wasn't found.
Sorry, this commit is a bit big. What happened is that I've pulled a thread: put a `StaticAccountData` there, look at caller; it seems to use only a
static account too, so keep up.
The only contender was the `OwnUserIdentity` data structure which really wants to sign things. Lucky for us, we could pass it a `ReadOnlyAccount`
from a store, in those cases, since we always had a `Store` hanging around; in the future it'll read it from the store cache, which is somewhat
identical.
We might need to rely on the data in the DB to already be corrected
before using it for another migration
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
- Bumps ruma to include https://github.com/ruma/ruma/pull/1665
- Enables ruma `compat-arbitrary-length-ids` flag instead of using the now deprecated `lax-id-validation`
Using a static lock would prevent two Client instances to call the
method at the same time.
We're going to assume that people won't have multiple Client instances
for the same user, in which case a static lock would have been helpful.
Co-authored-by: Jonas Platte <jplatte@matrix.org>
Instead of using three separate object stores, use a single one with some structured objects and indexes.
Fixes#2605 (or at least, should make whatever is going wrong much more obvious).
Consists of a series of commits which should be reviewable on their own.
Implement a `tracing_subsciber` `Layer` which sends output to the javascript
console.
Obviously, this only works on the wasm32 target.
This is lifted from `matrix-rust-sdk-crypto-wasm`, so that we can use it in
tests etc for other crates.
Fix a bug which caused `get_outgoing_secret_requests` to return no data.
This is exposed as a public function, and when called that way it does *not*
expect the key to be escaped and encrypted.
An encryption state request was failing, but before the patch, subsequent
requests would end up in success, while this wasn't the case. The new failure
introduced by this patch was a real one, because the encryption state was
not mocked as part of this test (which tries to send a message to a homeserver).
Ruma used to handle this during deserialization when the field was
an Option.
Especially meaningful when computing the room name, where
an empty string means the name is not set.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Adds a dummy fuield to `UnstableVoiceContent`, otherwise uniffi
will generate an empty Kotlin data class which breaks compilation
(Kotlin data classes must have at least 1 field).
Upstream bug: https://github.com/mozilla/uniffi-rs/issues/1760
Without this feature:
```sh
$ cargo check -p matrix-sdk --features experimental-sliding-sync
Checking matrix-sdk-base v0.6.1 (/Users/hwhost/Development/Element/matrix-rust-sdk/crates/matrix-sdk-base)
error[E0433]: failed to resolve: could not find `poll` in `events`
--> crates/matrix-sdk-base/src/latest_event.rs:7:5
|
7 | poll::unstable_start::SyncUnstablePollStartEvent, room::message::SyncRoomMessageEvent,
| ^^^^ could not find `poll` in `events`
error[E0599]: no variant or associated item named `UnstablePollStart` found for enum `AnySyncMessageLikeEvent` in the current scope
--> crates/matrix-sdk-base/src/latest_event.rs:40:68
|
40 | AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::UnstablePollStart(poll)) => {
| ^^^^^^^^^^^^^^^^^ variant or associated item not found in `AnySyncMessageLikeEvent`
```
With the feature, everything goes well.
This patch removes `SlidingSyncList::room_list_filtered_stream.
Of course, the only place where it was used,
`RoomList::entries_with_dynamic_adapters` now use `.filter` from
`VectorSubscriberExt`. It's basically less code.
This patch adds `Option<ImageInfo>` as an argument of
`Room::upload_avatar`.
To achieve that, this patch implements
`TryFrom<ImageInfo> for ruma::events::room::avatar::ImageInfo`.
The `TimelineError` type only contains error variants used for
`ImageInfo`, `AudioInfo`, `VideoInfo`, `FileInfo` and so on. It's never
used for something strictly related to the `Timeline`.
This patch then renames `TimelineError` to `MediaInfoError`.
The `MissingMediaInfoField` variant becomes `MediaField`. The
`InvalidMediaInfoField` becomes `InvalidField`.
Use `SharedObservable::set_if_not_eq` instead of `::set`, so that it
doesn't update the limit to the same value, which would be unnecesary in
this case.
This patch removes the `Mutex` around `Subscriber<Option<u32>>` inside
the `RoomListDynamicEntriesController`. The `Mutex` was necessary to
get a mutable reference, so that `Subscriber::next_now` could have been
used. However, it's not necessary to use `new_now` in this particular
context. We can use `get` instead, which take an immutable reference,
thus removing the need for the `Mutex`.
A `Mutex` has a non-negligeable cost. This function can be used in a
critical hot path, and must as fast as possible.
This patch updates `chunk_large_query_over`. This function works great
when it's required to chunk some data over a query. However, if the
data don't need to be chunked, it is possible to get a quicker path that
doesn't involve 2 `Vec` allocations, nor a `split_off` of one `Vec`, nor
a `Vec::extend` (which move data in memory). The quicker path, which is
also the most likely to be hit, simply does a single `Vec` allocation,
and that's it.
* chore(oidc): put impl of OIDC server behind a trait and add tests for the OIDC flow
chore(oidc): add first tests for login/AuthorizationResponse
chore(oidc): add tests for finish_authorization/finish_login/refresh_access_token
chore(oidc): add test for logout
chore(🤷): clippy/fmt
* chore(oidc): move account management test to the new `tests` module
* chore(oidc): address review comments
Prior to this patch, we were using 2 constants to define the
sync indicator delays: `SYNC_INDICATOR_DELAY_BEFORE_SHOWING` and
`SYNC_INDICATOR_DELAY_BEFORE_HIDING`. After some discussions with
some users, it appears that it's desirable to make these values
parameterizable.
Thus, this patch updates `RoomListService::sync_indicator` to accept 2
parameters: `delay_before_showing` and `delay_before_hiding`. The patch
also updates the FFI bindings.
In `RoomList::entries_with_dynamic_adapters`, we were using the
`Limit::dynamic` constructor. It builds a `Limit` stream adapter where
the limits come from a `Stream`. The immediate impact is that this
constructor does only return a new `Stream`, but it cannot return
the initial items of the vector. Thus, it wasn't possible to reset
the final `Stream` with a `VectorDiff::Reset { values }`. Instead, we
were emitting a `Clear` (from this code) then a `Append` (from the
`Limit` stream). Sadly, for some client apps, like Element X iOS, these
2 diffs (`Clear` and `Append`) result in a “rendering blink”.
Hopefully for us, now there is new constructors for `Limit`! One of them
is `Limit::dynamic_with_initial_limit`, which returns the initial items
along with the new `Stream`. That's perfect, we can reset the final
`Stream` with `Reset { values }` again, thus removing the “UI blinking”
effect in Element X iOS.
This patch switches `Limit::dynamic` to
`Limit::dynamic_with_initial_limit`, and updates/simplifies the tests
accordingly.
`RoomListService::new_internal` were creating the `invites` sliding
sync list, with a cache. It is not a good idea. It should not be cached.
Let's remove that :-).
`RoomListLoadingState` was initialized with the `NotLoaded` state. This
is always true… except when there is a cache! If the cache contains
N rooms, the loading state should be `Loaded { … }`. Then the next
sync (if any) will update the loading state to `Loaded { … }` with
an adjusted number of rooms or something like that, but semantically
speaking, the presence of the cache should result in a `Loaded` state.
This patch fixes this behavior, and also adds the tests.
For some room lists, the number of entries can be gigantic. For example,
some accounts have 800, 2500, or even 4000 rooms! It's not necessary for
the client/app to display all the 4000 rooms. First, it can create some
performance issues, second, nobody will scroll 4000 rooms to search for
a particular room :-). Such users are more likely to use a search bar or
something equivalent. The idea is that `RoomListService` will continue
to sync all the data, but only a _limited_ version of it will be shared
to the client/app.
This patch takes `RoomList::entries_with_dynamic_filter`, and improves
it to include this (dynamic) limit.
This patch renames `RoomList::entries_with_dynamic_filter`
to `::entries_with_dynamic_adapters`. It now returns a
`RoomListDynamicEntriesController`, which is a renaming of
`DynamicRoomListFilter`. Basically, the “dynamic filter” becomes a
“dynamic controller” because `RoomList::entries_with_dynamic_adapters`
manages more than a filter. It now uses
`eyeball_im_util::vector::DynamicLimit` to dynamically limit the size
of entries. And that's the major idea behind this patch.
`RoomListDynamicEntriesController::set` is renamed `::set_filter`, and 2
new methods are introduced: `add_one_page` and `reset_to_one_page`.
A _page_ is like a chunk of room entries we want to view or add. When
doing `next_page`, the limit increases to `old_limit + page_size`. The
`reset_pages` method resets the `limit` to `page_size` only.
This adds a cross-process lock for refresh to work correctly.
We want to coordinate token refresh across multiple processes. For that, we're using a cross-process lock, and a value in the database identifying the latest session tokens that are valid (a hash of the actual tokens, for security reasons).
Whenever we run into an HTTP error indicating that the tokens have been invalidated, we try to refresh the access tokens; that's already existing prior to this PR. The novelty introduced is that we take a cross-process lock before doing so, now. Taking this lock will also load a session hash from the database, and we'll compare it against the latest "known" session hash (that the current process saved into its memory).
If there's no mismatch (i.e. the database and the currently known are the same), then we're all good and can keep going with the refresh, synchronize the hashes everywhere (in-memory and database), make sure the client is notified about it (through a new user-provided callback `SaveSessionCallback`; on iOS this will save it into the device's keychain).
Otherwise, that means another process has done a refresh under our feet. In that case, we ask an authoritative source for trusted session tokens. On iOS, they're reloaded from the device keychain; that happens through a new user-provided callback `ReloadSessionCallback`. Then, we make sure that the DB and the in-memory value recall this latest value.
An embedder who would like to make use of the cross-process locking mechanism should call `client.oidc().set_session_callbacks` and `client.oidc().enable_cross_process_refresh`. If only interested with the pings for new sessions, the client may only call `client.oidc().set_session_callbacks`.
Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/2418.
Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/2476
## Future improvements
- More testing of the whole flow. Not sure if mocking will be quite fit for OIDC, as this may require setting up an HTTPS server for the authentication code exchange and other OIDC-specific flows.
- Get rid of `SessionChange`, which duplicates in some way how a client can be notified about session changes.
---
* chore: replace manual StateMemoryStore::new with derived Default
* feat: add store backing for cross-process locking in state store
* chore: rename CryptoStoreLock to CrossProcessStoreLock
* chore: generalize cross-process lock
* feat: move the cross-process locking mechanism to the main crate
* feat: add support for cross-process store lock in the state store 🥳
* feat: implement a cross-process lock for OIDC token refresh
* chore: tweak comment + function name
* feat: make restore_session safe wrt cross-process lock
* feat: add FFI method + add mechanism to reload from keychain
* fix rename
* feat: return early when there was another process refreshed tokens
* fix FFI compile error + tweak some comments
* fix: put the reload_session callback and cross-process locks behind Arc to share them across clients
* feat: Add session retrieval to FFI.
* HACKY; KIDS DON'T DO THIS AT HOME
* chore: log if the hash from db isn't the same from the one from the returned session
* make it simpler to test OIDC token refresh
* some work, that includes fixes and a first test
* feat: require that the reload_session_callback be set at the same time as the cross-process lock
* chore: traces, traces everywhere
* fix: inherit session_change_sender when creating the notification client
* Some FFI improvements to help with tokio problems
* feat: resilient mode when DB/callback disagree about session (callback wins!)
* chore: move sender.send to the finish_refreshing function
* feat: add a save_session callback in the FFI and use it to save the session in keychain while holding XP lock
* fix test expectation after adding the check 🤷
* feat: split the ClientDelegate into two parts, including brand new ClientSessionDelegate
* chore: get rid of lease lock impl in the state store, as it's now unused
* a mix of fmt + clippy
* feat: add ctor for the crossprocessrefreshlockctx
* Include user ID when retrieving session.
Necessary as this isn't known when creating the AuthenticationService.
* yo dawg, you can't block while you block
* share auth data between parent and child client, add lock, AAAAAA this is messy
* tweaks
* feat: make the cross-process store locks generic
And move the implementation to the common crate.
* chore: upgrade some code comments to doc comments in `OngoingMigration`
* feat: implement `CryptoStore::remove_custom_value`
As it's going to be used for the OIDC PR, so as to remove a remembered hash of session tokens.
* remove unneeded remnants
* correctly wait for current request to finish
* feat: make it possible to setup session delegates on android too(?)
* put the cross process stuff in its own file
* typos 🤷
* fix: detach before sending token refresh request, to make sure the response tokens are always properly saved
* kleepee
* First round of review, thanks jonas!
* review round 2. FIGHT
* remove useless logs + avoid using deref explicitly
* more specialized error when cross-process lock is enabled without session callbacks
* fix: avoid cyclic reference between the session callback and client
---------
Co-authored-by: Doug <douglase@element.io>
Co-authored-by: Jonas Platte <jplatte@matrix.org>
When receiving a sliding sync room response for a room that had no local events in its timeline cache,
we'd mark the room as limited before, which is incorrect. It was made worse by the fact that later in
the code, we'd clear the local cache if a room was marked as limited, so this is a problem that would
repeat itself over time (assuming empty responses for that room).
This fixes it and unifies logs so there's only one log line per room, at most.
Fixes https://github.com/vector-im/element-x-android/issues/1281
Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/2540
Currently when the AuthenticationService is given updated metadata, it is ignored if a dynamic registration has already been made for a selected issuer. This PR fixes that by storing the metadata's hash and resetting the store when there is a mis-match.
Additionally it moves OidcRegistrations out of the FFI into a new authentication module in the UI crate and adds some tests.
This patch updates `Room::members` to return
`Result<Arc<RoomMembersIterator>, ClientError>`. This
`RoomMembersIterator` type is new, and is implemented in this patch too.
The idea behind this patch is to allow the bindings to “paginate” over
the list of members for a particular room, in case the room has 17k
members for example.
This patch changes the backoff strategy from `fixed` to `exponential`
when a flaky test is retried. The `count` value is also updated to
3. Finally, we try to avoid the thundering herd problems with `jitter
= true`.
This patch adds the `RoomListService::sync_indicator` method, along
with the `RoomListServiceSyncIndicatorListener` callback interface, and
`RoomListServiceSyncIndicator` enum.
This patch implements a new method: `RoomListService::sync_indicator`.
It returns a `impl Stream<Item = SyncIndicator>` where `SyncIndicator`
is a new enum with 2 variants: `Show` and `Hide`.
`SyncIndicator` is the UI equivalent of a sync spinner/loader/toaster,
that the app might want to show to the user to indicate when a _first_
request is sent and might be slow, i.e. the first response is taking
a little bit of time to come. The term _first_ may be innapropriate as
it covers the actual first sync request, but also the recovering sync
request. It means that when a sync error happened, the sync indicator
will be shown too, which is a pretty useful information for the user.
It's not because a `SyncIndicator` should be shown that it must be send
immediately onto the `Stream`. In case of a normal network conditions,
without any delay, it can lead to a “blinking” visual effect. Some
constants configure how long it takes to consider that a request is
“slow”, and that the `SyncIndicator` is necessary to be shown (or
hidden).
Since the verification status of an event can change, we need to be able to
refetch the verification status without doing the whole decryption dance.
Hence, we expose a new `get_room_event_encryption_info` method.
This was previously accepted by the compiler but is being phased out;
it will become a hard error in a future release! See https://github.com/
rust-lang/rust/issues/115010.
Now that we can decrypt on both of {single|multi} process setups (i.e. available for both android
and ios), we can enable retrying decrypting notifications by default.
Also rejigger the parameters passed to the notification client builder, so that it's always required to pass
a process setup. With that, we're one step closer to removing the retry_decryption() function and enable it
by default.
The `Timeline` batches its updates to its subscribers (e.g. a client app,
like Element X). A batch is built every time the inner state lock of
the `Timeline` is released. On the paper, it's nice; in practise, even
a read operation on the `Timeline` leads to building a new batch. This
is inefficient and consumes resources (like CPU cycles, FFI boundary
crossings, memory allocations etc.) even if there is no update to put in
the batch: this will just batch empty updates.
To avoid that, one needs to make the difference between read or write
operations onto the inner state. Only the write operations will fire a
batch.
This patch splits the `TimelineInnerStateLock::lock` method into
`::read` and `::write`. The idea is that a read-only lock doesn't
hold a clone of the lock release observer (`lock_release_ob:
SharedObservable<()>`), it will not notify the observer. Then it's only
the write lock that holds a clone of the lock release observer, and will
notify it.
This patch updates the code accordingly as best as possible.
This patch updates `RoomListService` to install the `invites` sliding
sync list from the start, along with `all_rooms`. Prior to the patch,
`invites` was installed after the first sync.
`invites` is installed in selective sync-mode with a range of `0..=0`.
After the next sync, `invites` switched its sync-mode to growing with a
batch size of `0..=19`.
This patch takes inspiration of
https://github.com/matrix-org/matrix-rust-sdk/pull/2480.
This patch adds a new `Recovering` state, which is a “transition”
between `Error` and `Terminated` to `Running`. When moving from `Error`
or `Terminated` to `Recovering`, `all_rooms`' sync-mode is set to
`Selective` with its initial range. Then when moving from `Recovering`
to `Running`, `all_rooms`' sync-mode is set to `Growing` with its
initial range again. For the `invites` list, it is reset to its initial
range when moving to `Recovering` too.
`Error` and `Terminated` now act the same.
This patch updates the `batch_size` of `all_rooms` once in growing
sync-mode to 100. It was previously increased from 50 to 200 in
12a1f25ec3. In some situations, it
generates very large payloads that can take time to be delivered to the
user with a slow network.
This patch tries to fine-tuned the `batch_size` value.
This patch reverts b2cc279279. On the
paper, it was a good idea. In practise, it has revealed a bug on the
server side. The only solution to mitigate this bug for now is to revert
the `timeline_limit` to 1.
Prior to this patch, we have increased the
`SlidingSyncListInner::room_list` capacity, to avoid trigger
`VectorDiff::Reset` as much as possible. Now that Element X is optimised
to handle larger diff, we can discrease this number, yeeeee :-).
When a user has hundreds of rooms, `RoomListService` will fetch at worst
one event per room, which can generate large responses. Let's use a
`timeline_limit=0`.
This patch first off renames `BaseClient::ignore_user_list_changes_tx`
to `::ignore_user_list_changes`.
This patch then changes the type from `Arc<SharedObservable<_>>` to
simply `SharedObservable` as this type implement `Clone`. We basically
have a double-`Arc` here.
Finally, this patch adds documentation for this field.
… by swapping the branches around. Previously, invocations meant to
match the found + not_found branch were actually hitting the other one
prior to Rust 1.71, likely due to parser supporting type ascription
(even though it was an unstable feature).
Inside `RoomListService`, the `State` enum handles the transition from
one state to another. In case of some `State::Error`, the `all_rooms`
sliding sync was refreshed, i.e. its sync-mode was reset to its initial
value.
This patch also refreshes the `invites` sliding sync list! It adds
the `ResetInvitesListGrowingSyncMode` action, and attaches it to the
`refresh_lists` actions.
New constants are added to represent default `batch_size` values for the
growing sync-mode for various sliding sync list. It could be helpful for
further maintenance.
This patch finally adds and updates the tests accordingly.
This patch changes the `batch_size` of the sliding sync list `invites`
for `RoomListService`. Previous value was 100, new value is 20.
For accounts that have a large number of invites, it won't slow the
rendering of `visible_rooms`.
Usually, when the sliding sync session expires, it leads the state to
be `Error`, thus some actions (like refreshing the lists) are executed.
However, if the sync-loop has been stopped manually, the state is
`Terminated`, and when the session is forced to expire, the state
remains `Terminated`, thus the actions aren't executed as expected.
Consequently, this patch updates the state to `Error` manually.
- Use OIDC for logout when appropriate.
- Allow server's that support OIDC but not passwords to work.
- Only sign out users if token refresh is explicitly refused.
- Expose the OIDC account URL.
- Support for RP initiated logout.
In the previous situation, we had two locks with similar responsibilities, the `response_handling_lock`
and the `position` lock. The latter *almost* covered the former's critical zone, albeit for a single
function call, which left room for a deadlock situation (latter taken, then former, then latter).
This removes the former, and extends the critical zone of the latter up to the end of the response handling,
removing the possibility of the deadlock entirely.
This reverts commit 9b811009e1.
We have realized that the server might not handle timeouts as expected.
Thus, it's hard to know the difference between network timeout and poll
timeout (resp. the server is unreachable vs. the server has nothing to
respond with). We will come back on this later.
As a sequel of the previous commit (d4cbcd397d), this patch updates
`SlidingSync` to “ignore” timeouts, i.e. timeouts aren't reported to the
caller, and aren't stopping the sync-loop.
All errors inside `SlidingSync` are stopping the sync-loop, and errors
are returned to the caller. However, in some situation, some errors
should be ignored, i.e. they should not stop the sync-loop and they
should not be returned to the caller: the sync-loop just continues to
run. This patch does that for `Error::ResponseAlreadyReceived`. More
errors will come.
Why is it annoying? When `matrix_sdk_ui::SyncService` sees an error,
it stops all the sync-loops (`RoomListService`, `EncryptionSync`…) and
restarts them properly. In the case of `Error::ResponseAlreadyReceived`,
this is a waste of time and resources. This error is an error from the
`SlidingSync` point of view, but _not_ from the caller point of view.
This patch updates the `BaseClient::process_sliding_sync` method to put
the `LeftRoom` in the `SyncResponse::leave` list.
To achieve so, this patch updates
`BaseClient::process_sliding_sync_room` to returns `Option<JoinedRoom>`,
`Option<LeftRoom>` and `Option<InvitedRoom>` (previously, it was only
`JoinedRoom` and `Option<InvitedRoom>`).
SlidingSync emits state events inside `required_state`, but _also_
sometimes inside `timeline`! This patch looks for state events inside
both entries.
The rest of the patch is composed of tests.
Imagine the following scenario:
A request $R_1$ is sent. A response $S_1$ is received and is being
handled. In the meantime, the sync-loop is instructed to skip over any
remaining work in its iteration and to jump to the next iteration. As a
consequence, $S_1$ is detached, but continues to run. In the meantime, a
new request $R_2$ starts. Since $S_1$ has _not_ finished to be handled,
the `pos` isn't updated yet, and $R_2$ starts with the _same_ `pos`
as $R_1$.
The impacts are the following:
1. Since the `pos` is the same, even if some parameters are different,
the server will reply with the same response. It's a waste of time
and resources (incl. network).
2. Receiving the same response could have corrupt the state. It has been
fixed in https://github.com/matrix-org/matrix-rust-sdk/pull/2395
though.
Point 2 has been addressed, but point 1 remains to be addresed. This
patch fixes point 1.
How? It changes the `RwLock` around `SlidingSyncInner::position` to
a `Mutex`. An `OwnedMutexGuard` is fetched by locking the mutex when
the request is generated (i.e. when `pos` is read to be put in the new
request). This `OwnedMutexGuard` is kept during the entire lifetime
of the request extend to the response handling. It is dropped/released
when the response has been fully handled, or if any error happens along
the process.
It means that it's impossible for a new request to be generated and to
be sent if a request and response is running. It solves point 1 in case
of successful response, otherwise the `pos` isn't updated because of
an error.
This patch first off renames `Client::deserialize_events` as
`Client::deserialize_state_events`. Then, this patch initially updated
the return type of this method from `Vec<Option<_>>` to `Vec<_>` to
filter out events that failed to deserialize. The patch ultimately
updates the return type of this method to `Vec<(Raw<_>, _)>`, so that
the raw state events that map to the state event are collected too (this
is required for making `Client::handle_state` to work correctly if an
event failed to deserialized). Finally, the rest of the patch updates
the code accordingly.
`matrix_sdk_ui::room_list_service::Error` has a `SlidingSync` variant to
report errors from `matrix_sdk::sliding_sync::Error` (to be more exact,
from `matrix_sdk::Error::SlidingSync`).
This patch updates the error message from `SlidingSync failed` to
`SlidingSync failed: <explanation>`, where `<explanation>` is the
`Display` representation of the inner error.
It will help to provide more details to the end-user when looking at
logs.
The Room::request_encryption_state method employs a DashMap to uphold a
lock, facilitating the de-duplication of requests directed to the server.
The de-duplication logic involves creating a fresh Mutex and embedding
it into the DashMap through these stages:
1. Generate a new de-duplication mutex.
2. Insert a mutex copy into the DashMap.
2. Acquire the mutex.
Due to DashMap's limitation of enabling map locking solely during
insertion (step 1), a race condition emerges. Consequently, multiple
invocations of the Room::request_encryption_state method might
concurrently endeavor to insert the de-duplication mutex.
This commit removes the chance of such a race. It substitutes the
DashMap with a combination of a mutex and a BTreeMap. This adaptation
permits us to lock the map throughout all three specified operations.
This commit addresses a bug in the Room::is_encrypted functionality.
The issue stems from the Room::request_encryption_state method, which
Room::is_encrypted relies on. This method incorporates a de-duplication
mechanism to ensure that only a single request for the m.room.encryption
state event is made to the server.
Due to this de-duplication mechanism, Room::request_encryption_state
follows two code paths. One path receives the state event from the
server, while the other path merely waits for the first path to complete.
The primary goal of the Room::request_encryption_state method is to
furnish the requested state event. However, because the second code path
doesn't receive any content, it returns an Option.
The problem arises when Room::is_encrypted evaluates this Option. It
erroneously determines that if the Option is None, encryption remains
inactive for the room.
To rectify this, the commit proposes that Room::is_encrypted analyze the
information stored in the in-memory RoomInfo, which is maintained by the
Room::request_encryption_state method. This approach mirrors the existing
behavior of Room::is_encrypted when it processes the code path where the
m.room.encryption state event has already been retrieved.
## Feature
There are now three tasks running in the sync service:
- the room list sliding sync task
- the encryption sync task
- a new scheduler task, that listens to messages sent by both of the previous tasks, and will take care of cleaning up tasks, stopping the services, and setting states after it received a message.
When any of the sliding sync fails, it sends a message to the scheduler task, indicating why it stopped (error or not, was it an expired session or not). Then the scheduler task will do any necessary cleanup.
`stop()` (née `pause()`) can now make use of that scheduler task as well, by sending a report requesting to stop both tasks and services. Responsibilities are now cleanly split: in particular, I like it much better that the room list task doesn't have to stop the encryption sync itself, since it's not really its duty. (And this avoids lots of code duplication to cleanup tasks and stop services.)
## Testing
This also tests the `SyncService` states. Unfortunately it's not perfectly deterministic in two places:
- we can't predict how many requests will be sent to the server (although, with the mocking server responding in 50ms, and considering a waiting time of e.g. 300ms, it should send at least two requests).
- the test `pause()` and re`start()` the sync service at some point, and then we can't guess what the next `pos` will be, in any of the syncs: it could either be the previous value (if we aborted while processing the previous response), or the next value (meaning we could finish processing the sliding sync response).
Still, it confirms at least that pausing and resuming work as expected.
---
Fixes#2382
The timeouts were a bit too agressive, according to some user logs, resulting in failing to
load the event mentioned in the notification.
Here's the rationale for the new timeouts: we're only limited by the iOS process which has *at
most* 30 seconds to process a notification.
- We're running at most 3 requests of the notification sliding sync, so that will be (1+3)*3 = 12
seconds allocated for that.
- If we've found an event but it required decryption, we're running the encryption sync up to
2 times, (3+4) seconds each => 14 seconds.
At most we're eating up 26 seconds of the entire time, leaving some ballast for the rest of
the program.
This patch optimises the previous patch by simplifying the use of
`Timeline`. If the `Timeline` exists, let's return the `latest_event`
every time: whether it is a remote _or_ a local event.
This patch updates `Room::latest_event` to return the `Timeline` local
event if any.
First off, it starts by checking if a `Timeline` exists. It won't create
it if it doesn't exist! Second, it checks whether a latest event exists.
Finally it checks whether it's a local event.
This patch also updates the tests accordingly.
A Sliding Sync `v4::Response` contains a `pos` value. It helps to identify
progress during several requests/responses. If two requests with the
same `pos` are sent, the server **must** reply with the same response,
as defined in the specification.
The corollary is: If a response contains a `pos` that has already
been received, the client **must** ignore it, otherwise it can create
duplications. For example, let a response containing sync operations,
like `DELETE` or `INSERT`, with indexes: if this pair of operations are
repeated twice with the same index, it creates a broken state.
To avoid this behaviour, this patch stores the last 20 position markers
received from the server. If a response contains a `pos` that has
already been received, a (new) error is returned. As with all the other
errors, the sync-loop is stopped, and must restarted. The past positions
survive to this restart, as for the rest of the state.
When the server replies with a `M_UNKNOWN_POS`, the `pos` and the
`past_positions` are all cleared.
This patch removes
`RoomListEntriesDynamicFilter::set_with_fuzzy_match_pattern` and
replaces it by a single `::set` method. It takes a new enum as
parameter: `RoomListEntriesDynamicFilterKind`. This enum has a
`FuzzyMatchRoomName` variant to simulate the removed method. It also
adds the `All` variant, to represent the `all` filter.
This splits the method out so the bigger chunks doesn't persist the
changes in the store.
This will be useful if we need to hijack the changes and persist them in
a different store.
This patch tries to fix https://github.com/vector-im/element-x-
ios/issues/1204 by adding the following required states in the
`visible_rooms` sliding sync list: `m.room.member: $LAZY`.
* feat(ffi): add an optional file logger
Also makes logging to stdout/logcat optional.
* WIP: stupidly duplicate code 🤷
* ffi: Get rid of duplication in tracing initialization
* feat: add OtlpTracingConfiguration too
* feat: log either to stdout on non-android or logcat on android
---------
Co-authored-by: Jonas Platte <jplatte@matrix.org>
If we were invited and joined a room, then the computation of is_direct() was happening after
joining, and depended on the presence of a global account event of type "m.direct". This event
is added by the "set_is_direct()" call thereafter, so this couldn't ever happen.
This fixes it by computing the is_direct field *before* joining the room, and then marking the
room as a DM based on that.
The full timeline event contains both the timestamp (requested in #2361) and the sender id;
now the sender id for the invite can also be contained in there, minimizing the amount of copying we're doing in the
FFI code.
Fixes#2361.
There must be at most one sliding sync notification per connection id. While we could use multiple ids, it seems bad
to do so, in terms of proxy performance; so this makes it so that there is at most once notification being handled
at a time.
- include the conn_id at the stream level, not the sync_once level (that's a child scope of the stream)\
- also include whether e2ee is enabled at this level
- shorten most static logs
- remove duplicate "sending request" logs
Even if the two issues are likely caused by the same root cause (the user not being
authenticated), they should be used in different contexts, in my understanding:
- the authentication required error should happen only during HTTP requests
- the absence of olm machine should be signalled when we try to get one and it's not been initialized yet
This has caused a bit of confusion when looking at debug traces today, so this patches fixes it.
This patch updates `RoomListItem::avatar_url` to use
`matrix_sdk_ui::room_list_service::Room::avatar_url` instead of
`matrix_sdk::Room::avatar_url`.
This patch also moves `avatar_url` before `is_direct` (so that it's the
same order as other places in the code).
This patch implements `Room::avatar_url`. It tries to calculate
the best avatar URL as much as possible. It's either the URL from
`SlidingSyncRoom::avatar_url` or from `Room::avatar_url`.
Based on https://github.com/ruma/ruma/pull/1607, this patch adds
support for `avatar` from a sliding sync response. This patch implements
`SlidingSyncRoom::avatar_url` to get the avatar URL of a sliding sync
room.
The batch subscriber exists in `matrix_sdk_ui::RoomList` (https://
github.com/matrix-org/matrix-rust-sdk/pull/2322) instead of being added
here.
We must keep the observable capacity to 4096, but the `TODO` is no
longer relevant.
Why keeping the capacity to 4096? Because if the batch subscriber (in
`RoomList`) isn't “listened” quickly, we don't want to get a `Reset`.
This patch brings a nice code simplification.
Instead of creating a new `Stream` with `tokio` based on
`Subscriber<State>`` to drain the batch subscriber for
`RoomList::entries` and `::filtered_entries`, we can _simply_ use
`Subscriber<State>` directly! It removes one dependency: `tokio-
stream`, and remove possible issues with the broadcast channel
`tokio::sync::broadcast`. The code is much simpler and straighforward.
As discussed, we think the entire UI crate should be considered experimental. We've also
observed a proliferation of feature flags there (many of those are my wrong doings, sorry).
Since the one internal user (FFI) of that crate enabled all experimental features, it seems
fine to make them all default, while not providing any extra stability guarantee based on that
action.
* feat: run a room sliding sync upon receiving a notification, to get its full content
* test: add test for the new sliding-sync in notifications \o/
* fix: try to get the push rules *after* a possibly-successful event decryption
* feat: set `is_noisy` only if we could build a push context, and test it
* feat: expose the `legacy_get_notification` in the FFI layer
* feat: retrieve events with a `/context` query
* fix: also request the client's user id's member information to get their display name
* feat: include the list of invites in the notification sliding sync
* feat: repeat the query multiple times if the event hasn't been immediately found
* chore: simplify retrying decryption
* chore: fail the sliding sync when fetching a notification if not using a memory store
* chore: update test expectations + sort invites by recency
* chore: cargo fmt
* chore: simplify getting push actions
Either they were already available in the timeline event if we had to re-run decryption, or
we manually compute them. (Previous comment was incorrect, the `push_actions` are now optional
because of another PR of yours truly.)
* fixup! chore: fail the sliding sync when fetching a notification if not using a memory store
* feat: try to handle invites correctly
* chore: build a local client with an in-memory store instead of reusing the parent client
* fix: remove dubious annotation
* feat: allow cloning a Client and modify it on the fly, use that for the notification client
* feat: get rid of the with_memory_state_store public func on ClientBuilder
* feat: include sender_id in the notification invite event
* feat: put sender's id in the `NotificationSenderInfo`
* feat: inherit the parent session when creating a notification client
* chore: reformat comments
* TMP: add logs
* chore: regenerate the olm machine when inheriting a session too
* chore: keep the parent client around for legacy_get_notification/with_context
The proxy server can (and does) redispatch unrelated lists/rooms from other sliding sync connections,
so the encryption sync would sometimes see room events. Apparently since this is not considered a bug
in the SS proxy, so we shouldn't spam error traces every time this happens.
Since `RoomList::entries` returns a batch stream, the listener
now receives a `Vec<VectorDiff<RoomListEntry>>` instead of a
`VectorDiff<RoomListEntry>`.
Only updated the macro is required here. Instead of calling
`StreamExt::now_or_never` on the `$stream`, we call `Iterator::next` on
`$entries` which is a `Vec<VectorDiff<_>>` now.
This patch uses a newly implemented `async-rx` crate, that provides
`StreamExt`. This trait provides new features on `Stream`, like
`StreamExt::batch_with` which allows to batch values generated by a
`Stream` into a `Vec<Stream::Item>`. The batch is drained based on
another `Stream`: every time a value is produced, it drains the batch
stream.
This feature is used in `RoomList::entries` and
`RoomList::entries_filtered` to batch `Stream<Item = VectorDiff<_>>`
into `Stream<Item = Vec<VectorDiff<_>>>`.
The “drainer” is a broadcast sender, which sends an (empty) value every
time the room list service state changes, so every time something
happens during a sync. Note that it even drains when the room list
service state jumps to `Error` or `Terminated`.
In `RoomListService`, there is a cache over the `Room`s to avoid re-
computing them every time the user calls `RoomListService::room`. It
also allows async operations to have time to finish while the user
jumps back to the room list and so on.
When reading this cache, which is `matrix_sdk_common::RingBuffer`,
`RingBuffer::iter` is used with `Iter::find` to find if the
`Room` exists in the cache. The cache has a size of 128 (given by
`ROOM_OBJECT_CACHE_SIZE`), which is quite large.
`Iter::find` will iterate from the oldest to the newest room in the
cache, but realistically, I reckon we need to iterate from the newest
to the oldest room. Indeed, the app user is more likely to jump between
recently opened rooms more frequently, thus saving quite a lot of
iterations for usual cases.
One may argue than in practice, a user won't open 128 rooms anyway… :-p.
Up until now, users had to listen for to-device events to check for
secrets that were received as an `m.secret.send` event.
This has a bunch of shortcomings:
1. Once the has been given to the consumer, it's gone and can't be
retrieved anymore. Secrets may get lost if an app restart happens
before the consumer decides what to do with it.
2. The consumer can't be sure if the event was received in a
secure manner.
This commit ads a inbox for our received secrets where we will long-term
store all secrets we receive until the user decides to delete them.
It's deemed fine to store all secrets, since we only accept secrets we
have requested and if they have been received from a verified device of
ours.
It's preferrable that users make use of the `App::observe_state/current_state` methods, instead
as there's another sliding sync in the `App` and it properly unifies the current state of both.
Before this patch, the inability to compute push actions would result in an empty vector of
push actions, leading to the false assumption that there's no push actions to account for,
while we were unable to compute them properly. This changes things up so that it's now the
user's responsibility to decide what to do when those push actions are missing.
Previously we would generate one-time keys, if needed, whenever we tried
to upload them. This code-path is critically missing a `save_account()`
call and we would only persist the account once we uploaded the one-time
keys.
This patch changes things up to generate one-time keys whenever we
receive new one-time key counts from the sync response. This aligns
with the way we generate fallback keys and removes the need to introduce
a new place where we persist the `Account`.
It's still possible to re-upload the same one-time keys, in the case
where the upload process succeeds on the server side but we fail to
receive the response.
Co-authored-by: Denis Kasak <dkasak@termina.org.uk>
This will limit the memory used by the cache entries (while it was unbounded before). It's now possible to do this,
since we have the `latest_room_event` handy for all the rooms; using the unbounded cache before was papering over
the lack of that feature.
We can bikeshed on the number of entries in this cache. It has to be small enough to not blow up memory (and keep
the linear search over room id fast, but it's secondary), and high enough that we don't hit the full timeline
re-build path that often.
* chore: rename EncryptionSyncMode variants
* feat: split the encryption sync modes into two different functions
* feat: make locking optional in the `EncryptionSync`
* feat: experimental notification client that retries decryption if it failed the first time
* fix: don't iloop retrying decryption
* chore: helper to convert from bool to `WithLocking`
* feat: don't loop and just retry decryption of the notification event linearly
* feat: remove unused set_notification_delegate
Dead code is dead.
* ffi: get rid of `get_notification_item` and introduce the `NotificationClient`
* fmt
* feat: don't swallow encryption sync errors when retrying notification event decryption
* keeping a tidy commit history is NP-hard
* will i ever learn
* chore: enable experimental-notification-client in the FFI crate
* test: add basic integration test for the common path
* Address first batch of review comments, thanks Jonas!
f36a5b8cd7 introduced
deserialize_events, moving the deserialization out of handle_state.
However, the new code in deserialize_events used filter_map, which
caused a deserialization failure to lead to the indices of the
serialized and deserialized events to no longer match up.
This was not caught because iter::zip also does not require that its
arguments have matching lengths, causing the mismatch to cascade for
that batch of events, and persist in the store. An application
affected by this form of corruption can, for example, call
room.get_state_events requesting events of a certain type, and getting
back events of a different type.
Fix the bug by using Option to preserve the length of
deserialize_events' return value, and add an assertion to ensure
handle_state's contract.
Signed-off-by: Vladimir Panteleev <git@cy.md>
This file claimed to have lots of tests, but actually they are not independent
- they are just one big test.
It's not ideal to have one massive test like this, but I don't really have time
to rewrite them and we should stop pretending.
and the latest few encrypted events in Room.
Use the latest event to provide a room preview, and use the encrypted
events to replace the laest event when they are decrypted.
This patch introduces a new `chunk_large_query_over` function. Imagine
there is a _dynamic_ query that runs potentially large number of
parameters, so much that the maximum of parameters can be hit. Then,
this function is for you. It will execute the query on chunks of
parameters.
`chunk_large_query_over` uses `Vec::split_off` to avoid cloning `Key`s
as much as possible. It's difficult to use references here because of
the async nature of `SqliteObjectExt::prepare`.
This patch updates `get_kv_blobs`, `get_room_infos`,
`get_maybe_stripped_state_events_for_keys`, `get_profiles`,
`get_user_ids`, and `get_display_names` to use this new function.
This patch finally adds the `repeat_vars` function to replace in a
more efficient way the `vec!["?"; n].join(", ")` pattern. There is less
memory allocations.
The `RoomList` and `EncryptionSync` API provide a better developer
experience and address concrete needs. Nobody no longer uses the Sliding
Sync bindings, so we can remove them.
For sliding sync that starts both the actual sliding sync request along the e2ee requests,
we need to make sure that, in case of failure of sliding sync, we reset `pos` as soon as possible.
With the code before this patch, the sliding sync response might be available, but the whole
processing could be waiting for the e2ee requests to finish. In the updated version, we spawn
the e2ee requests in a background task, then run the sliding sync request immediately and fail
if it failed.
Another nice benefit is that the e2ee requests won't be interrupted in the middle of processing,
if the sliding sync changed parameters and we cancelled the whole sync a bit too early.
Fixes#2206.
`RoomListServer` defines an `all_room` sliding sync list. This list
starts in selective sync-mode, then it switches to growing sync-mode.
The previous batch size of the growing sync-mode was 50. This patch
updates it to 200, because empirically it seems a better value for
perceived performance.
This patch also rewrites how `State::next` is written. No change in the
code, just comestic.
* sdk: Allow to update notification settings
* Add an event listener for push rule events
* Fix: simplify insertion and deletion of rules
* Fix: Limit rules cloning
* Fix: Unit tests
* Fix: set_room_notification_mode
* Fix: potential race condition when updating the local ruleset
* Refactor RuleCommands
* RuleCommands Unit Tests
* Fix: limit lock usage
* nit: use expression assignment by default
* nit: pass Ruleset by ownership in RuleCommands' ctor
* a few nits: use to_owned() for &str -> String; use free function for notify actions; use clone() where it's more obvious
and use explicit variants so we know we have to consider this match if we were to add a new variant later
* nit: rename RuleCommands::set_enabled to set_enabled_internal
* nit: test modules don't need to be publicized
* tidy tests
* nit: pass an owned RuleCommands in Rules::apply
* add comments/questions in Rules tests
* nit: no need to publicize the tests module
* tweak NotificationSettings tests too
* rustfmt
---------
Co-authored-by: Benjamin Bouvier <public@benj.me>
When the sync has been terminated and restarts (e.g. an app is going from the background to the foreground), then
the sliding sync is always restarting in growing mode, independently of the previous state of the sync (error or normal
termination). This is something the current sliding sync proxy can't handle nicely, because it prioritizes a change in
parameters over live data; as a matter of fact, to alleviate the burden of the proxy, we can try to only restart in
growing mode if we ran into an error (and not if we had a normal termination).
It ensures that we know exactly what's happening here. Tests are still
valid without any changes in the expected data, but it prevents a
potential breaking.
The Sliding Sync list `all_rooms` and `visible_rooms` of the `RoomList`
must have the exact same configurations for `sort`, `filters`, and
`bump_event_types`. Instead of maintaining the same code, this patch
adds a new function: `configure_all_or_visible_rooms_list` that
configures the lists exactly the same. This function also explicitely
configures `sort` so that we do no rely on the default values from
`SlidingSyncListBuilder`.
This patch configures the same `bump_event_types` for the
`visible_rooms` list than for the `all_rooms` list, so that we may be
sure that the sorting/ordering is the same between the two lists.
* ui: Move EventSendState into timeline::event_item::local module
* [WIP] ui: Make pending local echoes stick to the bottom of the timeline
* test: update more test expectations
* chore: tweak comment to slightly better reflect reality
* nit: remove else after return
* fix: the item's insert position is insert_idx, not `items.len()` anymore
* fix: look for remote echo before local echo when processing send state
Previous code assumed that the latest timeline items would be the most recent, and that
if there was a remote echo, it would always be after the local echo, because of that.
That's not the case anymore, so we must look for possibly a remote echo first, and then
if we find it, apply the late update process.
Also, there might remain a day divider added by the local echo, if it were inserted last.
I'm not sure it covers all the cases, but I've now made it so that the day divider is removed
if it was the last element.
* feat: switch strategy; keep on pushing if there's nothing in the timeline yet
* Revert "test: update more test expectations"
This reverts commit 400cc93ba7c98042a28b5e8d5042899e854f6cff.
* test: reset test expectations
* Address review comments
* fix: don't mix up latest event with any status, with latest non-failed event index
---------
Co-authored-by: Jonas Platte <jplatte@matrix.org>
For verification-by-emoji, the spec uses the word "Aeroplane" rather than
"Airplane". Element-web expects us to use the specced words (and will otherwise
complain about the lack of i18n data).
It seems like following the spec will help maintain consistency.
Common extensions are confusing, and they've included `e2ee` and `to-device` by default. This is not a sane default anymore,
now that there's the concept of `EncryptionSync`: it's either we have the encryption sync that enables e2ee and to-device +
a room list sync that doesn't, OR we have a single room list that has both.
Room List was misconfigured to always use `e2ee` and `to-device`, which was incorrect when it's running with the `EncryptionSync`
in the background. This is now removed, and properly tested.
This implements a new time lease based lock for the `CryptoStore`, that doesn't require explicit unlocking, so that's more robust in the context of #1928, where any process may die because the device is running out of battery, or unexpected flows cause a lock to not be released properly in one or the other process.
```
//! This is a per-process lock that may be used only for very specific use
//! cases, where multiple processes might concurrently write to the same
//! database at the same time; this would invalidate crypto store caches, so
//! that should be done mindfully. Such a lock can be acquired multiple times by
//! the same process, and it remains active as long as there's at least one user
//! in a given process.
//!
//! The lock is implemented using time-based leases to values inserted in a
//! crypto store. The store maintains the lock identifier (key), who's the
//! current holder (value), and an expiration timestamp on the side; see also
//! `CryptoStore::try_take_leased_lock` for more details.
//!
//! The lock is initially acquired for a certain period of time (namely, the
//! duration of a lease, aka `LEASE_DURATION_MS`), and then a "heartbeat" task
//! renews the lease to extend its duration, every so often (namely, every
//! `EXTEND_LEASE_EVERY_MS`). Since the tokio scheduler might be busy, the
//! extension request should happen way more frequently than the duration of a
//! lease, in case a deadline is missed. The current values have been chosen to
//! reflect that, with a ratio of 1:10 as of 2023-06-23.
//!
//! Releasing the lock happens naturally, by not renewing a lease. It happens
//! automatically after the duration of the last lease, at most.
```
---
* feat: implement a time lease based lock for the crypto store
* feat: switch the crypto-store lock a time-leased based one
* chore: fix CI, don't use unixepoch in sqlite and do time math in rust
* chore: dummy implementation in indexeddb, don't run lease locks tests there
* feat: in NSE, wait the duration of a lease if first attempt to unlock failed
* feat: immediately release the lock when there are no more holders
* chore: clippy
* chore: add comment about atomic sanity
* chore: increase sleeps in timeline queue tests?
* feat: lower lease and renew durations
* feat: keep track of the extend-lease task
* fix: increment num_holders when acquiring the lock for the first time
* chore: reduce indent + abort prev renew task on non-wasm + add logs
Calling `RoomListItem::full_room` creates its associated `Timeline`.
ElementX calls `full_room` to get the `is_direct`, `avatar_url` and
`canonical_alias` values for _all_ rooms in the room list. Instead of
doing so, let's add those methods directly in `RoomListItem` so that the
`Timeline` isn't created for _all_ rooms.
So. `SlidingSyncList::update_room_list()` does the following:
1. It adjust room list entries,
2. It updates the `maximum_number_of_rooms`,
3. It applies the sync operations on the `room_list` entries,
4. It updates the `room_list` entries for rooms outside the sync
operations.
SlidingSync answers with a `lists` and `rooms`. The `room_list` entries
is updated as follows: either a sync operation from `lists` has been
applied and the entries are updated, or `rooms` contains rooms that
are not updated by `lists` but that receive new events, and thus must
trigger an update in the `room_list` entries.
This patch changes a little bit the last part of this. Initially,
we were updating rooms that are part of `rooms` but absent of
`lists` sync operations, only **for the current list's ranges**.
But that's wrong! First, we have noticed a bug here: the
correct code wasn't `skip(start).take(end.saturating_add(1))` but
`skip(start).take(end.saturating_add(1) - start)`. Note a big deal, we
were iterating on more entries like it was necessary, but everything
was filtered later, so no bug, just useless computations. Second,
this `skip` and `take` is actually… useless. A room to which we have
subscribed can be out of any range, but we still want `room_list`
entries to receive an update for that particular room.
This patch fixes that once and for all.
In practise, the bug wasn't happening because if someone subscribes
to a room, its timeline is likely to be fetched, and updates will be
received, but still, this is better now from a SlidingSync strict point
of view.
This patch does the following:
1. changes `add_timeline_listener` to be async, thus removing one
`RUNTIME.block_on`.
2. simplifies the logic by adopting a more classical pattern we use
elsewhere:
* removes one `spawn_blocking`,
* let's move the `listener` into the task as a `Box` instead of
cloning it as an `Arc`.
Imagine the room list has the viewport set to the range `0..=19`. The
user scrolls quickly to `50..=69` to see something below and immediately
scrolls back to its initial position, without stopping the scroll
at any moment in between. The app using this API might update the
viewport from `0..=19` to… `0..=19`. Updating the viewport, updates the
`visible_rooms` sliding sync list. Since a list is modified, the current
sync-loop iteration is skipped over and a new one restarts, i.e. the
current in-flight request is cancelled… for nothing.
This patch prevents this situation. The current viewport ranges is
stored in `RoomListService`. `RoomListService::apply_input` used to
return `Result<(), Error>`, now it returns `Result<InputResult, Error>`.
The `InputResult` enum is a new type. It represents whether an input has
been applied or ignored, which must be differentiate from errors.
So now, if the viewport changes and it's not different from the previous
value, `InputResult::Ignored` is returned.
According to the spec, when a device receives a verification request and wants
to refuse it, it should send the `m.key.verification.cancel` to the originating
device, rather than broadcasting it.
This opens up a possible race condition where an embedder calls the method twice, generating two new rooms,
but then it's fine as long as they don't cache them somewhere, which we expect they don't.
That avoids recreating a timeline object every time a user calls `RoomListService::room()` with the same room id, so that should speed up
operations like fetching the latest event for rooms we've already entered before.
This patch changes the capacity of the internal buffer of
`ObservableVector` for `SlidingSyncList::room_list` from 16 to 4096.
With an increased capacity, we reduce the probability to send a
`VectorDiff::Reset` to subscribers. `Reset` are happening quite often
for apps using `matrix-sdk`, and it impacts their performance. This
patch tries to improve this situation. This `room-list` contains
`RoomListEntry`, which is can cheap in terms of memory, compared to the
impact of sending `Reset`s too often. That's a tradeoff.
* bindings: Ignore silently malformed userId in migration of tracked users
* Clean: collect will now never fail
Co-authored-by: Benjamin Bouvier <public@benj.me>
* crypto: implement more primitives for the MemoryStore to work in tests
* crypto: change the shape of the `CryptoStoreLock` API
In particular:
- make the lock work across multiple threads of the same process trying to acquire it,
using `num_holders`.
- add a mechanism to get the lock only once (for the NSE process, in case the main app
had acquired the lock before).
* client: add a cross-process crypto store lock, enable it with `Encryption::enable_cross_process_store_lock`
* client: make `preshare_room_key` a critical section of the cross-process lock
* sliding sync: make it possible to define different timeouts for a `SlidingSyncInstance`
This will be handy for the NSE process on iOS, which has very little time to wait for the proxy's responses.
* feat: implement the `EncryptionSync` API (renamed from `Notification` API)
* fixup! client: add a cross-process crypto store lock, enable it with `Encryption::enable_cross_process_store_lock`
* feat: allow disabling e2ee / to-device in the RoomList API
* feat: use same SS id for main/NSE process, reload to-device token from disk before each encryption sync
* fix: better error handling if restoring the to-device token failed
* feat: add logs for the locking functions
* test: add a few tests for encryption sync
* feat: add `reload_caches` method in the `EncryptionSync` + FFI bindings
* chore: clean up FFI loop
* encryption sync: Remove unused errors, specialize some errors
* feat: include termination reason in the encryption sync loop
* feat: add more logs
* chore: fmt + clippy + doc
* comment: precise only in the presence of another process
* Tweak `room_list` APIs to include `_with_encryption` variants
* chore: rustfmt
This patch moves `RoomListService::entries` and `::filtered_entries`
inside `RoomList`. It also implements `RoomList::new`. Finally,
it implements `RoomListService::all_rooms` and re-implement
`RoomListService::invites`.
Basically:
```rust
// 1.
room_list.entries().await?
// 2.
room_list.invites().await?
```
becomes:
```rust
// 1.
room_list.all_rooms().await?.entries()
// 2.
room_list.invites().await?.entries()
```
`all_rooms` and `invites` both return a `RoomList`.
This patch introduces `State::Error`, which replaces
`State::Termianted`. And a “new” `State::Terminated` state is added.
They do the same, but their semantics is different. `Error` is when an
error happened, and it's fine to restart the sync again. `Terminated`
is when a termination has been reached (either because SS sync-loop
has ended naturally, or because `RoomList::stop_sync` has been called),
in this case, it may not be fine to restart the sync automatically for
example.
This patch renames `State::FirstRooms` to `State::SettingUp`, and
`State::Running` as requested by users.
It hides the “inner Sliding Sync logic”, the Sliding Sync lists etc.
This patch implements `RoomList::stop_sync` on the FFI bindings.
Technically, it's no more useful to store the `TaskHandle` of the
`sync`, but it's still here, in case of.
This patch implements `RoomList::stop_sync`. The goal is twofold:
1. It forces to stop the syncing, thus putting the state-machine into
the `Terminated` state, which is semantically better than “stop
polling the `sync`'s `Stream`”.
2. It literally forces to stop the syncing. It cancels pending futures,
it cancels in-flight HTTP requests etc. It's a more robust way to
stop the `RoomList` sync.
It has an account field besides the issuer field.
Also store it as immutable, the data used for authentication will be
stored in another variable.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
`SlidingSync::sync` had a legacy behaviour when a `M_UNKNOWN_POS`
error was received from the server: It was resetting `pos` and sticky
parameters before re-running a sync-loop iteration, hoping to get a
valid response from the server after that. This retrying
mechanism was running up to 3 times in row (represented by the
`MAXIMUM_SLIDING_SYNC_SESSION_EXPIRATION` constant) before stopping the
`sync` for real.
While it seemed a good idea, it actually brings numerous problems:
1. Each iteration in the sync-loop generates a new request, thus making
the `ranges` of the requests to move forwards. For `SlidingSyncList`
that are in `Selective` sync-mode, there is no problem, but for
`Growing` or `Paging` sync-modes, the `ranges` increased. Thus,
when a `SlidingSync` session expires, instead of returning to
the caller to do something clever (like what `RoomList` does: start
again with a small range so that the “first” sync after a session
expiration is guaranteed to be fast), it was running larger and
larger requests, up to 3 times.
2. `M_UNKNOWN_POS` _is_ an error. Yes, `SlidingSync` must reset `pos`
and must invalidate sticky parameters, but the `sync` must be
stopped, and the error must be returned so that the caller can do
something about it. Until now, this error was likely to be missed by
the caller.
3. This legacy mechanism was forbidding `SlidingSync::sync`'s caller to
do something with this special error. And since the caller was blind to
this error, it disallowed more smart error management.
This patch removes this legacy retry mechanism entirely. When
`M_UNKNOWN_POS` is received from the server, `pos` is reset and sticky
parameters are invalidated as before, but the error is returned like any
other error.
This patch renames and updates an existing test that was testing sticky
parameters invalidation on `M_UNKNOWN_POS`, to include some assertions
on `pos` and to ensure that the sync-loop is stopped accordingly.
This is the first PR for splitting the sync loop into two. This offers a new high-level API, `NotificationApi`, that makes use of a separate `SlidingSync` instance which sole role is to listen to to-device events and e2ee; it's pre-configured to do so. That means we're not force-enabling e2ee and to-device by default for every sliding sync instance, and as such we won't either generate Olm requests to the home server in general.
In the future, this new high-level API will hide some low-level dirty details so that its can be instantiated in multiple processes at the same time (lock across process, invalidate and refill crypto caches, etc.).
An embedder who would want to make use of this would need the following:
- a main sliding sync instance, without e2ee and to-device. Using the `matrix_sdk_ui::RoomList` would be the best bet, at this time.
- an instance of this `matrix_sdk_ui::NotificationApi`, with a different identifier.
Note that this is not ready to be used in an external process; or it will cause the same kind of issues that we're seeing as of today: invalid crypto caches resulting in UTD, etc.
Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/1961.
Usually it's impossible to create a Olm session from a pre-key message
twice. The one-time key that should be used for the 3DH step will be
used up and we're going to throw a `MissingOneTimeKey` error.
This used to be true and unproblematic until we added fallback keys,
these keys will not get discarded immediately after they have been used
once.
This means that a pre-key message, for which we already have a Session,
but decryption for it fails, might create a new Session overwriting the
existing one which will essentially reset the ratchet.
This implements a value-based lock in the crypto stores. The intent is to use that for multiple processes to be able to make writes into the store concurrently, while still cooperating on who does them. In particular, we need this for #1928, since we may have up to two different processes trying to write into the crypto store at the same time.
## New methods in the `CryptoStore` trait
The idea is to introduce two new methods touching **custom values** in the crypto store:
- one to atomically insert a value, only if it was missing (so, not following the semantics of `upsert` used in the `set_custom_value`)
- one to atomically remove a custom value
Those two operations match the semantics we want:
- take the lock only if it ain't taken already == insert an entry only if it was missing
- release the lock = remove the entry
By looking at the number of lines affected by the query, we can infer whether the insert/remove happened or not, that is, if we managed to take the lock or not.
## High-level APIs
I've also added an high-level API, `CryptoStoreLock`, that helps managing such a lock, and adds some niceties on top of that:
- exponential backoff to retry attempts at acquiring the lock, when it was already taken
- attempt to gracefully recover when the lock has been taken by an app that's been killed by the environment
- full configuration of the key / value / backoff parameters
While it'd be nice to have something like a `CryptoStoreLockGuard`, it's hard to implement without being racy, because of the `async` statements that would happen in the `Drop` method (and async drop isn't stable yet).
## Test program
There's also a test program in which I shamelessly show my rudimentary unix skills; I've put it in the `labs/` directory but this could as well be a large integration test. A parent program initially fills a custom crypto store, then creates a `pipe()` for 1-way communication with a child created with `fork()`; then the parent sends commands to the child. These commands consist in reading and writing into the crypto store, using a lock. And while the child attempts to perform these operations, the parent tries hard to get the lock at the same time. This helps figuring out a few issues and making sure that cross-process locking would work as intended.
This patch renames `SlidingSyncState` to `SlidingSyncListLoadingState`
because:
1. It's about a list information,
2. It's about the loading state, not a generic state.
The MSC3575 says that if no `ranges` for a list is provided, it defaults
to `0..=99`. We don't want that! This patch sets the default value to
`0..=19` for the `visible_rooms`.
* feat: lazily generate and include the transaction id, only if it's useful
* chore: add a small `LazyTransactionId` wrapper that ensures it's only created once
---------
Signed-off-by: Benjamin Bouvier <public@benj.me>
`RoomList::state` provides a `Subscriber` to the `State`. This patch
modifies the way the state is tested, to ensure that there is always
an update broadcasted even if the state is the same (e.g. if the state
moves from `CarryOn` to `CarryOn`).
This patch updates `RoomInner::timeline` and `::sneaky_timeline` to
`AsyncOnceCell<Arc<Timeline>>`. Adding `Arc` allows to copy the timeline
if necessary more easily.
This patch updates `Room::add_timeline_listener` to return a `RoomTimelineListenerResult`, a new type defined as:
```rust
pub RoomTimelineListenerResult {
items: Vec<Arc<TimelineItem>>,
items_stream: TaskHandle,
}
```
It is not possible to cancel the `items_stream` by cancelling the
`TaskHandle`. Without this, dropping `Room` won't drop the `Timeline`,
as a clone is moved inside the spawned task. As a consequence, it will
endlessly call the listener.
What the FFI user wants is to subscribe a listener to the
timeline updates. This feature is already supported by
`room_item.full_room().add_timeline_listener()`. So we can safely
remove `RoomListItem::timeline` as we are sure it's never going to be
used for now.
It's possible to do `room.inner_room().room_id()` but it's just simpler
to call `room.id()`. This patch adds this shortcut.
It's especially useful for FFI users, where creating a “full
room” (`room_item.full_room().room_id()`) may be expensive.
This patch adds the `subscribe` and `unsubscribe` method on `Room`.
To achieve that, `Room` receives an `Arc<SlidingSync>`, as the
subscription methods are on `SlidingSync`. It's just easier to put them
on `Room` for the API consumer. It makes the API also more elegant.
Finally, the patch adds the appropriate test.
Since stripped and non-stripped room infos use the same types,
the separation is not necessary anymore.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
… and simplify wasm spawn implementation.
This reduces the differences on wasm vs. non-wasm. Of course it's still
possible to rely on details of the different error types, but at least
both implement a few common traits:
`Debug`, `Display`, `Error`.
Look through m.room.member events when processing Sliding Sync
responses, finding those that refer to our user's membership, so that we
can recognise which rooms we have left.
Instead of storing `sliding_sync_proxy` inside the `Client`, this patch
updates the code to store it inside `matrix_sdk::Client` directly.
That way, there is unique place where to store the sliding sync proxy
URL.
This patch changes `matrix_sdk::Client::sliding_sync_proxy` to be a
`std::sync::RwLock<Option<Url>>` instead of `tokio::sync::RwLock<_>`.
It means that all methods reading or writing this field are sync instead
of async, which makes the code a lot more easier. Having an async-aware
lock wasn't necessary here.
* chore: add comments explaining what the position markers are
* feat(sdk): add scaffolding for sticky parameters
* test(sdk): add tests for the sticky parameters API
* feat(sdk): include the bump_event_types in the sticky parameters
* WIP: add TODO comments for deeply nested sticky parameters
* test(sdk): add extra test for sticky parameters
* feat: add extensions in the sticky parameters + test request generation + test since token
* chore: fix merge + get rid of unused inner extensions
* feat: use sticky parameters for lists too
* chore: Introduce the StickyManager, a reusable way to manage per-data sticky parameters
* chore: move the sticky scaffolding to its own module
* chore: clippy + fmt
* chore: get rid of `update_to_device_token`
* review: append SlidingSync prefix to Stick* data structures
* address review comments
* Replace nbsp with regular space
* Get rid of a write-access to a lock (because thanks inner mutability!)
* Add an integration test when losing `pos` in sticky parameters, and avoid `block_on`
Signed-off-by: Benjamin Bouvier <public@benj.me>
This patch implements `RoomList::rooms` to fetch one room. The type
name is `RoomListRoom` to avoid any collision with an existing `Room`
type already.
`RoomListRoom` implements `name`, `timeline` and `latest_event`.
By returning a `Subscriber`, one can call `Subscriber::get` to get the
inner value. Thus, having `state` and `state_stream` is useless. It's
possible to return have `state` which returns `Subscriber`, and we get
one value for two usages.
The example in MSC3575 shows a room with invite_state property, but also
timeline, joined_count etc. properties. That may or may not be very
likely in practice, but I think it makes sense to check for all these
properties even when the room is an invite, and use them if they are
present.
Added some comments based on my understanding of MSC3575. Previously we
had a FIXME about how we were setting the room state to invite or join
based on the presence or absence of the invite_state property. I think
this behaviour is correct, so I added some comments explaining why.
The functional change here is to note that we call
store.get_or_create_stripped_room to find/create the stripped room, and
this actually deletes any room of this ID that is in the store, so our
later call to store.get_room would always fall back to getting the
stripped room, which we already have, so there was no need to do the
get_room call at all.
This patch uses a new `async-once-cell` crate to make `Room::timeline`
and `Room::sneaky_timeline` by making them lazy. Their values are
computed lazily, on-demand, when calling the `Room::timeline` method or
`Room::latest_event`.
This extension was enabled by both ElementX apps by default, along with the e2ee / to-device extensions that will be handled
by the new Notification API. To maintain the current behavior, it's important to re-enable this extension before getting
RoomList in production.
Signed-off-by: Benjamin Bouvier <public@benj.me>
By default, get_media_file will attempt to use a default directory
for temporary files and directories. Unfortunately, there might not be
such a thing on some older Android devices, which use per-application
directories.
For this specific case, an optional temporary directory parameter
is added to the `get_media_files` parameters, so one can provide their own
temporary directory path.
This new parameter is expected to be set at least on Android; other platforms
should work just fine without it.
Signed-off-by: Benjamin Bouvier <public@benj.me>
This was a temporary replacement I've made, waiting for a PR of mine to be merged, and it's been merged since then. Oh well.
Signed-off-by: Benjamin Bouvier <public@benj.me>
* sliding sync: infer the storage key from the loop id and user id
* chore: rename `sync_id` to `id`
* chore: check that the sliding sync id is less than 16 chars
* chore: rejigger the storage key creation logic
Now the prefix is visible only in the `format_storage_key_prefix` function, and other `format_storage_key` function will be based off that.
* chore: update sliding sync README with API updates and fix outdated information
* chore: clippy + fix test
Signed-off-by: Benjamin Bouvier <public@benj.me>
This patch handles errors from the Sliding Sync loop properly.
When an error is received, the `RoomList` state machine enters the
`Terminated` state. Immediately after that, the sync stream is stopped.
When the sync stream is restarted, the next state will be calculated.
When the current state is `Terminated`, the next state is the state
that led to the `Terminated` state. To avoid having a “first” huge sync
(imagine a room list of 1000 rooms, you don't want to “resume” from an
error with a first sync for 1000 rooms), lists are partially “reset”
depending of the state where the machine is in.
An important test has been added to test all possibles cases with errors
and list' states.
First off, `SlidingSync::sync_once` no longer returns
`Option<UpdateSummary>` but `UpdateSummary`. It simplifies the rest of
the patch.
Next, `SlidingSync::sync` returns either `Ok(…)` and continues to sync,
or it returns `Err(…)` and it stops the sync stream immediately. No more
error could be returned with the stream to continue syncing.
Finally, the `UnknownPos` error no longer emits an err, but silently
skip over the sync-loop until the threshold is reached; which in this
case will generate a proper error.
This patch changes `RoomInner` to have an `room:
Option<matrix_sdk::room::Room>` field to `room: matrix_sdk::room::Room`.
`room` is not longer an `Option`.
Then, it's easier to have a new `timeline: Timeline` field, along with a
`sneaky_timeline: Timeline` field. Both are used by the new
`Room::timeline()` and `Room::latest_event()` method.
This patch implements `RoomList::room`, which returns a `Result<Room>`:
the room ay or may not exist.
A new `Room` type is created. It wraps an `Arc<RoomInner>` type, so that
it's cheap to clone it.
The `RoomInner` type owns a `SlidingSyncRoom` and
`matrix_sdk::room::Room` type. If some API or data are missing on
the former, the latter acts as a backup. This is the case for the
`Room::name` method which makes it best to return a name to the caller.
This patch lands the first design of the `Input` API. An `Input` is
something external that can be understood by the `RoomList` state
machine. The first inpuput is `Viewport` to change the “viewport” of the
`RoomList`, which translates to the change of the ranges of one specific
Sliding Sync list.
This patch implements `RoomList::update_entries_stream_filter` which
allows to… change the… entries stream filter. This patch also provides a
test for that. How nice it is.
This patch adds a new method on `SlidingSyncList` named
`room_list_filtered_stream`, which uses the new `eyeball-im-util` crate
with its new `FilteredSubscriber` type.
This patch does several things.
First, `SlidingSync::on_list` is now async, and accept async closures.
Second, `SlidingSync::lists` and `::rooms` are behind an `AsyncRwLock`
instead of a `StdRwLock`. The rest of the patch updates the consequence
of this.
This patch replaces a panic (`Option::expect`) by a `tracing::error`
log.
Imagine the Sliding Sync proxy responds with the following payload:
```json
{
"pos": …,
"lists": {
…: {
"count": …,
"ops": [
{
"op": "INSERT",
"index": 0,
"room_id": !foo:bar.org",
}
]
}
},
"rooms": []
}
```
It's an invalid response, as it will update the room list entry (because
it's present in `lists.$list.ops`), but the room won't be created (it's
absent in the `rooms`).
This situation creates a panic in the patched code. We don't want to
crash if the server replies with invalid data.
Ultimately, we should check that the sync operations are valid regarding
the rooms' updates.
This patch uses `FutureExt::now_or_never` so that we don't wait on the
future to be resolved to get a result. If we have a bug in our code and
the test expects a value, the test will hang forever, which is not a
desired behavior. With `now_or_never`, this situation cannot happen.
The only reason why a sender can fail to send a message is because there
is no receiver. In the current design of `SlidingSync`, there is only
one receiver active per sync-loop. Thus, calling `SlidingSync::add_list`
may fail if `sync` has not been started. Hence the need for a new
`internal_channel_send_if_possible` method which will never fail: it
will send a message is possible, otherwise it won't do anything.
This turns more functions infallible.
`tokio::sync::broadcast` is interesting as it's possible to
generate receivers per subscribers. Being able to do that allows
us to remove the new for an `AsyncLock` around the receiver in
`SlidingSync::internal_channel`, and it can even remove the need to hold
a receiver entirely!
Another improvement is that new receivers can't receive past messages,
so we no longer need to drain the internal channel.
Another improvement is that the sender' `send` method is synchronous!
Which helps to make many functions no longer `async`.
Something weird is happening with ElementX iOS. When
`SlidingSync::stop_sync` is called, the internal message `SyncLoopStop`
has time to be sent in the internal channel, but iOS decides to suspend
the code before the sync-loop processes it. When ElementX decides to
start the sync-loop again, it immediately processes the `SyncLoopStop`
message, and… stops the sync-loop.
This patch ensures that `SlidingSync::sync` drains the internal channel
before starting the sync-loop for real. `tokio::sync::mpsc::Receiver`
type has no `drain` method, so this patch implements its
own logic by calling `try_recv` in a loop, until it returns
`Err(TryRecvError::Empty)`.
* feat: introduce SlidingSyncSelectiveModeBuilder
* feat: get rid of `CannotModifyRanges` \o/
* chore: rustfmt
* chore: remove scope in test
* chore: move request generator update to its own mod, gets rid of `set_ranges`
* chore: add comments on the request generator methods
* chore: sink one range_end definition into the match arm that matters
* ffi: remove unused `add_range` method
* test: update incorrect test expectation
* chore: make clippy happy
* address first review comments
* review: don't reuse the ranges field for two things
* chore: use a builder pattern for sliding sync selective mode
* address review comments + CI
Signed-off-by: Benjamin Bouvier <public@benj.me>
In case it's not obvious to drop the `Stream` returned by
`SlidingSync::sync` immediately to “stop” the sync-loop, one can use the
new `stop_sync` method to do achieve the same result.
The `VerificationRequest` object is used to control the flow of the
verification but only up to a certain point.
Once we start handling of different specific verification flows (i.e.
SAS or QR code verification) the `VerificationRequest` object creates a
child object of the Verification type.
This patch adds a new `VerificationRequestState` variant called
`Transitioned` which holds the child verification object as associated
data.
This makes it much simpler to go through the whole verification flow by
allowing users to just listen to the `VerificationRequest::changes()`
method.
We suspect that there might be too many internal messages being pushed, causing a deadlock on the senders' side.
This attempts to increase the value of the buffer to give it more leeway.
Signed-off-by: Benjamin Bouvier <public@benj.me>
The `SlidingSyncListInner::timeline_limit` and `::ranges` fields were
observable (behind `eyeball::Observable`). It was actually useless
as those fields were never exposed to the public API, thus it was
impossible to subscribe to them.
This patch cleans up that. `timeline_limit` and `ranges` are no longer
observable.
The comment and the check in the code were contradicting each other; the comment was wrong, and the code was right, so there's that.
Signed-off-by: Benjamin Bouvier <public@benj.me>
This patch removes our dependency to libolm completely. This should
allow WASM targets to use the backups_v1 feature of the
matrix-sdk-crypto crate as well.
First off, this patch changes `pushd(…)` to `pushd(…)?` so that errors
are propagated.
Second, instead of assuming that all crates live in `crates/`, let's
allow to precise a prefix, like `crates/` or `bindings/` directly in the
“folder” path of `args`.
Changing the sync-mode of a list on-the-fly is necessary to optimise the
new `RoomList` API. For example, we can start with a list in a selective
mode, a range of `0..=50` and a `timeline_limit=0` to fetch the
beginning of the room list, and then _change_ the sync-mode to growing
to continue to sync the room list in the background.
Today, this is done with 2 lists and a merge of the lists, but
(i) this is error-prone, (ii) this is not optimal. Thank to
`SlidingSyncList::set_sync_mode`, merging lists is no more necessary,
thus removing a class of bugs in client's code.
This patch replaces `sync_mode` on `SlidingSyncListBuilder` by
`sync_mode_selective`, `sync_mode_paging` and `sync_mode_growing`, which
removes the need to use `SlidingSyncMode` directly.
This patch also removes `batch_size`, `room_limit` and `no_room_limit`
as it's now arguments of `sync_mode_paging` and `sync_mode_growing`.
`SlidingSyncMode` was previously declared as:
```rust
enum SlidingSyncMode {
Selective,
Paging,
Growing,
}
```
Now, the new declaration is:
```rust
enum SlidingSyncMode {
Selective,
Paging {
batch_size: u32,
maximum_number_of_rooms_to_fetch: Option<u32>,
},
Growing {
batch_size: u32,
maximum_number_of_rooms_to_fetch: Option<u32>,
}
}
```
First off, it helps to remove the `full_sync_batch_size` and
`full_sync_maximum_number_of_rooms_to_fetch` methods and fields from
`SlidingSyncListBuilder`. It was containing default values in case of.
That was useless and needed to be fixed. Also, calling a `full_sync_*`
method with the `Selective` mode had no effect, which could disturb
the user. Well, now everything is clean on that front.
Second, `SlidingSyncListRequestGenerator` no longer has constructors,
but a `From<SlidingSyncMode>` implementation. However, the constructors
now live in `SlidingSyncMode` with `new_selective`, `new_paging` and
`new_growing` methods which are helpers. All in all, it makes creating a
`SlidingSyncListRequestGeneator` simpler: just call `sync_mode.into()`
and boom.
Finally, this patch removes the `default_with_fullsync` list builder
helper, which makes no sense at all.
`SlidingSync::subscribe`, `::unsubscribe` and `::add_list` are
now async. `subscribe` has been renamed to `subscribe_to_room` and
`unsubscribe` to `unsubscribe_from_room`.
`SlidingSyncRoom::subscribe_and_add_timeline_listener` has
been removed, and replaced by a new `::subscribe_to_room` and
`::unsubscribe_from_room` methods (the `::add_timeline_listener` method
already exists).
`TaskHandle::finalizer` is no longer necessary, thus this code has been
cleaned up.
This bug has been found by @bnjbvr, all the credits go to him. I've just
added some comments around his code.
Prior to this patch, the room unsubscription buffer
(`SlidingSync::room_unsubscriptions`) was reset before the request was
sent. So if something went wrong, the next request would not include the
room unsubscriptions.
This patch updates this behavior. First, it replaces `Vec` by `HashSet`
to avoid a O(n^2) look up.
Second, a copy of room unsubscriptions used by the request is kept, so
that it can be used to cherry-pick which room unsubscription to remove
from the buffer once a response from the server is received. It's
important to not clear the entire room unsubscriptions buffer as more
unsubscriptions could have been inserted meanwhile.
This has slightly drifted from the initial design I thought about in the issue.
Instead of having `build()` be fallible and mutably borrow some substate (namely `BTreeMap<OwnedRoomId, SlidingSyncRoom>`) from `SlidingSync` (that may or may not already exist), I've introduced a new `add_cached_list` method on `SlidingSync` and `SlidingSyncBuilder`. This method hides all the underlying machinery, and injects the room data read from the list cache into the sliding sync room map.
In particular, with these changes:
- any list added with `add_list` **won't** be loaded from/written to the cache storage,
- any list added with `add_cached_list` will be cached, and an attempt to reload it from the cache will be done during this call (hence `async` + `Result`).
- `SlidingSyncBuilder::build()` now only fetches the `SlidingSync` data from the cache (assuming the storage key has been defined), not that of the lists anymore.
Fixes#1737.
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
Currently, the `body` of a `SignatureUploadRequest` includes a spurious
`signed_keys: {...}` property in which the actual content is wrapped. Fix that.
We were using `txn_id` as a way to identify the running stream. Now
things are cleaner so we can remove this “debug tool”. This class of
problems should not appear anymore. For the record, the biggest problem
was the following: It was possible to start multiple `stream`s at the
same time, and thus a stream could receive a response sent by another
stream. Since we no longer need to restart SlidingSync anymore (i.e. no
need to start multiple `stream`s at the same time), this problem should
not happen.
Prior to this patch, the `v4::Response`, aka the SlidingSync response,
was processed by the client and transformed into a `SyncResponse` into
`sync_once` before being passed to `handle_response`. Now it's done in
`handle_response`.
This is not mandatory _now_, but it will be helpful in a close future.
* chore: use RangeInclusive instead of raw start/end integers for ranges in sliding sync
* chore: have timeline_limit use a u32 instead of a Ruma UInt
* chore: Remove all the `Into<u32>` generics on timeline_limit and ranges APIs
* chore: introduce a `Bound` type alias for u32
Signed-off-by: Benjamin Bouvier <public@benj.me>
This patch changes `SlidingSyncRoom` to move all its fields inside an
inner type `SlidingSyncRoom` behind an `Arc`, so that cloning is cheap,
and all clones are sharing the same state.
A `SlidingSyncList` has a state, which can be either `NotLoaded`,
`Preloaded`, `PartiallyLoaded`, or `FullyLoaded`.
A `SlidingSyncList` can be reset, either manually when
`SlidingSyncList::reset` is called, or when a range is updated for
example.
Resetting a list is modifying its state. Prior to this patch, the state
was set to `NotLoaded`. However, it's not entirely true. This patch
updates this behavior to the following rules:
* When the state is `NotLoaded`, it's kept at this state as nothing has
happened yet,
* When the state is `Preloaded`, the list is restored from the cache,
but nothing has happened yet too, so it's kept at this state,
* When the state is `PartiallyLoaded` or `FullyLoaded`, it means some
(or all) updates have been done, so the new state is `PartiallyLoaded`
after the reset.
The list' state is used mostly by the client to know whether a loader
should be prompted to the users. The ranges are modified when the
users scroll inside the room list for example: scrolling in the room
list doesn't imply the state should go to `NotLoaded`. Some data
have potentially be loaded, so changing the ranges should result in a
`PartiallyLoaded` state. It seems more logical once explained like this.
`invite_state` is managed when the Sliding Sync response is processed/
handled by the `Client`. Inside `SlidingSyncRoom`, it's not necessary to
maintain it.
This patch creates a constant to represent the
maximum number of timeline event to put in the cache:
`NUMBER_OF_TIMELINE_EVENTS_TO_KEEP_FOR_THE_CACHE`. Verbose, but easily
understandable.
This patch also renames a few variables.
This patch simplifies `SlidingSyncRoom::timeline_queue` from
```rust
Arc<RwLock<ObservableVector<SyncTimelineEvent>>>
```
to
```rust
Vector<SyncTimelineEvent>
```
First, we don't need to be observable. It's never observed since
it's private, and even privately, it's never observed, there is no
subscriber.
Second, no lock is required as updates happen synchronously.
Third, `Arc` is not necessary. We want each clone of `SlidingSyncRoom`
to not share any state across them.
Finally, this patch simplifies the iterator + `.push_back` by a simple
`.extend` to update the `timeline_queue`. Behind the scene, `impl Extend
for Vector` actually does an iterate + `.push_back`; let's keep our code
simple though.
`SlidingSyncRoom::is_cold` is not well-named. This patch introduces an
enum, named `SlidingSyncRoomState` which contains more detailed state:
`NotLoaded`, `Preloaded`, and `Loaded`.
The use of `AtomicBool` is also removed. Thus, we no longer need an
`Arc` for the state, which makes `Clone` more obvious: the state is not
shared across clones anymore.
A `SlidingSyncRoom` receives an Ruma
`api::client::sync::sync_events::v4:SlidingSyncRoom`. This value is
stored in the `SlidingSyncRoom::inner` field. From here, some getters
like `name()`, `is_dm()` etc. are using the `inner` field to compute a
result. There was one exception though: `prev_batch`. This value is part
of `v4::SlidingSyncRoom` but it was copied and updated in its own field:
`SlidingSyncRoom::prev_batch`.
I was wondering why. Turns out, there is no reason. Its getter
`prev_batch()` is public, but it's not used by the FFI bindings, so
basically nobody uses it (as this project is experimental as the time of
writing, we know our users).
This patch removes the `SlidingSyncRoom::prev_batch` field.
This patch also removes the `SlidingSyncRoom::prev_batch()` getter.
This patch finally removes the `FrozenSlidingSyncRoom::prev_batch` field
too.
Prior to this patch, to check a room exists, it was removed from the
collection, then re-added if it exists. Instead of doing this `remove`
+ `insert` dance, we can simply use `get_mut`, which also returns an
`Option`. This patch does that, in addition to rewrite the code to use a
`match` instead of a `if` + `else`.
`SlidingSync::subscribe` and `SlidingSync::unsubscribe` will cancel in-
flight requests, i.e. the `SlidingSyncInternalMessage::ContinueSyncLoop`
will be sent in the internal channel, just like what `SlidingSyncList`s
already do when a parameter is changed.
If the redaction already happened server-side, it is a bug for the
server to even still have access to the non-redacted form.
We don't accept the server data in this case, and also log a warning.
`on_built` is a method that registers a closure. This closure is called
when a list is built by `SlidingSyncListBuilder::build`. It receives
a `SlidingSyncList` and returns a `SlidingSyncList`, the list can be
updated or returned as is. It allows to configure a `SlidingSyncList`
right after it's built. `SlidingSyncBuilder::build` is responsible to
finalize the configuration, and to build all the lists. Once they are
built, the state is restored from the cache. If one wants to configure
a list before the state is restored from the cache, `once_built` will
serve well.
The timestamp value of the Notification was not reliable since it was the
timestamp in which it was generated internally, not the timestamp of when the
event was sent, now we instead expose the `origin_server_ts` of the
TimelineEvent.
`is_read` is also very unreliable, so it's just removed for now.
Currently this takes a `CrossSigningKeyExport`, but that's not a class you can
construct from the JS side. Instead, let's just pass in the individual keys.
Using the #[derive(uniffi::Object)] on certain types make them not show up
in the .aar file when building for Android. Could not determine why that is,
but removing the derive macro, and adding the empty interface declarations
inside the UDL, makes them show up.
We can still use #[uniffi::export] and #[uniffi::constructor].
Signed-off-by: Simon Farre <simon.farre.cx@gmail.com>
Currently, we can't pass the response to a `SigningKeysUploadRequest` into
`mark_request_sent`, which apparently we should be able to do.
So, we need to make `SigningKeysUploadRequest` have a `type` like the other
`OutgoingRequest`s, and add support for `SigningKeysUploadResponse` to
`OwnedResponse`.
Because since SlidingSync no longer restarts, the behaviour is really
different. The current way the tests are written cannot assert the full
behaviour. We need to rewrite this test suite entirely.
The unknown filter can be difficult to match with MembershipState,
depending on the store implementation.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Storing as bitset is not future-proof.
Just fix the latest migration. We assume no-one used it yet.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
The MPSC channel that has been introduced in recent commits is now used
in this patch. The `SlidingSync::stream` method can be controlled via
the `internal_channel`.
This patch updates `SlidingSync::stream` to use the `tokio::select!
` macro to select any future that resolves first between the
`internal_channel` receiver, or `SlidingSync::sync_once`. Fairness is
biaised as the `internal_channel` has the priority.
This mechanism is already used by this patch: `SlidingSyncList::reset`
will send the `SlidingSyncInternalMessage::ContinueSyncLoop` to
“continue”… well… the sync loop, i.e. it will cancel in-flight waiting
for a response.
This entire mechanism removes the need to “stop” and “start”, i.e.
“restart” the `SlidingSync::stream` method manually, which was the
source of many bugs. Now everything is controlled internally.
Prior to this patch, `SlidingSync::add_list` was taking a
`SlidingSyncList`. However, we need to inject more data when building
the list, so let's modify `add_list` to take a `SlidingSyncListBuilder`
instead. It's even better for the user as calling `build()` isn't
necessary anymore.
It's not possible to clone a `SlidingSyncList` anymore. Why? Because
it's not correct. Prior to this patch, it was possible to add a list
to a `SlidingSync` instance, then add a clone of the same list to
another `SlidingSync` instance. Weird behaviors could happen, but more
importantly, for the next Sliding Sync design we are working on, it
could lead to gigantic bugs.
Removing `Clone` from `SlidingSyncList` makes the code simpler.
For example, `SlidingSyncList.inner` no longer needs an `Arc`, or
`SlidingSync::stream` no longer needs to clone all the lists.
The `SlidingSyncBuilder` now has a new method: `bump_event_types`, to
configure the `bump_event_types` HTTP request parameter. This value
is passed to `SlidingSync`, which is used to build the `SlidingSync`
request.
The format of the outbound group session struct has changed. We nowadays correctly rotate the group session if we can't restore it, but it's still good to avoid logging the error in this case.
An error is currently thrown if loading an outbound group session fails.
This error may only affect loading outbound group sessions, while other
store operations continue to work fine. As a result, the user is unable
to send messages, but can still use the application without any problems.
Since outbound group sessions are rotated frequently, we can simply
rotate the session if loading fails. However, if the error is related to
a more serious storage issue, persisting the newly rotated outbound group
session will also fail. If the error is specific to outbound group
sessions, the user will be able to continue using the application without
interruption.
The SQLite crypto store uses rmp_serde to serialize all the data we're
going to store. This works nicely for most things, one exception to this
is the OutboundGroupSession type.
The OutboundGroupSession type stores to-device requests to ensure that
the session doesn't get used before it is shared with the whole group
and to ensure that the to-device requests get restored if the
session gets restored after an application restart.
The to-device requests type critically contain `Raw<AnyToDeviceEvent>`,
the `Raw` type here being the serde_json::Raw type. rmp_serde seems to
serialize this just fine, but later deserialization fails.
We're avoiding the issue by using serde_json to serialize the
OutboundGroupSession.
Those methods are public, but never used by our current users.
Moreover, there is some big overlaps. For example, `storage_key`,
`cold_cache` and `no_cache` do the same thing: They update the
`storage_key` field. Or `add_fullsync_list` is just a helper that is
never used except in the tests etc.
The test does the following:
1. Create 2 (identical) lists,
2. Do a sync.
3. Assert that the 1st and 2nd lists are receiving an update,
4. Add a 3rd (identical) list,
5. Do a new sync,
6. Assert that 3rd list is receiving an update.
This last step is wrong. All lists should receive an update as they
are identical.
This patch updates a test to ensure that `room_list` receives “diff”
for rooms that are modified by a sync operations, but also by another
updates (like a new event).
The `SlidingSyncList.room_list` field is used to store the list of rooms
for a particular Sliding Sync list. It's used by the user to receive
“diff”s when a room sees its position modified. For example when a new
room receives a message, it “climbs back up” the entire room list to
be at the top place. Another example is when a new room is created,
some rooms will move around to give the new room a space. All those
moves will create “diff”.
However, the user also expects to receive a “diff” when a room has
received some updates, even if it's position doesn't change. For
example, when a room is already at the top of the list but receives a
new message: It has received an update, but its position stays the same.
This specific latter feature was implemented before, but it has been
removed by accident in https://github.com/matrix-org/matrix-rust-sdk/
pull/1699 (more specifically in https://github.com/matrix-org/matrix-
rust-sdk/pull/1699/commits/861a05be69a566d9a4ad125dc6ecb418d2b3210f).
At that time, it was not clear why the code was filtering for specific
filled room entires, to set the filled room entries, at the same
position. Zero comment, zero test. I didn't consider this as a feature
but as a bug.
So this patch re-introduces this feature. Hopefully in a more optimal
way. If a room has already triggered a first “diff” because of a
position change, it won't trigger a second “diff” because it has
received an update (which was the case before).
The `SlidingSyncList::handle_response` method has also been renamed
`update`, as it does update, whenever it comes from.
The code has been commented and documented to explain this feature.
Existing tests have been updated, especially for `apply_sync_operations`
which now ensure the `rooms_that_have_received_an_update` collections is
updated accordingly. Another test has been updated specifically to test
the “diff”s received by `room_list`.
This patch also re-uses the `format_storage_key_*` functions to use the
same key as the `restore_sliding_sync_state` function. It fixes a bug
where the keys were mismatching.
This PR is self explanatory.
Exposes said function to the FFI. Defined in the .adl file,
and defined to throw `ClientError`
Signed-off-by: Simon Farre <simon.farre.cx@gmail.com>
Co-authored-by: Damir Jelić <poljar@termina.org.uk>
In some cases, restoring client state from backups may cause Olm sessions
to become corrupted, resulting in a backward ratchet. As a consequence,
the receiving side is unable to decrypt messages. To address this issue,
the failed side initiates a new Olm session and sends a dummy encrypted
message.
However, this dummy message also creates a new Olm session on the
sender's side. To ensure that both sides use the same new session, we
must sort sessions by their creation timestamp before encrypting new
messages.
Although the session list was sorted correctly previously, we selected
the wrong side of the list, resulting in an outdated session being
selected instead of the newest one. This patch resolves the issue by
selecting the newest Olm session and adds a test to the session getter
logic to prevent reintroduction of this bug.
Prior to this patch, when `SlidingSync` was built with a storage key,
the storage was read to find a previous `SlidingSync` version that
was in a cache. To put a `SlidingSync` instance in the cache, it
was serialized to JSON. When one reads from the cache, the value is
deserialized. A problem happens when the internal representation of
`SlidingSync` and other types (e.g. `SlidingSyncList`) have changed (due
to an SDK update for example): Loading a previous state from the cache
results in an error.
After discussion with the teams, clients don't want to deal with
obsolete cache values. In such a scenario, (i) they cannot interact
with the cache to remove the obsolete entry, and (ii) their only
possibility is to log out and log in the user again. What an unfriendly
user experience!
This patch modifies this behaviour. When loading a `SlidingSync` type or
a `SlidingSyncList` type from the cache, 3 cases are now handled:
1. The cache entry exists and has been successfully deserialized: in
this case, we do our business.
2. The cache entry exists, but it wasn't possible to deserialize it: the
cache entry is declared as obsolete, and the entire `SlidingSync`
cache is cleaned up, so all entries for `SlidingSync` and
`SlidingSyncList` are removed from the storage,
3. The cache entry doesn't exist: we do nothing particular.
Note that if one cache is obsolete, all cache entries are removed.
The `SlidingSync::stream` method no longer resets the lists everytime
it is called. If one wants to reset the lists, they need to call
`SlidingSync::reset_lists`.
`SyncOp::Invalidate` means _invalidating_ a particular range. When a
room is `Filled`, it becomes `Invalidated`, when it is `Invalidated` it
stays `Invalidated`, and when it is `Empty` it stays `Empty`.
Before this patch, `Empty` was becoming `Invalidated`, which apparently
is a bug.
This patch also fixes out-of-bound accesses, and adds many tests.
Finally, this patch renames `update_state` to `update_room_lists`.
`SyncOp::Insert` means _inserting_ a new room ID. The `rooms_list`
contains all possible rooms (based on `maximum_number_of_rooms`, a value
returned by the server). Here, inserting = setting
`RoomListEntry::Filled` at a particular index of `rooms_list`, that's
it.
The previous code was doing very complex stuff, like removing things
around the `index` if something etc. It was using the requested ranges
(the range passed to the request) etc.
Applying `SyncOp` should be simple and is focused on updating
`rooms_list` only: the requested ranges have nothing to do here.
This patch also prevents against out-of-bounds acccesses, which wasn't
the case before.
This patch prevents out of bounds acceses for `SyncOp::Delete`, and adds
more tests.
This patch also removes a cast from `UInt` to `u32` to `usize`. It's now
from `UInt` to `usize` directly.
First off, this patch renames `ops` to `sync_operations` and `room_ops`
to `apply_sync_operations`.
Second, the `SlidingOp::Sync` was creating an out-of-bounds access
depending of the range present in the server's response. For example, if
the `rooms_list` contains 5 elements (because the
`maximum_number_of_rooms` is set to 5), and the server replies with:
```json
{
"op": "SYNC",
"ranges": [3, 17],
"room_ids": […]
}
```
the previous code was setting a new `RoomListEntry` at indices `3..=17`,
whilst the `rooms_list` contains only indices from `0..=4`. That's
annoying.
The previous code was also counting the number of `room_ids` for
nothing, just to execute the iterator that was applying the actual
changes in a `map`. Well, everything was fishy.
This patch updates the code to protect against an unexpected server's
reply by raising an `Err`. This patch also adds tests.
The `updated_rooms` argument was passed to `find_rooms_in_list` to
update the `room_list`: the update is setting the filtered room list
entry to `RoomListEntry::Filled`.
_But_, `find_rooms_in_list` was already filtering rooms which are
`Filled`. So it does… nothing: it filters rooms which are `Filled` to
update them to `Filled`.
So we can remove `find_rooms_in_list` because it becomes useless. And we
can remove `updated_rooms` too.
The `rooms_list` is updated by `rooms_ops` itself. Let's keep
modifications in one unique place.
The `update_state` method of `SlidingSyncListInner` has basically
2 cases:
1. For an initial response,
2. For other responses.
The code between the 2 cases were almost identical. Or, they could be
identical. The few exceptions are:
* In the first case, the `rooms_list` updates were taking the
form of a `VectorDiff::Append`, while the second case, it was a
`VectorDiff::PushBack`.
* In the first case, the `is_cold` flag was set to `false`.
It's fine for the clients to receive only a `VectorDiff::Append` event
only. So let's make it uniform.
And it appears that the `is_cold` field is now private, and never read
anywhere else. So… it's… basically useless. We can remove it! It was
previously used here to know which flow to use, but since we can make
both flows identical, its role becomes insignificant.
`create_range` and `SlidingSyncList::next_request` return a `Result`
instead of an `Option`. It was a legacy from the old `impl Iterator` of
`SlidingSyncListRequestGenerator`. But actually, when `create_range`
fails, it must return a `Err` value, not a `None` value.
And since it's a regular error, `sync_once` can propagate the
error. Thus, it is no longer necessary to “update” the list of
`SlidingSyncList`. It's not necessary to remove some items in this list
when the `next_request` method can no return a value: it always returns
a value, except when a error happens.
First off, `set_ranges`, `set_range`, `add_range` and `reset_ranges`
take a `&[(U, U)]` instead of a `Vec<(U, U)>` when a vector was needed,
or takes a `(U, U)` instead of 2 arguments when it was needed.
Second, all those methods now return a `Result<(), Error>`. The
`Error::CannotModifyRanges` is raised if the chosen `SlidingSyncMode`
doesn't allow to modify ranges. Basically, `Selective` does allow a
user to modufy the ranges, but there is no real ranges with `Growing` or
`Paging`. Let's make it an explicit error.
Finally, `SlidingSyncListInner` has 2 methods: `set_ranges` and
`add_range`, but without any “ranges check”; `SlidingSyncList` does call
the inner methods, but does the checks.
The `SlidingSyncListInner::find_rooms_in_list` method can be greatly
improved by using the `Iterator` API.
It was already using `Iterator::skip`, great! Let's continue by using
`Iterator::take` instead of using a stop index.
And let's use `Iterator::enumerate` to avoid using a mutable index.
`SlidingSyncList::find_room_in_list` and
`SlidingSyncList::find_rooms_in_list` aren't useful (and never used) by
the public consumer of this API. Let's remove it. They are only used for
internal purposes.
Before, the `update_request_generator_state` method was emitting an
`error!` log, but not an error. But that's clearly an error!
This patch updates the method to return a `Result<(), Error>`, and adds
a new variant to `Error`.
There are problems with `SlidingSyncListRequestGenerator`:
* It's part of the module API,
* It contains a clone of `SlidingSyncList`.
To create a `SlidingSyncListRequestGenerator`, one has to
call `SlidingSyncList::request_generator`. It was done in
`SlidingSync::stream`. The problem is that it clones `SlidingSyncList`.
So theoritically it is possible to create multiple request generators
for the _same_ list, and use them to send many requests and to update
the _same_ list with multiple responses. This is utterly error-prone and
can lead to really complex bugs to discover.
Moreover, it's a lot of clones. Cloning a
`SlidingSyncListRequestGenerator` isn't cheap as it means cloning a
`SlidingSyncList`. Moreover, cloning a `SlidingSyncList` isn't cheap as
it means cloning the entire struct.
Having `SlidingSyncListRequestGenerator` inside the module API also
makes the code of `SlidingSync` more complex (why having to deal with
lists and request generators at the same time? we must be very careful
to maintain both side by side? what if a list is removed but not its
request generator? and so on).
So. This patch simplifies all that.
First off, it extracts all the fields of `SlidingSyncList` into a
`SlidingSyncListInner` struct. Then, `SlidingSyncList` has only one
field: `Arc<SlidingSyncListInner>`. Boom, it's now cheap to clone it.
Second, `SlidingSyncListRequestGenerator` is only a
struct with constructors, but there is no extra methods.
`SlidingSyncListRequestGenerator` _no longer_ contains a
`SlidingSyncList`, and doesn't no need a list at all to work. It's just
values.
Third, `SlidingSyncList` holds a `SlidingSyncListRequestGenerator`, and
only one.
Fourth, `SlidingSyncList` (and `SlidingSyncListInner`) now has methods
to handle the internal request generator. The initial `impl Iterator for
SlidingSyncListRequestGenerator` becomes a simple
`SlidingSyncList::next_request` method.
Fifth, previously, the `SlidingSyncList::handle_response`
was never called directly by `SlidingSync`. It was called by
`SlidingSyncListRequestGenerator`! How confusing! `SlidingSync` called
`SlidingSyncListRequestGenerator::handle_response` which was calling
`SlidingSyncList::handle_response`. Now, the flow is more natural:
`SlidingSync` calls `SlidingSyncList::handle_response` and that's it.
Sixth, the `SlidingSyncList::handle_response` is now composed of 2
parts: updating the list itself, and updating the request generator. It
was kind of the case before, but onto two different types.
It was unclear which types were updating `SlidingSyncList`.
For example, `SlidingSyncList::state` was updated by…
`SlidingSyncListRequestGenerator`. Now `SlidingSyncList` is responsible
to update itself, and no one else.
Finally, `SlidingSync` no longer have to deal with
`SlidingSyncListRequestGenerator`. All it has is a set of
`SlidingSyncList`, and that's it!
The tests are still passing, hurray.
In `SlidingSyncListBuilder`, the `ranges`, `set_range`, and `add_range`
methods do not take a `u32` but a `U: Into<UInt>`. That's great!
However, in `SlidingSyncList`, the same methods take a `u32`. It makes
the API inconsistent.
This patch fixes that.
This patch renames `SlidingSyncList.set_ranges` to `ranges` so that
it matches the same terminology of the `SlidingSyncListBuilder` with
`ranges`, `set_range`, `add_range` and `reset_ranges`.
Consistency is important here.
The `SlidingSyncList::new_builder` method creates a new builder from a
`SlidingSyncList`. This method was missing some configurations, like
`full_sync_maximum_numbre_of_rooms_to_fetch`, `send_updates_for_items`,
`filters`, `ranges` and `timeline_limit`.
This patchs adds those missing configurations.
The `SlidingSyncListBuilder` struct has a `state` and a `rooms_list`
fields. Those fields are never updated and no setters or getters exist
to use them. There are just set with defaults, and passed to the final
`SlidingSyncList` type within the `build` method.
If a field is present in a builder type, it means they can be modified,
but it's not the case. So this patch removes them.
Event deduplication is supposed to be handled by the `Timeline`. Sliding
Sync is not responsible to de-duplicate the events, in our model. Having
two different de-duplication logics (one in Sliding Sync, and one in the
Timeline) could create weird situations, and hard to debug issues.
We give it a try by removing the deduplication from Sliding Sync.
The `m.no_olm` withheld code is supposed to be sent out only once for a
given device.
This patch prevents double sending of m.no_olm withheld code by adding
a test that ensures only one group session will send it out.
Previously, the logic for sharing group sessions was mostly correct, but
an edge case was forgotten where two group sessions attempted to share
the withheld code concurrently.
Although there's still a possibility for multiple m.no_olm withheld codes
to be sent out. This may happen during the propagation of the flag
remembering the fact that the `m.no_olm` code has been sent from the
OutboundGroupSession to Device. This scenario is likely unrealistic and
the consequences of sending things twice aren't problematic.
Currently, the tags used for releases of this binding have the format:
"matrix-sdk-crypto-js-v0.1.0-alpha.5". This is inconsistent with tags used for
other parts of the project, which omit the `v` prefix.
The url crate normalizes the string, but during OIDC verification steps,
the issuer verification must be made against the exact string that was
provided.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Like for the state store, the bindings expose the version as an f64
while the web API expects an unsigned integer.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch should ideally be split into multiple smaller ones, but life
is life.
This main purpose of this patch is to fix and to test
`SlidingSyncListRequestGenerator`. This quest has led me to rename
mutiple fields in `SlidingSyncList` and `SlidingSyncListBuilder`, like:
* `rooms_count` becomes `maximum_number_of_rooms`, it's not something
the _client_ counts, but it's a maximum number given by the server,
* `batch_size` becomes `full_sync_batch_size`, so that now, it
emphasizes that it's about full-sync only,
* `limit` becomes `full_sync_maximum_number_of_rooms_to_fetch`, so that
now, it also emphasizes that it's about ful-sync only _and_ what the
limit is about!
This quest has continued with the renaming of the `SlidingSyncMode`
variants. After a discussion with the ElementX team, we've agreed on the
following renamings:
* `Cold` becomes `NotLoaded`,
* `Preload` becomes `Preloaded`,
* `CatchingUp` becomes `PartiallyLoaded`,
* `Live` becomes `FullyLoaded`.
Finally, _le plat de résistance_.
In `SlidingSyncListRequestGenerator`, the `make_request_for_ranges`
has been renamed to `build_request` and no longer takes a `&mut self`
but a simpler `&self`! It didn't make sense to me that something
that make/build a request was modifying `Self`. Because the type of
`SlidingSyncListRequestGenerator::ranges` has changed, all ranges now
have a consistent type (within this module at least). Consequently, this
method no longer need to do a type conversion.
Still on the same type, the `update_state` method is much more
documented, and errors on range bounds (offset by 1) are now all fixed.
The creation of new ranges happens in a new dedicated pure function,
`create_range`. It returns an `Option` because it's possible to not be
able to compute a range (previously, invalid ranges were considered
valid). It's used in the `Iterator` implementation. This `Iterator`
implementation contains a liiiittle bit more code, but at least now
we understand what it does, and it's clear what `range_start` and
`desired_size` we calculate. By the way, the `prefetch_request` method
has been removed: it's not a prefetch, it's a regular request; it was
calculating the range. But now there is `create_range`, and since it's
pure, we can unit test it!
_Pour le dessert_, this patch adds multiple tests. It is now
possible because of the previous refactoring. First off, we test the
`create_range` in many configurations. It's pretty clear to understand,
and since it's core to `SlidingSyncListRequestGenerator`, I'm pretty
happy with how it ends. Second, we test paging-, growing- and selective-
mode with a new macro: `assert_request_and_response`, which allows to
“send” requests, and to “receive” responses. The design of `SlidingSync`
allows to mimic requests and responses, that's great. We don't really
care about the responses here, but we care about the requests' `ranges`,
and the `SlidingSyncList.state` after a response is received. It also
helps to see how ranges behaves when the state is `PartiallyLoaded`
or `FullyLoaded`.
The part of code that converts a list of users to the actual
/keys/query request uses the chunks() method. This method operates on
the slice. Our list/vec of users gets dereferenced into a slice before
we create our chunks. The chunks can't take ownership of the data, which
in turn requires us to clone the user IDs for them to be put into the
request.
Itertools has a chunks() method which operates on an iterator which we
can utilize to remove, not only the clone, but also a collect call.
At the same time, let's make the conversion step a simple functional
mapping and document what's going on.
The Option conveys our intention a bit better compared to the default
value. While nothing bad can happen, since the request IDs map will be
empty by default, it's a bit scary to have a valid sequence number since
an i64 defaults to 0.
This patch updates `SlidingSync.pos` and `.delta_token`. Each field has their
own lock. It's annoying because they must updated at the same time to not be
out-of-sync.
So a new field `SlidingSync.position` of type `SlidingSyncPositionMarkers` is
created. It holds `pos` and `delta_token. This new field is behind a single
`RwLock`.
The comments in the code should explain the general approach, but to give an overview:
We assign a unique sequence number to each device-list-invalidation notice, and keep track
of the sequence number at which each user was most recently invalidated. Then, when we
build a `/keys/query` request, we take note of the current sequence number. Finally, when
we get the response from that `/keys/query` request, we compare that sequence number
with the most-recent-invalidation of each user that is now supposedly up-to-date. All
of that means that we can see if the user has been invalidated *since* we built the
`/keys/query` request, in which case our request may have raced against the new device,
so we do not mark the user as up-to-date.
To make that work, we need to keep track of the sequence number for in-flight `/keys/query`
requests; we do that in the `IdentityManager`. Since there should only ever be at most one
batch of requests in flight, that is relatively easy; however, we also record the request ids
just to check that the application isn't doing something weird.
There is no need to persist the sequence numbers to permanent storage: provided the
`IdentityManager` and `Store` objects have a lifetime at least as long as an in-flight
`/keys/query` request, it is sufficient just to retain the `dirty` bit in permanent storage,
and then, when we reload the ephemeral `Store` cache, to treat everything with a `dirty`
bit as a "new" invalidation.
Fixes: #1386.
With https://github.com/matrix-org/matrix-rust-sdk/pull/1601, it's safer to
cancel a stream at any time, as any `Future` cancellation. This documentation
section of `SlidingSync` is no longer relevant as is, and can be rephrased.
With https://github.com/matrix-org/matrix-rust-sdk/pull/1601,
`TaskHandle::with_callback` is no longer necessary.
This patch updates `TaskHandle` as follows:
1. Before, the `handle` and `callback` fields were not mutually exclusive!
Indeed, the `callback` field was used as an abort mechanism, but _also_ as a
finalizer, i.e. a code that runs _after_ the cancellation of `handle`. Now
that the callback-based solution is no longer used, we can keep one usage for
this field and rename it `finalizer`.
2. The `handle` field is no longer optional, as it will always be set.
3. The `with_callback` method is renamed `new` as it's now the only constructor.
4. The `set_callback` method is renamed `set_finalizer`.
Tadaaa.
Move all `SlidingSync`'s fields into a new `SlidingSyncInner` type, and then
update `SlidingSync` to contain a single `inner: Arc<SlidingSyncInner>` field.
First off, it's much simpler to understand what's behind an `Arc` for
“clonability” of `SlidingSync`, and what's not. We don't need to worry about
adding a new field that is not behind an `Arc` for a specific reason or
anything. The pattern is clear now.
Second, this is required for next commits.
Before this patch, `SlidingSync::sync` was returning a callback-based
`TaskHandle`. It was waiting on the “stream loop” to finish (since it's a long-
poll, it means waiting 2*30s, cf. our code), before checking an atomic flag has
some value to decide whether it was time to leave the loop or not. So when the
user is cancelling this `TaskHandle`, a response (if any) was always handled.
But in the meantime, it was possible to start a new `sync`, and it seems like
it creates bugs.
After this patch, `SlidingSync::sync` now returns a handle-based `TaskHandle`.
It means that cancelling it will cancel the “stream loop” immediately. If
no response was in-flight from the server, that's perfect, no problem. If a
response was in-flight, the inner `pos` of the `SlidingSync` instance won't be
updated as the response won't be handled. So the server will re-send the same
response with the next sync request.
I guess it's better this way. Thoughts?
This patch adds a new feature.
Each call to `SlidingSync::stream` generates a unique `stream_id`. This
`stream_id` is passed to requests as a `txn_id` field. Returned responses must
hold the same `txn_id`, otherwise it means `stream` has received unexpected
responses from another `stream` or something else.
So far, when the `txn_id` field of the response and the `stream_id` don't
match, a log with the `ERROR` level is raised. Should we do more, like ignoring
the response entirely? Not sure yet.
The IndexedDB API expects an unsigned integer for the version.
The web-sys bindings expose it as a f64 so versions like 1.1 and 1.2
were used. The were converted back to uint in JS which means the version
was always 1.0 and no real upgrade was processed
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Simplify handling of sliding sync extensions
This patch simplifies the logic for handling sliding sync extensions.
These extensions are sticky, meaning that once enabled in one request,
they do not need to be repeatedly enabled for subsequent requests.
However, the previous logic tried to accommodate the case where the
extensions were initially disabled to reduce the size and processing
time of the initial response. This resulted in complex and error-prone
code.
With this patch, we have removed support for disabling to-device events
and streamlined the handling of sync extensions.
Co-authored-by: Jonas Platte <jplatte@matrix.org>
You can't update a GHA cache once you create it, so if we use the same cache
for each job in a matrix build, then we end up populating it for the first job
that completes, so any slower jobs don't get their dependencies cached.
On the other hand, if we create 20 500MB cache items on each build, we're going
to exhaust the cache storage as soon as we do a build. So, instead, let's just
do the caching for the main branch, and hope that other branches can still
benefit from it.
* Replace custom cancellation action with `concurrency`
* Improve step names
... so don't have three steps with the same name
* Bump version of checkout action
checkout@v2 uses an old version of nodejs, which is deprecated.
To declare the tests, the macro is still used but the content of the
tests is moved in the StateStoreIntegrationTests trait.
Allows to get the proper line reported when a panic occurs,
and have linting, formatting and IDE suggestions.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Currently, the cache of the xtask binary isn't working terribly well:
* we seem to build it on each run anyway, presumably because we don't cache any of the intermediate build
artifacts. Running the binary directly rather than indirecting via "cargo" prevents this.
* There is no sharing of the cache between the "rust" and "bindings" CI, because we use different cache keys.
This PR addresses both problems, and hopefully speeds up CI a bit as a result.
The Sliding Sync server might not send some parts of the response, because they
were sent before and the server wants to save bandwidth. So let's update values
only when they exist.
Running the same example twice
borks encryption.
Instead point to an example that shows how to do it.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch cleans up the `SlidingSync::stream` method. It renames variables,
improves log messages etc.
This patch also creates a new `MAXIMUM_SLIDING_SYNC_SESSION_EXPIRATION`
constant. This value was previously hardcoded and lost in the code, now it's
easier to spot it for further updates.
This patch finally renames the `failure_count` field to `reset_counter`,
because it doesn't count the number of failure, but the number of
`ErrorKind::UnknownPos` exactly, i.e. the number of times we reset the
`SlidingSync` state.
This patch cleans up the `SlidingSync::sync_once` method by renaming variables,
adding more comments, simplifying the code by reducing the number of variables
etc.
This patch also changes `futures_util::join!` to `futures_util::future::join`.
It does the same but the macro needs the `async-await-macros` feature to be
turned on, while the second works without any features.
Finally, this patch improves the log messages by making them more clear for a
new reader.
I admit this patch is quite tricky. Please try to follow me.
So, first off, in `SlidingSyncRoom::new`, we were clearing the timeline,
because somehow it exists twice in memory at this step.
Which led me to understand how `SlidingSync::handle_response` was working.
I've clarified how this part of the code works. We are dealing with 2 kind of
responses for a specific reason: `SyncResponse` and `v4::Response`, now it's
documented and I hope it's clearer.
Then, I notice that we were passing a clone of the entire sliding sync
response (`v4::Response`) to `Client::process_sliding_sync`. I thought it was
suboptimal, so I've updated the code to take a reference. It led me to update
`BaseClient::process_sliding_sync`. It was a little bit tricky, but I reckon we
have less clones now than before.
And now, back to `SlidingSync::handle_response`, I was able to compute the
`timeline` correctly by draining it from the `v4::Response`, or by moving it
from `SyncResponse`. So it's no longer necessary to have this clearing code
inside `SlidingSyncRoom::new`. Honestly it has nothing to do at this place
before.
To conclude: We have cleaner code, and less clones.
What thing I reckon could be optimized, is that the entire `timeline`
(`Vec<TimelineEvent>`) is cloned to be passed to `Client::handle_timeline`. So
this timeline exists in 2 places: in Sliding Sync, and somewhere else. I don't
believe it's a problem now, that's how it works, but we must be aware of that.
The `Client::login_custom` allows to login by using a custom login
method. In particular, it is possible to login to Synapse which supports
JWT authentication.
Signed-off-by: boxdot <d@zerovolt.org>
First off, it's not necessary for `SlidingSyncRoom::update` to take a reference
to `room_data: v4::SlidingSyncRoom`. When `update` is called, the iterator owns
its items.
Second, by taking ownership of `room_data`, we no longer need to clone all the
data we need to assign to `self.inner`.
First off, `SlidingSyncRoom.from` doesn't need to be visible to the entire
crate, only to `crate::sliding_sync.
Second, it's a constructor, so let's call it `new`.
1. Put `FrozenSlidingSyncRoom` at the bottom of the module.
2. Put merge 2 `impl SlidingSyncRoom` together.
3. Remove the `AliveRoomTimeline` type alias.
4. Improve the documentation.
So far, the `SlidingSync.pos` field was public to the crate. In order to avoid
breaking the internal state of this type, its visibility is now private.
However, we need to be able to change the value when testing the
`SlidingSync` type itself. To achieve that, this patch removes the old
`force_sliding_sync_pos` function, and implements 2 new functions: `set_pos`
and `pos` directly on `SlidingSync` only when `#[cfg(any(test, feature ="testing"))]`.
Previously, it was possible for us to use invalid indices when:
- We retried decrypting multiple events at once
- One of them (not the last) was an edit or reaction
This lead to a situation where we would remove the UTD item once
decryption for it was successfully retried, but not account for the
resulting index shift for all later timeline items.
Before this patch, `SlidingSyncView` has the following fields that were public:
`state`, `rooms_list` and `rooms_count`. Since they are `Mutable`, they can be
changed from the outside, and then will break the internal state of the view.
This problem is mentioned in https://github.com/matrix-org/matrix-rust-sdk/
issues/1474.
This patch solves this by making them prviate. Phew. That was simple!
But wait, we have a problem now. `matrix-sdk-ffi` was relying on them. So
this patch adds “helpers” methods on `SlidingSyncView`, like `state_stream`,
`rooms_list`, `rooms_list_stream`, `rooms_count` and `rooms_count_stream`.
Let's add new ones when it's necessary.
In https://github.com/matrix-org/matrix-rust-sdk/pull/1478, I've introduced
a regression. The FrozenSlidingSyncRoom.timeline` field has been renamed to
`timeline_queue`.
I didn't notice that this type can be serialized and deserialized. That's
actually a breaking change because this type is stored somewhere once
serialized. So with #1478, it was impossible to deserialize them!
This patch fixes that. It also adds a test to ensure the serialization form of
`FrozenSlidingSyncRoom` doesn't change.
This patch allows an `OlmMachine` to use a different store than Sled, like SQLite.
A new enum is introduced: `StoreType` which 2 variants: `Sled` (the default), and `Sqlite`.
The `OlmMachine.initialize` constructor now takes a new optional `store_type:
Option<StoreType>` argument. If no value is passed, the default `StoreType`
variant will be used (as mentioned: `StoreType.Sled`).
The code has been rewritten a little bit to make the type system happy without
introducing too much type indirections.
This patch finally adds a parameterized tests that exhaustively test: no store
type, `Sled` and `Sqlite`.
Building the crypto-js bindings in release mode is very slow and not really
necessary for local development.
`--release` is the default, so there is no need to specify it
explicitly. Instead, allow `wasm-pack` args to be specified by an env var, and
add a `build:debug` npm script which will build in debug mode.
We already have support for proper `ToDeviceRequest` objects in
`outgoing_requests`, so let's use it in `share_room_key` as well. It's much
easier for the application to work with the proper object than an untyped json
blob.
In the code `if let Some(global_data) = account_data.as_ref().map(|a|
&a.global)`, the `map` is not purely necessary. This patch simplifies the code
to remove the `map` and read the `global` field later on.
By using `Default::default` for the parameter value, we don't know which type
we are passing to the function. Hence the useful comment `room_info.ephemeral`.
This patch changes this to `Ephemeral::default`, so that we now know what we
pass without the need of a comment.
Javascript's `console.trace` is not a good equivalent for Rust's
`Level::TRACE`.
In particular:
* `console.trace` causes a stacktrace to be written with each message
* Under nodejs, `console.trace` writes the message to `stderr`, while
`console.debug` writes to stdout
`console.trace` is therefore somewhat analogous to `console.error`.
Since we already include the log level in the message, we might as well just
send trace messages to `console.debug` rather than `console.trace`.
This is useful for platforms which might want to avoid the cost of
password based key-derivation and have the ability to securely store an
encryption key.
Co-authored-by: Denis Kasak <dkasak@termina.org.uk>
Contrary to other tables that use their own name to encode their keys,
the `ROOM_INFO` table uses the `ROOM` table name to encode its keys.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
`wasm_timer` doesn't work outside the browser, which prevents it being used in
tests.
We can replace it with `gloo-timers` wich uses the standard Javascript
`setTimeout`.
This makes it easier to implement it. However, using a different error
type than CryptoStoreError is a non-trivial change, so left for the
coming commits for all stores except the memory store.
The key encoding in key-value stores is backwards-compatible so
old receipts are counted as unthreaded receipts.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
A fix for (de)serialization of redacted state events in Ruma made
old events undeserializable.
Bump the DB version.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
A fix for (de)serialization of redacted state events in Ruma made
old events undeserializable.
Bump the DB version.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This implementation is wrong in the sense of its semantics is not about
dereferencing a thin pointer to something, but just to give access to one
specific field of the entire structure. That's not how `Deref` is supposed to
be used.
Moreover, it creates conflict between the `SlidingSyncRoom.timeline` field,
and `SlidingSyncRoom.inner.timeline` field, which both exist, but not for the
same purposes. It creates confusion in the code.
Finally, it's better to expose proper getters to the outside world, so that we
control _and_ test _and_ know exactly what API we provide.
`Timeline::with_fully_read_tracking` was public although all public
Timeline constructors enable it by default.
It is also meant to be called when constructing the timeline so
using a builder pattern makes sense.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
`SlidingSyncView` exposes a `room` field. Problem: It's not updated
correctly and leads to error. The canonical way to get a room is by using
`SlidingSync::get_room`. This patch thus removes `SlidingSyncView.rooms`
entirely.
Before this patch, `SlidingSync::get_room` was taking ownership of an
`OwnedRoomId`, to only use it as a reference. Thus, calling this method was
creating useless clones.
This patch updates `SlidingSync::get_room` to receive a reference to
`OwnedRoomId`.
Note about "Write-Ahead Log" (WAL) mode: The SQLite WAL mode has a
bunch of advantages that are quite nice to have:
1. WAL is significantly faster in most scenarios.
2. WAL provides more concurrency as readers do not block writers and a
writer does not block readers. Reading and writing can proceed
concurrently.
3. Disk I/O operations tends to be more sequential using WAL.
4. WAL uses many fewer fsync() operations and is thus less vulnerable
to problems on systems where the fsync() system call is broken.
The downsides of WAL mode don't really affect us. So let's turn it on.
More info: https://www.sqlite.org/wal.html
Co-authored-by: Jonas Platte <jplatte@matrix.org>
Co-authored-by: Damir Jelić <poljar@termina.org.uk>
When we store an Olm session in the database, we encode the sender key and
session ID using the store cipher. That means that we also need to encode them
when we look them up, otherwise we won't find them.
This patch is trying to resolve the following issue:
When a local event is sent to the server, in case of an error, the
`Timeline::send` method just returned the error but it wasn't reflected inside
the `LocalEventTimelineItem` type itself. It's easy to loss this information.
So now, there is a new enum: `LocalEventTimelineItemSendState` with 3 states:
1. `NotSentYet`, when the local event has not been sent yet, it's theorically
the initial state,
2. `SendingFailed`, when the local event has been sent but it's failed!
3. `Sent`, when the local event has been sent successfully.
Therefore, the `LocalEventTimelineItem` struct has a new field: `send_state`.
The send state is managed by its `with_event_id` method which now receives an
`Option<OwnedEventId>` (prev. `OwnedEventId` directly):
* If it's `Some(_)`, then it means we got a successful response from the server
after the sending, so the send state is set to `Sent`,
* If it's `None`, then it means we got an error response from the server, so
the send state is set to `SentFailed`.
(A small change: The method `TimelineInner::add_event_id` has been renamed
`TimelineInner::update_event_id_of_local_event`.)
The `Timeline::send` still returns a `Result`, but the server's response is
passed to a new method: `TimelinerInner::handle_local_event_send_response`
(it's logically named after the `handle_local_event` method), which is
responsible to call `TimelineInner:update_event_id_of_local_event` (which was
previously called directly by `Timeline::send`).
And `TimelineInner::update_event_id_of_local_event` was already calling
`LocalEventTimelineItem::with_event_id`. So everything works like a charm here.
The local send state is closely managed by `LocalEventTimelineItem`, I hope it
will avoid state breakage as much as possible.
The sliding sync logic has another room type called `SlidingSyncRoom`.
This room type stores a limited amount of events in something called
`AliveRoomTimeline` in a in-memory cache. This room also gets persisted
to the store via another room type called `FrozenSyncRoom`.
These types are used when clients restore their view of the room, i.e.
when the user enters the room to look at messages.
When the client enters a room, a `Timeline` object will be created, but
it will be initially populated with events coming from
`AliveRoomTimeline`.
In a hot potato contest of who should be responsible to decrypt injected
events, everybody claims to be allergic to potatoes.
This patch modifies the `Timeline` constructor that populates the
timeline with events from the `AliveTimeline` to retry decryption on the
events that the `AliveTimeline` pushes into the `Timeline`.
Note that, because the `AliveRoomTimeline` never replaces encrypted
events with decrypted ones, every time the client enters/exits the room
events well get re-decrypted.
This is an omission from the spec where the subkeys are supposed to be
signed by the master key, but signing the master key wasn't mentioned.
This is still fine, since we always treated the master key and their
subkeys as a whole and never accepted one without the others.
To create an event next to the server, the event must be accompanied by
a transaction ID. When the server receives the event creation request, it
responds with a new event ID (which we will name `event_id_1`). Later on, when
a sync is done to the server, the server may respond with the transaction ID
and a new event ID (which we will name `event_id_2`).
The transaction ID is used to update the state of the local event. The event
ID received from the sync may ideally be the same as the one received by the
creation request's response.
Knowing that…
`EventTimelineItem` had a field: `event_id: Option<OwnedEventId>`. It was
`Some(event_id)` where `event_id` is the event ID responded by the server when
an event was created, i.e. it's `event_id_1`; otherwise it was `None`. This is
never `event_id_2`. Why?
Because `EventTimelineItem` has an other field: `key: TimelineKey`, which
represents the following states:
* `TimelineKey::TransactionId(OwnedTransactionId)`,
* `TimelineKey::EventId(OwnedEventId)`, where the event ID is `event_id_2`!
So it becomes obvious that `event_id_1` should be part of `TimelineKey`, not
`EventTimelineItem`. `TimelineKey` is where this transaction ID and event ID
logic is managed.
This patch updates `TimelineyKey::TransactionId` to be defined as:
* `TimelineKey::TransactionId { txn_id: OwnedTransactionId, event_id: Option<OwnedEventId> }`.
Why an `Option`? Because before receiving `event_id_1`, we may still want to
create a `TimelineKey`, the API must reflect that state.
So basically, `EventTimelineItem.event_id` is moved inside
`TimelineyKey::TransactionId`.
… rather than prepended, or added at a specific point as replacement for
an existing timeline item.
Previously, we would log lots of useless tracing events when retrying to
decrypt UTD items.
This patch fixes https://github.com/matrix-org/matrix-rust-sdk/issues/1379/.
This is similar to https://github.com/matrix-org/matrix-rust-sdk/pull/1197/.
We need to close the `OlmMachine`. Problem, `napi-rs` doesn't allow methods to
take ownership of `self`, so the following code:
```rs
fn close(self) {}
````
isn't allowed. There an [`ObjectFinalize`](https://docs.rs/napi/latest/napi/bindgen_prelude/trait.ObjectFinalize.html)
trait, but it's only called when the JS value is being collected by the GC.
It's not possible to force call `finalize` here.
This patch then introduces a new type:
```rs
enum OlmMachineInner {
Opened(matrix_sdk_crypto::OlmMachine),
Closed
}
```
that derefs to `matrix_sdk_crypto::OlmMachine` when its variant is `Opened`,
otherwise it panics. Calling the new `OlmMachine::close` method will change the
inner state from `Opened` to `Closed`.
Ideally, I wanted to throw an error instead of panicking, but `napi_env`
(used to build an `Env`, used to throw an error) is neither `Sync` nor `Send`.
Honestly, my knowledge of NodeJS and NAPI is too weak to implement `Sync`
and `Send` for `*mut napi_env__` safely. Especially because it can be executed
from various threads within `async fn` functions, that are driven by Tokio in
`napi-rs`. So, yeah, for the moment, it panics!
This moves the caches we have for tracked users out of the Sled, and
IndexedDB, CryptoStore into the Store struct. The Store struct is a
wrapper for the CryptoStore trait, so this is the natural place where
these caches should live.
Co-authored-by: Jonas Platte <jplatte@matrix.org>
Before we'd push one empty entry at a time, which lead to a lot of vecdiff updates pushed. With this
we now only push one event after the entire operation set has been applied on the fresh
or cold view.
A mismatch between the recorded Ed25519 and Curve25519 keys of the room
key and the identity keys of a Device used to be just a warning.
Considering that many clients will aggressively try to hide E2EE related
warnings let's upgrade this clear error into a full blown decryption
failure.
Co-authored-by: Denis Kasak <dkasak@termina.org.uk>
We might have a lot of tracked users, clients will usually have 1-10
thousand tracked users. This might slow down client startup if there are
so many tracked users.
We don't need the tracked users until we sync or try to send a message,
so load them lazily when they are first needed.
The conventional commit specification recommends the "docs" type for
documentation related changes. This also matches the initial usage of
this type in our commit history.
While the default git-cliff config does specify a regex "^doc", where I
suspect our types have been taken from, this regex actually allows "doc"
as well as "docs" for the type.
Nevertheless, let's specify that we're using the one recommended in the
spec.
I had no idea what a tracked user was, so I've attempted to clarify this
somewhat for future readers.
Co-authored-by: Damir Jelić <poljar@termina.org.uk>
Instead of putting all request fields inside a single `body` JSON-
encoded string field, there is now more types. There is less need to call
`JSON.parse`, and the type system will be able to catch more things.
For example, `ToDeviceRequest` now has 2 more fields: `event_type` and
`txn_id`.
This solves the issue of correlating different log lines that are talking
about the same request/response cycle. Plaintext logs would show this as
a flat structure, while something like Jaeger would show you a nice
tree. This allows you to reconstruct the tree in plaintext logs.
Various smaller fixes on sliding-sync for elem-x:
- Infrastructure and smoke test only for integration testing sliding-sync
- Expose Pop and Clear on VecDiff
- Expose adding views during sliding sync live runtime incl integration tests
- bug fix on replacing only in-window-items in the insert-code (was leading to a wrong list setting), including integration test
Required when `PaginationOptions` must implement `Send`,
e.g. with `tokio::runtime::Runtime::spawn`.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
When updating a device after we receive new device keys from a
/keys/query call we implicitly check that the user and device id match
since we need to fetch the device using the mentioned ids.
This moves the check into the update method as well to make it more
explicit. We also prevent upgrades of the Ed25519 key since we don't
support any different key type anyways.
The sliding sync proxy seems to have started to omit the e2ee extension
completely if no changes happened to the one-time key counts.
Our sync response processing assumed that, if there are some to-device
events that need to be passed to the OlmMachine, then we'll have some
data in the e2ee extension of the response as well. In other words, it
wouldn't pass in the to-device events to the OlmMachine if the e2ee
field of the sync response is `None`.
Since the assumption isn't true, we're going to pass in default values
for the one-time key counts when the e2ee extension field of the
response is `None`.
This is mainly done so we create a span with the name of the function,
since we can filter logs by the name of the span we're now able to
enable logs for this method only.
Previously, when we found a since-token in the frozen state, we'd always activate the to-device extensions
regardless of whether the user had activated it in their builder or didn't. With this change we are only
setting the since parameter from cache if the user actually activated the to-device extension.
- Don't actually fire off a request when the top of the timeline has
already been reached
- Allow making multiple requests without removing the loading indicator
in between
Index ranges are inclusive, but our loop would stop one short. This particuarly
tricky when the selective view is moved, as we didn't properly invalidate all items.
It's not a hard-and-fast-rule that Javascript exceptions should be `Error`
instances, but it's certainly a convention, and one that is going to reduce
surprises if we follow.
Add a second full-sync-up mode to sliding sync. Previously - and still the default for backwards compatibility, but now named `PagingFullSync` - was to page through the list by the page-size, de-validating the previous page of items. The newly added `GrowingFullSync` instead grows the window by the given number `batch_size` per request, starting and keeping it from `0`. In the latter we might be pushing more data over the connection and are slightly slower, but the top always stays active and thus reactive to changes.
Furthermore the developer can now configure an optional maximum number to grow/paginate the full-sync up to (so stopping before actually having reached `count` whatever is smaller). This is already exposed via FFI, too.
- [x] add new full-sync-mode
- [x] add limited sync-up mode (similar to full sync but only to a limit `n`)
- [x] implement sync-up in jack-in
Sliding Sync updates
- expose timeline listener via FFI on slidingsyncroom, too - returning a stoppable spawn
- connect room timeline and sliding-sync-room timeline together
- increase HTTP timeout on jack-in
- add jack-in support for actual timeline items.
We automatically request room keys to be forwarded from our other trusted devices if a decryption failure happens because the room key is missing.
This patch introduces automatic room key requests for decryption failures if the room key is available but has been ratcheted forward. In other words, we will now request a better version of the given room key automatically as well.
Co-authored-by: Damir Jelić <poljar@termina.org.uk>
`open_with_db` also has the passphrase parameter without mentioning it
in its name. The old name also sounded like the passphrase was required
when it's actually optional.
Clarify that the JSON-encoded `toDeviceEvents` passed to
`OlmMachine.receiveSyncChanges` must be the list of events themselves,
instead of a wrapper object that contains the event list.
Signed-off-by: Andrew Ferrazzutti <andrewf@element.io>
Tokens are not necessary for the restoration of the crypto/store, only
the meta. Required for OpenID Connect support where we need the tokens to get the
meta.
When the verification support was initially bound, Uniffi did not
support objects (the OlmMachine) returning other objects (our
verification objects). Instead we converted all the verification objects
into pure data structs which the other side had to poll for changes.
Now that Uniffi supports returning objects we can refactor this and make
the API on the other side the same as on the Rust side.
With these changes, the user of sliding sync can configure the SlidingSync-Builder to store and recover the state from storage. For that they should call cold_cache("my-sliding-sync-key") with their preferred storage location during setup time on SlidingSyncBuilder. If a cached version is found under that key, the internal state of the rooms-list as well as each view found (identified by their name) will be set from storage immediately - allowing the user to show the last cached state. The Builder then also uses the same key to store its latest state after every update received from remote.
👉 Note on room list:
As we store a disconnected state, we are saving all RoomList entries as either Empty or Invalidated. This allows for an easier updating when the server comes back with results, as we don't track the ranges in cache - in the view of the server, we haven't seen anything yet.
👉 Note on timeline events:
This does store all existing timeline events - initial and seen during the session (as we keep track of them right now) - and will recover that state. However, as we can't be sure wether we have gaps in the timeline, the timeline items are reset upon receiving updates for them from the server.
👉 Note on (full-sync-)view:
While we are caching the server returned results (view list, room count and room info) as is, we are not caching the internal state (whether we are catching up) nor the ranges (as they are likely to be out of date) - those will be acting the same way you configured them, just with preloaded results. So for a full-sync-view, it will still do requests in batches and replace the corresponding set of rooms. Which could mean you might see the same room appear twice, though the cached one would be marked as invalidated. This might be problematic if the list the UI shows is longer than the batches fetched, but would be resolved quickly when catching up.
👉 On recovery:
When the sliding sync receives a M_UNKNOWN_POS, indicating the server has expired our session, sliding sync now transparently retries with up to three times to restart the sliding sync with full set of extensions and the latest views at their existing windows, the current room state is held. For full sync this starts a new sync-up with the existing room list staying intact. This also works from the offline at start.
- matrix_sdk_base::sync::SyncResponse is the internal representation that
we can update to account for sliding sync
- matrix_sdk::sync::SyncResponse is `/v3/`-specific and should not change
* Move swift build scripts into xtask (#1201)
* fix(ffi): use target_path from `cargo metadata` rather than guessing
* ci(ffi): install necessary target arch for build-framework test
* feat(xtask): copy to target without rsync.
As it appears, the first sync for a room might not include the
encryption state, so we cannot set the encryption state to be synced on
incoming syncs. That way, we always fetch the encryption state manually.
* split logger into platform specific implementations
* logging support for android
* use platform independent wrapper for uniffi:exports
* activate colors for ansi-terminals
It turns out that Google Chrome refuses to initialise the wasm via the
synchronous `WebAssembly.Module` constructor, complaining that it is too
big. To be fair, it has a point.
Anyway, that means we need to provide a way to load the wasm
asynchronously. So, we introduce an `initAsync()` function which applications
can call before they do anything else, to load the wasm in the background.
If the app *doesn't* call `initAsync()`, then we load the wasm synchronously
the first time a function that accesses the wasm is called.
This new `OlmMachine.close` forces to drop/close the `OlmMachine` without
waiting on the JavaScript garbage collector to collect it.
`wasm-bindgen` generates the following JS glue code:
```js
close() {
const ptr = this.__destroy_into_raw();
wasm.olmachine_close(ptr);
}
```
And, `__destroy_into_raw` looks like this:
```js
__destroy_into_raw() {
const ptr = this.ptr;
this.ptr = 0;
OlmMachineFinalization.unregister(this);
return ptr;
}
```
It unregisters itself from the `FinalizationRegistry` correctly. We are
protected from a double-free.
Surprisingly, `indexed_db_futures::IdbDatabase` is not closed when dropped.
Hopefully, there is a [`IdbDatabase::close(&self)`][close] method, which calls
`web_sys::IdbDatabase.close`, aka [`IDBDatabase.close`][websys-close], so let's
use it!
`IDBDatabase.close` returns immediately and closes the connection in a separate
thread. The connection isn't actually closed until all transactions created
using this connection are complete. No new transactions can be created for this
connection once this method is called. Methods that create transactions throw
an exception if a closing operation is pending.
[close]: https://github.com/Alorel/rust-indexed-db/blob/8c106eb418aecdba2f3fde80d91a4673a875fdf6/src/idb_database.rs#L73-L77
[websys-close]: https://developer.mozilla.org/en-US/docs/Web/API/IDBDatabase/close
I have tried for hours and have concluded it is probably not possible
even with GATs to have RawEvent borrow its inner value.
It is also not a commonly-used feature, so removing the unnecessary
clones is not that important.
This patch adds a couple more states to the SAS verification state
machine. Namely we add the following states:
* KeySent
* KeysExchanged
This prevents users from confirming that the short auth string matches
before they have sent out a m.key.verification.key event, which is
necessary for the other side to present the short auth string.
The KeysExchanged state functionally replaces the KeyReceived state.
Meaning that the short auth string can only presented once we
transitioned into the KeysExchanged state.
We can transition into the KeysExchanged state from the KeyReceived
state or from the KeySent state, depending on if we first receive a
m.key.verification.key event or if we first send out our own
m.key.verification.key event.
This means that, if the transition into the KeysExchanged state happens
through the KeySent state, users won't be able to tell if the transition
happened. In other words, listening to `m.key.verification` events
doesn't work anymore, users will need to use the new `Sas::changes()`
API to listen to a stream of state changes.
`wasm_bindgen::JsValue::(from|to)_serde` now emit warnings because of a
circular dependency. To solve this problem, this patch now uses `gloo-utils`,
see https://rustwasm.github.io/wasm-bindgen/reference/arbitrary-data-with-serde.html#an-alternative-approach---using-json.
Ideally, we would like to use `serde_wasm_bindgen` but the behaviour isn't
the same. `gloo-utils` serializes and deserialized through JSON, whilst
`serde_wasm_bindgen` manipulates the `JsValue` directly which can be better or
worst dependending of the case. This patch conserves the JSON approach as it
was the previous and tested one.
By asking `wasm-bindgen` to generate JS glue code for `WeakRef` support,
it removes the need to call `free` manually to free objects, thus we reduce
potential memory leaks inside Rust. See https:// rustwasm.github.io/docs/wasm-
bindgen/reference/weak-references.html to learn more.
It uses [`FinalizationRegistry`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry)
under the hood, I reckon it's quite common and fits into Matrix's clients needs
in terms of browser support.
While technically a type constructor can return a `Promise`, it's not
considered as idiomatic JavaScript to do that. We are changing `new OlmMachine`
to `OlmMachine.initialize`. The rest of the code is strictly the same.
Add general extension framework for sliding sync, implementing the e2ee, to-device and account-data extensions as per existing proxy implementation. Add a new (ffi exposed) function to use activate the extensions.
Also extends jack-in to have permanent login and storage support now, rather than posting an access token and expose messages inside the sliding-sync layer to actually use the decrypted messages if given. Contains a lot of fixes around these aspects, too, like uploading any remaining messages from the olm-machine on every sliding-sync-request or processing even if no room data is present (which can happen now as processing only extensions might takes place).
This patch adds a way for users to listen to changes in the state of a
SAS verification.
This makes it much more pleasant to go through the verification flow and
incidentally easier to document it.
Now that we're not scoping the room keys by the Curve25519 sender key
we're opening the door of multiple devices trying to insert the same
room key into our store.
This patch changes our logic so we only store room keys from an
m.room_key event if we don't have one already or if the new key is
a better version of the one we already have.
This mostly assumes that the first room key with a given session id
is coming from the creator of the room key.
It was very confusing that matrix_sdk::store::make_store_config's first
argument was either a path or a database name depending on the build
configuration. ClientBuilder::{sled_store, indexeddb_store} are also
easier to use.
By default, the new `experimental-nodejs` feature is
disabled. However, when enabled, it uses our own fork of
`indexed_db_futures` that make it work on Node.js, and so this entire
`matrix-sdk-indexebdb` crate.
The crypto crate consumes to-device events, if needed decrypts them, and
in the end reserializes while zeroizing secrets.
It needs to do this reserialization cycle as loslessly as possible, this
is why we have a bunch of BTreeMaps lying in events and contents that
capture unknown fields.
The Deserialize implementation of ToDeviceEvent<C> put the event type
into this BTreeMap but the Serialize implementation figures it out from
the EventType trait of the content.
This patch simplifies things by putting the event type into
ToDeviceEvent<C>, this also means that we can just derive the Serialize
implementation for ToDeviceEvent<C>.
Inspired by the changes in #1013 I was thinking about the use case for `sync_*` and how we handle error cases. Most notably while we give the callback the option to stop the loop, we don't really give an indication to the outside, how to interpret that cancellation: was there a failure? should we restart?
Take e.g. a connectivity issue on the wire, we'd constantly loop and just `warn`, what you might or might not see. Even if you handle that in the `sync_with_result_callback` and thus break the loop, the outer caller now still doesn't know whether everything is honky dory or whether they should restart.
This Changes reworks that area by having all the `sync` return `Result<(), Error>`, where `()` means it was ended by the inner callback (which in `sync()` never occurs) or `Error` is the error either the inner `result_callback` found or the that was coming from the `send` in the first place. Thus allowing us to e.g. back down to sync as it was a dead wire or restart it if there was only a temporary problem. Making all that a just a bit more "rust-y".
… for matrix-sdk, matrix-sdk-base, matrix-sdk-common and matrix-sdk-crypto.
matrix-sdk-indexeddb as well as the JS bindings and wasm_command_bot are
left as-is because they will likely always require JS.
* starting with jack-in
* starting by flying tui
* connecting to real server, showing info
* add .env to gitignore
* infrastructure for tests
* display loading time for syncv2
* minor design updates
* initial sync
* finalise first edition of sliding sync
* directly link to sliding sync and show rooms list
* nicer UI, toggle logs
* passing through sliding sync homeserver
* separate syncs and disable v2 autostart
* selecting rooms
* nicer view
* configurable batches and more default needed events
* selecting rooms
* calculate and show status info per room
* precalculated room stats
* restructure code to allow for cancellation of streams
* finish up merge updates
* fix calculation error in room list len
* cleaning up system flow
* fixing sync up
* new multi-view API
* move sliding sync in separate module
* fixing format
* adding and clearning views
* expose filters and timeline limits
* renamed
* adding room subscriptions to sliding sync
* update summary
* live fetching and subscriptions in jack-in
* subscribe to selected room
* starting to switch to tuireal - using example
* status bar and first linkup
* re-adding rooms
* implementing port for customised update event
* showing details and timeline
* fix formatting
* cleaner UI, updating details quickly
* make it green
* implement other Ops
* proper handling of invalidation
* saving sliding sync results to db, too
* saving new prev_batch field if given
* split events and timeline
* cleaning up
* live updates
* upgrading to latest ruma and matrix-sdk
* Update tui-logger to fix the broken build
* fixing latest ruma sliding-sync-branch updates
* feat: first set of ffi sliding sync
* expose sliding sync via FFI
* implement un-/subscribe
* implement view state updates
* updating to latest JSON format and ruma update
* implementing room selecting for new data model
* fixing room selection
* fixing feature flag requirements for sliding-sync
* fixing style, clippy and docs
* style(sliding-sync): fixing rust format styles
* fix(ffi): fixing sliding sync merge failure
* fix(jack-in): update jack-in to latest ruma
* fix(sliding-sync): need to have a version set before polling proxy
* expose sliding sync builder over ffi
* add SlidingSyncViewBuilder
* add forgotten changes on sdk itself
* new file logger feature on jack for deeper logging
* fix(http-client): log the raw request we are sending out
* feat(sliding-sync): better logging
* fix(sliding-sync): switch to full-listen after reaching live state in full-sync and make sure we replace the correct entries
* feat(ffi): expose sliding sync view ranges
* fix(ffi): fixing sliding sync start_sync loop to actually loop
* feat(sliding-sync): allow lookup of room data
* feat(sliding-sync-ffi): fetching name of room
* feat(ffi): expose unread_notifications of rooms
* feat(ffi): stoppable spawn for sliding sync
* fix(ffi): expose has_unread_notifications on room
* feat(sliding-sync): latest room message
* fix(sliding-sync): update to latest ruma layout
* doc(sliding-sync): adding docs to builder fns
* feat(sliding-sync): extra signal on the view to inform about general updates
* fix(sync-v4-ffi): expose new callbacks via ffi
* fix(sliding-sync): reduce default required states to make things faster
* fix(sliding-sync): fix build params
* feat(jack-in): extended instrumentation
* fix(sliding-sync): unbreak faulty feature-gate
* fix(sliding-sync-ffi): mut where mut is due
* fix(sdk): allow separate homeserver on every request to unbreak using send on client while in sliding sync on a proxy
* fix(jack-in): update to latest dependencies, that work
* feat(ffi): helper to patch sliding sync room into regular room
* style(jack-in): cargo fmt
* fix(sliding-sync): Update to latest ruma changes
* fix(sliding-sync): fix missing FFI updates to latest ruma
* feat(sliding-sync)!: simplify stream cancellation, cancel ffi sync if already existing
* fix: timeline backwards pagination must work without synctoken
* fix(sliding-sync): clarify order of messaes in alive TL; pick correct last item
* fix: update view delegate api for clarity
* style(jack-in): fix cargo warnings
* feat(sliding-sync): update room details
* fix(sliding-sync): only update room info selectively when given
* fix(sliding-sync-ffi): convert and store counts as u32, check against 0 for has notificaitons
* style: cargo fmt, file endings and a few other minor style fixes
* docs(jack-in): improving CLI and Readme
* feat(sliding-sync): allow setting of required event_states on viewbuilder
* style(sliding-sync): docs and minor fixes
* style(sliding-sync): various clippy fixes
* style(jack-in): clippy suggestions applied
* fix(sliding-sync): Delegate becomes observer
* test(sdk): adding test for request config
* docs: Fixing copyright header
* style(ffi): Nicer naming of params for observer
* fix(ffi): sliding sync is not optional for now
* fix(sdk): remove superflusous tracing instrumentation
* fix(sdk): use structured logging
* fix(jack-in): removed unneded log import
* fix(jack-in): use server_name rather than deprecated user_id on ClientBuilder
* style: typo and clippy
* style(sliding-sync): clippy and formatting
* fix(sliding-sync): cleaning up minor syntax issues
* fix: remove unneded feature-definition section
* fix(sliding-sync): minor fixes as per review
* fix(sliding-sync): Make Builders owned
* fix(sliding-sync): more minor style improvements
* fix(sliding-sync): minor style improvements
* fix(sliding-sync): remove homeserver from RequestConfig, use specific internal fn instead
Co-authored-by: Stefan Ceriu <stefanc@matrix.org>
Merge pull request #1013 from matrix-org/ismail/logout
Add logout facilities to `client` and helpers to ffi for tracking the soft-logout issued by a server. Further more this adapts the `sync_with_callback` by adding a new `sync_with_result_callback` that hands the entire `Result` to the callback.
This patch removes the `decode_image` (default) feature for this
`matrix-sdk-qrcode` crate.
First reason is that `rqrr` panics for some particular QR codes, and
we don't want to panic.
Second reason is that it's not a feature that is used. For Element on
iOS and Android, it's very unlikely that every frame from the camera
will be sent to `matrix-sdk-qrcode` to see if it can be decoded. So
it's obvious that an external library is used to read the bytes from
the QR code, that are then sent to `matrix-sdk-qrcode`. For Element on
Web, it's basically the same argument.
This feature is actually used only in our tests to ensure the
generated QR code is valid, but it sometimes fails due to `rqrr`
(cf. First reason).
`AnyDecryptedOlmEvent::Custom` contains at least 440 bytes, while the
second-largest variant (`RoomKey`) contains at least 0 bytes. Let's
box `Custom` so that the size of `AnyDecryptedOlmEvent` stays low.
A room ID can no longer be invalid, it's just a string representing
either a `EventId` (we can validate that but…) or a `TransactionId`
(which is an opaque string, so it can be anything).
This patch updates `QrVerification::new_cross`, by passing a flow ID
as an owned `String` to `VerificationData`, thus removing a panic, and
allowing QR code verification to happen outside a room.
This patch updates `VerificationData` to receive a flow ID,
represented as an owned `String`, instead of an `OwnedEventId`. Why?
Because QR code verification can happen outside a room. In such
scenario, there is no event ID, but a transaction ID, unified behind
the `matrix_sdk_crypto::FlowId` enum. `VerificationData` doesn't
really care about that details. Proof is that `QrVerificationData`
receives an owned `String`, which is then casted into an
`OwnedEventId` to match this API properly; but at the top, it just
receives a string.
This patch brings also a little bit of clean up while editing code
around.
Merge pull request #979 from FlixCoder/cleanup
Fixes a problem, where non-stripped information of the room had predecence in all scenarios of the memory-, sled- and indexeddb-store, thought according to the spec, we prefer the full info only until we've received new stripped info (from another invite). This fixes that to be in line with the spec by removing stripped data when we see a full-event and keep stripped info preferred if found (so, after you joined, stripped data is removed and when you knocked or are invited again, it is preferred), without ever deleting full data. It also adds nice unit- and integration tests to ensure this works as intended.
This patch adds a method to format a list of emojis in a a terminal
friendly way.
This method was borrowed from weechat-matrix but it's also quite useful
in our own emoji verification example.
Since this example prints out messages to stdout, it becomes quite hard
to see when we ask for user input if all the logging is going on.
This patch adds a standard verbosity flag which disables all logging by
default.
Previously, when both a possibly-redacted timeline event handler and a
non-redacted timeline timeline event handler would apply to multiple
events in a sync response, they would individually run for every event
in order. With this change, they will instead both be called
for one event before the next is processed.
The use of `io::Read` wasn't helping since we had to buffer the whole
file in memory anyways, and we are unlikely to get around that in the
near future.
Inbound group sessions would fail to be deserialized if there was a
forwarding curve chain, this patch fixes it so we go through base64 when
we deserialize this field.
This patch introduces a new `pack` NPM script, which runs `wasm-pack
pack` behind the scene.
This patch modifies the `publish` NPM script to run the `pack` script
as a pre-script (so… in the `prepublish` script).
Finally, this patch no longer uses `$npm_execpath` as it doesn't work
on Windows. It should be `%npm_execpath%`. It's not obvious to make
scripts interoperable, so we will stick with `npm` for now.
The `OlmMachine` constructor now has 2 more optional arguments:
`store_name` and `store_passphrase`, to use Indexed DB as the backend
to store the Olm machine keys, rather than having an in-memory Olm
machine.
Node.js is used to test our binding. Indexed DB is absent of
Node.js. So, firstly, we are using the `fake-indexeddb` JavaScript
library to mimic the same API inside Node.js. And, secondly, we use
our own fork of the `indexed_db_futures` Rust crate to provide add
support for Node.js too ([PR is
here](https://github.com/Alorel/rust-indexed-db/pull/11)). It
basically looks in the Node.js global environment if the `indexedDB`
getter is present, just like it is already done for `Window` on any
browser, or `WorkerGlobalScope` for workers.
This patch updates the `publish` NPM script to explicitly run
`wasm-pack pack` to create the NPM package in the `pkg/`
directory. This patch also updates the Github Action workflow for the
`matrix-sdk-crypto-js` release, to create a new Github Release, with
the NPM package attached as an asset file.
This patch adds a new `npm run publish` script that:
1. Run `npm run prepublish` (which runs the `build` and `test` scripts),
2. Run `wasm-pack publish`.
Note 1: The `prepublish` script is using the `$npm_execpath`
environment variable instead of just “`npm`”, so that if someone is
using `yarn` or another JavaScript package manager, it _should_ work
(not tested yet).
Note 2: `wasm-pack publish` is run without running `wasm-pack login`
before that. _But_ we are updating the registry URL in the NPM
configuration file in the Github Action workflow, see below.
This patch then creates a new Github Action workflow that is triggered
when a new tag of the form `matrix-sdk-crypto-js-v[0_9]+.*` is
pushed. This workflow runs `npm run publish` basically, but before
that, it updates the NPM configuration file by setting a value for
`//registry.npmjs.org/:_authToken`. Thus, running `wasm-pack login` is
not necessary.
Implement the `Attachment.encrypt` and `Attachment.decrypt` methods,
along with its `EncryptedAttachment` companion.
The trick is to avoid copies as much as possible. Instead of dealing
with `Uint8Array` as I've initially done, `&[u8]` and `Vec<u8>` is
passed instead. `wasm-bindgen` is smart enough to do as few copies as
possible (from JavaScript to Wasm's memory is required, and we don't
want to do more), while typing everything as `Uint8Array`.
`EncryptedAttachment.encryptedData` returns a copy of the data
everytime it is called, and that's the last copy I'm not happy with.
… rather than deserializing at an enum type and taking the same branch
for getting a different enum variant¹ as failed deserialization.
¹ which should be impossible anyways
Initial state events aren't actually received through sync, they're sent from
the client to the server when creating a room.
This change makes it impossible to register event handlers for initial state
events. Previously, this was allowed, but the event handler was never called.
This patch adds some more stronger guarantees that received room key
events and other similarly security critical event types are only
handled if they have been received over a secure Olm channel.
… by waiting for the corresponding event confirming the action. Affects:
* Creating a room
* (Re-)Joining a room
* Leaving a room
* Accepting an invitation
* Rejecting an invitation
This patch delegates the decision making if a session is better to
vodozemac. Vodozemac has better insights if a group session can be
considered to be better.
This allows smoother development as a regular 'cargo check' will
validate most of the code. It also makes rust-analyzer work for the
crate without any configuration.
… instead of through `compile_error!` invocations. This helps avoid
unhelpful errors from the unsupported feature configuration by aborting
compilation earlier.
This patch ensures that the received secret has been sent by one of our
verified devices. We already ensure so for the secrets we import, since
we check that the cross signing keys match the public keys of our own
user idenity.
No other secrets get imported by this method but it could quite easily
become an issue if we start accepting more secret types.
This fixes a bug where we would lose already generated to-device
requests carrying an m.room_key when the client gets restarted.
This only happens if the request was generated after the initial
share of the room key. Such requests get generated if a new device
or member join the group.
This functionality is important to allow clients to log in using an OIDC server. The OIDC server returns only an access token, the client needs to fetch the user ID and device ID from the Matrix server separately.
This lives in the bindings for now since OIDC support in Matrix is being actively developed.
If the last sync was less then a 1s ago we wait for a 1s, it doesn't
make sense to wait an additional second on an error. Also the stream
sync api returns the error after a delay of 1s, which doesn't make
sense.
This patch creates one `inner` module for when `feature = "tracing"`,
and one for when `no(feature = "tracing")`. Then, let's expose
everything from `inner::*`.
This patch also replaces `Tracing.install` by `new Tracing`. In case
of `not(feature = "tracing")`, `new Tracing` raises an error.
The goal is to remove all the `#[cfg(…)]` annotations everywhere. Now
there is only 2 of them.
The patch updates the code to use `tracing_subscriber::reload`, so
that we get a `Handle` that can be used to modify the tracing at
runtime.
This patch also adds a new `tracing` feature.
This patch finally adds a test suite for the `Tracing` API.
* feat(bindings/crypto-nodejs): Add `#[napi(strict)]` to force type checking from JavaScript.
* chore(bindings/crypto-nodejs): Use our own fork of `napi-rs` for the moment.
* feat(crypto-nodejs): Download lib binary in postinstall
* build(crypto-nodejs): Workflow to prebuild napi bindings
* ci(crypto-nodejs): Disable broken target, install without download
* ci(apple-ffi): Don't run for drafts
* ci(coverage): Don't run for draft PRs
* fix(crypto-nodejs): bind to current version for download
* fix(crypto-nodejs): Ignore libs and package
* ci(crypto-nodejs): Build and upload NPM package
* fix(crypto-nodejs): Set proper target list
* ci(crypto-nodejs): Remove FreeBSD from build pipeline
* ci(crypto-nodejs): Linkers for linux cross compile
* ci(crypto-nodejs): Add arm64 build for windows
* ci(crypto-nodejs): Proper linkers for arm and musl
* ci(crypto-nodejs): Correct apt command for musl
* fix(crypto-nodejs): Drop arm64 linux musl support
* ci(crypto-nodejs): Manual Workflow trigger process
* chore(crypto-nodejs): Get Github to pickup our action
* ci(crypto-nodejs): Add i686 Linux built
* ci(crypto-nodejs): Configure cliff for nodejs changelogs
* ci(crypto-nodejs): Proper gcc for i868 targets
* docs(crypto-nodejs): Add supported targets for npm install
* ci(crypto-nodejs): Limit building of binaries to tags
* style: consol.log -> console.info; Improve docs
Co-authored-by: Ivan Enderlin <ivan@mnt.io>
* activate for testing
* fix broken merge
* 0.1.0
* fix(js): put in the proper package name
* activate for PR for testing
* fix(nodejs): getting ready for publishing
* ci(crypto-nodejs): Adding docs and fixing naming for workflows
* typo: missed one
* fixing package name
Co-authored-by: Ivan Enderlin <ivan@mnt.io>
This patch adds customized event types, currently only for the
m.room_key and m.secret.send to-device events.
This allows us to:
a) Deserialize the session_key field into a vodozemac type
b) Control when we zeroize secrets better
We don't want to clone a struct that contains a secret.
However, on the Node.js side, we can only receive arguments by
references. The problem we have is that we cannot transfer the
ownership of `MediaEncryptionInfo` to `AttachmentDecryptor` because we
don't own it. To simulate this behavior, we use `Option.take`.
A new method then appears:
`EncryptedAttachment.hasMediaEncryptionInfoBeenConsumed` to know if
the media encryption info has been consumed by `Attachment.decrypt`
already or not. That way, we can decrypt only once. It is possible to
do a JSON-encoded backup of the media encryption info by calling
`EncryptedAttachment.mediaEncryptionInfo` though.
First, u128 has a bug in `serde`,
cf. https://github.com/serde-rs/json/issues/625.
Second, we don't need to represent the timeout as a u128, it's clearly
too large. This patch tries to convert it to u64. It should never
fail, but we propagate the error anyway.
This patch provides a new API to encrypt and decrypt attachment,
i.e. big buffer of type `Uint8Array`.
It's based on `matrix_sdk_crypto::AttachmentEncryptor` and `AttachmentDecryptor`.
Vodozemac used to accept and return strings when encrypting and
decrypting. This is quite unusual for a pure cryptographic library so we
switched towards the usual setup where we encrypt/decrypt raw bytes.
Since we do encrypt/decrypt JSON strings in Matrix land, we do the
string conversions over here.
In async functions, the Node.js GC may or may not (that's a random
behavior) collect the arguments passed to the function as soon as it
returns. The function may not be executed yet, since it's async. Thus,
it leads to memory corruption: The function tries to read later on the
value inside an argument and… it crashes at best.
To avoid this bug, there is no other choice than cloning the values
before the function returns, in its “sync path” (so before any
transformation of an `.await` point into an “async block”).
The performance impact is not “massive”, I'm not sure it could be
noticeable easily since it is most of the time related to identifiers
(e.g. `UserId`), which are cheap to clone. I have to find the balance
here, and cloning offers the best trade off from my point of view.
So, Clippy suggests to change `(&member).into()` to `member.into()`
but it's the same, and `From<T>` is not implemented for this `T`, only
`From<&T>` is present. Thus, to deceive Clippy, I'm using
`std::borrow::Borrow` here. Not super happy with that though…
Clippy on nigtly is raising a warning, which is turned into an error
on the CI. It's the [`explicit_auto_deref`
lint](https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref). I
suspect it's a false-positive but I'm not sure. Anyway, to workaround
this and unblock our CI, let's call `DerefMut::deref_mut` manually:
it's clearer anyway.
This patch first implements the new `Signatures`, `Signature` and `MaybeSignature` types.
Then, it moves some Vodozemac types into their own module, and
implements the new `Ed25519Signature` type.
Finally, it implements `OlmMachine.sign`.
For some unknown reasons, sometimes, randomly, one test (initializing
an `OlmMachine` with a local store with a passphrase) can take more
than 5s, only on Github Actions. Let's increase the test timeout value
so that the entire test suite doesn't fail.
`matrix-sdk-crypto-nodejs` and `matrix-sdk-crypto-js` are no longer
default members of the Cargo virtual workspace. The Github Actions
workflows for the bindings now live in a `bindings_ci.yml` files
(ideally, it should be in a subdirectory,
`.github/workflows/bindings/ci.yml` but it doesn't work).
> [`cargo-nextest`](https://nexte.st/index.html) is a next-generation
> test runner for Rust projects.
This patch installs and uses `nextest` to run our own tests.
Comparing `cargo test` and `cargo nextest` with hyperfine provides the
following results:
```sh
$ hyperfine 'cargo test --workspace' 'cargo nextest run --workspace && cargo test --doc'
Benchmark 1: cargo test --workspace
Time (mean ± σ): 51.785 s ± 2.066 s [User: 183.471 s, System: 10.563 s]
Range (min … max): 49.151 s … 56.641 s 10 runs
Benchmark 2: cargo nextest run --workspace && cargo test --doc
Time (mean ± σ): 44.556 s ± 0.894 s [User: 192.213 s, System: 11.441 s]
Range (min … max): 43.170 s … 45.762 s 10 runs
```
Benchmark 2 is 1.16 times faster than Benchmark 1.
We now support only “current”, “active” or “maintenance” versions
according to https://nodejs.org/en/about/releases/, which are
compatible with NAPI v6.
There is a segfault with `napi-rs` and Node.js in v12.17. It's an old
version, it may be fair to drop its support for now. Let's see if
people would need it in the future, we may work on `napi-rs` to fix
this bug in case it's really necessary.
This patch adds the `store_path` and the `store_passphrase` arguments
to the `OlmMachine` constructor to use a `CryptoStore` instead of
having an in-memory Olm machine.
Don't assume virtual client membership is join if none is stored, since
that leads to a client being told it's joined in rooms it has no
membership of. The main appservice client still assumes it's joined
every room it receives transaction events about.
The spec claims that we should cancel verifications if multiple
verifications are attempted at once[1]:
When the same device attempts to initiate multiple verification
attempts, the recipient should cancel all attempts with that device.
So let's start doing this.
[1]: https://spec.matrix.org/v1.2/client-server-api/#error-and-exception-handling
This should mostly remove the wild west of signature verification. We
define a trait that tells us which objects can contain signatures and
thus can be passed on to signature verification methods.
It also should be slightly more efficient, since we removed a bunch of
duplicate canonicalization steps.
An appservice can log in by providing a valid appservice token and a user within the appservice’s namespace.
This allows the appservice to acquire a scoped access token for a single user. matrix-appservice-sdk can now take a persisted device id for virtual users. If e2ee is enabled, it will automatically perform an appservice login to create the an E2EE backed session for the virtual user.
Since signatures can be uploaded by anyone, unlike one-time keys which
might only be uploaded by the given device for themselves, a Signatures
struct might fail to be deserialized because a signature was maliciously
attached to the object by someone else.
We don't want to fail to deserialize all the signatures just because
someone attached an invalid one. We still would like to have the
signature be deserialized into its proper type if possible so the Raw<T>
pattern was not used.
This commit also adds a virtual user builder, which allows you to
control the device id, change the client builder, perform an appservice
login, and even restore a previously created session.
https://spec.matrix.org/v1.2/client-server-api/#appservice-login
As discussed with @poljar, the Matrix event we are creating is
`m.room.key`, so it's preferable to rename the function
`share_room_key`. Note: Olm calls the things an inbound group session,
that's where the name comes from, but it's not aligned with the
specification.
Why? Because `napi` and `wasm-bindgen` are too different. At this
step, the most notable bugs are the way `napi` is handling its proc
macros. There is too much conflicts when used with `#[cfg_attr]`. It
makes the code repetitive and harder to read and to understand (and
also to compile, we must be very careful). But on the short-term,
quickly, we will see more notable differences between `wasm-bindgen`
and `napi`, e.g. with array (in `wasm-bindgen`, we can downcast array
items into particular types, with `napi` it's going to be a very
different code).
Instead of fighting the proc macros bugs now, and having to split the
code later inside the same crate, we believe it's a good idea to split
the code now into 2 crates. At first we will see obvious code
duplications, but on the short-term, the code is likely to be more and
more different.
To implement this method, a new `responses::OwnedResponse`
intermediate type was needed. In addition to that, the `http` crate is
now required when the `js` feature is enabled.
`wasm_bindgen_future::future_to_promise` expects a `Future<Output =
Result<JsValue, JsValue>>`. We reimplement this function by expecting
a `Future<Output = Result<T, anyhow::Error>>` where `T:
Into<JsValue>`. That way, we apply the type conversions to `JsValue`
inside this helper rather than in the call site. Additionally, all
errors are managed automatically without having to deal with `JsError`
or `JsValue`. It makes the code simpler to read and easier to write
from my point of view.
The test json used for mocking a sync response contained an ill-formed
event with a room_id key present in the event. Since the deserialization
ignores the room_id key, this resulted in the client's membership state
evaluating to left going by the contents of the state events, despite
the room being in the "joined" section of the sync response. This is a
violation of the spec.
THe CryptoHolder is just used to hold the crypto-store till the
matrix_sdk::Session is set and it is possible to create the OlmMachine.
But with this approch we need to use a Mutex and getting the OlmMachine
becomes more expensive because of this. Therefore this replaces the
Mutex with a OnceCell. The only downside is that the BaseClient needs to
hold onto a referance to the crypto-store. (It would be possible to drop
it after the OlmMachine is created, but I don't think gives use any
improvment)
- [x] Make the `DisplayName` an actual type to allow API users to localize the generic name
- [x] tests for various naming use cases
- [x] fix the cases where we currently calculate a faulty name, fixes#133
- [x] strong-typed EitherMemberEvent from storage
- [x] add tests for eithermember in store integration tests
- [x] update sled store for new trait signature
- [x] update indexeddb store for new signature
- [x] rename `DisplayName::Computed` to `DisplayName::Calculated`
- [x] fix renaming failing test(s)
Merge pull request #591 from matrix-org/ben-feature-fixup
- [x] switches to latest rust 1.60 as MSRV
- [x] renames `encryption`-feature to `e2e-encryption` for clarity
- [x] combines the `*-state-store`/`*-crypto-store` features into a single feature `sled` and `indexeddb` respectivly
- [x] and combines them with weak `e2e-encryption`-dependencies to activate the features as necessary
- [x] moves the code back to the sled-store, where it belongs
- [x] updates the docs accordingly
BREAKING CHANGE
This patch moves all the important identities that we need for
verification into the already existing IdentitiesBeingVerified struct.
It adds a method to get those identities to the store, and streamlines
our to-device and in-room verification constructors.
- separate Styles from ci.yaml into its own for clarity
- extend pre-commit config with other useful defaults
- move clippy, tests and typos into a push-based pre-commit
This ensures that we're using the correct Session even if our store
doesn't provide those in the correct order. The set is small anyways, so
this shouldn't have any performance impact.
Merge pull request #586 from matrix-org/ben-remove-store-key
This removes the store_key and replaces it with the new key cipher for both sled and indexeddb state stores. If they are given a passphrase, we are now encrypted/hashing all keys and values stored in the store, no leaking of metadata in any key or value anymore.
This patch does a couple of things:
1. Fix the compilation of the example, Message -> MessageLike
2. Update Webpack and friends for the example
3. Remove futures-timer's Delay method and use equivalent methods from
Tokio and wasm-timers.
The last point was needed because futures-timer would end up triggering
this error:
ReferenceError: can't access lexical declaration '__wbg_clearTimeout_d8b36ad8fa330187' before initialization
This is needed because we now have multiple consumers of the
matrix-sdk-crypto crate in the workspace. The different consumers
might enable different feature sets.
Compilation of the workspace might fail if the qrcode feature is enabled
on the `matrix-sdk-crypto` crate but disabled on the `matrix-sdk` crate.
This patch adds a wildcard pattern to the Verification enum so we handle
the case where the matrix-sdk qrcode feature remains disabled.
This includes a few not-strictly-related changes that made sense to do at the
same time:
* Check for a functioning homeserver via get_supported_versions regardless of
whether a homeserver URL or user ID was supplied
* Rename use_discovery_response to respect_login_well_known
* Small appservice documentation improvement because those docs had to be
touched anyways
* Some test refactorings in tests that had to be touched anyways
CI passes `-D warnings` to clippy, so there it will be an error either way.
For local development it is nice not to block compilation of dependent
crates on lints though.
Simplifies the cache configuration and could also improve build times by
disabling incremental compilation and potentially caching fewer things
or using a better cache key.
Running CI on every push means running it twice for any PR as well as commit on branches that aren't PRs. This limits the CI run to pull-requests against main and pushes to main directly, removing unnecessary workload for the CI and making PRs CI faster.
In the latest version of Ruma, the event enums no longer implement
`Serialize` because it was somewhat of a footgun when custom /
unrecognized events were involved.
The test is only run when the `sled_state_store` feature is enabled.
This also changes the test_json::MORE_SYNC and test_json::MORE_SYNC_2 to
not responsed with a limited timeline.
This writes slices from StateChanges::timeline to the SledStore.
The cache is removed when ever a limited sync response is received or
when an event already known to the store is received.
Stored events are redacted when a redaction event is received.
Methods moved from Client to Account:
- display_name renamed to get_display_name
- set_display_name
- avatar_url renamed to get_avatar_url
- set_avatar_url
- avatar renamed to get_avatar
- upload_avatar
It looks like the force_auth is working correctly when restoring the
session, but it's not working with the call to client.login, so it
cannot be used.
The access_token is not available in the login query, so if the option
force_auth is set, this call will fail always. This patch just ignores
the force_auth if there's no session, so any query will work when
force_auth is true.
Fix https://github.com/matrix-org/matrix-rust-sdk/issues/488
`drain` is designed to be used when you want to reuse the `Vec` or only
move out a subset range. Using `.into_iter()` is more efficient and
idiomatic, and can be dropped entirely when used in `for` loops.
Done messages are supposed to indicate that the verification was
successful and _can be_ finalized. The correct time to send Done events
is after the user has confirmed the verification (after checking the
emoji or similar) and after receiving and checking a mac from the other
party (whatever happens later). Waiting for the other side to send Done
before sending it ourselves does not make sense.
Being in MacReceived state means that the other party has confirmed that
emojis/numbers match and the sent mac messages has been successfully
checked. When now confirming ourselves this means that the verification
was successful and thus the Done message can be sent to the other party.
When during verification we are the first party to complete the
verification workflow and then receive a (correct) MAC from the other
party, we need to send out the Done message even if we have not received
the Done message from the other party ourselves and are thus still
"WaitingForDone".
Previously, these methods returned `SyncRoomEvent`s instead, even though
the decrypted events are required to contain a room_id field (in order
for the server to not to be able to change these). As a side effect,
glue code which adds a room_id to the `SyncRoomEvent` to convert it to a
`RoomEvent` is be removed in some places.
When a `SyncRoomEvent` is required (such as in code handling sync
responses), the `From` implementation of `SyncRoomEvent` can be used.
According to the guide for implementing verification started from
a verification request, both parties should (or at least allowed to)
send a start request when the verification is ready. However, only the
start request from the party with lexicographically smaller user id (or
device id, for device verification, and thus equal user id) is supposed
to be accepted, and the other one ignored.
Previously, when a sas-workflow was started by the other party (e.g.
from a verification request), but the own start request was answered with
an "accept" this accept would be ignored. However, this is incorrect
since the official guide on implementing verification actually states
that both parties are expected to send start-requests, where of those
only one request is actually accepted (depending on user id and possibly
the device id).
Without this change, a batch with a message that cannot be deserialized
(for whatever reason) means that the batch cannot be processed at all by
the caller. Now, those messages are returned unchanged in the batch so
that the caller can handle them.
Due to additional error possibilities and encryption info for messages,
the return type of the method was changed accordingly and a wrapper
struct `Messages` was introduced.
Clients might want to retry decryption after they import room keys, for
this to be efficient they'll need to know which room keys got imported.
This patch extends the return value of the room key import result to
include the map of room keys that got imported.
Some clients might have skipped the serialization of an empty
sender_claimed_keys and the forwarding_curve25519_key_chain, while these
fields are required there's really no upside in requiring them if they
are empty.
So create defaults if they are missing.
Users of the crypto crate should mark all members that are part of an
E2EE room for tracking, this will of course mark your own user for
tracking.
If the crypto crate user doesn't have any E2EE rooms he may not add any
members to be tracked. The user might still want to inspect their own
E2EE devices and identities. This patch makes sure that our member is
tracked.
… by moving all of its fields into an inner reference-counted type
rather than using reference counting for all fields individually.
Some fields are still refcounted individually and thus go through an
extra layer of indirection, but the size and clone simplification should
still make this change worthwhile.
The new backup algorithm reuses the same key but uses a different
construct to encrypt/decrypt room keys. This reflects that our decrypt
method uses the v1 algorithm.
Instead of manually modifying *EventContent structs, the ruma
constructors *::encrypted and *::plain are used now depending on whether
encryption should be used for the attachment or not.
Some clients (including those using matrix-rust-sdk currently) may add
encrypted file info as well as the url to the encrypted blob to the
*EventContent. Interpreting the presence of an url as an indicator of
unencrypted message (like was done before) would result in access to the
encrypted blob in `get_media_content`.
Fixes#406.
This is helpful if the caller doesn't have propper request retrying.
Calling the outgoing_requests() method without actually uploading
all the requests would end up generating too many one-time keys and
removing some one-time keys that weren't used yet.
This would manifest itself down the line as undecryptable to-device
messages.
This changes `QrVerificationData::from_luma()` to accept any struct that
implements GenericImage and GenericImageView.
This way we don't force a specific container for the ImageBufffer, hence
the developer doesn't necessarily need to copy the image data into a
Vec<u8>.
And add some docs and logging statements.
cb8a42a529 broke encryption because it
elided the call to `send_raw` from `send` and instead did a direct HTTP
request. Since event encryption is only called from `send_raw`, this
made all events cleartext.
This fixes an edge case in the key sharing decision algorithm in which
your own trusted devices could be getting a limited session upon a
reshare. It also simplifies the algorithm a bit by bringing the check
for an own and trusted device to the start.
If the requesting device is a trusted/verified device of our own, we
share the session in full, regardless of whether we've previously shared
with that device and at which index. Otherwise, we do the "have we
shared previously" check via the outbound session and only share from
the index it was originally shared at, as before.
Signed-off-by: Denis Kasak <dkasak@termina.org.uk>
Since now sync_once() sends out E2EE requests after the sync and our
tests only set up the `/sync` endpoint using mockito we're going to be
stuck in a retry loop trying to send out the mentioned requests.
It's fine to disable request retrying and let those requests fail.
This would allow people to avoid the `sync()` method and let them drive
their own syncs.
Until now using `sync()` only would mean that the crypto requests would
never have been sent out.
This method is a bit dangerous if the room is encrypted, e.g. people can
send events before the room state has been fetched and thus accidentally
send out unencrypted events in an encrypted room.
This is a rough migration guide to help you upgrade your code using matrix-sdk 0.5 to the newly released matrix-sdk 0.6 . While it won't cover all edge cases and problems, we are trying to get the most common issues covered. If you experience any other difficulties in upgrade or need support with using the matrix-sdk in general, please approach us in our [matrix-sdk channel on matrix.org][matrix-channel].
## Minimum Supported Rust Version Update: `1.60`
We have updated the minimal rust version you need in order to build `matrix-sdk`, as we require some new dependency resolving features from it:
> These crates are built with the Rust language version 2021 and require a minimum compiler version of 1.60
## Dependencies
Many dependencies have been upgraded. Most notably, we are using `ruma` at version `0.7.0` now. It has seen some renamings and restructurings since our last release, so you might find that some Types have new names now.
## Repo Structure Updates
If you are looking at the repository itself, you will find we've rearranged the code quite a bit: we have split out any bindings-specific and testing related crates (and other things) into respective folders, and we've moved all `examples` into its own top-level-folder with each example as their own crate (rendering them easier to find and copy as starting points), all in all slimming down the `crates` folder to the core aspects.
## Architecture Changes / API overall
### Builder Pattern
We are moving to the [builder pattern][] (familiar from e.g. `std::io:process:Command`) as the main configurable path for many aspects of the API, including to construct Matrix-Requests and workflows. This has been and is an on-going effort, and this release sees a lot of APIs transitioning to this pattern, you should already be familiar with from the `matrix_sdk::Client::builder()` in `0.5`. This pattern been extended onto:
- the [login configuration][login builder] and [login with sso][ssologin builder],
Most have fallback (though maybe with deprecation warning) support for an existing code path, but these are likely to be removed in upcoming releases.
### Splitting of concerns: Media
In an effort to declutter the `Client` API dedicated types have been created dealing with specific concerns in one place. In `0.5` we introduced `client.account()`, and `client.encryption()`, we are doing the same with `client.media()` to manage media and attachments in one place with the [`media::Media` type][media typ] now.
The signatures of media uploads, have also changed slightly: rather than expecting a reader `R: Read + Seek`, it now is a simple `&[u8]`. Which also means no more unnecessary `seek(0)` to reset the cursor, as we are just taking an immutable reference now.
### Event Handling & sync updaes
If you are using the `client.register_event_handler` function to receive updates on incoming sync events, you'll find yourself with a deprecation warning now. That is because we've refactored and redesigned the event handler logic to allowing `removing` of event handlers on the fly, too. For that the new `add_event_handler()` (and `add_room_event_handler`) will hand you an `EventHandlerHandle` (pardon the pun), which you can pass to `remove_event_handler`, or by using the convenient `client.event_handler_drop_guard` to create a `DropGuard` that will remove the handler when the guard is dropped. While the code still works, we recommend you switch to the new one, as we will be removing the `register_event_handler` and `register_event_handler_context` in a coming release.
Secondly, you will find a new [`sync_with_result_callback` sync function][sync with result]. Other than the previous sync functions, this will pass the entire `Result` to your callback, allowing you to handle errors or even raise some yourself to stop the loop. Further more, it will propagate any unhandled errors (it still handles retries as before) to the outer caller, allowing the higher level to decide how to handle that (e.g. in case of a network failure). This result-returning-behavior also punshes through the existing `sync` and `sync_with_callback`-API, allowing you to handle them on a higher level now (rather than the futures just resolving). If you find that warning, just adding a `?` to the `.await` of the call is probably the quickest way to move forward.
### Refresh Tokens
This release now [supports `refresh_token`s][refresh tokens PR] as part of the [`Session`][session]. It is implemented with a default-flag in serde so deserializing a previously serialized Session (e.g. in a store) will work as before. As part of `refresh_token` support, you can now configure the client via `ClientBuilder.request_refresh_token()` to refresh the access token automagically on certain failures or do it manually by calling `client.refresh_access_token()` yourself. Auto-refresh is _off_ by default.
You can stay informed about updates on the access token by listening to `client.session_tokens_signal()`.
### Further changes
- [`MessageOptions`][message options] has been updated to Matrix 1.3 by making the `from` parameter optional (and function signatures have been updated, too). You can now request the server sends you messages from the first one you are allowed to have received.
-`client.user_id()` is not a `future` anymore. Remove any `.await` you had behind it.
-`verified()`, `blacklisted()` and `deleted()` on `matrix_sdk::encryption::identities::Device` have been renamed with a `is_` prefix.
-`verified()` on `matrix_sdk::encryption::identities::UserIdentity`, too has been prefixed with `is_` and thus is now called `is_verified()`.
- The top-level crypto and state-store types of Indexeddb and Sled have been renamed to unique types>
-`state_store` and `crypto_store` do not need to be boxed anymore when passed to the [`StoreConfig`][store config]
- Indexeddb's `SerializationError` is now `IndexedDBStoreError`
- Javascript specific features are now behind the `js` feature-gate
- The new experimental next generation of sync ("sliding sync"), with a totally revamped api, can be found behind the optional `sliding-sync`-feature-gate
## Quick Troubleshooting
You find yourself focused with any of these, here are the steps to follow to upgrade your code accordingly:
### warning: use of deprecated associated function `matrix_sdk::Client::register_event_handler`: Use [`Client::add_event_handler`](#method.add_event_handler) instead
As it says on the tin: we have deprecated this function in favor of the newer removable handler approach (see above). You can still continue to use this `fn` for now, but it will be removed in a future release.
### warning: use of deprecated associated function `matrix_sdk::Client::login`: Replaced by [`Client::login_username`](#method.login_username)
We have replaced the login facilities with a `LoginBuilder` and recommend you use that from now on. This isn't an error yet, but the function will be removed in a future release.
### expected slice `[u8]`, found struct ...
We've updated the `send_attachment` and `Media` signatures to use `&[u8]` rather than `reader: Read + Seek` as it is more convenient and common place for most architectures anyways. If you are using `File::open(path)?` to get that handler, you can just replace that with `std::fs::read(path)?`
### no method named `verified` found for struct `matrix_sdk::encryption::identities::Device` in the current scope
Boolean flags like `verified`, `deleted`, `blacklisted`, etc have been renamed with a `is_` prefix. So, just follow the cargo suggestion:
```
|
69 | device.verified()
| ^^^^^^^^ help: there is an associated function with a similar name: `is_verified`
Ruma has been updated to `0.7.0`, you will find some ruma Events names have changed, most notably, the `AnySyncRoomEvent` is now named `AnySyncTimelineEvent` (and not `AnySyncStateEvent`, which cargo wrongly suggests). Just rename the import and usage of it.
### `std::option::Option<&matrix_sdk::ruma::UserId>` is not a future
You are seeing something along the lines of:
```
19 | if room_member.state_key != client.user_id().await.unwrap() {
| ^^^^^^ `std::option::Option<&matrix_sdk::ruma::UserId>` is not a future
|
= help: the trait `Future` is not implemented for `std::option::Option<&matrix_sdk::ruma::UserId>`
= note: std::option::Option<&matrix_sdk::ruma::UserId> must be a future or must implement `IntoFuture` to be awaited
= note: required because of the requirements on the impl of `IntoFuture` for `std::option::Option<&matrix_sdk::ruma::UserId>`
help: remove the `.await`
|
19 - if room_member.state_key != client.user_id().await.unwrap() {
19 + if room_member.state_key != client.user_id().unwrap() {
```
You are using `client.user_id().await` but `user_id()` is no longer `async`. Just follow the cargo suggestion and remove the `.await`, it is not necessary any longer.
**matrix-rust-sdk** leverages [UniFFI](https://mozilla.github.io/uniffi-rs/) to generate bindings for host languages (eg. Swift and Kotlin).
Rust code related with bindings live in the [matrix-rust-sdk/bindings](https://github.com/matrix-org/matrix-rust-sdk/tree/main/bindings) folder.
Developers can expose Rust code to UniFFI using two different approaches:
- Using an `.udl` file. When a crate has one, you find it under the `src` folder (an example is [here](https://github.com/matrix-org/matrix-rust-sdk/blob/main/bindings/matrix-sdk-ffi/src/api.udl)).
- Add UniFFI directivies as Rust attributes. In this case Rust source files (`.rs`) contain attributes related to UniFFI (e.g. `#[uniffi::export]`). Attributes are preferred, where applicable.
## Expose Rust definitions to UniFFI
### Check if the API is already on UniFFI
First of all check if the Rust definition you are looking for exists on UniFFI already. Most of exposed matrix definitions are collected in the crate [matrix-sdk-ffi](https://github.com/matrix-org/matrix-rust-sdk/tree/main/bindings/matrix-sdk-ffi).
This crate contains mainly small Rust wrappers around the actual Rust SDK (e.g. the crate [matrix-sdk](https://github.com/matrix-org/matrix-rust-sdk/tree/main/crates/matrix-sdk))
If the Rust definition is on UniFFI already, you either:
- find it in a `.udl` file like [matrix-sdk-ffi/src/api.udl](https://github.com/matrix-org/matrix-rust-sdk/blob/main/bindings/matrix-sdk-ffi/src/api.udl)
- see it marked with a proper UniFFI Rust attribute like this `#[uniffi::export]`
### Adding a missing matrix API
1. Unless you want to contribute on the crypto side, you probably need to add some code in the [matrix-sdk-ffi](https://github.com/matrix-org/matrix-rust-sdk/tree/main/bindings/matrix-sdk-ffi) crate. After you find the crate you need to understand which file is best to contain the new Rust definition. When exposing new matrix API often (but not always) you need to touch the file [client.rs](https://github.com/matrix-org/matrix-rust-sdk/blob/main/bindings/matrix-sdk-ffi/src/client.rs)
2. Identify the API to expose in the target Rust crate (typically in [matrix-sdk](https://github.com/matrix-org/matrix-rust-sdk/tree/main/crates/matrix-sdk). If you can’t find it, you probably need to touch the actual Rust SDK as well. In this case you typically just need to write some code around [ruma](https://github.com/ruma/ruma) (a Rust SDK’s dependency) which already implements most of the matrix protocol
3. After you got (by finding or writing) the required Rust code, you need to expose to UniFFI. To do that just write a small Rust wrapper in the related UniFFI crate (most of the time is **matrix-sdk-ffi**) you found on step 1.
4. When your new (wrapping) Rust definition is ready, remember to expose it to UniFFI.
It’s best to do it using UniFFI Rust attributes (e.g. `#[uniffi::export]`). Otherwise add the new definition in the crate’s `.udl` file. For the **matrix-sdk-ffi** crate the definition file is [api.udl](https://github.com/matrix-org/matrix-rust-sdk/blob/main/bindings/matrix-sdk-ffi/src/api.udl). **Remember**: the language inside a `.udl` file isn’t Rust. To learn more about how map Rust into UDL read [here](https://mozilla.github.io/uniffi-rs/udl_file_spec.html)
## FAQ
**Q**: I wrote my Rust code and exposed it to UniFFI. How can I check if the compiler is happy?\
**A**: Run `cargo build` in the crate you touched (e.g. matrix-sdk-ffi). The compiler will complain if the Rust code and/or the `.udl` is wrong.
**Q**: The compiler is happy with my code but the CI is failing on GitHub. How can I fix it?\
**A**: The CI may fail for different reasons, you need to have a look on the failing GitHub workflow. One common reason though is that the linter ([Clippy](https://github.com/rust-lang/rust-clippy)) isn’t happy with your code. If this is the case, you can run `cargo clippy` in the crate you touched to see what’s wrong and fix it accordingly.
This project and build script demonstrate how to create an XCFramework that can be imported into an Xcode project and run on Apple platforms. It can compile and bundle an [entire SDK](#Building-the-SDK), or only a smaller [Crypto module](#Building-only-the-Crypto-SDK) that provides end-to-end encryption for clients that already depend on an SDK (e.g. [Matrix iOS SDK](https://github.com/matrix-org/matrix-ios-sdk))
## Building
### Prerequisites for building universal frameworks
-`xcodebuild` command line tool from [Apple](https://developer.apple.com/library/archive/technotes/tn2339/_index.html)
-`lipo` for creating the fat static libs
### Building the SDK
```
cargo xtask swift build-framework
```
The `build-framework` task will go through all the steps required to generate a fully usable `.xcframework`:
1. compile `matrix-sdk-ffi` libraries for iOS, the iOS simulator and macOS under `/target`. Some targets are not part of the standard library and they will be built using the nightly toolchain.
2.`lipo` together the libraries for the same platform under `/generated`
3. run `uniffi` and generate the C header, module map and swift files
4.`xcodebuild` an `xcframework` from the fat static libs and the original iOS one, and add the header and module map to it under `generated/MatrixSDKFFI.xcframework`
5. cleanup and delete the generated files except the .xcframework and the swift sources (that aren't part of the framework)
For development purposes, it will additionally generate a `Package.swift` file in the root of the repo that can be used to add the framework to your project and enable debugging through the use of [rust-xcode-plugin](https://github.com/BrainiumLLC/rust-xcode-plugin) (make sure to run the task with the argument `--profile=reldbg`).
When building the SDK for release you should pass the `--release` argument to the task, which will strip away any symbols and optimise the created binary.
### Building only the Crypto SDK
```
build_crypto_xcframework.sh
```
The `build_crypto_xcframework.sh` script will go through all the steps required to generate a fully usable `.xcframework`:
1. compile `matrix-sdk-crypto-ffi` libraries for iOS and the iOS simulator under `/target`
2.`lipo` together the libraries for the same platform under `/generated`
3. run `uniffi` and generate the C header, module map and swift files
4.`xcodebuild` an `xcframework` from the fat static libs and the original iOS one, and add the header and module map to it under `generated/MatrixSDKCryptoFFI.xcframework`
5. cleanup and delete the generated files except the .xcframework and the swift sources (that aren't part of the framework)
### Building & testing the Swift package
The `Package.swift` file in this directory provides a simple example on how to integrate everything together but also a place to run unit and integration tests from.
It's pre-configured to link to the generated static lib and .swift files so successfully running `cargo xtask swift build-library` first is necessary for it to compile. Afterwards you can execute the tests with `swift test`. Note that for the moment this only works on macOS but we're planning to add Linux support in the future.
## Distribution
The generated framework and Swift code can be distributed and integrated directly but in order to make things simpler we bundle them together as a [Swift package](https://github.com/matrix-org/matrix-rust-components-swift/) in the case of SDK, and as a CocoaPods podspec in the case of Crypto SDK.
### Publishing MatrixSDKCrypto
1. Navigate into `bindings/apple` and run [`build_crypto_xcframework.sh`](#building-only-the-crypto-sdk) script which generates a .zip file with the framework in `./generated` folder
Note that whilst you can run this command from any git branch, if you want to make a public release, ensure you are building from latest `main`
2. Tag the commit you just used to build the release and push the tag to GitHub
Use `matrix-sdk-crypto-ffi-<major>.<minor>.<patch>` tag name
3. Create a new [GitHub release](https://github.com/matrix-org/matrix-rust-sdk/releases) using this tag (see [example](https://github.com/matrix-org/matrix-rust-sdk/releases/tag/matrix-sdk-crypto-ffi-0.3.4))
4. Upload the previously generated .zip file to this release
5. Increment the version in [`MatrixSDKCrypto.podspec`](./MatrixSDKCrypto.podspec)
Note that this is not automated and is a local-only change. To determine the most recent published version, run `pod repo update && pod search MatrixSDKCrypto`
or check git tags via `git tag | grep matrix-sdk-crypto-ffi`
6. Push new Podspec version to Cocoapods via `pod trunk push MatrixSDKCrypto.podspec --allow-warnings`
This uses [`uniffi`](https://mozilla.github.io/uniffi-rs/Overview.html) to build the matrix bindings for native support and wasm-bindgen for web-browser assembly support. Please refer to the specific section to figure out how to build and use the bindings for your platform.
# OpenTelemetry support
The bindings have support for OpenTelemetry, allowing for the upload of traces to
any OpenTelemetry collector. This support is provided through the
outbound_exists [label="Session previously previously_shared with this user ID/device ID pair?"]
previously_shared [label="Requesting device sender (Curve25519) key matches the key we originally shared with?"]
own_device_check [label="Is this our own device?"]
/* End states */
allow_verified [label="Share the entire session from the earliest known index.\n\nOk(None)", color=4, fillcolor=3]
allow_limited [label="Share a limited session starting from index i, which is the index we previously shared at.\n\nOk(Some(i))", color=4, fillcolor=3]
refuse_device_key_changed [label="Sender key changed, refuse to share.\n\nErr(KeyForwardDecision::ChangedSenderKey)", color=6, fillcolor=5]
refuse_not_shared [label="Session was never shared with this device, refuse to share.\n\nErr(KeyForwardDecision::OutboundSessionNotShared)", color=6, fillcolor=5]
refuse_untrusted_own_device [label="Our own device, but it is untrusted and we haven't previously shared with it. Refuse to share.\n\nErr(KeyForwardDecision::UntrustedDevice)", color=6, fillcolor=5]
refuse_missing_outbound_session [label="Not our device and haven't previously shared with it. Refuse to share.\n\nErr(KeyForwardDecision::MissingOutboundSession)", color=6, fillcolor=5]
This crate implements the base to build a [Matrix](https://matrix.org/) client
library.
## Crate Feature Flags
The following crate feature flags are available:
*`encryption`: Enables end-to-end encryption support in the library.
*`qrcode`: Enables QRcode generation and reading code.
*`testing`: Provides facilities and functions for tests, in particular for integration testing store implementations. ATTENTION: do not ever use outside of tests, we do not provide any stability warantees on these, these are merely helpers. If you find you _need_ any function provided here outside of tests, please open a Github Issue and inform us about your use case for us to consider.
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Damir Jelić
