Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: delay outgoing message before sync is finished [#WPB-15141] #3216

Merged
merged 1 commit into from
Jan 9, 2025
Merged

fix: delay outgoing message before sync is finished [#WPB-15141] #3216

merged 1 commit into from
Jan 9, 2025

Conversation

sbakhtiarov
Copy link
Contributor

@sbakhtiarov sbakhtiarov commented Jan 7, 2025 •
edited by github-actions bot
Loading

BugWPB-15141 [Android] After playtest, coming back to the app, getting a lot of "Cannot decrypt" system messages

https://wearezeta.atlassian.net/browse/WPB-15141

What's new in this PR?

Issues

Preconditions:

  • Client was offline for a long time and has a lot of messages to sync.
  • Conversation has self deleting messages with short expiration.

User opens the app, sync is started and user see some messages that cannot be decrypted in the conversation.

Causes

  • Client receives a self delete message that expires before the sync is complete
  • Client sends delete message to sender to confirm message deletion before sync is finished.
  • Delete message is sent with old epoch value.
  • Client receives 'mls-stale-message' error and re-joins conversation.
  • All subsequent messages cannot be decrypted after re-join.

Solutions

Delay sending delete message until sync is complete. This ensures that message is sent with correct epoch value.

The issue has a broader context. Any message a client sends before sync is complete can cause re-join and break decryption. This should be covered as a part of: https://wearezeta.atlassian.net/browse/WPB-15262

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Jan 7, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 7, 2025 •
edited
Loading

Test Results

3 346 tests  ±0   3 238 ✅ ±0   5m 3s ⏱️ -46s
  572 suites ±0     108 💤 ±0 
  572 files   ±0       0 ❌ ±0 

Results for commit 3892e6c. ± Comparison against base commit 70b8394.

♻️ This comment has been updated with latest results.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.

Project coverage is 54.18%. Comparing base (70b8394) to head (3892e6c).

Files with missing lines Patch % Lines
.../com/wire/kalium/logic/feature/debug/DebugScope.kt 0.00% 2 Missing ⚠️
.../wire/kalium/logic/feature/message/MessageScope.kt 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@                Coverage Diff                 @@
##           release/candidate    #3216   +/-   ##
==================================================
  Coverage              54.17%   54.18%           
==================================================
  Files                   1250     1250           
  Lines                  36514    36518    +4     
  Branches                3696     3696           
==================================================
+ Hits                   19783    19788    +5     
+ Misses                 15304    15303    -1     
  Partials                1427     1427
Files with missing lines Coverage Δ
...eteEphemeralMessageForSelfUserAsReceiverUseCase.kt 94.28% <100.00%> (+0.16%) ⬆️
.../com/wire/kalium/logic/feature/debug/DebugScope.kt 0.00% <0.00%> (ø)
.../wire/kalium/logic/feature/message/MessageScope.kt 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70b8394...3892e6c. Read the comment docs.

@datadog-wireapp
Copy link

Datadog Report

Branch report: fix/outgoing-message-before-sync-finished
Commit report: b79d81f
Test service: kalium-jvm

✅ 0 Failed, 3238 Passed, 108 Skipped, 59.55s Total Time

@sbakhtiarov sbakhtiarov added this pull request to the merge queue Jan 9, 2025
Merged via the queue into release/candidate with commit 00bc08e Jan 9, 2025
22 checks passed
@sbakhtiarov sbakhtiarov deleted the fix/outgoing-message-before-sync-finished branch January 9, 2025 11:49
github-actions bot pushed a commit that referenced this pull request Jan 9, 2025
@sbakhtiarov @github-actions
fix: delay outgoing message before sync is finished [#WPB-15141] (#3216)
400c3ac
@github-actions github-actions bot mentioned this pull request Jan 9, 2025
Open
@echoes-hq echoes-hq bot added the echoes: product-roadmap/bug Work contributing to resolve a bug not critical enough to have raised an incident. label Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vitorhugods vitorhugods vitorhugods approved these changes

@yamilmedina yamilmedina yamilmedina approved these changes

Assignees

@sbakhtiarov sbakhtiarov

Labels
echoes: product-roadmap/bug Work contributing to resolve a bug not critical enough to have raised an incident. 🚨 Potential breaking changes 👕 size: S type: bug / fix 🐞
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sbakhtiarov @codecov-commenter @yamilmedina @vitorhugods