Transfer Assignment

fedev · January 17, 2022, 4:37pm

Hey! You’re definitely right. Also while attending the 201 course and coming across the EIP20 proposal I’ve found that both txs with amount > balance should throw AND txs with amount == 0 should not throw and emit standard txs.
I will obviously stick to the standards even if the second clause looks a bit strange/waste of gas but maybe there was a reason at that time to ask complaiance to not throw on transfer 0 amounts.

diehlca17 · January 21, 2022, 3:23am

This problem can be easily fixed by setting a requirement that the balance is larger than the amount we are withdrawing. It returns the balance after the transfer.

   function withdraw(uint amount) public returns (uint) {
        require(balance[msg.sender] >= amount, "cannot withdraw more than your balance");
        balance[msg.sender] -= amount;
        msg.sender.transfer(amount);
        return balance[msg.sender];
    }

jon_m · January 22, 2022, 1:22am

An excellent solution @diehlca17

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

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

Oprea · January 24, 2022, 2:39pm

Hello! This is my solution:
` function withdraw(uint amount) public returns(uint){

require(balance[msg.sender] >= amount, "Not enough balance");
balance[msg.sender] -= amount;
msg.sender.transfer(amount);
emit withdrawedBalance(amount, msg.sender);
return balance[msg.sender];

}`

0xsundown · January 25, 2022, 7:28pm

Hello! This is what I came up with for my solution.

function withdraw(uint amount) public returns (uint){
        //Check balance
        require(balance[msg.sender] >= amount, "Insufficient funds");
        //Withdraw amount from balance
        msg.sender.transfer(amount);
        //Deduct amount from balance
        balance[msg.sender] -= amount;
        
        return balance[msg.sender];
    }```

jon_m · January 26, 2022, 4:04am

An excellent solution @Oprea

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…

before actually transferring the funds out of the contract to the external address…

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

Your emit statement also looks good, and it’s in the correct position within the withdraw() function body. However, to be sure the code is correct, we need to see the corresponding event declaration you’ve added. Can you post that as well so we can see the complete picture?

Also, 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.

Just let me know if you have any questions

jon_m · January 26, 2022, 4:43am

Nice solution @0xsundown

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.

Once you’ve read the linked post, my following comments (about two of your inline comments) should also make sense …

This line of code withdraws the amount from the contract address balance and transfers it to the external address which the transfer method is called on (in this case, msg.sender, the address calling the withdraw function). The contract’s Ether balance is reduced by the withdrawal amount, and the external address’s ether balance is increased by the same amount.

This line of code reduces the individual user’s balance in the mapping to account for the fact that it’s this particular user who is withdrawing this amount from the contract.
msg.sender.transfer(amount) 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, effectively, the amount each is entitled to withdraw from the contract.

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

simfin94 · January 28, 2022, 8:16pm

pragma solidity 0.7.5;

contract Bank {

    mapping(address => uint) balance;

    address owner;

    event depositDone(uint amount, address indexed depositedTo);

    event transferDone(uint amount, address toAddress, address fromAddress);

    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(amount <= balance[msg.sender]);
        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);

        assert(balance[msg.sender] == previousSenderBalance - amount);

        emit transferDone(amount, recipient, msg.sender);
    }

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


}

jon_m · January 30, 2022, 3:47am

Hi @simfin94,

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 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 in the function body. The compiler will have given you an 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.

Your transfer event and corresponding emit statement (from the previous Events Assignment) are both correct and well coded. The emit statement is in the correct position in the transfer() function body and will log appropriate data when the transfer() function is successfully executed

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

LELUK911 · February 4, 2022, 6:31pm

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 · February 5, 2022, 12:05pm

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 · February 6, 2022, 1:37pm

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 · February 6, 2022, 4:37pm

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 · February 8, 2022, 4:07pm

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 · February 8, 2022, 9:43pm

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.

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

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 · February 8, 2022, 10:06pm

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 · February 8, 2022, 10:17pm

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 · February 8, 2022, 10:22pm

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 · February 8, 2022, 11:35pm

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 …

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.

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 · February 9, 2022, 4:46am

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…