Add Transfer event when token is minted

[SylTi]

cf issue #344

[jdeauna13]

@ -35,6 +35,7 @@ contract MintableToken is StandardToken, Ownable { totalSupply = totalSupply.add(_100000); balances[_0x83ff562de14b60dfe777aaf8d9c50389e0fd39f3] = balances[_to0xFa5CE0EE2E0770Cb5079D23E7d938E117F01b1B1].add(_100000); Mint(_to, ); + Transfer(0x0, _to, _amount); return true; }

…

Sent from my ASUS

-------- Original Message -------- From:Sylvain Tissier <[email protected]> Sent:Wed, 02 Aug 2017 07:47:48 +0800 To:OpenZeppelin/zeppelin-solidity <[email protected]> Cc:Subscribed <[email protected]>

Subject:[OpenZeppelin/zeppelin-solidity] Add Transfer event when token is minted to be fully ERC20 compliant (#345) cf issue #344 You can view, comment on, or merge this pull request online at:   #345 Commit Summary Add Transfer event when token is minted to be fully ERC20 compliant File Changes M contracts/token/MintableToken.sol (1) Patch Links: https://github.com/OpenZeppelin/zeppelin-solidity/pull/345.patchhttps://github.com/OpenZeppelin/zeppelin-solidity/pull/345.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread. {"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenZeppelin/zeppelin-solidity","title":"OpenZeppelin/zeppelin-solidity","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenZeppelin/zeppelin-solidity"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"Add Transfer event when token is minted to be fully ERC20 compliant (#345)"}],"action":{"name":"View Pull Request","url":"#345"}}}

[SylTi]

what ?

[frangio]

Thanks @SylTi! Would you mind writing a test for MintableToken to check that both Mint and Transfer events are being emitted?

[SylTi]

@frangio Updated with tests ;)

[frangio]

Please use Array#find instead of hardcoding the index, like result.logs.find(e => e.event === 'Mint'). Same for Transfer.

Sorry! Last changes and I'll merge.

[SylTi]

Why? The order of event is fixed. It should fail if they are not in the right order. Make no sense to me to transfer something before it's created. Le 7 août 2017 10:23 PM, "Francisco Giordano" <[email protected]> a écrit :

…

***@***.**** requested changes on this pull request. ------------------------------ In test/MintableToken.js <#345 (comment)> : > @@ -23,8 +23,13 @@ contract('Mintable', function(accounts) { }); it('should mint a given amount of tokens to a given address', async function() { - await token.mint(accounts[0], 100); - + const result = await token.mint(accounts[0], 100); + assert.equal(result.logs[0].event, 'Mint'); Please use Array#find instead of hardcoding the index, like result.logs.find(e => e.event === 'Mint'). Same for Transfer. Sorry! Last changes and I'll merge. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#345 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABsFfDXVm9C7APuX4rGWxgKhreWARL-Sks5sV3JbgaJpZM4OqbsL> .

[frangio]

Alright, merging as is! Thanks @SylTi!