Bind the event TXN ID to the device ID instead of the access token ID

[sandhose]

When sending an event, the client sets a locally unique txnID on it, which serves two purposes:

• deduplicating events in case of network failures/retries
• when the client receives an event from /sync that they created, map it correctly to the locally-created event (for proper local echo)

The problem is, this txnID is currently bound to the user ID and the access token ID. Since MSC2918 (refresh tokens), a single client might deal with multiple access tokens, meaning that the current scenario is possible:

• client starts a /sync with its current access token
• this token is about to expire, so it refreshes it and gets a new access token
• the client sends a new event, with a random txnID, using the new access token
• /sync gets back, with the new event but not the txnID, since this /sync was done with another access token than when the event was created

I think the proper way to deal with this would be to have the txnIDs bound to devices instead of access tokens.

This is also relevant for the OIDC patches, since we don't really have access token IDs, but we do have the device ID.

What I would like to do is:

• add a column to the event_txn_id to store the device ID
• add the device_id field in the _EventInternalMetadata (and ensure we're persisting it when saving the txn IDs)
• when looking up existing events, consider both the token_id and the device_id
• release Synapse like that, so current transactions don't break
• remove the token_id from event transactions (event_txn_id table, _EventInternalMetadata) everywhere, and do another release

[sandhose]

I've noticed two more places where we're using the access token/access token ID where we might be better off using the device ID:

• in the HttpTransactionCache
• when adding a pusher, for some reason?

Does it make sense to also change those?

[hughns]

MSC3970 now proposes changing the transaction ID such that the present issue becomes spec compliant.