Skip to content
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

Approval events on transferFrom and burnFrom #1524

Merged
merged 3 commits into from
Nov 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions contracts/token/ERC20/ERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -73,14 +77,17 @@ 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
*/
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;
}

Expand All @@ -90,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.
*/
Expand All @@ -107,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.
*/
Expand Down Expand Up @@ -165,13 +174,13 @@ 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.
*/
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]);
}
}
20 changes: 19 additions & 1 deletion test/token/ERC20/ERC20.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -521,14 +531,22 @@ 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,
});

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),
});
});
});
};

Expand Down