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

Bug: senderAddress returns inboxId in 1.34.0-beta.2 #401

Open
Tracked by #309
peterferguson opened this issue Jun 2, 2024 · 6 comments · Fixed by #402
Open
Tracked by #309

Bug: senderAddress returns inboxId in 1.34.0-beta.2 #401

peterferguson opened this issue Jun 2, 2024 · 6 comments · Fixed by #402
Labels
bug Something isn't working

Comments

@peterferguson
Copy link
Contributor

Describe the bug

Hey this is probably better in a discussions section but since there is a bug & no discussions I thought an issue works.

I assume this bug is just a placeholder while inboxId is still being implemented as it is the same in each of the client libraries.

We use senderAddress (and just generally the users XMTP address) extensively throughout the app.
So I am wondering whether the longer term api will also include the senderAddress in messages?

It is still the case that a client is instantiated using a particular address so this could be sent with the message but maybe that is going to change?

It is pretty essential to be able to access the available addresses related the users inboxId (again I assume this will happen & I am just early to the party) right now we are using a patch that just the addresses off the group

+ AsyncFunction("listMemberAddresses") { (clientAddress: String, groupId: String) -> [String: [String]] in
+	guard let group = try await findGroup(clientAddress: clientAddress, id: groupId) else {
+		throw Error.conversationNotFound("no group found for \(groupId)")
+	}
+
+	return try group.members.reduce(into: [String: [String]]()) { result, member in
+		result[member.inboxId] = member.addresses
+	}
+ }
+

Should I assume that the main way to get a users address will be through a method linked to the inboxId that will be available in the future or one of their linked addresses will be sent on the message as before?

Expected behavior

No response

Steps to reproduce the bug

No response

@peterferguson peterferguson added the bug Something isn't working label Jun 2, 2024
@nplasterer
Copy link
Contributor

Opened a PR here to expose the member information better #402

@nplasterer
Copy link
Contributor

nplasterer commented Jun 3, 2024

I'm not sure if there is a scenario where the senderAddress will be an address again instead of an inboxId. Is that going to be a problem? Wondering how we might be able to mitigate that for you in this transition aside from the members work I did in PR 402? I could maybe return the first address from the list of address associated with the inboxId and add an additional field called inboxId to the message just to make them explicitly different? Would that help?

@peterferguson
Copy link
Contributor Author

I'm not sure if there is a scenario where the senderAddress will be an address again instead of an inboxId. Is that going to be a problem? Wondering how we might be able to mitigate that for you in this transition aside from the members work I did in PR 402? I could maybe return the first address from the list of address associated with the inboxId and add an additional field called inboxId to the message just to make them explicitly different? Would that help?

We have moved to use the listGroupMembers function and basically search the result to find any addresses that we expect might be in that list for now. So I don't think more work needs to be done to return addresses.

My question is more on whether senderAddress will remain a thing that is actually sent in the future or will be replaced completely by senderInboxId?

In my head it is still possible to send both from the client side, as the client has typically signed the user into their inboxId with a wallet. So the client could attach the actual senderAddress that was used to create the XmtpClient to the message.

I guess once the keys are pulled in from the initial signature it could be the case that the wallet is no longer associated directly with the client though.

Our use-case is something like we have users sign-in with their eoa & we create them a passkey wallet that is then attached to their inboxId. Then we use the passkey wallet from the senderAddress in some custom content types to track which wallets are doing certain actions.

If the senderAddress is likely not to be attached then we can just move them into the content type itself. So maybe just some clarification on the future of this api is all we need.

Another issue we have with lack of senderAddress is which address to use as the primary identity of the user (i.e. their socials, pfp, displayName etc). Will there be some form of this with the inboxId or even a way for the user to select a primary address that all clients then use to extract the above information?

@nplasterer
Copy link
Contributor

In the future inboxId will be the primary source of truth as identities will be able to add multiple addresses and even remove addresses. So senderAddress won't really work for messages as the senderAddress may get removed from the inboxId but the inboxId will still remain. While 1to1 messages are on V2 and group messages are on V3 we are in a bit of limbo where renaming to senderInboxId doesn't work just yet but in the future when we move 1to1 also into V3 the inboxId will be more prevalent.

I'm not certain if inboxIds will have the ability to set a primary address but asked a few team members to chime in on that.

@37ng
Copy link

37ng commented Jun 5, 2024

Another issue we have with lack of senderAddress is which address to use as the primary identity of the user (i.e. their socials, pfp, displayName etc). Will there be some form of this with the inboxId or even a way for the user to select a primary address that all clients then use to extract the above information?

In the V3 InboxId will be the one that represents a user. All socials, pfp, displayName should be attached to InboxId

Currently InboxId doesn't have the ability to set a primary address.

@neekolas
Copy link
Contributor

neekolas commented Jun 5, 2024

In my head it is still possible to send both from the client side, as the client has typically signed the user into their inboxId with a wallet. So the client could attach the actual senderAddress that was used to create the XmtpClient to the message.

Agree with everything that has been said about inbox_id being the future, and that multiple addresses per inbox_id being the norm.

With the new system messages really aren't sent by a wallet. Once you have registered a set of installation keys (which are XMTP specific) you don't need to use the wallet in subsequent client creation. From a protocol perspective we don't need the wallet after first use. You can use our SDK and send messages just relying on the installation keys stored in libxmtp's DB (with the exception of some special cases like revoking keys or adding more wallets). This is made a little confusing because our mobile SDKs do require you to pass in a wallet or PrivateKeyBundle today. But that will go away as we sunset some legacy features.

There is one thing that is technically possible, but I'd have to understand the use-case better before we committed to doing it. Every message is signed by an installation key. And every installation key was added to the inbox by a wallet originally. So there is a line that connects back from the message to a wallet. I just don't know if that's really solving the user problem, or if we should have some notion of a "primary wallet" that apps use when they only want to show one wallet address or ENS name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants