-
Notifications
You must be signed in to change notification settings - Fork 11.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Transfer event when token is minted #345
Conversation
@ -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 <notifications@github.com>
Sent:Wed, 02 Aug 2017 07:47:48 +0800
To:OpenZeppelin/zeppelin-solidity <zeppelin-solidity@noreply.github.com>
Cc:Subscribed <subscribed@noreply.github.com>
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"}}}
|
what ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @SylTi! Would you mind writing a test for MintableToken
to check that both Mint
and Transfer
events are being emitted?
@frangio Updated with tests ;) |
await token.mint(accounts[0], 100); | ||
|
||
const result = await token.mint(accounts[0], 100); | ||
assert.equal(result.logs[0].event, 'Mint'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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" <notifications@github.com> 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>
.
|
Alright, merging as is! Thanks @SylTi! |
Add Transfer event when token is minted
cf issue #344