Merge pull request #466 from phiferd/master

Using require for Token preconditions

contracts/token/BasicToken.sol

@@ -21,6 +21,7 @@ contract BasicToken is ERC20Basic {
   */
   function transfer(address _to, uint256 _value) public returns (bool) {
     require(_to != address(0));
+    require(_value <= balances[msg.sender]);
 
     // SafeMath.sub will throw if there is not enough balance.
     balances[msg.sender] = balances[msg.sender].sub(_value);

contracts/token/BurnableToken.sol

@@ -16,6 +16,9 @@ contract BurnableToken is StandardToken {
      */
     function burn(uint256 _value) public {
         require(_value > 0);
+        require(_value <= balances[msg.sender]);
+        // no need to require value <= totalSupply, since that would imply the
+        // sender's balance is greater than the totalSupply, which *should* be an assertion failure
 
         address burner = msg.sender;
         balances[burner] = balances[burner].sub(_value);

contracts/token/StandardToken.sol

@@ -25,15 +25,12 @@ contract StandardToken is ERC20, BasicToken {
    */
   function transferFrom(address _from, address _to, uint256 _value) public returns (bool) {
     require(_to != address(0));
-
-    uint256 _allowance = allowed[_from][msg.sender];
-
-    // Check is not needed because sub(_allowance, _value) will already throw if this condition is not met
-    // require (_value <= _allowance);
+    require(_value <= balances[_from]);
+    require(_value <= allowed[_from][msg.sender]);
 
     balances[_from] = balances[_from].sub(_value);
     balances[_to] = balances[_to].add(_value);
-    allowed[_from][msg.sender] = _allowance.sub(_value);
+    allowed[_from][msg.sender] = allowed[_from][msg.sender].sub(_value);
     Transfer(_from, _to, _value);
     return true;
   }

test/StandardToken.js

 'use strict';
 
 const assertJump = require('./helpers/assertJump');
+const expectThrow = require('./helpers/expectThrow');
 var StandardTokenMock = artifacts.require('./helpers/StandardTokenMock.sol');
 
 contract('StandardToken', function(accounts) {
@@ -70,6 +71,17 @@ contract('StandardToken', function(accounts) {
     }
   });
 
+  it('should throw an error when trying to transferFrom more than _from has', async function() {
+    let balance0 = await token.balanceOf(accounts[0]);
+    await token.approve(accounts[1], 99);
+    try {
+      await token.transferFrom(accounts[0], accounts[2], balance0+1, {from: accounts[1]});
+      assert.fail('should have thrown before');
+    } catch (error) {
+      assertJump(error);
+    }
+  });
+
   describe('validating allowance updates to spender', function() {
     let preApproved;
 
@@ -88,6 +100,13 @@ contract('StandardToken', function(accounts) {
     })
   });
 
+  it('should increase by 50 then set to 0 when decreasing by more than 50', async function() {
+    await token.approve(accounts[1], 50);
+    await token.decreaseApproval(accounts[1], 60);
+    let postDecrease = await token.allowance(accounts[0], accounts[1]);
+    postDecrease.should.be.bignumber.equal(0);
+});
+
   it('should throw an error when trying to transfer to 0x0', async function() {
     let token = await StandardTokenMock.new(accounts[0], 100);
     try {