Injected Wallet Standard #370
Conversation
| receiverId: "guest-book.testnet", | ||
| methodNames: [], | ||
| }, | ||
| accounts: accounts.map(({ accountId }) => { |
There was a problem hiding this comment.
So is it expected that connect() and signIn() are now separate actions that the user needs to respond to in the wallet ?
It's mentioned that during connect() the user must select which accounts they want to "expose" to the dapp. And then the signIn() action is separate to allow a FunctionCall access on the accounts... This feels like a lot of steps which could be combined into one? (connect() could optionally pass FunctionCall parameters if the dapp wants method access on the selected accounts)
I also feel it's a little strange connecting with multiple accounts, seems like it might be confusing UX for users and requires more management on the dapp side to make sure account selection is well implemented for user actions. Whenever I've connected to a dapp- its usually with the expectation of a single account.
There was a problem hiding this comment.
Yes - connect() is separate from signIn() for a few reasons.
From the dApp side:
connect() means 'wallet, please tell me what account(s) I can request signing requests for'
signIn() means 'wallet, please add this access key to one of the accounts that was exposed when I connect'd to the wallet, so that I (the dapp) can submit transactions using that key instead of having to request explicit signing via the wallet even for simple and 'safe' transactions.
From the wallet side:
connect() means 'dApp, here are the accounts that the user would like to be able to use to interact with you'
signIn() means 'dApp, I have added an access key to the requested account so that you can submit limited transactions directly on behalf of the user without needing explicit Wallet interactions for each transaction'
Because connect() does not involve a blockchain transaction and is only an interaction between the dApp and the wallet, its failure modes are specific to wallet interactions and will never include failures such as 'transaction failed' etc.
Because signIn() involves adding an access key to the requested account, its failure mode is specific to those modes encountered when a blockchain transaction fails for whatever reason.
The separation between connect() and signIn() becomes more clear when we look forward to off-chain account verification.
For example, lets say you want to have someone 'log in' to your off-chain forum site by using their NEAR account as their identity.
Currently, you would need to have the user signIn() so that you could verify that they own a particular account by looking on-chain and verifying that they added the access key you requested to the account. Although this approach is secure (we know that someone owns a particular account if they can add access keys to it), it is sub-optimal in cases where the requesting app never intends to submit any transactions using that limited access key; essentially we are having the user mutate their on-chain account to prove they own it, but in a way that costs them on-chain storage and isn't actually useful after they have added the key, thus proving they own the account.
Note that this can be the case even for dApps that do interact with the blockchain! Some dApps require that all transactions they submit must be signed using a Full Access Key, so the 'sign in' process that creates a limited access key is superfluous to them.
So the idea is that while you will always connect() a dApp to a wallet instance, depending on the dApp type, it may request a signIn() if the dApp needs/wants a limited access key in its local scope, but alternatively it may request something like e.g. verifyAccountOwner() instead, if the dApp only needs to be absolutely sure that the requesting user controls a particular on-chain account. Note that the failure mode of this new verifyAccountOwner() method will be different than either connect() or signIn() because it will not involve submitting an on-chain transaction; by keeping these behaviours separate, we allow for more flexible usage patterns and simplify each of the APIs involved and their possible failure modes.
There was a problem hiding this comment.
I've recently implemented verifyOwner() in our wallet and I definitely see the benefits you outlined here about the separation between connect() and signIn(). What follows is more a critique of the actual interface of connect().
From the dApp side:
connect() means 'wallet, please tell me what account(s) I can request signing requests for'
From the wallet side:
connect() means 'dApp, here are the accounts that the user would like to be able to use to interact with you'
I just feel like this is going to be a confusing UX for the user, as well as for wallets- now we have to provide an interface for users to select multiple accounts on a connect() request ? I don't think I've ever "logged in" to a website with multiple accounts before and I don't really see the benefit ? Especially since we can easily swap and change between accounts by just making a new connect request to the wallet (potentially there could be a swapAccount() method or something of the like, which opens the wallet to select a new account, even if we are currently "logged in" with an account). What I was trying to say is that I feel this is going to cause more complexity for both dapps and wallets (and potentially confuse users). More direct action paths are always better in my view.
Also dapps are now going to have to know for sure which one of the multiple selected wallets the user wanted to complete an action with. Does this mean its expected of all dapps to provide a kind of account dropdown list (or similar UI) for "currently active" account? You wouldn't want to do a forum action with the wrong account by mistake... now that we have "connected" with multiple accounts we require additional layers to prevent such mistakes.
If you are hard set on this interface, then I would request that you allow Dapps to provide an extra parameter to connect() - something like { occupancy?: "multple" | "single" | number } - that would allow those Dapps which only expect to work with single accounts the option to build their UI around that.
Lastly, I do hope that the connect() method is going to provide even more context to the Wallet. Especially network, as I mentioned in another comment. Connecting with a wallet in an incorrect network is only going to get caught later on the Dapp side and could be a surprising error for users. Better to have these things determined from the get go.
Please don't take this as me arguing against connect() though ( I'm merely a bit critical about how it is currently being implemented)- I definitely do see the benefits of connect()- being able to verify and use accounts without blockchain interactions- I've recently added this (verifyOwner()) functionality to Meteor Wallet and its an exciting feature for sure! It opens up the door to sleek interactions on Dapps while still providing solid account identity.
There was a problem hiding this comment.
It does make things more complex to allow users to select more than one account to be shared with the dApp from a wallet -- but this is actually a very often requested feature that we hope to support as a first-class concept in the API. Note that although connect() can return multiple accounts, only one of those accounts generally would be considered 'active' at once in the dApp context -- and generally that account would be set after either a verifyOwner() or signIn() call has been completed for the account that the user wants to be their active account.
Although each dApp author could write their own code to handle interacting with wallets and prompting the user to select the 'active' account, we don't think it's a good use of their time to do so (this is the equivalent of boiler-plate that distracts the dApp developer from solving the real problems they are trying to solve). This aligns with the philosophy that integrating with many different wallets and providing the functionality to select which of those wallets to use should also be something that we provide tools for the dApp author to use. We will be adding this functionality to the wallet selector package / modal UIs, so that the dApp developer can use the modal UI it provides to allow the user to see the list of accounts, and to select which account should be the 'active' account for signing in, verifying owner, or signing requests.
If a dApp author wants to write their own account switcher/selector UI, they could do so by piggy-backing on the core package which exposes events, accounts, and a concept of 'active account' for the dApp developer -- or they could just use the UI components that we publish from the wallet-selector and be up-and-running immediately.
Note that it currently stands, we do not dictate that a wallet must return multiple accounts from a connect() call, even if there are multiple loaded into that wallet. The only requirement is that it returns an array with at least 1 account in it -- which allows wallet authors to make the UI for the connect() method simple (select one) or more complex (select multiple) as they see fit for their wallet design, while ensuring that our API is also compatible with more than one being returned.
@lostpebble Does this address your concerns here, or do you think that adding occupancy as an argument to the connect() method is still a requirement for this to be reasonable, keeping in mind that if we do add that option, wallet providers implementation and UI will become more complex.
There was a problem hiding this comment.
Hi @MaximusHaximus - thanks for taking Network into consideration with the connect() method.
@lostpebble Does this address your concerns here, or do you think that adding occupancy as an argument to the connect() method is still a requirement for this to be reasonable, keeping in mind that if we do add that option, wallet providers implementation and UI will become more complex.
I think its okay to leave it out- as long as its always okay for a user to only select a single account to connect with. If you think certain dApps might require the user to connect with multiple (or a certain amount) of accounts, for whatever reason, then having an occupancy option might be a good idea so the wallet can show the relevant UI. But I don't think should ever be the case?
This "multi-account" concept does complicate the Wallet UI a bit though. I guess in our case, when a Dapp calls the connect({ network: "testnet" }) method, we'll show the user an interface that allows them to do one of the following:
- Select a single testnet account
- Select multiple specific testnet accounts
- Connect with all known wallet testnet accounts
We'll have to think of ways of streamlining such an interface.
And after that, I would suppose that future calls to verifyOwner() do not require any further prompts from the user (since they've already allowed the connection). But signIn() would run the specific sign-in flow for the account the dApp has chosen to use.
frol
added
the
WG-wallet-standards
MaximusHaximus
changed the title
|
@lewis-sqa Would signing arbitrary messages be in scope for this NEP? I could see use cases where dapps need arbitrary messages signed with a FAK which would be stored in the wallet instead of the LAKs in the dapp. This would be similar to |
|
Discussing and making decisions on NEPs is crucial for evolving the NEAR platform. The NEP process introduces Working Groups as a way to operationalize the review of NEPs that builds community support. I propose that we use this NEP as the first one to help bootstrap the Wallet Standards Working Group. @cameron-NEAR, @MaximusHaximus, @esaminu – Could you please fully read this NEP and comment in the thread with any feedback/concerns by September 30, and specifically if you would approve this NEP? If not, please specify reasoning and what it would take for you to approve it. I can schedule the first working group meeting after your review in early October. Thank you! |
|
I notice that wallets are not interacting with the |
frol
added
the
S-draft/needs-author-revision
|
@lewis-sqa Let's get this NEP running 🚀 I am acting as a moderator here, so I am here to just clarify who is holding the ball now, and I feel currently we need the clarifications from the author 🙏 |
MaximusHaximus
changed the title
|
Hello folks, Lewis is no longer working on this task, so I am taking over the NEP as author and am creating a new PR based on a branch that I control; I'll be pushing updates to that PR in a few minutes to address some of the questions/concerns mentioned in these discussions. Thanks so much for the input and engagement, this stuff is important and we want to make sure we're solving the right problems :). Please come along to the new PR at #408 :). I'll ping there as soon as new commits are pushed. cc: @PinkiNice @lostpebble @esaminu Thanks for jumping in as moderator, @frol! :) |
We're working to make a separate NEP describing the API for verifying account ownership, and will add a reference to that to this NEP when it has landed. |
## Summary This is a proposal for a new NEP regarding an Injected Wallet Standard. Adopted from #370 as the original author is no longer working on this task; see #370 for prior discussion. ## Demo You can find a demo of this standard using the project detailed below: - Implementation: https://github.com/lewis-sqa/near-injected-wallet-poc - Running the project: - Execute `yarn install && yarn start` from root. - Navigate to http://localhost:1234. Co-authored-by: Lewis Barnes <lewis.barnes@sqa-consulting.com> Co-authored-by: Osman Abdelnasir <esaminu1@icloud.com>
This NEP has been re-authored in a new PR, so it could be adopted from the original author for some tweaks to be made -- thanks so much for your time and efforts to get it this far, @lewis-sqa :)
See: NEP-408