[NayNay] Updating Units within Entropy #306
Conversation
- it had come to our attn that we were using the wrong units for the tokens display - we were under the impression that bits were the smallest unit of the entropy token, that was a mistake - updated all mentions of tokens and bits to match their respective definition - updated return of balance to the human readable format, using BITS - added new method to grab token details from chain
src/balance/command.ts
Outdated
| cliWrite(`${balance.toLocaleString('en-US')} BITS`) | ||
| const tokensBalance = await balanceService.getBalance(address) | ||
| const balance = tokensToBits(tokensBalance, decimals) | ||
| cliWrite(`${balance} ${symbol}`) |
There was a problem hiding this comment.
🔥 I want to see output that i can easily consume in another command.
This output would force people to parse the string (see #221 (comment))
You can make it another issue/ PR which fixes that, but this is something I think it really important
There was a problem hiding this comment.
the output is the same as it was before the change, this change is mainly to display the human readable amount as well as pull the symbol from the chain. not sure what your 🔥 is referring too.
There was a problem hiding this comment.
if BITS needs to be removed, that's fine, but wouldn't that cause confusion if users don't know what the number represents?
There was a problem hiding this comment.
Yeah the 🔥 was because "1.234 BITS" is kinda a pain to use in other scripts. e.g. if I write a script which moves all my balance to another account I cannot take the results of balance and put them into transfer ... because balance is not giving numbers, it's giving numbers and letters and a space. So it forces me to write some awk or something.
I think if you want the unit, you should have --show-units in the command? CLI commands are meant to be easy to chain IMO.
There was a problem hiding this comment.
If it feels like a different issue I'm raising (and it might well be), please open a new issue, put it in a milestone and mark this resolved.
src/faucet/interaction.ts
Outdated
| // there are limits in place to ensure user's are leftover with a certain balance in their accounts | ||
| // increasing amount send here, will allow user's to register right away | ||
|
|
||
| // 2 BITS |
There was a problem hiding this comment.
you should change this to use the definition of BITS
There was a problem hiding this comment.
what are you referring too? the 2 BITS comment?
There was a problem hiding this comment.
Probably the comment and amount below it
There was a problem hiding this comment.
Unable to change the value sent to transfer as it requires we send the bigint nanoBITS otherwise it doesn't send the value you expect.
There was a problem hiding this comment.
I think what I shoulda said is "you have hard coded how many nanoBits you are sending. This is fine, but if you really want to guarentee it's 2 bits you should getTokenDetails and use the results of that in how much you send".
What I'm wrinkling my nose at is that we've shifted to use a chain-definition of how many nanoBits in a BITS... except here? It's fine, let's just resolve this, it's just the faucet
|
@mixmix here is the issue for tracking the readme/docs update entropyxyz/entropy-docs#261 |
|
Related docs issue: entropyxyz/entropy-docs#265 |
…pdated amount for faucet to send two whole BITS, rather than a hardcoded string
Related Issue(s)
Proposed Changes
Testing
Screenshots (if applicable)
Additional Context
Checklist
CHANGELOG.mdentry.github.com:entropyxyz/entropy-docs, where necessary.