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.

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

removed and tested unecessary gas cost #1017

Merged
merged 5 commits into from
Jul 20, 2018
Merged

Conversation

YZhenY
Copy link
Contributor

@YZhenY YZhenY commented Jun 16, 2018

Fixes #977

🚀 Description

Added tests for behaviour of removeTokenFrom.
Removed redundant line from ERC721 contract to improve gas cost

@come-maiz
Copy link
Contributor

Thanks @YZhenY!
I was doing some tests on the solidity behavior, and confirmed that when you decrease the length of an array and increase it again, the values are reset to zero:
https://github.com/elopio/solidity-experiments/blob/28d622aabea4492032916be9dab0627b15b6118c/test/Arrays.test.js#L62

So, I don't see any reason reset the value before reducing the length, not even clearing traces. I'm +1 here, but can you please revert your changes to package-lock.json?

@nventuro
Copy link
Contributor

I restored the original package-lock.json file and added a clarifying comment, thanks for your contribution @YZhenY!

@nventuro nventuro merged commit 6bd8842 into OpenZeppelin:master Jul 20, 2018
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