diff --git a/contracts/mocks/ERC721TokenMock.sol b/contracts/mocks/ERC721TokenMock.sol index 4695ad296..5e07f9f5f 100644 --- a/contracts/mocks/ERC721TokenMock.sol +++ b/contracts/mocks/ERC721TokenMock.sol @@ -24,4 +24,8 @@ contract ERC721TokenMock is ERC721Token { function setTokenURI(uint256 _tokenId, string _uri) public { super._setTokenURI(_tokenId, _uri); } + + function _removeTokenFrom(address _from, uint256 _tokenId) public { + super.removeTokenFrom(_from, _tokenId); + } } diff --git a/contracts/mocks/RBACCappedTokenMock.sol b/contracts/mocks/RBACCappedTokenMock.sol index 9de98a893..d14116dd3 100644 --- a/contracts/mocks/RBACCappedTokenMock.sol +++ b/contracts/mocks/RBACCappedTokenMock.sol @@ -3,6 +3,7 @@ pragma solidity ^0.4.24; import "../token/ERC20/RBACMintableToken.sol"; import "../token/ERC20/CappedToken.sol"; + contract RBACCappedTokenMock is CappedToken, RBACMintableToken { constructor( uint256 _cap diff --git a/contracts/mocks/WhitelistMock.sol b/contracts/mocks/WhitelistMock.sol index d6c2eef8e..d4d8e5a3e 100644 --- a/contracts/mocks/WhitelistMock.sol +++ b/contracts/mocks/WhitelistMock.sol @@ -2,6 +2,7 @@ pragma solidity ^0.4.24; import "../access/Whitelist.sol"; + contract WhitelistMock is Whitelist { function onlyWhitelistedCanDoThis() diff --git a/contracts/token/ERC721/ERC721Token.sol b/contracts/token/ERC721/ERC721Token.sol index 4cdfb1ac6..31f808aaf 100644 --- a/contracts/token/ERC721/ERC721Token.sol +++ b/contracts/token/ERC721/ERC721Token.sol @@ -140,17 +140,19 @@ contract ERC721Token is SupportsInterfaceWithLookup, ERC721BasicToken, ERC721 { function removeTokenFrom(address _from, uint256 _tokenId) internal { super.removeTokenFrom(_from, _tokenId); + // To prevent a gap in the array, we store the last token in the index of the token to delete, and + // then delete the last slot. uint256 tokenIndex = ownedTokensIndex[_tokenId]; uint256 lastTokenIndex = ownedTokens[_from].length.sub(1); uint256 lastToken = ownedTokens[_from][lastTokenIndex]; ownedTokens[_from][tokenIndex] = lastToken; - ownedTokens[_from][lastTokenIndex] = 0; + ownedTokens[_from].length--; // This also deletes the contents at the last position of the array + // Note that this will handle single-element arrays. In that case, both tokenIndex and lastTokenIndex are going to // be zero. Then we can make sure that we will remove _tokenId from the ownedTokens list since we are first swapping // 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; } diff --git a/test/token/ERC721/ERC721Token.test.js b/test/token/ERC721/ERC721Token.test.js index ae2c9d859..1cd9240a7 100644 --- a/test/token/ERC721/ERC721Token.test.js +++ b/test/token/ERC721/ERC721Token.test.js @@ -77,6 +77,31 @@ contract('ERC721Token', function (accounts) { }); }); + describe('removeTokenFrom', function () { + beforeEach(async function () { + await this.token._removeTokenFrom(creator, firstTokenId, { from: creator }); + }); + + it('has been removed', async function () { + await assertRevert(this.token.tokenOfOwnerByIndex(creator, 1)); + }); + + it('adjusts token list', async function () { + const token = await this.token.tokenOfOwnerByIndex(creator, 0); + token.toNumber().should.be.equal(secondTokenId); + }); + + it('adjusts owner count', async function () { + const count = await this.token.balanceOf(creator); + count.toNumber().should.be.equal(1); + }); + + it('does not adjust supply', async function () { + const total = await this.token.totalSupply(); + total.toNumber().should.be.equal(2); + }); + }); + describe('metadata', function () { const sampleUri = 'mock://mytoken';