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.

Sign up for GitHub

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

Jump to bottom

Disallow ERC20._transfer from the zero address. #1752

Merged
merged 10 commits into from
May 16, 2019
Merged

Disallow ERC20._transfer from the zero address. #1752

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
aa4cd0b
Add requirement of non-zero from to ERC20 transfer.
nventuro May 15, 2019
8a74793
Add test for transferFrom zero address to behavior.
nventuro May 15, 2019
3940149
Create ERC20.transfer behavior.
nventuro May 15, 2019
cf011e1
Add tests for _transfer.
nventuro May 15, 2019
73f7fbb
Add changelog entry.
nventuro May 15, 2019
2a90883
Fix linter error.
nventuro May 15, 2019
5fe5dd1
Delete repeated test.
nventuro May 15, 2019
e618dce
Fix hardcoded error prefix.
nventuro May 15, 2019
dd36523
Update CHANGELOG.md
nventuro May 16, 2019
8989f4c
Address review comments.
nventuro May 16, 2019
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
1 change: 1 addition & 0 deletions contracts/token/ERC20/ERC20.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ contract ERC20 is IERC20 {
* @param value The amount to be transferred.
*/
function _transfer(address from, address to, uint256 value) internal {
require(from != address(0), "ERC20: transfer from the zero address");
require(to != address(0), "ERC20: transfer to the zero address");

_balances[from] = _balances[from].sub(value);
Expand Down
139 changes: 77 additions & 62 deletions test/token/ERC20/ERC20.behavior.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,101 +73,116 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip
describe('transfer from', function () {
const spender = recipient;

describe('when the recipient is not the zero address', function () {
const to = anotherAccount;
describe('when the token owner is not the zero address', function () {
const tokenOwner = initialHolder;

describe('when the spender has enough approved balance', function () {
beforeEach(async function () {
await this.token.approve(spender, initialSupply, { from: initialHolder });
});
describe('when the recipient is not the zero address', function () {
const to = anotherAccount;

describe('when the spender has enough approved balance', function () {
beforeEach(async function () {
await this.token.approve(spender, initialSupply, { from: initialHolder });
});

describe('when the initial holder has enough balance', function () {
const amount = initialSupply;
describe('when the token owner has enough balance', function () {
const amount = initialSupply;

it('transfers the requested amount', async function () {
await this.token.transferFrom(initialHolder, to, amount, { from: spender });
it('transfers the requested amount', async function () {
await this.token.transferFrom(tokenOwner, to, amount, { from: spender });

(await this.token.balanceOf(initialHolder)).should.be.bignumber.equal('0');
(await this.token.balanceOf(tokenOwner)).should.be.bignumber.equal('0');

(await this.token.balanceOf(to)).should.be.bignumber.equal(amount);
});
(await this.token.balanceOf(to)).should.be.bignumber.equal(amount);
});

it('decreases the spender allowance', async function () {
await this.token.transferFrom(initialHolder, to, amount, { from: spender });
it('decreases the spender allowance', async function () {
await this.token.transferFrom(tokenOwner, to, amount, { from: spender });

(await this.token.allowance(initialHolder, spender)).should.be.bignumber.equal('0');
});
(await this.token.allowance(tokenOwner, spender)).should.be.bignumber.equal('0');
});

it('emits a transfer event', async function () {
const { logs } = await this.token.transferFrom(initialHolder, to, amount, { from: spender });
it('emits a transfer event', async function () {
const { logs } = await this.token.transferFrom(tokenOwner, to, amount, { from: spender });

expectEvent.inLogs(logs, 'Transfer', {
from: initialHolder,
to: to,
value: amount,
expectEvent.inLogs(logs, 'Transfer', {
from: tokenOwner,
to: to,
value: amount,
});
});
});

it('emits an approval event', async function () {
const { logs } = await this.token.transferFrom(initialHolder, to, amount, { from: spender });
it('emits an approval event', async function () {
const { logs } = await this.token.transferFrom(tokenOwner, to, amount, { from: spender });

expectEvent.inLogs(logs, 'Approval', {
owner: initialHolder,
spender: spender,
value: await this.token.allowance(initialHolder, spender),
expectEvent.inLogs(logs, 'Approval', {
owner: tokenOwner,
spender: spender,
value: await this.token.allowance(tokenOwner, spender),
});
});
});
});

describe('when the initial holder does not have enough balance', function () {
const amount = initialSupply.addn(1);
describe('when the token owner does not have enough balance', function () {
const amount = initialSupply.addn(1);

it('reverts', async function () {
await shouldFail.reverting.withMessage(this.token.transferFrom(
initialHolder, to, amount, { from: spender }), 'SafeMath: subtraction overflow'
);
it('reverts', async function () {
await shouldFail.reverting.withMessage(this.token.transferFrom(
tokenOwner, to, amount, { from: spender }), 'SafeMath: subtraction overflow'
);
});
});
});
});

describe('when the spender does not have enough approved balance', function () {
beforeEach(async function () {
await this.token.approve(spender, initialSupply.subn(1), { from: initialHolder });
});
describe('when the spender does not have enough approved balance', function () {
beforeEach(async function () {
await this.token.approve(spender, initialSupply.subn(1), { from: tokenOwner });
});

describe('when the initial holder has enough balance', function () {
const amount = initialSupply;
describe('when the token owner has enough balance', function () {
const amount = initialSupply;

it('reverts', async function () {
await shouldFail.reverting.withMessage(this.token.transferFrom(
initialHolder, to, amount, { from: spender }), 'SafeMath: subtraction overflow'
);
it('reverts', async function () {
await shouldFail.reverting.withMessage(this.token.transferFrom(
tokenOwner, to, amount, { from: spender }), 'SafeMath: subtraction overflow'
);
});
});
});

describe('when the initial holder does not have enough balance', function () {
const amount = initialSupply.addn(1);
describe('when the token owner does not have enough balance', function () {
const amount = initialSupply.addn(1);

it('reverts', async function () {
await shouldFail.reverting.withMessage(this.token.transferFrom(
initialHolder, to, amount, { from: spender }), 'SafeMath: subtraction overflow'
);
it('reverts', async function () {
await shouldFail.reverting.withMessage(this.token.transferFrom(
tokenOwner, to, amount, { from: spender }), 'SafeMath: subtraction overflow'
);
});
});
});
});
});

describe('when the recipient is the zero address', function () {
const amount = initialSupply;
const to = ZERO_ADDRESS;
describe('when the recipient is the zero address', function () {
const amount = initialSupply;
const to = ZERO_ADDRESS;

beforeEach(async function () {
await this.token.approve(spender, amount, { from: tokenOwner });
});

beforeEach(async function () {
await this.token.approve(spender, amount, { from: initialHolder });
it('reverts', async function () {
await shouldFail.reverting.withMessage(this.token.transferFrom(
tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer to the zero address`
);
});
});
});

describe('when the token owner is the zero address', function () {
const amount = 0;
nventuro marked this conversation as resolved.
Show resolved Hide resolved
const to = recipient;

it('reverts', async function () {
await shouldFail.reverting.withMessage(this.token.transferFrom(
initialHolder, to, amount, { from: spender }), `${errorPrefix}: transfer to the zero address`
ZERO_ADDRESS, to, amount, { from: to }), `${errorPrefix}: transfer from the zero address`
nventuro marked this conversation as resolved.
Show resolved Hide resolved
);
});
});
Expand Down