Preventing Race-to-Empty

Our Banks withdraw function looks like this:

  // This is wrong and an anti-example. Don't ever do this
  function worstWithdraw() {
    msg.sender.call.value(balances[msg.sender])();
    balances[msg.sender] = 0;
  }

What can we do to fix this?

Well, your particular application is unlikely to be exactly this Bank, and so there are a few different things we need to consider.

Proper Ordering

The first thing we could try is switch the two lines so that we zero out the balance before we try to send the funds:

  function withdraw() {
    balances[msg.sender] = 0;
    msg.sender.call.value(balances[msg.sender])();
  }

But of course, that won’t work because we zeroed out balance which means the second line will always be zero! So we need to store the intermediate balance in a variable like this:

  function withdraw() {
    var balance = balances[msg.sender];
    balances[msg.sender] = 0;
    msg.sender.call.value(balance)();
  }

Although, this will prevent against our previous attack, it may cause problems for legitimate users in the case where the .call fails.

If the .call fails, you may zero out the internal balance for that account, without the Ether funds being transferred on the blockchain.

If .call fails, it doesn’t revert changes – you have to check the return value. That is, .call returns a boolean true or false, depending on if the call succeeded. If it didn’t succeed you need to revert your changes: