Create detailed ERC20 interface#477
Conversation
frangio
left a comment
There was a problem hiding this comment.
Thanks a lot @facuspagnuolo! Good idea. 😄
Suggested a few changes before merging
| contract DetailedERC20 is ERC20 { | ||
| string public name; | ||
| string public symbol; | ||
| uint256 public decimals; |
|
|
||
| contract DetailedERC20Mock is StandardToken, DetailedERC20 { | ||
| function DetailedERC20Mock(uint256 _decimals, string _symbol, string _name) { | ||
| require(_decimals > 0); |
There was a problem hiding this comment.
Hm, I think decimals could be 0, in the case of indivisible tokens.
| import '../../contracts/token/DetailedERC20.sol'; | ||
|
|
||
| contract DetailedERC20Mock is StandardToken, DetailedERC20 { | ||
| function DetailedERC20Mock(uint256 _decimals, string _symbol, string _name) { |
There was a problem hiding this comment.
I think I'd actually make this the constructor of DetailedERC20 itself. In that way, contracts can inherit from it and call the constructor without having to modify the state variables. It will look cleaner IMO. (And the mock wouldn't be necessary for testing.)
Also, the order of arguments (name, symbol, decimals) makes a bit more sense to me.
|
|
||
| it('has a name', async function () { | ||
| const name = await detailedERC20.name(); | ||
| name.should.be.equal('My Detailed ERC20'); |
There was a problem hiding this comment.
You can use the constants defined above in these tests! _name, etc.
| uint256 public decimals; | ||
| uint8 public decimals; | ||
|
|
||
| function DetailedERC20(string name, string _symbol, uint8 _decimals) { |
There was a problem hiding this comment.
Travis is failing because name should be _name.
|
@facuspagnuolo 1 test still failing, please check |
2a289e4 to
cde7f44
Compare
cde7f44 to
365c875
Compare
|
Thanks @facuspagnuolo! |
…detailed_erc20_interface Create detailed ERC20 interface
No description provided.