Skip to content

Conversation

@gefjon
Copy link
Contributor

@gefjon gefjon commented Jan 13, 2026

Description of Changes

Draft PR, currently containing only the new format itself.

The changes in question are:

Remove total_host_execution_duration_micros

  • BitCraftClient does use this info, specifically on TransactionUpdate messages, to show its latency overlay.
  • The values for queries are unused.
  • Bloats message size.
  • IMO clients shouldn't even get this info.
    • Clients are not generally trusted, and so shouldn't really get details about the module's implementation and performance.
    • Seems like a source of timing sidechannels.
    • Developers have other ways to get this info (e.g. metrics dashboard).

Add query IDs to database updates

  • This will enable correlating rows with the queries they matched in clients.

Remove rows from UnsubscribeApplied

  • Now we can just drop the subscription, and the client can know what rows it needs to get rid of on its own!
  • I am still sending UnsubscribeApplied as a confirmation, though this is unnecessary and I may change my mind.

Remove table_id from SubscriptionError

  • After discussion with @jsdt , this is unused and unlikely to be useful.

Rename QueryId to QuerySetId

  • More explicit.

Remove OutOfEnergy error variants

  • If we want to report this to clients at all, we can treat it as an InternalError.

Remove ReducerEvent info from TransactionUpdate

  • TODO: Decide if we want to include a timestamp here, or just to tell users to emit an event with a timestamp.

Separate ReducerResult from TransactionUpdate

  • Or, more accurately, ReducerResult wraps a TransactionUpdate when successful.
  • ReducerResult also gets a return value when Ok.
  • In error case, distinguish between structured user error and string internal error.

Nix FormatSwitch and the JSON format

  • We should just be using the BSATN format. The JSON format is inefficient, and BSATN is simple enough that we can just implement ser/de wherever we need it.
  • The only place we don't do this already is the website.

Remove table IDs

  • We haven't implemented ID handshaking. When we do that, it can be protocol V3.

Open questions:

Is CallReducerFlags::NoSuccessNotify still useful?

An empty ReducerResult should be much smaller now than a v1 empty TransactionUpdate, since it doesn't need to contain the arguments and such. By my count it would be 17 bytes (not counting whatever WS framing):

| Field name          | Field size (bytes) |
|---------------------+--------------------|
| `request_id`        |                  4 |
| `timestamp`         |                  8 |
| `result` tag        |                  1 |
| `ret_value`         |                  0 |
| `query_sets` length |                  4 |

API and ABI breaking changes

Expected complexity level and risk

Testing

@joshua-spacetime
Copy link
Collaborator

Nix FormatSwitch and the JSON format

We're still supporting v1, so we can't really nix it from the codebase. Is there still a reason to remove it from v2?

@egormanga
Copy link
Contributor

Remove OutOfEnergy error variants

If we want to report this to clients at all, we can treat it as an InternalError.

Unless you want to eventually support per-client energy limits/balances!

@egormanga
Copy link
Contributor

Clients are not generally trusted, and so shouldn't really get details about the module's implementation and performance.

Unless you also remove (or add a setting to disable) WS v1, this won't be really helpful for security.

@gefjon
Copy link
Contributor Author

gefjon commented Jan 14, 2026

Nix FormatSwitch and the JSON format

We're still supporting v1, so we can't really nix it from the codebase. Is there still a reason to remove it from v2?

@joshua-spacetime Each additional supported format increases the worst-case amount of serialization we have to do at runtime, and the volume of serialization code we have to write and maintain. The V2 definitions without switching on WebSocketFormat are dramatically simpler than the V1 definitions.

Also, forcing clients to migrate to BSATN in order to use the new features introduced by V2 gives us a path to eventually deprecating JSON over WebSockets entirely. I imagine we will someday deprecate and remove the V1 format, though it will probably not be for quite some time.

Remove OutOfEnergy error variants

If we want to report this to clients at all, we can treat it as an InternalError.

Unless you want to eventually support per-client energy limits/balances!

@egormanga We have no plans to do this within the expected lifetime of the V2 WebSocket format. If that changes and we do, clients could still receive an InternalError when out of energy. Or, IMO more likely, when a client exceeded its energy limit, we could simply disconnect them with an appropriate message in the close frame.

Clients are not generally trusted, and so shouldn't really get details about the module's implementation and performance.

Unless you also remove (or add a setting to disable) WS v1, this won't be really helpful for security.

@egormanga Just because we can't immediately remove the old format doesn't mean we'll be stuck with it forever. Gradual deprecation and eventual removal is a graceful path which still allows us to improve SpacetimeDB.

@jsdt
Copy link
Contributor

jsdt commented Jan 14, 2026

This looks good to me. I'm all for removing FormatSwitch, since that adds a lot of complexity. For v1 clients, we could add some stats to see if anyone is actually using the json format. If no one is using it, we could just remove support for it.

pub enum ReducerOutcome {
Ok(SubscriptionOk),
Err(Bytes),
InternalError(Box<str>),
Copy link
Contributor

Choose a reason for hiding this comment

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

Alt: We could have the internal error be an enum type so that we can change this type later if we want a more structure error (e.g. status codes or something). Not critical.

@JulienLavocat
Copy link
Contributor

The only place we don't do this already is the website.

About this, I ran into a few issues with the JSON format and the best way to go forward with the website (specifically the table explorer) is to switch to BSATN, especially if we want to properly show u64 values (currently they aren't displayed properly if they go above 2^53.
I'm in favor of removing the JSON format entirely

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.

7 participants