0

Should this function be marked as nonReentrant or is it overkill?

function sendEthToTokenOwner(uint256 _tokenId) external payable nonReentrant {
        address _tokenOwner = ownerOf(_tokenId);
        require(msg.sender != _tokenOwner, "Sender can't be owner");

        uint256 _price = tokenIdToPrice[_tokenId];
        require(msg.value == _price, "Please submit the correct amount of ether");


        (bool success, ) = payable(_tokenOwner).call{value: _price}("");
        require(success, "Eth cannot be transferred");
}
jwarshack
  • 89
  • 1
  • 5

1 Answers1

1

This particular snippet doesn't require a reentrancy protection because the sendEthToTokenOwner() function simply acts as a "transfer proxy". Performing a validation, and then redirecting the msg.value to the token owner.

If you were transferring out amount larger than msg.value or an amount independent on msg.value, then you might need a reentrancy protection (depending on other context).

Petr Hejda
  • 40,554
  • 8
  • 72
  • 100
  • Thanks so much! That is what I was thinking but just wanted confirmation. – jwarshack Feb 06 '22 at 18:57
  • What if the contract takes a percentage of msg.value `uint256 fee = _price / 25;` and then for the transfer `(bool success, ) = payable(_tokenOwner).call{value: _price - fee}("");` I assume it would still hold true that nonReentrant is not needed – jwarshack Feb 08 '22 at 04:14
  • @jwarshack Correct. Because the amount is always smaller than `msg.value`. – Petr Hejda Feb 08 '22 at 08:24