Update open zeppelin and oraclize #398
Conversation
|
New version of Ownable contract emits OwnershipTransferred event in a constructor. |
|
@kostind could you please remove the conflicts so I can approve the PR. |
|
Hmm, PR Travis build is broken. |
@satyamakgec done |
@maxsam4 npm run wintest -> 935 passing (5m) Confilcts in package.json were resolved recently. |
|
@kostind The push commit is building fine even in Travis as well. Pull PR merges the files and runs coverage. I resolved the package.json conflicts and it could be that something went wrong there. unlikely though. |
|
@maxsam4 I tried to resolve conflicts as well, but you won. |
|
I've got the same result locally for |
Just confirming, From the same result, You mean it timed out as well? |
|
@maxsam4 Yes, build hangs on the same step. |
|
|
|
@satyamakgec Solidity-coverage is now fixed, please review this again. |
| bool verified = _updateTransfer(_from, address(0), _value, _data); | ||
| _adjustTotalSupplyCheckpoints(); | ||
| balances[_from] = balances[_from].sub(_value); | ||
| totalSupply_ = totalSupply_.sub(_value); | ||
| _burn(_from, _value); |
There was a problem hiding this comment.
@maxsam4 @satyamakgec @kostind Should this be super._burn instead? Or otherwise we should maybe rename our _burn function so we don't clash with the OZ namespace.
There was a problem hiding this comment.
(bit surprised this actually works as currently written)
There was a problem hiding this comment.
I was confused by this when I first saw it as well. It works because our _burn() takes an extra parameter. Therefore, it's function overloading and not overriding that's happening here.
contracts/tokens/SecurityToken.sol
Outdated
| @@ -740,16 +734,23 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr | |||
| require(_burn(msg.sender, _value, _data), "Burn invalid"); | |||
| } | |||
|
|
|||
| function _burnFrom(address _from, uint256 _value, bytes _data) internal returns(bool) { | |||
| require(_value <= balanceOf(_from), "Value too high"); | |||
There was a problem hiding this comment.
This is already checked in the _burn call (from _burnFrom)
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
Update OpenZeppelin and Oraclize
What is the current behavior?
(You can also link to an open issue here)
What is the new behavior?
(Define and describe any new functionality. Clarify if this is a feature change)
Does this PR introduce a breaking change?
Run "npm install" to update version of openzeppelin-solidity.
Any Other information: