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 missing requirements to ERC777 #2212

Merged
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
4 changes: 4 additions & 0 deletions contracts/mocks/ERC777Mock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,8 @@ contract ERC777Mock is Context, ERC777 {
) public {
_mint(operator, to, amount, userData, operatorData);
}

function approveInternal(address holder, address spender, uint256 value) public {
_approve(holder, spender, value);
}
}
10 changes: 7 additions & 3 deletions contracts/token/ERC777/ERC777.sol
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ contract ERC777 is Context, IERC777, IERC20 {
)
internal
{
nventuro marked this conversation as resolved.
Show resolved Hide resolved
require(operator != address(0), "ERC777: operator is the zero address");
require(from != address(0), "ERC777: send from the zero address");
require(to != address(0), "ERC777: send to the zero address");

Expand Down Expand Up @@ -408,10 +409,13 @@ contract ERC777 is Context, IERC777, IERC20 {
emit Transfer(from, to, amount);
}

/**
* @dev See {ERC20-_approve}.
*
* Note that accounts cannot have allowance issued by their operators.
nventuro marked this conversation as resolved.
Show resolved Hide resolved
*/
function _approve(address holder, address spender, uint256 value) internal {
// TODO: restore this require statement if this function becomes internal, or is called at a new callsite. It is
// currently unnecessary.
//require(holder != address(0), "ERC777: approve from the zero address");
require(holder != address(0), "ERC777: approve from the zero address");
require(spender != address(0), "ERC777: approve to the zero address");

_allowances[holder][spender] = value;
Expand Down
22 changes: 20 additions & 2 deletions test/token/ERC777/ERC777.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const { accounts, contract, web3 } = require('@openzeppelin/test-environment');

const { BN, expectEvent, expectRevert, singletons } = require('@openzeppelin/test-helpers');
const { BN, constants, expectEvent, expectRevert, singletons } = require('@openzeppelin/test-helpers');
const { ZERO_ADDRESS } = constants;

const { expect } = require('chai');

Expand All @@ -15,6 +16,7 @@ const {

const {
shouldBehaveLikeERC20,
shouldBehaveLikeERC20Approve,
} = require('../ERC20/ERC20.behavior');

const ERC777 = contract.fromArtifact('ERC777Mock');
Expand All @@ -40,7 +42,23 @@ describe('ERC777', function () {
this.token = await ERC777.new(holder, initialSupply, name, symbol, defaultOperators);
});

shouldBehaveLikeERC20('ERC777', initialSupply, holder, anyone, defaultOperatorA);
describe('as an ERC20 token', function () {
shouldBehaveLikeERC20('ERC777', initialSupply, holder, anyone, defaultOperatorA);

describe('_approve', function () {
shouldBehaveLikeERC20Approve('ERC777', holder, anyone, initialSupply, function (owner, spender, amount) {
return this.token.approveInternal(owner, spender, amount);
});

describe('when the owner is the zero address', function () {
it('reverts', async function () {
await expectRevert(this.token.approveInternal(ZERO_ADDRESS, anyone, initialSupply),
'ERC777: approve from the zero address'
);
});
});
});
});

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