decentdao / decent-interface

Govern at startup speed
https://app.decentdao.org
MIT License
14 stars 7 forks source link

Token amounts in the CreateADAO token details step are not parsed to account for its decimals. #472

Closed christopherdancy closed 2 years ago

christopherdancy commented 2 years ago

Explanation A user which is attempting to deploy a 1:1 token DAO - would most likely want to assign/allocate a certain amount of tokens to holders. If user x is assigned 10 tokens, their token balance should be 10 tokens. The user should not have to input 10 XX 18 in order to send their holders the 10 tokens.

My suggestion for to fix this unexpected behaviour is to parse the total token supply, parent allocation amount, and the tokenallocaitons so that users can work with simple numbers instead of dealing with decimals.

Steps to reproduce

  1. Create a dao
  2. Allocate funds to an address that you have access to - type 10 within the allocations
  3. Go to delegate and see that your current balance is .000000000010 and not 10.0.

Screenshot 2022-08-31 at 17 29 07

adamgall commented 2 years ago

@christopherdancy How is the formatting incorrect here?

Does the user in this example own 100 units of the fer token, which has 18 decimal places? If so, then the formatting looks correct to me.

Da-Colon commented 2 years ago

@adamgall So played around a little bit here. So context I guess is that before our recent change, to using BigNumbers here, this was considered a whole number only.

100 === 100

With The BigNumber updates its now:

100 === 0.0000000000000001

So in the case of the Token Supply The question I think we have here is do we want users to simply set 100 to mint 100 tokens or require users to enter the units in 'wei' (in which case we might want to update the helper text)

adamgall commented 2 years ago

do we want users to simply set 100 to mint 100 tokens

If a user enters "10", and the referenced token has 18 decimals, then it should result in a value of "10000000000000000000" (10e18) being sent to the smart contract.

Similarly, a user should be able to enter "10.123" which results in a value of "101230000000000000000" (1.0123456e16) being sent to the smart contract.

Da-Colon commented 2 years ago

okay cool, a couple updates are going to be made to allow for this to happen. I did not think of some considerations when checking the BigNumber update. right now 18 decimals are not being considered. I'll look at making this update.

adamgall commented 2 years ago

Ha, we did recognize that something was happening here around this functionality.

https://github.com/decent-dao/fractal-interface/pull/443#pullrequestreview-1084411481

But our problem was that we were thinking about a new feature (supporting decimal values), while missing the bug we introduced (this very issue).

So actually, let's approach this in two PRs:

  1. First, a quick one: Make it so that user input amounts are multiplied by 10**token.decimals(). This fixes the bug.
  2. Next, a more complicated UI improvement: Support a user being able to input "decimal" numbers into the inputs, such as 10.123. Please refer to code I've already built to handle this kind of user input, with lots of edge cases considered: https://github.com/decent-dao/fractal-interface/pull/443#discussion_r954252435
Da-Colon commented 2 years ago

So for the first one. I think I almost have a PR finished.

For the second one. While it's nice to have this converted on change and saved to state, it makes trying to have a clean UX very difficult. as is leaves a '0.0' that really makes typing harder than it needs to be.

I think we can approach this two ways. (This is with some playing around with adding ability for decimals.) Both solution do involve updating the types again.

type TokenAllocation = {
  address: string;
  amount: {
    value: string,
    valueBN: BigNumber,
  }
}
adamgall commented 2 years ago

For the second one. While it's nice to have this converted on change and saved to state, it makes trying to have a clean UX very difficult. as is leaves a '0.0' that really makes typing harder than it needs to be.

How difficult?

I put a pretty clean UX together that satisfies lots of edge cases here: https://vesting.decentlabs.io/#/vesting-schedules/new

Check it out and let me know if this isn't possible in Fractal for whatever reason.

The code i linked to in my previous comment (https://github.com/decent-dao/fractal-interface/pull/443#discussion_r954252435) is what makes that input possible.