Transfer Assignment

jon_m · October 14, 2021, 6:31pm

That’s looking good now @mareng91

I’m glad you found the feedback helpful

Just one final comment …

This return statement is correct, and your function does now return a uint value, but do you think returning the same value as the one input into the function is useful? Normally, the output is the result of the computation performed by the function. The result of depositing ether (the input amount) into the contract is the user’s new balance. A user’s individual balance is tracked and stored in the mapping. Can you work out how to reference the user’s updated balance from the mapping, and return that instead? If you need some help, you can always have a look at some of the other students’ solutions posted here in this discussion topic.

Just let me know if you have any questions about this, or any other matter related to the course or the assignments. I know it can be a lot to absorb if you are new to coding
By the way, have you already done the JavaScript Programming course? If you are new to coding, then you should definitely do that course before this one, because it also covers the fundamentals of programming in general. This Solidity course assumes that you are already comfortable with those fundamentals, and builds upon that.

mareng91 · October 14, 2021, 6:58pm

function withdraw(uint amount) public returns (uint) {

  require(balance [msg.sender] >= amount, "Insufficent funds");
  balance[msg.sender] -= amount;
  msg.sender.transfer(amount);
  return balance[msg.sender];
}

mareng91 · October 14, 2021, 7:07pm

I do this on the evenings and construction work on the days + family time with 2 kids.
not an excuse But i don’t get so many hours a week, for now …
I have done the javascript course, but i realise that i might have to backtrack at some point , because i forgot some of it

I really appreciate your feedback , it makes me more avare of the implications of the code i write

jon_m · October 15, 2021, 2:34pm

You’re very welcome Marcus!

Your modified return statement is now much more useful

That’s actually a pretty understandable excuse! … yeh, that definitely makes learning to program smart contracts doubly challenging. Just remember, doing a little but often will be more productive than trying to do too much but infrequently.

That’s good that you’ve done the JavaScript course, and so you’ll definitely be OK to continue with this course. When you’re a beginner, lots of repetition and practice is important and not trying to cover too much too quickly. Going back over the JavaScript course will definitely be helpful. Experimenting with the code from lectures and assignments yourself can also be a very productive learning method. You can try coding as much yourself from memory as possible, and then start reworking, adapting and personalising the same syntax and patterns so that you end up with your own variations on the same theme. When you’re going back over the code, and experimenting with it, really try to actively think about what it’s doing. Also, have a good read of some of the other posts in these forum discussions (for additional information, clarification, confirmation and inspiration), and then ask questions if anything is still unclear.

zlr1984 · October 19, 2021, 8:06am

require(balance [msg.sender] >= amount, “not enough funds”);
msg.sender.transfer (amount);

jon_m · October 19, 2021, 1:00pm

Hi @zlr1984,

The require statement you’ve added is correct, but your withdraw function is missing an essential line of code …

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 address receives the requested withdrawal amount, but their balance in the mapping is not reduced! This means that 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). So, I’m sure you can see why this is a very serious bug!

msg.sender.transfer(amount) transfers ether from the contract address balance to the caller’s external 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.

Notice that the original withdraw function header for this assignment contains returns(uint) . Did you remove it from your solution? It’s not mandatory to include, and the function can still operate effectively without returning a value. But if you’ve included it in the function header, you should also include a return statement in the function body. The compiler would give you a yellow/orange warning about this.

Once you’ve made the above modifications to your code, have a look at this post which explains the importance of the order of the statements within your withdraw function body.

Please, format your code before posting it. This will make it clearer and easier to read. It will also make it easier to copy and paste if we need to deploy and test it.
Follow the instructions in this FAQ: How to post code in the forum

And post your full withdraw function, so we can see whether it is returning a value or not.

Let me know if anything is unclear, or if you have any questions about any of these points

zlr1984 · October 20, 2021, 2:35am

Thanks. I will check that.

josejalapeno · October 22, 2021, 2:49pm

function withdraw(uint amount) public returns(uint){
require(balance[msg.sender] >= amount, “Not Enough Funds”);
msg.sender.transfer(amount);

jon_m · October 22, 2021, 5:53pm

Hi @josejalapeno

The require statement you’ve added is correct, but your withdraw function is missing an essential line of code …

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 address receives the requested withdrawal amount, but their balance in the mapping is not reduced! This means that 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). So, I’m sure you can see why this is a very serious bug!

msg.sender.transfer(amount) transfers ether from the contract address balance to the caller’s external 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.

Notice that the withdraw function header contains returns(uint). It’s not mandatory to include, and the function can still operate effectively without returning a value. But if you’ve included it in the function header, you should also include a return statement in the function body. The compiler will have given you a yellow/orange warning about this.

Once you’ve made the above modifications to your code, have a look at this post which explains the importance of the order of the statements within your withdraw function body.

And don’t forget to format your code before posting it. This will make it clearer, easier to read, and will also help you to spot any syntax errors before posting — for example, you’ve missed a closing curly bracket at the end of your withdraw function. Formatting your code also makes it easier to copy and paste if we need to deploy and test it.
Follow the instructions in this FAQ: How to post code in the forum

Let me know if anything is unclear, or if you have any questions about any of these points

josejalapeno · October 22, 2021, 6:09pm

I saw your answer, its really difficult until right now for me to fully understand everything, I still don’t have any ideas what kind of code should I be putting.

Lennart_Lucas · October 22, 2021, 7:54pm

Add a require and reduce balance:

function withdraw(uint amount) public returns (uint) {
   require(balance[msg.sender] >= amount, "Not enough funds!");
   msg.sender.transfer(amount);
   balance[msg.sender] -= amount;
   return balance[msg.sender];
}

jon_m · October 24, 2021, 3:16am

Hi @josejalapeno,

I’ll try to explain things in a simpler way …

Correct

This line of code only reduces the total contract balance (the total amount of ether held in the contract by all individual users). The caller is the address calling the withdraw function, in order to withdraw the ether amount. In Remix, you can see the caller’s external account balance in the Account field at the top of the Deploy & Run Transactions panel. Think of it as their wallet address. You will see their wallet’s ether balance increase by the withdrawal amount.

We also have to reduce this user’s balance in the mapping by the same amount. Think of the balances in the mapping as recording each user’s individual share of the total funds held in the contract. Your require statement checks the user’s balance in the mapping to see if they have enough to cover the requested amount. If we don’t reduce the user’s balance in the mapping each time they withdraw ether from the contract, then the require statement will no longer prevent the user from withdrawing too much.

// Look at this line in the deposit() function ...
balance[msg.sender] += msg.value;

This declares the data type of the value returned by the function as an unsigned integer (uint). But in order to actually return (output) a value, we need a return statement at the end of the function. The withdraw function can return the user’s reduced balance after the withdrawal, just like the deposit function returns the user’s increased balance after a deposit.

I hope that’s clearer, and easier to understand. If you’re still not sure what code to add to your function, then have a look at some of the other students’ solutions in this discussion topic.

Just let me know if you have any more questions

jon_m · October 24, 2021, 3:22am

Nice solution @Lennart_Lucas

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 withdraw function body.

Just let me know if you have any questions

josejalapeno · October 24, 2021, 1:31pm

Hi Jon, this '>= is the thing I am not understanding, in english this would be my balance as I am the msg.sender should be more than or equal to amount. meaning I should be able to withdraw more than or equal to the amount I deposited. Pls clarify it with me.

jon_m · October 24, 2021, 2:35pm

Hi Jose,

Correct…

Your (the caller’s, msg.sender's) balance is referenced by balance[msg.sender]
This code finds the msg.sender address in the balance mapping (the address is the key) and then references the value that is mapped to this key (the balance).

This is where you are confused. It’s the opposite …

  balance[msg.sender]           >=               amount
/* amount deposited      IS GREATER THAN     amount requested
                           OR EQUAL TO         to withdraw                */

If this is true, then msg.sender will be able to withdraw the amount requested, because their ether balance in the Bank contract is greater than this amount.

But if it is false, then this means their ether balance in the Bank contract is less than the amount requested; so, there is “Not Enough Funds”, require() will fail, and the transaction will revert.

Does that make sense now?

jon_m · October 24, 2021, 2:45pm

… and one further point, which is important for understanding why you need to add an extra line of code…

For the require() statement to work as it should (i.e. check whether msg.sender has enough funds to cover the withdrawal amount) then balance[msg.sender] must always reflect the user’s current balance (their share of the ether deposited in the contract). The user’s balance in the mapping (balance[msg.sender]) will only be accurate if it is always increased when ether is deposited in the contract…

… and decreased when ether is withdrawn from the contract: this is the extra line of code you need to add to your withdraw function.

Markus_Bielaszka · October 28, 2021, 12:39am

function withdraw(uint256 _amount) external {
  require(balance[msg.sender] >= _amount, "You do not have enough funds to withdraw");
        
  balance[msg.sender] -= _amount;
  payable(msg.sender).transfer(_amount);
}

PaulS96 · October 27, 2021, 10:50am

    function withdraw(uint amount) public returns(uint){
        require(balance[msg.sender] >= amount, "Error! Not enough ETH!");
        balance[msg.sender] -= amount;
        msg.sender.transfer(amount);
        return balance[msg.sender];
    }

jon_m · October 27, 2021, 4:19pm

An excellent solution @PaulS96

You have added 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

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…

msg.sender.transfer(amount);

… just in case there is an attack after the transfer, but before the state is modified to reflect this operation. You’ll learn about the type of attack this prevents, and how it does it, in the courses which follow this one. But it’s great you’re already getting into good habits in terms of smart contract security

Keep up the great coding!

MightyYak · October 29, 2021, 6:35am

Here is my go:

function withdraw(uint amount) public {
    require(balance[msg.sender] >= amount, "insufficient funds");
    payable(msg.sender).transfer(amount);
    balance[msg.sender] -= amount;
}

Quick question, does the order of the payable line and internal balance line matter much in production?