stacksgov / sips

Community-submitted Stacks Improvement Proposals (SIPs)
132 stars 80 forks source link

SIP: Modern Wallet APIs #166

Open janniks opened 8 months ago

janniks commented 8 months ago

SIP

Will remain a draft while Xverse, Leather, and other are asked to contribute

kyranjamie commented 8 months ago

Great work @janniks. Leather supports this SIP πŸ–€

janniks commented 7 months ago

Thanks for the input @m-aboelenein @kyranjamie . I updated the stuff we chatted about on the call, and also added some OPEN QUESTIONS to the top.

Do you think you could do another review with final remarks until ~Tue. 20.02.2024 to wrap this up with bipartisan support? πŸ™

janniks commented 7 months ago

Added methods getAddresses and getAccounts for better compatibility with current solution.

janniks commented 7 months ago

@aryzing @kyranjamie I added an update to the encoding (as discussed on last weeks call). Let me know what you think! This can be used as the canonical repr and Stacks.js would also migrate towards it. (Serialized hex encoded string could also still be used (easy to tell apart).

Also updated:

Ping me if you disagree with anything -- trying to make the naming more logical and short

janniks commented 7 months ago

CC @pradel would also love to get your feedback on this updated approach to "Connect"/auth -- it's less convoluted, but also more explicit (might need multiple steps for some things, that were just magically done in auth previously , e.g. requesting a gaia token would be it's own step, (although wallets could also add it to something like getAddresses / getAccounts)

pradel commented 7 months ago

@janniks I like this change, less magic on what's going on and the behavior seems to be pretty similar to how ETH is doing things, making it easier for ETH devs to use the API

janniks commented 7 months ago

CC @friedger would also love to get some of your feedback+expertise, since you've worked on this area in the past πŸ™

janniks commented 7 months ago

Also, Thanks for all the feedback everyone! πŸ‘

I feel like we're nearing a good document here. I think we can wrap it up soon 🫑

janniks commented 7 months ago

Let me know which email-addresses/github-usernames to add to the author list. cc @kyranjamie @m-aboelenein @aryzing

kyranjamie commented 7 months ago

@janniks sips@kyranjamie.com & @kyranjamie

aryzing commented 7 months ago

Let me know which email-addresses/github-usernames to add to the author list

Aren't all the commits yours? :joy: I wasn't even thinking about making it to the author list, not sure I've contributed enough, up to you :shrug:

janniks commented 7 months ago

I’ll consider anybody an author/editor that contributed in some say πŸ˜‰ so lmk. Honestly I don’t need the section, but probably even changeable later

aryzing commented 7 months ago

Sure, why not :) link to my github, @aryzing , thanks :pray:

314159265359879 commented 6 months ago

@janniks does this modern wallet API, include API's that would help the wallet/dapp know if there is a difference between the network and/or account between what is active on the extension and what is signed in with on the dapp? Say you are on account 1 in the dapp and your Leather is active on account 2. In Metamask you would get a warning (do you want to switch to account 2?) or something like that. I have a feeling that could help prevent some issues where users are confused about which account is signing.

A. Account switching, There may be other relevant issues, but this is a recent one that came to my mind https://github.com/leather-wallet/extension/issues/4859 As part of that, this one is probably harder/impossible? (detect when different secret key is used) https://github.com/leather-wallet/extension/issues/3006

Similar with different networks being used. Especially relevant when users are on testnet and forget to switch back to mainnet... the experience is currently confusing for users. This issue pops up every time there is a popular public testnet.

I think we'll see similar issues once subnets and other layered approaches become popular which require users to install/add networks much like Metamask does for L2 on Ethereum. B. Network: https://github.com/leather-wallet/extension/issues/4194 (changing network, adding network, dapp to prompt switch or addition)

m-aboelenein commented 6 months ago

@janniks @m-aboelenein & mahmoud@xverse.app

m-aboelenein commented 6 months ago

Exciting update! I am thrilled to share that we now have a beta version of sats-connect that implements the SIP check the docs for details. https://docs.xverse.app/sats-connect-wallet-api-for-bitcoin-and-stacks-1 Very excited to hear your thoughts and feedback πŸ™Œ

aryzing commented 6 months ago

does this modern wallet API, include API's that would help the wallet/dapp know if there is a difference between the network and/or account between what is active on the extension and what is signed in with on the dapp?

Doesn't this imply that the account management strategy / state management strategy of wallets would leak into the spec? Seems that the concept of an "active" account is just a way for some wallets to organize or handle what data they keep in memory or present to the user. A "comprehensive" wallet could present all accounts & balances simultaneously to the user (even from both mainnet & testnet).

Would it make sense to include the address & network as part of the request? After all, if the result of the operations in this SIP are a function of the network and address, should those not be included in the requests too?

If included, the "comprehensive" wallet described above (that has no concept of an "active" account) would know exactly what to do too. For wallets segregating state based on the "active" account abstraction, they'd have a clear way of checking whether address / network match the currently "active" ones and act as seen fit.

janniks commented 6 months ago

There was similar ideas to https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1193.md#chainchanged-1 -- we haven't scoped them onto this SIP yet, but we can add it.

314159265359879 commented 6 months ago

There was similar ideas to https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1193.md#chainchanged-1 -- we haven't scoped them onto this SIP yet, but we can add it.

address and network as part of the request, I would love to see that scoped in. @aryzing thanks for that explanation I do believe that is a valuable addition to this sip to help deal with the issues I mentioned with the wallet.