Passkey Read #389
Passkey Read #389
Conversation
| @@ -39,15 +41,16 @@ data class ClientOptions( | |||
| ) | |||
| } | |||
|
|
|||
| @JvmInline | |||
| value class InboxId(val value: String) | |||
There was a problem hiding this comment.
I made this a value class so that it would error if people pass an address instead of an inboxId into certain methods. If you pass an address instead of a inboxId you get this error: xmtp/libxmtp#1693
There are less aggressive ways to do typing like a typealias... but it wouldn't give clear in line coding warnings which is what I think is necessary here with this level of a change. Open to thoughts though.
There was a problem hiding this comment.
yea this was the first thing I checked - I think this is perfect for providing the lint warning and compile error on existing code that strings need to be InboxId 👍
Maybe we can put a descriptive comment here for integrators who have only ever dealt with address who have the question, what is an InboxId, how to do find someone's inboxId etc.
// InboxId is the constant unique identifier for all XMTP users.
// To find an inboxId from a `PublicIdentifier`, (Ethereum Address, Passkey), see `inboxIdFromIdentity()`There was a problem hiding this comment.
I walked this change back because it felt too heavy. Long term I want the SDK to be a bit more light. InboxIds should just be a typealias around a string. I'm waffling on which direction is more right. Maybe another kotlin perspective from @mchenani ?
There was a problem hiding this comment.
I agree too, also I would say we need to do it more and more, also we're doing it in rust more often.
| } | ||
|
|
||
| suspend fun addMembers(addresses: List<String>) { |
There was a problem hiding this comment.
Big breaking change here is addMembers now takes a inboxId instead of a address... but because they are both strings it will blow up only after it's been hit in the code.
| suspend fun newConversationWithInboxId( | ||
| peerInboxId: String, | ||
| suspend fun newConversation( | ||
| peerInboxId: InboxId, |
There was a problem hiding this comment.
Big breaking change here is newConversation and new Group now takes a inboxId instead of a address... but because they are both strings it will blow up only after it's been hit in the code.
There was a problem hiding this comment.
Noticed something odd, testCannotStartGroupWithSelf started failing in this branch, looks good on main
looks like no error thrown when you start a group with yourself 🤔
GroupTest.kt:483
@Test
fun testCannotStartGroupWithSelf() {
assertThrows("Recipient is sender", XMTPException::class.java) {
runBlocking { boClient.conversations.newGroup(listOf(boClient.inboxId)) }
}
}|
Ahh ya I thought I fixed the self dms error. But honestly this should probably be enforced in libxmtp. I'll look into making the fix in the lower level. |
Co-authored-by: Cameron Voell <1103838+cameronvoell@users.noreply.github.com>
addMember,removeMember,createGroup, andnewConversationto default toInboxId