1

There seem to be two different ways to write a withdrawal function to get ETH funds out of a contract you've deployed. I'm wondering which is the better - or "safer" way to do this (in 2022.)

Here are the two versions of the withdrawal methods that I've come across:

// Version 1: 
function withdraw() public onlyOwner {
   uint balance = address(this).balance;
   msg.sender.transfer(balance);
}

// Version 2:
function withdrawMoney() external onlyOwner {
   (bool success, ) = msg.sender.call{ value: address(this).balance } ("");
   require(success, "Transfer failed.");
}

I'm also noticing that neither version seems to handle or usereentrancy. Is this because both use the onlyOwner modifier, so there's no need to worry about reentrancy?

(I'd obviously be curious about WHY it'd be better to go with one of these methods over the other.)

Mark55
  • 1,011
  • 2
  • 13
  • 27

1 Answers1

3

No, the use of the onlyOwner doesn’t necessary guard against reentrancy attacks. Let’s say you have a public function called setOwner that sets the caller as the owner, then by calling the setOwner function, the caller becomes the owner and an reentrancy attack can still occur.

The second method is the recommend way to send ETH to a contract. The empty argument (“”) triggers the fallback function of receiving address. By using call, one can also trigger other functions defined in the contract and send a fixed amount of gas to execute the function. The transaction status is sent as a boolean and the return value is sent in the data variable.

See:

https://medium.com/coinmonks/solidity-transfer-vs-send-vs-call-function-64c92cfc878a

<address>.send vs <address>.transfer best practice usage?

Yongjian P.
  • 4,170
  • 1
  • 3
  • 10
  • OK, you wrote "The second method is the recommend way to send ETH to a contract" - so I wanna make sure I understand you correctly 100%. Cause I was asking about sending ETH from a contract, not "to" a contract. And those are very different things. My goal is to WITHDRAW money FROM my contract - meaning to send the money FROM that contract to MY WALLET. Not to send money TO my contract. – Mark55 Nov 20 '22 at 21:54
  • Yes, both transfer and call include the act of withdrawal. This withdrawal can be sending money from contract to contract or contract to wallet. See: https://blog.logrocket.com/smart-contract-development-common-mistakes-avoid/#reentrancy-attacks and https://stackoverflow.com/questions/68588594/how-do-i-send-ether-and-data-from-a-smart-contract-to-an-eoa – Yongjian P. Nov 20 '22 at 23:07