Transfer Assignment

[LELUK911]

This is my code.
I use the modifier to control the balance and stop function during execution to avoid re-entry attacks and I updated the receipt before sending Eth from the contract to the user.

// SPDX-License-Identifier: Leluk911
pragma solidity 0.8.7;
contract Bank_Eth {
    // deposit receipt Eth
    mapping(address=>uint) public receipt;
    // variable modifier 
    bool paused=false;
    // balance eth bank
    uint public balance_Bank = address(this).balance;
    // event register transaction
    event Deposit_bank (address indexed,uint indexed);
    //event register withdrawal
    event withdrawal_bank(address indexed,uint indexed);
    
    // stop contract during execution
    modifier stay_paused{
        require (!paused, "contract in exet");
        paused = true;
        _;
        paused = false;
    }
    // control receipt user after withdrawal
    modifier control_receipt(uint _amount){
        require (receipt[msg.sender]>=_amount, "you deposit is less you require");
        _;
    }

    // function deposit in contract
    function depositEth()payable public stay_paused {
        receipt[msg.sender] += msg.value;
        emit Deposit_bank(msg.sender,msg.value);
    }

    // function withdrawal
    function withdrawalEth(uint _amount)public control_receipt(_amount) stay_paused {
        receipt[msg.sender] -= _amount;
        (bool success,) = msg.sender.call{value:_amount}("");
        require(success,"transaction faill");
        emit withdrawal_bank(msg.sender,_amount);
    }



}

[imran82]

pragma solidity 0.7.5;

contract Bank{

mapping(address => uint)balance;

// This function will deposit money to the contract

function deposit() public payable returns(uint){

    balance[msg.sender]+=msg.value;

    return balance[msg.sender];

   

    }

//this function will withdraw the amount from the contract to the address calling this function    

function withdraw(uint amount)  public  {

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

    msg.sender.transfer(amount);

    balance[msg.sender]-=amount;

   

}

//this function will get the balance of address executing this function

function getyourBalance() public view returns(uint){

    return balance[msg.sender];

}

//this function will get you the total balance of ether in your contract

function CheckBalanceofContract() public view returns(uint){

    return address(this).balance;

}

}

[DeMarco_Saunders]

Here’s my answer.

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

[Techman120]

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

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

        msg.sender.transfer(amount);

        balance[msg.sender] -= amount;

    }

[Arch.xyz]

pragma solidity 0.7.5;

contract Bank{

    address owner;

    mapping(address => uint) balance;

    event depositDone(address depositedTo, uint amount);

    event transferred (address indexed from, address indexed to, uint amount);

    event withdrawal(address indexed To, uint amount);

    modifier onlyOwner {

        require(msg.sender == owner);

        _;

    }

   

    constructor(){

        owner = msg.sender;

    }

    function deposit() public payable returns (uint){

        balance[msg.sender] = balance[msg.sender] + msg.value;

        emit depositDone(msg.sender, msg.value);

        return balance[msg.sender];

    }

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

        require(balance[msg.sender] >= _amount, "You dont have enough eth");

        msg.sender.transfer(_amount);

        balance[msg.sender] -= _amount;

        emit withdrawal(msg.sender, _amount);

        return(balance[msg.sender]);

     

    }

    function getBalance() public view returns(uint){

        return balance[msg.sender];

    }

    function transfer(address resipiant, uint amount) public{

        require(balance[msg.sender] >= amount, "ERROR");

        require(msg.sender != resipiant, "ERROR");

        _transfer(msg.sender, resipiant, amount);

        emit transferred(msg.sender, resipiant, amount);

    }

    function _transfer(address from, address to, uint amount) private {

        balance[from] -= amount;

        balance[to] += amount;

    }

}

[jon_m]

Hi @LELUK911,

This is an excellent solution which not only solves the problem with the withdraw() function, but does it by implementing some advanced smart-contract coding techniques

Correctly coded and well implemented modifier with a require() statement to prevent users from withdrawing more than their individual balance.

Correct implementation of the call method, as an alternative to using the transfer method, in order to transfer the withdrawal amount from the contract address balance to the withdrawer’s own external address balance.

You are right that it’s not necessary to return the updated balance, and so returns(uint) can be removed from both the withdrawalEth() and the depositEth() function headers.
However, do you not think the getBalance() function is a more straightforward way for a user to retrieve their current balance, instead of the automatically created getter for the public mapping? The getBalance() function uses msg.sender to avoid having to input an address argument.

Your withdraw event and corresponding emit statement are coded well enough to enable appropriate data to be logged when the withdrawalEth() function is successfully executed. The emit statement is also in the correct position in the function body.
However, you haven’t named the parameters in either of the event declarations (for both the depositEth and the withdrawalEth functions). Naming the event data isn’t mandatory, and both data values will still be emitted and logged for both events; but it will make the data clearer and easier to identify, and especially in situations where you have more than one parameter with the same data type.

@LELUK911

I updated the receipt before sending Eth from the contract to the user.

Yes … your withdrawalEth() function code ensures that each operation is executed in the correct order to reduce security risk:

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

Modifying the contract state for the reduction in the individual user’s balance before actually transferring the funds out of the contract to the external address helps to prevent re-entrancy attacks from occuring. In fact, in terms of protection against re-entrancy attacks, your stay_paused modifier reinforces your contract’s security even further

---

@LELUK911

uint public balance_Bank = address(this).balance;

The automatic getter created for this public state variable will not retrieve the current contract balance, because it is assigned to the state variable on deployment (when the contract balance is zero) and then never updated. So, the balance_Bank getter will always return zero. Instead, you should add a getter to your contract which returns the contract balance at the time the getter is called e.g.

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

---

By the way, don’t forget to do the Events Assignment and post your solution here. This is the assignment from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course, and it provides useful practice at coding events.

Just let me know if you have any questions about anything

[jon_m]

Hi @imran82,

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

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 a user can call separately to consult their own individual updated balance after a withdrawal.

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.

By the way, don’t forget to do the Events Assignment and post your solution here. This is the assignment from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course, and it provides useful practice at coding events.

Just let me know if you have any questions

[jon_m]

Hi @DeMarco_Saunders,

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 (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…

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. But it’s great you’re already getting into good habits in terms of smart contract security

It’s just a shame you omitted the function body’s closing curly bracket

Don’t forget to post your solution to the Events Assignment here. This is the assignment from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course.

Let me know if you have any questions.

[jon_m]

Hi @Techman120,

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

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 should also include a return statement at the end of the function body. The compiler should have given you an orange warning for this.

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.

Let me know if you have any questions.

[jon_m]

Nice solution @Arch.xyz

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

The withdrawal and transfer events you’ve also added, and their corresponding emit statements, are correct and well coded. Both emit statements are correctly positioned within their respective function bodies and will log appropriate data when these functions are successfully executed

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.

---

A couple of other minor observations …

@Arch.xyz

return(balance[msg.sender]);

If you are only returning one value, you don’t need to enclose it within parentheses in the return statement. You only need the parentheses if you are returning more than one value within the same statement, each separated by a comma.

@Arch.xyz

event withdrawal(address indexed To, uint amount);

Keep your use of upper v lower case consistent for the first letter of names/identifiers. The convention is to start the names of structs, events, contracts and interfaces with a capital letter, and the names of variables (including arrays and mappings), functions, modifiers, parameters, arguments and return values with a lower case letter. Keeping to this convention, or at least being consistent across all of the smart contracts in the same project — so that, for example, you don’t do what you’ve done in the event declaration above and start one parameter name with a capital letter and another with a lower case letter — makes your code more readable and developer-friendly.

Just let me know if you have any questions

[LELUK911]

hi @jon_m
, thank for your response.
I will study the arguments better that you have me see for correct my error in next code…

[Techman120]

Thanks for the feedback!

function withdraw(uint amount) public {

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

        balance[msg.sender] -= amount;

        msg.sender.transfer(amount);

    }

[Wyse_Sousa]

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

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

        balance[msg.sender] -= amount;

        return balance[msg.sender];

[Arch.xyz]

thank you for the reply and ive taken note, remix did a good job in keeping me in check when i made similar mistake on the project.

[Arch.xyz]

Corrected

    }

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

        require(balance[msg.sender] >= _amount, "You dont have enough eth");

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

        emit withdrawal(msg.sender, _amount);

        return(balance[msg.sender]);

[jon_m]

Hi @Wyse_Sousa,

You have added the additional lines of code needed to solve the problem with the withdraw function, but you’ve left out the following statement which was in the code you were given to start with …

msg.sender.transfer(amount);

If you deploy your contract, make a deposit, call your withdraw function as it is, and then call getBalance() with the same withdrawer’s address, you will notice that the caller’s individual balance in the mapping is reduced by the requested withdrawal amount, but their external address does not receive the Ether. If the function is coded correctly, you should see the withdrawer’s external address balance (showing in the Account field near the top of the Deploy & Run Transactions panel in Remix) increase by this amount.

Instead, each time your withdraw() function is called, the caller is effectively losing ownership of the amount of Ether they have requested to withdraw. Essentially, the smart contract has been coded to steal users’ funds, so I’m sure you can see why this is a very serious bug!

The individual user balances in the mapping 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). However, reducing a user’s individual balance in the mapping for a specific amount doesn’t actually transfer an equivalent amount of Ether from the contract address balance to the user’s external address balance: that’s what the transfer method does …

<payable address>.transfer(uint amount);

Once you’ve included the missing transfer method, have a look at this post to make sure the order of your statements within the withdraw() function body ensures your smart contract is protected against re-entrancy attacks.

Let me know if anything is unclear, or if you have any questions

[Wyse_Sousa]

Once again Thank you for the explanation!
I saw the errors in the video and understood the mistakes.

[Liquidbaker89]

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

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

balance[msg.sender] -= amount;

msg.sender.transfer(amount);

return balance[msg.sender];

}

[Giupi]

Hi everyone, I thought that it’d be pretty straightforward to have a require statement, to make sure that the balance is higher or equal than the balance to withdraw, then adjust the balance accordingly.

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

[MichaelAudire]

My withdraw function solution so that an account may not withdraw funds that is not theirs.

pragma solidity 0.7.5;

contract Bank {

    mapping(address => uint) balance;
    address owner;
    

    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    modifier insureAccFunds(uint _amount) {
        require(balance[msg.sender] >= _amount, "Insufficient funds.");
        _;
    }

    event fundsAdded(uint amount, address depoAddy);
    event fundsTransferred(address indexed fromAccount, address indexed toAccount, uint _amount);

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

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

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


    function transfer(address recipient, uint amount) public onlyOwner insureAccFunds(amount) {
        require(msg.sender != recipient, "(recipient != msg.sender)");
        uint balanceBeforeTransfer = balance[msg.sender];
        _transfer(msg.sender, recipient, amount);
        assert(balance[msg.sender] == balanceBeforeTransfer - amount);
        emit fundsTransferred(msg.sender, recipient, amount);

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


}