chiru-labs / ERC721A

https://ERC721A.org
MIT License
2.5k stars 841 forks source link

Replacement for _safeTransfer / ability to transfer tokens that don't belong to msg.sender? #395

Closed intenex closed 2 years ago

intenex commented 2 years ago

In earlier versions, _safeTransfer allowed for transferring tokens without a check that the token was owned by the msg.sender.

This allowed for useful functionality in a number of cases - specifically, there are two cases we've used this functionality for:

  1. When implementing loan functionality (see MetaAngels https://etherscan.io/address/0xad265ab9b99296364f13ce5b8b3e8d0998778bfb#code) where an owner could lend a token to someone else via a safeTransferFrom and then reclaim it back via a _safeTransfer

  2. When creating SBTs and having an override functionality for an owner or a group of delegates to transfer a token from a compromised or old wallet to a new wallet for a given end user

Is there a way to implement functionality along these lines with 4.2 of ERC721A? I see ERC721A now supports ERC4907, which is great, but given that ERC4907 isn't widely supported just yet for NFT rentals/loans, it would still be nice to specifically be able to support the first use case presented above where a user can actually initiate a real transfer of a token to another user and then reclaim it back themselves without having to rely on the borrower doing so.

If there is no way to implement functionality like this - any ideas on what might be the easiest way to override the existing functionality in ERC721A to support a transfer function without an ownership check?

Thank you!

Vectorized commented 2 years ago

We have inlined all the _transfer functions to chip away at excess gas usage for the majority of use-cases.

If you are using the main non-upgradeable ERC721A, use this function to directly approve the token for transfers by the current message sender:

Make sure ERC721A is the first class in the inheritance, or else this may not work.

contract Bypass is ERC721A {

    constructor() ERC721A("A", "A") {}

    function _directApproveMsgSenderFor(uint256 tokenId) internal {
        assembly {
            mstore(0x00, tokenId)
            mstore(0x20, 6) // `_tokenApprovals` is at slot 6.
            sstore(keccak256(0x00, 0x40), caller())
        }
    }
}

As the transfer will read and clear the slot later, the gas overhead is very minimal,
probably somewhere around 100 to 200 gas.

intenex commented 2 years ago

Whoah - thank you SO much for taking the time to write all of that out @Vectorized! Seriously - you're the best repo maintainer I've ever seen - really appreciate how hands on you are with helping everyone with this repo. Definitely wouldn't have come up with this on my own lol.

So if I'm understanding this correctly, I could just call this function directly ahead of any safeTransferFrom/transferFrom call and it would directly approve the msg.sender()/caller() so that _getApprovedSlotAndAddress(tokenId) would return the msg.sender() as the approvedAddress, thereby passing the _isSenderApprovedOrOwner() call?

So I'd just write something like this:

function retrieveLoanedToken(uint256 tokenId) public {
    if (msg.sender() != originalOwnerOfToken(tokenId)) revert NotOriginalOwner();
    _directApproveMsgSenderFor(tokenId);
    safeTransferFrom(ownerOf(tokenId), msg.sender(), tokenId);
}

And that would just work?

THANK YOU!!! Literal godsend - so insanely helpful. Really appreciate it!

Vectorized commented 2 years ago

yes. It will just work.

intenex commented 2 years ago

You are the best. THANK YOU!!!