From 4fad1505c7b095062ca080440c8090372a814d3b Mon Sep 17 00:00:00 2001 From: Manuel Araoz Date: Thu, 6 Apr 2017 16:43:47 -0300 Subject: [PATCH 1/3] Make SafeMath a library --- contracts/SafeMath.sol | 2 +- contracts/payment/PullPayment.sol | 10 ++++++---- contracts/token/BasicToken.sol | 7 ++++--- contracts/token/CrowdsaleToken.sol | 6 +++--- contracts/token/StandardToken.sol | 6 +++--- contracts/token/VestedToken.sol | 19 ++++++++++--------- test/helpers/SafeMathMock.sol | 8 ++++---- 7 files changed, 31 insertions(+), 27 deletions(-) diff --git a/contracts/SafeMath.sol b/contracts/SafeMath.sol index 869c3139a..64d1e0bc2 100644 --- a/contracts/SafeMath.sol +++ b/contracts/SafeMath.sol @@ -4,7 +4,7 @@ pragma solidity ^0.4.8; /** * Math operations with safety checks */ -contract SafeMath { +library SafeMath { function safeMul(uint a, uint b) internal returns (uint) { uint c = a * b; assert(a == 0 || c / a == b); diff --git a/contracts/payment/PullPayment.sol b/contracts/payment/PullPayment.sol index 694b68837..2284ec112 100644 --- a/contracts/payment/PullPayment.sol +++ b/contracts/payment/PullPayment.sol @@ -9,14 +9,16 @@ import '../SafeMath.sol'; * Base contract supporting async send for pull payments. * Inherit from this contract and use asyncSend instead of send. */ -contract PullPayment is SafeMath { +contract PullPayment { + using SafeMath for uint; + mapping(address => uint) public payments; uint public totalPayments; // store sent amount as credit to be pulled, called by payer function asyncSend(address dest, uint amount) internal { - payments[dest] = safeAdd(payments[dest], amount); - totalPayments = safeAdd(totalPayments, amount); + payments[dest] = payments[dest].safeAdd(amount); + totalPayments = totalPayments.safeAdd(amount); } // withdraw accumulated balance, called by payee @@ -32,7 +34,7 @@ contract PullPayment is SafeMath { throw; } - totalPayments = safeSub(totalPayments, payment); + totalPayments = totalPayments.safeSub(payment); payments[payee] = 0; if (!payee.send(payment)) { diff --git a/contracts/token/BasicToken.sol b/contracts/token/BasicToken.sol index 5e6f3278a..07b941acd 100644 --- a/contracts/token/BasicToken.sol +++ b/contracts/token/BasicToken.sol @@ -9,7 +9,8 @@ import '../SafeMath.sol'; * Basic token * Basic version of StandardToken, with no allowances */ -contract BasicToken is ERC20Basic, SafeMath { +contract BasicToken is ERC20Basic { + using SafeMath for uint; mapping(address => uint) balances; @@ -22,8 +23,8 @@ contract BasicToken is ERC20Basic, SafeMath { } function transfer(address _to, uint _value) onlyPayloadSize(2 * 32) { - balances[msg.sender] = safeSub(balances[msg.sender], _value); - balances[_to] = safeAdd(balances[_to], _value); + balances[msg.sender] = balances[msg.sender].safeSub(_value); + balances[_to] = balances[_to].safeAdd(_value); Transfer(msg.sender, _to, _value); } diff --git a/contracts/token/CrowdsaleToken.sol b/contracts/token/CrowdsaleToken.sol index 062bf9d34..040e5123d 100644 --- a/contracts/token/CrowdsaleToken.sol +++ b/contracts/token/CrowdsaleToken.sol @@ -32,10 +32,10 @@ contract CrowdsaleToken is StandardToken { throw; } - uint tokens = safeMul(msg.value, getPrice()); - totalSupply = safeAdd(totalSupply, tokens); + uint tokens = msg.value.safeMul(getPrice()); + totalSupply = totalSupply.safeAdd(tokens); - balances[recipient] = safeAdd(balances[recipient], tokens); + balances[recipient] = balances[recipient].safeAdd(tokens); if (!multisig.send(msg.value)) { throw; diff --git a/contracts/token/StandardToken.sol b/contracts/token/StandardToken.sol index 9be7d0eeb..d61de6bfb 100644 --- a/contracts/token/StandardToken.sol +++ b/contracts/token/StandardToken.sol @@ -22,9 +22,9 @@ contract StandardToken is BasicToken, ERC20 { // Check is not needed because safeSub(_allowance, _value) will already throw if this condition is not met // if (_value > _allowance) throw; - balances[_to] = safeAdd(balances[_to], _value); - balances[_from] = safeSub(balances[_from], _value); - allowed[_from][msg.sender] = safeSub(_allowance, _value); + balances[_to] = balances[_to].safeAdd(_value); + balances[_from] = balances[_from].safeSub(_value); + allowed[_from][msg.sender] = _allowance.safeSub(_value); Transfer(_from, _to, _value); } diff --git a/contracts/token/VestedToken.sol b/contracts/token/VestedToken.sol index 9c0766945..9c4f4cdda 100644 --- a/contracts/token/VestedToken.sol +++ b/contracts/token/VestedToken.sol @@ -52,8 +52,8 @@ contract VestedToken is StandardToken, LimitedTransferToken { grants[_holder][_grantId] = grants[_holder][grants[_holder].length - 1]; grants[_holder].length -= 1; - balances[msg.sender] = safeAdd(balances[msg.sender], nonVested); - balances[_holder] = safeSub(balances[_holder], nonVested); + balances[msg.sender] = balances[msg.sender].safeAdd(nonVested); + balances[_holder] = balances[_holder].safeSub(nonVested); Transfer(_holder, msg.sender, nonVested); } @@ -98,32 +98,33 @@ contract VestedToken is StandardToken, LimitedTransferToken { return tokens; } - uint256 cliffTokens = safeDiv(safeMul(tokens, safeSub(cliff, start)), safeSub(vesting, start)); + uint256 cliffTokens = tokens.safeMul(cliff.safeSub(start)).safeDiv(vesting.safeSub(start)); vestedTokens = cliffTokens; - uint256 vestingTokens = safeSub(tokens, cliffTokens); + uint256 vestingTokens = tokens.safeSub(cliffTokens); - vestedTokens = safeAdd(vestedTokens, safeDiv(safeMul(vestingTokens, safeSub(time, cliff)), safeSub(vesting, cliff))); + vestedTokens = vestedTokens.safeAdd(vestingTokens.safeMul(time.safeSub(cliff)).safeDiv(vesting.safeSub(cliff))); } function nonVestedTokens(TokenGrant grant, uint64 time) private constant returns (uint256) { - return safeSub(grant.value, vestedTokens(grant, time)); + return grant.value.safeSub(vestedTokens(grant, time)); } function lastTokenIsTransferableDate(address holder) constant public returns (uint64 date) { date = uint64(now); uint256 grantIndex = grants[holder].length; for (uint256 i = 0; i < grantIndex; i++) { - date = max64(grants[holder][i].vesting, date); + date = SafeMath.max64(grants[holder][i].vesting, date); } } function transferableTokens(address holder, uint64 time) constant public returns (uint256 nonVested) { uint256 grantIndex = grants[holder].length; for (uint256 i = 0; i < grantIndex; i++) { - nonVested = safeAdd(nonVested, nonVestedTokens(grants[holder][i], time)); + uint256 current = nonVestedTokens(grants[holder][i], time); + nonVested = nonVested.safeAdd(current); } - return min256(safeSub(balances[holder], nonVested), super.transferableTokens(holder, time)); + return SafeMath.min256(balances[holder].safeSub(nonVested), super.transferableTokens(holder, time)); } } diff --git a/test/helpers/SafeMathMock.sol b/test/helpers/SafeMathMock.sol index e31aefbef..c0be1d70f 100644 --- a/test/helpers/SafeMathMock.sol +++ b/test/helpers/SafeMathMock.sol @@ -4,18 +4,18 @@ pragma solidity ^0.4.8; import '../../contracts/SafeMath.sol'; -contract SafeMathMock is SafeMath { +contract SafeMathMock { uint public result; function multiply(uint a, uint b) { - result = safeMul(a, b); + result = SafeMath.safeMul(a, b); } function subtract(uint a, uint b) { - result = safeSub(a, b); + result = SafeMath.safeSub(a, b); } function add(uint a, uint b) { - result = safeAdd(a, b); + result = SafeMath.safeAdd(a, b); } } From 609869f08732fce8cd946996d326c15889e441c8 Mon Sep 17 00:00:00 2001 From: Manuel Araoz Date: Wed, 19 Apr 2017 16:19:22 -0300 Subject: [PATCH 2/3] change safe* to * --- contracts/SafeMath.sol | 8 ++++---- contracts/payment/PullPayment.sol | 6 +++--- contracts/token/BasicToken.sol | 8 +++++--- contracts/token/CrowdsaleToken.sol | 6 +++--- contracts/token/MintableToken.sol | 4 ++-- contracts/token/StandardToken.sol | 8 ++++---- contracts/token/VestedToken.sol | 16 ++++++++-------- test/helpers/SafeMathMock.sol | 6 +++--- 8 files changed, 32 insertions(+), 30 deletions(-) diff --git a/contracts/SafeMath.sol b/contracts/SafeMath.sol index 64d1e0bc2..0d7c160e5 100644 --- a/contracts/SafeMath.sol +++ b/contracts/SafeMath.sol @@ -5,25 +5,25 @@ pragma solidity ^0.4.8; * Math operations with safety checks */ library SafeMath { - function safeMul(uint a, uint b) internal returns (uint) { + function mul(uint a, uint b) internal returns (uint) { uint c = a * b; assert(a == 0 || c / a == b); return c; } - function safeDiv(uint a, uint b) internal returns (uint) { + function div(uint a, uint b) internal returns (uint) { assert(b > 0); uint c = a / b; assert(a == b * c + a % b); return c; } - function safeSub(uint a, uint b) internal returns (uint) { + function sub(uint a, uint b) internal returns (uint) { assert(b <= a); return a - b; } - function safeAdd(uint a, uint b) internal returns (uint) { + function add(uint a, uint b) internal returns (uint) { uint c = a + b; assert(c >= a); return c; diff --git a/contracts/payment/PullPayment.sol b/contracts/payment/PullPayment.sol index 2284ec112..69e30c1d8 100644 --- a/contracts/payment/PullPayment.sol +++ b/contracts/payment/PullPayment.sol @@ -17,8 +17,8 @@ contract PullPayment { // store sent amount as credit to be pulled, called by payer function asyncSend(address dest, uint amount) internal { - payments[dest] = payments[dest].safeAdd(amount); - totalPayments = totalPayments.safeAdd(amount); + payments[dest] = payments[dest].add(amount); + totalPayments = totalPayments.add(amount); } // withdraw accumulated balance, called by payee @@ -34,7 +34,7 @@ contract PullPayment { throw; } - totalPayments = totalPayments.safeSub(payment); + totalPayments = totalPayments.sub(payment); payments[payee] = 0; if (!payee.send(payment)) { diff --git a/contracts/token/BasicToken.sol b/contracts/token/BasicToken.sol index 07b941acd..58b519f6f 100644 --- a/contracts/token/BasicToken.sol +++ b/contracts/token/BasicToken.sol @@ -18,13 +18,15 @@ contract BasicToken is ERC20Basic { * Fix for the ERC20 short address attack */ modifier onlyPayloadSize(uint size) { - assert(msg.data.length >= size + 4); + if(msg.data.length < size + 4) { + throw; + } _; } function transfer(address _to, uint _value) onlyPayloadSize(2 * 32) { - balances[msg.sender] = balances[msg.sender].safeSub(_value); - balances[_to] = balances[_to].safeAdd(_value); + balances[msg.sender] = balances[msg.sender].sub(_value); + balances[_to] = balances[_to].add(_value); Transfer(msg.sender, _to, _value); } diff --git a/contracts/token/CrowdsaleToken.sol b/contracts/token/CrowdsaleToken.sol index 040e5123d..fad970c1e 100644 --- a/contracts/token/CrowdsaleToken.sol +++ b/contracts/token/CrowdsaleToken.sol @@ -32,10 +32,10 @@ contract CrowdsaleToken is StandardToken { throw; } - uint tokens = msg.value.safeMul(getPrice()); - totalSupply = totalSupply.safeAdd(tokens); + uint tokens = msg.value.mul(getPrice()); + totalSupply = totalSupply.add(tokens); - balances[recipient] = balances[recipient].safeAdd(tokens); + balances[recipient] = balances[recipient].add(tokens); if (!multisig.send(msg.value)) { throw; diff --git a/contracts/token/MintableToken.sol b/contracts/token/MintableToken.sol index 30235a815..cef7ff8d4 100644 --- a/contracts/token/MintableToken.sol +++ b/contracts/token/MintableToken.sol @@ -29,8 +29,8 @@ contract MintableToken is StandardToken, Ownable { } function mint(address _to, uint _amount) onlyOwner canMint returns (bool) { - totalSupply = safeAdd(totalSupply, _amount); - balances[_to] = safeAdd(balances[_to], _amount); + totalSupply = totalSupply.add(_amount); + balances[_to] = balances[_to].add(_amount); Mint(_to, _amount); return true; } diff --git a/contracts/token/StandardToken.sol b/contracts/token/StandardToken.sol index d61de6bfb..ee499b964 100644 --- a/contracts/token/StandardToken.sol +++ b/contracts/token/StandardToken.sol @@ -19,12 +19,12 @@ contract StandardToken is BasicToken, ERC20 { function transferFrom(address _from, address _to, uint _value) { var _allowance = allowed[_from][msg.sender]; - // Check is not needed because safeSub(_allowance, _value) will already throw if this condition is not met + // Check is not needed because sub(_allowance, _value) will already throw if this condition is not met // if (_value > _allowance) throw; - balances[_to] = balances[_to].safeAdd(_value); - balances[_from] = balances[_from].safeSub(_value); - allowed[_from][msg.sender] = _allowance.safeSub(_value); + balances[_to] = balances[_to].add(_value); + balances[_from] = balances[_from].sub(_value); + allowed[_from][msg.sender] = _allowance.sub(_value); Transfer(_from, _to, _value); } diff --git a/contracts/token/VestedToken.sol b/contracts/token/VestedToken.sol index 9c4f4cdda..ded654ebe 100644 --- a/contracts/token/VestedToken.sol +++ b/contracts/token/VestedToken.sol @@ -52,8 +52,8 @@ contract VestedToken is StandardToken, LimitedTransferToken { grants[_holder][_grantId] = grants[_holder][grants[_holder].length - 1]; grants[_holder].length -= 1; - balances[msg.sender] = balances[msg.sender].safeAdd(nonVested); - balances[_holder] = balances[_holder].safeSub(nonVested); + balances[msg.sender] = balances[msg.sender].add(nonVested); + balances[_holder] = balances[_holder].sub(nonVested); Transfer(_holder, msg.sender, nonVested); } @@ -98,16 +98,16 @@ contract VestedToken is StandardToken, LimitedTransferToken { return tokens; } - uint256 cliffTokens = tokens.safeMul(cliff.safeSub(start)).safeDiv(vesting.safeSub(start)); + uint256 cliffTokens = tokens.mul(cliff.sub(start)).div(vesting.sub(start)); vestedTokens = cliffTokens; - uint256 vestingTokens = tokens.safeSub(cliffTokens); + uint256 vestingTokens = tokens.sub(cliffTokens); - vestedTokens = vestedTokens.safeAdd(vestingTokens.safeMul(time.safeSub(cliff)).safeDiv(vesting.safeSub(cliff))); + vestedTokens = vestedTokens.add(vestingTokens.mul(time.sub(cliff)).div(vesting.sub(cliff))); } function nonVestedTokens(TokenGrant grant, uint64 time) private constant returns (uint256) { - return grant.value.safeSub(vestedTokens(grant, time)); + return grant.value.sub(vestedTokens(grant, time)); } function lastTokenIsTransferableDate(address holder) constant public returns (uint64 date) { @@ -122,9 +122,9 @@ contract VestedToken is StandardToken, LimitedTransferToken { uint256 grantIndex = grants[holder].length; for (uint256 i = 0; i < grantIndex; i++) { uint256 current = nonVestedTokens(grants[holder][i], time); - nonVested = nonVested.safeAdd(current); + nonVested = nonVested.add(current); } - return SafeMath.min256(balances[holder].safeSub(nonVested), super.transferableTokens(holder, time)); + return SafeMath.min256(balances[holder].sub(nonVested), super.transferableTokens(holder, time)); } } diff --git a/test/helpers/SafeMathMock.sol b/test/helpers/SafeMathMock.sol index c0be1d70f..f4c6e90f6 100644 --- a/test/helpers/SafeMathMock.sol +++ b/test/helpers/SafeMathMock.sol @@ -8,14 +8,14 @@ contract SafeMathMock { uint public result; function multiply(uint a, uint b) { - result = SafeMath.safeMul(a, b); + result = SafeMath.mul(a, b); } function subtract(uint a, uint b) { - result = SafeMath.safeSub(a, b); + result = SafeMath.sub(a, b); } function add(uint a, uint b) { - result = SafeMath.safeAdd(a, b); + result = SafeMath.add(a, b); } } From 7592122e4df1b0632fcef48e8987c0bd90006437 Mon Sep 17 00:00:00 2001 From: Manuel Araoz Date: Wed, 19 Apr 2017 18:34:53 -0300 Subject: [PATCH 3/3] fix indentation --- contracts/token/BasicToken.sol | 6 +++--- contracts/token/ERC20.sol | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/contracts/token/BasicToken.sol b/contracts/token/BasicToken.sol index 58b519f6f..81192144b 100644 --- a/contracts/token/BasicToken.sol +++ b/contracts/token/BasicToken.sol @@ -14,9 +14,9 @@ contract BasicToken is ERC20Basic { mapping(address => uint) balances; -/* - * Fix for the ERC20 short address attack - */ + /* + * Fix for the ERC20 short address attack + */ modifier onlyPayloadSize(uint size) { if(msg.data.length < size + 4) { throw; diff --git a/contracts/token/ERC20.sol b/contracts/token/ERC20.sol index 6efe7b5d4..2116ab95b 100644 --- a/contracts/token/ERC20.sol +++ b/contracts/token/ERC20.sol @@ -10,7 +10,6 @@ import './ERC20Basic.sol'; */ contract ERC20 is ERC20Basic { function allowance(address owner, address spender) constant returns (uint); - function transferFrom(address from, address to, uint value); function approve(address spender, uint value); event Approval(address indexed owner, address indexed spender, uint value);