Add support for MSC4140 (Delayed Events) #1840

Open
eve wants to merge 7 commits from eve/ackduck:I903 into main
Contributor

Implements #903

Submitting this as WIP as I wait for https://github.com/ruma/ruma/pull/2493 to be merged, although it is mostly finished.

(also depends on #1799)

I have tested this and it seems to work, though I still intend to do some more thorough testing. In the meantime, feel free to look at the general design choices and see if they seem good to you.

This pull request...

Pull request checklist:

  • This pull request targets the main branch, and the branch is named something other than
    main.
  • I have written an appropriate pull request title and my description is clear.
  • I understand I am responsible for the contents of this pull request.
  • I have followed the contributing guidelines:
Implements #903 Submitting this as WIP as I wait for https://github.com/ruma/ruma/pull/2493 to be merged, although it is mostly finished. (also depends on https://forgejo.ellis.link/continuwuation/continuwuity/pulls/1799) I have tested this and it seems to work, though I still intend to do some more thorough testing. In the meantime, feel free to look at the general design choices and see if they seem good to you. <!-- In order to help reviewers know what your pull request does at a glance, you should ensure that 1. Your PR title is a short, single sentence describing what you changed 2. You have described in more detail what you have changed, why you have changed it, what the intended effect is, and why you think this will be beneficial to the project. If you have made any potentially strange/questionable design choices, but didn't feel they'd benefit from code comments, please don't mention them here - after opening your pull request, go to "files changed", and click on the "+" symbol in the line number gutter, and attach comments to the lines that you think would benefit from some clarification. --> This pull request... <!-- Example: This pull request allows us to warp through time and space ten times faster than before by double-inverting the warp drive with hyperheated jump fluid, both making the drive faster and more efficient. This resolves the common issue where we have to wait more than 10 milliseconds to engage, use, and disengage the warp drive when travelling between galaxies. --> <!-- Closes: #... --> <!-- Fixes: #... --> <!-- Uncomment the above line(s) if your pull request fixes an issue or closes another pull request by superseding it. Replace `#...` with the issue/pr number, such as `#123`. --> **Pull request checklist:** <!-- You need to complete these before your PR can be considered. If you aren't sure about some, feel free to ask for clarification in #dev:continuwuity.org. --> - [x] This pull request targets the `main` branch, and the branch is named something other than `main`. - [x] I have written an appropriate pull request title and my description is clear. - [x] I understand I am responsible for the contents of this pull request. - I have followed the [contributing guidelines][c1]: - [x] My contribution follows the [code style][c2], if applicable. - [x] I ran [pre-commit checks][c1pc] before opening/drafting this pull request. - [ ] I have [tested my contribution][c1t] (or proof-read it for documentation-only changes) myself, if applicable. This includes ensuring code compiles. - [x] My commit messages follow the [commit message format][c1cm] and are descriptive. <!-- Notes on these requirements: - While not required, we encourage you to sign your commits with GPG or SSH to attest the authenticity of your changes. - While we allow LLM-assisted contributions, we do not appreciate contributions that are low quality, which is typical of machine-generated contributions that have not had a lot of love and care from a human. Please do not open a PR if all you have done is asked ChatGPT to tidy up the codebase with a +-100,000 diff. - In the case of code style violations, reviewers may leave review comments/change requests indicating what the ideal change would look like. For example, a reviewer may suggest you lower a log level, or use `match` instead of `if/else` etc. - In the case of code style violations, pre-commit check failures, minor things like typos/spelling errors, and in some cases commit format violations, reviewers may modify your branch directly, typically by making changes and adding a commit. Particularly in the latter case, a reviewer may rebase your commits to squash "spammy" ones (like "fix", "fix", "actually fix"), and reword commit messages that don't satisfy the format. - Pull requests MUST pass the `Checks` CI workflows to be capable of being merged. This can only be bypassed in exceptional circumstances. If your CI flakes, let us know in matrix:r/dev:continuwuity.org. - Pull requests have to be based on the latest `main` commit before being merged. If the main branch changes while you're making your changes, you should make sure you rebase on main before opening a PR. Your branch will be rebased on main before it is merged if it has fallen behind. - We typically only do fast-forward merges, so your entire commit log will be included. Once in main, it's difficult to get out cleanly, so put on your best dress, smile for the cameras! --> [c1]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md [c2]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/docs/development/code_style.mdx [c1pc]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#pre-commit-checks [c1t]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#running-tests-locally [c1cm]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#commit-messages
refactor: Use a shared helper function for submitting state events at room creation
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 7s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 9s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 7s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m30s
9cea8d2080
On room creation, A list of initial state events may be provided that
should be submitted to the room after creation. These events
should be treated as any other state event and submitted to
the same checks.

With this change, state events submitted on room creation will be
submitted through the same helper as those through the usual endpoint.

The submission helper is moved to the timeline service to make it
available everywhere. This will be useful for implementing MSC4140
(issue #903).
eve force-pushed I903 from 00fe46af71
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Auto Labeler / Apply labels based on changed files (pull_request_target) Successful in 5s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 9s
Checks / Prek / Check changed files (pull_request) Failing after 7s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 10s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been skipped
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m31s
to 2de95ff2fb
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 9s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m17s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m24s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 12m14s
2026-06-12 15:49:05 +00:00
Compare
eve force-pushed I903 from 2de95ff2fb
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 9s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m17s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m24s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 12m14s
to abd2bf2617
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 10s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m8s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m28s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 8m54s
2026-06-13 15:24:59 +00:00
Compare
eve force-pushed I903 from abd2bf2617
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 10s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m8s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m28s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 8m54s
to 32ff746032
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m6s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m8s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 3m15s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 7s
2026-06-13 17:29:45 +00:00
Compare
Member

The Ruma PR looks to have been merged, is this waiting on anything before you can resume work on it?

The Ruma PR looks to have been merged, is this waiting on anything before you can resume work on it?
First-time contributor

@Henry-Hiles wrote in #1840 (comment):

The Ruma PR looks to have been merged, is this waiting on anything before you can resume work on it?

Seems like it's waiting on #1799

@Henry-Hiles wrote in https://forgejo.ellis.link/continuwuation/continuwuity/pulls/1840#issuecomment-31512: > The Ruma PR looks to have been merged, is this waiting on anything before you can resume work on it? Seems like it's waiting on #1799
Author
Contributor

@Henry-Hiles It's waiting on me making the final missing changes and tests. Been kind of busy, sorry. Will get around to it soon.

@Henry-Hiles It's waiting on me making the final missing changes and tests. Been kind of busy, sorry. Will get around to it soon.
eve force-pushed I903 from 32ff746032
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m6s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m8s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 3m15s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 7s
to 615199a46b
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 7s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m18s
Checks / Prek / Check changed files (pull_request) Successful in 7s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m28s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
Deploy Element Web / 🏗️ Build and Deploy (pull_request) Failing after 6m52s
2026-08-16 15:39:28 +00:00
Compare
eve changed title from WIP: add support for MSC4140 (Delayed Events) to Add support for MSC4140 (Delayed Events) 2026-08-16 15:40:14 +00:00
Author
Contributor

Okay this should be good to go now.

https://github.com/matrix-org/matrix-spec-proposals/pull/4140 is still regularly receiving minor tweaks and modifications, so this PR isn't going to correspond exactly to the current state of the MSC. I figure we can still merge this feature as unstable, and add the needed corrections over time or when the MSC gets merged.

Okay this should be good to go now. https://github.com/matrix-org/matrix-spec-proposals/pull/4140 is still regularly receiving minor tweaks and modifications, so this PR isn't going to correspond exactly to the current state of the MSC. I figure we can still merge this feature as unstable, and add the needed corrections over time or when the MSC gets merged.
docs: Add changelog
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 7s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m11s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m28s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m1s
038fd1a58f
nex requested changes 2026-08-18 16:15:42 +00:00
@ -0,0 +43,4 @@
// We use the room state lock to get the required synchronization guarantees
// on transaction IDs
let state_lock = services.rooms.state.mutex.lock(room_id.as_str()).await;
Owner

Getting a state lock before checking if this is a duplicate request induces unnecessary lock contention with other unrelated operations. If you want to lock around this endpoint, I'd probably create a new mutex map specifically for this endpoint (perhaps in the transactions service itself?)

Getting a state lock before checking if this is a duplicate request induces unnecessary lock contention with other unrelated operations. If you want to lock around this endpoint, I'd probably create a new mutex map specifically for this endpoint (perhaps in the transactions service itself?)
eve marked this conversation as resolved
@ -0,0 +110,4 @@
let Ruma {
body: delayed_events::update_delayed_event::unstable_v2::Request { delay_id, action, .. },
..
} = body;
Owner

Is there a reason to extract the args like this? would update_delayed_event(body.delay_id, body.action) not suffice here?

Is there a reason to extract the args like this? would `update_delayed_event(body.delay_id, body.action)` not suffice here?
Author
Contributor

Not without cloning. The borrow checker isn't smart enough.

Not without cloning. The borrow checker isn't smart enough.
Owner

I believe that the necessary improvement to the borrow checker is in nightly right now? Anyway, a while before we can use it.

I believe that the necessary improvement to the borrow checker is in nightly right now? Anyway, a while before we can use it.
Owner

Gotcha, that's fine then

Gotcha, that's fine then
nex marked this conversation as resolved
@ -0,0 +128,4 @@
let Ruma {
body: delayed_events::get_delayed_event::unstable::Request { delay_id, .. },
..
} = body;
Owner

same here, this is a lot of text just to extract delay_id

same here, this is a lot of text just to extract `delay_id`
nex marked this conversation as resolved
@ -0,0 +209,4 @@
}
}
async fn finalize_delayed_event(
Owner

Needs a docstring

Needs a docstring
eve marked this conversation as resolved
@ -0,0 +80,4 @@
queue.queue.push((Reverse(time), delay_id));
},
// Loop regularly to check if the service needs to stop even when there are no in-flight delayed events
() = sleep(Duration::from_secs(2)) => (),
Owner

The more idiomatic approach is to listen for the interrupt signal directly, either by having a channel that is written to by the Service::interrupt() method, or just listening for services.server.signal directly

The more idiomatic approach is to listen for the interrupt signal directly, either by [having a channel that is written to by the `Service::interrupt()` method](https://forgejo.ellis.link/continuwuation/continuwuity/src/commit/1cdf369a805b1939acb18c642fe966f0f1cf0457/src/service/announcements/mod.rs#L112), or just listening for [`services.server.signal`](https://forgejo.ellis.link/continuwuation/continuwuity/src/commit/1cdf369a805b1939acb18c642fe966f0f1cf0457/src/core/server.rs#L41) directly
eve marked this conversation as resolved
@ -23,3 +26,4 @@
use crate::rooms::state::RoomMutexGuard;
impl super::Service {
#[allow(clippy::too_many_arguments)]
Owner

I don't like this, and the lint is definitely highlighting a problem - too many arguments is specifically why we have PartialPdu. If we end up adding or removing a new field to this function, EVERY callsite needs to be updated, making changes expensive refactors. I'd recommend changing the function signature to just accept a PartialPdu, and then whatever else it needs that isn't included in the partial.

I don't like this, and the lint is definitely highlighting a problem - too many arguments is specifically why we have `PartialPdu`. If we end up adding or removing a new field to this function, EVERY callsite needs to be updated, making changes expensive refactors. I'd recommend changing the function signature to just accept a PartialPdu, and then whatever else it needs that isn't included in the partial.
Author
Contributor

Fair point. I've rethought the approach a little bit. Now we create a PartialPdu as an argument, and this helper is in charge of the verification.

Fair point. I've rethought the approach a little bit. Now we create a PartialPdu as an argument, and this helper is in charge of the verification.
eve marked this conversation as resolved
eve force-pushed I903 from 038fd1a58f
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 7s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m11s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m28s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m1s
to 1357a907e5
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 9s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m19s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m19s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 4m50s
2026-08-22 17:34:57 +00:00
Compare
eve force-pushed I903 from 1357a907e5
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 9s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m19s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m19s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 4m50s
to 48ce822d81
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m20s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m20s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m9s
2026-08-22 18:12:41 +00:00
Compare
ginger left a comment

looks pretty good overall

looks pretty good overall
@ -0,0 +28,4 @@
} = body;
let sender_user = identity
.sender_user()
Owner

this could be a call to expect_sender_user

this could be a call to `expect_sender_user`
eve marked this conversation as resolved
@ -0,0 +111,4 @@
) -> Result<delayed_events::get_all_delayed_events::unstable::Response> {
let sender_user = body
.identity
.sender_user()
Owner

this could be a call to expect_sender_user

this could be a call to `expect_sender_user`
eve marked this conversation as resolved
@ -254,3 +252,1 @@
self.depth > 0,
"serializing string at the top-level; you can skip serialization instead"
);
fn serialize_str(self, v: &str) -> Result<Self::Ok> { self.serialize_bytes(v.as_bytes()) }
Owner

why were these asserts removed?

why were these asserts removed?
Author
Contributor

The columns that I introduce in this change use a string as a key (the delay_id) which gets serialized at some points (by calls to Map::put, for example).

This assert tells me to not serialize it, and instead use a function like Map::raw_put (which will lead to the same result as my code with the assert remove). However, I do not think there is a good reason for this restriction exists. Allowing the string to be serialized and then using a function like put is perfectly correct. Using raw_put would also be correct, but I see no good reason to force the users to pick the second method.

I asked about this in the dev room at the time and reached the conclusion that this assert has no good reason to exist and can be removed.

The columns that I introduce in this change use a string as a key (the delay_id) which gets serialized at some points (by calls to `Map::put`, for example). This assert tells me to not serialize it, and instead use a function like `Map::raw_put` (which will lead to the same result as my code with the assert remove). However, I do not think there is a good reason for this restriction exists. Allowing the string to be serialized and then using a function like `put` is perfectly correct. Using `raw_put` would also be correct, but I see no good reason to force the users to pick the second method. I asked about this in the dev room at the time and reached the conclusion that this assert has no good reason to exist and can be removed.
ginger marked this conversation as resolved
@ -0,0 +188,4 @@
// There are no synchronization primitives ensuring that the finalized event has
// made it to the database before we look for it. A best-effort answer is
// acceptable here, so we simply wait a bit and look again.
sleep(Duration::from_millis(200)).await;
Owner

this is cursed but I guess we don't have a better option

this is cursed but I guess we don't have a better option
ginger marked this conversation as resolved
@ -0,0 +378,4 @@
self.submission_queue_sender.send((submission_time, delay_id.clone()))
.map_err(|_err|{
err!(Request(Unknown(
debug_error!(%event.user_id, "Server was unable to process delayed even request (queue full).")
Owner

"delayed even"

"delayed even"
eve marked this conversation as resolved
@ -0,0 +28,4 @@
let mut queue = service
.submission_queue
.try_lock()
.map_err(|_| err!(Err("Attempted to launch multiple instances of the worker")))?;
Owner

this should be an expect(), that should never happen

this should be an expect(), that should never happen
eve marked this conversation as resolved
@ -0,0 +67,4 @@
let next_receive = receiver.recv_async();
let next_interrupt = interrupt.recv_async();
// RescvFuture is cancellation-safe
Owner

typo in comment

typo in comment
eve marked this conversation as resolved
@ -70,0 +69,4 @@
if *event_type == MessageLikeEventType::CallInvite
&& self.services.directory.is_public_room(room_id).await
{
return Err!(Request(Forbidden("Room call invites are not allowed in public rooms")));
Owner

🤨is this in the MSC or is it something we enforce

🤨is this in the MSC or is it something we enforce
Author
Contributor

I do not know. It's a restriction that was there already, I just left it there.

I do not know. It's a restriction that was there already, I just left it there.
eve force-pushed I903 from 48ce822d81
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m20s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m20s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m9s
to be1a5fb648
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m14s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m27s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 4m45s
2026-09-10 12:23:20 +00:00
Compare
First-time contributor
https://github.com/matrix-org/matrix-spec-proposals/pull/4140 has been merged finally.
Jade left a comment

Still need to review the service module, but looks good so far

Still need to review the service module, but looks good so far
@ -0,0 +14,4 @@
) -> Result<delayed_events::send_delayed_event::unstable::Response> {
let Ruma {
body:
delayed_events::send_delayed_event::unstable::Request {
Owner

You can destructure structs in the arguments to avoid this intermediary assignment

You can destructure structs in the arguments to avoid this intermediary assignment
Owner

Same applies for the other routes, too

Same applies for the other routes, too
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 6s
Required
Details
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Required
Details
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m14s
Required
Details
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m27s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 4m45s
Required
Details
This pull request has changes requested by an official reviewer.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u I903:eve-I903
git switch eve-I903
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
8 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
continuwuation/continuwuity!1840
No description provided.