ERC4907

[Vectorized]

https://eips.ethereum.org/EIPS/eip-4907

@emojidao @0xanders

Notes:

• File is called ERC4907A.sol because it inherits from ERC721A.sol.

• The ERC-4907 standard is a fully valid extension of ERC-721.

• User info is retained after transfer, unlike the reference implementation which resets it after each transfer to a different address.

  • The standard permits the user info to remain the same upon transfer.

  • Retaining the user info by default saves more gas overall.

  • The new owner can choose to reset the user if they want.

  • Good for use cases such as retaining user info while sending the token to a personal cold wallet.

  • The _beforeTokenTransfers function can be overriden by devs to alter this behavior.

• Unlike the reference implementation, we do not have an internal mapping of structs.

  • Instead, we provide a _explicitUserOf which returns the address of the user, regardless of expiry status.

Some minor changes to ERC721A.sol:

• Add _isApprovedOrOwner(address spender, uint256 tokenId).

• Renamed private function _isOwnerOrApproved to _isSinglyApprovedOrOwner for more clarity.

[Vectorized]

Explanation

First, we know that:

packed = (expires << 160) | user

The straightforward way of implementing block.timestamp <= expires is:

!(block.timestamp > (packed >> 160)) which in Yul, is: iszero(gt(timestamp(), shr(160, packed)))

---

Now, notice that if user != address(0):

(block.timestamp << 160) < ((expires << 160) | user) is true for block.timestamp <= expires.

And when user == address(0):

address(uint160((expires << 160) | user)) evaluates to address(0) anyway.

So that's how we avoid the extra iszero opcode.

---

Having an optimized userOf function is important for on-chain verification by third party contracts.

[0xanders]

// for gas savings, the following lines of code can be deleted
// if the new owner want to change the user to zero address ,
// the new owner could call setUser

        bool mayNeedClearing;
        assembly {
            // Branchless `!(from == address(0) || from == to)`.
            // Saves 60+ gas.
            // The addresses are masked with `_BITMASK_ADDRESS` to
            // clear any non-zero excess upper bits.
            mayNeedClearing := iszero(
                or(
                    // Whether it is a mint (i.e. `from == address(0)`).
                    iszero(and(from, _BITMASK_ADDRESS)),
                    // Whether the owner is unchanged (i.e. `from == to`).
                    eq(and(from, _BITMASK_ADDRESS), and(to, _BITMASK_ADDRESS))
                )
            )
        }

        if (mayNeedClearing) {
            // If either `user` or `expires` are non-zero.
            if (_packedUserInfo[startTokenId] != 0) {
                delete _packedUserInfo[startTokenId];
                emit UpdateUser(startTokenId, address(0), 0);
            }
        }

[Vectorized]

Ah. It’s just to stay faithful to the original implementation.

I personally feel that resetting user upon transfer like the original code is a good default behaviour.

But I also can see very valid use cases for keeping the user by default.

Let me think again with the team if it is better to reset or keep the user by default.

[Vectorized]

@cygaar After some marinating, I'm feeling it's better to remove the auto user reset, as suggested.

Let the users add it on their own if they want.

[0xanders]

Awesome work, thanks @Vectorized

[0xanders]

Suggestion: Add an readme file erc4907a.md to the docs folder.

[Vectorized]

@0xanders Definitely. Readying a separate PR for that soon. :)

Thanks for your work on the EIP. Simple, practical, elegant.

[Vectorized]

I think we can remove the _isApprovedOrOwner.

I forgot we removed it cuz it can be easily substituted with other functions.

[Vectorized]

I think I'll undo any changes to ERC721A.sol in this PR, and move them over to PR #377 instead.

[cygaar]

One comment but otherwise lgtm. We should add in docs in the future

[cygaar]

Let's add a comment here mentioning the gas savings of not having branching similar to what we have in 721A

[Vectorized]

The if-if-if will branch in this case.

The innards of ERC721A uses assembly to compute msg.sender == owner || msg.sender == approvedAddress branchlessly for transfers and burns.
In those very specific scenarios, we have approvedAddress already loaded for determining if we need to clear the slot, so we may as recycle it in a branchless way.

Outside of transfers and burns (i.e. if we don't need to decide whether to clear the approvedAddress slot),
an if-if-if will be better, since it is way more likely for users to approve all tokens for an operator, than approve on a per-token-basis.

[cygaar]

What's the reason behind just making it 3 if statements instead of a single if?

[Vectorized]

The following actually costs more gas

if (_msgSenderERC721A() == owner || 
    isApprovedForAll(owner, _msgSenderERC721A() ||
    getApproved(tokenId) == _msgSenderERC721A())