Solidity Payable Functions - Discussion

As mentioned in the payable function video, how come the function will be pay without the " balance[msg.sender] = msg.value; " . It is mentioned in the video that just writing ‘payable’ in the function header the function will make the payment, but how? Without adding the msg.value in the balance[address] how will the value be added? Any explanation and help is appreciated.

1 Like

Hi @KennethJP,

Apologies in the delay in giving you some feedback on this.

You have added all of the additional lines of code needed to solve the problem with the withdraw function. However there are also a few issues which need resolving:

(1) We should not mark the withdraw function as payable , because it does not need to receive ether from an external address in order to update the contract balance. If we made it payable , then, if the caller sent an ether value to the withdraw function by mistake, as well as calling it with a withdrawal amount, then this ether would be added to the contract balance and potentially lost to the user, because there is no accounting adjustment made to the mapping in the withdraw function for such a deposit (only for withdrawals). However, the function is much securer if we leave it as non-payable, because then, if the caller made the same mistake, the function would revert, and the ether would not be sent.

(2) At the moment, if you only use the withdraw() function to withdraw an amount (its intended purpose), then your withdrawal event will log 0 for the amount withdrawn. This is because msg.value references an ether value that is sent to a payable function and is automatically added to the contract address balance. The emit statement should reference the amount parameter, instead. When you restrict the function to non-payable, using msg.value will throw a compiler error, anyway.
Apart from this, your withdrawal event and corresponding emit statement are both correct, and the emit statement is placed in the correct position within the function body.

(3) Your transfer event and corresponding emit statement are also correctly coded, and the emit statement will log relevant information about a transfer. However, events should only be emitted after the event itself (here, the transfer) has occurred. Therefore, the emit statement should be placed at the end of your transfer() function, after the assert() statement.

(4) Finally, have a look at this post which explains an important security modification that should be made to the order of the statements within your withdraw function body.

Let me know if anything is unclear, or if you have any questions :slight_smile:

Hi @Flippy,

Apologies for the delay in giving you some feedback on this assignment.

You have added all of the additional lines of code needed to solve the problem with the withdraw function :ok_hand:

That’s because you are using Solidity v0.8. That’s absolutely fine, but then you also need to bear in mind that this course material is based on v0.7 syntax. The compiler will always generate an error when your syntax needs to be changed for v0.8, and the error message will tell you why, and often what you should change it to.
In order to receive ether, an address needs to be payable . In versions prior to v0.8 msg.sender is payable by default and so doesn’t need to be explicitly converted. However, from v0.8 msg.sender is non-payable by default, and so when it needs to refer to a payable address, it needs to be explicitly converted. One of the ways to do this is using payable() , which is what you’ve done:

payable(msg.sender).transfer(amount);

Well done for working this out :ok_hand:


You’ve modified the withdraw function with the onlyOwner modifier, but this means that only the contract owner can withdraw funds up to their own individual balance. But the contract allows multiple addresses to deposit funds (we are simulating a bank with bank account holders) and so it only seems fair that all users should be able to withdraw their funds as well, don’t you think? :sweat_smile:

Also, have a look at this post which explains an important security modification that should be made to the order of the statements within your withdraw function body.

Just let me know if you have any questions :slight_smile:

1 Like

Hi @sohamdey_2006,

Apologies for the delay in answering your question…

Any function marked payable will automatically add to the contract address balance any ether/wei value sent to the function when it is called. This happens without us having to add any further code for this. This is what happens in our deposit() function.

As well as the ether/wei value being added to the contract address balance automatically,
the line   balance[msg.sender] += msg.value;   does then add the same ether/wei value to the individual user’s balance in the mapping, in order to keep a track of their share of the total funds held in the contract.

function deposit() public payable returns(uint) {
    balance[msg.sender] += msg.value;
    emit Deposit(msg.value, msg.sender);
    return balance[msg.sender];
}

What may be confusing you is that all ether deposited into the contract is held altogether as one single contract address balance. This contract balance is different to each individual user’s balance stored in the mapping. However, at any one time, they should all add up to the contract balance. These individual balances are like accounting entries which enable the contract to keep track of each individual user’s share of the total pooled funds held in the contract balance.

When we make a function payable , Solidity automatically makes msg.value available to us as a way to reference any amount of ether that is sent to the function when it is called. Effectively, msg.value is a “pre-defined parameter”, which already comes “built-in” to functions we write and mark as payable . In a similar way to msg.sender (which already comes “built-in” to every function we write) we don’t have to explicitly declare msg.value as a parameter within the parentheses in the function header like we do with other values we want to pass to a function.

I hope this clarifies things for you. But just let me know if anything is still unclear, or if you have any further questions :slight_smile:

function withdraw(uint amount) public returns (uint) {
        require(balance[msg.sender] >= amount, "Insufficient funds");    
            
        msg.sender.transfer(amount);
        emit withdrawDone(amount, msg.sender);
        balance[msg.sender] -= amount;
        return balance[msg.sender];
}
1 Like

Nice solution @Jacky_Li :ok_hand:

You have added all of the additional lines of code needed to solve the problem with the withdraw function.

Now have a look at this post which explains an important security modification that should be made to the order of the statements within your function body.

Your additional emit statement looks well-coded, but can we also see the corresponding event declaration, which I’m assuming you’ve added at the top of the contract with the other event declarations.

Just let me know if you have any questions :slight_smile:

I’ve tried to solve the withdrawal problem with a require function.
When testing I was not able to withdraw more than the balance, but I would appreciate someone having a look to confirm it does exactly what we want :slight_smile:

 function withdraw(uint amount) public returns (uint) {
        require(balance[msg.sender]>= amount);
        //msg.sender is an address
        msg.sender.transfer(amount);
        
    }

hi. how can one check the totoal amount of ETH in the Bank contract using Remix? I only managed to get the balance by address.

1 Like

Hi Claudio,

address(this).balance

You can implement a getter to retrieve the contract’s account balance:

function getContractBalance() public view returns(uint) {
    return address(this).balance;
}

This is a very useful thing to implement, because it helps a lot with testing.

Just let me know if you have any further questions :slightly_smiling_face:

2 Likes

Hi @Konzaih,

The require statement you’ve added is correct, but your withdraw function is missing 2 other lines of code…

(1) If you deploy your contract, make a deposit, and then call your withdraw function as it is, you will notice that the caller’s external wallet address receives the requested withdrawal amount, but their balance in the mapping is not reduced (you can call getBalance to check this). This means that, while they cannot withdraw more than their balance for their 1st withdrawal, they can make repeat withdrawals — each one no greater than their initial balance, which isn’t being reduced — up to the total amount of ether held in the contract (the sum of all of the individual account holders’ balances). This would obviously be a very serious bug.

msg.sender.transfer(amount) transfers ether from the contract address balance to the caller’s external wallet address, but it doesn’t adjust the individual user balances in the mapping. These balances perform an internal accounting role, and record each user’s share of the contract’s total ether balance. So, just as we increase a user’s individual balance when they deposit ether in the contract, we need to do the opposite whenever they make a withdrawal.

(2) Notice that the withdraw function header also includes returns(uint) . This is not mandatory to include, and the function can still operate effectively without returning a value. But if you include it in the function header, you should also include a return statement in the function body. If you don’t, the compiler will give you a yellow/orange warning for this.

Let me know if anything is unclear, or if you have any further questions :slight_smile:

2 Likes

thank you! very helpful

1 Like

Hi Everyone, I’m a bit confused as to why the deposit function is payable but not the witdraw function. Is the contract not sending Ether back to msg.sender?

Hey @DavidV, hope you are well.

The payable keyword is used on any function that must receive funds in ETH, while in the withdraw function there is no need for it because is not receiving, instead it will sent ETH out of the contract.

Carlos Z

Hi Carlos, thank you for that. I’m still not quite clear though… sorry…

For clarity I’m looking at the bank contract provided by Filip :


pragma solidity 0.8.0;
pragma abicoder v2;
import "./Ownable.sol";
import "./SafeMath.sol";

contract Bank is Ownable {
    
    using SafeMath for uint256;
    
    mapping(address => uint) balance;
    address[] customers;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    function deposit() public payable returns (uint)  {
        balance[msg.sender] = balance[msg.sender].add(msg.value);
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender];
    }
    
    function withdraw(uint amount) public onlyOwner returns (uint){
        require(balance[msg.sender] >= amount);
        balance[msg.sender] = balance[msg.sender].sub(msg.value);
        payable(msg.sender).transfer(amount);
        return balance[msg.sender];
    }
    
    function getBalance() public view returns (uint){
        return balance[msg.sender];
    }
    
      function contractBalance() public view returns (uint){
        return address(this).balance;
    }
    
    
    function transfer(address recipient, uint amount) public {
        require(balance[msg.sender] >= amount, "Balance not sufficient");
        require(msg.sender != recipient, "Don't transfer money to yourself");
        
        uint previousSenderBalance = balance[msg.sender];
        
        _transfer(msg.sender, recipient, amount);
        
        assert(balance[msg.sender] == previousSenderBalance - amount);
    }
    
    function _transfer(address from, address to, uint amount) private {
        balance[from] -= amount;
        balance[to] += amount;
    }
    
}

I assume for the ETH to move from the contract to the msg.sender address this method

 payable(msg.sender).transfer(amount);

is responsible for moving the ETH and the internal balance is handled by the balance mapping

mapping(address => uint) balance;

Correct?

I have a problem though:

Here is the withdrawal function from the bank contract that Filips provided, however without making it payable I get an error :

  function withdraw(uint amount) public onlyOwner returns (uint){
        require(balance[msg.sender] >= amount);
        balance[msg.sender] = balance[msg.sender].sub(msg.value);
        payable(msg.sender).transfer(amount);
        return balance[msg.sender];
    }
    

This is the error.

TypeError: “msg.value” and “callvalue()” can only be used in payable public functions. Make the function “payable” or use an internal function to avoid this error.
–> Dawie_Laptop/Remix Exercises/BANK 2.0/Bank_Fillip.sol:24:55:
|
24 | balance[msg.sender] = balance[msg.sender].sub(msg.value);
| ^^^^^^^^^

If you have some insight for me I’d much appreciate it. Obviously, this is a fundamental part of Solidity that I want to fully understand.

Hey @thecil,

Thank you for that. I’m still not quite clear though… sorry…

For clarity I’m looking at the bank contract provided by Filip :

pragma solidity 0.8.0;
pragma abicoder v2;
import "./Ownable.sol";
import "./SafeMath.sol";

contract Bank is Ownable {
    
    using SafeMath for uint256;
    
    mapping(address => uint) balance;
    address[] customers;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    function deposit() public payable returns (uint)  {
        balance[msg.sender] = balance[msg.sender].add(msg.value);
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender];
    }
    
    function withdraw(uint amount) public onlyOwner returns (uint){
        require(balance[msg.sender] >= amount);
        balance[msg.sender] = balance[msg.sender].sub(msg.value);
        payable(msg.sender).transfer(amount);
        return balance[msg.sender];
    }
    
    function getBalance() public view returns (uint){
        return balance[msg.sender];
    }
    
      function contractBalance() public view returns (uint){
        return address(this).balance;
    }
    
    
    function transfer(address recipient, uint amount) public {
        require(balance[msg.sender] >= amount, "Balance not sufficient");
        require(msg.sender != recipient, "Don't transfer money to yourself");
        
        uint previousSenderBalance = balance[msg.sender];
        
        _transfer(msg.sender, recipient, amount);
        
        assert(balance[msg.sender] == previousSenderBalance - amount);
    }
    
    function _transfer(address from, address to, uint amount) private {
        balance[from] -= amount;
        balance[to] += amount;
    }
    
}

I assume for the ETH to move from the contract to the msg.sender address this method

 payable(msg.sender).transfer(amount);

is responsible for moving the ETH and the internal balance is handled by the balance mapping

mapping(address => uint) balance;

Correct?

I have a problem though:

Here is the withdrawal function from the bank contract that Filips provided, however without making it payable I get an error :

  function withdraw(uint amount) public onlyOwner returns (uint){
        require(balance[msg.sender] >= amount);
        balance[msg.sender] = balance[msg.sender].sub(msg.value);
        payable(msg.sender).transfer(amount);
        return balance[msg.sender];
    }
    

This is the error.

TypeError: “msg.value” and “callvalue()” can only be used in payable public functions. Make the function “payable” or use an internal function to avoid this error.
–> Dawie_Laptop/Remix Exercises/BANK 2.0/Bank_Fillip.sol:24:55:
|
24 | balance[msg.sender] = balance[msg.sender].sub(msg.value);
| ^^^^^^^^^

If you have some insight for me I’d much appreciate it. Obviously, this is a fundamental part of Solidity that I want to fully understand.

Correct! :nerd_face:

That error comes from this change, since solidity version 0.8.0
https://docs.soliditylang.org/en/latest/080-breaking-changes.html#interface-changes

image

image

Carlos Z

Hi Guys,
I just was studying payable function from Etth101.
I did similar contract as Filip on remix and has different output after deploying it.
I send ss of both mine and his o output after deploying:
image

image

1)In my case I have to write the amount next to the red button “deposit” in the Deployed contract and his write the amount above in the Value section and he could choose the unit wei or ETH, I couldn’t.
2)When I deposited Eth the amount on my account next to the address was the same 99…, in his case the value of Eth decreased.

Could anyone explain me that? Or tell what did i do wrong?

1 Like

Good, Fine and Okay.
I got it.
That’s just because decelerate the deposit function with parametr.

But now I have another questions about payable function:
How i could call payable function with a value if the function does not take any arguments?
Yes, We can run transactions and pass there the value we would like to call the payable function with in the Remix IDE.
But how it would be if not Remix?
Do we always need a software e g MetaMask to call such a function?
If Yes, how the program, software, wallet like MetaMask call non parametric function with an argument - a value - in our case amount of ETH?
And how it was before first web3 software - blockchain handler software on Blockchain?

1 Like

Correct, it was an extra parameter that you dont need.

payable allows to use msg.value to receive ethers.

In Ethereum Smart Contract Programming 201, you will learn how to use truffle, which is another tool to compile solidity contracts.

To interact with dapps, yes.

through web3.js library.

Connection to nodes directly.

Carlos Z

Thank you very much for your answers.
I wonder how does calling payable function works when connecting nodes directly.
Could you explain or tell where I could find explanation?

Kacper Pajak

1 Like