Transfer Assignment

[Aiden_Calhoun]

Transfer Assignment

pragma solidity 0.7.5;

contract bank{
    
    mapping(address => uint) balance;
    
    address owner;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    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){
        //first - I set a requirement for the balance of the sender to be greater than or equal to the amount to be withdrawn
        require(balance[msg.sender] >= amount, "Not enough funds!");
        msg.sender.transfer(amount);
        //second - this function adjusts the balance by the amount withdrawn.
        balance[msg.sender] -= 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 @Aiden_Calhoun,

You have added all of the essential code needed to solve the problem with the withdraw function

However, 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 as you’ve included it in your function header, you should also include a return statement in your function body. You should have got a yellow/orange compiler warning about this.

Also, have a look at this post which explains the importance of the order of the statements within your withdraw function body.

Just let me know if you have any questions

[Aiden_Calhoun]

Hi @jon_m,

thanks for the reply! After I submitted my code, I looked through other answers on here to see what they came up with and noticed that I was missing that returns(uint) function! I should pay more careful attention next time haha!

I’ll give that post a read now!

Thank you!

[kylecullen]

Error handling require function to get msg.sender balance and compare to Withdrawl amount, must be >=.

Update msg.sender balance -=amount

pragma solidity 0.7.5;

contract Bank {
    
    mapping(address => uint) balance;
    
    address owner;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    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) {
        require(balance[msg.sender] >= amount, "Balance not sufficient");
        balance[msg.sender] -= amount;
        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, "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;
    }

}

[Mike_Power]

Transfer Assignment

pragma solidity 0.7.5;

contract Bank {
    
    mapping(address => uint) balance;
    
    address owner;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    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) {
        require(balance[msg.sender] >= amount);
        balance[msg.sender] -= amount;
        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);
        require(msg.sender != recipient);
        
        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;
    }

}

[HRMS2021]

pragma solidity 0.7.5;

contract Bank {
    
    mapping(address => uint) balance;
    
    address owner;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    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) {
        require(balance[msg.sender] >= amount);
        balance[msg.sender] -= amount;
        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]

An excellent solution @kylecullen

You have added both of the essential 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:

1. check inputs (require statements)
2. effects (update the contract state)
3. 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 wallet 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!

[jon_m]

An excellent solution @Mike_Power
… and it’s good to see you back here in the forum!

You have added both of the essential 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:

1. check inputs (require statements)
2. effects (update the contract state)
3. 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 wallet 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

[jon_m]

An excellent solution @HRMS2021

You have added both of the essential 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:

1. check inputs (require statements)
2. effects (update the contract state)
3. 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 wallet 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!

[EduardoDaSilva]

pragma solidity 0.7.5;
contract MemoryAndStorage {

    mapping(address => uint) balance;

    event transferedTo(address, uint);

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

    function withdraw(uint amount) public returns(uint) {
        require(balance[msg.sender] > amount);
        msg.sender.transfer(amount);
        balance[msg.sender] -= amount;
        emit transferedTo(msg.sender, amount);
        return balance[msg.sender];
    }
    
    function balanceOf() public view returns (uint) {
        return balance[msg.sender];
    }

}

[jon_m]

Hi @EduardoDaSilva,

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

Notice, however, that by using the > instead of the >= operator in your require statement, the user will never be able to withdraw their total balance from the bank contract.

@EduardoDaSilva

require(balance[msg.sender] > amount);

Your event and corresponding emit statement are both quite well coded. The emit statement is in the correct position within your withdraw function body, and will log appropriate data when the function is successfully executed. But you haven’t given the parameters names in the event declaration. You cannot search or listen for emitted event data by these argument names, unless they are indexed, but I still think they are important to include so that it’s clear what the values themselves represent.
And just one other consideration about the event name transferedTo… when you include the transfer() function as well as deposit() and withdraw(), do you think this event name for a withdrawal may cause confusion?

Your concise implementation of the deposit() function — adding the deposited amount to the user’s individual balance in the mapping, and returning the new balance, all in one single return statement — is excellent. And you are right that a similar solution in the withdraw function (reducing instead of increasing the user’s balance) is not appropriate. However, this isn’t only because of the emit statement: take 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

[EduardoDaSilva]

Thanks Jon for your considerations. They’re very helpful.
Follow the updated code with all corrections.

pragma solidity 0.7.5;
contract MemoryAndStorage {

    mapping(address => uint) balance;

    event withdrawDone(address destination, uint value);

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

    function withdraw(uint amount) public returns(uint) {
        require(balance[msg.sender] >= amount);
        balance[msg.sender] -= amount;
        msg.sender.transfer(amount);
        emit withdrawDone(msg.sender, amount);
        return balance[msg.sender];
    }
    
    function balanceOf() public view returns (uint) {
        return balance[msg.sender];
    }

}

[vale_dinap]

Here is my code - apologies for using a different version of the compiler:

pragma solidity ^0.8.7;

contract Bank{
    
    // state variables
    mapping (address=>uint) balance;
    address owner;
    
    // constructor
    constructor(){
        owner = msg.sender;
    }
    
    // modifiers
    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    // events
    event depositDone(uint indexed amount, address indexed depositedTo); //deposit
    event withdrawDone(uint amount, address withdrawnFrom); //withdraw
    
    // functions
    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){
        require(amount<=balance[msg.sender], "The amount requested to withdraw exceeds the available balance");
        uint balanceBefore = balance[msg.sender]; //store original balance
        payable(msg.sender).transfer(amount); //transfer ether
        balance[msg.sender]-=amount; //update balance
        emit withdrawDone(amount, msg.sender);
        assert(balanceBefore - amount == getBalance());
        return balance[msg.sender]; //update balance;
    }
    
    function addBalance(uint _toAdd) public onlyOwner returns (uint){
        balance[msg.sender]+=_toAdd;
        return balance[msg.sender];
    }
    
    function getBalance() public view returns (uint){
        return balance[msg.sender];
    }

[jon_m]

Hi @vale_dinap,

@vale_dinap

apologies for using a different version of the compiler

It’s perfectly OK to use v0.8, as long as you take into consideration any changes to the syntax — which you have Prior to v0.8 msg.sender is payable by default and so doesn’t need to be explicitly converted when transfer() is called on it. That’s why the course video uses   msg.sender.transfer(amount);   without the compiler throwing an error. 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, which you have done correctly using payable() .

---

You have added the essential lines of code needed to solve the problem with the withdraw function, and your additional assert statement is also appropriate and well coded.

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.

---

Your withdraw event and corresponding emit statement are both well coded, and the emit statement will log relevant information when the function has executed successfully. I would just add, though, that in general, emit statements are probably better placed after assert statements, rather than before.

---

@vale_dinap

return balance[msg.sender]; //update balance;

Note that this statement is returning / outputting the user’s updated balance from the function, not updating it. You have already correctly identified the statement which updates the user’s individual balance …

@vale_dinap

balance[msg.sender]-=amount; //update balance

---

Did you mean to include the addBalance() function as well as the deposit() function, or did you leave this in by mistake? If the owner can increase their own individual balance by any amount they wish, without depositing an equivalent amount of ether into the contract, then this effectively means that they can withdraw other users’ funds! I’m sure you agree that this wouldn’t inspire much user confidence in the contract

Just let me know if you have any questions

[Shadowstep2003]

pragma solidity 0.7.5;

contract Bank {
    
 mapping (address => uint) balance; 
 address owner; 
 
 event depositDone(uint amount, address depositedTo);
 event transferSent(uint amount, address sentFrom, address sentTo);

 // When placed in a function this will run first
 modifier onlyOwner {
     require(msg.sender == owner);
     _; // run the function that called this
 }
 
 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 {
     //msg.sender is an address
     require(balance[msg.sender] >= amount, "Insufficient amount to transfer");
     balance[msg.sender] -= amount;
     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, "Don't transfer money to yourself");
     
     uint previousSenderBalance = balance[msg.sender];
     
     _transfer(msg.sender, recipient, amount);
     
     emit transferSent(amount, msg.sender, recipient);
     
     assert(balance[msg.sender] == previousSenderBalance - amount);
     
 }
 
 function _transfer(address from, address to, uint amount) private {
     balance[from] -= amount;
     balance[to] += amount;
 }
 
 
}

[alp257]

function withdraw(uint amountToWithdraw) public returns(uint) {
    require(balance[msg.sender] >= amountToWithdraw, "Balance is not sufficient");

    msg.sender.transfer(amountToWithdraw);

    balance[msg.sender] -= amountToWithdraw;
    
    return balance[msg.sender]; 
}

[jon_m]

An excellent solution @Shadowstep2003

You have added both of the essential 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:

1. check inputs (require statements)
2. effects (update the contract state)
3. 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

You are also right that it’s not necessary to return the updated balance, and so  returns(uint)  can be removed from the function header. It is useful information to return, but we do already have the getBalance function which can be called separately to consult the individual account holder’s updated balance after the withdrawal.

Just one observation …

@Shadowstep2003

require(balance[msg.sender] >= amount, "Insufficient amount to transfer");

Is it the amount that is insufficient, or the balance? I would probably keep the error message in this require statement the same as the one in the transfer() function, because they both perform exactly the same check …

@Shadowstep2003

require(balance[msg.sender] >= amount, "Balance not sufficient");

And recognising the fact that both require statements perform the same check, means that we can avoid code duplication by placing just one require statement within a modifier, and applying this single modifier to both functions instead.

Just let me know if you have any questions

[jon_m]

Nice solution @alp257

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

[CryptoByNight]

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

I thought the .transfer function was a potential security risk. Are safer ways for practical smart contract deployment explained in future sections?

[vale_dinap]

Hi Jon,
I do hugely appreciate your answer!

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.

Now that you mentioned it and after reading your post I took a moment to think about it and it makes a lot of sense.

Your withdraw event and corresponding emit statement are both well coded, and the emit statement will log relevant information when the function has executed successfully. I would just add, though, that in general, emit statements are probably better placed after assert statements, rather than before.

Same here, I now have a much more clear statements order in mind.

Note that this statement is returning / outputting the user’s updated balance from the function, not updating it. You have already correctly identified the statement which updates the user’s individual balance …

My apologies, it was actually a typo, I meant to write “updateD”

Question: in other object-oriented languages (Python, C++, etc.) I have the habit of including (almost) always a return statement at the end of every function/method in order to maintain a solid I/O flow structure and for future utility, regardless if I already need it: is that a bad practice in solidity? For example, does it make the contract less clean or more expensive to deploy/run?

Did you mean to include the addBalance() function as well as the deposit() function, or did you leave this in by mistake? If the owner can increase their own individual balance by any amount they wish, without depositing an equivalent amount of ether into the contract, then this effectively means that they can withdraw other users’ funds! I’m sure you agree that this wouldn’t inspire much user confidence in the contract

Forgot to remove it from the code (I started from a recycled a piece of code I wrote while following the lessons, sorry about that).

Here is the amended code with the corrections we discussed:

pragma solidity ^0.8.7;

contract Bank{
    
    // state variables
    mapping (address=>uint) balance;
    address owner;
    
    // constructor
    constructor(){
        owner = msg.sender;
    }
    
    // modifiers
    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    // events
    event depositDone(uint indexed amount, address indexed depositedTo); //deposit
    event withdrawDone(uint amount, address withdrawnFrom); //withdraw
    
    // functions
    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){
        require(amount<=balance[msg.sender], "The amount requested to withdraw exceeds the available balance");
        uint balanceBefore = balance[msg.sender]; //store original balance for later safety checks
        balance[msg.sender]-=amount; //update balance, BEFORE actually transfering ETH in order to avoid attacks
        payable(msg.sender).transfer(amount); //transfer ether
        assert(balanceBefore - amount == getBalance()); // ensure that the actual balance corresponds to the operation performed
        emit withdrawDone(amount, msg.sender); //withdraw event logging
        return balance[msg.sender]; //updateD balance;
    }
    
    function getBalance() public view returns (uint){
        return balance[msg.sender];
    }
}

Many thanks for taking time to carefully review my code and answering to all of my questions!