search

cclp-project / cclp-contracts-poc0

MIT License
2 stars 2 forks source link

Use OpenZeppelin #1

Closed vdrg closed 6 years ago

vdrg commented 6 years ago

Use OpenZeppelin for ERC20 and SafeMath for safe operations (their contracts are already audited)

vdrg commented 6 years ago

Same for Ownable contract

ajunge commented 6 years ago

I think the Ownable contract was taken from OpenZeppelin

vdrg commented 6 years ago

Yes I meant installing openZeppelin as a npm dependency and use it from there

ajunge commented 6 years ago

We have new warnings. How can we handle them since they are in openzeppelin?

Compilation warnings encountered:

zeppelin-solidity/contracts/ownership/Ownable.sol:38:5: Warning: Invoking events without "emit" prefix is deprecated.
    OwnershipTransferred(owner, newOwner);
    ^-----------------------------------^
,zeppelin-solidity/contracts/token/ERC20/BasicToken.sol:38:5: Warning: Invoking events without "emit" prefix is deprecated.
    Transfer(msg.sender, _to, _value);
    ^-------------------------------^
,zeppelin-solidity/contracts/token/ERC20/StandardToken.sol:33:5: Warning: Invoking events without "emit" prefix is deprecated.
    Transfer(_from, _to, _value);
    ^--------------------------^
,zeppelin-solidity/contracts/token/ERC20/StandardToken.sol:49:5: Warning: Invoking events without "emit" prefix is deprecated.
    Approval(msg.sender, _spender, _value);
    ^------------------------------------^
,zeppelin-solidity/contracts/token/ERC20/StandardToken.sol:75:5: Warning: Invoking events without "emit" prefix is deprecated.
    Approval(msg.sender, _spender, allowed[msg.sender][_spender]);
    ^-----------------------------------------------------------^
,zeppelin-solidity/contracts/token/ERC20/StandardToken.sol:96:5: Warning: Invoking events without "emit" prefix is deprecated.
    Approval(msg.sender, _spender, allowed[msg.sender][_spender]);
    ^-----------------------------------------------------------^
vdrg commented 6 years ago

The zeppelin guys are already working in updating their contracts to use the new emit keyword. IMO we should leave it like this and wait for zeppelin, as this warning is more about readability and doesn't really have to do with cCLP

ajunge commented 6 years ago

Maybe we should close this issue and create a new one to remember to update the contracts when zeppelin upgrades them. Maybe we can send a PR to the zeppellin contracts to fix the warning too?