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

Fix clearing of ownedTokensIndex and allTokensIndex in ERC721Token.sol #912

Closed
wants to merge 3 commits into from
Closed

Fix clearing of ownedTokensIndex and allTokensIndex in ERC721Token.sol #912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99cb4f7
Remove redundant whitespaces from ERC721Token.test.js
michald Apr 23, 2018
60de783
Fix clearing of ownedTokensIndex and allTokensIndex in ERC721Token.sol
michald Apr 23, 2018
43e644a
Merge branch 'master' into fix-clearing-of-ownedTokensIndex-and-allTo…
frangio Jun 11, 2018
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
8 changes: 8 additions & 0 deletions contracts/mocks/ERC721TokenMock.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,12 @@ contract ERC721TokenMock is ERC721Token {
function setTokenURI(uint256 _tokenId, string _uri) public {
super._setTokenURI(_tokenId, _uri);
}

function ownedTokenIndex(uint256 _tokenId) public view returns(uint256) {
return ownedTokensIndex[_tokenId];
}

function allTokenIndex(uint256 _tokenId) public view returns(uint256) {
return allTokensIndex[_tokenId];
}
}
15 changes: 10 additions & 5 deletions contracts/token/ERC721/ERC721Token.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,11 @@ contract ERC721Token is ERC721, ERC721BasicToken {
// the lastToken to the first position, and then dropping the element placed in the last position of the list

ownedTokens[_from].length--;
ownedTokensIndex[_tokenId] = 0;
ownedTokensIndex[lastToken] = tokenIndex;
delete ownedTokensIndex[_tokenId];

if (_tokenId != lastToken) {
ownedTokensIndex[lastToken] = tokenIndex;
}
}

/**
Expand Down Expand Up @@ -185,8 +188,10 @@ contract ERC721Token is ERC721, ERC721BasicToken {
allTokens[lastTokenIndex] = 0;

allTokens.length--;
allTokensIndex[_tokenId] = 0;
allTokensIndex[lastToken] = tokenIndex;
}
delete allTokensIndex[_tokenId];

if (_tokenId != lastToken) {
allTokensIndex[lastToken] = tokenIndex;
}
}
}
25 changes: 19 additions & 6 deletions test/token/ERC721/ERC721Token.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,38 @@ contract('ERC721Token', function (accounts) {
});

describe('burn', function () {
const tokenId = firstTokenId;
const tokenId = secondTokenId;
const sender = creator;

beforeEach(async function () {
await this.token.burn(tokenId, { from: sender });
});

it('removes that token from the token list of the owner', async function () {
const token = await this.token.tokenOfOwnerByIndex(sender, 0);
token.toNumber().should.be.equal(secondTokenId);
const firstToken = await this.token.tokenOfOwnerByIndex(sender, 0);
firstToken.toNumber().should.be.equal(firstTokenId);
await assertRevert(this.token.tokenOfOwnerByIndex(sender, 1));
});

it('adjusts all tokens list', async function () {
const token = await this.token.tokenByIndex(0);
token.toNumber().should.be.equal(secondTokenId);
const firstToken = await this.token.tokenByIndex(0);
firstToken.toNumber().should.be.equal(firstTokenId);
await assertRevert(this.token.tokenByIndex(1));
});

['ownedTokenIndex', 'allTokenIndex'].forEach(fnName => {
const mappingName = fnName.replace('Token', 'Tokens');

it(`adjusts ${mappingName}`, async function () {
const firstTokenIndex = await this.token[fnName](firstTokenId);
const secondTokenIndex = await this.token[fnName](secondTokenId);
firstTokenIndex.toNumber().should.be.equal(0);
secondTokenIndex.toNumber().should.be.equal(0);
});
});

it('burns all tokens', async function () {
await this.token.burn(secondTokenId, { from: sender });
await this.token.burn(firstTokenId, { from: sender });
const total = await this.token.totalSupply();
total.toNumber().should.be.equal(0);
await assertRevert(this.token.tokenByIndex(0));
Expand Down