near / near-wallet

Web wallet for NEAR Protocol which stores keys in browser's localStorage
https://wallet.near.org
MIT License
220 stars 176 forks source link

Implicit accounts as first-class citizen #1453

Closed ilblackdragon closed 2 years ago

ilblackdragon commented 3 years ago

Currently NEAR Wallet has pretty broken flow of funding. There are constant issues with users not understanding and needing to recover funds. This has affected everyone from small to large holders already.

We had this conversation previously, and it was mostly due to us wanting to push for "named" accounts that we are doing this flow. At the same time, we are building up new ways of onboarding users via implicit accounts.

I suggest we accept that implicit accounts are first class citizens. Which means that user should be able to create and maintain accounts like that. If they want to create named account after they already have funds in the wallet - the "Create Account" would allow that.

User flows

User doesn't have any existing account

  1. Create Account just allows to setup seed phrase or use Ledger. Less secure option is to email / sms that same seed phrase to the user. This is the same seed phrase that is stored in local storage.
  2. This creates an implicit account

After account gets funded, there is a prominent button appears that says "You can create a new account like .near" that leads to "Create Account".

The implicit account that they have created still in the list of accessible accounts even if named account(s) are created. And they have a way to recover it as well. That account doesn't get deleted either.

Also after account gets funded, it should suggest to improve security of the account via switching recovery key, etc.

User has already existing account in the wallet

  1. Create Account offers two options: create "named" account or less prominent create "anonymous" account.
  2. Select recovery / 2FA / etc.
  3. Uses funds from the account user already have in the wallet to fund this account.
stefanopepe commented 3 years ago

@corwinharrell I want to discuss any modifications to the current flow (and the impact on other activities in the pipeline), so we can think of any temporary mitigations.

I see this request tangent to:

1165 and #1297. 1297 in particular includes the functionality to save and restore the onboarding state, so we may have an opportunity to capitalize on this request for future UX improvements.

Also see:

1188

1130

1131

cc @kcole16

heycorwin commented 3 years ago

@ilblackdragon I am in agreement for the most part. In fact, the initial design proposal for the create/fund account flow had implicit accounts being created before presenting the option to add a unique account name, which the user could choose to ignore if they wished. (This has even already been designed for the most part). Here is the flow that was created to illustrate those changes:

https://whimsical.com/create-account-2y3VYfTQXAAoPo6P2BgFBH

One question I have about a clear UX pitfall in the variant you described above:

If I sign up for a NEAR account as a new user, my goal is to acquire a single account. No user ever signs up to a service hoping to receive multiple accounts.

If the initial implicit account is not deleted after a user opts in to receive a named account, and the named account is not acting as a "replacement" to which the balance of the implicit account if transferred, we are now telling users "You have no choice but to maintain 2 accounts, sorry."

This experience is muddy and is a clear indicator of the UX being dictated by forced limitations rather than by intentional design. I'd be curious to hear some thoughts in response, and if we can think of any ways to mitigate this.

azhang2013 commented 3 years ago

If the initial implicit account is not deleted after a user opts in to receive a named account, and the named account is not acting as a "replacement" to which the balance of the implicit account if transferred, we are now telling users "You have no choice but to maintain 2 accounts, sorry."

so my 2 cents is that this is actually quite acceptable for crypto. I probably have like 20+ different ETH addresses over the year...I dont think there is any downside have having the redundancy. If the user wants to use the name account, he could do that (since it's simple and easy). But if the relationship of the 2 accounts are somehow not easily identifiable via the explorer, it's actually great for the more privacy minded folks. For example, i would use the named acocunts for games or small transcations, but might use the implicit account for staking or anything I would like to keep some level of anonymity.

On the other hand, it's much worse if the account gets deleted and somehow funds got deposited to the old implicit account (which get recreated anyway currently...) But I could tell you there are at least 10+ cases I am aware of where users funded the deleted implicit account. The panic is real (put yourself into the position of someone just purchased 50k NEAR from Binance and whoops, money is gone.)

While this still represents a small % of the users, given the financial nature of crypto, we should aim for the safer choice.

kcole16 commented 3 years ago

At this point, I'm of the opinion that we should not, under any circumstances, issue a deleteAccount transaction from the wallet. It is far too dangerous, and even in the best case scenario can be scary and confusing. It will get even more dangerous once users start acquiring tokens and other assets, as we have no way to migrate/transfer these to another account.

That being said, I strongly agree with @corwinharrell it is suboptimal to force users to create two accounts just to have a named account. We should look into ways to mitigate this (though may need to consider them longer term).

heycorwin commented 3 years ago

@azhang2013 I think those are valid points. I'm not arguing against the fact that keeping the implicit account is the objectively safer option. Ultimately what I am trying to point out is the differences between user expectations and reality. While it's not the end of the world, it's always going to be a little bit of a surprise and a series of inconveniences to users who go in looking to create a single account and end up with 2.

A significant factor to consider as well is the balances of these accounts. For example:

  1. If I create a new account with the Wallet, I first create an implicit account. By necessity, this account will have a balance (regardless of the funding source)
  2. Then, if I wish to create a named account, the new named account will, by necessity, also have a balance. I now have two accounts with 2 balances. If I want to consolidate my balances, I have to go in and move my funds from one to another.

While none of this presents a security risk, there's lots of things here that are simply inelegant and are an inconvenience for the user's original expectation and intent to sign up and create a single account with a single balance.

vgrichina commented 3 years ago

I agree with @corwinharrell that just giving user 2 accounts gonna be suboptimal and confusing.

I think a solid compromise here which we discussed before is to keep implicit account info, but only show it if it still has funds. Or maybe even show it specifically as "TODO item" saying "claim your funds" or smth like that if you accidentally send more tokens there.

In any case, something that we need to address is to have a flow to let user finish something they dropped off at (like creating account). We have quite a few of different tasks which often aren't completed: https://github.com/near/near-wallet/issues/1182

We had this conversation previously, and it was mostly due to us wanting to push for "named" accounts that we are doing this flow. At the same time, we are building up new ways of onboarding users via implicit accounts. I suggest we accept that implicit accounts are first class citizens.

@ilblackdragon another alternative is to change funding flow to send directly to .near accounts. Basically have a faucet that creates limited .near account and removes it within given amount of time if not funded. I think this is the best solution in terms of avoiding confusion, but creates mild DoS risk (attacker can lockup faucet tokens for a while).

vgrichina commented 3 years ago

At this point, I'm of the opinion that we should not, under any circumstances, issue a deleteAccount transaction from the wallet. It is far too dangerous, and even in the best case scenario can be scary and confusing. It will get even more dangerous once users start acquiring tokens and other assets, as we have no way to migrate/transfer these to another account.

I think what we need to do is check whether account has any other keys except the one that is "implicit":

ilblackdragon commented 3 years ago

if it only has default key – we can delete it safely as it will get recreated with it's implicit key once it receives funds

The point is that the deletion of account is very unexpected behavior for existing blockchain users. Even if it gets recreated and there is a normal flow there. Making sure uses are not surprised and delighted is the goal here.

Basically have a faucet that creates limited .near account

Yes, if we make a faucet - that would be the best.

vgrichina commented 3 years ago

The point is that the deletion of account is very unexpected behavior for existing blockchain users. Even if it gets recreated and there is a normal flow there. Making sure uses are not surprised and delighted is the goal here.

Using this argument we should also revert back to using one seed phrase and key and approving every transaction through wallet – as having multiple keys, app logins, etc is also by itself surprising for existing blockchain users.

Note that I agree 100% that nobody should lose access to funds or even feel like they lost access to their funds.

But "make sure existing blockchain users not surprised" isn't a sufficient argument here, as we've consciously decided to use surprising model of accounts with multiple keys. It's always gonna keep surprising users from existing blockchains.

I have a fix here to avoid deleting accounts when they aren't exactly equivalent to not created ones: https://github.com/near/near-wallet/pull/1461/files

Basically have a faucet that creates limited .near account

Yes, if we make a faucet - that would be the best.

Glad we have consensus here. Funding through implicit accounts is definitely at best a kludge for existing infra, not a desired experience that we need to optimize.

ilblackdragon commented 3 years ago

I think what we need to do is check whether account has any other keys except the one that is "implicit":

  • if it has any other keys – means you actually imported it into wallet as full account and it's in use. We don't want to delete such account
  • if it only has default key – we can delete it safely as it will get recreated with it's implicit key once it receives funds

That's not always correct. If someone created an account in another wallet application and then NEAR Wallet removes this account - the other wallet application may not know how to behave around that. Also that account is probably should not be drained of funds in the first place.

So I second @kcole16's point to not delete any accounts. Instead add implicit account into the list of available accounts in the NEAR Wallet.

Also note what Robert pointed out - that if someone else creates an account with the name you entered while you are funding your implicit account - user is stuck in a very weird and unexpected state (they see account they entered exists but it's not theirs).

we've consciously decided to use surprising model of accounts with multiple keys

There is surprising as new functionality (named accounts, multiple keys, contracts on accounts) which is fine and there is "I just sent $100k to this account but it doesn't show up in my wallet" surprising.

heycorwin commented 3 years ago

Alright so in order to move forward with design, I think we need consensus on a couple key components here. @ilblackdragon @vgrichina @kcole16 if we could achieve this by the end of the week we could work to design around this problem in the next sprint. Would be great to get answers articulated as succinctly as possible.

  1. Account deletion: Will we ever delete an account during the create/fund flow? If so, at what point?
  2. Faucet: Is it realistic for us to plan on designing around a faucet any time soon?
  3. Multiple Accounts: Is it possible for us to avoid the issue of supplying the user with more accounts than they need, given our constraints?

Then ultimately, it would be great to get a summary of how these considerations alter the flow you have outlined in the issue description @ilblackdragon.

stefanopepe commented 3 years ago

I agree with Corwin, let's wrap this discussion.

I don't have enough elements to decide myself which hypothesis is better, but based on what I already know:

  1. the account deletion is still useful to sweep funds from unused accounts, so I would disable it from the onboarding and the wallet in general, but be ready to keep it for very specific (and tested at every release) corner cases - e.g. decommissioning of lockup accounts

  2. please do this temporary faucet! We also have the feature #17 in the roadmap, proposed by Nima.

  3. it may not be KISS-compliant, but we may propose a sweep* command as a middle-ground between keeping the implicit account, and using a faucet to maintain the named account as primary.

*Common Bitcoin wallets have two distinct actions: import a private key, and sweep a private key. While the first is self-explanatory (you simply add more keypairs to your wallet, along with their addresses), the latter was used to transfer funds from an old type of private key (WIF format) to a new one (BIP32). In our case, the wallet would detect funds in the original implicit accounts, and kindly ask the user to one-click the transfer to the new main account.

vgrichina commented 3 years ago

Faucet: Is it realistic for us to plan on designing around a faucet any time soon?

@corwinharrell faucet is definitely realistic as long as there is will to fund it with tokens + maintain current flow as fallback if faucet is under DOS attack.

vgrichina commented 3 years ago

That's not always correct. If someone created an account in another wallet application and then NEAR Wallet removes this account - the other wallet application may not know how to behave around that. Also that account is probably should not be drained of funds in the first place.

If somebody has created account in other wallet application how would it end up as a funding account? If user imported it in the wallet with seed phrase – it would have extra keys on account.

The problem with deleting account is specific to Ledger, because with same Ledger you get the same funding account ID on every device, which we didn't account for specifically.

Instead add implicit account into the list of available accounts in the NEAR Wallet.

This is very suboptimal for a happy path. First – we'd have to leave some extra tokens to keep implicit account around (why even do it when it is actually recreated when you send tokens again?), second – instead of getting questions and confusion about 2 accounts only for failure cases we now would get them for success case as well.

vgrichina commented 3 years ago

Also note what Robert pointed out - that if someone else creates an account with the name you entered while you are funding your implicit account - user is stuck in a very weird and unexpected state (they see account they entered exists but it's not theirs).

This is separate orthogonal issue. Your implicit account won't be removed in this case (at least after bugfix to make sure create_account doesn't fail silently )

we've consciously decided to use surprising model of accounts with multiple keys

There is surprising as new functionality (named accounts, multiple keys, contracts on accounts) which is fine and there is "I just sent $100k to this account but it doesn't show up in my wallet" surprising.

Sure, that's what I think too:

Note that I agree 100% that nobody should lose access to funds or even feel like they lost access to their funds.

Also check out https://github.com/near/near-wallet/issues/1182 – I think there is quite a few other issues with dropping off something, etc that we need to surface to user.

Like e.g. if user deposited funds again accidentally to implicit account, we should show it somewhere. But IMO it shouldn't be like having an actual account in a list. But more like "Claim funds from 343432434...342343 to what_you_wanted.near".

In any case my preferred solution would be to invest most of the time to get funding .near directly working 95% of the time. Then implicit account funding becomes a fallback and we wouldn't need to find an ideal solution for a "fallback for a fallback".

stefanopepe commented 3 years ago

@vgrichina the idea of a faucet seems the best one, but it's also tangent to the wallet today, as we need a constant inflow of tokens and some "budgeting". Also, providing free access to named wallets opens multiple attack vectors (see below).

So, to recap the discussion, and following the latest comments:

I am not sure if forcing the creation of named accounts will turn away privacy-savvy users, and will keep the application usage lower. I'm also worried that we are not following @ilblackdragon initial request, which was to include the implicit accounts as a viable, first-class alternative to named accounts inside NEAR Wallet. I think it's worth forking this discussion into two distinct parts:

  1. How to remove the implicit-to-named "wallet dance" by using a faucet
  2. How to adapt the existing wallet UX to use an implicit account instead of a named one

// Potential attacks I can think of:

  1. create a named account, receive funds, authorize near-cli, delete account and sweep the received tokens
  2. .near accounts name-squatting

// Mitigations (not to discuss here, just giving an idea of what we may need to do):

  1. withhold the creation fee from the first outbound transaction, and send it back to the faucet (so it's essentially a loan, not a gift).
  2. attach a randomly generated string to the name. E.g., I input stefano, the system would generate a random 1-2048 number, and attach the corresponding word -> the number 23 would generate stefano-actress.near (with the possibility to re-generate the random world, because, you know).

I feel sorry this is taking a bit longer than expected, happy to wrap everything into a bullet of decisions to take, and have a meeting within this Friday

vgrichina commented 3 years ago

@vgrichina the idea of a faucet seems the best one, but it's also tangent to the wallet today, as we need a constant inflow of tokens and some "budgeting".

We don't need constant inflow of tokens. We only need liquidity, as users either eventually fund accounts themselves or have it expire.

I am not sure if forcing the creation of named accounts will turn away privacy-savvy users, and will keep the application usage lower.

We aren't forcing anybody to create account with their name or anything else privacy revealing. Users have no problem staying anonymous on various Internet forums, Reddit, Twitter, etc.

I think it is also far better in setting actual privacy expectations. The idea that all privacy you have is a pseudonym to hide behind (like on Twitter and Reddit) is more visible here. "crypto" addresses might create expectation of privacy that is actually not there de facto.

I'm also worried that we are not following @ilblackdragon initial request, which was to include the implicit accounts as a viable, first-class alternative to named accounts inside NEAR Wallet. I think it's worth forking this discussion into two distinct parts:

I don't think we should do something just because this was initial suggestion by @ilblackdragon. Like he himself confirmed that .near faucet would be the best: https://github.com/near/near-wallet/issues/1453#issuecomment-779825159

  1. How to adapt the existing wallet UX to use an implicit account instead of a named one

I think we shouldn't do it. There are other wallets on NEAR focusing on implicit account model. Like you can even use Trust Wallet for this.

I think it makes sense for NEAR Wallet to focus on stuff that other wallets generally wouldn't do.

  1. create a named account, receive funds, authorize near-cli, delete account and sweep the received tokens

This is not going to be possible. User won't get full access key to account until they fund account. It is achievable through either:

  1. .near accounts name-squatting

To protect against squatting for free we'd basically need to have following:

Given that you only can squat for duration of grace period it isn't profitable.

withhold the creation fee from the first outbound transaction, and send it back to the faucet (so it's essentially a loan, not a gift).

yeah, there is no plan to give tokens for free in this scheme, you get loaned .near account to receive funds until you either pay for it or loan expires

stefanopepe commented 3 years ago

All makes sense. I like the concept of using .near accounts in any case, so the user is aware that privacy comes from his behavior and not the illusion of having a difficult to remember hexadecimal string.

So to recap and unblock @corwinharrell :

  1. narrow down the user experience to .near accounts only, replacing any temporary implicit account with this "lending faucet"
  2. find the ideal/reasonable time-frame during which the user must fund the wallet and complete the onboarding
  3. draft some basic behavior of the faucet in terms of messages, possible errors, and any sort of latency that could be experienced by the user, so Corwin can include them in the user flow

I prefer the idea of an on-chain contract, it makes it reusable by others in the ecosystem, it is transparent and is composable.

vgrichina commented 3 years ago
  1. find the ideal/reasonable time-frame during which the user must fund the wallet and complete the onboarding

We don't have to find it right now, it is best determined experimentally

I would start with 24 hours and reduce as needed if/when system gets abused.

  1. draft some basic behavior of the faucet in terms of messages, possible errors, and any sort of latency that could be experienced by the user, so Corwin can include them in the user flow

so obtaining account from faucet can have 3 results: 1) success (then it just follows through with funding flow using .near) 2) error because name already claimed (this one has to surface to user) 3) error because faucet exhausted or any other reason (continue to existing implicit account funding flow as a fallback)

as for latencies I don't think there is any significant difference with existing flow

I prefer the idea of an on-chain contract, it makes it reusable by others in the ecosystem, it is transparent and is composable.

yeah, similar contract also would work for app-specific account funding

heycorwin commented 3 years ago

The account creation experience is much improved with a faucet in the background temporarily initiating the reserved .near account. Here is what the sequence looks like once we're able to omit the implicit address from the funding flow (With some improvements):

Screen Shot 2021-03-16 at 11 39 52 AM

@vgrichina My main question is:

Once the Account ID is reserved and the user has reached the funding step, is there anything preventing us from allowing the user to resume their account setup by simply going through the recovery flow? (I think we can tweak some of the language depending on their entry point and the state of their account)

vgrichina commented 3 years ago

dropped priority to P1, as we are clearly not working on it as P0 de facto

vgrichina commented 3 years ago

@vgrichina My main question is:

Once the Account ID is reserved and the user has reached the funding step, is there anything preventing us from allowing the user to resume their account setup by simply going through the recovery flow? (I think we can tweak some of the language depending on their entry point and the state of their account)

I don't think anything is preventing us to do it. In fact we can do that even for our existing funding flow with implicit account. Maybe actually this is what we should start with before doing faucet.

I like the flow above, but I have small concern that we aren't letting user know beforehand that account will need to be funded. So we might end up in situation where way too often users claim .near account temporarily only to give up on funding stage.

I think it's worth looking for existing users on how often they drop off on funding stage. @TiffanyGYJ @icerove do we have this data for mainnet wallet?

ilblackdragon commented 3 years ago

I would expect for crypto wallets it's probably 50-90% of all accounts created never get funded (looking at bunch of random accounts I've created for ETH with 0 tokens in them). We can ask around.

heycorwin commented 3 years ago

I like the flow above, but I have small concern that we aren't letting user know beforehand that account will need to be funded. So we might end up in situation where way too often users claim .near account temporarily only to give up on funding stage.

@vgrichina This is good feedback. Happy to include some messaging earlier in the flow to set expectations around funding 👌

TiffanyGYJ commented 3 years ago

@vgrichina My main question is: Once the Account ID is reserved and the user has reached the funding step, is there anything preventing us from allowing the user to resume their account setup by simply going through the recovery flow? (I think we can tweak some of the language depending on their entry point and the state of their account)

I don't think anything is preventing us to do it. In fact we can do that even for our existing funding flow with implicit account. Maybe actually this is what we should start with before doing faucet.

I like the flow above, but I have small concern that we aren't letting user know beforehand that account will need to be funded. So we might end up in situation where way too often users claim .near account temporarily only to give up on funding stage.

I think it's worth looking for existing users on how often they drop off on funding stage. @TiffanyGYJ @icerove do we have this data for mainnet wallet?

Yes, we have data for mainnet wallet and we can pull data around the step of funding

heycorwin commented 2 years ago

Closing this issue as we are now working on implementation here: https://github.com/near/near-wallet/issues/2284