Optimized ERC721 transfers.

[nventuro]

Credit goes to @abandeali1 for coming up with the original idea in #1031.

Because Solidity doesn't inline internal calls, it misses the opportunity to optimize consecutive SSTOREs to the same location, leading to extra gas costs in the current ERC721 implementation (where transfer is implemented as removeFrom followed by addTo). This PR adds a new function that encapsulates this behavior, implementing the optimization directly on the source code.

[nventuro]

For reference, a call to transferFrom has an execution cost of 56337 gas before this PR, and 35520 gas after.

[frangio]

Save for my Transfer comment above, the logic LGTM.

[nventuro]

Coverage dropped since a condition of remove was only being checked in burn and not transfer: I'll add the necessary tests. Due to #1148 this may be a bit painful, we might want to prioritize working on it sometime soon (2.2.0?).

I'll check coveralls and see if we're missing a setting, since this should've been a test failure.

[frangio]

Definitely. Created a v2.2 milestone.

[nventuro]

@frangio please review the new _transferFrom function. Coverage will probably remain below 100%, I'll review that once the results are in.

[nventuro]

Created #1541 to track future work that could be done here (don't want to increase the scope of this PR).

[frangio]

👍 to _transferFrom.

I have huge doubts about the new _removeTokenFromOwnerEnumeration function. See comments below.

[frangio]

There seems to be missing coverage in ERC721. https://coveralls.io/jobs/42933704/source_files/440498540

[nventuro]

We should be good now - due to the original refactor _clearApprovals had a bizarre (and redundant) require that wasn't being tested, getting rid of it keeps the original public/internal API the same.

@frangio should be ready to merge now.

[frangio]

Nice work!