Pull v3 client into its own PR #233
Conversation
| let v3Client: FfiXmtpClient? | ||
|
|
||
| if options?.enableAlphaMLS == true && options?.api.env == .local { | ||
| let dbURL = URL.documentsDirectory.appendingPathComponent("xmtp-\(options?.api.env.rawValue ?? "")-\(address).db3") |
There was a problem hiding this comment.
I don't know how rawValue works but in Android this would return the actual value so 10.2.2.2 or whatever. I was thinking this would return just enum name like local, dev, or prod. Atleast thats how we do it in android. 🤷♀️ Not a huge deal tho.
There was a problem hiding this comment.
Ya I'm ok with it just returning hostnames.
Sources/XMTPiOS/Client.swift
Outdated
| if let v3Client, let textToSign = v3Client.textToSign() { | ||
| guard let signingKey else { | ||
| throw ClientError.creationError("No v3 keys found, you must pass a SigningKey in order to enable alpha MLS features") | ||
| } | ||
|
|
||
| let signature = try await signingKey.sign(message: textToSign) | ||
| try await v3Client.registerIdentity(recoverableWalletSignature: signature.rawData) | ||
| } |
There was a problem hiding this comment.
Since this check is required for V3 client creation I think it should move into the above method. And the above method should take an optional signer.
There was a problem hiding this comment.
I also think the logic here is incorrect if there is no text to sign that just means a V3 identity already exists but you still need to call registerIdentity on it. If there is text to sign you sign it and call register identity. Otherwise then error.
if (v3Client.textToSign() == null) {
v3Client.registerIdentity(null)
} else if (account != null) {
v3Client.textToSign()?.let {
v3Client.registerIdentity(account.sign(it))
}
} else {
Log.i(TAG, "No signer passed but signer was required.")
}
| for codec in options.codecs { | ||
| result.register(codec: codec) | ||
| } |
There was a problem hiding this comment.
Is there a reason you added this here? Feels like if we didn't have it before than it doesn't make sense to add it now.
There was a problem hiding this comment.
It was busted before.
Tests/XMTPTests/ClientTests.swift
Outdated
| } | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Might be nice to add a test for the creating with a bundle scenario since it's a bit of an outlier.
| for codec in (options?.codecs ?? []) { | ||
| client.register(codec: codec) | ||
| } |
There was a problem hiding this comment.
Same about the codec code? Was this meant to be committed here?
There was a problem hiding this comment.
This was actually not working before.
* Pull v3 client into its own PR * Add tests * rename mls alpha * cleanup * bring back skip
Contains the client related changes from #229.