Solidity Payable Functions - Discussion

[jon_m]

Hi @Azmil_K,

… and welcome to the forum!

@Azmil_K

when I invoke the getAddress function, I’m getting the address of the account in box ‘1’. i.e. the msg.sender is the account in Box1. Does this mean that the account in Box 1 is actually ‘calling’ the getAddress function ?

Yes … you can change the address showing in the Account field (your “Box 1”) using the dropdown, to simulate different external addresses calling specific functions in your smart contract.

@Azmil_K

If I write the following function to transfer some Eth:

    function getMoney() external payable {

    }

I see that the amount in box 1 diminishes by the value in enter in the ‘VALUE’ box. but where does this Eth go to ?

Ether/wei values sent to a function marked payable are automatically added to the contract address balance.

@Azmil_K

If its going to the contract how can I check this value …

address(this).balance  will give you the contract balance. You can add a getter so that any external address can check this balance e.g.

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

You also need to keep a record of each individual user address’s share of the total contract balance in a mapping. Effectively, this records and tracks each user’s entitlement to withdraw funds from the contract. We show you how to start doing this, with ether transferred into the contract, in one of the lectures in this section of the course, using the deposit() function.

@Azmil_K

… and transfer it back to the account shown in Box 1 ?

This is the withdraw() function task we give you in the Transfer Assignment near the end of this section of the course. Continue with the video lectures, complete the Transfer Assignment, and then, if you still have questions about how an external address can withdraw the ether they have deposited in the contract, let me know and I’ll be glad to help you further.

Also, don’t forget to post your solutions to all of the assignmnets, here in the forum. A link to where you need to post each of your solutions is provided in the relevant lesson of the course. This is important, because we will review your solutions and, where necessary, provide you with useful feedback. The assignments you should have already completed by the beginning of the Payable Functions section of the course are the Data Location Assignment and the Events Assignment.

[Tomaage]

Im not getting the same result as Philip on the lessons Payable and Transfer

I can not put a value in the “value” box, and then push the red deposit button. I need to wrtite the amount in the Deposit button as sown in picture.

The withdraw function also does not seem to deploy

Heres is the code im using

pragma solidity 0.8.7;

contract HelloWorld {

    mapping(address => uint) balance;

    address owner;

    event depositDone(uint amount, address depositedTo);

    modifier onlyOwner {
        require(msg.sender == owner );
        _;  //run the function
    }

    constructor() {
        owner = msg.sender;
    }

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

    function getBalance() public view returns (uint){
        return balance[msg.sender];
    }

    function transfer(address recipient, uint amount) public {
        require(balance[msg.sender] >= amount, "Balance not sufficient");
        require(msg.sender != recipient, "Dont 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;
    }
}

[jon_m]

Hi @Tomaage,

From the screen shot it looks like you still have a different contract deployed to the one actually displayed in the text editor, and you’re calling functions on that other contract instead.

If you look at the panel with the function-call buttons for your deployed contract, your red deposit button has an input field for a uint _toAdd argument. This must be from the previous contract we coded which had the addBalance() function, and where we input unsigned integer values instead of ether values. If you still have that contract deployed, then this will be why it’s not letting you call the deposit function with an ether value in the Value field. As you’ve realised, it requires a uint argument in the input field instead. When you’ve deployed the contract you want to interact with, the red deposit button should appear without any input field next to it, because the deposit() function in this contract doesn’t have any parameters defined within the parentheses immediately after the function name in the function header.

You can also see that your displayed contract has a withdraw() function, but the transfer() function-call button is the only other one displayed in the panel for the contract which is actually deployed.

It’s a good idea to have Auto-compile turned on: mark this option under Compiler Configuration in the Solidity Compiler panel. This will make sure that any errors are highlighed by the compiler while you’re coding. If you don’t have Auto-compile turned on then you must remember to click the blue Compile button, and check that your contract has successfully compiled, before trying to deploy it. If you still have compiler errors then you won’t be able to deploy your contract until you resolve them.

From looking at your code, I can see that the contract you want to deploy won’t compile, even though you’ve used the same code from the video. This is because you’ve chosen to use the more up-to-date Solidity v0.8 (which is only to be encouraged) instead of v0.7 (the course uses v0.7.5). If you try to compile your contract, you will notice that you get a red compiler error for this line of code …

@Tomaage

msg.sender.transfer(amount);

Prior to Solidity v0.8, msg.sender is already a payable address by default, and so doesn’t need to be explicitly converted whenever the syntax requires it to reference a payable address (such as, here, with the transfer method). That’s why the code in the video (based on Solidity v0.7 syntax) uses  msg.sender.transfer(amount);

However, from Solidity v0.8, msg.sender is non-payable by default, which means having to explicity convert it to a payable address when necessary. So, in order to get your contract to compile you need to make the following modification to this line of code …

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

You will find that the only difference in syntax between these two versions of Solidity, which affects the code covered in this course, concerns msg.sender. Paying close attention to any compiler errors that you get will help you to easily identify when you need to explicity convert it using payable()

Also, make sure you always remove your previously deployed contract(s) by clicking on the bin icon to the right of where it says Deployed Contracts; or if you only have one contract deployed, clicking on the cross just below the bin icon will also remove the contract. Then, just before you deploy your new contract, make sure it is the one displayed in the Contract field just below the Value field.

Let me know if you experience any further difficulties, or if you have any questions about any of the issues I’ve raised.

[Tomaage]

Thanks

The

@jon_m

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

made it work.

[Tomaage]

When I deposit 2 eth from adress A, into a function in the smart contract, the ETH arrvies at the smart contract Adress B.
Then I make a call to withdraw the same 2 ETH. I see it arrives at adress A.
Why does the get balance function continue to show 2 ETH? Does it not check the balance that the msg.sender has in the smart contract?

pragma solidity 0.8.11;

import "./Destroyable.sol";

contract Bank is Destroyable {

    mapping(address => uint) balance;

    event depositDone(uint amount, address depositedTo);

    function deposit() public payable returns (uint) {
        balance[msg.sender] += 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);
        payable(msg.sender).transfer(amount);
        return balance[msg.sender];
    }

    function getBalance() public view returns (uint){
        return balance[msg.sender];
    }

    function transfer(address recipient, uint amount) public {
        require(balance[msg.sender] >= amount, "Balance not sufficient");
        require(msg.sender != recipient, "Dont 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;
    }
}

[jon_m]

Hi @Tomaage,

I’ve answered this question in my feedback for your Inheritance Assignment (see below), because I saw your withdraw() function there first. Point (2) specifically addresses the issue you’ve raised here…

@Tomaage

Why does the get balance function continue to show 2 ETH? Does it not check the balance that the msg.sender has in the smart contract?

Pay particular attention to the sentences that I’ve highlighted.

Point (1) addresses another issue with your withdraw() function that you should also consider.

@jon_m

FEEDBACK ON YOUR WITHDRAW FUNCTION (Transfer Assignment)

(1) You’ve modified the withdraw() function with the onlyOwner modifier, but this means that only the contract owner will be able to withdraw funds while the Bank contract is deployed and operating normally. The contract allows multiple addresses to deposit funds (we are simulating a bank with bank account holders) and so, while the contract is still deployed, it only seems fair that all users should be able to withdraw their funds as well, don’t you think?

(2) The require statement and the return statement you’ve added to your withdraw function are both correct But you are missing one very important line of code…

If you deploy your contract, make a deposit, and then call your withdraw function, you will notice that the caller’s address receives the requested withdrawal amount, but their balance in the mapping is not reduced (call getBalance to check this). I’m sure you can see that this is a very serious bug because, even though there is a limit on each separate withdrawal, this limit is never reduced to reflect each withdrawal. This means that a user can withdraw more than their individual balance (the share of the total contract balance to which they are entitled).

payable(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 (effectively, each user’s entitlement to withdraw ether from the contract). 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.

Once you’ve had a go at correcting your code for this, have a look at this post which explains the importance of the order of the statements within your withdraw function body.

I think you will find it helpful to also add the following getter to your Bank contract, which will return the contract address balance. You can then see more clearly how this changes in relation to the individual user balances in the mapping (which are returned by the getBalance function).

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

When you’re ready, post your corrected solution and I’ll check that you’re made the right changes. And just let me know if you have any further questions.

---

And don’t forget to post your solution to the Events Assignment here. It looks like you may have missed it, because you are missing a Transfer event in your code, and a corresponding emit statement in the transfer() function. This assignment is from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course.

[Tomaage]

1 A Bank that anybody can deposit to, but only I can withdraw from? I kinda like it, but I see why it would not be popular. So remove onlyOwner

2 So since I was missing the code

balance[msg.sender]-=amount;

in my withdraw function, I was not telling the function to update the smart contracts internal balance of this particular msg.sender . So they could withdraw all the eth stored in the contract. Even eth from other adresses. When one of those adresses came and wanted to withdraw their ETH, the contract balance would be 0, but would their getBalance function still show what they had put in?

Some more question for my understanding. That transfer function I have there. It only transfers the value internaly in the smart contract, and updates the internal balance. It does not send it out to that recipients external wallet? So this bank only lets you use its internal system to send to somebody else. If they want the money, they neeed to call the bank with the withdrawl function? They would not know that you sent it to them in this contract unless you told them, I they would not have the money should the smart contract be hacked.
To make it transfer out of the contract to the recipient external wallet; I would have to make it a payable function? I tried messing around, but could not really make it work

[jon_m]

Hi @Tomaage,

@Tomaage

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

You now have all of the lines of code needed to solve the problem with the withdraw function, and your statements are also in the correct order within the function body to reduce security risk …

• Check inputs (require statements)
• Effects (update the contract state)
• External Interactions (e.g. transfer method)

Just as you have done, it’s important to modify the contract state for the reduction in the individual user’s balance…

balance[msg.sender] -= amount;

before actually transferring the funds out of the contract to the external address…

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

This is to prevent what is known as a re-entrancy attack from occuring after the transfer, but before the user’s individual balance (effectively, the share of the total contract balance they are entitled to withdraw) is reduced to reflect this operation. You’ll learn more about this type of attack, and how the above order of operations helps to prevent it, in the courses which follow this one.

---

Answers to your questions, and additional comments

@Tomaage

1 A Bank that anybody can deposit to, but only I can withdraw from? I kinda like it, but I see why it would not be popular.

@Tomaage

2 So since I was missing the code

balance[msg.sender]-=amount;

in my withdraw function, I was not telling the function to update the smart contracts internal balance of this particular msg.sender . So they could withdraw all the eth stored in the contract. Even eth from other adresses. When one of those adresses came and wanted to withdraw their ETH, the contract balance would be 0, but would their getBalance function still show what they had put in?

Yes, that’s correct. The getBalance() function would return what has been updated in the mapping. This would be the cumulative effect of …

• balance increases for deposits, updated with …
  balance[msg.sender] += msg.value;

   and

• balance increases (recipients) and balance decreases (senders) for internal transfers executed by the transfer() function, and updated with the _transfer() helper function …

    function _transfer(address from, address to, uint amount) private {
        balance[from] -= amount;
        balance[to] += amount;
    }

   but not any

• balance decreases for withdrawals

Basically, the internal accounting function of the balance mapping would no longer work, would no longer provide a true record of each user’s share of the total contract balance, and would no longer serve as a means to prevent users from withdrawing more than they are entitled to.

As time went by and more and more transactions took place, including withdrawals, the difference between (i) the sum of all of the individual user balances in the mapping, and (ii) the total contract balance, would become greater and greater. A situation could quickly arise, where any ether deposited in the contract could be withdrawn by anyone who could “get there first”. As the ether held in the contract is finite, and the contract balance is always correct and accurate, where there were “winners” (“thieves”), there would be equal and opposite “losers” (“victims”).

@Tomaage

That transfer function I have there. It only transfers the value internaly in the smart contract, and updates the internal balance. It does not send it out to that recipients external wallet?

Yes … but it only performs an internal transfer of funds (effectively, a reallocation of funds) between two individual users of the Bank contract. Because no ether is entering or leaving the contract, the net effect to the contract address balance is zero, and the only change is to the individual user balances in the mapping, in order to adjust the amount each of the parties involved in the internal transfer is entitled to withdraw from the contract (their share of the total pooled funds) i.e. the recipient’s entitlement (balance) is increased by the same amount the sender’s entitlement is reduced.

@Tomaage

So this bank only lets you use its internal system to send to somebody else. If they want the money, they neeed to call the bank with the withdrawl function?

Correct … but we could easily add an additional function which allows users to transfer ether (part of their individual share as recorded in the mapping) out of the contract to an external address that is different to their own. I can see you’ve already tried this …

@Tomaage

To make it transfer out of the contract to the recipient external wallet; I would have to make it a payable function?

No… a function only needs to be marked payable when it needs to receive ether from an external address and add it to the contract address balance (e.g. our deposit function). It might help to think of payable as the code that enables msg.value to be added to the contract address balance, without us having to add any further code for this. In the deposit function this happens automatically because we have marked it payable. The line  balance[msg.sender] += msg.value;  then adds the same value to the individual user’s balance in the mapping, in order to keep track of their share of the total funds held in the contract.

Instead, to create a function that allows users to transfer ether out of the contract to an external address that is different to their own, we would just need to make a minor adjustment to the withdraw() function e.g.

function externalTransfer(uint amount, address recipient) public {
   require(balance[msg.sender] >= amount);
   balance[msg.sender] -= amount;
   payable(recipient).transfer(amount);
}

/* Notice, we don't actually need to return a value in either the
   withdraw() function, this new function, or the deposit() function */

If we make a function payable when it doesn’t need to be, and the caller sends an ether value to the function by mistake (as well as calling it with the arguments that it does require), then this ether would be added to the contract balance and potentially lost to the user. So, if a function doesn’t need to receive any ether, it is much securer if we leave it as non-payable, because then, if the caller makes the same mistake, the function will revert, and the ether won’t be sent.

@Tomaage

They would not know that you sent it to them in this contract unless you told them

That’s what events are for. We would have a dapp, with a front-end and a user interface. In other courses you can learn how the Web3 client can use a Web3.js library to listen out for events and retrieve the data. It would then be for the front-end code to handle how the end user is notified of the captured event data that is relevant to them.

@Tomaage

they would not have the money should the smart contract be hacked.

Unfortunately, that’s correct … and why smart contract security is critically important!

As always, just let me know if anything is unclear, or if you have any further questions

[Tomaage]

Thanks @jon_m

Just want to say that feedback like this is super valuable for my understanding . I am totaly new to coding, so lots of basics that I dont have a understanding of yet that slows my progress. But knowing that this kind of help is waiting in the forum is a big boost to my motivation to continue on even though there is a big mountain of stuff infront of me of stuff I dont understand yet, but have to learn if I want to contribute to the decentralised world of web 3.

[jon_m]

Yes, I totally understand!

But you’re asking all the right questions, and they show that you are really wrestling with the new concepts. As a beginner, you may well feel like your progress is slow, but by working through things carefully and methodically, like I can see you are, you’re building the solid foundations which, in the long run, you’ll really benefit from

By the way, just one word of advice. Especially when you’re starting out, no matter how much time you spend trying to understand certain concepts, there’ll always be a few loose ends and further questions that you just can’t seem to resolve. That’s perfectly normal. Some things you won’t fully understand until you have a few more building blocks in place, or have simply spent more time being exposed and working with the code.

Obviously you do need to put the time in asking questions, doing your own research and experimenting with the code, to see if you can resolve things in the here-and-now, but some things you should just make a note of, so that they stay on your radar, and then come back to them later on. You’ll find that some questions get answered indirectly, while you’re working on something else, because you learn about something that was preventing you from answering certain questions before. So it can be a kind of knock-on-effect, which you benefit from by being patient.

Anyway, I hope there’s something there that you find helpful and encouraging.

You’re making great progress!

[seybastiens]

If a variable used/declared in a function is payable, then the function has to be payable too right ?

[seybastiens]

In the Transfer video where you explain about the withdrawal and write :
msg.sender.transfer(amount) ;
The transfer function is a pre-built one right ?

I mean it’s not the function we created earlier in the course that has the same exact name right ?

If it’s a pre-built one, how can we see the exact code behind it ?

[jon_m]

Hi @seybastiens,

@seybastiens

If a variable used/declared in a function is payable, then the function has to be payable too right ?

No … a function is only marked payable when it needs to receive Ether from an external address and add it to the contract address balance e.g. the deposit() function in our Bank contract.

If a function needs to send Ether (but not receive it) then it shouldn’t be marked payable. It is bad practice and poor risk management not to restrict a function to non-payable if it doesn’t need to receive Ether, because it won’t prevent Ether being sent to the function and added to the contract address balance by mistake. The sender could then experience difficulties in getting this Ether refunded, or they may even lose it!

[jon_m]

Hi again @seybastiens,

@seybastiens

msg.sender.transfer(amount) ;
The transfer function is a pre-built one right ?

Yes, it is.

@seybastiens

I mean it’s not the function we created earlier in the course that has the same exact name right ?

No, it’s not.

I think it’s clearer to call this transfer a method rather than a function.

Solidity’s in-built transfer method is called on a payable address. It has the following pre-defined syntax:

<address payable>.transfer(uint amount)

Just as the execution of a function marked with the payable keyword results in the EVM performing low level operations which add any Ether value sent to the function to the contract address balance …
… when Solidity’s in-built transfer method is executed, it results in the EVM performing low level operations which (i) deduct an amount of Ether from the contract address balance which is equivalent to the unsigned-integer argument the transfer method is called with, and (ii) transfer and add this amount of Ether to the external address the transfer method is called on.

Let me know if you have any further questions about this

[seybastiens]

Ok makes more sense thanks.

Is it normal though that I get a :
"TypeError: Type address is not implicitly convertible to expected type address payable. "
on

address payable toSend; 
toSend = msg.sender;

or address payable toSend = msg.sender; ?

It seems like on my end msg.sender isn’t recognized as a payable address by default.

Solution (found on StackExchange):

From Solidity 8.0.0 the transfer structure should be :
payable(msg.sender).transfer(amount);

[jon_m]

@seybastiens

From Solidity 8.0.0 the transfer structure should be :
payable(msg.sender).transfer(amount);

Yes, that’s right

But the syntax change itself doesn’t relate to the transfer method, which has always required a payable address. Instead, it relates specifically to msg.sender, as you first suspected …

@seybastiens

It seems like on my end msg.sender isn’t recognized as a payable address by default.

Prior to Solidity v0.8 msg.sender is already a payable address by default, and so doesn’t need to be explicitly converted whenever the syntax requires it to reference a payable address, such as the address on which the transfer method is called …

// Solidity v0.7
msg.sender.transfer(amount);

… or when we want to assign it to a payable address variable …

// Solidity v0.7
address payable toSend; 
toSend = msg.sender;
toSend.transfer(amount);

// or

address payable toSend = msg.sender;
toSend.transfer(amount);

However, from Solidity v0.8, msg.sender is non-payable by default, which means having to explicity convert it to a payable address when necessary. And this is why we need to modify the code in the above examples as follows …

// Solidity v0.8
payable(msg.sender).transfer(amount);

… or when we want to assign msg.sender to a payable address variable …

// Solidity v0.8
address payable toSend; 
toSend = payable(msg.sender);
toSend.transfer(amount);

// or

address payable toSend = payable(msg.sender);
toSend.transfer(amount);

[patgeek]

hello

how can we know the total amount of wei deposited in the smart contract after
users deposited wei with a deposit function

i mean how can we know the total amount of money a smart contract has

[Gry]

You can see the total amount of money in smart contract by including line “return address(this).balance;” in the smart contract and making getBalance() function for it as below:

function getBalance() public view returns (uint) {

        return address(this).balance;

    }

Also remember to have a mapping tracking addresses / their deposited amounts to balance somewhere

mapping(address => uint) balance;

[patgeek]

i have declared a recipient address as payable ====> address payable _toAddress
i do not understand why this payable address does not receive the amount withdrawed from
the address msg.sender

  function withdraw(uint _amount, address payable _toAddress) public payable returns(uint){
              //  require(balance[msg.sender] >= _amount, "insufficient Balance");

               // uint fromaddressbalanceBeforeWithdraw = balance[msg.sender];
              //  uint toAddressBalanceBeforeWithdraw = balance[_toAddress];

                _toAddress.transfer(_amount);
                msg.sender.transfer(_amount);
                
                  balance[msg.sender] -= _amount;
                  balance[_toAddress] += _amount;

               // assert( balance[_toAddress] == toAddressBalanceBeforeWithdraw + _amount);
              //  assert(balance[msg.sender] == fromaddressbalanceBeforeWithdraw - _amount);

                emit withdrawDone(_amount, msg.sender, _toAddress);


                return msg.value;

        }

[Gry]

I assume you are attempting to spend funds from a wallet to another wallet. If that´s the case, there is no way to program a contract to spend from another wallet.

You can how ever deposit funds to contract with a wallet and withdraw from the contract with another wallet. Remember to deduct balances before doing the transfers/interactions by the way, for security reasons. (to prevent re-entrancy attack)