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

Simplified Sliding Sync should be enabled by default if your homeserver supports it (and doesn't have a SS proxy). #3256

Closed
Half-Shot opened this issue Jul 30, 2024 · 12 comments
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements

Comments

@Half-Shot
Copy link
Member

Half-Shot commented Jul 30, 2024

Update

Currently in order to use EX you need to:

  • Add org.matrix.msc3575.proxy to your .well-known/matrix/client.
  • Sign in to your account, skip all prompts to verify your device.
  • Find the magic setting in developer options to enable simplified sliding sync.
  • Be logged out, and then log back in. To the delight of myself it then worked perfectly :)

So probably this just needs to be a thing where if the simplified feature is enabled and the proxy isn't, we go straight for simplified.

My quite wrong ramblings are preserved below.

Your use case

What would you like to do?

I'd like to run Element X using just Synapse and the Simplified Sliding Sync feature.

Why would you like to do it?

I'd really like to use Element X, and I don't want to run the SS Proxy.

How would you like to achieve it?

Probably we should have a mechanism to detect if the homeserver supports Simplified Sliding Sync and pre-enable the developer flag for the moment. Maybe a /versions check, although the feature doesn't have a corresponding flag. I suppose you could also just POST the endpoint and check to see if it comes back with something other than 404. The possibilities are endless!

Have you considered any alternatives?

The clear alternative is running a proxy, and then turning it off. For obvious reasons, absolutely not :)

Additional context

#3222 is the PR that added the feature.

Are you willing to provide a PR?

No

@Half-Shot Half-Shot added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Jul 30, 2024
@Half-Shot
Copy link
Member Author

Half-Shot commented Jul 30, 2024

The Rust side of this is https://github.com/matrix-org/matrix-rust-sdk/blob/d03d3cff1707743af898c89a2e30b301e79e3845/crates/matrix-sdk/src/client/builder.rs#L475-L493.

Which is triggered by

suspend fun create(sessionData: SessionData): RustMatrixClient = withContext(coroutineDispatchers.io) {
val client = getBaseClientBuilder(
sessionPath = sessionData.sessionPath,
passphrase = sessionData.passphrase,
slidingSync = if (appPreferencesStore.isSimplifiedSlidingSyncEnabledFlow().first()) {
ClientBuilderSlidingSync.Simplified
} else {
ClientBuilderSlidingSync.Restored
},
)
.homeserverUrl(sessionData.homeserverUrl)
.username(sessionData.userId)
.use { it.build() }
client.restoreSession(sessionData.toSession())
val syncService = client.syncService()
.withUtdHook(utdTracker)
.finish()
val (anonymizedAccessToken, anonymizedRefreshToken) = sessionData.anonymizedTokens()
RustMatrixClient(
client = client,
syncService = syncService,
sessionStore = sessionStore,
appCoroutineScope = appCoroutineScope,
dispatchers = coroutineDispatchers,
baseDirectory = baseDirectory,
baseCacheDirectory = cacheDirectory,
clock = clock,
).also {
Timber.tag(it.toString()).d("Creating Client with access token '$anonymizedAccessToken' and refresh token '$anonymizedRefreshToken'")
}
}
internal fun getBaseClientBuilder(
sessionPath: String,
passphrase: String?,
slidingSyncProxy: String? = null,
slidingSync: ClientBuilderSlidingSync,
): ClientBuilder {
return ClientBuilder()
.sessionPath(sessionPath)
.passphrase(passphrase)
.slidingSyncProxy(slidingSyncProxy)
.userAgent(userAgentProvider.provide())
.addRootCertificates(userCertificatesProvider.provides())
.autoEnableBackups(true)
.autoEnableCrossSigning(true)
.run {
when (slidingSync) {
ClientBuilderSlidingSync.Restored -> this
ClientBuilderSlidingSync.Discovered -> requiresSlidingSync()
ClientBuilderSlidingSync.Simplified -> simplifiedSlidingSync(true)
}
}
.run {
// Workaround for non-nullable proxy parameter in the SDK, since each call to the ClientBuilder returns a new reference we need to keep
proxyProvider.provides()?.let { proxy(it) } ?: this
}
}
}
. Essentially perhaps the Rust SDK needs to say "hey, we couldn't find a proxy but the homeserver claims simplified sync support" which the client might be able to optionally decide to accept (perhaps with a warning to the user that dragons exist).

@Half-Shot Half-Shot changed the title Simplified Sliding Sync can't be enabled without first having setup a SS proxy Simplified Sliding Sync should be enabled by default if your homeserver supports it (and doesn't have a SS proxy). Jul 30, 2024
@bmarty
Copy link
Member

bmarty commented Aug 19, 2024

which the client might be able to optionally decide to accept

I do not think the client need to validate the usage of SSS. I believe the feature flag has been added to prevent user from being stuck if there is a problem with SSS and using the proxy was OK.

So I think the SDK should use SSS if there is no proxy.

WDYT @Hywan ?

@Hywan
Copy link
Member

Hywan commented Aug 19, 2024

It's the entire migration from SS proxy to SS native project, element-hq/element-meta#2499. I think it's better to close this issue, as it will create duplicated discussions.

@xxfogs
Copy link

xxfogs commented Sep 14, 2024

But so will we see this anytime soon?

@Hywan
Copy link
Member

Hywan commented Sep 16, 2024

@bmarty Does Element X Android support detecting and switching from SS proxy to SSS (MSC3575 <> MSC4186)?

@Hywan
Copy link
Member

Hywan commented Sep 16, 2024

But so will we see this anytime soon?

Element X iOS (nightly) does already support this yes.

@xxfogs
Copy link

xxfogs commented Sep 16, 2024

But so will we see this anytime soon?

Element X iOS (nightly) does already support this yes.

Oh, that's splendid. I hope that soon gets implemented in the android version as well.

@athei
Copy link

athei commented Sep 17, 2024

But so will we see this anytime soon?

Element X iOS (nightly) does already support this yes.

Can you maybe point me to the commit that added this support? I am looking through the changes since the last release and can't find it.

@Hywan
Copy link
Member

Hywan commented Sep 24, 2024

cc @stefanceriu @jmartinesp :-)

@stefanceriu
Copy link
Member

Default to native sliding sync discovery. -> element-hq/element-x-ios@e47e76c

@jmartinesp
Copy link
Member

Migration to native SS PR: #3429

Force logout when SS proxy is no longer available but native SS is (aka forced migration): #3458

Fix for native sliding sync being incorrectly forced: #3489

Note that having native sliding sync enabled in the EX app is currently based in a developer setting flag that it's now true by default, but if you changed it in the past, it'll keep that value, whether it was enabled or disabled.

@manuroe
Copy link
Member

manuroe commented Sep 24, 2024

We have everything in place to have this working. We just forgot to close this issue in favor of the work that happened in element-hq/element-meta#2499.
Check it on 0.6.3. If there is an issue, raise a new ticket explaining the problem.

@manuroe manuroe closed this as completed Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements
Projects
None yet
Development

No branches or pull requests

8 participants