Transfer Assignment

Rishi_Remesh · January 7, 2021, 9:01pm

Thanks for the insight @jon_m! I’ll keep this in mind in future assignments. Change state of the contract before an operation.

cincinnato · January 8, 2021, 9:32pm

function withdraw(uint amount) public returns (uint) {
require(balance[msg.sender] >= amount, “Not enough funds. Please amend your amount”);
msg.sender.transfer(amount);
}

user.balance = balance;

gwen · January 10, 2021, 10:42pm

Edited after jon_m feedback

 function withdraw(uint amount) public returns (uint){
        //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 it’s been included in the function header -> include a return statement in the function body.
        //msg.sender is an address and payable
        
        //Option 1: send to other address
        
        //address payable toSend = '0x5B38Da6a701c568545dCfcB03FcB875f56beddC4' // any other addresses
        //toSend.transfer(amount);
        
        //Option 2: withdraw and sent to onlyOwner 
        require (balance[msg.sender] >= amount, "Balance not sufficient");
        balance[msg.sender] -= amount;
        //it’s important to modify the contract state before you actually perform the transfer of funds out of the contract,
        //just in case there is an attack after the transfer, but before the state is modified to reflect this operation. 
        
        msg.sender.transfer(amount); //transfer is built-in method in msg.sender, safer to do this because it has built-in error handling
        return balance[msg.sender];
        
    }

bjamRez · January 9, 2021, 10:01pm

pragma solidity 0.7.5;

// FIX withdraw CODE SO THAT IT DOES NOT ALLOW USER TO withdraw MORE THAN balance AND IT UPDATES THE balance

contract myBank {

mapping(address => uint) balance;

address owner;

event depositDone(uint amount, address depositedTo);
event amountTransferred(uint amount, address depositedTo, address depositedFrom, uint balance); // 1/9/2021 : added balance and changed event name // MY TRANSFER function 


modifier onlyOwner { // break out function: modifier for require function; to check for owner == owner status
    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 getBalance() public view returns (uint) {
    
    return balance[msg.sender];
}

//=====================withdraw workplace========================================

function withdraw(uint amount) public returns (uint) {
    // 1. check balance of owner - 2. update owner balance after withdraw
     
    // if withdraw amount > than owner balance; do not allow
    // ...check balance mapping and error-handling functions
    
    require(balance[msg.sender] >= amount, "balance not sufficient"); // checks (require statement)  //////// re-order of withdraw functions
    balance[msg.sender] -= amount; // effects (update the contract state for reduction in balance)
    msg.sender.transfer(amount); // withdraw function has a built-in error-handling function, like "require" does.
   
    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 to yourself");
    
    uint previousSenderBalance = balance[msg.sender];
    
    _transfer(msg.sender, recipient, amount);
    emit amountTransferred(amount, recipient, msg.sender, balance[msg.sender]); // added balance to msg.sender after transfer // HERE IS MY ANSWER
    
    assert (balance[msg.sender] == previousSenderBalance - amount);
    
}

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

}

MindFrac · January 9, 2021, 2:48am

I added 2 lines of code to the withdraw function and it seems to work:

    function withdraw(uint amount) public returns (uint) {
        require(balance[msg.sender] >= amount, "Balance not sufficient.");
        msg.sender.transfer(amount);
        return balance[msg.sender];
    }

MindFrac · January 9, 2021, 2:53am

Oops looks like I’m missing this line of code:

balance[msg.sender] -= amount;

zai · January 9, 2021, 7:30am

function withdraw(uint amount) public returns (uint){
        require(balance[msg.sender] >= amount, "Insufficient balance."); //to make sure msg.sender has enough balance
        balance[msg.sender] -= amount; //deducting the amount to be withdrawn
        msg.sender.transfer(amount); //code that does the withdrawal or transfer
        return balance[msg.sender]; // returning the updated balance
    }

jon_m · January 9, 2021, 8:31am

Glad it was helpful @Rishi_Remesh

… before transacting externally (with an external smart contract or other external address), yes, and also this applies to value transactions i.e. involving transfers of funds (ether or other tokens).

jon_m · January 9, 2021, 8:49am

Hi @cincinnato,

The require statement you’ve added is correct but you are missing 2 other important lines of code:

When you deploy your contract and call your withdraw function as it is, you will notice that the sender’s address receives the requested withdrawal amount, but their balance in the mapping is not reduced! I’m sure you can see that this is another very serious bug! How would you correct this? If you can’t work this out yourself, you’ll find the answer by taking a look at some of the other posts in this topic.
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 it’s been included in the function header, you also need to include a return statement in the function body.

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

Also, I think you’ve included the following line by mistake

Let us know if you have any questions about how to correct this, or if there is anything that you don’t understand

jon_m · January 9, 2021, 8:53am

Hi @gwen,

The require statement you’ve added is correct but you are missing 2 other important lines of code:

When you deploy your contract and call your withdraw function as it is, you will notice that the sender’s address receives the requested withdrawal amount, but their balance in the mapping is not reduced! I’m sure you can see that this is another very serious bug! How would you correct this? If you can’t work this out yourself, you’ll find the answer by taking a look at some of the other posts in this topic.
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 it’s been included in the function header, you also need to include a return statement in the function body.

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

Let us know if you have any questions about how to correct this, or if there is anything that you don’t understand

Mihir23 · January 9, 2021, 9:28am

pragma solidity 0.7.5;

contract Bank {
    
   mapping(address => uint) balance;
   address owner;
   
   event deposited(uint amount, address indexed depositedTo);
   event transferProcess(address sender, address reciever, uint amount);
   
   modifier onlyOwner {
       require(msg.sender == owner);
       _; // run the function
   }
   modifier costs(uint price){
       require(msg.value >= price);
       _; // run the function
   }
   constructor(){
       owner = msg.sender;
   }
   
   function deposit() public payable returns (uint) {
       
       balance[msg.sender] += msg.value;
       emit deposited(msg.value, msg.sender);
       return balance[msg.sender];
       
   }
   function getBalance() public view returns (uint){
       return balance[msg.sender];
   }
   
   function withdraw(uint amount) public returns (uint) {
       require(balance[msg.sender] >= amount, "You cannot overdraw.");
       
       balance[msg.sender] -= amount;
       
       msg.sender.transfer(amount);
       return balance[msg.sender];
   }
   
   function transfer(address recipient, uint amount) public {
       require(balance[msg.sender] >= amount, "Insufficient balance!");
       require(msg.sender != recipient, "Don't transfer money to yourself");
       
       uint previousSenderBalance = balance[msg.sender];
       
       _transfer(msg.sender, recipient, amount);
       emit transferProcess(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 · January 9, 2021, 11:24am

Hi @bjamRez,

You have included all of the additional lines of code needed to solve the problem with the withdraw function, but you need to have them in a different order:

checks (require statement)
effects (update the contract state for reduction in balance)
external interactions (perform the transfer of funds from the smart contract address to the external wallet address).

In terms of the require statement, this needs to come first, because it is checking that the input is valid. While it is true that, if require() is placed later in the function body, all previous operations will still be reverted if it throws, this will cost more gas, because only the remaining gas after require() has thrown will be refunded, and not the gas consumed for executing require() itself and any code executed previously.

Have a look at this post which explains the importance of the order of the other statements within your withdraw function body.

Your Transfer event is well coded, and successfully emits the event upon transfer. The only additional piece of data that I think would also be useful to include in this event is the new balance of msg.sender after the transfer. Perhaps amountTransferred would also be a better name than transferHistory for this event, because each event emitted is only associated with a single transfer (although you are correct that looking at the log of all transfer events emitted will provide us with a transfer history).

You’re making great progress! Keep it up

FredrikLarsson · January 9, 2021, 11:11am

pragma solidity 0.7.5;

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

jon_m · January 9, 2021, 11:32am

Nice self-correction @MindFrac

Where exactly would you place this additional line of code within the function body? The order is important. Do you know why? If not, then have a look at this post which explains the importance of the order of the statements within your withdraw function body.

jon_m · January 9, 2021, 11:54am

Great coding @zai
…and the comments you’ve added are also a very good description of what the different statements do.

You have included all of the additional lines of code needed to solve the problem with the withdraw function, and you have them all in an order that optimises security

checks (require statement)
effects (update the contract state for reduction in balance)
external interactions (perform the transfer of funds from the smart contract address to the external wallet address).

Haysoose · January 9, 2021, 8:10pm

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, "Must Be 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, "Insufficient Balance");
    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, "Insufficient Balance");
    require(msg.sender != recipient, "Cannot Send To Self");
    
    uint prevBalance = balance[msg.sender];
    
    _transfer(msg.sender, recipient, amount);
    
    assert(balance[msg.sender] == prevBalance - amount);
}

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

}

Haysoose · January 9, 2021, 8:12pm

I didn’t subtract the balance. Corrected:

function withdraw(uint amount) public returns (uint){
require(balance[msg.sender] >= amount, “Insufficient Balance”);
balance[msg.sender] -= amount;
msg.sender.transfer(amount);
return balance[msg.sender];
}

bjamRez · January 9, 2021, 9:35pm

thanks jon - I will look into this re-ordering - thanks again

bjamRez · January 9, 2021, 10:03pm

I just updated my code

jon_m · January 9, 2021, 10:45pm

Well spotted @Haysoose

You now have all of the additional functionality needed to solve the problem with the withdraw function, and you also have the different lines in the correct order for optimal security:

check inputs (require statement)
effects (update the contract state for reduction in balance)
external interactions (perform the transfer of funds from the smart contract address to the external wallet address).

Keep up the great work!