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

Add ERC20 compatibility to ERC777. #1735

Merged
merged 17 commits into from
May 8, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
30 changes: 15 additions & 15 deletions contracts/drafts/ERC777/ERC777.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ contract ERC777 is IERC777, ERC20Detailed {
uint256 granularity,
address[] memory defaultOperators
) public ERC20Detailed(name, symbol, 18) { // The spec requires that decimals be 18
nventuro marked this conversation as resolved.
Show resolved Hide resolved
require(granularity > 0);
require(granularity > 0, "ERC777: granularity is 0");

_granularity = granularity;

Expand Down Expand Up @@ -86,7 +86,7 @@ contract ERC777 is IERC777, ERC20Detailed {
)
external
{
require(isOperatorFor(msg.sender, from));
require(isOperatorFor(msg.sender, from), "ERC777: caller is not an operator for holder");
_send(msg.sender, from, to, amount, data, operatorData);
}

Expand Down Expand Up @@ -135,7 +135,7 @@ contract ERC777 is IERC777, ERC20Detailed {
* @param operatorData bytes extra information provided by the operator (if any)
*/
function operatorBurn(address from, uint256 amount, bytes calldata data, bytes calldata operatorData) external {
require(isOperatorFor(msg.sender, from));
require(isOperatorFor(msg.sender, from), "ERC777: caller is not an operator for holder");
_burn(msg.sender, from, amount, data, operatorData);
}

Expand All @@ -144,7 +144,7 @@ contract ERC777 is IERC777, ERC20Detailed {
* @param operator address to be authorized as operator
*/
function authorizeOperator(address operator) external {
require(msg.sender != operator);
require(msg.sender != operator, "ERC777: operator authorization to self");
nventuro marked this conversation as resolved.
Show resolved Hide resolved

if (_defaultOperators[operator]) {
delete _revokedDefaultOperators[msg.sender][operator];
Expand All @@ -160,7 +160,7 @@ contract ERC777 is IERC777, ERC20Detailed {
* @param operator address to revoke operator rights from
*/
function revokeOperator(address operator) external {
require(operator != msg.sender);
require(operator != msg.sender, "ERC777: operator revocation of self");
nventuro marked this conversation as resolved.
Show resolved Hide resolved

if (_defaultOperators[operator]) {
_revokedDefaultOperators[msg.sender][operator] = true;
Expand Down Expand Up @@ -265,8 +265,8 @@ contract ERC777 is IERC777, ERC20Detailed {
)
internal
{
require(to != address(0));
require((amount % _granularity) == 0);
require(to != address(0), "ERC777: mint to the zero address");
require((amount % _granularity) == 0, "ERC777: mint of non-multiple amount");

// Update state variables
_totalSupply = _totalSupply.add(amount);
Expand Down Expand Up @@ -297,9 +297,9 @@ contract ERC777 is IERC777, ERC20Detailed {
)
private
{
require(from != address(0));
require(to != address(0));
require((amount % _granularity) == 0);
require(from != address(0), "ERC777: transfer from the zero address");
require(to != address(0), "ERC777: transfer to the zero address");
require((amount % _granularity) == 0, "ERC777: transfer of non-multiple amount");

_callTokensToSend(operator, from, to, amount, userData, operatorData);

Expand Down Expand Up @@ -330,8 +330,8 @@ contract ERC777 is IERC777, ERC20Detailed {
)
private
{
require(from != address(0));
require((amount % _granularity) == 0);
require(from != address(0), "ERC777: burn from the zero address");
require((amount % _granularity) == 0, "ERC777: burn of non-multiple amount");

_callTokensToSend(operator, from, address(0), amount, data, operatorData);

Expand All @@ -344,8 +344,8 @@ contract ERC777 is IERC777, ERC20Detailed {
}

function _approve(address owner, address spender, uint256 value) private {
require(owner != address(0));
require(spender != address(0));
require(owner != address(0), "ERC777: approve from the zero address");
require(spender != address(0), "ERC777: approve to the zero address");

_allowances[owner][spender] = value;
emit Approval(owner, spender, value);
Expand Down Expand Up @@ -400,7 +400,7 @@ contract ERC777 is IERC777, ERC20Detailed {
if (implementer != address(0)) {
IERC777Recipient(implementer).tokensReceived(operator, from, to, amount, userData, operatorData);
} else {
require(!to.isContract());
require(!to.isContract(), "ERC777: token recipient contract has no implementer for ERC777TokensRecipient");
}
}
}
49 changes: 35 additions & 14 deletions test/drafts/ERC777/ERC777.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ contract('ERC777', function ([
});

it('reverts with a granularity of zero', async function () {
await shouldFail.reverting(ERC777.new(holder, initialSupply, name, symbol, 0, []));
await shouldFail.reverting.withMessage(
ERC777.new(holder, initialSupply, name, symbol, 0, []), 'ERC777: granularity is 0'
);
});

context('with a granularity of one', function () {
Expand All @@ -46,7 +48,7 @@ contract('ERC777', function ([
this.token = await ERC777.new(holder, initialSupply, name, symbol, granularity, defaultOperators);
});

shouldBehaveLikeERC20(initialSupply, holder, anyone, defaultOperatorA);
shouldBehaveLikeERC20('ERC777', initialSupply, holder, anyone, defaultOperatorA);

it.skip('does not emit AuthorizedOperator events for default operators', async function () {
expectEvent.not.inConstructor(this.token, 'AuthorizedOperator'); // This helper needs to be implemented
Expand Down Expand Up @@ -164,11 +166,15 @@ contract('ERC777', function ([
});

it('reverts when self-authorizing', async function () {
await shouldFail.reverting(this.token.authorizeOperator(holder, { from: holder }));
await shouldFail.reverting.withMessage(
this.token.authorizeOperator(holder, { from: holder }), 'ERC777: operator authorization to self'
);
});

it('reverts when self-revoking', async function () {
await shouldFail.reverting(this.token.revokeOperator(holder, { from: holder }));
await shouldFail.reverting.withMessage(
this.token.revokeOperator(holder, { from: holder }), 'ERC777: operator revocation of self'
);
});

it('non-operators can be revoked', async function () {
Expand Down Expand Up @@ -229,7 +235,10 @@ contract('ERC777', function ([
});

it('cannot be revoked for themselves', async function () {
await shouldFail.reverting(this.token.revokeOperator(defaultOperatorA, { from: defaultOperatorA }));
await shouldFail.reverting.withMessage(
this.token.revokeOperator(defaultOperatorA, { from: defaultOperatorA }),
'ERC777: operator revocation of self'
);
});

context('with revoked default operator', function () {
Expand Down Expand Up @@ -279,18 +288,23 @@ contract('ERC777', function ([
});

it('send reverts', async function () {
await shouldFail.reverting(this.token.send(this.recipient, amount, data));
await shouldFail.reverting.withMessage(
this.token.send(this.recipient, amount, data, { from: holder }),
'ERC777: token recipient contract has no implementer for ERC777TokensRecipient',
);
});

it('operatorSend reverts', async function () {
await shouldFail.reverting(
this.token.operatorSend(this.sender, this.recipient, amount, data, operatorData, { from: operator })
await shouldFail.reverting.withMessage(
this.token.operatorSend(this.sender, this.recipient, amount, data, operatorData, { from: operator }),
'ERC777: token recipient contract has no implementer for ERC777TokensRecipient',
);
});

it('mint (internal) reverts', async function () {
await shouldFail.reverting(
this.token.mintInternal(operator, this.recipient, amount, data, operatorData)
await shouldFail.reverting.withMessage(
this.token.mintInternal(operator, this.recipient, amount, data, operatorData),
'ERC777: token recipient contract has no implementer for ERC777TokensRecipient',
);
});
});
Expand Down Expand Up @@ -433,15 +447,21 @@ contract('ERC777', function ([
shouldDirectSendTokens(from, anyone, granularity.muln(2), data);

it('reverts when sending an amount non-multiple of the granularity', async function () {
await shouldFail.reverting(this.token.send(anyone, granularity.subn(1), data, { from }));
await shouldFail.reverting.withMessage(
this.token.send(anyone, granularity.subn(1), data, { from }),
'ERC777: transfer of non-multiple amount',
);
});

shouldDirectBurnTokens(from, new BN('0'), data);
shouldDirectBurnTokens(from, granularity, data);
shouldDirectBurnTokens(from, granularity.muln(2), data);

it('reverts when burning an amount non-multiple of the granularity', async function () {
await shouldFail.reverting(this.token.burn(granularity.subn(1), data, { from }));
await shouldFail.reverting.withMessage(
this.token.burn(granularity.subn(1), data, { from }),
'ERC777: burn of non-multiple amount',
);
});
});

Expand All @@ -450,8 +470,9 @@ contract('ERC777', function ([
shouldInternalMintTokens(anyone, defaultOperatorA, granularity.muln(2), data, operatorData);

it('reverts when minting an amount non-multiple of the granularity', async function () {
await shouldFail.reverting(
this.token.mintInternal(defaultOperatorA, anyone, granularity.subn(1), data, operatorData)
await shouldFail.reverting.withMessage(
this.token.mintInternal(defaultOperatorA, anyone, granularity.subn(1), data, operatorData),
'ERC777: mint of non-multiple amount',
);
});
});
Expand Down
18 changes: 10 additions & 8 deletions test/token/ERC20/ERC20.behavior.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const { BN, constants, expectEvent, shouldFail } = require('openzeppelin-test-helpers');
const { ZERO_ADDRESS } = constants;

function shouldBehaveLikeERC20 (initialSupply, initialHolder, recipient, anotherAccount) {
function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recipient, anotherAccount) {
describe('total supply', function () {
it('returns the total amount of tokens', async function () {
(await this.token.totalSupply()).should.be.bignumber.equal(initialSupply);
Expand Down Expand Up @@ -64,7 +64,7 @@ function shouldBehaveLikeERC20 (initialSupply, initialHolder, recipient, another

it('reverts', async function () {
await shouldFail.reverting.withMessage(this.token.transfer(to, initialSupply, { from: initialHolder }),
'ERC20: transfer to the zero address'
`${errorPrefix}: transfer to the zero address`
);
});
});
Expand Down Expand Up @@ -167,20 +167,22 @@ function shouldBehaveLikeERC20 (initialSupply, initialHolder, recipient, another

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

describe('approve', function () {
shouldBehaveLikeERC20Approve(initialHolder, recipient, initialSupply, function (owner, spender, amount) {
return this.token.approve(spender, amount, { from: owner });
});
shouldBehaveLikeERC20Approve(errorPrefix, initialHolder, recipient, initialSupply,
function (owner, spender, amount) {
return this.token.approve(spender, amount, { from: owner });
}
);
});
}

function shouldBehaveLikeERC20Approve (owner, spender, supply, approve) {
function shouldBehaveLikeERC20Approve (errorPrefix, owner, spender, supply, approve) {
describe('when the spender is not the zero address', function () {
describe('when the sender has enough balance', function () {
const amount = supply;
Expand Down Expand Up @@ -254,7 +256,7 @@ function shouldBehaveLikeERC20Approve (owner, spender, supply, approve) {
describe('when the spender is the zero address', function () {
it('reverts', async function () {
await shouldFail.reverting.withMessage(approve.call(this, owner, ZERO_ADDRESS, supply),
'ERC20: approve to the zero address'
`${errorPrefix}: approve to the zero address`
);
});
});
Expand Down
5 changes: 3 additions & 2 deletions test/token/ERC20/ERC20.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ const ERC20Mock = artifacts.require('ERC20Mock');

contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) {
const initialSupply = new BN(100);

beforeEach(async function () {
this.token = await ERC20Mock.new(initialHolder, initialSupply);
});

shouldBehaveLikeERC20(initialSupply, initialHolder, recipient, anotherAccount);
shouldBehaveLikeERC20('ERC20', initialSupply, initialHolder, recipient, anotherAccount);

describe('decrease allowance', function () {
describe('when the spender is not the zero address', function () {
Expand Down Expand Up @@ -330,7 +331,7 @@ contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) {
});

describe('_approve', function () {
shouldBehaveLikeERC20Approve(initialHolder, recipient, initialSupply, function (owner, spender, amount) {
shouldBehaveLikeERC20Approve('ERC20', initialHolder, recipient, initialSupply, function (owner, spender, amount) {
return this.token.approveInternal(owner, spender, amount);
});

Expand Down