node icon indicating copy to clipboard operation
node copied to clipboard

Minor bugs in default token contract

Open tkoen93 opened this issue 5 years ago • 4 comments

Describe the bug The default token contract that is being used with the web wallet contains some minor bugs. Should be pretty easy to fix, but the default token contract needs some revision.

Thanks to Steffen in tech chat for pointing this issue out as well.

What currently needs to be done is the correct use of 'totalCoins' (TotalSupply). Upon deploying a token contract, the entire supply will be added to the public key of the contract owner. That means that the contract address does not contain any of that just created token.

When someone tries to burn some tokens, the burn function is called and defined as follows in the default token contract:

public boolean burn(String amount) {
        contractIsNotFrozen();
        BigDecimal decimalAmount = toBigDecimal(amount);
        if (!initiator.equals(owner))
            throw new RuntimeException("can not burn tokens! The wallet " + initiator + " is not owner");
        if (totalCoins.compareTo(decimalAmount) < 0) totalCoins = ZERO.setScale(decimal, ROUND_DOWN);
        else totalCoins = totalCoins.subtract(decimalAmount);
        return true;
    }

This function allows only the owner of the contract to use this function. Next to that, when 'burning' tokens, only the 'totalCoins' is being reduced. 'totalCoins' is equal to the total supply on the monitor. There are no tokens actually being burned, as nothing gets subtracted from an account.

With the current burn function it's actually possible to enter an amount below zero. This way you can even increase the totalCoins / total supply.

I suggest the following as a solution for this function.

public boolean burn(String amount) {
        contractIsNotFrozen();
        BigDecimal decimalAmount = toBigDecimal(amount);
        BigDecimal sourceBalance = getTokensBalance(initiator);
        if(decimalAmount.compareTo(ZERO) < 0) {
            throw new IllegalArgumentException("the amount cannot be negative");
        }
        if (sourceBalance.compareTo(decimalAmount) < 0) {
            throw new RuntimeException("the wallet " + initiator + " doesn't have enough tokens to burn");
        }
            totalCoins = totalCoins.subtract(decimalAmount);
            balances.put(initiator, sourceBalance.subtract(decimalAmount));
        return true;
    }

In my opinion anyone should be able to burn a token they possess. It's the users own responsibility when they burn tokens.

The code above checks if the entered amount is below zero. If so, it throws an error. After that, the entered amount is being checked against the actual balance of the token. When someone tries to burn more tokens than they possess, it throw another error. That causes these transactions to fail.

When it passes both checks, the 'totalCoins' is reduced. Next to that, the burned tokens get subtracted from the account that executed this transaction, causing them to actually being 'burned'.

totalCoins is currently also being used in the public void payable() function. Might need to be checked as well.

To Reproduce

  • Deploy a token contract through web wallet
  • Try to burn tokens through the burn function
  • Verify that total supply changes, but the amount of tokens in your wallet does not change.

tkoen93 avatar May 28 '19 11:05 tkoen93

Default token contract changed.

rustem-s avatar Jun 18 '19 18:06 rustem-s

@rustem-s Hi, thanks for reply.

Tried to deploy the contract via the web wallet, some empty alert. Copied the contract into the wallet-desktop.jar and getting the following error: image Looks like there is no checkNegative function in the contract, so it doesn't work at the moment.

tkoen93 avatar Jun 18 '19 19:06 tkoen93

please clear your browser cache and try again

ghost avatar Jun 19 '19 09:06 ghost

Able to deploy it now because there is a checkNegative function.

Still other issues. totalCoins is set to 0 as default. freeCoins has been added and set to 10 million as default? tokenCost doesn't do anything in the payable function.

I understand that this is not really an issue, as the deployer is responsible for the contract they deploy. But as an offered default contract I wouldn't recommend using this default contract at it's current state.

tkoen93 avatar Jun 19 '19 09:06 tkoen93