search

near / near-wallet

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

Improve Balances #1189

Closed kcole16 closed 3 years ago

kcole16 commented 3 years ago

Overview

Our current implementation of NEAR balances has created confusion for some users. Much of this confusion may be due to our attempts to abstract certain complexities from the user, e.g. that "Locked" tokens are actually in a separate account, and resulting inconsistencies in these abstractions.

Goals

  1. Show the user all of the NEAR that their account controls
  2. When the user initiates an action that involves using or transferring NEAR, show the user the max amount available to them for this action
  3. Clearly explain how and why the total amount and the available amount for a given action are different
  4. Enable the user to clearly determine when and why a balance has changed (staking, tokens unlocking, etc)

Balance Components

Account balance All accounts have a NEAR balance. This balance must be >= 0.

Minimum balance/state stake Accounts must maintain a variable minimum balance, based on the amount of storage they are using. For example, accounts with 2FA have a multisig contract deployed to them, and the storage used by the contract "costs" 35 NEAR to maintain.

Staked (in a staking pool) Users can stake their tokens via a staking pool. These tokens will be in one of three states:

Users will earn yield on tokens in the "Staked" state.

Lockups Some accounts are "owners" of lockup accounts, which are accounts with a lockup contract deployed to them. Lockup accounts have the same balance components as all other accounts, with one additional token state:

"Locked" tokens can staked in a staking pool, used for gas costs, and used as state stake.

Lockup accounts have a minimum balance/state stake of > 35 NEAR, due to the lockup contract.

Designs

https://www.figma.com/file/Ow6fxJZpmCnpL2qnHceCJq/Balance-Update?node-id=0%3A1

Acceptance Criteria

Implementation

  1. Merge staking balance calculation PR from @mattlockyer
  2. Construct a new ‘Balance’ object that includes
    • Account balance object w/ breakdown
    • Lockup account balance object w/ breakdown
    • Total account balance for all accounts
    • Staking pool balances?
  3. Clean up/revert including the ‘unlocked’ balance as available on any send / app sign txs etc across wallet
  4. Implement the new balance on the /profile
kcole16 commented 3 years ago

Proposal: Break out account balances

Main Account

Lockup Account

Potentially, add a universal "Total" balance, which represents all of the NEAR the user's account controls.

This would be:

Total Balance = main account balance + main account total staked + lockup account balance + lockup account total staked
frol commented 3 years ago

I like the proposal to explicitly separate the main account and the secondary accounts!

Please, keep in mind that in a general case, a single account may potentially have more "secondary" accounts, and also some ERC20 or other assets. I suggest we take that into account and let users add more secondary accounts right in the UI (at least, design with that in mind)

kcole16 commented 3 years ago

@corwinharrell @Patrick1904 What are your thoughts on implementing the proposal above in the current /profile as a quick fix? And then a more comprehensive balance/UI update as a separate issue? (which I believe @corwinharrell already has some plans for)

heycorwin commented 3 years ago

@kcole16 I'm a bit concerned with having the profile just be a giant wall of balances that the user has to parse :/ But let me explore the lowest-lift option for just displaying these balances on the profile in a semi-digestible manner. Can share EOD Monday?

Patrick1904 commented 3 years ago

I made some minor adjustments, any thoughts on this? @kcole16 @corwinharrell.

Main Account

Lockup Account

heycorwin commented 3 years ago

I'm fine with that although the withdraw functionality increases scope. Aren't we going for the bare minimum before Christmas?

I'd also suggest some styling to indicate a hierarchy/breakdown of each total balance and how it is computed.

image

heycorwin commented 3 years ago

I personally don't think we should include staking rewards in balances, since right now those are automatically re-staked. I think staking balances should be simplified to Staked and Unstaked(awaiting withdrawal)

Patrick1904 commented 3 years ago

Yes I meant more just what actual info to include, and grouping, not necessarily how we display it. I agree with your design proposal. The reason I think we should add the 'withdraw' button to 'unlocked balance' is because several users have found this confusing and have not been able to actually 'use' the 'unlocked' balance.

ilblackdragon commented 3 years ago

Big ask is to show how much was staked and how much reward was received. For example in the displays above "Total staking balance" with 4234 and "staking in pools" of 522 is super confusing. I would expect "Total staking balance" let's say 10234, where "Staked" 10000, and "Rewards" 230, "Unstaked" 4.

(It may be because you put random numbers - I would recommend to use numbers that add up together as we are really trying to fix the confusion of the numbers here and mockups should represent that and numbers are part of the mock up.)

ilblackdragon commented 3 years ago

And as mentioned above - there can be multiple lockup contracts and there will be other contracts, I would start training users of lockups as separate app vs trying to keep it as a "standard" thing by creating a top level selection.

In your case this choice even goes above account ID - which feels a bit weird, as seems like one chooses a different account.

heycorwin commented 3 years ago

It may be because you put random numbers - I would recommend to use numbers that add up together as we are really trying to fix the confusion of the numbers here and mockups should represent that and numbers are part of the mock up.

@ilblackdragon I agree, I'll try and make a more conscious effort to use figures that add up in mockups instead of banging on my keyboard like a monkey for random placeholders 😂

I would start training users of lockups as separate app...

@ilblackdragon can you clarify what you mean by separate app? Also, It's a good reminder that we need to be accommodating of accounts with multiple lockups, however, do we have an idea of how frequent this is?

kcole16 commented 3 years ago

@ilblackdragon While we can't control what other devs do re: multiple lockups on a single account, I'd push back strongly on our team adding multiple lockups to a single account.

If we keep the lockups we deploy (NEAR, Inc and NF) as 1-1 to the "controlling" accounts, this interface makes clear both the association of the "controlling" account and the lockup account, while allowing us to display a more discrete breakdown of the lockup account's balances (hidden entirely in the current interface).

vgrichina commented 3 years ago

@ilblackdragon While we can't control what other devs do re: multiple lockups on a single account, I'd push back strongly on our team adding multiple lockups to a single account.

If we keep the lockups we deploy (NEAR, Inc and NF) as 1-1 to the "controlling" accounts, this interface makes clear both the association of the "controlling" account and the lockup account, while allowing us to display a more discrete breakdown of the lockup account's balances (hidden entirely in the current interface).

@kcole16 I agree that we should keep mapping of lockups 1-1, but I agree with @ilblackdragon that we need to take into accounts that there’s going to be more and more various smart contract balances for given account.

User will have tokens deposited into lending, provide liquidity for AMMs, etc. So it makes sense to design for multiple smart contract future.

kcole16 commented 3 years ago

I don't see this as mutually exclusive.

Ideally we can track balances across the many smart contracts they will reside in, but those are not separate accounts (at least not 1-1) in the way that lockups are, and the interfaces for those applications will exist externally to the wallet (in most cases).

Certainly, we should approach the new portfolio management focused UI expecting to include these additional balances, which I know @corwinharrell is already accounting for.

ilblackdragon commented 3 years ago

Well, we could have told users from the start that lockup balance is actually a "separate app". It could even have it's own UI, where you would control your lockup contract after login in with your wallet. It would show you the state of it, show that you need to ping it to update the staking state, etc.

I don't know if it's too late for this or not (might be a tooltip or a banner for people who have lockups "where my lockup balance go"), but given how much confusion there is around trying to make it as "the same account", it seems reasonable in wallet UI to show that lockup as a separate concept from your accounts balance.

On Tue, Jan 5, 2021 at 11:33 AM Kendall Cole notifications@github.com wrote:

I don't see this as mutually exclusive.

Ideally we can track balances across the many smart contracts they will reside in, but those are not separate accounts (at least not 1-1) in the way that lockups are, and the interfaces for those applications will exist externally to the wallet (in most cases).

Certainly, we should approach the new portfolio management focused UI expecting to include these additional balances, which I know @corwinharrell https://github.com/corwinharrell is already accounting for.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/near/near-wallet/issues/1189#issuecomment-754851635, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABK27WF42T3TOXR2IMZWNDSYNSSJANCNFSM4T4E2KNQ .

-- Best regards, Illia Polosukhin

heycorwin commented 3 years ago

it seems reasonable in wallet UI to show that lockup as a separate concept from your accounts balance.

The problem here is that even if we were to treat the lockup separately with its own UI, people still log into the wallet expecting to see the tokens in their lockup as a part of their Total Balance. They see their total balance as "Funds under my ownership", and expect to see their lockup tokens reflected in this figure. In addition, we are allowing users to stake from their lockups, so we had to enable a way for users to switch between their wallet and lockup accounts from /staking to allow for this. Even if lockups had their own UI, we'd still have to accomodate them in the staking part of the app to allow users to stake from both accounts.

Well, we could have told users from the start that lockup balance is actually a "separate app".

@ilblackdragon I agree that we could have communicated better around lockups around the time of the sale and as tokens were being distributed. I echoed this sentiment in a slack message posted before the break:

it’s very clear after recent conversations that outside of just displaying balances, there’s a whole lot of opportunity to communicate better to our users about lockups in general. Not only displaying their “balance sheets”, but doing a better job of “onboarding” users to the concept and behavior inherent to lockups who haven’t had to manage one before, and making the experience of interacting with them less painful in general.

In hindsight, I wish we could have prioritized this around the sale, but I understand that we were moving rapidly to implement more critical functionality on a limited timeline (no one is to blame).

I do think there is an opportunity to take a step back and consider how to make it easier to interact with lockups while avoiding the scenario we are currently in where lockup balances are buried too far beneath the primary account. This is definitely making things difficult for people, and I think we can do a better job of showing how a user's portfolio is the aggregate of their wallet + lockup. I've begun playing with some simple concepts about how this can be conveyed in the wallet:

Portfolio Balance

heycorwin commented 3 years ago

Figma file for the updates to the balance table on /profile: https://www.figma.com/file/Ow6fxJZpmCnpL2qnHceCJq/Balance-Update?node-id=0%3A1

marcinbodnar commented 3 years ago

First question, do we want to round values to no decimal, like on the design or differently? I would suggest using at least 3 decimals because most of the users will not care about 0,001 imprecision in Near amount.

Without decimals we can have situation: In staking pool: 2 (real value: 2,8888) - staked - 1 (real value: 1,4444) - unstaked - 1 (real value: 1,4444)

Second question, how do we want to approach value rounding. Edge cases:

  1. Sum rounded values: In staking pool: 2,889 (real value: 2,8888) - staked - 1,444 (real value: 1,4444) - unstaked - 1,444 (real value: 1,4444)

  2. Sum real values, and than round: In staking pool: 2,888 (real value: 2,8888) - staked - 1,444 (real value: 1,4444) - unstaked - 1,444 (real value: 1,4444)

  3. Or we can use one decimal less for sum: In staking pool: 2,889 (real value: 2,8888) - staked - 1,4444 (real value: 1,4444) - unstaked - 1,4444 (real value: 1,4444)

kcole16 commented 3 years ago

@marcinbodnar Don't we already use 5 decimal places of precision for all NEAR token balances? We should continue with this standard, as users care quite a bit about precision. I believe @corwinharrell just used dummy data in the designs for simplicity.

ilblackdragon commented 3 years ago

@corwinharrell

The problem here is that even if we were to treat the lockup separately with its own UI, people still log into the wallet expecting to see the tokens in their lockup as a part of their Total Balance.

I guess if we are talking more specifically about "Total Balance" - will it in the future include funds from various lending pools, liquidity pools, tokens, etc?

I think we should definitely go away from expectation to see all funds available when sending funds and at other moments like this. Ideally the top number should be consistent with other wallets and when you log in with apps. We can still have a "Available Portfolio" or something, that shows all the known tokens / contracts (because there always be some unknown places where ppl have funds as well).

Having extra funds in a lockup in a way similar to when you have funds in staking pools, funds in liquidity pools, funds in lending protocols, etc. So asking user to explicitly withdraw from lockup seems reasonable, as we have similar expectation to withdraw from staking or from other apps. It is also how lockup contract has been built, and a reason why it's really hard to make it work in a "transparent" way between account's balance and lockup's balance.

To communicate transition we can add a banner on top that explains where funds go and how to withdraw them. Also temporarily add an explanation on pages like send funds where right now the available balance from both account and lockup is shown (saying for example "Your XYZ $N lockup funds are not included above. Lockup is a separate app, click here to withdraw funds from your lockup to your account first.").

It seems like on the screenshot above you already going in that direction with "You have 5N to withdraw".

What do you think about this?

heycorwin commented 3 years ago

@ilblackdragon yes I think our mental models are pretty in sync. What you've described is largely how I'm thinking of the relationship between balances, with maybe one exception:

Having extra funds in a lockup in a way similar to when you have funds in staking pools, funds in liquidity pools, funds in lending protocols, etc.

This is true with a slight twist I think. Since users can stake from their lockup, the staking funds are actually one level lower in the hierarchy. In other words, you can either stake from your main account balance, or from your lockup balance. So I think of the hierarchy of balances as the following:

Total Balance --> Wallet Balance || Lockup Balance --> Staking pools | Liquidity Pools | Lending Protocols

This means that funds deposited in various pools belong to either your wallet or your lockup. The trickiest part is the transfer of unlocked funds from your lockup to your wallet. I think we can definitely work to make it more clear that the funds in your lockup are restricted to certain actions, and to allow seamless transition of funds from your lockup to your wallet, when this option is available. We can also work to "notify" the user when locked funds become unlocked, and nudge them to take action (ie transfer to wallet).

I guess if we are talking more specifically about "Total Balance" - will it in the future include funds from various lending pools, liquidity pools, tokens, etc?

Exactly. I think of Total Balance as being a complete aggregate balance of funds in your possession ie top level. Available balance, as you mentioned above, is contextual. For actions like sending / staking / etc... the user would see an Available Balance, which would include only the funds available to them to perform a certain action in this context. We can also provide some of this detail for them in this context, so they understand why certain funds are not available to them when performing certain actions.

Does all of this sound in line with your thinking? cc @kcole16 @vgrichina @jimmy3dita

frol commented 3 years ago

Let me put my 2 cents here. I think we should get away from "total balance" concept as it is quite hard to define. Using traditional mobile banking I am ok with having multiple balances on different cards with various credit limits and currencies as well as separate "lockup balances" (deposits).

heycorwin commented 3 years ago

@frol It is typical though for mobile banking or any other finance management products to show your "Portfolio Balance" Or "Total balance" as a sum of the balances of all of your accounts. For example, if you have 2 checking and 1 savings account all with the same bank, when you log into your mobile banking app, you will see your total balance, as well as the individual balances of each account. Similarly, if you import several accounts, cards, or loans into a financial mgmt tool like say Mint or something similar, you will always be able to see your total balance as a representation of your cumulative balances and debts.

Now, this is not to say that this number needs to appear everywhere (See comments about contextual balances above, which are more useful when performing specific actions), but it's still useful for users to see their total balance as a representation of various lockups / pools / etc.