Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor bugs in default token contract #11

Open
tkoen93 opened this issue May 28, 2019 · 4 comments
Open

Minor bugs in default token contract #11

tkoen93 opened this issue May 28, 2019 · 4 comments
Assignees
Labels
discuss To talk about a subject in detail, especially considering different ideas and opinions related to it

Comments

@tkoen93
Copy link

tkoen93 commented May 28, 2019

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.
@kondrashovsv kondrashovsv assigned ghost , 0xAAE and IGoryunov May 29, 2019
@ksv81 ksv81 added the discuss To talk about a subject in detail, especially considering different ideas and opinions related to it label May 29, 2019
@rustem-s
Copy link

Default token contract changed.

@tkoen93
Copy link
Author

tkoen93 commented Jun 18, 2019

@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.

@ghost
Copy link

ghost commented Jun 19, 2019

please clear your browser cache and try again

@tkoen93
Copy link
Author

tkoen93 commented Jun 19, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss To talk about a subject in detail, especially considering different ideas and opinions related to it
Projects
None yet
Development

No branches or pull requests

5 participants