From 3cef880803da7f8cfabc09f7748f3ba0eaaad0fe Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Fri, 9 Mar 2018 13:49:13 -0300 Subject: [PATCH] Allow operators to call approve on a token --- contracts/token/ERC721/ERC721BasicToken.sol | 5 +- .../ERC721/ERC721BasicToken.behaviour.js | 96 +++++++++---------- 2 files changed, 48 insertions(+), 53 deletions(-) diff --git a/contracts/token/ERC721/ERC721BasicToken.sol b/contracts/token/ERC721/ERC721BasicToken.sol index 3f2985130..850a7c2b2 100644 --- a/contracts/token/ERC721/ERC721BasicToken.sol +++ b/contracts/token/ERC721/ERC721BasicToken.sol @@ -83,12 +83,15 @@ contract ERC721BasicToken is ERC721Basic { * @dev Approves another address to transfer the given token ID * @dev The zero address indicates there is no approved address. * @dev There can only be one approved address per token at a given time. + * @dev Can only be called by the token owner or an approved operator. * @param _to address to be approved for the given token ID * @param _tokenId uint256 ID of the token to be approved */ - function approve(address _to, uint256 _tokenId) public onlyOwnerOf(_tokenId) { + function approve(address _to, uint256 _tokenId) public { address owner = ownerOf(_tokenId); require(_to != owner); + require(msg.sender == owner || isApprovedForAll(owner, msg.sender)); + if (getApproved(_tokenId) != 0 || _to != 0) { tokenApprovals[_tokenId] = _to; Approval(owner, _to, _tokenId); diff --git a/test/token/ERC721/ERC721BasicToken.behaviour.js b/test/token/ERC721/ERC721BasicToken.behaviour.js index 210678111..afab5a8d3 100644 --- a/test/token/ERC721/ERC721BasicToken.behaviour.js +++ b/test/token/ERC721/ERC721BasicToken.behaviour.js @@ -287,6 +287,30 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) { const to = accounts[1]; let logs = null; + + const itClearsApproval = function () { + it('clears approval for the token', async function () { + const approvedAccount = await this.token.getApproved(tokenId); + approvedAccount.should.be.equal(ZERO_ADDRESS); + }); + }; + + const itApproves = function (address) { + it('sets the approval for the target address', async function () { + const approvedAccount = await this.token.getApproved(tokenId); + approvedAccount.should.be.equal(address); + }); + }; + + const itEmitsApprovalEvent = function (address) { + it('emits an approval event', async function () { + logs.length.should.be.equal(1); + logs[0].event.should.be.eq('Approval'); + logs[0].args._owner.should.be.equal(sender); + logs[0].args._approved.should.be.equal(address); + logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + }); + }; describe('when clearing approval', function () { describe('when there was no prior approval', function () { @@ -294,10 +318,7 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) { ({ logs } = await this.token.approve(ZERO_ADDRESS, tokenId, { from: sender })); }); - it('clears the approval for that token', async function () { - const approvedAccount = await this.token.getApproved(tokenId); - approvedAccount.should.be.equal(ZERO_ADDRESS); - }); + itClearsApproval(); it('does not emit an approval event', async function () { logs.length.should.be.equal(0); @@ -310,18 +331,8 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) { ({ logs } = await this.token.approve(ZERO_ADDRESS, tokenId, { from: sender })); }); - it('clears the approval for that token', async function () { - const approvedAccount = await this.token.getApproved(tokenId); - approvedAccount.should.be.equal(ZERO_ADDRESS); - }); - - it('emits an approval event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.eq('Approval'); - logs[0].args._owner.should.be.equal(sender); - logs[0].args._approved.should.be.equal(ZERO_ADDRESS); - logs[0].args._tokenId.should.be.bignumber.equal(tokenId); - }); + itClearsApproval(); + itEmitsApprovalEvent(ZERO_ADDRESS); }); }); @@ -331,18 +342,8 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) { ({ logs } = await this.token.approve(to, tokenId, { from: sender })); }); - it('sets the approval for that token', async function () { - const approvedAccount = await this.token.getApproved(tokenId); - approvedAccount.should.be.equal(to); - }); - - it('emits an approval event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.eq('Approval'); - logs[0].args._owner.should.be.equal(sender); - logs[0].args._approved.should.be.equal(to); - logs[0].args._tokenId.should.be.bignumber.equal(tokenId); - }); + itApproves(to); + itEmitsApprovalEvent(to); }); describe('when there was a prior approval to the same address', function () { @@ -351,18 +352,8 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) { ({ logs } = await this.token.approve(to, tokenId, { from: sender })); }); - it('keeps the approval for that token', async function () { - const approvedAccount = await this.token.getApproved(tokenId); - approvedAccount.should.be.equal(to); - }); - - it('emits an approval event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.eq('Approval'); - logs[0].args._owner.should.be.equal(sender); - logs[0].args._approved.should.be.equal(to); - logs[0].args._tokenId.should.be.bignumber.equal(tokenId); - }); + itApproves(to); + itEmitsApprovalEvent(to); }); describe('when there was a prior approval to a different address', function () { @@ -371,18 +362,8 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) { ({ logs } = await this.token.approve(to, tokenId, { from: sender })); }); - it('sets the approval for that token', async function () { - const approvedAccount = await this.token.getApproved(tokenId); - approvedAccount.should.be.equal(to); - }); - - it('emits an approval event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.eq('Approval'); - logs[0].args._owner.should.be.equal(sender); - logs[0].args._approved.should.be.equal(to); - logs[0].args._tokenId.should.be.bignumber.equal(tokenId); - }); + itApproves(to); + itEmitsApprovalEvent(to); }); }); @@ -405,6 +386,17 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) { }); }); + describe('when the sender is an operator', function () { + const operator = accounts[2]; + beforeEach(async function () { + await this.token.setApprovalForAll(operator, true, { from: sender }); + ({ logs } = await this.token.approve(to, tokenId, { from: operator })); + }); + + itApproves(to); + itEmitsApprovalEvent(to); + }); + describe('when the given token ID does not exist', function () { it('reverts', async function () { await assertRevert(this.token.approve(to, unknownTokenId, { from: sender }));