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.

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

Add TestNotificationClientDupeOTKUpload #134

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kegsay
Copy link
Member

@kegsay kegsay commented Sep 13, 2024

This currently fails when used in single process mode.

Test that the notification client doesn't cause duplicate OTK uploads.
Regression test for matrix-org/matrix-rust-sdk#1415

This test creates a normal Alice rust client and lets it upload OTKs. It then:

  • hooks into /keys/upload requests and artificially delays them by adding 4s of latency
  • creates a Bob client who sends a message to Alice, consuming 1 OTK in the process
  • immediately calls GetNotification on Bob's event as soon as it 200 OKs, which creates
    a NotificationClient inside the same process.

This means there are 2 sync loops: the main Client and the NotificationClient. Both clients
will see the OTK count being lowered so both may try to upload a new OTK. Because we are
delaying upload requests by 4s, this increases the chance of both uploads being in-flight at
the same time. If they do not synchronise this operation, they will both try to upload
different keys with the same key ID, which causes synapse to HTTP 400 with:

 One time key signed_curve25519:AAAAAAAAADI already exists

Which will fail the test.

This currently fails when used in single process mode.
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.

1 participant