Solidity Payable Functions - Discussion

Ethereum Smart Contract Programming 101
#299

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 


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.

1 Like
#300

Thanks

The

made it work.

1 Like
#301

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?

image
image1250×1007 110 KB

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;
    }
}
#302

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


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

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

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.

#303

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

1 Like
#304

Hi @Tomaage,

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 :muscle: 


  • 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

:money_mouth_face:

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”).

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.

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 


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.

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.

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 :muscle:

2 Likes
#305

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.

1 Like
#306

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 :muscle:

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! :smiley:

1 Like
#307

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

#308

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 ?

1 Like
#309

Hi @seybastiens,

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!

1 Like
#310

Hi again @seybastiens,

Yes, it is.

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 :slight_smile:

1 Like
#311

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);

1 Like
#312

Yes, that’s right :+1:

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 


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);
1 Like
#313

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

#314

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;

1 Like
#315

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;

        }
#316

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)

#317

i have tried the following, and i manage to send money from one account to another

function send(address payable _receiver) public payable {
                        _receiver.send(msg.value);
                }
1 Like
#318

You can also just do the following, keep in mind that from solidity version 0.8, you have to convert an address into payable to be able to transfer to it.

function send(address _receiver) public payable {
                        payable(_receiver).transfer(msg.value);
                }

Carlos Z