Metamask-extension: token decimal places not correct (testrpc)

[carchrae]

i have been working though this tutorial: http://truffleframework.com/tutorials/robust-smart-contracts-with-openzeppelin

and thought i would check the token balance in meta-mask. it isn't correct. it should show 11877 - as it does on the webpage, but instead the value is off by two decimal places. (decimals=2 in this token contract).

in both examples below metamask correctly pre-populated the decimal fields in both cases, but does not interpret them correctly.

what it looks like with decimal places set to 2, actual 118.77, expected 11877.00

to add to the fun, i decided to re-run the contract with decimals set to 18. now i see 0.12 (actual) instead of 12000.00 (expected)

this is seen using chrome (Version 60.0.3112.90 (Official Build) (64-bit)) on ubuntu 16.04. it is connecting to testrpc (ethererumjs-testrpc). to reproduce you can simply follow the tutorial linked above.

[carchrae]

i will repeat this test on a geth node on a private chain and post back.

[danfinlay]

I'll try to write a test for this and figure out what's wrong tomorrow, just landed from London, my time is way off.

We have an automated suite for our token logic here: https://github.com/flyswatter/eth-token-tracker

[carchrae]

it should be sufficient to simply deploy the contract to reproduce. in the first case (11877) i had sent some tokens (123).

[carchrae]

contract code

pragma solidity ^0.4.4;
import 'zeppelin-solidity/contracts/token/StandardToken.sol';

contract TutorialToken is StandardToken {
    string public name = 'TutorialToken';
    string public symbol = 'TT';
    uint public decimals = 2; 
    uint public INITIAL_SUPPLY = 12000;

    function TutorialToken() {
        totalSupply = INITIAL_SUPPLY;
        balances[msg.sender] = INITIAL_SUPPLY;
    }
}

[danfinlay]

I recreated the zeppelin TutorialToken in a new automated test for the eth-token-tracker.

It seems to be rendering precisions correctly, but I did notice behavior that surprised me at first, and almost made me think the same thing that prompted this issue:

Token decimals are not added to a token initial supply, they are subtracted from it.

So if initial supply is 12000 and you have decimals of 2, your correct rendered balance should be 120 tokens. If there is disparity between your token tutorial page and the MetaMask window on this point, it's probably the tutorial that is wrong.

Let me know if this accurately describes your issue.

[carchrae]

@flyswatter - i think you are absolutely correct regarding decimal places 2, and the error in that case is in the tutorial. so, i agree, the bug is in the tutorial. (i'm finding a lot of token documentation that is sloppy on the use of decimal and not clear about what a unit is)

however, what about the second example when i changed decimal places to 18? shouldn't metamask show 0.000000000000012 (12000/10^18) instead of 0.120

[danfinlay]

There we go, now that's a bug.

[carchrae]

phew, i hope so! i felt bad about the first one. :man_facepalming:

[danfinlay]

Thanks for catching this, I'd had a lingering feeling something was a little off, but didn't have a solid test case like this one.

[danfinlay]

I fixed the bug, will be patched & published in MetaMask soon:
https://github.com/MetaMask/metamask-extension/pull/1941

[carchrae]

thanks for the speedy response. i checked out the fix. i appreciate that you have limited space in the UI to display numbers, but i think there could be some confusion with '0.000' representing a small number vs it actually being zero.

i wrote a simpler format function and compared it to the current one. while 1e-7 isn't very user friendly it is more accurate than 0.000

const BN = require('ethjs').BN
const zero = new BN(0)


function stringifyBalance(balance, bnDecimals) {
    if (balance.eq(zero)) {
        return '0'
    }

    const decimals = parseInt(bnDecimals.toString())
    if (decimals === 0) {
        return balance.toString()
    }

    let bal = balance.toString()
    let len = bal.length
    let decimalIndex = len - decimals
    let prefix = ''

    if (decimalIndex < 0) {
        while (prefix.length <= decimalIndex * -1) {
            prefix += '0'
            len++
        }
        bal = prefix + bal
        decimalIndex = 1
    }

    const result = `${bal.substr(0, len - decimals)}.${bal.substr(decimalIndex, 3)}`
    return result;
}

function stringifyBalance2(balance, decimals) {
    balance = parseInt(balance.toString());
    decimals = parseInt(decimals.toString());
    return Number(balance / Math.pow(10, decimals)).toString();
}

function print(balance, decimals) {
    console.log('balance= ' + balance + ' decimals=' + decimals);
    console.log('v1: ' + stringifyBalance(new BN(balance), decimals));
    console.log('v2: ' + stringifyBalance2(new BN(balance), decimals));
}

print(10,2);
print(10,4);
print(10,6);
print(10,8);
print(10,0);
print(0,10);
print(0,0);

output

balance= 10 decimals=2
v1: .10
v2: 0.1
balance= 10 decimals=4
v1: 0.001
v2: 0.001
balance= 10 decimals=6
v1: 0.000
v2: 0.00001
balance= 10 decimals=8
v1: 0.000
v2: 1e-7
balance= 10 decimals=0
v1: 10
v2: 10
balance= 0 decimals=10
v1: 0
v2: 0
balance= 0 decimals=0
v1: 0
v2: 0

[carchrae]

this would avoid the 1e-7

function stringifyBalance2(balance, decimals) {
    balance = parseInt(balance.toString());
    decimals = parseInt(decimals.toString());
    return Number(balance / Math.pow(10, decimals)).toFixed(decimals).toString();
}

output

balance= 10 decimals=2
v1: .10
v2: 0.10
balance= 10 decimals=4
v1: 0.001
v2: 0.0010
balance= 10 decimals=6
v1: 0.000
v2: 0.000010
balance= 10 decimals=8
v1: 0.000
v2: 0.00000010
balance= 10 decimals=0
v1: 10
v2: 10
balance= 0 decimals=10
v1: 0
v2: 0.0000000000
balance= 0 decimals=0
v1: 0
v2: 0
balance= 100000000000010 decimals=18
v1: 0.000
v2: 0.000100000000000010

[danfinlay]

Thanks for providing some sample code for doing it full precision. We should maybe just do that, but I want to pass it by our designer first, to make sure we have UI room for full balances.

@MetaMask/design What do you think? Should we make room for balances, even if they have 18 decimal points after them, never rounding?

[carchrae]

i can definitely see an argument for not including trailing (right) zeros, especially with 18 decimal places.

[carchrae]

there is an issue with the code i posted :/ ... JS cannot handle the range of integers that BN.js supports, so i wouldn't suggest it. a correct fix would use a bignum library that supports decimals like https://github.com/MikeMcl/bignumber.js/ (edit: or better big.js or better, https://github.com/MikeMcl/big.js/wiki )

[danfinlay]

Yeah, we've been around the bend with bignumber.js, basically it bites us every time we try to use it for ethereum-sized numbers. It's from experience that we've tried to roll our own crappy decimal management with BN.

[carchrae]

well, at least now i appreciate why you're doing that confusing while loop. https://github.com/MetaMask/eth-token-tracker/blob/6135595aeda70057e9e7c70826c25174c9197ab7/lib/util.js#L21

[carchrae]

this one works nicely (v3)

function stringifyBalance3(balance, decimals) {
    var leftValue, rightValue;
    balance = balance.toString();
    decimals = parseInt(decimals.toString());

    //how many numbers are to the left of the decimal point
    let leftLength = balance.length-decimals;

    //what numbers are to the right - note special case for zero decimals
    rightValue = decimals>0 ? balance.slice(-decimals) : '';

    //pad right side with zeros if needed
    rightValue = new Array(Math.max(0,1+(decimals-balance.length))).join('0') + rightValue;

    //remove trailing zeros
    rightValue = rightValue.replace(/0+$/,'');

    if (leftLength>0) {
        //use the left side value 
        leftValue = balance.slice(0, leftLength);
    }else{
        //no left size value, so use zero
        leftValue = '0';
    }
    return leftValue + (rightValue?'.' + rightValue :'');
}

output

bal=120 decimals=2
v1: 1.20
v2: 1.20
v3: 1.2
bal=120 decimals=4
v1: 0.012
v2: 0.0120
v3: 0.012
bal=120 decimals=6
v1: 0.000
v2: 0.000120
v3: 0.00012
bal=120 decimals=8
v1: 0.000
v2: 0.00000120
v3: 0.0000012
bal=120 decimals=0
v1: 120
v2: 120
v3: 120
bal=0 decimals=10
v1: 0
v2: 0.0000000000
v3: 0
bal=0 decimals=0
v1: 0
v2: 0
v3: 0
bal=100000000000000010 decimals=18
v1: .100
v2: 0.100000000000000019
v3: 0.10000000000000001
bal=123456789 decimals=10
v1: 0.012
v2: 0.0123456789
v3: 0.0123456789

i can make a PR if you want

[ratacat]

@danfinlay, Is there a way to change the decimals places entered on Metamask after you add a token? I entered it with 18 decimals, and I wanted to do 0, I just didn't understand what I was doing. So now my number is all super messed up, and I haven't figured out how to change it?

[danfinlay]

@ratacat This is an unrelated issue. Pretty sure we have an issue open for that, haven’t gotten to it. Workaround is reinstall.