(SafeMath: subtraction overflow) - while trying to burn tokens

Support Contracts
1

I want to create exchange contract by editing your Crowdsale.sol and adding to it sellToken().
Inside sellToken() I want burn amount of tokens which msg.sender want to sell.
When I test it with truffle the subtraction overflow error appear.

How I can make it work properly?

Bellow short versions of contract and test.

Exchange.sol

contract Exchange {
	using SafeMath for uint256;

	Token private _token;
	address payable private _wallet;
	uint256 private _rate;	
	uint256 private _weiRaised;

	event TokensBurned(address indexed seller, uint256 amount);

	constructor (uint256 rate, address payable wallet, Token token) public {
		_rate = rate;
		_wallet = wallet;
		_token = token;
	}

	function token() public view returns (Token) {
	    return _token;
	}

    function sellTokens(address beneficiary, uint256 tokenAmount) public {
	  Token(address(token())).burnFrom(beneficiary, tokenAmount);
	  emit TokensBurned(beneficiary, tokenAmount);
   }
}

Test.js

contract('Exchange', function ([_, deployer, investor, wallet, purchaser]) {
	const rate = new BN('10');
	const value = ether('1');
	const expectedTokenAmount = rate.mul(value);
    const sellingTokenAmount = new BN(1);

	describe('Tests for buying tokens & selling tokens:', function () {
		beforeEach(async function () {
			this.token = await Token.new();
			this.exchange = await Exchange.new(rate, wallet, this.token.address);
			await this.token.addMinter(this.exchange.address, { from: deployer });
			await this.token.renounceMinter({ from: deployer});	
		});

		describe('Testing selling tokens', function ()  {
			beforeEach(async function() {
			    await this.exchange.send(value);
		        await this.exchange.buyTokens(investor, { value: value, from: purchaser });
			});

			it('should buy tokens', async function () {
				expect(await this.token.balanceOf(investor)).to.be.bignumber.equal(expectedTokenAmount);
			});

			it('should sell token', async function () {
				await this.exchange.sellTokens(investor, sellingTokenAmount);
			});
		});
	});
});

BTW.
The same issue appears also when I run test and inside sellTokens(); there is withdraw(); instead of burnFrom();
Before run 'should sell token' test I've checked that balanceOf(investor) > sellingTokenAmount

1 Like
2

Hi @3rr0r welcome to the community :wave:

Looking at the code you provided sellTokens calls burnFrom (the exchange contract is msg.sender). The exchange contract has no allowance, so burnFrom (I assume this calls _burnFrom) fails when it attempts to decrease the callers allowance.

it('should sell token', async function () { await this.exchange.sellTokens(investor, sellingTokenAmount); });

To make this work, you need to set an allowance for the exchange contract before calling burnFrom.

Let me know if I can provide more information.

1 Like
3

Hi @abcoathup and thanks you for quick answer.

I’ve tried to set an allowance like you said, with:

Exchange.sol

  function sellTokens(address beneficiary, uint256 tokenAmount) public nonReentrant payable {
  	Token(address(token())).burnfrom(beneficiary, tokenAmount);
   }

Test.js

await this.token.approve(burner, sellingTokenAmount, {from: investor} );
await this.exchange.sellTokens(investor, sellingTokenAmount, {from: burner } );

And burn directly from holder:

Exchange.sol

  function sellTokens() public nonReentrant payable {
  	uint256 tokenAmount = msg.value;
  	Token(address(token())).burn(tokenAmount);
   }

Test.js

await this.exchange.sellTokens( {value: sellingTokenAmount, from: investor } );

but unfortunately subtraction overflow error still remains. If I skip value: sellingTokenAmount the test is passed. It seems to me that exchange contract treat balanceOf(investor) like equal to 0, but I can’t figure out why. Do you have any other ideas?

1 Like
4

Hi @3rr0r

The issue is still that the caller of burnFrom doesn’t have an allowance, what I missed when I looked at this the first time is that the caller of burnFrom is the Exchange contract. Apologies for my error, I have updated my original answer.

The token holder needs to set an allowance for the exchange contract before calling sellTokens, so the test could look as follows:

		describe('Testing selling tokens', function ()  {
			beforeEach(async function() {
			    await this.exchange.buyTokens(investor, { value: value, from: purchaser });
			});

			it('should buy tokens', async function () {
				expect(await this.token.balanceOf(investor)).to.be.bignumber.equal(expectedTokenAmount);
			});

			it('should sell token', async function () {
                await this.token.approve(this.exchange.address, sellingTokenAmount, {from: investor} );
                await this.exchange.sellTokens(investor, sellingTokenAmount );
            });
		});
5

Thank you so much, it works!

1 Like
6

Hi @3rr0r great. Sorry I didn’t spot it the first time.

2 Likes