Fix error loading older sender data from storage #4425

andybalaam · 2024-12-17T15:50:14Z

richvdh · 2024-12-17T15:53:31Z

crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data.rs

+    #[serde(alias = "SenderUnverifiedButPreviouslyVerified")]
    VerificationViolation(KnownSenderData),


There are a whole bunch of other renames in #4067 that we need to check for; notably OwnUserIdentityVerifiedState (https://github.com/matrix-org/matrix-rust-sdk/pull/4067/files#diff-d28a9285dd9fd0d1ced29b1986dc92d7a754bb846a24cab48ec78073a1581166R874) which was the one that caused problems for Valere, but also VerificationLevel, and there may be others.

Sorry, was rushing to find a fix and missed the other places. Will expand this.

Added commits dealing with the other cases

poljar · 2024-12-17T15:57:46Z

This talks about a crash, but we just changed the serialization format, deserialization failures should not result in crashes. So we need to also find where the crash happens and convert that to an error.

codecov · 2024-12-17T16:05:50Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.40%. Comparing base (0ca35d6) to head (385c0ab).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4425   +/-   ##
=======================================
  Coverage   85.39%   85.40%           
=======================================
  Files         283      283           
  Lines       31472    31472           
=======================================
+ Hits        26877    26880    +3     
+ Misses       4595     4592    -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

andybalaam · 2024-12-17T17:38:42Z

This talks about a crash, but we just changed the serialization format, deserialization failures should not result in crashes. So we need to also find where the crash happens and convert that to an error.

It was a crash, right @BillCarsonFr ? Can you paste the stack trace?

BillCarsonFr · 2024-12-18T10:41:08Z

It was a crash, right @BillCarsonFr ? Can you paste the stack trace?

Yes all crashes https://github.com/element-hq/element-android-rageshakes/issues?q=is%3Aissue+is%3Aopen+VerifiedStateOrBool+

--------------------------------- crash call stack ---------------------------------
Element Build : 40106242
Element Version : 1.6.24 [40106242] (G-de6b7c53-2024-11-19 16:40:32 +0100)
SDK Version : 1.6.24 (de6b7c53)
Phone : SM-F741B (F741BXXS2AXKC 14 REL)
Memory statuses
usedSize 31 MB
freeSize 4 MB
totalSize 36 MB
Thread: main, Exception: org.matrix.rustcomponents.sdk.crypto.CryptoStoreException$CryptoStore: data did not match any variant of untagged enum VerifiedStateOrBool
at org.matrix.rustcomponents.sdk.crypto.FfiConverterTypeCryptoStoreError.read(SourceFile:8)
at org.matrix.rustcomponents.sdk.crypto.FfiConverterTypeCryptoStoreError.read(SourceFile:1)
at org.matrix.rustcomponents.sdk.crypto.FfiConverter$DefaultImpls.liftFromRustBuffer(SourceFile:13)
at org.matrix.rustcomponents.sdk.crypto.FfiConverterRustBuffer$DefaultImpls.liftFromRustBuffer(SourceFile:6)
at org.matrix.rustcomponents.sdk.crypto.FfiConverterTypeCryptoStoreError.liftFromRustBuffer(SourceFile:1)
at org.matrix.rustcomponents.sdk.crypto.FfiConverterRustBuffer$DefaultImpls.lift(SourceFile:6)
at org.matrix.rustcomponents.sdk.crypto.CryptoStoreException$ErrorHandler.lift(SourceFile:3)
at org.matrix.rustcomponents.sdk.crypto.CryptoStoreException$ErrorHandler.lift(SourceFile:1)
at org.matrix.rustcomponents.sdk.crypto.Matrix_sdk_crypto_ffiKt.uniffiCheckCallStatus(SourceFile:79)
at org.matrix.rustcomponents.sdk.crypto.OlmMachine.getUserDevices-Qn1smSk(SourceFile:72)
at org.matrix.android.sdk.internal.crypto.OlmMachine$getUserDevices$2.invokeSuspend(SourceFile:18)
at org.matrix.android.sdk.internal.crypto.OlmMachine$getUserDevices$2.invoke(SourceFile:2)
at org.matrix.android.sdk.internal.crypto.OlmMachine$getUserDevices$2.invoke(SourceFile:1)
at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(SourceFile:17)
at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(SourceFile:52)
at org.matrix.android.sdk.internal.crypto.OlmMachine.getUserDevices(SourceFile:11)
at org.matrix.android.sdk.internal.crypto.OlmMachine.getCryptoDeviceInfo(SourceFile:6)
at org.matrix.android.sdk.internal.crypto.RustCryptoService$getCryptoDeviceInfo$4.invokeSuspend(SourceFile:35)
at org.matrix.android.sdk.internal.crypto.RustCryptoService$getCryptoDeviceInfo$4.invoke(SourceFile:2)
at org.matrix.android.sdk.internal.crypto.RustCryptoService$getCryptoDeviceInfo$4.invoke(SourceFile:1)
at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(SourceFile:17)
at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(SourceFile:52)
at org.matrix.android.sdk.internal.crypto.RustCryptoService.getCryptoDeviceInfo(SourceFile:9)
at org.matrix.android.sdk.flow.FlowSession$liveUserCryptoDevices$1.invokeSuspend(SourceFile:37)
at org.matrix.android.sdk.flow.FlowSession$liveUserCryptoDevices$1.invoke(SourceFile:2)
at org.matrix.android.sdk.flow.FlowSession$liveUserCryptoDevices$1.invoke(SourceFile:1)
at org.matrix.android.sdk.flow.FlowExtKt$startWith$1$value$1.invokeSuspend(SourceFile:29)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(SourceFile:12)
at kotlinx.coroutines.DispatchedTask.run(SourceFile:129)
at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(SourceFile:4)
at kotlinx.coroutines.scheduling.TaskImpl.run(SourceFile:3)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(SourceFile:1)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(SourceFile:15)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(SourceFile:29)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(SourceFile:1)
Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@9e63343, Dispatchers.Main.immediate]

richvdh

LGTM

crates/matrix-sdk-common/src/deserialized_responses.rs

poljar · 2024-12-18T10:46:19Z

It was a crash, right @BillCarsonFr ? Can you paste the stack trace?

Yes all crashes https://github.com/element-hq/element-android-rageshakes/issues?q=is%3Aissue+is%3Aopen+VerifiedStateOrBool+

--------------------------------- crash call stack ---------------------------------
Element Build : 40106242
Element Version : 1.6.24 [40106242] (G-de6b7c53-2024-11-19 16:40:32 +0100)
SDK Version : 1.6.24 (de6b7c53)
Phone : SM-F741B (F741BXXS2AXKC 14 REL)
Memory statuses
usedSize 31 MB
freeSize 4 MB
totalSize 36 MB
Thread: main, Exception: org.matrix.rustcomponents.sdk.crypto.CryptoStoreException$CryptoStore: data did not match any variant of untagged enum VerifiedStateOrBool
at org.matrix.rustcomponents.sdk.crypto.FfiConverterTypeCryptoStoreError.read(SourceFile:8)
at org.matrix.rustcomponents.sdk.crypto.FfiConverterTypeCryptoStoreError.read(SourceFile:1)
at org.matrix.rustcomponents.sdk.crypto.FfiConverter$DefaultImpls.liftFromRustBuffer(SourceFile:13)
at org.matrix.rustcomponents.sdk.crypto.FfiConverterRustBuffer$DefaultImpls.liftFromRustBuffer(SourceFile:6)
at org.matrix.rustcomponents.sdk.crypto.FfiConverterTypeCryptoStoreError.liftFromRustBuffer(SourceFile:1)
at org.matrix.rustcomponents.sdk.crypto.FfiConverterRustBuffer$DefaultImpls.lift(SourceFile:6)
at org.matrix.rustcomponents.sdk.crypto.CryptoStoreException$ErrorHandler.lift(SourceFile:3)
at org.matrix.rustcomponents.sdk.crypto.CryptoStoreException$ErrorHandler.lift(SourceFile:1)
at org.matrix.rustcomponents.sdk.crypto.Matrix_sdk_crypto_ffiKt.uniffiCheckCallStatus(SourceFile:79)
at org.matrix.rustcomponents.sdk.crypto.OlmMachine.getUserDevices-Qn1smSk(SourceFile:72)
at org.matrix.android.sdk.internal.crypto.OlmMachine$getUserDevices$2.invokeSuspend(SourceFile:18)
at org.matrix.android.sdk.internal.crypto.OlmMachine$getUserDevices$2.invoke(SourceFile:2)
at org.matrix.android.sdk.internal.crypto.OlmMachine$getUserDevices$2.invoke(SourceFile:1)
at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(SourceFile:17)
at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(SourceFile:52)
at org.matrix.android.sdk.internal.crypto.OlmMachine.getUserDevices(SourceFile:11)
at org.matrix.android.sdk.internal.crypto.OlmMachine.getCryptoDeviceInfo(SourceFile:6)
at org.matrix.android.sdk.internal.crypto.RustCryptoService$getCryptoDeviceInfo$4.invokeSuspend(SourceFile:35)
at org.matrix.android.sdk.internal.crypto.RustCryptoService$getCryptoDeviceInfo$4.invoke(SourceFile:2)
at org.matrix.android.sdk.internal.crypto.RustCryptoService$getCryptoDeviceInfo$4.invoke(SourceFile:1)
at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(SourceFile:17)
at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(SourceFile:52)
at org.matrix.android.sdk.internal.crypto.RustCryptoService.getCryptoDeviceInfo(SourceFile:9)
at org.matrix.android.sdk.flow.FlowSession$liveUserCryptoDevices$1.invokeSuspend(SourceFile:37)
at org.matrix.android.sdk.flow.FlowSession$liveUserCryptoDevices$1.invoke(SourceFile:2)
at org.matrix.android.sdk.flow.FlowSession$liveUserCryptoDevices$1.invoke(SourceFile:1)
at org.matrix.android.sdk.flow.FlowExtKt$startWith$1$value$1.invokeSuspend(SourceFile:29)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(SourceFile:12)
at kotlinx.coroutines.DispatchedTask.run(SourceFile:129)
at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(SourceFile:4)
at kotlinx.coroutines.scheduling.TaskImpl.run(SourceFile:3)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(SourceFile:1)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(SourceFile:15)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(SourceFile:29)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(SourceFile:1)
Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@9e63343, Dispatchers.Main.immediate]

Alright, but that's EA not catching an exception, right? The SDK itself doesn't crash.

andybalaam · 2024-12-18T10:50:40Z

Alright, but that's EA not catching an exception, right? The SDK itself doesn't crash.

Correct. I fixed the PR description and will fix the commits in my rebase.

…deserializing

…l when deserializing

…when deserializing

…iouslyVerifiedButNoLonger value

andybalaam · 2024-12-18T11:13:19Z

Pausing this PR so we can make a release branch to release this for crypto-wasm based on the version used in matrix-rust-sdk-crypto-wasm 11.0 (70bcddf)

poljar · 2024-12-18T11:22:36Z

Err, please don't pause it? The WASM bindings are not the only ones affected by this.

andybalaam · 2024-12-18T11:28:40Z

I merged this code into release-for-crypto-wasm-11 in #4430 , then in merged that branch into main in commit 0394761

andybalaam requested review from a team as code owners December 17, 2024 15:50

andybalaam requested review from Hywan and poljar and removed request for a team December 17, 2024 15:50

richvdh requested changes Dec 17, 2024

View reviewed changes

poljar removed the request for review from Hywan December 17, 2024 15:55

andybalaam requested a review from richvdh December 18, 2024 10:27

andybalaam changed the title ~~fix(crypto): Fix crash loading older sender data from storage~~ Fix error loading older sender data from storage Dec 18, 2024

richvdh approved these changes Dec 18, 2024

View reviewed changes

crates/matrix-sdk-common/src/deserialized_responses.rs Show resolved Hide resolved

andybalaam added 4 commits December 18, 2024 10:51

task(crypto): Accept old PreviouslyVerified value of SenderData when …

10ed99d

…deserializing

task(crypto): Accept old PreviouslyVerified value of VerificationLeve…

3ace3bc

…l when deserializing

task(crypto): Accept old PreviouslyVerified value of ShieldStateCode …

3c9aa7e

…when deserializing

fix(crypto): Fix error when reading VerifiedStateOrBool with old Prev…

d17ba70

…iouslyVerifiedButNoLonger value

andybalaam force-pushed the andybalaam/migration-for-previouslyverified branch from 785504d to d17ba70 Compare December 18, 2024 10:52

andybalaam enabled auto-merge (rebase) December 18, 2024 10:52

poljar approved these changes Dec 18, 2024

View reviewed changes

andybalaam disabled auto-merge December 18, 2024 11:03

andybalaam closed this Dec 18, 2024

andybalaam mentioned this pull request Dec 18, 2024

Fix error loading older serialized data from storage #4430

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix error loading older sender data from storage #4425

Fix error loading older sender data from storage #4425

andybalaam commented Dec 17, 2024 •

edited

Loading

richvdh Dec 17, 2024

andybalaam Dec 17, 2024

andybalaam Dec 18, 2024

poljar commented Dec 17, 2024

codecov bot commented Dec 17, 2024

andybalaam commented Dec 17, 2024

BillCarsonFr commented Dec 18, 2024 •

edited

Loading

richvdh left a comment

poljar commented Dec 18, 2024

andybalaam commented Dec 18, 2024

andybalaam commented Dec 18, 2024

poljar commented Dec 18, 2024

andybalaam commented Dec 18, 2024 •

edited

Loading

		#[serde(alias = "SenderUnverifiedButPreviouslyVerified")]
		VerificationViolation(KnownSenderData),

Fix error loading older sender data from storage #4425

Fix error loading older sender data from storage #4425

Conversation

andybalaam commented Dec 17, 2024 • edited Loading

richvdh Dec 17, 2024

Choose a reason for hiding this comment

andybalaam Dec 17, 2024

Choose a reason for hiding this comment

andybalaam Dec 18, 2024

Choose a reason for hiding this comment

poljar commented Dec 17, 2024

codecov bot commented Dec 17, 2024

Codecov Report

andybalaam commented Dec 17, 2024

BillCarsonFr commented Dec 18, 2024 • edited Loading

richvdh left a comment

Choose a reason for hiding this comment

poljar commented Dec 18, 2024

andybalaam commented Dec 18, 2024

andybalaam commented Dec 18, 2024

poljar commented Dec 18, 2024

andybalaam commented Dec 18, 2024 • edited Loading

andybalaam commented Dec 17, 2024 •

edited

Loading

BillCarsonFr commented Dec 18, 2024 •

edited

Loading

andybalaam commented Dec 18, 2024 •

edited

Loading