Virtual removed from safeTransferFrom in ERC721Upgradeable

Support Contracts
1

Why was virtual removed from safeTransferFrom function in ERC721Upgradeable

Looks like it happened in this commit

This is preventing this safeTransferFrom function from being overridden.

:1234: Code to reproduce

function safeTransferFrom(address from, address to, uint256 tokenId)
    public
    override (ERC721Upgradeable, IERC721) onlyAllowedOperator(from) {
        super.safeTransferFrom(from, to, tokenId);
    }

:computer: Environment

Hardhat

2

It calls a function of the same name, which receives the same input arguments in the same order, alongside an additional bytes memory data input argument:

function safeTransferFrom(address from, address to, uint256 tokenId) public {
    safeTransferFrom(from, to, tokenId, "");
}

function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public virtual {
    transferFrom(from, to, tokenId);
    _checkOnERC721Received(from, to, tokenId, data);
}

As you can see, that other function IS virtual, so you can override it instead and change the behavior of the original function as desired.

1 Like
3

Ahh I see my mistake thank you for clarifying.

1 Like
4

We kept it this way exactly to avoid a situation where you may override the safeTransferFrom(address from, address to, uint256 tokenId) version without overriding the other. It may be a critical bug!

2 Likes
5

Thanks for clarification

6

As both are with same name, so only the number of arguments will differentiate the two functions ?? And ofc we have to override the function with virtual keyword in it, so why they kept other functions eliminating argument of bytes memory data ??

7

It's mandated by the specification:

/// @notice Transfers the ownership of an NFT from one address to another address
/// @dev Throws unless `msg.sender` is the current owner, an authorized
///  operator, or the approved address for this NFT. Throws if `_from` is
///  not the current owner. Throws if `_to` is the zero address. Throws if
///  `_tokenId` is not a valid NFT. When transfer is complete, this function
///  checks if `_to` is a smart contract (code size > 0). If so, it calls
///  `onERC721Received` on `_to` and throws if the return value is not
///  `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`.
/// @param _from The current owner of the NFT
/// @param _to The new owner
/// @param _tokenId The NFT to transfer
/// @param data Additional data with no specified format, sent in call to `_to`
function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable;

/// @notice Transfers the ownership of an NFT from one address to another address
/// @dev This works identically to the other function with an extra data parameter,
///  except this function just sets data to "".
/// @param _from The current owner of the NFT
/// @param _to The new owner
/// @param _tokenId The NFT to transfer
function safeTransferFrom(address _from, address _to, uint256 _tokenId) external payable;

We just adhere to it.

1 Like