gas required exceeds allowance or always failing transaction when I try to transfer ether...

Question

I am trying to play with very simple contract on testnet (ropsten). My contract code is as follows.

contract Bank11{
   mapping(address=>uint) userBalances;
   function getUserBalance(address user) constant returns(uint) {
     return userBalances[user];
   }

   function addToBalance() payable {
     userBalances[msg.sender] = userBalances[msg.sender] + msg.value;
   }
   function withdrawBalance() {
     uint amountToWithdraw = userBalances[msg.sender];
     if (msg.sender.call.value(amountToWithdraw)() == false) {
         throw;
     }
     userBalances[msg.sender] = 0;
   }
}

No problem when I try with run on remix. (Javascript VM Mode) But I've got error every time when I try to call withdrawBalance.

transaction to Bank11.withdrawBalance errored: gas required exceeds allowance or always failing transaction.

I set Gas limit, 3000000. What am I doing wrong??

Answer 1

what about writing the function in this way?

   function withdrawBalance() {
     uint amountToWithdraw = userBalances[msg.sender];
     userBalances[msg.sender] = 0;

     msg.sender.transfer(amountToWithdraw);
   }

transfer will fail in case you don't have enough balance or any other error and will revert the transaction.

http://solidity.readthedocs.io/en/develop/common-patterns.html

Answer 2

It seems that the problem is setting the value part of the mapping to 0.

contract Bank11{
   mapping(address=>uint) userBalances;
   function getUserBalance(address user) constant returns(uint) {
     return userBalances[user];
   }

   function addToBalance() payable {
     userBalances[msg.sender] = userBalances[msg.sender] + msg.value;
   }
   function withdrawBalance() {
     uint amountToWithdraw = userBalances[msg.sender];
     if (msg.sender.call.value(amountToWithdraw)() == false) {
         throw;
     }
     userBalances[msg.sender] = 1;
   }
}

This works fine, 1 wei is nothing even compared to gas fees. It's a quick and dirty fix but your contract needs a lot of cleaning anyway, so if you need a quick fix use that, if you want to make something good and robust there's a few things you should change.

• You shouldn't use if(...){throw;}, use require or assert instead.

• You didn't do anything to send back the ether to the caller, you should add msg.sender.transfer(amountToWithdraw);

• You should let the user specify the amount he wants to withdraw, shouldn't you? In case he doesn't want to withdraw everything or nothing

And more generally, you don't need to reinvent the wheel, if you're looking for a banking contract there are tutorials here and here.