From 21ea36621d7a67710f227df5b5b064c2fbb16ec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 28 Nov 2018 08:49:40 -0300 Subject: [PATCH 1/3] transferFrom now emits an Approval event, indicating the updated allowance. --- contracts/token/ERC20/ERC20.sol | 1 + test/token/ERC20/ERC20.test.js | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index abb43b74d13..69d65096c41 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -81,6 +81,7 @@ contract ERC20 is IERC20 { function transferFrom(address from, address to, uint256 value) public returns (bool) { _allowed[from][msg.sender] = _allowed[from][msg.sender].sub(value); _transfer(from, to, value); + emit Approval(from, msg.sender, _allowed[from][msg.sender]); return true; } diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index d60e9130cc1..833e946234c 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -199,6 +199,16 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { value: amount, }); }); + + it('emits an approval event', async function () { + const { logs } = await this.token.transferFrom(owner, to, amount, { from: spender }); + + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: await this.token.allowance(owner, spender), + }); + }); }); describe('when the owner does not have enough balance', function () { From c77b41914bbdbd4b2a7b66ba1b04558b8e659e36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 28 Nov 2018 09:00:20 -0300 Subject: [PATCH 2/3] Updated burnFrom to also emit Approval. --- contracts/token/ERC20/ERC20.sol | 3 +-- test/token/ERC20/ERC20.test.js | 10 +++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 69d65096c41..61a77a0b8da 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -170,9 +170,8 @@ contract ERC20 is IERC20 { * @param value The amount that will be burnt. */ function _burnFrom(address account, uint256 value) internal { - // Should https://github.com/OpenZeppelin/zeppelin-solidity/issues/707 be accepted, - // this function needs to emit an event with the updated approval. _allowed[account][msg.sender] = _allowed[account][msg.sender].sub(value); _burn(account, value); + emit Approval(account, msg.sender, _allowed[account][msg.sender]); } } diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index 833e946234c..42bd872fd65 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -531,7 +531,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { (await this.token.allowance(owner, spender)).should.be.bignumber.equal(expectedAllowance); }); - it('emits Transfer event', async function () { + it('emits a Transfer event', async function () { const event = expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, @@ -539,6 +539,14 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { event.args.value.should.be.bignumber.equal(amount); }); + + it('emits an Approval event', async function () { + expectEvent.inLogs(this.logs, 'Approval', { + owner: owner, + spender: spender, + value: await this.token.allowance(owner, spender), + }); + }); }); }; From 5f89706f7ceebf0108670c46ba08ecb553d56a85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 29 Nov 2018 07:13:56 -0300 Subject: [PATCH 3/3] Added notices about the extra Approval events. --- contracts/token/ERC20/ERC20.sol | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 61a77a0b8da..b41f64462f1 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -9,6 +9,10 @@ import "../../math/SafeMath.sol"; * @dev Implementation of the basic standard token. * https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20.md * Originally based on code by FirstBlood: https://github.com/Firstbloodio/token/blob/master/smart_contract/FirstBloodToken.sol + * + * This implementation emits additional Approval events, allowing applications to reconstruct the allowance status for + * all accounts just by listening to said events. Note that this isn't required by the specification, and other + * compliant implementations may not do it. */ contract ERC20 is IERC20 { using SafeMath for uint256; @@ -73,7 +77,9 @@ contract ERC20 is IERC20 { } /** - * @dev Transfer tokens from one address to another + * @dev Transfer tokens from one address to another. + * Note that while this function emits an Approval event, this is not required as per the specification, + * and other compliant implementations may not emit the event. * @param from address The address which you want to send tokens from * @param to address The address which you want to transfer to * @param value uint256 the amount of tokens to be transferred @@ -91,6 +97,7 @@ contract ERC20 is IERC20 { * allowed value is better to use this function to avoid 2 calls (and wait until * the first transaction is mined) * From MonolithDAO Token.sol + * Emits an Approval event. * @param spender The address which will spend the funds. * @param addedValue The amount of tokens to increase the allowance by. */ @@ -108,6 +115,7 @@ contract ERC20 is IERC20 { * allowed value is better to use this function to avoid 2 calls (and wait until * the first transaction is mined) * From MonolithDAO Token.sol + * Emits an Approval event. * @param spender The address which will spend the funds. * @param subtractedValue The amount of tokens to decrease the allowance by. */ @@ -166,6 +174,7 @@ contract ERC20 is IERC20 { * @dev Internal function that burns an amount of the token of a given * account, deducting from the sender's allowance for said account. Uses the * internal burn function. + * Emits an Approval event (reflecting the reduced allowance). * @param account The account whose tokens will be burnt. * @param value The amount that will be burnt. */