Skip to content

Conversation

@poljar
Copy link
Contributor

@poljar poljar commented Apr 23, 2025

This patch updates the sending side of the sender_device_keys field introduced in MSC4147.

Since the MSC got merged, we're switching from the unstable identifier to the stable one.

A couple of snapshot tests were added modified to make this happen.

  • Public API changes documented in changelogs (optional)

@poljar poljar requested review from a team as code owners April 23, 2025 12:00
@poljar poljar requested review from bnjbvr and richvdh and removed request for a team April 23, 2025 12:00
@poljar
Copy link
Contributor Author

poljar commented Apr 23, 2025

Should close: element-hq/element-meta#2683.

@codecov
Copy link

codecov bot commented Apr 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.97%. Comparing base (6d1c24f) to head (824102b).
Report is 32 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4964      +/-   ##
==========================================
+ Coverage   85.92%   85.97%   +0.04%     
==========================================
  Files         325      325              
  Lines       35649    35665      +16     
==========================================
+ Hits        30633    30664      +31     
+ Misses       5016     5001      -15     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

… keys

This patch updates the sending side of the `sender_device_keys` field
introduced in MSC4147.

Since the MSC got merged, we're switching from the unstable identifier
to the stable one.

A couple of snapshot tests were added modified to make this happen.
@poljar poljar force-pushed the poljar/stable-identifier-msc-4147 branch from 422bc96 to 824102b Compare April 23, 2025 12:19
Copy link
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have absolutely no clues about what this does, but I assume you do, and the code looks good to me 👍

(In case this question is useful: since the OlmV1Event seem to be serialized/deserialized, this might mean that a previous sender_device_keys inserted before this patch will be missing after this patch: is this OK, or does this warrant a data migration?)

@poljar
Copy link
Contributor Author

poljar commented Apr 23, 2025

I have absolutely no clues about what this does, but I assume you do, and the code looks good to me 👍

It just renames an identifier, nothing fancy.

(In case this question is useful: since the OlmV1Event seem to be serialized/deserialized, this might mean that a previous sender_device_keys inserted before this patch will be missing after this patch: is this OK, or does this warrant a data migration?)

Hmm, I don't think it will be missing after this patch. The deserialization is still using a serde alias:

#[serde(alias = "org.matrix.msc4147.device_keys")]
pub sender_device_keys: Option<DeviceKeys>,

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@poljar poljar merged commit a60e336 into main Apr 23, 2025
42 checks passed
@poljar poljar deleted the poljar/stable-identifier-msc-4147 branch April 23, 2025 13:25
richvdh added a commit to matrix-org/matrix-js-sdk that referenced this pull request Jun 21, 2025
For js-sdk users, this includes the following:

    -   Send stable identifier `sender_device_keys` for MSC4147 (Including device keys with Olm-encrypted events).
        ([#4964](matrix-org/matrix-rust-sdk#4964))

    -   Check the `sender_device_keys` field on _all_ incoming Olm-encrypted to-device messages and ignore any to-device messages which include the field but whose data is invalid (as per [MSC4147](matrix-org/matrix-spec-proposals#4147)).
        ([#4922](matrix-org/matrix-rust-sdk#4922))

    -   Fix bug which caused room keys to be unnecessarily rotated on every send in the presence of blacklisted/withheld devices in the room.
        ([#4954](matrix-org/matrix-rust-sdk#4954))

    -   Fix [matrix-rust-sdk#2729](matrix-org/matrix-rust-sdk#2729) which in rare cases can cause room key oversharing.
        ([#4975](matrix-org/matrix-rust-sdk#4975))
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this pull request Jun 23, 2025
For js-sdk users, this includes the following:

    -   Send stable identifier `sender_device_keys` for MSC4147 (Including device keys with Olm-encrypted events).
        ([#4964](matrix-org/matrix-rust-sdk#4964))

    -   Check the `sender_device_keys` field on _all_ incoming Olm-encrypted to-device messages and ignore any to-device messages which include the field but whose data is invalid (as per [MSC4147](matrix-org/matrix-spec-proposals#4147)).
        ([#4922](matrix-org/matrix-rust-sdk#4922))

    -   Fix bug which caused room keys to be unnecessarily rotated on every send in the presence of blacklisted/withheld devices in the room.
        ([#4954](matrix-org/matrix-rust-sdk#4954))

    -   Fix [matrix-rust-sdk#2729](matrix-org/matrix-rust-sdk#2729) which in rare cases can cause room key oversharing.
        ([#4975](matrix-org/matrix-rust-sdk#4975))
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this pull request Jun 23, 2025
For js-sdk users, this includes the following:

    -   Send stable identifier `sender_device_keys` for MSC4147 (Including device keys with Olm-encrypted events).
        ([#4964](matrix-org/matrix-rust-sdk#4964))

    -   Check the `sender_device_keys` field on _all_ incoming Olm-encrypted to-device messages and ignore any to-device messages which include the field but whose data is invalid (as per [MSC4147](matrix-org/matrix-spec-proposals#4147)).
        ([#4922](matrix-org/matrix-rust-sdk#4922))

    -   Fix bug which caused room keys to be unnecessarily rotated on every send in the presence of blacklisted/withheld devices in the room.
        ([#4954](matrix-org/matrix-rust-sdk#4954))

    -   Fix [matrix-rust-sdk#2729](matrix-org/matrix-rust-sdk#2729) which in rare cases can cause room key oversharing.
        ([#4975](matrix-org/matrix-rust-sdk#4975))
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this pull request Jun 23, 2025
For js-sdk users, this includes the following:

    -   Send stable identifier `sender_device_keys` for MSC4147 (Including device keys with Olm-encrypted events).
        ([#4964](matrix-org/matrix-rust-sdk#4964))

    -   Check the `sender_device_keys` field on _all_ incoming Olm-encrypted to-device messages and ignore any to-device messages which include the field but whose data is invalid (as per [MSC4147](matrix-org/matrix-spec-proposals#4147)).
        ([#4922](matrix-org/matrix-rust-sdk#4922))

    -   Fix bug which caused room keys to be unnecessarily rotated on every send in the presence of blacklisted/withheld devices in the room.
        ([#4954](matrix-org/matrix-rust-sdk#4954))

    -   Fix [matrix-rust-sdk#2729](matrix-org/matrix-rust-sdk#2729) which in rare cases can cause room key oversharing.
        ([#4975](matrix-org/matrix-rust-sdk#4975))
toger5 pushed a commit to matrix-org/matrix-js-sdk that referenced this pull request Jul 18, 2025
For js-sdk users, this includes the following:

    -   Send stable identifier `sender_device_keys` for MSC4147 (Including device keys with Olm-encrypted events).
        ([#4964](matrix-org/matrix-rust-sdk#4964))

    -   Check the `sender_device_keys` field on _all_ incoming Olm-encrypted to-device messages and ignore any to-device messages which include the field but whose data is invalid (as per [MSC4147](matrix-org/matrix-spec-proposals#4147)).
        ([#4922](matrix-org/matrix-rust-sdk#4922))

    -   Fix bug which caused room keys to be unnecessarily rotated on every send in the presence of blacklisted/withheld devices in the room.
        ([#4954](matrix-org/matrix-rust-sdk#4954))

    -   Fix [matrix-rust-sdk#2729](matrix-org/matrix-rust-sdk#2729) which in rare cases can cause room key oversharing.
        ([#4975](matrix-org/matrix-rust-sdk#4975))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants