3

As far as I understand msg.sender.send just gives enough gas to the call to do one simple logging and not more, so the contracts which does:

msg.sender.send(winnings)
balances[msg.sender] -= winnings

are safe, or not? Sorry I couldn't find a simple answer to this. I have an old contract and I am not sure if I should update it or if it's okay. Better option would be:

balances[msg.sender] -= winnings
if(!msg.sender.send(winnings)){
    balances[msg.sender] += winnings
}

but is the first one still good?

4

Both send() and transfer() are subject to the 2300 gas stipend. The main difference between them is that transfer throws on error and that transfer() supports the .gas() modifier, allowing you to override the gas being passed.

Look here for further info: https://github.com/ethereum/solidity/issues/610

Nevertheless, if you wanted to be completely safeguarded agains re-entrant attacks, the function should look like you suggested in second place as what you have in first place is vulnerable. transfer/send should be the last thing you do. Plus, send() does not throw/revert, so if the transaction failed you still would have subtracted from the internal balance.

3

You should use transfer() (it will throw if something bad happens and revert all previous operations) and you don´t need to add again value to the balance. Your code would be like this:

balances[msg.sender] -= winnings
msg.sender.transfer(winnings);

If transfer has a problem, it will throw and balances[msg.sender] will get the previous value.

ps: Don´t forget to log an event. It is a good practice.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.