Stellar
Add a standard wallet interface #1467
Replies: 20 comments 34 replies
-
|
It seems like the Discord discussion is pretty dated, so I'll offer some thoughts on this here, also to get this discussion going again. I like the proposed interface, but I'd like to suggest an additional generic signing method that could actually encapsulate the other two and also be used for arbitrary signing purposes: interface Wallet {
sign: (arbitraryB64: string) => Promise<string>;
}(Internally, an implementation could have something like Also, should we discuss the trade-off between allowing encoded XDR passed in vs. the actual XDR object? I think encoded is fine, but we need to be sure to communicate to the implementers that they should NOT trust the input string to be well-formed or even necessarily represent an |
Beta Was this translation helpful? Give feedback.
-
|
Ah, that's a great reason! Keeping it encoded is fine, then, we'll just have to remember the "tell people to not trust the input string" part. |
Beta Was this translation helpful? Give feedback.
-
|
Another motivation to stick with XDR is that to rely on the xdr.Transaction object is to make the interface only work with applications that use the js-stellar-sdk/base lib, which an application may not be using. |
Beta Was this translation helpful? Give feedback.
-
|
A generic signing function presents a couple challenges:
|
Beta Was this translation helpful? Give feedback.
-
|
@leighmcculloch That's great context, thanks. Without investigating, I had assumed that maybe there was a way to identify a xdr type from the xdr itself. Also very interesting that trying to decode as different types until success wouldn't reliably work either We can still create a generic |
Beta Was this translation helpful? Give feedback.
-
You certainly should pass strings in window.postMessage/chrome.runtime.sendMessage if you want things to arrive like you sent them, but the API could still have met users where users are. Users are dealing with stellar.Transactions, not xdrs. |
Beta Was this translation helpful? Give feedback.
-
|
If we are adding extra methods, I would like to include an extra one:
I also prefer separated functions instead of a single generic function because it's easier for a wallet implementing something unique that they only have (like albedo with its messages signing features) and dapp developers can easily decide if they want to support that or not by just adding that extra function. But that being said the more "features" we add (sighAuthEntry included) the more we will reduce the support for non stellar native wallets like those using wallet connect (Lobstr), hardware wallets, airgapped wallets, etc so at the end of the day devs will still need to handle different interfaces no matter what |
Beta Was this translation helpful? Give feedback.
-
|
I have mixed feelings about such an interface. It's simple and unified, and that's great. On the other hand, I really don't like the idea of returning plain strings from the method calls. Can we change the interface to return objects (with the predefined set of fields) instead of strings? For example, {
getPublicKey: () => Promise<{publicKey: string}>,
signTransaction: (xdr: string) => Promise<{signedXdr: string}>,
signAuthEntry: (authEntryXdr: string) => Promise<{signedAuthEntry: string}>,
signMessage: (message: string) => Promise<{signedMessage: string}>
}Motivation:
|
Beta Was this translation helpful? Give feedback.
-
|
Aside from the successful result standardization, we also have to define the standardized error output format. Albedo client lib defines several standard errors for different scenarios. I'm not advocating for sticking with our vendor format, but it would be nice to have a few basic standard errors defined in a minimalistic format, e.g. {
message: string, // general description message returned to the client app
code: number, // unique error code
ext: Array<string> // optional extended details
}Proposed standard errors: {
unhandled: { // internal wallet error, likely caused by the wallet logic itself
message: 'Unhandled error occurred.',
code: -1
},
external: { // external service (Horizon, RPC, etc.) returned an error
message: 'External error occurred.',
code: -2,
ext: ['Operation 2: Invalid "amount" value.', "Operation 5: Asset issuer is required."] // malformed tx error example
},
invalid: { // client app request is invalid (wrong parameters, invalid transaction XDR, etc.)
message: 'Request is invalid.',
code: -3,
ext: ['Invalid transaction XDR'] // example error
},
rejected: { // user rejected the request, client app should not try to retry the request
message: 'Rejected by the user.',
code: -4
}
} |
Beta Was this translation helpful? Give feedback.
-
|
One more thing to consider. Multi-account wallets in some cases need an option to identify the specific account to utilize for the request. However, if the user interacted with the app from two different accounts (for example, traded with DEX from two different accounts), I have a problem. The obvious solution is to show the list (or dropdown) of all wallet accounts with pre-selected the most recent account. Yet allowing an app to request signature for a specific account makes the UX much more streamlined on the wallet side, decreasing the possibility of a human error and saving a few clicks. How about adding a second optional parameter |
Beta Was this translation helpful? Give feedback.
-
|
I agree with this and maybe instead of adding a second parameter we send an object as first parameter right away, it gives more flexibility if in the future we want to add something else |
Beta Was this translation helpful? Give feedback.
-
|
Lastly, kudos to @earrietadev for maintaining the StellarWalletsKit lib that allows client apps to avoid complexity of dealing with different wallets by abstracting the interface. |
Beta Was this translation helpful? Give feedback.
-
|
One thing I notice about the proposed API is that it doesn't expose any concept of a network, or a network passphrase. If a web app knew that it was only deployed to testnet or pubnet, how would it communicate the supported network, or see what network the wallet supported? |
Beta Was this translation helpful? Give feedback.
-
|
All 4 extension wallets (Albedo, Freighter, Rabet and xBull) already support sending the network as a parameter, that could be an option. For example in my kit I have the function: signTx(params: { xdr: string; publicKeys: string[]; network: WalletNetwork }): Promise<{ result: string }>;This way the dapp can always tell the wallet the exact network it needs to use |
Beta Was this translation helpful? Give feedback.
-
|
The interface above was definitely a bit simplified to kickoff the discussion. IMO, an actual interface would look more like: |
Beta Was this translation helpful? Give feedback.
-
|
We could draw some inspiration from Ethereum as well. EIP-1193 was introduced to standardize a dapp connecting with different wallets from different providers - which is something I believe this proposal is trying to achieve as well EIP-6963 was introduced to let a dapp discover when a browser wallet was installed, which becomes important when a user has multiple wallets installed |
Beta Was this translation helpful? Give feedback.
-
|
@piyalbasu I'd like to suggest some interface additions. It could be for next iteration of this SEP. |
Beta Was this translation helpful? Give feedback.
-
|
Methods like Regarding |
Beta Was this translation helpful? Give feedback.
-
|
As a way of migrating to better supporting multiple accounts, which is better handled in the client, I suggest adding a getAccounts() that returns a list of all accounts that the user has authorized the wallet to share. By default, that could be one account, and everything works as before.
|
Beta Was this translation helpful? Give feedback.
-
|
Oh, and the reason it's better dealt with in the client, is that as a user you want to minimize mental context switches. |
Beta Was this translation helpful? Give feedback.
-
|
I've incorporated changes we've agreed to here as well as those I've collected on Discord on this PR: #1478 |
Beta Was this translation helpful? Give feedback.
-
|
Just wanted bring something else to the table that it might be good to include: a way to import an asset. Since we are promoting the usage of assets list, something I have used a lot when using EVM wallets is clicking on a protocol's site and add the asset to my wallet (basically it adds the asset contract ID). This is good for users that might be using a protocol that is not that popular yet or for custom assets that will probably be outside of the lists (like pool shares) |
Beta Was this translation helpful? Give feedback.
-
|
I'd like to suggest an Occasionally wallets shutdown for various reasons (no more run way, key member departure, so on and so forth). Users could migrate off the wallet by using seedphrase, but in all instances, this is too challenging for all the users. If wallets consistently have the ability to import account by private key, the wallet about to shutdown can push a feature to export keys and import to other wallets. It makes migration off a wallet less choatic. In general be able to import by key is useful from dev perspective too. |
Beta Was this translation helpful? Give feedback.
-
|
Usually wallets use seed phrases. This helps users create multiple wallets and is much easier to export/import. |
Beta Was this translation helpful? Give feedback.
-
|
This is actually something some others in the ecosystem have requested Freighter implement. We've decided not to implement this as we have felt we want to discourage users from copy/pasting private keys. cc: @brunomuler |
Beta Was this translation helpful? Give feedback.
-
|
I think there is value in making an standard for migrating form one wallet to another one, in xBull I already allow that by exporting an encrypted file, later I can import this to any xBull (mobile, web and extension) when I need to. If all wallets accept something similar I think we could make it easy for users to migrate if they need to and it can be done without them needing to copy/paste keys nor phrases |
Beta Was this translation helpful? Give feedback.
-
|
Yes, the key should be encrypted using a password during transfer. Alternatively, we can consider sweep an account. The point is aligning on something consistent such that it makes it easier to migrate users. |
Beta Was this translation helpful? Give feedback.
-
|
I like the idea of an encrypted account migration. I think that it may be a bit out of scope for this standard, which focuses on exposing an API - maybe we should consider this for a different standard as I do think this is definitely important functionality to standardize! I'd mostly be concerned about exposing this method to dapps, which could lead to scams. I'd rather this functionality be contained to the wallet where only the wallet owner has access to it |
Beta Was this translation helpful? Give feedback.
-
|
@earrietadev and @orbitlens we're planning to finalize this proposed standard during the Stellar Protocol Meeting next week on Thursday, June 6th at 1:00 PM PDT. We'd love your support and participation in the meeting. |
Beta Was this translation helpful? Give feedback.
-
|
What happened? :) |
Beta Was this translation helpful? Give feedback.
-
|
@johansten https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0043.md |
Beta Was this translation helpful? Give feedback.
-
|
Yeah, I've seen that. But there's nothing about how to actually use it, in a browser. How do I get exposed to this API? How do I inject my wallet into this? |
Beta Was this translation helpful? Give feedback.
-
|
The corresponding proposal was put to vote on 6/6/24. The votes represent ecosystem sentiment and can be used by the author to determine next steps The voting card is on testnet deployment of Soroban Governor (link) Here is the snapshot of the final count:
|
Beta Was this translation helpful? Give feedback.
-
|
I'm curious which wallets are planning to implement or have already implemented this standard? @piyalbasu @earrietadev @orbitlens |
Beta Was this translation helpful? Give feedback.
-
|
This interface is included in the Stellar Wallets Kit, xBull already supports part of the interface but not all of the methods yet |
Beta Was this translation helpful? Give feedback.
-
|
Do we really need Someone mentioned earlier that it can be useful for multisig. However, to my mind, |
Beta Was this translation helpful? Give feedback.
-
|
Because of that, we opted for a separate signAuthEntry method |
Beta Was this translation helpful? Give feedback.
-
|
How Do we want to make sure that a user really owns the account he authenticating with? I mean, someone can easily clone the open-source wallet repo, and slightly change the code to impersonate some other account owner, returning a public key from the account they doesn't own (e.g., one of SDF's accounts). We have two options: either sign and return some challenge in this method, similar to the |
Beta Was this translation helpful? Give feedback.
-
|
@johansten The examples you're using are not relevant with Stellar:
That's exactly how it works with most of the Stellar wallets |
Beta Was this translation helpful? Give feedback.
-
|
It was an example of how API design can have unintended downstream consequences on UX. |
Beta Was this translation helpful? Give feedback.
-
|
@earrietadev Just for context, @johansten is an OG and the creator of a one of the first Stellar wallets. |
Beta Was this translation helpful? Give feedback.
-
|
@orbitlens I'm aware of it
The proposed method was "getaddress" not "getaddresses"... There is always the possibility to just extend the standard if is that big of a deal. But to be honest even if "getaddresses" were proposed I'm not sure I will be adding it to xBull, why? because I have 741 stellar accounts across 37 wallets on different networks in xBull, most sites won't even be able to handle them. Heck most apps don't even keep one single account across sessions in the app so now imagine asking devs to even show a proper UI to select one of your accounts My case is of course an extreme case because not everybody is that active on the network, but my point is that not all wallets handle all accounts in the same way because in my case xBull allows you to set names to "wallets" and "accounts" inside of it to better organize everything, asking dapps to do the same so they are compatible with xBull "getaddresses" method is not going to be easy because most of them will just pass (like they already do by not caring about accounts management) |
Beta Was this translation helpful? Give feedback.
-
|
If we don't do a wallet API that supports multiple accounts now, will the dapps ever adapt? That was the only reason I did Stargazer back the in the days. The 2015 protocol was so ahead of its time it was insane, and the other wallets at the time, primarily Centaurus and Lobstr, hardly even scratched the surface in terms of what they could do. I think it took them years before the even did non-native assets. And from the start, I did non-native assets, I did multiple accounts, I did transactions with multiple source accounts, I did source accounts with multiple signers, the lot. Because the protocol deserved it. |
Beta Was this translation helpful? Give feedback.
-
|
We probably need to standardize the implementation of the Our algorithm: function signMessage(message, keypair) {
const publicKey = keypair.publicKey()
const signedMessage = keypair.publicKey()+':'+message //protection from spoofing
const messageHash = sha256(signedMessage)
const signature = keypair.sign(messageHash).toString('hex')
return {publicKey, signature, signedMessage}
}From my point of view, the |
Beta Was this translation helpful? Give feedback.
-
|
Update: See https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0053.md We should ensure that any string (Chinese, Japanese, etc.) can be properly processed. Also, I think using function signMessage(message, keypair) {
const signedMessage = 'Stellar Signed Message:\n' + message;
const messageBuffer = Buffer.from(signedMessage, 'utf8');
const messageHash = sha256(messageBuffer);
const signature = keypair.sign(messageHash);
const publicKey = keypair.publicKey();
return {
publicKey,
signature,
signedMessage
};
} |
Beta Was this translation helpful? Give feedback.
-
|
Regarding the As I see it, the network selection during the transaction signing should be controlled by the dapp UI. A user should see the requested network in the wallet interface during the request authorization, but shouldn't be able to change it (again, to avoid weird glitches). The network, selected by the user in the wallet interface prior to the transaction signing request must not affect the request received from the dapp. If the network was not specified in the request, the wallet should assume that the dapp uses Stellar Pubnet by default. |
Beta Was this translation helpful? Give feedback.
-
|
Do you think there's a security implication here? For example, a dapp could claim that their transaction is only meant for testnet, but then actually configure for mainnet. The user would have the opportunity to see the discrepancy in the wallet's UI, but they might miss it if they're not careful |
Beta Was this translation helpful? Give feedback.
-
|
FWIW, I started defining a version of Wallet Standard, https://wallet-standard.github.io/wallet-standard/, for Stellar. One more standard, you say? <insert obligatory xkcd strip> The naming is a bit presumptuous, but as a spec, "Wallet Standard" simply slaps. It's the most sane browser extension wallet framework I've seen. I think emphasizing framework here is important, because this is an end-to-end solution. It's not just an interface to implement, it's the supporting framework to use your interface within. It covers what you need to do as a wallet developer to implement your interfaces, and how to register them. It covers what you need to do as a dapp developer to discover registered interfaces, and how to access them. Nothing about signing transactions? Well, Wallet Standard is a framework, right? There's what's called "features", with which a wallet can expose functions, and information about those functions, e.g. versioning. There's a few standard features that wallets have to implement, Other than that, it's a free-for-all. With Solana defining this, they obviously had their perspective on things when they designed the interfaces for Solana wallets, and a lot of them make a lot of sense in general. They rightfully take the stance that the wallet state has nothing to do with dapp state(s). Thus, when a user connects a wallet to a website, they decide which account(s) to expose to the dapp, and it's then up to the website to keep track of which account it's currently using, and pass that along with any call to the wallet. There is no global, currently active account. There is no global, currently active network. Changing network in one dapp, shouldn't affect either the wallet or any other dapp. Now, I'm not sure if I agree with their idea of using arrays for inputs/output, i.e., always allow the user so sign an arbitrary amount of things in one call, or with the idea of returning fully signed transactions instead of just a signature. But yeah, you're free to go your own way. You can, e.g. look at Aptos, who took everything good about the Solana interface, and threw it out the window. Codehttps://github.com/johansten/majestic-suitcase/
Things I've learned
Going forward
|
Beta Was this translation helpful? Give feedback.
-
|
Example web at https://johansten.github.io/majestic-suitcase/ |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Background
Many browser wallets expose an API so dapps can interact with a user's wallet. This allows a dapp to ask a wallet to do things like transmit a public key or sign an XDR.
Problem
Right now wallets all implement this API differently, which can make building a dapp or an SDK difficult; a dev must account for all these differences and manually integrate different wallet API's in order to support as many wallets as possible. This will conceivably become more complex as more wallets enter the ecosystem.
Proposal
The ecosystem agrees to a standard wallet API interface to make wallet integration easier. If all wallets agree to a minimum standard interface, a dapp or SDK developer could reliably infer what methods will be supported and what the function signature will be.
Example API interface:
Relevant discord conversation: https://discord.com/channels/897514728459468821/1019346446014759013/1206810627377467413
Beta Was this translation helpful? Give feedback.
All reactions