Transfer Assignment

madingo · January 4, 2021, 12:59pm

Transfer Assignment.

A built in error handling and mapping to check balance and stop transfer not happening if sender does not have enough funds.

mapping (address ==> uint ) balance ;

function withdraw (uint Amount ) public return (uint){
msg. transfer (amount)
requires (mg.sender = owner)
-----> // run the function

assert (balance[msg.sender] == previous senderBalance - amount);
// event logs and further checks
}

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

jon_m · January 4, 2021, 3:26pm

Hi @deanb3,

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 where I explain an important security modification that should be made to the order of the statements within your withdraw function body.

-= not -+
It’s needed to update the balance mapping, yes, but this updated balance will only be stored on the blockchain if you update it with this line of code.
msg.sender.transfer(amount) transfers the amount from the contract’s ether balance to msg.sender’s address. The new account balances associated with each of these 2 addresses (the contract and the msg.sender), after the transfer, will be updated and stored on the blockchain as a result of this line of code in this transaction. But this line of code will only affect the contract balance, and not the separate user balances which the total contract balance is allocated between. This is why it is vital to also update the balance mapping, so that the contract records which user account the withdrawal came from. If we don’t do that then there is no way of knowing how much each user address can withdraw from the total funds in the contract, because the balance we would be checking in the require statement in the first line wouldn’t have been reduced each time a withdrawal was made (only the total contract balance would have been reduced).

Correct, these are 2 separate functions.
The transfer function we have defined in our contract transfers ether between separate user account balances within the same contract. The overall contract balance remains the same.
<address payable>.transfer(uint256 amount); transfers ether from the contract address to <address payble> . The overall contract balance reduces, but the balance of the individual user account (within this contract) only reduces if we add the line balance[msg.sender] -= amount;

It is a pre-defined Solidity function which can be called on payable addresses, and will transfer ether from the contract it is called within, to the payable address it is appended to.

Correct - it transfers the amount of wei given as the argument.

jon_m · January 4, 2021, 3:33pm

Hi @Caspian1234,

You’ve nearly got it
You are just missing a return statement in the function body.

Also, have a look at this post where I explain an important security modification that should be made to the order of the statements within your withdraw function body.

jon_m · January 4, 2021, 4:15pm

Well done for persevering, researching here in the discussion topic, and for working things through @DUUKA_SMASH

Your final solution includes all of the additional functionality needed to solve the problem with the withdraw function, and you have the different lines in the correct order to reduce the security risk:

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

Exactly! We set the owner variable with the constructor to the address that deploys the contract when the contract is first deployed. If you restrict the withdraw function to onlyOwner, then only that single address can withdraw funds from their own individual balance within the contract. The line of code that prevents each account holder from withdrawing more than they have deposited is balance[msg.sender] -= amount; because this reduces the account balance by the amount withdrawn, so that this reduced balance is the one that is checked in the require statement in the first line to see if the caller still has sufficient funds when they call the function next time. If they try to withdraw more than their remaining balance, then the require statement will revert, which is what you describe here:

Without the return statement at the end of the function body, you would still see a difference in the balance after completing the withdrawal if you call the getBalance function. The withdraw function would still work without both the returns(uint) in the function header and the return statement in the function body. However, it is useful if we want to return this value to use somewhere else in our code. You have to look in the transaction details in the terminal under decoded output to see this return value.

You are right that learning Solidity is challenging, but you’re making great progress! Not everything will click into place immediately — some concepts take longer than others, and will only truely make sense after more practice and experimentation.

jon_m · January 4, 2021, 4:31pm

Hi @RCV,

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?
You are also missing a return statement in the function body.

Once you’ve had a go at making the above modifications, have a look at this post where I explain another important security modification that should be made to the order of the statements within your withdraw function body.

Also, I wouldn’t use “Funds Withdrawn” as your error message when the require function throws. This won’t only throw an error when a user’s funds have all been withdrawn, but also when they have funds but the balance is not enough to cover the amount requested for withdrawal.

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 4, 2021, 5:18pm

Excellent @Depth-Hoar

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.

jon_m · January 4, 2021, 5:24pm

Excellent @CC123

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.

Also, don’t forget the function body’s closing curly bracket

Cristi_Preda-Carp · January 4, 2021, 5:25pm

pragma solidity 0.7.5;

contract EtherBank {

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){
    //check if balance of msg.sender is enough.
    require(balance[msg.sender] >= amount, "Not enough balance!");
    //Adjusting balance
    balance[msg.sender] -= amount;
    //msg.sender is an address and address has a method to transfer.
    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;
}

}

jon_m · January 4, 2021, 6:53pm

Hi @madingo,

You seem to have got quite confused here, so let’s have a look at each of the additional lines of code you should be adding to solve the problem with the withdraw function. But before we do that, you need to review your code carefully because you have lots of syntax errors (spelling mistakes, incorrect operators etc.). Accuracy is important, because the slightest error (e.g. capital letter instead of lower case, = instead of == etc.) will mean your code won’t compile. Before you deploy your code in Remix, you should be checking and correcting any compiler errors. Then, once you have a contract without any compiler errors, which deploys successfully and solves the assignment problem, you can just copy and paste your solution code into the forum text editor and format it using the </> icon in the menu at the top of the text editor. This will wrap your code in 2 sets of 3 back ticks, like this:
```
My code needs formatting
```
Then when your post is displayed, your code will be appear formatted, like this:

My code is now formatted

Syntax Errors in your posted code:

==>
uint Amount
return(uint) in the function header
msg.transfer(amount)
requires(mg.sender = owner) x2 errors: (i) requires (ii) =
previous senderBalance
missing semicolons at the end of most statements (;)
getBalance function opening curly bracket is the wrong way round.

Now let’s look at each of the lines of code you should be adding to the withdraw function:

require statement: If you restrict the withdraw function to owner, then only that single address can withdraw funds from their own individual balance within the contract. The owner address is set by the constructor when the contract is deployed, and will be the address which deploys the contract. Instead, the require statement should do what you have stated here:

So you need a condition in your require statement that checks whether the msg.sender has a balance in the mapping which is greater than or equal to the withdrawal amount requested.

The line of code transferring amount to msg.sender’s wallet address will only reduce the contract’s ether balance as a whole, and not the separate account balance mapped to msg.sender in the mapping. It is vital to also update the balance mapping, so that the contract records which user account the withdrawal came from. If we don’t do that then there is no way of knowing how much each user address can withdraw from the total funds in the contract, because the balance we would be checking in the require statement wouldn’t have been reduced each time a withdrawal was made (only the total contract balance would have been reduced).
You also need a return statement

Once you’ve had a go at making the above modifications, have a look at this post which explains why the order of the statements is important for security. Then make sure the statements in your solution are in the correct order.

Your assert statement is a good idea, but will only work appropriately if you declare a local variable previousSenderBalance within this function, and assign balance[msg.sender] to it before it is reduced by the withdrawal amount. At the moment your assert statement will throw a compiler error because previousSenderBalance doesn’t exist.

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

DUUKA_SMASH · January 5, 2021, 1:01am

Wow thank you so much for the feedback. This is the most challenging thing ive done all year so far…hehe get it…XD. Seriously though thanks brotha and i will try my best to do as much as i can to practice well and in order to take this stuff to the next level someday. Happy New Year!

DUUKA!!

madingo · January 5, 2021, 11:31pm

Hi Jonathan,
Thanks for the clarifications. I am trying to really understand the subject matters. Your advice is so much appreciated and well noted. I actually did not use the compiler as I was trying to do it on paper. I will try and use the compiler next time. I really want to understand this subject thoroughly. Please don’t be surprised if I have to be calling on you as I go along. Sometimes it is of benefits to throw issues to the forum in the hope that someone will give some clarification like the ones you gave to me. Thank you very much. Please let stay in touch.

AaronRene · January 6, 2021, 12:54am

 
    function withdraw(uint amount) public returns(uint){
        require(balance[msg.sender] >= amount, "You do not have enough funds to complete transaction.");
        msg.sender.transfer(amount);
    }

thecil · January 6, 2021, 10:36am

Hi @AaronRene, hope you are good.

You are missing an important part in your function, update the balance, you made the transfer but you never update the balance of the msg.sender so this user could just ask unlimited amount of withdraws.

If you have any more questions, please let us know so we can help you!

Carlos Z.

Rishi_Remesh · January 6, 2021, 6:31pm

pragma solidity 0.7.5; 

contract Bank {
    
    mapping (address => uint) balance; 
    address owner; 
    
    event deposited(uint amount, address depositedTo) ;
    event amountTransferred(uint amount, address toAddress, address fromAddress); //answer
    modifier onlyOwner {
        
        require(msg.sender == owner);
        _; 
    }
    
    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 withdraw(uint amount) public returns (uint) {
        
        require(balance[msg.sender] >= amount);
        
        uint previousSenderBalance = balance[msg.sender];
        msg.sender.transfer(amount);
        balance[msg.sender] -= amount;
        assert(balance[msg.sender] == previousSenderBalance - 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, "Can't transfer money to yourself");
        
        uint previousSenderBalance = balance[msg.sender];
        
        _transfer(msg.sender, recipient, amount);
        
        assert(balance[msg.sender] == previousSenderBalance - amount);
        
        emit amountTransferred(amount, recipient, msg.sender); //answer
        
    }
    
    function _transfer(address from, address to, uint amount) private {
        
        balance[from] -= amount; 
        balance[to] += amount;
        
    }
    
}

Samuel_Carmona · January 7, 2021, 2:56am

function withdraw(uint amount) public returns (uint) {
        // Checks that the balance of the address has sufficient money
        require(balance[msg.sender] >= amount, "Not enough funds!");
        
        // Updates our balance dictionary
        balance[msg.sender] -= amount;
        
        // makes the transfer
        msg.sender.transfer(amount);
        
        return balance[msg.sender];
    }

jon_m · January 7, 2021, 6:22am

Excellent coding @Cristi_Preda-Carp

You have included all of the additional functionality needed to solve the problem with the withdraw function, and you have the different lines in the correct order to reduce the security risk:

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

Keep up the great work!

jon_m · January 7, 2021, 6:44am

An excellent assignment solution @Rishi_Remesh and great coding!

You have added all of the additional lines of code needed to solve the problem with the withdraw function You’ve also successfully coded an event which emits and logs relevant information when a call to the transfer function has executed successfully 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.

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. I think you’ll find it interesting

jon_m · January 7, 2021, 6:48am

Great coding @Samuel_Carmona

You have included all of the additional functionality needed to solve the problem with the withdraw function, and you have the different lines in the correct order to reduce the security risk:

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

Keep up the great work!

derdis14 · January 7, 2021, 12:58pm

function withdraw(uint _amount) public{
	require(balance[msg.sender] >= _amount, “Withdrawal amount exceeds balance”);
	balance[msg.sender] -= _amount;
	msg.sender.transfer(_amount);
}

Jethrodog · January 7, 2021, 6:41pm

Here is the code with my solution:

function withdraw(uint amount) public returns (uint){
    require(balance[msg.sender] >= amount, "Balance not sufficient");
    
    msg.sender.transfer(amount);  // this must include an address
    balance[msg.sender] -= amount;
    
    return balance[msg.sender];
}