Skip to content

Conversation

@NimrodHunter
Copy link

@NimrodHunter NimrodHunter commented Mar 6, 2018

Fixes #

🚀 Description

In the function removeToken of ERC721Token contract, if the token that you want remove is the same than the last token of the owner list, ownedTokensIndex[lastToken] = tokenIndex; rewrite the index. to avoid this i added an if condition.

  • 📘 I've reviewed the OpenZeppelin Contributor Guidelines
  • ✅ I've added tests where applicable to test my new functionality.
  • 📖 I've made sure that my contracts are well-documented.
  • 🎨 I've run the JS/Solidity linters and fixed any issues (npm run lint:all:fix).

@facuspagnuolo
Copy link
Contributor

Thanks @NimrodHunter . IMHO I don't see the advantage of saving some gas just for the last token but performing an additional check for the rest of the tokens of the list. Do you agree?

@nventuro
Copy link
Contributor

Agree with @facuspagnuolo, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants