-
Notifications
You must be signed in to change notification settings - Fork 269
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.
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
crypto: change withheld code for IdentityBased share strategy #3985
crypto: change withheld code for IdentityBased share strategy #3985
Conversation
99f9d3c
to
3d545a1
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3985 +/- ##
==========================================
- Coverage 84.54% 84.49% -0.05%
==========================================
Files 266 266
Lines 28478 28478
==========================================
- Hits 24076 24063 -13
- Misses 4402 4415 +13 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me but can we have an explanation as to why this happened?
You can find more here https://github.com/element-hq/element-web-rageshakes/issues/27055#issuecomment-2344092110 TL;DR It's for the new invisible crypto, and I was not sure of what withheld code would be the best to return (unauthorised, unverified, a new one?). After some discussion with @richvdh we decided to use |
Even more context:
Nevertheless, it feels less bad than co-opting Ideally, we would have a different withheld code, more relevant to this case, but doing so would require spec changes and a bunch of work on the recipient side to handle the new code. Using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise
3d545a1
to
2bb0c50
Compare
Mostly, to pick up matrix-org/matrix-rust-sdk#3985
Mostly, to pick up matrix-org/matrix-rust-sdk#3985
Change the withheld code for the
IdentityBasedStrategy
fromWithheldCode::Unauthorised
to
WithheldCode::Unverified
.Signed-off-by: