NoBey/openzeppelin-contracts
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ERC721 bugfix + gas optimizations (#1549)

Browse Source 
* Now only swapping when needed.

* Removed _addTokenTo and _removeTokenFrom

* Removed removeTokenFrom test.

* Added tests for ERC721 _mint and _burn

* _burn now uses the same swap and pop mechanism as _removeFromOwner

* Gas optimization on burn
This commit is contained in:
Nicolás Venturo
2018-12-12 18:51:43 -03:00
committed by GitHub
parent 2da19eebd3
commit 12533bcb2b
6 changed files with 170 additions and 124 deletions
Download Patch File Download Diff File Expand all files Collapse all files

107
test/token/ERC721/ERC721.test.js
View File

@ -1,13 +1,112 @@
const { shouldBehaveLikeERC721 } = require('./ERC721.behavior');
require('../../helpers/setup');
const { ZERO_ADDRESS } = require('../../helpers/constants');
const expectEvent = require('../../helpers/expectEvent');
const send = require('../../helpers/send');
const shouldFail = require('../../helpers/shouldFail');
const { shouldBehaveLikeERC721 } = require('./ERC721.behavior');
const ERC721Mock = artifacts.require('ERC721Mock.sol');
require('../../helpers/setup');
contract('ERC721', function ([_, creator, ...accounts]) {
contract('ERC721', function ([_, creator, tokenOwner, anyone, ...accounts]) {
beforeEach(async function () {
this.token = await ERC721Mock.new({ from: creator });
});
shouldBehaveLikeERC721(creator, creator, accounts);
describe('internal functions', function () {
const tokenId = 5042;
describe('_mint(address, uint256)', function () {
it('reverts with a null destination address', async function () {
await shouldFail.reverting(this.token.mint(ZERO_ADDRESS, tokenId));
});
context('with minted token', async function () {
beforeEach(async function () {
({ logs: this.logs } = await this.token.mint(tokenOwner, tokenId));
});
it('emits a Transfer event', function () {
expectEvent.inLogs(this.logs, 'Transfer', { from: ZERO_ADDRESS, to: tokenOwner, tokenId });
});
it('creates the token', async function () {
(await this.token.balanceOf(tokenOwner)).should.be.bignumber.equal(1);
(await this.token.ownerOf(tokenId)).should.equal(tokenOwner);
});
it('reverts when adding a token id that already exists', async function () {
await shouldFail.reverting(this.token.mint(tokenOwner, tokenId));
});
});
});
describe('_burn(address, uint256)', function () {
it('reverts when burning a non-existent token id', async function () {
await shouldFail.reverting(send.transaction(this.token, 'burn', 'address,uint256', [tokenOwner, tokenId]));
});
context('with minted token', function () {
beforeEach(async function () {
await this.token.mint(tokenOwner, tokenId);
});
it('reverts when the account is not the owner', async function () {
await shouldFail.reverting(send.transaction(this.token, 'burn', 'address,uint256', [anyone, tokenId]));
});
context('with burnt token', function () {
beforeEach(async function () {
({ logs: this.logs } =
await send.transaction(this.token, 'burn', 'address,uint256', [tokenOwner, tokenId]));
});
it('emits a Transfer event', function () {
expectEvent.inLogs(this.logs, 'Transfer', { from: tokenOwner, to: ZERO_ADDRESS, tokenId });
});
it('deletes the token', async function () {
(await this.token.balanceOf(tokenOwner)).should.be.bignumber.equal(0);
await shouldFail.reverting(this.token.ownerOf(tokenId));
});
it('reverts when burning a token id that has been deleted', async function () {
await shouldFail.reverting(send.transaction(this.token, 'burn', 'address,uint256', [tokenOwner, tokenId]));
});
});
});
});
describe('_burn(uint256)', function () {
it('reverts when burning a non-existent token id', async function () {
await shouldFail.reverting(send.transaction(this.token, 'burn', 'uint256', [tokenId]));
});
context('with minted token', function () {
beforeEach(async function () {
await this.token.mint(tokenOwner, tokenId);
});
context('with burnt token', function () {
beforeEach(async function () {
({ logs: this.logs } = await send.transaction(this.token, 'burn', 'uint256', [tokenId]));
});
it('emits a Transfer event', function () {
expectEvent.inLogs(this.logs, 'Transfer', { from: tokenOwner, to: ZERO_ADDRESS, tokenId });
});
it('deletes the token', async function () {
(await this.token.balanceOf(tokenOwner)).should.be.bignumber.equal(0);
await shouldFail.reverting(this.token.ownerOf(tokenId));
});
it('reverts when burning a token id that has been deleted', async function () {
await shouldFail.reverting(send.transaction(this.token, 'burn', 'uint256', [tokenId]));
});
});
});
});
});
});

31
test/token/ERC721/ERC721Full.test.js
View File

@ -23,7 +23,6 @@ contract('ERC721Full', function ([
owner,
newOwner,
another,
anyone,
] = accounts;
beforeEach(async function () {
@ -70,36 +69,6 @@ contract('ERC721Full', function ([
});
});
describe('removeTokenFrom', function () {
it('reverts if the correct owner is not passed', async function () {
await shouldFail.reverting(
this.token.removeTokenFrom(anyone, firstTokenId, { from: owner })
);
});
context('once removed', function () {
beforeEach(async function () {
await this.token.removeTokenFrom(owner, firstTokenId, { from: owner });
});
it('has been removed', async function () {
await shouldFail.reverting(this.token.tokenOfOwnerByIndex(owner, 1));
});
it('adjusts token list', async function () {
(await this.token.tokenOfOwnerByIndex(owner, 0)).toNumber().should.be.equal(secondTokenId);
});
it('adjusts owner count', async function () {
(await this.token.balanceOf(owner)).toNumber().should.be.equal(1);
});
it('does not adjust supply', async function () {
(await this.token.totalSupply()).toNumber().should.be.equal(2);
});
});
});
describe('metadata', function () {
const sampleUri = 'mock://mytoken';