We have always said that we should not send a token directly to a contract and use the deposit function to ensure the contract can validate the deposit.
Would it be safe to use IERC721 receiver to execute a deposit to a smart contract?
Other than the user not being aware of the actual operation he is performing by calling the transfer call directly to a contract. Are there any other security implication related to not using a deposit function and execute the logic on reception of a token.
By implementing IERC721Receiver, your contract can signal that it's aware of ERC721 standard and it's safe to send ERC721 tokens directly to your contract. This allows your contract to receive the tokens, and at the same time do something else as well, like updating the balance of sender or issuing tokens in return.
For depositing ERC20 tokens, we should use deposit function, otherwise the recipient contract has no mechanism of knowing it has received. You can't use IERC721 Receiver to execute the deposit of ERC20 tokens to your contract, because that is only called by transferring ERC721 tokens to your contract.
If your contract is supposed to receive ERC721 tokens and do something in return, the best way is to implement IERC721Receiver, as it allows you to do all in one transaction, rather than setting allowance, and calling a deposit function.
It is accessible but I suppose it should be zero. Here is the code that calls the onERC721Received hook in the receiver contracts when transferring or minting ERC721 tokens.(OpenZeppelin implementation). No ETH is being transferred here so the msg.value will be zero.
function _safeTransfer(
address from,
address to,
uint256 tokenId,
bytes memory data
) internal virtual {
_transfer(from, to, tokenId);
require(_checkOnERC721Received(from, to, tokenId, data), "ERC721: transfer to non ERC721Receiver implementer");
}
function _checkOnERC721Received(
address from,
address to,
uint256 tokenId,
bytes memory data
) private returns (bool) {
if (to.isContract()) {
try IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, data) returns (bytes4 retval) {
return retval == IERC721Receiver.onERC721Received.selector;
} catch (bytes memory reason) {
if (reason.length == 0) {
revert("ERC721: transfer to non ERC721Receiver implementer");
} else {
/// @solidity memory-safe-assembly
assembly {
revert(add(32, reason), mload(reason))
}
}
}
} else {
return true;
}
}
The msg.value would get sent to the token contract and not the contract that implement IERC721Receiver since the transfer call is sent to the token contract itself.
So IERC721Receiver mains goal is to validate that the contract is aware that it received an NFT but nothing replace an actual formal deposit function with values attached to it to execute logic.
The whole point of IERC721Receiver is to provide a mechanism to do the transfer of token and accounting for it (minting/paying back another token) in the same transaction. This can be done when we are dealing with ETH as we can send ETH when we are calling a function in a contract, but wasn't possible with tokens. So if you are writing a contract that receives an NFT and mints or pays back with ERC20 token, onERC721Received hook will be enough, and you won't need a separate deposit function.
I understand that.
The user will see that he is executing a transfer call in the UI(Metamask) when in fact he perform a deposit that might involve more logic.
The logic remain minimal as no value can be attached to this call. (unless we send the value to To define in the transfer)
The main point is return retval == IERC721Receiver.onERC721Received.selector that safeTransfer require or revert so the token don't get stuck.
The address where you are sending the token To could me manipulated in the UI.
I'm not aware or any marketplace that does this. Any example in prod?
Thank you @Mohandes. I made up my mind and will keep going as usual with a formal deposit even if they are NFT's. I understand your point of view on what is defined by the IERCReceiver that can be used to perform certain operation. But it does not totally replace a proper deposit function. I would rather use a permit to skip the approval than have the token sent somewhere.