Updates: 0 address transfer prevention and emit keyword #135
Conversation
Add code to prevent transferring to the 0x0 address, and the contract address, according to Token Implementation Best Practices. Reference: https://consensys.github.io/smart-contract-best-practices/tokens/
Added short comments
Added SafeEIP20 that inherits from EIP20 and prevent transferring tokens to 0x0 and contract address. EIP20Factory is changed to use SafeEIP20. Reverted EIP20 to its state before the pull request created. JavaScript Truffle Tests are writen for SafeEIP20.
Modify Function Declaration to follow Solidity Style Guide: https://solidity.readthedocs.io/en/v0.3.1/style-guide.html
Encapsulated EIP compatible functions' tests inside a function and make it called for both EIP20 and SafeEIP20 smart contracts.
| @@ -37,7 +37,7 @@ contract EIP20Factory { | |||
| public | |||
| returns (address) { | |||
|
|
|||
| EIP20 newToken = (new EIP20(_initialAmount, _name, _decimals, _symbol)); | |||
| SafeEIP20 newToken = (new SafeEIP20(_initialAmount, _name, _decimals, _symbol)); | |||
There was a problem hiding this comment.
Do we want to undo this change?
There was a problem hiding this comment.
@skmgoldin I had the impression that we were going to revert the factory back to EIP20.
There was a problem hiding this comment.
Since we all agree that: it is part of the best practices to prevent transferring to 0x0 and the contract-address, why not apply this best practice every where? In other words, what is the harm of using the SafeEIP20 in the factory?
(The link to the best-practices I am referring: https://github.com/ConsenSys/smart-contract-best-practices/blob/master/docs/tokens.md).
Thanks,
There was a problem hiding this comment.
My only comments pertain to:
- which contract to deploy from the factory
- whitespace style of long
transferFromcalls
Highly recommend viewing with the Diff setting to ignore whitespace changes.
https://github.com/ConsenSys/Tokens/pull/135/files?utf8=%E2%9C%93&diff=split&w=1
| const allowance = await HST.allowance.call(accounts[0], accounts[1]); | ||
| assert.strictEqual(allowance.toNumber(), 100); | ||
|
|
||
| await assertRevert(HST.transferFrom.call(accounts[0], ZERO_ADDRESS, 10, { from: accounts[1] })); |
There was a problem hiding this comment.
I quite like the multiline approach taken on transferFrom tests in the above file, any objection to me adding a commit on top for that?
No description provided.